Skip to content
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

added both LB IP and hostname to sans #391

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

PietroPasotti
Copy link
Contributor

Issue

the current logic in server_cert_sans_dns returns the external host provided by metallb if it is a hostname, else, if it's an IP, returns the "true hostname" returned by socket.gethostbyaddr(IP).

On certain environments (such as my local development one) that means that ``server_cert_sans_dns` returns the name of my computer, and none of the cos-lite components will be able to use TLS over traefik because traefik's IP address has nothing to do with my computer's name.

Solution

Solution is adding the true hostname to the SANS instead of replacing the IP with it.
Traefik will get a cert valid for both the IP and the hostname.

@PietroPasotti
Copy link
Contributor Author

@sed-i I'm going to need you to double-check this, and help clarify that this isn't breaking anything important.
Advice on how to test this on top of the existing tls integration tests is also appreciated.

Copy link

@gruyaume gruyaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would generally discourage from using IP addresses in your certificates, especially in Kubernetes because:

  1. IP addresses change, you will need to make sure that when your pod reschedules and you get a new IP address, you also ask for a new certificate
  2. Let's Encrypt does not generate certificates for IP addresses and would reject such a request with an error like acme: error: 400 :: POST :: https://acme-staging-v02.api.letsencrypt.org/acme/new-order :: urn:ietf:params:acme:error:unsupportedIdentifier :: NewOrder request included invalid non-DNS type identifier: type "ip", value "1.2.3.4"

Plus if you want to use IP addresses, you should likely use the sans_ip field instead of sans_dns.

@PietroPasotti
Copy link
Contributor Author

I would generally discourage from using IP addresses in your certificates, especially in Kubernetes

OK. How do we test TLS when doing development locally?
Issue is traefik provides ingress at its metallb-provided IP address, so units will try to talk to one another (over https) using traefik's IP and get a TLS error because that IP address is not in the SANS.
Is the only solution for local development to set traefik's hostname to X and update your resolv.conf to point X to traefik's IP?
Will that also make the hostname resolvable for all ingressed charms?

Plus if you want to use IP addresses, you should likely use the sans_ip field instead of sans_dns.

The conversion is done in CertHandler.

@sed-i
Copy link
Contributor

sed-i commented Aug 9, 2024

I think setting a sans_ip when external_hostname isn't given is correct behavior, and it's also great that real CAs would reject this. We probably just need to make sure that sans_ip is not populated when external_hostname is set? @gruyaume

Apart from being a valid use case (cf. 1.1.1.1), it makes testing much simpler, as we won't need to update the CI machine's /etc/hosts (or equivalent).


# If all else fails, we'd rather use the bare IP
return [target] if target else []
return targets
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a couple of test cases? I'm thinking:

  • When external_hostname is set, then the CSR should be for the external hostname.
  • When external_hostname is unset, then the CSR should be for the loadbalancer IP.

@gruyaume
Copy link

gruyaume commented Aug 9, 2024

I would generally discourage from using IP addresses in your certificates, especially in Kubernetes

OK. How do we test TLS when doing development locally? Issue is traefik provides ingress at its metallb-provided IP address, so units will try to talk to one another (over https) using traefik's IP and get a TLS error because that IP address is not in the SANS. Is the only solution for local development to set traefik's hostname to X and update your resolv.conf to point X to traefik's IP? Will that also make the hostname resolvable for all ingressed charms?

Plus if you want to use IP addresses, you should likely use the sans_ip field instead of sans_dns.

The conversion is done in CertHandler.

For local development, I think sans_ip's are fine. You would likely need to be careful in how users can impact what ends up in their certificate.s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants