crypto/x509: reconsider the expansion of CertificateRequest #43477
Comments
ALTree
added
the
NeedsDecision
|
cc @FiloSottile |
|
I admit I did not consider the use case at length when recommending that proposal be approved. My bad. I agree that applications should not be encouraged to extract any of these values from CSRs. Moreover, if they really need to, they can parse the extensions themselves, which presumably is what they've been doing so far. I'm in favour of rolling back these new APIs. In practice this would look like removing some code rather than rolling back a commit because the commit also did some refactoring that I tink would be more risky to remove than leave in place. I assume this needs to be speed-tracked to make the RC? /cc @golang/proposal-review @golang/release |
|
Please prepare the CL so we can see the change, and make it as small as possible. Thanks. |
|
Change https://golang.org/cl/281235 mentions this issue: |
|
I discussed this with proposal review and everyone agrees with rolling this back and using the CL you sent. |
rsc
added
the
NeedsFix
gopherbot
removed
the
NeedsDecision
|
Change https://golang.org/cl/287392 mentions this issue: |
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>
|
Change https://golang.org/cl/287432 mentions this issue: |
Go 1.16 adds a number of additional convenience fields to
CertificateRequest. I think these fields are a bad idea and should be removed before Go 1.16 is released and these fields have to be supported forever.Go's Cryptography Principles state that the Go crypto library should be easy to use securely. Unfortunately, there are numerous examples of CAs accidentally copying unverified data from CSR fields into final certificates:
Ingesting CSRs is so error-prone that a prudent CA should be discarding everything in a CSR except the public key. Of course, the library can't prevent people from copying raw extensions out of
Extensions, but it could try to avoid nudging people in the wrong direction, and making CSR fields more conveniently accessible seems like a nudge in the wrong direction.The Cryptography Principles also state that the library should focus on "common use cases." However, the bug which proposed this change (#37172) doesn't explain the use case for taking these fields from CSRs, and received only one positive reaction from a non-Go-teammember. I don't think there's any reason to believe that this is a common use case, particularly in the WebPKI, which crypto/x509 is aimed towards.
I'm particularly alarmed by the addition of the BasicConstraints fields (e.g.
IsCA). In a robust PKI, the process for issuing a CA certificate should be distinct from the process for issuing an end-entity certificate. It's not something that should ever be based on the contents of the CSR.The text was updated successfully, but these errors were encountered: