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

Certificate Discovery: wildcard certificates should not take precedence #3934

Open
grafanalf opened this issue Nov 6, 2024 · 1 comment
Open
Labels
triage/needs-information Indicates an issue needs more information in order to work on it.

Comments

@grafanalf
Copy link

Is your feature request related to a problem?

In my opinion, this feature request is more related to potential security issues, and specifically how the Certificate Discovery algorithm works.

My understanding, by reading the source code, is that Certificate Discovery will give precedence to wildcard certificates with respect to matching, non-wildcard certificates. As an example, if a Kubernetes Ingress for host foo.example.com is created that does not say which certificate ARN to use, Certificate Discovery will be triggered. Let's say that two certificates exist in AWS ACM, a wildcard certificate for *.example.com and a non-wildcard certificate for foo.example.com. The Certificate Discovery algorithm will select the wildcard certificate, as I understand by looking at the code [1]:

func (d *acmCertDiscovery) domainMatchesHost(domainName string, tlsHost string) bool {
	if strings.HasPrefix(domainName, "*.") {
		ds := strings.Split(domainName, ".")
		hs := strings.Split(tlsHost, ".")

		if len(ds) != len(hs) {
			return false
		}

		return cmp.Equal(ds[1:], hs[1:], cmpopts.EquateEmpty())
	}

	return domainName == tlsHost
}

Describe the solution you'd like

From a security point of view, selecting the wildcard certificate over another, non-wildcard one, that also matches the requested host is not ideal. This will leads to (ab)use of the wildcard certificate. In case it gets compromised, all ELBs that rely on it will also be compromised. Besides the risk, they will all need to get rotated, etc. I think this can be avoided by changing the Certificate Discovery algorithm to give precedence to non-wildcard certificates that match the TLS host.

To make the change to the algorithm more controllable, you can introduce some command-line flag gate to the AWS Load Balancer Controller that allows selecting the "old" behaviour (wildcard takes precedence) or the "new' behaviour (non-wildcard takes precedence). Hence, users of the AWS Load Balancer can select which behaviour they want, and to avoid unexpected breakages.

Describe alternatives you've considered

Unfortunately, there are no alternatives for us. Using the wildcard certificate works for us, but introduces undesired risk of having it compromised and exposing all data incoming through ELBs using this wildcard. Mostly because we also have Certificates in AWS ACM that match specifically all the TLS hosts that are involved here.

And we cannot remove the wildcard certificate, because we have also Ingress resources that are associated with an equilvalent DNS wildcard. However, for Ingress resources that have an specific TLS host, we would like to use the Certificate whose SN matches the TLS host, instead of relying on the wildcard certificate.

[1]

func (d *acmCertDiscovery) domainMatchesHost(domainName string, tlsHost string) bool {

@shraddhabang
Copy link
Collaborator

@grafanalf If you want to restrict the ingress to specific certificate, have you tried using the cert-arn annotation instead of cert-discovery option?

@shraddhabang shraddhabang added the triage/needs-information Indicates an issue needs more information in order to work on it. label Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

No branches or pull requests

2 participants