Skip to content

Commit

Permalink
acme/autocert: make host of TLS certificate to be obtained always Pun…
Browse files Browse the repository at this point in the history
…ycode

According to the [RFC 5280, section 4.2.1.6](https://tools.ietf.org/html/rfc5280#section-4.2.1.6):

> ...
> When the subjectAltName extension contains a domain name system
> label, the domain name MUST be stored in the dNSName (an IA5String).
> The name MUST be in the "preferred name syntax", as specified by
> Section 3.5 of [RFC1034] and as modified by Section 2.1 of
> [RFC1123].  Note that while uppercase and lowercase letters are
> allowed in domain names, no significance is attached to the case.
> ...

And the [RFC 1034, section 3.5](https://tools.ietf.org/html/rfc1034#section-3.5):

> ...
> Note that while upper and lower case letters are allowed in domain
> names, no significance is attached to the case.  That is, two names with
> the same spelling but different case are to be treated as if identical.
> ...

We should return the same TLS certificate for both `example.com` and `EXAMPLE.COM`. But the `autocert.Manager.GetCertificate` treats the two as totally different, it signs and returns two different TLS certificates.

Moreover, now the `autocert.Manager.GetCertificate` and the `autocert.HostWhitelist` can only handle Punycode IDN. If the client sends a Unicode IDN to `autocert.Manager.GetCertificate` (cURL is doing this), the "Invalid character in DNS name" error will be triggered.

This PR corrects these problems by converting the host of the TLS certificate to be obtained to Punycode via `idna.Lookup.ToASCII`.

Change-Id: I993821b3a6ae532a53772e2db00524479ef111af
GitHub-Last-Rev: 6c12694
GitHub-Pull-Request: golang#85
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/171997
Reviewed-by: Brad Fitzpatrick <[email protected]>
  • Loading branch information
aofei authored and bradfitz committed Apr 22, 2019
1 parent af44ce2 commit f856061
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 6 deletions.
21 changes: 19 additions & 2 deletions acme/autocert/autocert.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"time"

"golang.org/x/crypto/acme"
"golang.org/x/net/idna"
)

// createCertRetryAfter is how much time to wait before removing a failed state
Expand Down Expand Up @@ -62,10 +63,16 @@ type HostPolicy func(ctx context.Context, host string) error
// HostWhitelist returns a policy where only the specified host names are allowed.
// Only exact matches are currently supported. Subdomains, regexp or wildcard
// will not match.
//
// Note that all hosts will be converted to Punycode via idna.Lookup.ToASCII so that
// Manager.GetCertificate can handle the Unicode IDN and mixedcase hosts correctly.
// Invalid hosts will be silently ignored.
func HostWhitelist(hosts ...string) HostPolicy {
whitelist := make(map[string]bool, len(hosts))
for _, h := range hosts {
whitelist[h] = true
if h, err := idna.Lookup.ToASCII(h); err == nil {
whitelist[h] = true
}
}
return func(_ context.Context, host string) error {
if !whitelist[host] {
Expand Down Expand Up @@ -243,7 +250,17 @@ func (m *Manager) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate,
if !strings.Contains(strings.Trim(name, "."), ".") {
return nil, errors.New("acme/autocert: server name component count invalid")
}
if strings.ContainsAny(name, `+/\`) {

// Note that this conversion is necessary because some server names in the handshakes
// started by some clients (such as cURL) are not converted to Punycode, which will
// prevent us from obtaining certificates for them. In addition, we should also treat
// example.com and EXAMPLE.COM as equivalent and return the same certificate for them.
// Fortunately, this conversion also helped us deal with this kind of mixedcase problems.
//
// Due to the "σςΣ" problem (see https://unicode.org/faq/idn.html#22), we can't use
// idna.Punycode.ToASCII (or just idna.ToASCII) here.
name, err := idna.Lookup.ToASCII(name)
if err != nil {
return nil, errors.New("acme/autocert: server name contains invalid character")
}

Expand Down
25 changes: 24 additions & 1 deletion acme/autocert/autocert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,28 @@ func TestGetCertificate_trailingDot(t *testing.T) {
testGetCertificate(t, man, "example.org", hello)
}

func TestGetCertificate_unicodeIDN(t *testing.T) {
man := &Manager{Prompt: AcceptTOS}
defer man.stopRenew()

hello := clientHelloInfo("σσσ.com", true)
testGetCertificate(t, man, "xn--4xaaa.com", hello)

hello = clientHelloInfo("σςΣ.com", true)
testGetCertificate(t, man, "xn--4xaaa.com", hello)
}

func TestGetCertificate_mixedcase(t *testing.T) {
man := &Manager{Prompt: AcceptTOS}
defer man.stopRenew()

hello := clientHelloInfo("example.org", true)
testGetCertificate(t, man, "example.org", hello)

hello = clientHelloInfo("EXAMPLE.ORG", true)
testGetCertificate(t, man, "example.org", hello)
}

func TestGetCertificate_ForceRSA(t *testing.T) {
man := &Manager{
Prompt: AcceptTOS,
Expand Down Expand Up @@ -906,13 +928,14 @@ func TestCache(t *testing.T) {
}

func TestHostWhitelist(t *testing.T) {
policy := HostWhitelist("example.com", "example.org", "*.example.net")
policy := HostWhitelist("example.com", "EXAMPLE.ORG", "*.example.net", "σςΣ.com")
tt := []struct {
host string
allow bool
}{
{"example.com", true},
{"example.org", true},
{"xn--4xaaa.com", true},
{"one.example.com", false},
{"two.example.org", false},
{"three.example.net", false},
Expand Down
5 changes: 4 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
module golang.org/x/crypto

require golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e
require (
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3
golang.org/x/sys v0.0.0-20190412213103-97732733099d
)
10 changes: 8 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e h1:nFYrTHrdrAOpShe27kaFHjsqYSEQ0KWqdWLu3xuZJts=
golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 h1:0GoQqolDA55aaLxZyTzK/Y2ePZzZTUrRacwib7cNsYQ=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d h1:+R4KGOnez64A81RvjARKc4UT5/tI9ujCIVX+P5KiHuI=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=

0 comments on commit f856061

Please sign in to comment.