diff options
author | Matthew Holt <[email protected]> | 2018-03-17 17:05:30 -0600 |
---|---|---|
committer | Matthew Holt <[email protected]> | 2018-03-17 17:05:30 -0600 |
commit | ef40659c7047978f8f5150d94a3139021e0ad173 (patch) | |
tree | 661375609c13179fde9fa04936222d52dbca4672 | |
parent | 3afb1ae380e7405e58c6166d98dc30ef54e73e14 (diff) | |
parent | 6e2de19d9fc29f8dec059c5345819a68a13832f2 (diff) | |
download | caddy-ef40659c7047978f8f5150d94a3139021e0ad173.tar.gz caddy-ef40659c7047978f8f5150d94a3139021e0ad173.zip |
Merge branch 'master' into acmev2
-rw-r--r-- | caddytls/certificates.go | 8 | ||||
-rw-r--r-- | caddytls/certificates_test.go | 9 | ||||
-rw-r--r-- | caddytls/crypto.go | 7 | ||||
-rw-r--r-- | caddytls/handshake.go | 31 | ||||
-rw-r--r-- | caddytls/handshake_test.go | 9 |
5 files changed, 35 insertions, 29 deletions
diff --git a/caddytls/certificates.go b/caddytls/certificates.go index 29c0c8c21..a0d9fb9c4 100644 --- a/caddytls/certificates.go +++ b/caddytls/certificates.go @@ -261,21 +261,21 @@ func fillCertFromLeaf(cert *Certificate, tlsCert tls.Certificate) error { return err } - if leaf.Subject.CommonName != "" { + if leaf.Subject.CommonName != "" { // TODO: CommonName is deprecated cert.Names = []string{strings.ToLower(leaf.Subject.CommonName)} } for _, name := range leaf.DNSNames { - if name != leaf.Subject.CommonName { + if name != leaf.Subject.CommonName { // TODO: CommonName is deprecated cert.Names = append(cert.Names, strings.ToLower(name)) } } for _, ip := range leaf.IPAddresses { - if ipStr := ip.String(); ipStr != leaf.Subject.CommonName { + if ipStr := ip.String(); ipStr != leaf.Subject.CommonName { // TODO: CommonName is deprecated cert.Names = append(cert.Names, strings.ToLower(ipStr)) } } for _, email := range leaf.EmailAddresses { - if email != leaf.Subject.CommonName { + if email != leaf.Subject.CommonName { // TODO: CommonName is deprecated cert.Names = append(cert.Names, strings.ToLower(email)) } } diff --git a/caddytls/certificates_test.go b/caddytls/certificates_test.go index 817d16496..5f8b17e18 100644 --- a/caddytls/certificates_test.go +++ b/caddytls/certificates_test.go @@ -43,10 +43,11 @@ func TestUnexportedGetCertificate(t *testing.T) { t.Errorf("Didn't get wildcard cert for 'sub.example.com' or got the wrong one: %v, matched=%v, defaulted=%v", cert, matched, defaulted) } - // When no certificate matches and SNI is provided, return no certificate (should be TLS alert) - if cert, matched, defaulted := cfg.getCertificate("nomatch"); matched || defaulted { - t.Errorf("Expected matched=false, defaulted=false; but got matched=%v, defaulted=%v (cert: %v)", matched, defaulted, cert) - } + // TODO: Re-implement this behavior when I'm not in the middle of upgrading for ACMEv2 support. :) (it was reverted in #2037) + // // When no certificate matches and SNI is provided, return no certificate (should be TLS alert) + // if cert, matched, defaulted := cfg.getCertificate("nomatch"); matched || defaulted { + // t.Errorf("Expected matched=false, defaulted=false; but got matched=%v, defaulted=%v (cert: %v)", matched, defaulted, cert) + // } // When no certificate matches and SNI is NOT provided, a random is returned if cert, matched, defaulted := cfg.getCertificate(""); matched || !defaulted { diff --git a/caddytls/crypto.go b/caddytls/crypto.go index f2ea4f9b5..51cab7f4d 100644 --- a/caddytls/crypto.go +++ b/caddytls/crypto.go @@ -218,10 +218,13 @@ func makeSelfSignedCert(config *Config) error { KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, } + var names []string if ip := net.ParseIP(config.Hostname); ip != nil { + names = append(names, strings.ToLower(ip.String())) cert.IPAddresses = append(cert.IPAddresses, ip) } else { - cert.DNSNames = append(cert.DNSNames, config.Hostname) + names = append(names, strings.ToLower(config.Hostname)) + cert.DNSNames = append(cert.DNSNames, strings.ToLower(config.Hostname)) } publicKey := func(privKey interface{}) interface{} { @@ -247,7 +250,7 @@ func makeSelfSignedCert(config *Config) error { PrivateKey: privKey, Leaf: cert, }, - Names: cert.DNSNames, + Names: names, NotAfter: cert.NotAfter, Hash: hashCertificateChain(chain), }) diff --git a/caddytls/handshake.go b/caddytls/handshake.go index 841d06cd0..1e8586d26 100644 --- a/caddytls/handshake.go +++ b/caddytls/handshake.go @@ -59,10 +59,9 @@ func (cg configGroup) getConfig(name string) *Config { } } - // try a config that serves all names (this - // is basically the same as a config defined - // for "*" -- I think -- but the above loop - // doesn't try an empty string) + // try a config that serves all names (the above + // loop doesn't try empty string; for hosts defined + // with only a port, for instance, like ":443") if config, ok := cg[""]; ok { return config } @@ -166,17 +165,19 @@ func (cfg *Config) getCertificate(name string) (cert Certificate, matched, defau return } - // if nothing matches and SNI was not provided, use a random - // certificate; at least there's a chance this older client - // can connect, and in the future we won't need this provision - // (if SNI is present, it's probably best to just raise a TLS - // alert by not serving a certificate) - if name == "" { - for _, certKey := range cfg.Certificates { - defaulted = true - cert = cfg.certCache.cache[certKey] - return - } + // if nothing matches, use a random certificate + // TODO: This is not my favorite behavior; I would rather serve + // no certificate if SNI is provided and cause a TLS alert, than + // serve the wrong certificate (but sometimes the 'wrong' cert + // is what is wanted, but in those cases I would prefer that the + // site owner explicitly configure a "default" certificate). + // (See issue 2035; any change to this behavior must account for + // hosts defined like ":443" or "0.0.0.0:443" where the hostname + // is empty or a catch-all IP or something.) + for _, certKey := range cfg.Certificates { + cert = cfg.certCache.cache[certKey] + defaulted = true + return } return diff --git a/caddytls/handshake_test.go b/caddytls/handshake_test.go index f0b8f7be2..bf427d245 100644 --- a/caddytls/handshake_test.go +++ b/caddytls/handshake_test.go @@ -27,7 +27,7 @@ func TestGetCertificate(t *testing.T) { hello := &tls.ClientHelloInfo{ServerName: "example.com"} helloSub := &tls.ClientHelloInfo{ServerName: "sub.example.com"} helloNoSNI := &tls.ClientHelloInfo{} - helloNoMatch := &tls.ClientHelloInfo{ServerName: "nomatch"} + // helloNoMatch := &tls.ClientHelloInfo{ServerName: "nomatch"} // TODO (see below) // When cache is empty if cert, err := cfg.GetCertificate(hello); err == nil { @@ -69,8 +69,9 @@ func TestGetCertificate(t *testing.T) { t.Errorf("Expected random cert with no matches, got: %v", cert) } + // TODO: Re-implement this behavior (it was reverted in #2037) // When no certificate matches, raise an alert - if _, err := cfg.GetCertificate(helloNoMatch); err == nil { - t.Errorf("Expected an error when no certificate matched the SNI, got: %v", err) - } + // if _, err := cfg.GetCertificate(helloNoMatch); err == nil { + // t.Errorf("Expected an error when no certificate matched the SNI, got: %v", err) + // } } |