diff options
-rw-r--r-- | caddyconfig/httpcaddyfile/httptype.go | 28 | ||||
-rw-r--r-- | caddyconfig/httpcaddyfile/tlsapp.go | 41 | ||||
-rw-r--r-- | modules/caddyhttp/autohttps.go | 19 |
3 files changed, 61 insertions, 27 deletions
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index a9ee838..a22dd40 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/caddyconfig/httpcaddyfile/httptype.go @@ -324,6 +324,10 @@ func (st *ServerType) serversFromPairings( 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) + } for i, p := range pairings { srv := &caddyhttp.Server{ @@ -362,7 +366,8 @@ func (st *ServerType) serversFromPairings( return specificity(iLongestHost) > specificity(jLongestHost) }) - var hasCatchAllTLSConnPolicy, usesTLS bool + var hasCatchAllTLSConnPolicy, addressQualifiesForTLS bool + autoHTTPSWillAddConnPolicy := true // create a subroute for each site in the server block for _, sblock := range p.serverBlocks { @@ -401,9 +406,9 @@ func (st *ServerType) serversFromPairings( } } - // exclude any hosts that were defined explicitly with - // "http://" in the key from automated cert management (issue #2998) 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) @@ -412,9 +417,16 @@ func (st *ServerType) serversFromPairings( srv.AutoHTTPS.Skip = append(srv.AutoHTTPS.Skip, addr.Host) } } - if addr.Scheme != "http" && addr.Host != "" && addr.Port != httpPort { - usesTLS = true + // 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" || + (addr.Scheme != "http" && addr.Host != "" && addr.Port != httpPort) { + addressQualifiesForTLS = true } + // predict whether auto-HTTPS will add the conn policy for us; if so, we + // may not need to add one for this server + autoHTTPSWillAddConnPolicy = autoHTTPSWillAddConnPolicy && + (addr.Port == httpsPort || (addr.Port != httpPort && addr.Host != "")) } // set up each handler directive, making sure to honor directive order @@ -477,9 +489,9 @@ func (st *ServerType) serversFromPairings( // 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 // policy missing for any HTTPS-enabled hosts, if so, add it... maybe? - if usesTLS && + if addressQualifiesForTLS && !hasCatchAllTLSConnPolicy && - (len(srv.TLSConnPolicies) > 0 || defaultSNI != "") { + (len(srv.TLSConnPolicies) > 0 || !autoHTTPSWillAddConnPolicy || defaultSNI != "") { srv.TLSConnPolicies = append(srv.TLSConnPolicies, &caddytls.ConnectionPolicy{DefaultSNI: defaultSNI}) } @@ -539,7 +551,7 @@ func detectConflictingSchemes(srv *caddyhttp.Server, serverBlocks []serverBlock, if err := checkAndSetHTTP(addr); err != nil { return err } - } else if addr.Scheme == "https" || addr.Port == httpsPort { + } else if addr.Scheme == "https" || addr.Port == httpsPort || len(srv.TLSConnPolicies) > 0 { if err := checkAndSetHTTPS(addr); err != nil { return err } diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go index 5c62233..f66fcae 100644 --- a/caddyconfig/httpcaddyfile/tlsapp.go +++ b/caddyconfig/httpcaddyfile/tlsapp.go @@ -20,9 +20,11 @@ import ( "fmt" "reflect" "sort" + "strconv" "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/caddyserver/caddy/v2/modules/caddytls" "github.com/caddyserver/certmagic" ) @@ -36,17 +38,26 @@ func (st ServerType) buildTLSApp( tlsApp := &caddytls.TLS{CertificatesRaw: make(caddy.ModuleMap)} var certLoaders []caddytls.CertificateLoader - // count how many server blocks have a key with no host, - // and find all hosts that share a server block with a - // hostless key, so that they don't get forgotten/omitted + httpsPort := strconv.Itoa(caddyhttp.DefaultHTTPSPort) + if hsp, ok := options["https_port"].(int); ok { + httpsPort = strconv.Itoa(hsp) + } + + // count how many server blocks have a TLS-enabled key with + // no host, and find all hosts that share a server block with + // a hostless key, so that they don't get forgotten/omitted // by auto-HTTPS (since they won't appear in route matchers) - var serverBlocksWithHostlessKey int + var serverBlocksWithTLSHostlessKey int hostsSharedWithHostlessKey := make(map[string]struct{}) for _, pair := range pairings { for _, sb := range pair.serverBlocks { for _, addr := range sb.keys { if addr.Host == "" { - serverBlocksWithHostlessKey++ + // this address has no hostname, but if it's explicitly set + // to HTTPS, then we need to count it as being TLS-enabled + if addr.Scheme == "https" || addr.Port == httpsPort { + serverBlocksWithTLSHostlessKey++ + } // this server block has a hostless key, now // go through and add all the hosts to the set for _, otherAddr := range sb.keys { @@ -149,9 +160,11 @@ func (st ServerType) buildTLSApp( } if ap != nil { - // encode issuer now that it's all set up - issuerName := ap.Issuer.(caddy.Module).CaddyModule().ID.Name() - ap.IssuerRaw = caddyconfig.JSONModuleObject(ap.Issuer, "module", issuerName, &warnings) + if ap.Issuer != nil { + // encode issuer now that it's all set up + issuerName := ap.Issuer.(caddy.Module).CaddyModule().ID.Name() + ap.IssuerRaw = caddyconfig.JSONModuleObject(ap.Issuer, "module", issuerName, &warnings) + } // first make sure this block is allowed to create an automation policy; // doing so is forbidden if it has a key with no host (i.e. ":443") @@ -163,12 +176,12 @@ func (st ServerType) buildTLSApp( // this is an example of a poor mapping from Caddyfile to JSON but that's // the least-leaky abstraction I could figure out if len(sblockHosts) == 0 { - if serverBlocksWithHostlessKey > 1 { + if serverBlocksWithTLSHostlessKey > 1 { // this server block and at least one other has a key with no host, // making the two indistinguishable; it is misleading to define such // a policy within one server block since it actually will apply to // others as well - return nil, warnings, fmt.Errorf("cannot make a TLS automation policy from a server block that has a host-less address when there are other server block addresses lacking a host") + return nil, warnings, fmt.Errorf("cannot make a TLS automation policy from a server block that has a host-less address when there are other TLS-enabled server block addresses lacking a host") } if catchAllAP == nil { // this server block has a key with no hosts, but there is not yet @@ -293,9 +306,11 @@ func (st ServerType) buildTLSApp( // if there is a global/catch-all automation policy, ensure it goes last if catchAllAP != nil { - // first, encode its issuer - issuerName := catchAllAP.Issuer.(caddy.Module).CaddyModule().ID.Name() - catchAllAP.IssuerRaw = caddyconfig.JSONModuleObject(catchAllAP.Issuer, "module", issuerName, &warnings) + // first, encode its issuer, if there is one + if catchAllAP.Issuer != nil { + issuerName := catchAllAP.Issuer.(caddy.Module).CaddyModule().ID.Name() + catchAllAP.IssuerRaw = caddyconfig.JSONModuleObject(catchAllAP.Issuer, "module", issuerName, &warnings) + } // then append it to the end of the policies list if tlsApp.Automation == nil { diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go index ad0a716..f62543b 100644 --- a/modules/caddyhttp/autohttps.go +++ b/modules/caddyhttp/autohttps.go @@ -152,12 +152,12 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er } // nothing more to do here if there are no domains that qualify for - // automatic HTTPS or there are no explicit TLS connection policies; - // if there is at least one domain but no TLS conn policy, we'll add - // one below; if there is a TLS conn policy (meaning TLS is enabled) - // and no domains, it could be a catch-all with on-demand TLS, and - // in that case we would still need HTTP->HTTPS redirects, which we - // do below + // automatic HTTPS and there are no explicit TLS connection policies: + // if there is at least one domain but no TLS conn policy (F&&T), we'll + // add one below; if there are no domains but at least one TLS conn + // policy (meaning TLS is enabled) (T&&F), it could be a catch-all with + // on-demand TLS -- and in that case we would still need HTTP->HTTPS + // redirects, which we set up below; hence these two conditions if len(serverDomainSet) == 0 && len(srv.TLSConnPolicies) == 0 { continue } @@ -345,6 +345,13 @@ uniqueDomainsLoop: // not entirely clear what the redirect destination should be, // so I'm going to just hard-code the app's HTTPS port and call // it good for now... + // TODO: This implies that all plaintext requests will be blindly + // redirected to their HTTPS equivalent, even if this server + // doesn't handle that hostname at all; I don't think this is a + // bad thing, and it also obscures the actual hostnames that this + // server is configured to match on, which may be desirable, but + // it's not something that should be relied on. We can change this + // if we want to. appendCatchAll := func(routes []Route) []Route { redirTo := "https://{http.request.host}" if app.httpsPort() != DefaultHTTPSPort { |