From 88399b2e4662ad5a8ca8dc50c5d60fbe42d86396 Mon Sep 17 00:00:00 2001 From: "Jacob H. Haven" Date: Thu, 26 Mar 2015 15:05:09 -0700 Subject: [PATCH] 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 --- src/crypto/x509/x509.go | 51 +++++++++++++++++++++----- src/crypto/x509/x509_test.go | 70 ++++++++++++++++++++---------------- 2 files changed, 83 insertions(+), 38 deletions(-) diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index e75120e9f31..0c096e2b6f5 100644 --- 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 b74cbaba1e1..7373157e41e 100644 --- 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) - } - - 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.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 + 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.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) + } + + 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==", +}