From f81aa23cf04f2be5fd2159e855e185b2d75ba503 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Thu, 30 Apr 2020 22:35:35 -0400 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Katie Hockman --- src/crypto/x509/verify.go | 4 ++-- src/crypto/x509/verify_test.go | 2 +- src/crypto/x509/x509_test.go | 10 +++++++++- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go index a9516fc375d074..a058f349c54804 100644 --- 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 18271540c75131..650b2d2fc6e9ec 100644 --- 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 d69c8ba72ee474..7e001471dd4b67 100644 --- 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) {