crypto/x509: ignore Common Name by default

Common Name has been deprecated for 20 years, and has horrible
interactions with Name Constraints. The browsers managed to drop it last
year, let's try flicking the switch to disabled by default.

Return helpful errors for things that would get unbroken by flipping the
switch back with the environment variable.

Had to refresh a test certificate that was too old to have SANs.

Updates #24151

Change-Id: I2ab78577fd936ba67969d3417284dbe46e4ae02f
Reviewed-on: https://go-review.googlesource.com/c/go/+/231379
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>

diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 7427c5714f7aa12d288736115cb4b30cdcf52a97..e8886c14c7e4844482aeb26eb12f07b77351532a 100644 (file)
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -19,7 +19,7 @@ import (
 )
 
 // ignoreCN disables interpreting Common Name as a hostname. See issue 24151.
-var ignoreCN = strings.Contains(os.Getenv("GODEBUG"), "x509ignoreCN=1")
+var ignoreCN = !strings.Contains(os.Getenv("GODEBUG"), "x509ignoreCN=0")
 
 type InvalidReason int
 
@@ -48,9 +48,9 @@ const (
        // contains name constraints, and the Common Name can be interpreted as
        // a hostname.
        //
-       // You can avoid this error by setting the experimental GODEBUG environment
-       // variable to "x509ignoreCN=1", disabling Common Name matching entirely.
-       // This behavior might become the default in the future.
+       // This error is only returned when legacy Common Name matching is enabled
+       // by setting the GODEBUG environment variable to "x509ignoreCN=1". This
+       // setting might be removed in the future.
        NameConstraintsWithoutSANs
        // UnconstrainedName results when a CA certificate contains permitted
        // name constraints, but leaf certificate contains a name of an
@@ -109,10 +109,16 @@ type HostnameError struct {
 func (h HostnameError) Error() string {
        c := h.Certificate
 
-       if !c.hasSANExtension() && !validHostname(c.Subject.CommonName) &&
-               matchHostnames(c.Subject.CommonName, h.Host) {
-               // This would have validated, if it weren't for the validHostname check on Common Name.
-               return "x509: Common Name is not a valid hostname: " + c.Subject.CommonName
+       if !c.hasSANExtension() && matchHostnames(c.Subject.CommonName, h.Host) {
+               if !ignoreCN && !validHostname(c.Subject.CommonName) {
+                       // This would have validated, if it weren't for the validHostname check on Common Name.
+                       return "x509: Common Name is not a valid hostname: " + c.Subject.CommonName
+               }
+               if ignoreCN && validHostname(c.Subject.CommonName) {
+                       // This would have validated if x509ignoreCN=0 were set.
+                       return "x509: certificate relies on legacy Common Name field, " +
+                               "use SANs or temporarily enable Common Name matching with GODEBUG=x509ignoreCN=0"
+               }
        }
 
        var valid string
@@ -944,7 +950,7 @@ func validHostname(host string) bool {
 
 // commonNameAsHostname reports whether the Common Name field should be
 // considered the hostname that the certificate is valid for. This is a legacy
-// behavior, disabled if the Subject Alt Name extension is present.
+// behavior, disabled by default or if the Subject Alt Name extension is present.
 //
 // It applies the strict validHostname check to the Common Name field, so that
 // certificates without SANs can still be validated against CAs with name
@@ -1028,10 +1034,10 @@ func toLowerCaseASCII(in string) string {
 // against the DNSNames field. If the names are valid hostnames, the certificate
 // fields can have a wildcard as the left-most label.
 //
-// If the Common Name field is a valid hostname, and the certificate doesn't
-// have any Subject Alternative Names, the name will also be checked against the
-// Common Name. This legacy behavior can be disabled by setting the GODEBUG
-// environment variable to "x509ignoreCN=1" and might be removed in the future.
+// The legacy Common Name field is ignored unless it's a valid hostname, the
+// certificate doesn't have any Subject Alternative Names, and the GODEBUG
+// environment variable is set to "x509ignoreCN=0". Support for Common Name is
+// deprecated will be entirely removed in the future.
 func (c *Certificate) VerifyHostname(h string) error {
        // IP addresses may be written in [ ].
        candidateIP := h

diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
index 86fe76a57d7f83e8bf65fa42566bbf9ec8b479d0..8a9036a3d0a66b5fd01eb0a63010846070e910ce 100644 (file)
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -374,7 +374,7 @@ var verifyTests = []verifyTest{
                systemSkip:  true,
                ignoreCN:    true,
 
-               errorCallback: expectHostnameError("Common Name is not a valid hostname"),
+               errorCallback: expectHostnameError("not valid for any names"),
        },
        {
                leaf:        validCNWithoutSAN,
@@ -384,7 +384,7 @@ var verifyTests = []verifyTest{
                systemSkip:  true,
                ignoreCN:    true,
 
-               errorCallback: expectHostnameError("not valid for any names"),
+               errorCallback: expectHostnameError("certificate relies on legacy Common Name field"),
        },
        {
                // A certificate with an AKID should still chain to a parent without SKID.

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index 7e431a6e9ee0d656b952e7b5598927c8efaa2dee..f29e322bb4f036f4b03957aedbdc0146a91a371b 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -433,7 +433,7 @@ func TestMatchIP(t *testing.T) {
 }
 
 func TestCertificateParse(t *testing.T) {
-       s, _ := hex.DecodeString(certBytes)
+       s, _ := base64.StdEncoding.DecodeString(certBytes)
        certs, err := ParseCertificates(s)
        if err != nil {
                t.Error(err)
@@ -452,7 +452,7 @@ func TestCertificateParse(t *testing.T) {
                t.Error(err)
        }
 
-       const expectedExtensions = 4
+       const expectedExtensions = 10
        if n := len(certs[0].Extensions); n != expectedExtensions {
                t.Errorf("want %d extensions, got %d", expectedExtensions, n)
        }
@@ -496,48 +496,50 @@ func TestMismatchedSignatureAlgorithm(t *testing.T) {
        }
 }
 
-var certBytes = "308203223082028ba00302010202106edf0d9499fd4533dd1297fc42a93be1300d06092a864886" +
-       "f70d0101050500304c310b3009060355040613025a4131253023060355040a131c546861777465" +
-       "20436f6e73756c74696e67202850747929204c74642e311630140603550403130d546861777465" +
-       "20534743204341301e170d3039303332353136343932395a170d3130303332353136343932395a" +
-       "3069310b3009060355040613025553311330110603550408130a43616c69666f726e6961311630" +
-       "140603550407130d4d6f756e7461696e205669657731133011060355040a130a476f6f676c6520" +
-       "496e63311830160603550403130f6d61696c2e676f6f676c652e636f6d30819f300d06092a8648" +
-       "86f70d010101050003818d0030818902818100c5d6f892fccaf5614b064149e80a2c9581a218ef" +
-       "41ec35bd7a58125ae76f9ea54ddc893abbeb029f6b73616bf0ffd868791fba7af9c4aebf3706ba" +
-       "3eeaeed27435b4ddcfb157c05f351d66aa87fee0de072d66d773affbd36ab78bef090e0cc861a9" +
-       "03ac90dd98b51c9c41566c017f0beec3bff391051ffba0f5cc6850ad2a590203010001a381e730" +
-       "81e430280603551d250421301f06082b0601050507030106082b06010505070302060960864801" +
-       "86f842040130360603551d1f042f302d302ba029a0278625687474703a2f2f63726c2e74686177" +
-       "74652e636f6d2f54686177746553474343412e63726c307206082b060105050701010466306430" +
-       "2206082b060105050730018616687474703a2f2f6f6373702e7468617774652e636f6d303e0608" +
-       "2b060105050730028632687474703a2f2f7777772e7468617774652e636f6d2f7265706f736974" +
-       "6f72792f5468617774655f5347435f43412e637274300c0603551d130101ff04023000300d0609" +
-       "2a864886f70d01010505000381810062f1f3050ebc105e497c7aedf87e24d2f4a986bb3b837bd1" +
-       "9b91ebcad98b065992f6bd2b49b7d6d3cb2e427a99d606c7b1d46352527fac39e6a8b6726de5bf" +
-       "70212a52cba07634a5e332011bd1868e78eb5e3c93cf03072276786f207494feaa0ed9d53b2110" +
-       "a76571f90209cdae884385c882587030ee15f33d761e2e45a6bc308203233082028ca003020102" +
-       "020430000002300d06092a864886f70d0101050500305f310b3009060355040613025553311730" +
-       "15060355040a130e566572695369676e2c20496e632e31373035060355040b132e436c61737320" +
-       "33205075626c6963205072696d6172792043657274696669636174696f6e20417574686f726974" +
-       "79301e170d3034303531333030303030305a170d3134303531323233353935395a304c310b3009" +
-       "060355040613025a4131253023060355040a131c54686177746520436f6e73756c74696e672028" +
-       "50747929204c74642e311630140603550403130d5468617774652053474320434130819f300d06" +
-       "092a864886f70d010101050003818d0030818902818100d4d367d08d157faecd31fe7d1d91a13f" +
-       "0b713cacccc864fb63fc324b0794bd6f80ba2fe10493c033fc093323e90b742b71c403c6d2cde2" +
-       "2ff50963cdff48a500bfe0e7f388b72d32de9836e60aad007bc4644a3b847503f270927d0e62f5" +
-       "21ab693684317590f8bfc76c881b06957cc9e5a8de75a12c7a68dfd5ca1c875860190203010001" +
-       "a381fe3081fb30120603551d130101ff040830060101ff020100300b0603551d0f040403020106" +
-       "301106096086480186f842010104040302010630280603551d110421301fa41d301b3119301706" +
-       "035504031310507269766174654c6162656c332d313530310603551d1f042a30283026a024a022" +
-       "8620687474703a2f2f63726c2e766572697369676e2e636f6d2f706361332e63726c303206082b" +
-       "0601050507010104263024302206082b060105050730018616687474703a2f2f6f6373702e7468" +
-       "617774652e636f6d30340603551d25042d302b06082b0601050507030106082b06010505070302" +
-       "06096086480186f8420401060a6086480186f845010801300d06092a864886f70d010105050003" +
-       "81810055ac63eadea1ddd2905f9f0bce76be13518f93d9052bc81b774bad6950a1eededcfddb07" +
-       "e9e83994dcab72792f06bfab8170c4a8edea5334edef1e53d906c7562bd15cf4d18a8eb42bb137" +
-       "9048084225c53e8acb7feb6f04d16dc574a2f7a27c7b603c77cd0ece48027f012fb69b37e02a2a" +
-       "36dcd585d6ace53f546f961e05af"
+var certBytes = "MIIE0jCCA7qgAwIBAgIQWcvS+TTB3GwCAAAAAGEAWzANBgkqhkiG9w0BAQsFADBCMQswCQYD" +
+       "VQQGEwJVUzEeMBwGA1UEChMVR29vZ2xlIFRydXN0IFNlcnZpY2VzMRMwEQYDVQQDEwpHVFMg" +
+       "Q0EgMU8xMB4XDTIwMDQwMTEyNTg1NloXDTIwMDYyNDEyNTg1NlowaTELMAkGA1UEBhMCVVMx" +
+       "EzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDU1vdW50YWluIFZpZXcxEzARBgNVBAoT" +
+       "Ckdvb2dsZSBMTEMxGDAWBgNVBAMTD21haWwuZ29vZ2xlLmNvbTBZMBMGByqGSM49AgEGCCqG" +
+       "SM49AwEHA0IABO+dYiPnkFl+cZVf6mrWeNp0RhQcJSBGH+sEJxjvc+cYlW3QJCnm57qlpFdd" +
+       "pz3MPyVejvXQdM6iI1mEWP4C2OujggJmMIICYjAOBgNVHQ8BAf8EBAMCB4AwEwYDVR0lBAww" +
+       "CgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUI6pZhnQ/lQgmPDwSKR2A54G7" +
+       "AS4wHwYDVR0jBBgwFoAUmNH4bhDrz5vsYJ8YkBug630J/SswZAYIKwYBBQUHAQEEWDBWMCcG" +
+       "CCsGAQUFBzABhhtodHRwOi8vb2NzcC5wa2kuZ29vZy9ndHMxbzEwKwYIKwYBBQUHMAKGH2h0" +
+       "dHA6Ly9wa2kuZ29vZy9nc3IyL0dUUzFPMS5jcnQwLAYDVR0RBCUwI4IPbWFpbC5nb29nbGUu" +
+       "Y29tghBpbmJveC5nb29nbGUuY29tMCEGA1UdIAQaMBgwCAYGZ4EMAQICMAwGCisGAQQB1nkC" +
+       "BQMwLwYDVR0fBCgwJjAkoCKgIIYeaHR0cDovL2NybC5wa2kuZ29vZy9HVFMxTzEuY3JsMIIB" +
+       "AwYKKwYBBAHWeQIEAgSB9ASB8QDvAHYAsh4FzIuizYogTodm+Su5iiUgZ2va+nDnsklTLe+L" +
+       "kF4AAAFxNgmxKgAABAMARzBFAiEA12/OHdTGXQ3qHHC3NvYCyB8aEz/+ZFOLCAI7lhqj28sC" +
+       "IG2/7Yz2zK6S6ai+dH7cTMZmoFGo39gtaTqtZAqEQX7nAHUAXqdz+d9WwOe1Nkh90EngMnqR" +
+       "mgyEoRIShBh1loFxRVgAAAFxNgmxTAAABAMARjBEAiA7PNq+MFfv6O9mBkxFViS2TfU66yRB" +
+       "/njcebWglLQjZQIgOyRKhxlEizncFRml7yn4Bg48ktXKGjo+uiw6zXEINb0wDQYJKoZIhvcN" +
+       "AQELBQADggEBADM2Rh306Q10PScsolYMxH1B/K4Nb2WICvpY0yDPJFdnGjqCYym196TjiEvs" +
+       "R6etfeHdyzlZj6nh82B4TVyHjiWM02dQgPalOuWQcuSy0OvLh7F1E7CeHzKlczdFPBTOTdM1" +
+       "RDTxlvw1bAqc0zueM8QIAyEy3opd7FxAcGQd5WRIJhzLBL+dbbMOW/LTeW7cm/Xzq8cgCybN" +
+       "BSZAvhjseJ1L29OlCTZL97IfnX0IlFQzWuvvHy7V2B0E3DHlzM0kjwkkCKDUUp/wajv2NZKC" +
+       "TkhEyERacZRKc9U0ADxwsAzHrdz5+5zfD2usEV/MQ5V6d8swLXs+ko0X6swrd4YCiB8wggRK" +
+       "MIIDMqADAgECAg0B47SaoY2KqYElaVC4MA0GCSqGSIb3DQEBCwUAMEwxIDAeBgNVBAsTF0ds" +
+       "b2JhbFNpZ24gUm9vdCBDQSAtIFIyMRMwEQYDVQQKEwpHbG9iYWxTaWduMRMwEQYDVQQDEwpH" +
+       "bG9iYWxTaWduMB4XDTE3MDYxNTAwMDA0MloXDTIxMTIxNTAwMDA0MlowQjELMAkGA1UEBhMC" +
+       "VVMxHjAcBgNVBAoTFUdvb2dsZSBUcnVzdCBTZXJ2aWNlczETMBEGA1UEAxMKR1RTIENBIDFP" +
+       "MTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANAYz0XUi83TnORA73603WkhG8nP" +
+       "PI5MdbkPMRmEPZ48Ke9QDRCTbwWAgJ8qoL0SSwLhPZ9YFiT+MJ8LdHdVkx1L903hkoIQ9lGs" +
+       "DMOyIpQPNGuYEEnnC52DOd0gxhwt79EYYWXnI4MgqCMS/9Ikf9Qv50RqW03XUGawr55CYwX7" +
+       "4BzEY2Gvn2oz/2KXvUjZ03wUZ9x13C5p6PhteGnQtxAFuPExwjsk/RozdPgj4OxrGYoWxuPN" +
+       "pM0L27OkWWA4iDutHbnGjKdTG/y82aSrvN08YdeTFZjugb2P4mRHIEAGTtesl+i5wFkSoUkl" +
+       "I+TtcDQspbRjfPmjPYPRzW0krAcCAwEAAaOCATMwggEvMA4GA1UdDwEB/wQEAwIBhjAdBgNV" +
+       "HSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwEgYDVR0TAQH/BAgwBgEB/wIBADAdBgNVHQ4E" +
+       "FgQUmNH4bhDrz5vsYJ8YkBug630J/SswHwYDVR0jBBgwFoAUm+IHV2ccHsBqBt5ZtJot39wZ" +
+       "hi4wNQYIKwYBBQUHAQEEKTAnMCUGCCsGAQUFBzABhhlodHRwOi8vb2NzcC5wa2kuZ29vZy9n" +
+       "c3IyMDIGA1UdHwQrMCkwJ6AloCOGIWh0dHA6Ly9jcmwucGtpLmdvb2cvZ3NyMi9nc3IyLmNy" +
+       "bDA/BgNVHSAEODA2MDQGBmeBDAECAjAqMCgGCCsGAQUFBwIBFhxodHRwczovL3BraS5nb29n" +
+       "L3JlcG9zaXRvcnkvMA0GCSqGSIb3DQEBCwUAA4IBAQAagD42efvzLqlGN31eVBY1rsdOCJn+" +
+       "vdE0aSZSZgc9CrpJy2L08RqO/BFPaJZMdCvTZ96yo6oFjYRNTCBlD6WW2g0W+Gw7228EI4hr" +
+       "OmzBYL1on3GO7i1YNAfw1VTphln9e14NIZT1jMmo+NjyrcwPGvOap6kEJ/mjybD/AnhrYbrH" +
+       "NSvoVvpPwxwM7bY8tEvq7czhPOzcDYzWPpvKQliLzBYhF0C8otZm79rEFVvNiaqbCSbnMtIN" +
+       "bmcgAlsQsJAJnAwfnq3YO+qh/GzoEFwIUhlRKnG7rHq13RXtK8kIKiyKtKYhq2P/11JJUNCJ" +
+       "t63yr/tQri/hlQ3zRq2dnPXK"
 
 func parseCIDR(s string) *net.IPNet {
        _, net, err := net.ParseCIDR(s)
@@ -2054,11 +2056,11 @@ func TestEmptyNameConstraints(t *testing.T) {
 }
 
 func TestPKIXNameString(t *testing.T) {
-       pem, err := hex.DecodeString(certBytes)
+       der, err := base64.StdEncoding.DecodeString(certBytes)
        if err != nil {
                t.Fatal(err)
        }
-       certs, err := ParseCertificates(pem)
+       certs, err := ParseCertificates(der)
        if err != nil {
                t.Fatal(err)
        }
@@ -2079,7 +2081,7 @@ func TestPKIXNameString(t *testing.T) {
                        Country:            []string{"GB"},
                }, "SERIALNUMBER=RFC 2253,CN=Steve Kille,OU=RFCs,O=Isode Limited,POSTALCODE=TW9 1DT,STREET=The Square,L=Richmond,ST=Surrey,C=GB"},
                {certs[0].Subject,
-                       "CN=mail.google.com,O=Google Inc,L=Mountain View,ST=California,C=US"},
+                       "CN=mail.google.com,O=Google LLC,L=Mountain View,ST=California,C=US"},
                {pkix.Name{
                        Organization: []string{"#Google, Inc. \n-> 'Alphabet\" "},
                        Country:      []string{"US"},