[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 <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
(cherry picked from commit 56d18207823d6e1c18ca46409180c40ae800230c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/460236
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Auto-Submit: Heschi Kreinick <heschi@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Filippo Valsorda <filippo@golang.org>

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 23c514bd78dda143be07c1e081fdffb0c99d57b2..6cd51e58c695985bcfb7d287e46dbc418a5d268c 100644 (file)
--- 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 167ddb7fd04d9e358cdce3dc3f10c57da37fbebd..39ff1cd8fb8ef9d5a29214f75923c9a5ddbb9590 100644 (file)
--- 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")
        }
 }