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 validation fails when certificate in chain is missing CN #2372

Closed
nowylie opened this issue Mar 23, 2020 · 10 comments
Closed

Certificate validation fails when certificate in chain is missing CN #2372

nowylie opened this issue Mar 23, 2020 · 10 comments
Assignees
Milestone

Comments

@nowylie
Copy link

nowylie commented Mar 23, 2020

What steps did you take and what happened:

Contour failed to validate our full chain certificate when the last certificate in the chain was missing a CN. The error in the Contour logs was:

time="2020-03-20T06:39:20Z" level=error msg="invalid TLS certificate: certificate has no common name or subject alt name" context=KubernetesCache kind=Secret name=... namespace=... version=v1

Specifically, Contour doesn't appear to like the root certificate in the GoDaddy Certificate Bundles - G2 With Cross to G1, includes Root (gd_bundle-g2-g1.crt) bundle found here.

The result was that no HTTPS endpoint was configured for the specified ingress.

What did you expect to happen:

An HTTPS endpoint for the Ingress.

Anything else you would like to add:

N/A

Environment:

  • Contour version: 1.2.1
  • Kubernetes version: (use kubectl version):
    Client Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.3", GitCommit:"06ad960bfd03b39c8310aaf92d1e7c12ce618213", GitTreeState:"archive", BuildDate:"2020-02-29T16:37:45Z", GoVersion:"go1.14", Compiler:"gc", Platform:"linux/amd64"}
    Server Version: version.Info{Major:"1", Minor:"16", GitVersion:"v1.16.8", GitCommit:"ec6eb119b81be488b030e849b9e64fda4caaf33c", GitTreeState:"clean", BuildDate:"2020-03-12T20:52:22Z", GoVersion:"go1.13.8", Compiler:"gc", Platform:"linux/amd64"}
    
  • Kubernetes installer & version:
    kubeadm version: &version.Info{Major:"1", Minor:"16", GitVersion:"v1.16.8", GitCommit:"ec6eb119b81be488b030e849b9e64fda4caaf33c", GitTreeState:"clean", BuildDate:"2020-03-12T20:57:57Z", GoVersion:"go1.13.8", Compiler:"gc", Platform:"linux/amd64"}
    
  • Cloud provider or hardware configuration: baremetal
  • OS (e.g. from /etc/os-release): Debian GNU/Linux 9.9 (stretch)
@stevesloka
Copy link
Member

Contour has some validation to make sure that referenced certs are valid. Does your generated cert have a CN or SubjectAltName?

@nowylie
Copy link
Author

nowylie commented Mar 31, 2020

I'm not sure what you mean by "generated cert". The validation was failing for the root certificate in the chain that we had stored in a Secret. I linked to the particular certificate chain above.

@jpeach
Copy link
Contributor

jpeach commented Apr 1, 2020

This certificate does have a CN, not sure why we would not see it

$ openssl x509 -text -noout -in gd_bundle-g2-g1.crt
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 7 (0x7)
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: C = US, ST = Arizona, L = Scottsdale, O = "GoDaddy.com, Inc.", CN = Go Daddy Root Certificate Authority - G2
        Validity
            Not Before: May  3 07:00:00 2011 GMT
            Not After : May  3 07:00:00 2031 GMT
        Subject: C = US, ST = Arizona, L = Scottsdale, O = "GoDaddy.com, Inc.", OU = http://certs.godaddy.com/repository/, CN = Go Daddy Secure Certificate Authority - G2
...

@nowylie
Copy link
Author

nowylie commented Apr 1, 2020

If you copy the last certificate in the chain into it's own file and run the same you can see that the root certificate doesn't have a CN:

$ openssl x509 -text -noout -in root.crt 
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 0 (0x0)
        Signature Algorithm: sha1WithRSAEncryption
        Issuer: C = US, O = "The Go Daddy Group, Inc.", OU = Go Daddy Class 2 Certification Authority
        Validity
            Not Before: Jun 29 17:06:20 2004 GMT
            Not After : Jun 29 17:06:20 2034 GMT
        Subject: C = US, O = "The Go Daddy Group, Inc.", OU = Go Daddy Class 2 Certification Authority

