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).

1 files changed, 9 insertions, 3 deletions

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++ {