crypto/x509: Fix parsing bug in uncommon CSR Attributes.

A CSR containing challengePassword or unstructuredName Attributes
(included in default OpenSSL prompts) would break ASN.1 parsing.
This updates the parsing structures to allow but then ignore these
fields.

See this CFSSL issue: https://github.com/cloudflare/cfssl/issues/115

Change-Id: I26a3bf1794589d27e6e763da88ae32276f0170c7
Reviewed-on: https://go-review.googlesource.com/8160
Reviewed-by: Adam Langley <agl@golang.org>

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index e75120e9f31882131970cf8cee89516fffe690be..0c096e2b6f57892b903e4c03244b57dd1312fa37 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -1669,11 +1669,11 @@ type CertificateRequest struct {
 // signature requests (see RFC 2986):
 
 type tbsCertificateRequest struct {
-       Raw        asn1.RawContent
-       Version    int
-       Subject    asn1.RawValue
-       PublicKey  publicKeyInfo
-       Attributes []pkix.AttributeTypeAndValueSET `asn1:"tag:0"`
+       Raw           asn1.RawContent
+       Version       int
+       Subject       asn1.RawValue
+       PublicKey     publicKeyInfo
+       RawAttributes []asn1.RawValue `asn1:"tag:0"`
 }
 
 type certificateRequest struct {
@@ -1687,6 +1687,36 @@ type certificateRequest struct {
 // extensions in a CSR.
 var oidExtensionRequest = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 14}
 
+// newRawAttributes converts AttributeTypeAndValueSETs from a template
+// CertificateRequest's Attributes into tbsCertificateRequest RawAttributes.
+func newRawAttributes(attributes []pkix.AttributeTypeAndValueSET) ([]asn1.RawValue, error) {
+       var rawAttributes []asn1.RawValue
+       b, err := asn1.Marshal(attributes)
+       rest, err := asn1.Unmarshal(b, &rawAttributes)
+       if err != nil {
+               return nil, err
+       }
+       if len(rest) != 0 {
+               return nil, errors.New("x509: failed to unmarshall raw CSR Attributes")
+       }
+       return rawAttributes, nil
+}
+
+// parseRawAttributes Unmarshals RawAttributes intos AttributeTypeAndValueSETs.
+func parseRawAttributes(rawAttributes []asn1.RawValue) []pkix.AttributeTypeAndValueSET {
+       var attributes []pkix.AttributeTypeAndValueSET
+       for _, rawAttr := range rawAttributes {
+               var attr pkix.AttributeTypeAndValueSET
+               rest, err := asn1.Unmarshal(rawAttr.FullBytes, &attr)
+               // Ignore attributes that don't parse into pkix.AttributeTypeAndValueSET
+               // (i.e.: challengePassword or unstructuredName).
+               if err == nil && len(rest) == 0 {
+                       attributes = append(attributes, attr)
+               }
+       }
+       return attributes
+}
+
 // CreateCertificateRequest creates a new certificate based on a template. The
 // following members of template are used: Subject, Attributes,
 // SignatureAlgorithm, Extensions, DNSNames, EmailAddresses, and IPAddresses.
@@ -1799,6 +1829,11 @@ func CreateCertificateRequest(rand io.Reader, template *CertificateRequest, priv
                }
        }
 
+       rawAttributes, err := newRawAttributes(attributes)
+       if err != nil {
+               return
+       }
+
        tbsCSR := tbsCertificateRequest{
                Version: 0, // PKCS #10, RFC 2986
                Subject: asn1.RawValue{FullBytes: asn1Subject},
@@ -1809,7 +1844,7 @@ func CreateCertificateRequest(rand io.Reader, template *CertificateRequest, priv
                                BitLength: len(publicKeyBytes) * 8,
                        },
                },
-               Attributes: attributes,
+               RawAttributes: rawAttributes,
        }
 
        tbsCSRContents, err := asn1.Marshal(tbsCSR)
@@ -1866,7 +1901,7 @@ func parseCertificateRequest(in *certificateRequest) (*CertificateRequest, error
                PublicKeyAlgorithm: getPublicKeyAlgorithmFromOID(in.TBSCSR.PublicKey.Algorithm.Algorithm),
 
                Version:    in.TBSCSR.Version,
-               Attributes: in.TBSCSR.Attributes,
+               Attributes: parseRawAttributes(in.TBSCSR.RawAttributes),
        }
 
        var err error
