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.

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

crypto/x509: x509 certificate with issuerUniqueID and/or subjectUniqueID parse error #51754

Closed
mic6090 opened this issue Mar 17, 2022 · 6 comments
Closed

crypto/x509: x509 certificate with issuerUniqueID and/or subjectUniqueID parse error #51754

mic6090 opened this issue Mar 17, 2022 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.19

Comments

@mic6090
Copy link

mic6090 commented Mar 17, 2022 •
edited

go/src/crypto/x509/parser.go

Line 944 in c379c3d

if !tbs.SkipOptionalASN1(cryptobyte_asn1.Tag(1).Constructed().ContextSpecific()) {

go/src/crypto/x509/parser.go

Line 947 in c379c3d

if !tbs.SkipOptionalASN1(cryptobyte_asn1.Tag(2).Constructed().ContextSpecific()) {

RFC 5280 quote:

TBSCertificate ::= SEQUENCE {
version [0] Version DEFAULT v1,
serialNumber CertificateSerialNumber,
signature AlgorithmIdentifier,
issuer Name,
validity Validity,
subject Name,
subjectPublicKeyInfo SubjectPublicKeyInfo,
issuerUniqueID [1] IMPLICIT UniqueIdentifier OPTIONAL,
-- If present, version MUST be v2 or v3
subjectUniqueID [2] IMPLICIT UniqueIdentifier OPTIONAL,
-- If present, version MUST be v2 or v3
extensions [3] Extensions OPTIONAL
-- If present, version MUST be v3 -- }

I think Constructed() calls in mentioned strings are erroneous. As a result, all extensions from certificate with issuerUniqueID or subjectUniqueID fields not parsed.

golang 1.17.8, 1,18 affected.
@seankhliao seankhliao changed the title crypto/x509/parser.go: x509 certificate with issuerUniqueID and/or subjectUniqueID parse error crypto/x509: x509 certificate with issuerUniqueID and/or subjectUniqueID parse error Mar 17, 2022
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 17, 2022
@seankhliao
Copy link
Member

cc @golang/security

@gopherbot
Copy link

Change https://go.dev/cl/394297 mentions this issue: crypto/x509: properly handle issuerUniqueID and subjectUniqueID

@rolandshoemaker
Copy link
Member

@gopherbot please open backport issues, this is a regression (albeit extremely minor, since these fields are almost never used.)

This was referenced Mar 22, 2022
Closed
Closed
@gopherbot
Copy link

Backport issue(s) opened: #51858 (for 1.17), #51859 (for 1.18).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@dmitshur dmitshur added this to the Go1.19 milestone Apr 6, 2022
@dmitshur dmitshur 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 6, 2022
@gopherbot
Copy link

Change https://go.dev/cl/399501 mentions this issue: [release-branch.go1.17] crypto/x509: properly handle issuerUniqueID and subjectUniqueID

@gopherbot
Copy link

Change https://go.dev/cl/399500 mentions this issue: [release-branch.go1.18] crypto/x509: properly handle issuerUniqueID and subjectUniqueID

gopherbot pushed a commit that referenced this issue May 4, 2022
@rolandshoemaker @dmitshur
[release-branch.go1.18] crypto/x509: properly handle issuerUniqueID a…
c6688b5 
…nd subjectUniqueID

Updates #51754
Fixes #51859

Change-Id: I3bfa15db3497de9fb82d6391d87fca1ae9ba6543
Reviewed-on: https://go-review.googlesource.com/c/go/+/394297
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 9a53b47)
Reviewed-on: https://go-review.googlesource.com/c/go/+/399500
Reviewed-by: Russ Cox <rsc@golang.org>
gopherbot pushed a commit that referenced this issue May 4, 2022
@rolandshoemaker @dmitshur
[release-branch.go1.17] crypto/x509: properly handle issuerUniqueID a…
9511f6d 
…nd subjectUniqueID

Updates #51754
Fixes #51858

Change-Id: I3bfa15db3497de9fb82d6391d87fca1ae9ba6543
Reviewed-on: https://go-review.googlesource.com/c/go/+/394297
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 9a53b47)
Reviewed-on: https://go-review.googlesource.com/c/go/+/399501
@golang golang locked and limited conversation to collaborators Apr 11, 2023
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.19
Development

No branches or pull requests
5 participants
@dmitshur @rolandshoemaker @gopherbot @seankhliao @mic6090