From 4217217badf220d7d2c25f43f955fdc8454f2c64 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 11 Sep 2020 13:45:21 -0600 Subject: httpcaddyfile: Properly record whether we added catch-all conn policy We recently introduced `if !cp.SettingsEmpty()` which conditionally adds the connection policy to the list. If the condition evaluates to false, the policy wouldn't actually be added, even if hasCatchAllTLSConnPolicy was set to true on the previous line. Now we set that variable in accordance with whether we actually add the policy. While debugging this I noticed that catch-all policies added early in that loop (i.e. not at the end if we later determine we need one) are not always at the end of the list. They should be, though, since they are selected by which one matches first, and having a catch-all first would nullify any more specific ones later in the list. So I added a sort in consolidateConnPolicies to take care of that. Should fix #3670 and https://caddy.community/t/combining-on-demand-tls-with-custom-ssl-certs-doesnt-seem-to-work-in-2-1-1/9719 but I won't know for sure until somebody verifies it, since at least in the GitHub issue there is not yet enough information (the configs are redacted). --- caddyconfig/httpcaddyfile/httptype.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'caddyconfig') diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index 130067d..f2d1568 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/caddyconfig/httpcaddyfile/httptype.go @@ -446,12 +446,12 @@ func (st *ServerType) serversFromPairings( } } else { cp.DefaultSNI = defaultSNI - hasCatchAllTLSConnPolicy = true } // only append this policy if it actually changes something if !cp.SettingsEmpty() { srv.TLSConnPolicies = append(srv.TLSConnPolicies, cp) + hasCatchAllTLSConnPolicy = len(hosts) == 0 } } } @@ -653,9 +653,15 @@ func detectConflictingSchemes(srv *caddyhttp.Server, serverBlocks []serverBlock, return nil } -// consolidateConnPolicies removes empty TLS connection policies and combines -// equivalent ones for a cleaner overall output. +// consolidateConnPolicies sorts any catch-all policy to the end, removes empty TLS connection +// policies, and combines equivalent ones for a cleaner overall output. func consolidateConnPolicies(cps caddytls.ConnectionPolicies) (caddytls.ConnectionPolicies, error) { + // catch-all policies (those without any matcher) should be at the + // end, otherwise it nullifies any more specific policies + sort.SliceStable(cps, func(i, j int) bool { + return cps[j].MatchersRaw == nil && cps[i].MatchersRaw != nil + }) + for i := 0; i < len(cps); i++ { // compare it to the others for j := 0; j < len(cps); j++ { -- cgit v1.2.3