crypto/x509: parse all names in an RDN.

The Subject and Issuer names in a certificate look like they should be a
list of key-value pairs. However, they're actually a list of lists of
key-value pairs. Previously we only looked at the first element of each
sublist and the vast majority of certificates only have one element per
sublist.

However, it's possible to have multiple elements and some 360
certificates from the “Pilot” log are so constructed.

This change causes all elements of the sublists to be processed.

Fixes #16836.

Change-Id: Ie0a5159135b08226ec517fcf251aa17aada37857
Reviewed-on: https://go-review.googlesource.com/30810
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/crypto/x509/pkix/pkix.go b/src/crypto/x509/pkix/pkix.go
index faad4061fc71347f1459fa1ea665f0159e4af622..39fd78df596078fb3f192251dbed09edc778d6b8 100644 (file)
--- a/src/crypto/x509/pkix/pkix.go
+++ b/src/crypto/x509/pkix/pkix.go
@@ -64,34 +64,36 @@ func (n *Name) FillFromRDNSequence(rdns *RDNSequence) {
                if len(rdn) == 0 {
                        continue
                }
-               atv := rdn[0]
-               n.Names = append(n.Names, atv)
-               value, ok := atv.Value.(string)
-               if !ok {
-                       continue
-               }
 
-               t := atv.Type
-               if len(t) == 4 && t[0] == 2 && t[1] == 5 && t[2] == 4 {
-                       switch t[3] {
-                       case 3:
-                               n.CommonName = value
-                       case 5:
-                               n.SerialNumber = value
-                       case 6:
-                               n.Country = append(n.Country, value)
-                       case 7:
-                               n.Locality = append(n.Locality, value)
-                       case 8:
-                               n.Province = append(n.Province, value)
-                       case 9:
-                               n.StreetAddress = append(n.StreetAddress, value)
-                       case 10:
-                               n.Organization = append(n.Organization, value)
-                       case 11:
-                               n.OrganizationalUnit = append(n.OrganizationalUnit, value)
-                       case 17:
-                               n.PostalCode = append(n.PostalCode, value)
+               for _, atv := range rdn {
+                       n.Names = append(n.Names, atv)
+                       value, ok := atv.Value.(string)
+                       if !ok {
+                               continue
+                       }
+
+                       t := atv.Type
+                       if len(t) == 4 && t[0] == 2 && t[1] == 5 && t[2] == 4 {
+                               switch t[3] {
+                               case 3:
+                                       n.CommonName = value
+                               case 5:
+                                       n.SerialNumber = value
+                               case 6:
+                                       n.Country = append(n.Country, value)
+                               case 7:
+                                       n.Locality = append(n.Locality, value)
+                               case 8:
+                                       n.Province = append(n.Province, value)
+                               case 9:
+                                       n.StreetAddress = append(n.StreetAddress, value)
+                               case 10:
+                                       n.Organization = append(n.Organization, value)
+                               case 11:
+                                       n.OrganizationalUnit = append(n.OrganizationalUnit, value)
+                               case 17:
+                                       n.PostalCode = append(n.PostalCode, value)
+                               }
                        }
                }
        }

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index 36c2b91353f0c41a58b12f8babedcf49639a934e..ae77331a41a7a4e60943c2aad2031cd2e3573e77 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -1405,3 +1405,55 @@ func TestISOOIDInCertificate(t *testing.T) {
                t.Errorf("ISO OID not recognised in certificate")
        }
 }
+
+// certMultipleRDN contains a RelativeDistinguishedName with two elements (the
+// common name and serial number). This particular certificate was the first
+// such certificate in the “Pilot” Certificate Transparency log.
+const certMultipleRDN = `
+-----BEGIN CERTIFICATE-----
+MIIFRzCCBC+gAwIBAgIEOl59NTANBgkqhkiG9w0BAQUFADA9MQswCQYDVQQGEwJz
+aTEbMBkGA1UEChMSc3RhdGUtaW5zdGl0dXRpb25zMREwDwYDVQQLEwhzaWdvdi1j
+YTAeFw0xMjExMTYxMDUyNTdaFw0xNzExMTYxMjQ5MDVaMIGLMQswCQYDVQQGEwJz
+aTEbMBkGA1UEChMSc3RhdGUtaW5zdGl0dXRpb25zMRkwFwYDVQQLExB3ZWItY2Vy
+dGlmaWNhdGVzMRAwDgYDVQQLEwdTZXJ2ZXJzMTIwFAYDVQQFEw0xMjM2NDg0MDEw
+MDEwMBoGA1UEAxMTZXBvcnRhbC5tc3MuZWR1cy5zaTCCASIwDQYJKoZIhvcNAQEB
+BQADggEPADCCAQoCggEBAMrNkZH9MPuBTjMGNk3sJX8V+CkFx/4ru7RTlLS6dlYM
+098dtSfJ3s2w0p/1NB9UmR8j0yS0Kg6yoZ3ShsSO4DWBtcQD8820a6BYwqxxQTNf
+HSRZOc+N/4TQrvmK6t4k9Aw+YEYTMrWOU4UTeyhDeCcUsBdh7HjfWsVaqNky+2sv
+oic3zP5gF+2QfPkvOoHT3FLR8olNhViIE6Kk3eFIEs4dkq/ZzlYdLb8pHQoj/sGI
+zFmA5AFvm1HURqOmJriFjBwaCtn8AVEYOtQrnUCzJYu1ex8azyS2ZgYMX0u8A5Z/
+y2aMS/B2W+H79WcgLpK28vPwe7vam0oFrVytAd+u65ECAwEAAaOCAf4wggH6MA4G
+A1UdDwEB/wQEAwIFoDBABgNVHSAEOTA3MDUGCisGAQQBr1kBAwMwJzAlBggrBgEF
+BQcCARYZaHR0cDovL3d3dy5jYS5nb3Yuc2kvY3BzLzAfBgNVHREEGDAWgRRwb2Rw
+b3JhLm1pemtzQGdvdi5zaTCB8QYDVR0fBIHpMIHmMFWgU6BRpE8wTTELMAkGA1UE
+BhMCc2kxGzAZBgNVBAoTEnN0YXRlLWluc3RpdHV0aW9uczERMA8GA1UECxMIc2ln
+b3YtY2ExDjAMBgNVBAMTBUNSTDM5MIGMoIGJoIGGhldsZGFwOi8veDUwMC5nb3Yu
+c2kvb3U9c2lnb3YtY2Esbz1zdGF0ZS1pbnN0aXR1dGlvbnMsYz1zaT9jZXJ0aWZp
+Y2F0ZVJldm9jYXRpb25MaXN0P2Jhc2WGK2h0dHA6Ly93d3cuc2lnb3YtY2EuZ292
+LnNpL2NybC9zaWdvdi1jYS5jcmwwKwYDVR0QBCQwIoAPMjAxMjExMTYxMDUyNTda
+gQ8yMDE3MTExNjEyNDkwNVowHwYDVR0jBBgwFoAUHvjUU2uzgwbpBAZXAvmlv8ZY
+PHIwHQYDVR0OBBYEFGI1Duuu+wTGDZka/xHNbwcbM69ZMAkGA1UdEwQCMAAwGQYJ
+KoZIhvZ9B0EABAwwChsEVjcuMQMCA6gwDQYJKoZIhvcNAQEFBQADggEBAHny0K1y
+BQznrzDu3DDpBcGYguKU0dvU9rqsV1ua4nxkriSMWjgsX6XJFDdDW60I3P4VWab5
+ag5fZzbGqi8kva/CzGgZh+CES0aWCPy+4Gb8lwOTt+854/laaJvd6kgKTER7z7U9
+9C86Ch2y4sXNwwwPJ1A9dmrZJZOcJjS/WYZgwaafY2Hdxub5jqPE5nehwYUPVu9R
+uH6/skk4OEKcfOtN0hCnISOVuKYyS4ANARWRG5VGHIH06z3lGUVARFRJ61gtAprd
+La+fgSS+LVZ+kU2TkeoWAKvGq8MAgDq4D4Xqwekg7WKFeuyusi/NI5rm40XgjBMF
+DF72IUofoVt7wo0=
+-----END CERTIFICATE-----`
+
+func TestMultipleRDN(t *testing.T) {
+       block, _ := pem.Decode([]byte(certMultipleRDN))
+       cert, err := ParseCertificate(block.Bytes)
+       if err != nil {
+               t.Fatalf("certificate with two elements in an RDN failed to parse: %v", err)
+       }
+
+       if want := "eportal.mss.edus.si"; cert.Subject.CommonName != want {
+               t.Errorf("got common name of %q, but want %q", cert.Subject.CommonName, want)
+       }
+
+       if want := "1236484010010"; cert.Subject.SerialNumber != want {
+               t.Errorf("got serial number of %q, but want %q", cert.Subject.SerialNumber, want)
+       }
+}