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: parse additional fields in CertificateRequest #37172

Closed
chauncyc opened this issue Feb 11, 2020 · 9 comments
Closed

crypto/x509: parse additional fields in CertificateRequest #37172

chauncyc opened this issue Feb 11, 2020 · 9 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Proposals (old)
Milestone
Go1.16

Comments

@chauncyc
Copy link
Contributor

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

$ 1.13.4

ParseCertificate parses many common extensions and puts them in the Certificate struct. I'd like for ParseCertificateRequest to do the same. Specifically, I currently manually parse the following fields from CertificateRequest.Extensions, and would like for them to be added to CertificateRequest, with the corresponding changes to ParseCertificateRequest and CreateCertificateRequest, exactly as they're implemented for Certificate:

  • KeyUsage
  • ExtKeyUsage
  • UnknownExtKeyUsage
  • IsCA
  • MaxPathLen
  • BasicConstraintsValid
  • MaxPathLenZero
  • SubjectKeyId
  • PolicyIdentifier
@gopherbot gopherbot added this to the Proposal milestone Feb 11, 2020
@FiloSottile
Copy link
Contributor

Sounds good, we should even be able to share code with the Certificate parser.

@FiloSottile FiloSottile added the Proposal-Crypto Proposal related to crypto packages or other security issues label Feb 11, 2020
@rsc rsc added this to Active in Proposals (old) Feb 12, 2020
@rsc
Copy link
Contributor

rsc commented Feb 12, 2020

Adding to proposal minutes, seems headed for likely accept.

@rsc rsc mentioned this issue Feb 12, 2020
Open
@rsc
Copy link
Contributor

rsc commented Mar 4, 2020

Based on the discussion above this seems like a likely accept.

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Mar 4, 2020
@rsc rsc moved this from Likely Accept to Active in Proposals (old) Mar 4, 2020
@rsc rsc moved this from Active to Likely Accept in Proposals (old) Mar 4, 2020
@rsc
Copy link
Contributor

rsc commented Mar 11, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Mar 11, 2020
@FiloSottile FiloSottile added NeedsFix The path to resolution is known, but the work has not been done. and removed Proposal Proposal-FinalCommentPeriod labels Mar 11, 2020
@FiloSottile FiloSottile changed the title proposal: crypto/x509: parse additional fields in CertificateRequest crypto/x509: parse additional fields in CertificateRequest Mar 11, 2020
@FiloSottile FiloSottile modified the milestones: Proposal, Go1.15 Mar 11, 2020
@FiloSottile
Copy link
Contributor

Did not happen for Go 1.15.

@FiloSottile FiloSottile modified the milestones: Go1.15, Go1.16 May 8, 2020
@gopherbot
Copy link

Change https://golang.org/cl/233163 mentions this issue: crypto/x509: add additional convenience fields to CertificateRequest

@dmitshur dmitshur mentioned this issue Dec 28, 2020
Closed
@AGWA AGWA mentioned this issue Jan 3, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/281235 mentions this issue: crypto/x509: rollback new CertificateRequest fields

gopherbot pushed a commit that referenced this issue Jan 6, 2021
@FiloSottile
crypto/x509: rollback new CertificateRequest fields
4787e90 
In general, we don't want to encourage reading them from CSRs, and
applications that really want to can parse the Extensions field.

Note that this also fixes a bug where the error of
parseKeyUsageExtension was not handled in parseCertificateRequest.

Fixes #43477
Updates #37172

Change-Id: Ia5707b0e23cecc0aed57e419a1ca25e26eea6bbe
Reviewed-on: https://go-review.googlesource.com/c/go/+/281235
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/287392 mentions this issue: crypto/x509: remove leftover CertificateRequest field

gopherbot pushed a commit that referenced this issue Jan 27, 2021
@rolandshoemaker
crypto/x509: remove leftover CertificateRequest field
35334ca 
Removes the KeyUsage field that was missed in the rollback in
CL 281235.
Also updates CreateCertificateRequest to reflect that these fields
were removed.

For #43407.
Updates #43477.
Updates #37172.

Change-Id: I6244aed4a3ef3c2460c38af5511e5c2e82546179
Reviewed-on: https://go-review.googlesource.com/c/go/+/287392
Trust: Alexander Rakoczy <alex@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/287432 mentions this issue: crypto/x509: rollback new CertificateRequest.KeyUsage field

@golang golang locked and limited conversation to collaborators Jan 27, 2022
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. Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
No open projects
Proposals (old)
Accepted
Milestone
Go1.16
Development

No branches or pull requests
4 participants
@rsc @FiloSottile @gopherbot @chauncyc