Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
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
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.15

Comments

@m-j-jam
Copy link

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

go/src/crypto/x509/x509.go

Line 1746 in 801cd7c

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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 3, 2020
@andybons andybons added this to the Unplanned milestone Apr 3, 2020
@andybons
Copy link
Member

andybons commented Apr 3, 2020

@katiehockman @FiloSottile

@katiehockman katiehockman modified the milestones: Unplanned, Go1.15 Apr 3, 2020
@katiehockman katiehockman 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 Apr 3, 2020
@katiehockman
Copy link
Contributor

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
Contributor

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

@gopherbot
Copy link
Contributor

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
@katiehockman
crypto/x509: allow setting MaxPathLen to -1 without IsCA
b5bf2f0 
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>
@i-to-r i-to-r mentioned this issue May 30, 2020
Closed
@golang golang locked and limited conversation to collaborators May 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.15
Development

No branches or pull requests
4 participants
@andybons @gopherbot @katiehockman @m-j-jam