caddytls: Fix for TLS conn policy being applied to HTTP-only servers (#3243)

* httpcaddyfile: Don't add TLS policy to HTTP-only server (#3193, #3223)

* Account for HTTP port

* Add integration test written by @sarge
This commit is contained in:
Matt Holt 2020-04-09 12:39:05 -06:00 committed by GitHub
parent d33926b63f
commit d89ad2fd5b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 55 additions and 2 deletions

View file

@ -332,6 +332,11 @@ func (st *ServerType) serversFromPairings(
servers := make(map[string]*caddyhttp.Server) servers := make(map[string]*caddyhttp.Server)
defaultSNI := tryString(options["default_sni"], warnings) defaultSNI := tryString(options["default_sni"], warnings)
httpPort := strconv.Itoa(caddyhttp.DefaultHTTPPort)
if hp, ok := options["http_port"].(int); ok {
httpPort = strconv.Itoa(hp)
}
for i, p := range pairings { for i, p := range pairings {
srv := &caddyhttp.Server{ srv := &caddyhttp.Server{
Listen: p.addresses, Listen: p.addresses,
@ -369,7 +374,7 @@ func (st *ServerType) serversFromPairings(
return specificity(iLongestHost) > specificity(jLongestHost) return specificity(iLongestHost) > specificity(jLongestHost)
}) })
var hasCatchAllTLSConnPolicy bool var hasCatchAllTLSConnPolicy, usesTLS bool
// create a subroute for each site in the server block // create a subroute for each site in the server block
for _, sblock := range p.serverBlocks { for _, sblock := range p.serverBlocks {
@ -419,6 +424,9 @@ func (st *ServerType) serversFromPairings(
srv.AutoHTTPS.Skip = append(srv.AutoHTTPS.Skip, addr.Host) srv.AutoHTTPS.Skip = append(srv.AutoHTTPS.Skip, addr.Host)
} }
} }
if addr.Scheme != "http" && addr.Host != "" && addr.Port != httpPort {
usesTLS = true
}
} }
// set up each handler directive, making sure to honor directive order // set up each handler directive, making sure to honor directive order
@ -481,7 +489,9 @@ func (st *ServerType) serversFromPairings(
// TODO: maybe a smarter way to handle this might be to just make the // TODO: maybe a smarter way to handle this might be to just make the
// auto-HTTPS logic at provision-time detect if there is any connection // auto-HTTPS logic at provision-time detect if there is any connection
// policy missing for any HTTPS-enabled hosts, if so, add it... maybe? // policy missing for any HTTPS-enabled hosts, if so, add it... maybe?
if !hasCatchAllTLSConnPolicy && (len(srv.TLSConnPolicies) > 0 || defaultSNI != "") { if usesTLS &&
!hasCatchAllTLSConnPolicy &&
(len(srv.TLSConnPolicies) > 0 || defaultSNI != "") {
srv.TLSConnPolicies = append(srv.TLSConnPolicies, &caddytls.ConnectionPolicy{DefaultSNI: defaultSNI}) srv.TLSConnPolicies = append(srv.TLSConnPolicies, &caddytls.ConnectionPolicy{DefaultSNI: defaultSNI})
} }

View file

@ -272,3 +272,46 @@ func TestDefaultSNIWithPortMappingOnly(t *testing.T) {
// makes a request with no sni // makes a request with no sni
caddytest.AssertGetResponse(t, "https://127.0.0.1:9443/version", 200, "hello from a") caddytest.AssertGetResponse(t, "https://127.0.0.1:9443/version", 200, "hello from a")
} }
func TestHttpOnlyOnDomainWithSNI(t *testing.T) {
caddytest.AssertAdapt(t, `
{
default_sni a.caddy.localhost
}
:80 {
respond /version 200 {
body "hello from localhost"
}
}
`, "caddyfile", `{
"apps": {
"http": {
"servers": {
"srv0": {
"listen": [
":80"
],
"routes": [
{
"match": [
{
"path": [
"/version"
]
}
],
"handle": [
{
"body": "hello from localhost",
"handler": "static_response",
"status_code": 200
}
]
}
]
}
}
}
}
}`)
}