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: ParsePKIXPublicKey parses invalid RSA public keys without errors #16166

Closed
szank opened this issue Jun 23, 2016 · 2 comments
Closed

crypto/x509: ParsePKIXPublicKey parses invalid RSA public keys without errors #16166

szank opened this issue Jun 23, 2016 · 2 comments
Labels
FrozenDueToAge
Milestone
Go1.8

Comments

@szank
Copy link

szank commented Jun 23, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    go version go1.6.2 darwin/amd64
  2. What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/mgalkowski/work/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.6.2/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.6.2/libexec/pkg/tool/darwin_amd64"
GO15VENDOREXPERIMENT="1"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fno-common"
CXX="clang++"
CGO_ENABLED="1"
  1. What did you do?
    If possible, provide a recipe for reproducing the error.
    A complete runnable program is good.
    A link on play.golang.org is best.
    https://play.golang.org/p/qMQ1aqX5Ox
  2. What did you expect to see?
    Last line should say:
    Error parsing incorrect RSA public key
  3. What did you see instead?
    INCORRECT public key have been parsed

According to
RFC3279 https://tools.ietf.org/html/rfc3279#section-2.3.1 :

The rsaEncryption OID is intended to be used in the algorithm field
of a value of type AlgorithmIdentifier.  The parameters field MUST
have ASN.1 type NULL for this algorithm identifier.

Could you please fix it ?
@szank
Copy link
Author

szank commented Jun 23, 2016

I am happy to fix it myself if I find some time this weekend.

@ianlancetaylor ianlancetaylor changed the title crypto/x509 ParsePKIXPublicKey parses invalid RSA public keys without errors crypto/x509: ParsePKIXPublicKey parses invalid RSA public keys without errors Jun 23, 2016
@ianlancetaylor ianlancetaylor added this to the Go1.8 milestone Jun 23, 2016
@ianlancetaylor
Copy link
Member

CC @agl

@agl agl self-assigned this Jun 23, 2016
@bprashanth bprashanth mentioned this issue Feb 16, 2017
Closed
joeshaw added a commit to fastly/go-utils that referenced this issue Mar 6, 2017
@joeshaw
fix TLS tests for Go 1.8.
db0c4df 
This meant regenerating a lot of certs.  For background,
golang/go#16166.
@golang golang locked and limited conversation to collaborators Aug 17, 2017
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
@agl
crypto/x509: require a NULL parameters for RSA public keys.
5ad0511 
The RFC is clear that the Parameters in an AlgorithmIdentifer for an RSA
public key must be NULL. BoringSSL enforces this so we have strong
evidence that this is a widely compatible change.

Embarrassingly enough, the major source of violations of this is us. Go
used to get this correct in only one of two places. This was only fixed
in 2013 (with 4874bc9). That's why lots of test certificates are
updated in this change.

Fixes golang#16166.

Change-Id: Ib9a4551349354c66e730d44eb8cee4ec402ea8ab
Reviewed-on: https://go-review.googlesource.com/27312
Reviewed-by: Brad Fitzpatrick <[email protected]>
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
@agl
crypto/x509: require a NULL parameters for RSA public keys.
cde0e97 
The RFC is clear that the Parameters in an AlgorithmIdentifer for an RSA
public key must be NULL. BoringSSL enforces this so we have strong
evidence that this is a widely compatible change.

Embarrassingly enough, the major source of violations of this is us. Go
used to get this correct in only one of two places. This was only fixed
in 2013 (with 4874bc9). That's why lots of test certificates are
updated in this change.

Fixes golang#16166.

Change-Id: Ib9a4551349354c66e730d44eb8cee4ec402ea8ab
Reviewed-on: https://go-review.googlesource.com/27312
Reviewed-by: Brad Fitzpatrick <[email protected]>
@rsc rsc unassigned agl Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.8
Development

No branches or pull requests
4 participants
@agl @szank @ianlancetaylor @gopherbot