[go] crypto/x509: ignore Common Name by default
239 views
Skip to first unread message
Filippo Valsorda (Gerrit)
Apr 30, 2020, 10:51:55 PM4/30/20
to goph...@pubsubhelper.golang.org, Filippo Valsorda, golang-co...@googlegroups.com
Filippo Valsorda has uploaded this change for review.
crypto/x509: ignore Common Name by default
Had to refresh a test certificates that was too old to have SANs.
Updates #24151
Change-Id: I2ab78577fd936ba67969d3417284dbe46e4ae02f
---
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509_test.go
3 files changed, 70 insertions(+), 62 deletions(-)
diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 356965d..838dc93 100644
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -18,8 +18,8 @@
"unicode/utf8"
)
-// ignoreCN disables interpreting Common Name as a hostname. See issue 24151.
-var ignoreCN = strings.Contains(os.Getenv("GODEBUG"), "x509ignoreCN=1")
+// ignoreCN enables interpreting Common Name as a hostname. See issue 24151.
+var ignoreCN = !strings.Contains(os.Getenv("GODEBUG"), "x509ignoreCN=0")
type InvalidReason int
@@ -48,9 +48,9 @@
// 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 @@
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 && !validHostnamePattern(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 && validHostnamePattern(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
@@ -1028,10 +1034,10 @@
// against the DNSNames field. If the names are valid hostnames, the certificate
// fields can have a wildcard in 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 86fe76a..8a9036a 100644
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -374,7 +374,7 @@
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 @@
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 81d54ee..9870a96 100644
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -432,7 +432,7 @@
}
func TestCertificateParse(t *testing.T) {
- s, _ := hex.DecodeString(certBytes)
+ s, _ := base64.StdEncoding.DecodeString(certBytes)
certs, err := ParseCertificates(s)
if err != nil {
t.Error(err)
@@ -451,7 +451,7 @@
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)
}
@@ -495,48 +495,50 @@
}
}
-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)
@@ -2053,11 +2055,11 @@
}
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)
}
@@ -2078,7 +2080,7 @@
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"},
To view, visit change 231379. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
Apr 30, 2020, 10:52:18 PM4/30/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
TryBots beginning. Status page: https://farmer.golang.org/try?commit=8cec6401
Gobot Gobot (Gerrit)
Apr 30, 2020, 10:54:43 PM4/30/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Build is still in progress...
This change failed on misc-compile-ppc:
See https://storage.googleapis.com/go-build-log/8cec6401/misc-compile-ppc_f66dae21.log
Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test *exactly* your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.
Gobot Gobot (Gerrit)
Apr 30, 2020, 11:01:18 PM4/30/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
20 of 20 TryBots failed:
Failed on misc-compile-ppc: https://storage.googleapis.com/go-build-log/8cec6401/misc-compile-ppc_f66dae21.log
Failed on misc-compile-linuxarm: https://storage.googleapis.com/go-build-log/8cec6401/misc-compile-linuxarm_2d02a5e4.log
Failed on misc-compile-solaris: https://storage.googleapis.com/go-build-log/8cec6401/misc-compile-solaris_be60aad6.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/8cec6401/linux-amd64_9e7c2c39.log
Failed on js-wasm: https://storage.googleapis.com/go-build-log/8cec6401/js-wasm_c9cdb4ff.log
Failed on freebsd-amd64-12_0: https://storage.googleapis.com/go-build-log/8cec6401/freebsd-amd64-12_0_20f3b756.log
Failed on misc-compile-freebsd: https://storage.googleapis.com/go-build-log/8cec6401/misc-compile-freebsd_6e43242e.log
Failed on misc-compile-netbsd: https://storage.googleapis.com/go-build-log/8cec6401/misc-compile-netbsd_cf16a16c.log
Failed on android-amd64-emu: https://storage.googleapis.com/go-build-log/8cec6401/android-amd64-emu_779a7690.log
Failed on misc-compile-other: https://storage.googleapis.com/go-build-log/8cec6401/misc-compile-other_6767ac3c.log
Failed on (x/tools) linux-amd64: https://storage.googleapis.com/go-build-log/8cec6401/linux-amd64_0d143572.log
Failed on misc-compile-openbsd: https://storage.googleapis.com/go-build-log/8cec6401/misc-compile-openbsd_b9fd2721.log
Failed on misc-compile-darwin: https://storage.googleapis.com/go-build-log/8cec6401/misc-compile-darwin_5c738261.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/8cec6401/windows-386-2008_49dd63fd.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/8cec6401/windows-amd64-2016_bcabbedb.log
Failed on misc-compile-plan9: https://storage.googleapis.com/go-build-log/8cec6401/misc-compile-plan9_5fe3c4c7.log
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/8cec6401/openbsd-amd64-64_01c05fb0.log
Failed on misc-compile-mips: https://storage.googleapis.com/go-build-log/8cec6401/misc-compile-mips_6508157f.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/8cec6401/linux-386_4dc022f2.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/8cec6401/linux-amd64-race_232abeac.log
Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test *exactly* your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.
Patch set 1:TryBot-Result -1
Filippo Valsorda (Gerrit)
May 1, 2020, 1:23:35 AM5/1/20
to Filippo Valsorda, Gobot Gobot, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Filippo Valsorda uploaded patch set #2 to this change.
crypto/x509: ignore Common Name by default
Had to refresh a test certificates that was too old to have SANs.
Updates #24151
Change-Id: I2ab78577fd936ba67969d3417284dbe46e4ae02f
---
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509_test.go
3 files changed, 70 insertions(+), 62 deletions(-)
To view, visit change 231379. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
May 1, 2020, 1:24:05 AM5/1/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Gobot Gobot (Gerrit)
May 1, 2020, 1:35:33 AM5/1/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
TryBots are happy.
Patch set 2:TryBot-Result +1
Filippo Valsorda (Gerrit)
May 5, 2020, 1:53:21 AM5/5/20
to Filippo Valsorda, Gobot Gobot, Katie Hockman, Ryan Sleevi, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Filippo Valsorda uploaded patch set #3 to this change.
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
---
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509_test.go
3 files changed, 70 insertions(+), 62 deletions(-)
To view, visit change 231379. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
May 5, 2020, 1:53:38 AM5/5/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, Ryan Sleevi, Katie Hockman, golang-co...@googlegroups.com
Gobot Gobot (Gerrit)
May 5, 2020, 2:05:11 AM5/5/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, Ryan Sleevi, Katie Hockman, golang-co...@googlegroups.com
TryBots are happy.
Patch set 3:TryBot-Result +1
Katie Hockman (Gerrit)
May 5, 2020, 6:18:42 PM5/5/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, Gobot Gobot, Ryan Sleevi, golang-co...@googlegroups.com
Just a few comments, looks good otherwise!
Patch set 3:Code-Review +2
2 comments:
File src/crypto/x509/verify.go:
Patch Set #3, Line 21:
enablesI think I see what you're doing here, but isn't this backwards? If ignoreCN is true, then we *aren't* interpreting the Common Name as a hostname, since we're ignoring it, right?
Patch Set #3, Line 953:
disabled if the Subject Alt Name extension is present"disabled by default or if the Subject Alt Name extension is present"
To view, visit change 231379. To unsubscribe, or for help writing mail filters, visit settings.
Filippo Valsorda (Gerrit)
May 7, 2020, 7:40:18 PM5/7/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, Katie Hockman, Gobot Gobot, Ryan Sleevi, golang-co...@googlegroups.com
2 comments:
Patch Set #3, Line 21:
enables
I think I see what you're doing here, but isn't this backwards? If ignoreCN is true, then we *aren't […]
Ah yep, thanks
Patch Set #3, Line 953:
disabled if the Subject Alt Name extension is present"disabled by default or if the Subject Alt Name extension is present"
Done
To view, visit change 231379. To unsubscribe, or for help writing mail filters, visit settings.
Filippo Valsorda (Gerrit)
May 7, 2020, 7:40:18 PM5/7/20
to Filippo Valsorda, Gobot Gobot, Katie Hockman, Ryan Sleevi, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Filippo Valsorda uploaded patch set #4 to this change.
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
---
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509_test.go
3 files changed, 70 insertions(+), 62 deletions(-)
To view, visit change 231379. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
May 7, 2020, 7:40:39 PM5/7/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, Katie Hockman, Ryan Sleevi, golang-co...@googlegroups.com
Gobot Gobot (Gerrit)
May 7, 2020, 8:00:13 PM5/7/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, Katie Hockman, Ryan Sleevi, golang-co...@googlegroups.com
TryBots are happy.
Patch set 4:TryBot-Result +1
Filippo Valsorda (Gerrit)
May 7, 2020, 8:05:34 PM5/7/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gobot Gobot, Katie Hockman, Ryan Sleevi, golang-co...@googlegroups.com
Filippo Valsorda submitted this change.
Approvals:
Katie Hockman: Looks good to me, approved
Filippo Valsorda: Run TryBots
Gobot Gobot: TryBots succeeded
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 <fil...@golang.org>
TryBot-Result: Gobot Gobot <go...@golang.org>
Reviewed-by: Katie Hockman <ka...@golang.org>
---
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509_test.go
3 files changed, 70 insertions(+), 62 deletions(-)
diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 7427c57..e8886c1 100644
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -19,7 +19,7 @@
)
// 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 @@
// 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 @@
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 @@
// 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 @@
// against the DNSNames field. If the names are valid hostnames, the certificate
// fields can have a wildcard as the left-most label.index 7e431a6..f29e322 100644
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -433,7 +433,7 @@
}
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 @@
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 @@@@ -2054,11 +2056,11 @@
}
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 @@
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"},
To view, visit change 231379. To unsubscribe, or for help writing mail filters, visit settings.
Russ Cox (Gerrit)
Jun 26, 2020, 12:01:11 PM6/26/20
to Filippo Valsorda, goph...@pubsubhelper.golang.org, Russ Cox, Gobot Gobot, Katie Hockman, Ryan Sleevi, golang-co...@googlegroups.com
RELNOTES=yes
Reply all
Reply to author
Forward
0 new messages