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: can set pathlen in certificate when not CA #38216

Closed
m-j-jam opened this issue Apr 2, 2020 · 4 comments
Closed

crypto/x509: can set pathlen in certificate when not CA #38216

m-j-jam opened this issue Apr 2, 2020 · 4 comments
Assignees
Labels
Milestone

Comments

@m-j-jam
Copy link

@m-j-jam m-j-jam commented Apr 2, 2020

RFC 5280 prohibits setting path lengths when not a CA.
From section 4.2.1.9

CAs MUST NOT include the pathLenConstraint field unless the cA
boolean is asserted and the key usage extension asserts the
keyCertSign bit.

This isn't restricted by the library, and means you can create invalid certificates. These are now failing checks in the latest version of OpenSSL (openssl/openssl#11456)

The relevant code is around

ret[n].Value, err = asn1.Marshal(basicConstraints{template.IsCA, maxPathLen})

Something like
if !template.IsCA {
maxPathLen = -1
}
would probably fix, but I'm not an expert in either Go or security, so don't want to change critical code.

@andybons andybons changed the title Can set pathlen in certificate when not CA crypto/x509: can set pathlen in certificate when not CA Apr 3, 2020
@andybons andybons added this to the Unplanned milestone Apr 3, 2020
@andybons
Copy link
Member

@andybons andybons commented Apr 3, 2020

@katiehockman katiehockman modified the milestones: Unplanned, Go1.15 Apr 3, 2020
@katiehockman
Copy link
Member

@katiehockman katiehockman commented Apr 3, 2020

Yes this looks like something we should fix. Let's aim for 1.15. Thanks for filing the issue.

/cc @FiloSottile if you want to comment further

@katiehockman katiehockman self-assigned this Apr 3, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 17, 2020

Change https://golang.org/cl/228777 mentions this issue: crypto/x509: disallow pathLenConstraint when not CA

@gopherbot gopherbot closed this in 141b11d Apr 22, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented May 26, 2020

Change https://golang.org/cl/235280 mentions this issue: crypto/x509: allow setting MaxPathLen to -1 without IsCA

gopherbot pushed a commit that referenced this issue May 26, 2020
This fixes a bug in CL 228777 which disallowed
a MaxPathLen of -1 without IsCA, even though the
x509.Certificate documentation indicates that
MaxPathLen of -1 is considered "unset".

Updates #38216

Change-Id: Ib7240e00408d060f27567be8b820d0eee239256f
Reviewed-on: https://go-review.googlesource.com/c/go/+/235280
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.