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

crypto/x509: Certificate.Verify does not enforce name constraints correctly #59171

Closed
rittneje opened this issue Mar 21, 2023 · 3 comments
Closed
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@rittneje
Copy link

In Go 1.19, Certificate.Verify was changed to also enforce name constraints on non-leaf certificates. https://tip.golang.org/doc/go1.19#crypto/x509

However, as per the source code, this is enforced by saying that if certificate X has a name constraint, then it gets enforced on both (and only) the leaf and certificate X. This violates RFC 5280 section 4.2.1.10:

The name constraints extension, which MUST be used only in a CA
certificate, indicates a name space within which all subject names in
subsequent certificates in a certification path MUST be located.
Restrictions apply to the subject distinguished name and apply to
subject alternative names. Restrictions apply only when the
specified name form is present. If no name of the type is in the
certificate, the certificate is acceptable.

Note that the RFC clearly says "subsequent certificates", which means that enforcing a certificate's name constraints on itself is incorrect. In addition, Go is not enforcing the name constraints on the subsequent intermediate certificates in the chain, which also violates the RFC.

As an example, here we have a root certificate with a name constraint for "example.com", an intermediate certificate with a SAN of "www.sample.com", and a leaf certificate with a SAN of "www.example.com". This should be rejected due to the invalid intermediate SAN, but is accepted. https://go.dev/play/p/Ozj9G8lZaD6

In this example, we have a root certificate with a name constraint for "example.com" and a SAN of "foobar.com", an intermediate certificate with no SANs, and a leaf certificate with a SAN of "www.example.com". This should be accepted because the root name constraint is not supposed to apply to itself, but is rejected. https://go.dev/play/p/eeq2mE4Dzby

Note that OpenSSL properly follows the RFC in both cases. (That is, it properly rejects the first example, and properly accepts the second example.)

@heschi heschi added this to the Go1.21 milestone Mar 21, 2023
@heschi heschi added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 21, 2023
@heschi
Copy link
Contributor

heschi commented Mar 21, 2023

cc @golang/security

@rolandshoemaker rolandshoemaker added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 21, 2023
@gopherbot
Copy link

Change https://go.dev/cl/478216 mentions this issue: crypto/x509: properly apply name constrains to roots and intermediates

@rolandshoemaker
Copy link
Member

I don't think there is any real security impact here, the main reason we decided to apply name constraints uniformly was to enforce the rejection of chains which are otherwise invalid according to CABF (And 5280) rules. That said since we claimed we added this restriction in the 1.19 release notes, we should backport this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants