crypto/x509: ignore CN if SAN extension present.

The code previously tested only whether DNS-name SANs were present in a
certificate which is only approximately correct. In fact, /any/ SAN
extension, including one with no DNS names, should cause the CN to be
ignored.

Change-Id: I3d9824918975be6d4817e7cbb48ed1b0c5a2fc8b
Reviewed-on: https://go-review.googlesource.com/36696
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 29345a1755c8e1cde125946ae1cd3eb3299256e7..021fe5e644717569023b1e6a2718af404e4c2b4c 100644 (file)
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -87,7 +87,7 @@ func (h HostnameError) Error() string {
                        valid += san.String()
                }
        } else {
-               if len(c.DNSNames) > 0 {
+               if c.hasSANExtension() {
                        valid = strings.Join(c.DNSNames, ", ")
                } else {
                        valid = c.Subject.CommonName
@@ -482,7 +482,7 @@ func (c *Certificate) VerifyHostname(h string) error {
 
        lowered := toLowerCaseASCII(h)
 
-       if len(c.DNSNames) > 0 {
+       if c.hasSANExtension() {
                for _, match := range c.DNSNames {
                        if matchHostnames(toLowerCaseASCII(match), lowered) {
                                return nil

diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
index 2b4c25997feb8224eb5d23f51b0cda604453e78b..2783feb15591747a64dc2eb7f2f20c2017cb24c3 100644 (file)
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -274,6 +274,17 @@ var verifyTests = []verifyTest{
 
                errorCallback: expectNotAuthorizedError,
        },
+       {
+               // If any SAN extension is present (even one without any DNS
+               // names), the CN should be ignored.
+               leaf:        ignoreCNWithSANLeaf,
+               dnsName:     "foo.example.com",
+               roots:       []string{ignoreCNWithSANRoot},
+               currentTime: 1486684488,
+               systemSkip:  true,
+
+               errorCallback: expectHostnameError,
+       },
 }
 
 func expectHostnameError(t *testing.T, i int, err error) (ok bool) {
@@ -1334,6 +1345,51 @@ CwUAA4GBADYzYUvaToO/ucBskPdqXV16AaakIhhSENswYVSl97/sODaxsjishKq9
 /jt8qszOXCv2vYdUTPNuPqufXLWMoirpuXrr1liJDmedCcAHepY/
 -----END CERTIFICATE-----`
 
+const ignoreCNWithSANRoot = `
+-----BEGIN CERTIFICATE-----
+MIIDPzCCAiegAwIBAgIIJkzCwkNrPHMwDQYJKoZIhvcNAQELBQAwMDEQMA4GA1UE
+ChMHVEVTVElORzEcMBoGA1UEAxMTKipUZXN0aW5nKiogUm9vdCBDQTAeFw0xNTAx
+MDEwMDAwMDBaFw0yNTAxMDEwMDAwMDBaMDAxEDAOBgNVBAoTB1RFU1RJTkcxHDAa
+BgNVBAMTEyoqVGVzdGluZyoqIFJvb3QgQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IB
+DwAwggEKAoIBAQC4YAf5YqlXGcikvbMWtVrNICt+V/NNWljwfvSKdg4Inm7k6BwW
+P6y4Y+n4qSYIWNU4iRkdpajufzctxQCO6ty13iw3qVktzcC5XBIiS6ymiRhhDgnY
+VQqyakVGw9MxrPwdRZVlssUv3Hmy6tU+v5Ok31SLY5z3wKgYWvSyYs0b8bKNU8kf
+2FmSHnBN16lxGdjhe3ji58F/zFMr0ds+HakrLIvVdFcQFAnQopM8FTHpoWNNzGU3
+KaiO0jBbMFkd6uVjVnuRJ+xjuiqi/NWwiwQA+CEr9HKzGkxOF8nAsHamdmO1wW+w
+OsCrC0qWQ/f5NTOVATTJe0vj88OMTvo3071VAgMBAAGjXTBbMA4GA1UdDwEB/wQE
+AwICpDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwDwYDVR0TAQH/BAUw
+AwEB/zAZBgNVHQ4EEgQQQDfXAftAL7gcflQEJ4xZATANBgkqhkiG9w0BAQsFAAOC
+AQEAGOn3XjxHyHbXLKrRmpwV447B7iNBXR5VlhwOgt1kWaHDL2+8f/9/h0HMkB6j
+fC+/yyuYVqYuOeavqMGVrh33D2ODuTQcFlOx5lXukP46j3j+Lm0jjZ1qNX7vlP8I
+VlUXERhbelkw8O4oikakwIY9GE8syuSgYf+VeBW/lvuAZQrdnPfabxe05Tre6RXy
+nJHMB1q07YHpbwIkcV/lfCE9pig2nPXTLwYZz9cl46Ul5RCpPUi+IKURo3x8y0FU
+aSLjI/Ya0zwUARMmyZ3RRGCyhIarPb20mKSaMf1/Nb23pS3k1QgmZhk5pAnXYsWu
+BJ6bvwEAasFiLGP6Zbdmxb2hIA==
+-----END CERTIFICATE-----`
+
+const ignoreCNWithSANLeaf = `
+-----BEGIN CERTIFICATE-----
+MIIDaTCCAlGgAwIBAgIJAONakvRTxgJhMA0GCSqGSIb3DQEBCwUAMDAxEDAOBgNV
+BAoTB1RFU1RJTkcxHDAaBgNVBAMTEyoqVGVzdGluZyoqIFJvb3QgQ0EwHhcNMTUw
+MTAxMDAwMDAwWhcNMjUwMTAxMDAwMDAwWjAsMRAwDgYDVQQKEwdURVNUSU5HMRgw
+FgYDVQQDEw9mb28uZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAw
+ggEKAoIBAQDBqskp89V/JMIBBqcauKSOVLcMyIE/t0jgSWVrsI4sksBTabLsfMdS
+ui2n+dHQ1dRBuw3o4g4fPrWwS3nMnV3pZUHEn2TPi5N1xkjTaxObXgKIY2GKmFP3
+rJ9vYqHT6mT4K93kCHoRcmJWWySc7S3JAOhTcdB4G+tIdQJN63E+XRYQQfNrn5HZ
+hxQoOzaguHFx+ZGSD4Ntk6BSZz5NfjqCYqYxe+iCpTpEEYhIpi8joSPSmkTMTxBW
+S1W2gXbYNQ9KjNkGM6FnQsUJrSPMrWs4v3UB/U88N5LkZeF41SqD9ySFGwbGajFV
+nyzj12+4K4D8BLhlOc0Eo/F/8GwOwvmxAgMBAAGjgYkwgYYwDgYDVR0PAQH/BAQD
+AgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAMBgNVHRMBAf8EAjAA
+MBkGA1UdDgQSBBCjeab27q+5pV43jBGANOJ1MBsGA1UdIwQUMBKAEEA31wH7QC+4
+HH5UBCeMWQEwDwYDVR0RBAgwBocEfwAAATANBgkqhkiG9w0BAQsFAAOCAQEAGZfZ
+ErTVxxpIg64s22mQpXSk/72THVQsfsKHzlXmztM0CJzH8ccoN67ZqKxJCfdiE/FI
+Emb6BVV4cGPeIKpcxaM2dwX/Y+Y0JaxpQJvqLxs+EByRL0gPP3shgg86WWCjYLxv
+AgOn862d/JXGDrC9vIlQ/DDQcyL5g0JV5UjG2G9TUigbnrXxBw7BoWK6wmoSaHnR
+sZKEHSs3RUJvm7qqpA9Yfzm9jg+i9j32zh1xFacghAOmFRFXa9eCVeigZ/KK2mEY
+j2kBQyvnyKsXHLAKUoUOpd6t/1PHrfXnGj+HmzZNloJ/BZ1kiWb4eLvMljoLGkZn
+xZbqP3Krgjj4XNaXjg==
+-----END CERTIFICATE-----`
+
 var unknownAuthorityErrorTests = []struct {
        cert     string
        expected string

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index a8d30d58db4f202327166cb7080352a7b71ee5ae..2934168c744fbe2c2cbddee954552bf2823b7efa 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -723,6 +723,10 @@ func (c *Certificate) Equal(other *Certificate) bool {
        return bytes.Equal(c.Raw, other.Raw)
 }
 
+func (c *Certificate) hasSANExtension() bool {
+       return oidInExtensions(oidExtensionSubjectAltName, c.Extensions)
+}
+
 // Entrust have a broken root certificate (CN=Entrust.net Certification
 // Authority (2048)) which isn't marked as a CA certificate and is thus invalid
 // according to PKIX.