From e04be8b24c20816f3429a8193c324ea67892e61f Mon Sep 17 00:00:00 2001 From: Rob Stradling Date: Tue, 6 Sep 2022 17:30:31 +0100 Subject: [PATCH] [release-branch.go1.19] crypto/x509: reallow duplicate attributes in CSRs Fixes #57556 Updates #54936 Change-Id: I3fb4331c2b1b6adafbac3e76eaf66c79cd5ef56f Reviewed-on: https://go-review.googlesource.com/c/go/+/428636 Run-TryBot: Roland Shoemaker Reviewed-by: Damien Neil TryBot-Result: Gopher Robot Reviewed-by: Roland Shoemaker Auto-Submit: Roland Shoemaker (cherry picked from commit 56d18207823d6e1c18ca46409180c40ae800230c) Reviewed-on: https://go-review.googlesource.com/c/go/+/460236 Reviewed-by: Filippo Valsorda Auto-Submit: Heschi Kreinick Reviewed-by: Heschi Kreinick Reviewed-by: Bryan Mills Run-TryBot: Filippo Valsorda --- src/crypto/x509/x509.go | 8 +------- src/crypto/x509/x509_test.go | 26 ++++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index 23c514bd78..6cd51e58c6 100644 --- a/src/crypto/x509/x509.go +++ b/src/crypto/x509/x509.go @@ -1816,18 +1816,13 @@ func parseCSRExtensions(rawAttributes []asn1.RawValue) ([]pkix.Extension, error) } var ret []pkix.Extension - seenExts := make(map[string]bool) + requestedExts := make(map[string]bool) for _, rawAttr := range rawAttributes { var attr pkcs10Attribute if rest, err := asn1.Unmarshal(rawAttr.FullBytes, &attr); err != nil || len(rest) != 0 || len(attr.Values) == 0 { // Ignore attributes that don't parse. continue } - oidStr := attr.Id.String() - if seenExts[oidStr] { - return nil, errors.New("x509: certificate request contains duplicate extensions") - } - seenExts[oidStr] = true if !attr.Id.Equal(oidExtensionRequest) { continue @@ -1837,7 +1832,6 @@ func parseCSRExtensions(rawAttributes []asn1.RawValue) ([]pkix.Extension, error) if _, err := asn1.Unmarshal(attr.Values[0].FullBytes, &extensions); err != nil { return nil, err } - requestedExts := make(map[string]bool) for _, ext := range extensions { oidStr := ext.Id.String() if requestedExts[oidStr] { diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go index 167ddb7fd0..39ff1cd8fb 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go @@ -3750,10 +3750,32 @@ VLOVx0i+/Q7fikp3hbN1JwuMTU0v2KL/IKoUcZc02+5xiYrnOIt5 func TestDuplicateExtensionsCSR(t *testing.T) { b, _ := pem.Decode([]byte(dupExtCSR)) if b == nil { - t.Fatalf("couldn't decode test certificate") + t.Fatalf("couldn't decode test CSR") } _, err := ParseCertificateRequest(b.Bytes) if err == nil { - t.Fatal("ParseCertificate should fail when parsing certificate with duplicate extensions") + t.Fatal("ParseCertificateRequest should fail when parsing CSR with duplicate extensions") + } +} + +const dupAttCSR = `-----BEGIN CERTIFICATE REQUEST----- +MIIBbDCB1gIBADAPMQ0wCwYDVQQDEwR0ZXN0MIGfMA0GCSqGSIb3DQEBAQUAA4GN +ADCBiQKBgQCj5Po3PKO/JNuxr+B+WNfMIzqqYztdlv+mTQhT0jOR5rTkUvxeeHH8 +YclryES2dOISjaUOTmOAr5GQIIdQl4Ql33Cp7ZR/VWcRn+qvTak0Yow+xVsDo0n4 +7IcvvP6CJ7FRoYBUakVczeXLxCjLwdyK16VGJM06eRzDLykPxpPwLQIDAQABoB4w +DQYCKgMxBwwFdGVzdDEwDQYCKgMxBwwFdGVzdDIwDQYJKoZIhvcNAQELBQADgYEA +UJ8hsHxtnIeqb2ufHnQFJO+wEJhx2Uxm/BTuzHOeffuQkwATez4skZ7SlX9exgb7 +6jRMRilqb4F7f8w+uDoqxRrA9zc8mwY16zPsyBhRet+ZGbj/ilgvGmtZ21qZZ/FU +0pJFJIVLM3l49Onr5uIt5+hCWKwHlgE0nGpjKLR3cMg= +-----END CERTIFICATE REQUEST-----` + +func TestDuplicateAttributesCSR(t *testing.T) { + b, _ := pem.Decode([]byte(dupAttCSR)) + if b == nil { + t.Fatalf("couldn't decode test CSR") + } + _, err := ParseCertificateRequest(b.Bytes) + if err != nil { + t.Fatal("ParseCertificateRequest should succeed when parsing CSR with duplicate attributes") } } -- 2.48.1