From 809a1de1ac1ccc76f7a4faf630017626f2f68231 Mon Sep 17 00:00:00 2001
From: Adam Langley <agl@golang.org>
Date: Mon, 10 Oct 2016 16:26:51 -0700
Subject: [PATCH] crypto/x509: parse all names in an RDN.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 src/crypto/x509/pkix/pkix.go | 56 +++++++++++++++++++-----------------
 src/crypto/x509/x509_test.go | 52 +++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 27 deletions(-)

diff --git a/src/crypto/x509/pkix/pkix.go b/src/crypto/x509/pkix/pkix.go
index faad4061fc..39fd78df59 100644
--- 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 36c2b91353..ae77331a41 100644
--- 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)
+	}
+}