Whether this is correct or not I'm unsure...

We've since started using the gdig2_bundle.crt chain (which is identical but omits the root certificate) and everything seems to work fine.

@jpeach
Copy link
Contributor

jpeach commented Apr 1, 2020

Thanks @nowylie! Yeh, it's fine per x509 to not have a CN. Only the leaf serving certificate really needs to have either the CN or SAN.

@youngnick
Copy link
Member

We may need to update the CN check then to exclude root certificates or something.

@jpeach
Copy link
Contributor

jpeach commented Apr 1, 2020

Ideally, we would validate the hostname from the proxy against the certificate bundle, but it's probably tricky to ensure that we are doing it the same way that Envoy will do it. I'm not sure how much validation Envoy will do at xDS time, but maybe that's the best place to know whether the certificates were bad.

@youngnick
Copy link
Member

Okay, the requirement for there to be a CN or SAN is there for serving certificates. See #1965 and envoyproxy/envoy#9182 for more information, but Envoy used to crash if at least one of those fields is not present, and will now reject the config, putting Contour in the dreaded #1472 NACK state where no config updates will flow to Envoy if a serving certificate like this is found.

I'm going to test out a few extra cases and see if we can update the validation logic to check that at least one certificate in a bundle contains a CN or SAN, and if that will be enough to allow Envoy to accept the cert. That would solve this issue, as currently, all certs in the bundle must have CN or SAN set.

@youngnick
Copy link
Member

After speaking to @jpeach, it seems that Envoy expects the first cert in a bundle to be the serving cert, and will reject the whole thing if that first cert does not have a CN or SAN set.

Working on a PR to fix this up now, and add additional testing to cover the "certificate in chain missing CN" use cases.

youngnick pushed a commit to youngnick/contour that referenced this issue Nov 8, 2021
This commit updates the Secret validation framework to address bugs
around CA certificates, certificates with no CN, and various combinations
of the above.

Also updates documentation to make the rules for the types of Secrets
clearer.

Updates projectcontour#2372
Updates projectcontour#3889
Updates projectcontour#3496

Future work is to add some of these checks into the e2e tests, and ensure
that all these configs produce valid Envoy config, with no NACKs.

Signed-off-by: Nick Young <[email protected]>
youngnick pushed a commit to youngnick/contour that referenced this issue Nov 8, 2021
This commit updates the Secret validation framework to address bugs
around CA certificates, certificates with no CN, and various combinations
of the above.

Also updates documentation to make the rules for the types of Secrets
clearer.

Updates projectcontour#2372
Updates projectcontour#3889
Updates projectcontour#3496

Future work is to add some of these checks into the e2e tests, and ensure
that all these configs produce valid Envoy config, with no NACKs.

Signed-off-by: Nick Young <[email protected]>
youngnick pushed a commit to youngnick/contour that referenced this issue Nov 10, 2021
This commit updates the Secret validation framework to address bugs
around CA certificates, certificates with no CN, and various combinations
of the above.

Also updates documentation to make the rules for the types of Secrets
clearer.

Updates projectcontour#2372
Updates projectcontour#3889
Updates projectcontour#3496

Future work is to add some of these checks into the e2e tests, and ensure
that all these configs produce valid Envoy config, with no NACKs.

Signed-off-by: Nick Young <[email protected]>
youngnick pushed a commit that referenced this issue Nov 21, 2021
This commit updates the Secret validation framework to address bugs
around CA certificates, certificates with no CN, and various combinations
of the above.

Also updates documentation to make the rules for the types of Secrets
clearer.

Updates #2372
Updates #3889
Updates #3496

Future work is to add some of these checks into the e2e tests, and ensure
that all these configs produce valid Envoy config, with no NACKs.

Signed-off-by: Nick Young <[email protected]>
@youngnick
Copy link
Member

Okay, I believe the fix now merged into main should fix this issue. I'll close this, but please reopen if there are still issues once 1.20 is released.

Repository owner moved this from Todo to Done in Contour Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

4 participants