@@ -1884,7 +1919,7 @@ func parseCertificateRequest(in *certificateRequest) (*CertificateRequest, error
 
        var extensions []pkix.AttributeTypeAndValue
 
-       for _, atvSet := range in.TBSCSR.Attributes {
+       for _, atvSet := range out.Attributes {
                if !atvSet.Type.Equal(oidExtensionRequest) {
                        continue
                }

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index b74cbaba1e135d16cde435a43b098c26e86fd35b..7373157e41e88c27ec34e57af55157cea169198b 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -1014,33 +1014,35 @@ func TestCertificateRequestOverrides(t *testing.T) {
 }
 
 func TestParseCertificateRequest(t *testing.T) {
-       csrBytes := fromBase64(csrBase64)
-       csr, err := ParseCertificateRequest(csrBytes)
-       if err != nil {
-               t.Fatalf("failed to parse CSR: %s", err)
-       }
+       for _, csrBase64 := range csrBase64Array {
+               csrBytes := fromBase64(csrBase64)
+               csr, err := ParseCertificateRequest(csrBytes)
+               if err != nil {
+                       t.Fatalf("failed to parse CSR: %s", err)
+               }
 
-       if len(csr.EmailAddresses) != 1 || csr.EmailAddresses[0] != "gopher@golang.org" {
-               t.Errorf("incorrect email addresses found: %v", csr.EmailAddresses)
-       }
+               if len(csr.EmailAddresses) != 1 || csr.EmailAddresses[0] != "gopher@golang.org" {
+                       t.Errorf("incorrect email addresses found: %v", csr.EmailAddresses)
+               }
 
-       if len(csr.DNSNames) != 1 || csr.DNSNames[0] != "test.example.com" {
-               t.Errorf("incorrect DNS names found: %v", csr.DNSNames)
-       }
+               if len(csr.DNSNames) != 1 || csr.DNSNames[0] != "test.example.com" {
+                       t.Errorf("incorrect DNS names found: %v", csr.DNSNames)
+               }
 
-       if len(csr.Subject.Country) != 1 || csr.Subject.Country[0] != "AU" {
-               t.Errorf("incorrect Subject name: %v", csr.Subject)
-       }
+               if len(csr.Subject.Country) != 1 || csr.Subject.Country[0] != "AU" {
+                       t.Errorf("incorrect Subject name: %v", csr.Subject)
+               }
 
-       found := false
-       for _, e := range csr.Extensions {
-               if e.Id.Equal(oidExtensionBasicConstraints) {
-                       found = true
-                       break
+               found := false
+               for _, e := range csr.Extensions {
+                       if e.Id.Equal(oidExtensionBasicConstraints) {
+                               found = true
+                               break
+                       }
+               }
+               if !found {
+                       t.Errorf("basic constraints extension not found in CSR")
                }
-       }
-       if !found {
-               t.Errorf("basic constraints extension not found in CSR")
        }
 }
 
@@ -1129,12 +1131,20 @@ func TestASN1BitLength(t *testing.T) {
        }
 }
 
-// This CSR was generated with OpenSSL:
-//  openssl req -out CSR.csr -new -newkey rsa:2048 -nodes -keyout privateKey.key -config openssl.cnf
+// These CSR was generated with OpenSSL:
+//  openssl req -out CSR.csr -new -sha256 -nodes -keyout privateKey.key -config openssl.cnf
 //
-// The openssl.cnf needs to include this section:
+// With openssl.cnf containing the following sections:
 //   [ v3_req ]
 //   basicConstraints = CA:FALSE
 //   keyUsage = nonRepudiation, digitalSignature, keyEncipherment
 //   subjectAltName = email:gopher@golang.org,DNS:test.example.com
-const csrBase64 = "MIIC4zCCAcsCAQAwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAOY+MVedRg2JEnyeLcSzcsMv2VcsTfkB5+Etd6hihAh6MrGezNyASMMKuQN6YhCX1icQDiQtGsDLTtheNnSXK06tAhHjAP/hGlszRJp+5+rP2M58fDBAkUBEhskbCUWwpY14jFtVuGNJ8vF8h8IeczdolvQhX9lVai9G0EUXJMliMKdjA899H0mRs9PzHyidyrXFNiZlQXfD8Kg7gETn2Ny965iyI6ujAIYSCvam6TnxRHYH2MBKyVGvsYGbPYUQJCsgdgyajEg6ekihvQY3SzO1HSAlZAd7d1QYO4VeWJ2mY6Wu3Jpmh+AmG19S9CcHqGjd0bhuAX9cpPOKgnEmqn0CAwEAAaBZMFcGCSqGSIb3DQEJDjFKMEgwCQYDVR0TBAIwADALBgNVHQ8EBAMCBeAwLgYDVR0RBCcwJYERZ29waGVyQGdvbGFuZy5vcmeCEHRlc3QuZXhhbXBsZS5jb20wDQYJKoZIhvcNAQEFBQADggEBAC9+QpKfdabxwCWwf4IEe1cKjdXLS1ScSuw27a3kZzQiPV78WJMa6dB8dqhdH5BRwGZ/qsgLrO6ZHlNeIv2Ib41Ccq71ecHW/nXc94A1BzJ/bVdI9LZcmTUvR1/m1jCpN7UqQ0ml1u9VihK7Pe762hEYxuWDQzYEU0l15S/bXmqeq3eF1A59XT/2jwe5+NV0Wwf4UQlkTXsAQMsJ+KzrQafd8Qv2A49o048uRvmjeJDrXLawGVianZ7D5A6Fpd1rZh6XcjqBpmgLw41DRQWENOdzhy+HyphKRv1MlY8OLkNqpGMhu8DdgJVGoT16DGiickoEa7Z3UCPVNgdTkT9jq7U="
+//   [ req_attributes ]
+//   challengePassword = ignored challenge
+//   unstructuredName  = ignored unstructured name
+var csrBase64Array = [...]string{
+       // Just [ v3_req ]
+       "MIIDHDCCAgQCAQAwfjELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDEUMBIGA1UEAwwLQ29tbW9uIE5hbWUxITAfBgkqhkiG9w0BCQEWEnRlc3RAZW1haWwuYWRkcmVzczCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAK1GY4YFx2ujlZEOJxQVYmsjUnLsd5nFVnNpLE4cV+77sgv9NPNlB8uhn3MXt5leD34rm/2BisCHOifPucYlSrszo2beuKhvwn4+2FxDmWtBEMu/QA16L5IvoOfYZm/gJTsPwKDqvaR0tTU67a9OtxwNTBMI56YKtmwd/o8d3hYv9cg+9ZGAZ/gKONcg/OWYx/XRh6bd0g8DMbCikpWgXKDsvvK1Nk+VtkDO1JxuBaj4Lz/p/MifTfnHoqHxWOWl4EaTs4Ychxsv34/rSj1KD1tJqorIv5Xv2aqv4sjxfbrYzX4kvS5SC1goIovLnhj5UjmQ3Qy8u65eow/LLWw+YFcCAwEAAaBZMFcGCSqGSIb3DQEJDjFKMEgwCQYDVR0TBAIwADALBgNVHQ8EBAMCBeAwLgYDVR0RBCcwJYERZ29waGVyQGdvbGFuZy5vcmeCEHRlc3QuZXhhbXBsZS5jb20wDQYJKoZIhvcNAQELBQADggEBAB6VPMRrchvNW61Tokyq3ZvO6/NoGIbuwUn54q6l5VZW0Ep5Nq8juhegSSnaJ0jrovmUgKDN9vEo2KxuAtwG6udS6Ami3zP+hRd4k9Q8djJPb78nrjzWiindLK5Fps9U5mMoi1ER8ViveyAOTfnZt/jsKUaRsscY2FzE9t9/o5moE6LTcHUS4Ap1eheR+J72WOnQYn3cifYaemsA9MJuLko+kQ6xseqttbh9zjqd9fiCSh/LNkzos9c+mg2yMADitaZinAh+HZi50ooEbjaT3erNq9O6RqwJlgD00g6MQdoz9bTAryCUhCQfkIaepmQ7BxS0pqWNW3MMwfDwx/Snz6g=",
+       // Both [ v3_req ] and [ req_attributes ]
+       "MIIDaTCCAlECAQAwfjELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDEUMBIGA1UEAwwLQ29tbW9uIE5hbWUxITAfBgkqhkiG9w0BCQEWEnRlc3RAZW1haWwuYWRkcmVzczCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAK1GY4YFx2ujlZEOJxQVYmsjUnLsd5nFVnNpLE4cV+77sgv9NPNlB8uhn3MXt5leD34rm/2BisCHOifPucYlSrszo2beuKhvwn4+2FxDmWtBEMu/QA16L5IvoOfYZm/gJTsPwKDqvaR0tTU67a9OtxwNTBMI56YKtmwd/o8d3hYv9cg+9ZGAZ/gKONcg/OWYx/XRh6bd0g8DMbCikpWgXKDsvvK1Nk+VtkDO1JxuBaj4Lz/p/MifTfnHoqHxWOWl4EaTs4Ychxsv34/rSj1KD1tJqorIv5Xv2aqv4sjxfbrYzX4kvS5SC1goIovLnhj5UjmQ3Qy8u65eow/LLWw+YFcCAwEAAaCBpTAgBgkqhkiG9w0BCQcxEwwRaWdub3JlZCBjaGFsbGVuZ2UwKAYJKoZIhvcNAQkCMRsMGWlnbm9yZWQgdW5zdHJ1Y3R1cmVkIG5hbWUwVwYJKoZIhvcNAQkOMUowSDAJBgNVHRMEAjAAMAsGA1UdDwQEAwIF4DAuBgNVHREEJzAlgRFnb3BoZXJAZ29sYW5nLm9yZ4IQdGVzdC5leGFtcGxlLmNvbTANBgkqhkiG9w0BAQsFAAOCAQEAgxe2N5O48EMsYE7o0rZBB0wi3Ov5/yYfnmmVI22Y3sP6VXbLDW0+UWIeSccOhzUCcZ/G4qcrfhhx6gTZTeA01nP7TdTJURvWAH5iFqj9sQ0qnLq6nEcVHij3sG6M5+BxAIVClQBk6lTCzgphc835Fjj6qSLuJ20XHdL5UfUbiJxx299CHgyBRL+hBUIPfz8p+ZgamyAuDLfnj54zzcRVyLlrmMLNPZNll1Q70RxoU6uWvLH8wB8vQe3Q/guSGubLyLRTUQVPh+dw1L4t8MKFWfX/48jwRM4gIRHFHPeAAE9D9YAoqdIvj/iFm/eQ++7DP8MDwOZWsXeB6jjwHuLmkQ==",
+}