-
Notifications
You must be signed in to change notification settings - Fork 162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
let linux/docker controller/router address be IPv4/IPv6 #2438
base: main
Are you sure you want to change the base?
Conversation
@@ -71,14 +73,22 @@ issueLeafCerts() { | |||
ZITI_PKI_CTRL_SERVER_CERT="${ZITI_PKI_ROOT}/${ZITI_INTERMEDIATE_FILE}/certs/${ZITI_SERVER_FILE}.chain.pem" | |||
if [[ "${ZITI_AUTO_RENEW_CERTS}" == true || ! -s "$ZITI_PKI_CTRL_SERVER_CERT" ]]; then | |||
# server cert | |||
if [[ "${ZITI_CTRL_ADVERTISED_ADDRESS}" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's better to not try to make these sorts of determiniations and instead have the user not mix/match between IP and DNS. Also this won't work proplery with ipv6 either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My initial goal was to fix the immediate problem reported in Discourse that prevents using an IP address as a router address. The symptom is that the IP is not set as a DNS SAN, breaking certificate verification for edge clients and link dialers.
The underlying problem is the ziti create config router edge
command currently parses the generic ZITI_ROUTER_ADVERTISED_ADDRESS
input, and ejects it from DNS SANs if it matches an IPv4 pattern. This results in a silent failure (no exit code and a warning is printed) and no IP SAN is set, because the confirmed IPv4 address is not set in IP SANs. Instead, the command requires that a separate input variable is set, ZITI_ROUTER_IP_OVERRIDE
, which has precisely the same meaning as the generically-named address input variable, ZITI_ROUTER_ADVERTISED_ADDRESS
, except that it sets an IP SAN instead.
// If CSR SANs is an IP, ignore it by setting it blank
result, _ := regexp.MatchString("^[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}$", data.Router.Edge.CsrSans)
if result {
logrus.Warnf("DNS provided (%s) appears to be an IP, ignoring for DNS entry", data.Router.Edge.CsrSans)
data.Router.Edge.CsrSans = ""
}
from: https://github.com/openziti/ziti/blob/v1.1.11/ziti/cmd/create/create_config.go#L294
The proposed fix will wrap this behavior with a safety check to set the alternative, IP-specific input if the string is an IPv4.
I expanded the fix to support controller IP SANs, allowing router or controller addresses to be DNS a name or IPv4 addresses by parsing the string to match an IPv4.
That implementation proved too controversial, and these requirements were added:
- The address inputs for DNS name and IP address must be separate. The implementation must not parse the string to determine if it's an IP address.
- The separate IP address input must support both IPv4 and IPv6.
Notably, the controller does not currently support advertising an IPv6 ctrl plane endpoint, so the IPv6 requirement is "future proofing" for controller deployments.
This expanded scope is not a match for current priorities, so I'm leaving this note here for future self or others to pick up the work when it can be prioritized. For now, controllers and routers must be configured with a DNS name, not an IP address, when using the auto-config generator features of Ziti.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the current draft branch, I experimented with matching both IPv4 and IPv6 with an extended regexp. This pattern is not practical to audit or debug, and I don't find it an acceptable solution, for the record.
ba3ca6f
to
4570aee
Compare
resolves #1993