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: ParseRevocationList misses entry extensions #53592

Closed
aarongable opened this issue Jun 28, 2022 · 4 comments
Closed

crypto/x509: ParseRevocationList misses entry extensions #53592

aarongable opened this issue Jun 28, 2022 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@aarongable
Copy link
Contributor

aarongable commented Jun 28, 2022 •
edited

What version of Go are you using (go version)?

go1.19beta1

What did you do?

  1. Create an x509.RevocationList with an entry which has extensions (such as a reasonCode)
  2. Use x509.CreateRevocationList to sign and serialize the CRL to DER
  3. Use x509.ParseRevocationList to parse the CRL

Full demonstration: https://go.dev/play/p/ejqFzRaValY?v=gotip

What did you expect to see?

The exact same in-memory structures should be recreated. In particular, the individual entries should still have all of their extensions.

What did you see instead?

Individual entries are missing their extensions.
@gopherbot
Copy link

Change https://go.dev/cl/414877 mentions this issue: crypto/x509: Correctly parse CRL entry extensions

@dr2chase
Copy link
Contributor

@agl @FiloSottile can one or both of you give the CL a look? This code is not my department, though the bug looks completely plausible.

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 30, 2022
@aarongable
Copy link
Contributor Author

aarongable commented Jul 7, 2022 •
edited

@rolandshoemaker I believe this also qualifies as a release blocker? I've added a go playground demonstration to the original report.

@rolandshoemaker
Copy link
Member

Bah, good catch. We can just treat this as a bug fix for new functionality.

jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@aarongable @jproberts
crypto/x509: correctly parse CRL entry extensions
8bdf941 
When checking to see if a CRL entry has any extensions, attempt to read
them from the individual revokedCertificate, rather than from the parent
TBSCertList.

Additionally, crlEntryExtensions is not an EXPLICIT field (c.f.
crlExtension and Certificate extensions), so do not perform an extra
layer of unwrapping when parsing the field.

The added test case fails without the accompanying changes.

Fixes golang#53592

Change-Id: Icc00e4c911f196aef77e3248117de64ddc5ea27f
Reviewed-on: https://go-review.googlesource.com/c/go/+/414877
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: 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>
@golang golang locked and limited conversation to collaborators Jul 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dr2chase @aarongable @rolandshoemaker @gopherbot