From d68cff8eb6211be10fc79d3e8d469562420b78cd Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 19 Jan 2021 14:16:06 -0700 Subject: httpcaddyfile: Skip TLS APs for HTTP-only hosts (fix #3977) This is probably an invasive change, but existing tests continue to pass. It seems to make sense this way. There is likely an edge case I haven't considered. --- caddyconfig/httpcaddyfile/httptype.go | 44 ++++++++++++++++----- caddyconfig/httpcaddyfile/tlsapp.go | 9 +++++ .../caddyfile_adapt/http_only_hostnames.txt | 45 ++++++++++++++++++++++ 3 files changed, 88 insertions(+), 10 deletions(-) create mode 100644 caddytest/integration/caddyfile_adapt/http_only_hostnames.txt diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index f16a0df..c6225df 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/caddyconfig/httpcaddyfile/httptype.go @@ -409,7 +409,7 @@ func (st *ServerType) serversFromPairings( var iLongestHost, jLongestHost string var iWildcardHost, jWildcardHost bool for _, addr := range p.serverBlocks[i].keys { - if strings.Contains(addr.Host, "*.") { + if strings.Contains(addr.Host, "*") { iWildcardHost = true } if specificity(addr.Host) > specificity(iLongestHost) { @@ -420,7 +420,7 @@ func (st *ServerType) serversFromPairings( } } for _, addr := range p.serverBlocks[j].keys { - if strings.Contains(addr.Host, "*.") { + if strings.Contains(addr.Host, "*") { jWildcardHost = true } if specificity(addr.Host) > specificity(jLongestHost) { @@ -500,16 +500,20 @@ func (st *ServerType) serversFromPairings( } for _, addr := range sblock.keys { - // exclude any hosts that were defined explicitly with "http://" - // in the key from automated cert management (issue #2998) - if addr.Scheme == "http" && addr.Host != "" { - if srv.AutoHTTPS == nil { - srv.AutoHTTPS = new(caddyhttp.AutoHTTPSConfig) - } - if !sliceContains(srv.AutoHTTPS.Skip, addr.Host) { - srv.AutoHTTPS.Skip = append(srv.AutoHTTPS.Skip, addr.Host) + // if server only uses HTTPS port, auto-HTTPS will not apply + if listenersUseAnyPortOtherThan(srv.Listen, httpPort) { + // exclude any hosts that were defined explicitly with "http://" + // in the key from automated cert management (issue #2998) + if addr.Scheme == "http" && addr.Host != "" { + if srv.AutoHTTPS == nil { + srv.AutoHTTPS = new(caddyhttp.AutoHTTPSConfig) + } + if !sliceContains(srv.AutoHTTPS.Skip, addr.Host) { + srv.AutoHTTPS.Skip = append(srv.AutoHTTPS.Skip, addr.Host) + } } } + // we'll need to remember if the address qualifies for auto-HTTPS, so we // can add a TLS conn policy if necessary if addr.Scheme == "https" || @@ -1183,6 +1187,26 @@ func sliceContains(haystack []string, needle string) bool { return false } +// listenersUseAnyPortOtherThan returns true if there are any +// listeners in addresses that use a port which is not otherPort. +// Mostly borrowed from unexported method in caddyhttp package. +func listenersUseAnyPortOtherThan(addresses []string, otherPort string) bool { + otherPortInt, err := strconv.Atoi(otherPort) + if err != nil { + return false + } + for _, lnAddr := range addresses { + laddrs, err := caddy.ParseNetworkAddress(lnAddr) + if err != nil { + continue + } + if uint(otherPortInt) > laddrs.EndPort || uint(otherPortInt) < laddrs.StartPort { + return true + } + } + return false +} + // specificity returns len(s) minus any wildcards (*) and // placeholders ({...}). Basically, it's a length count // that penalizes the use of wildcards and placeholders. diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go index 10b5e7d..dbf3cc7 100644 --- a/caddyconfig/httpcaddyfile/tlsapp.go +++ b/caddyconfig/httpcaddyfile/tlsapp.go @@ -40,6 +40,10 @@ func (st ServerType) buildTLSApp( tlsApp := &caddytls.TLS{CertificatesRaw: make(caddy.ModuleMap)} var certLoaders []caddytls.CertificateLoader + httpPort := strconv.Itoa(caddyhttp.DefaultHTTPPort) + if hp, ok := options["http_port"].(int); ok { + httpPort = strconv.Itoa(hp) + } httpsPort := strconv.Itoa(caddyhttp.DefaultHTTPSPort) if hsp, ok := options["https_port"].(int); ok { httpsPort = strconv.Itoa(hsp) @@ -91,6 +95,11 @@ func (st ServerType) buildTLSApp( } for _, p := range pairings { + // avoid setting up TLS automation policies for a server that is HTTP-only + if !listenersUseAnyPortOtherThan(p.addresses, httpPort) { + continue + } + for _, sblock := range p.serverBlocks { // get values that populate an automation policy for this block ap, err := newBaseAutomationPolicy(options, warnings, true) diff --git a/caddytest/integration/caddyfile_adapt/http_only_hostnames.txt b/caddytest/integration/caddyfile_adapt/http_only_hostnames.txt new file mode 100644 index 0000000..d867e16 --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/http_only_hostnames.txt @@ -0,0 +1,45 @@ +# https://github.com/caddyserver/caddy/issues/3977 +http://* { + respond "Hello, world!" +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":80" + ], + "routes": [ + { + "match": [ + { + "host": [ + "*" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Hello, world!", + "handler": "static_response" + } + ] + } + ] + } + ], + "terminal": true + } + ] + } + } + } + } +} \ No newline at end of file -- cgit v1.2.3