crypto/x509: treat hostnames with colons as invalid

Colons are port separators, so it's risky to allow them in hostnames.
Per the CL 231377 rule, if we at least consider them invalid we will not
apply wildcard processing to them, making behavior a little more
predictable.

We were considering hostnames with colons valid (against spec) because
that meant we'd not ignore them in Common Name. (There was at least
one deployment that was putting colons in Common Name and expecting it
to verify.)

Now that Common Name is ignored by default, those clients will break
again, so it's a good time to drop the exception. Hopefully they moved
to SANs, where invalid hostnames are checked 1:1 (ignoring wildcards)
but still work. (If they didn't, this change means they can't use
GODEBUG=x509ignoreCN=0 to opt back in, but again you don't get to use a
legacy deprecated field AND invalid hostnames.)

Updates #24151

Change-Id: Id44b4fecb2d620480acdfc65fea1473f7abbca7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/231381
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 a9516fc375d074bb6fedd9d40463b493311912d0..a058f349c54804a972c6def1de0f952aed70886a 100644 (file)
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -940,8 +940,8 @@ func validHostname(host string, isPattern bool) bool {
                        if c == '-' && j != 0 {
                                continue
                        }
-                       if c == '_' || c == ':' {
-                               // Not valid characters in hostnames, but commonly
+                       if c == '_' {
+                               // Not a valid character in hostnames, but commonly
                                // found in deployments outside the WebPKI.
                                continue
                        }

diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
index 18271540c7513167293928ea8c680723488998a7..650b2d2fc6e9ecc898a2f59a3a6a9ce7befb7d25 100644 (file)
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -2004,7 +2004,7 @@ func TestValidHostname(t *testing.T) {
                {host: "foo.*.example.com"},
                {host: "exa_mple.com", validInput: true, validPattern: true},
                {host: "foo,bar"},
-               {host: "project-dev:us-central1:main", validInput: true, validPattern: true},
+               {host: "project-dev:us-central1:main"},
        }
        for _, tt := range tests {
                if got := validHostnamePattern(tt.host); got != tt.validPattern {

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index d69c8ba72ee474bc28b27fd88724a1f70c6623a1..7e001471dd4b673e0a3f1d4fb70d2788c869418b 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -376,7 +376,15 @@ var matchHostnamesTests = []matchHostnamesTest{
        {"*.com", "example.com", true},
        {"*.com", "example.com.", true},
        {"foo:bar", "foo:bar", true},
-       {"*.foo:bar", "xxx.foo:bar", true},
+       {"*.foo:bar", "xxx.foo:bar", false},
+       {"*.2.3.4", "1.2.3.4", false},
+       {"*.2.3.4", "[1.2.3.4]", false},
+       {"*:4860:4860::8888", "2001:4860:4860::8888", false},
+       {"*:4860:4860::8888", "[2001:4860:4860::8888]", false},
+       {"2001:4860:4860::8888", "2001:4860:4860::8888", false},
+       {"2001:4860:4860::8888", "[2001:4860:4860::8888]", false},
+       {"[2001:4860:4860::8888]", "2001:4860:4860::8888", false},
+       {"[2001:4860:4860::8888]", "[2001:4860:4860::8888]", false},
 }
 
 func TestMatchHostnames(t *testing.T) {