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.

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

crypto/x509: ParsePKIXPublicKey ignores tail of ASN.1 encoding #10583

Closed
rsc opened this issue Apr 27, 2015 · 1 comment
Closed

crypto/x509: ParsePKIXPublicKey ignores tail of ASN.1 encoding #10583

rsc opened this issue Apr 27, 2015 · 1 comment
Assignees
Milestone

Comments

@rsc
Copy link
Contributor

@rsc rsc commented Apr 27, 2015

We probably need to fix ParsePKIXPublicKey not to ignore the remainder of the DER encoding.

@agl, please speak up if you think the code is correct as is. Otherwise we'll take care of it. Thanks.

---------- Forwarded message ----------
From: jabczynskimichal@gmail.com
Date: Fri, Apr 24, 2015 at 3:36 PM
Subject: [golang-dev] proposal: ASN1 (un)marshalling from Reader/Writer
To: golang-dev@googlegroups.com

Hi golang-dev,

...

In crypto/x509/x509.go, we have the following code:

// ParsePKIXPublicKey parses a DER encoded public key. These values are
// typically found in PEM blocks with "BEGIN PUBLIC KEY".
func ParsePKIXPublicKey(derBytes []byte) (pub interface{}, err error) {
    var pki publicKeyInfo
    if _, err = asn1.Unmarshal(derBytes, &pki); err != nil {
        return
    }
    algo := getPublicKeyAlgorithmFromOID(pki.Algorithm.Algorithm)
    if algo == UnknownPublicKeyAlgorithm {
        return nil, errors.New("x509: unknown public key algorithm")
    }
    return parsePublicKey(algo, &pki)
}

Notice that the rest value is ignored when parsing the public key. If key validity is checked using a hash function, a malicious entity could add bytes after the public key, changing its hash value without altering the key itself. This is unacceptable in x509 implementation.

@rsc rsc added this to the Go1.5 milestone Apr 27, 2015
@agl agl self-assigned this Apr 27, 2015
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 29, 2015

CL https://golang.org/cl/9473 mentions this issue.

@agl agl closed this in 1ddb8c2 Apr 30, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.