aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorFrancis Lavoie <[email protected]>2024-10-30 13:09:12 -0400
committerGitHub <[email protected]>2024-10-30 17:09:12 +0000
commitb129ed6be88e40667a843bfab74abb3e5239bc8f (patch)
tree0d4e1b47ded49079123b0f1fcd8f558c2d2c28fc
parentd398898b352a6a7e8ac5c24da01dd948fc334d77 (diff)
downloadcaddy-b129ed6be88e40667a843bfab74abb3e5239bc8f.tar.gz
caddy-b129ed6be88e40667a843bfab74abb3e5239bc8f.zip
httpcaddyfile: Fixes for `prefer_wildcard` mode (#6636)
* httpcaddyfile: Fixes for prefer_wildcard mode The wildcard hosts need to be collected first, then considered after, because there's no guarantee that all non-wildcards will appear after all wildcards when looping. Also we should not add a domain to Skip if it doesn't qualify for TLS anyway. * Alternate solution by avoiding adding APs altogether if covered by wildcard
-rw-r--r--caddyconfig/httpcaddyfile/httptype.go42
-rw-r--r--caddyconfig/httpcaddyfile/tlsapp.go91
-rw-r--r--caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard.caddyfiletest3
-rw-r--r--caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard_multi.caddyfiletest268
4 files changed, 353 insertions, 51 deletions
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index a238a33b4..704424acb 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -706,6 +706,16 @@ func (st *ServerType) serversFromPairings(
return specificity(iLongestHost) > specificity(jLongestHost)
})
+ // collect all hosts that have a wildcard in them
+ wildcardHosts := []string{}
+ for _, sblock := range p.serverBlocks {
+ for _, addr := range sblock.parsedKeys {
+ if strings.HasPrefix(addr.Host, "*.") {
+ wildcardHosts = append(wildcardHosts, addr.Host[2:])
+ }
+ }
+ }
+
var hasCatchAllTLSConnPolicy, addressQualifiesForTLS bool
autoHTTPSWillAddConnPolicy := srv.AutoHTTPS == nil || !srv.AutoHTTPS.Disabled
@@ -791,13 +801,6 @@ func (st *ServerType) serversFromPairings(
}
}
- wildcardHosts := []string{}
- for _, addr := range sblock.parsedKeys {
- if strings.HasPrefix(addr.Host, "*.") {
- wildcardHosts = append(wildcardHosts, addr.Host[2:])
- }
- }
-
for _, addr := range sblock.parsedKeys {
// if server only uses HTTP port, auto-HTTPS will not apply
if listenersUseAnyPortOtherThan(srv.Listen, httpPort) {
@@ -813,18 +816,6 @@ func (st *ServerType) serversFromPairings(
}
}
- // If prefer wildcard is enabled, then we add hosts that are
- // already covered by the wildcard to the skip list
- if srv.AutoHTTPS != nil && srv.AutoHTTPS.PreferWildcard && addr.Scheme == "https" {
- baseDomain := addr.Host
- if idx := strings.Index(baseDomain, "."); idx != -1 {
- baseDomain = baseDomain[idx+1:]
- }
- if !strings.HasPrefix(addr.Host, "*.") && slices.Contains(wildcardHosts, baseDomain) {
- srv.AutoHTTPS.Skip = append(srv.AutoHTTPS.Skip, addr.Host)
- }
- }
-
// If TLS is specified as directive, it will also result in 1 or more connection policy being created
// Thus, catch-all address with non-standard port, e.g. :8443, can have TLS enabled without
// specifying prefix "https://"
@@ -841,6 +832,19 @@ func (st *ServerType) serversFromPairings(
(addr.Scheme != "http" && addr.Port != httpPort && hasTLSEnabled) {
addressQualifiesForTLS = true
}
+
+ // If prefer wildcard is enabled, then we add hosts that are
+ // already covered by the wildcard to the skip list
+ if addressQualifiesForTLS && srv.AutoHTTPS != nil && srv.AutoHTTPS.PreferWildcard {
+ baseDomain := addr.Host
+ if idx := strings.Index(baseDomain, "."); idx != -1 {
+ baseDomain = baseDomain[idx+1:]
+ }
+ if !strings.HasPrefix(addr.Host, "*.") && slices.Contains(wildcardHosts, baseDomain) {
+ srv.AutoHTTPS.SkipCerts = append(srv.AutoHTTPS.SkipCerts, addr.Host)
+ }
+ }
+
// 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 &&
diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go
index ed708524d..bec860610 100644
--- a/caddyconfig/httpcaddyfile/tlsapp.go
+++ b/caddyconfig/httpcaddyfile/tlsapp.go
@@ -92,6 +92,25 @@ func (st ServerType) buildTLSApp(
tlsApp.Automation.Policies = append(tlsApp.Automation.Policies, catchAllAP)
}
+ // collect all hosts that have a wildcard in them, and arent HTTP
+ wildcardHosts := []string{}
+ for _, p := range pairings {
+ var addresses []string
+ for _, addressWithProtocols := range p.addressesWithProtocols {
+ addresses = append(addresses, addressWithProtocols.address)
+ }
+ if !listenersUseAnyPortOtherThan(addresses, httpPort) {
+ continue
+ }
+ for _, sblock := range p.serverBlocks {
+ for _, addr := range sblock.parsedKeys {
+ if strings.HasPrefix(addr.Host, "*.") {
+ wildcardHosts = append(wildcardHosts, addr.Host[2:])
+ }
+ }
+ }
+ }
+
for _, p := range pairings {
// avoid setting up TLS automation policies for a server that is HTTP-only
var addresses []string
@@ -115,6 +134,12 @@ func (st ServerType) buildTLSApp(
return nil, warnings, err
}
+ // make a plain copy so we can compare whether we made any changes
+ apCopy, err := newBaseAutomationPolicy(options, warnings, true)
+ if err != nil {
+ return nil, warnings, err
+ }
+
sblockHosts := sblock.hostsFromKeys(false)
if len(sblockHosts) == 0 && catchAllAP != nil {
ap = catchAllAP
@@ -217,9 +242,21 @@ func (st ServerType) buildTLSApp(
catchAllAP = ap
}
+ hostsNotHTTP := sblock.hostsFromKeysNotHTTP(httpPort)
+ sort.Strings(hostsNotHTTP) // solely for deterministic test results
+
+ // if the we prefer wildcards and the AP is unchanged,
+ // then we can skip this AP because it should be covered
+ // by an AP with a wildcard
+ if slices.Contains(autoHTTPS, "prefer_wildcard") {
+ if hostsCoveredByWildcard(hostsNotHTTP, wildcardHosts) &&
+ reflect.DeepEqual(ap, apCopy) {
+ continue
+ }
+ }
+
// associate our new automation policy with this server block's hosts
- ap.SubjectsRaw = sblock.hostsFromKeysNotHTTP(httpPort)
- sort.Strings(ap.SubjectsRaw) // solely for deterministic test results
+ ap.SubjectsRaw = hostsNotHTTP
// if a combination of public and internal names were given
// for this same server block and no issuer was specified, we
@@ -258,6 +295,7 @@ func (st ServerType) buildTLSApp(
ap2.IssuersRaw = []json.RawMessage{caddyconfig.JSONModuleObject(caddytls.InternalIssuer{}, "module", "internal", &warnings)}
}
}
+
if tlsApp.Automation == nil {
tlsApp.Automation = new(caddytls.AutomationConfig)
}
@@ -418,10 +456,7 @@ func (st ServerType) buildTLSApp(
}
// consolidate automation policies that are the exact same
- tlsApp.Automation.Policies = consolidateAutomationPolicies(
- tlsApp.Automation.Policies,
- slices.Contains(autoHTTPS, "prefer_wildcard"),
- )
+ tlsApp.Automation.Policies = consolidateAutomationPolicies(tlsApp.Automation.Policies)
// ensure automation policies don't overlap subjects (this should be
// an error at provision-time as well, but catch it in the adapt phase
@@ -567,7 +602,7 @@ func newBaseAutomationPolicy(
// consolidateAutomationPolicies combines automation policies that are the same,
// for a cleaner overall output.
-func consolidateAutomationPolicies(aps []*caddytls.AutomationPolicy, preferWildcard bool) []*caddytls.AutomationPolicy {
+func consolidateAutomationPolicies(aps []*caddytls.AutomationPolicy) []*caddytls.AutomationPolicy {
// sort from most specific to least specific; we depend on this ordering
sort.SliceStable(aps, func(i, j int) bool {
if automationPolicyIsSubset(aps[i], aps[j]) {
@@ -652,31 +687,6 @@ outer:
j--
}
}
-
- if preferWildcard {
- // remove subjects from i if they're covered by a wildcard in j
- iSubjs := aps[i].SubjectsRaw
- for iSubj := 0; iSubj < len(iSubjs); iSubj++ {
- for jSubj := range aps[j].SubjectsRaw {
- if !strings.HasPrefix(aps[j].SubjectsRaw[jSubj], "*.") {
- continue
- }
- if certmagic.MatchWildcard(aps[i].SubjectsRaw[iSubj], aps[j].SubjectsRaw[jSubj]) {
- iSubjs = slices.Delete(iSubjs, iSubj, iSubj+1)
- iSubj--
- break
- }
- }
- }
- aps[i].SubjectsRaw = iSubjs
-
- // remove i if it has no subjects left
- if len(aps[i].SubjectsRaw) == 0 {
- aps = slices.Delete(aps, i, i+1)
- i--
- continue outer
- }
- }
}
}
@@ -748,3 +758,20 @@ func automationPolicyHasAllPublicNames(ap *caddytls.AutomationPolicy) bool {
func isTailscaleDomain(name string) bool {
return strings.HasSuffix(strings.ToLower(name), ".ts.net")
}
+
+func hostsCoveredByWildcard(hosts []string, wildcards []string) bool {
+ if len(hosts) == 0 || len(wildcards) == 0 {
+ return false
+ }
+ for _, host := range hosts {
+ for _, wildcard := range wildcards {
+ if strings.HasPrefix(host, "*.") {
+ continue
+ }
+ if certmagic.MatchWildcard(host, "*."+wildcard) {
+ return true
+ }
+ }
+ }
+ return false
+}
diff --git a/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard.caddyfiletest b/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard.caddyfiletest
index 8880d71ae..04f2c4665 100644
--- a/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard.caddyfiletest
+++ b/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard.caddyfiletest
@@ -74,6 +74,9 @@ foo.example.com {
}
],
"automatic_https": {
+ "skip_certificates": [
+ "foo.example.com"
+ ],
"prefer_wildcard": true
}
}
diff --git a/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard_multi.caddyfiletest b/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard_multi.caddyfiletest
new file mode 100644
index 000000000..4f8c26a5d
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard_multi.caddyfiletest
@@ -0,0 +1,268 @@
+{
+ auto_https prefer_wildcard
+}
+
+# Covers two domains
+*.one.example.com {
+ tls {
+ dns mock
+ }
+ respond "one fallback"
+}
+
+# Is covered, should not get its own AP
+foo.one.example.com {
+ respond "foo one"
+}
+
+# This one has its own tls config so it doesn't get covered (escape hatch)
+bar.one.example.com {
+ respond "bar one"
+}
+
+# Covers nothing but AP gets consolidated with the first
+*.two.example.com {
+ tls {
+ dns mock
+ }
+ respond "two fallback"
+}
+
+# Is HTTP so it should not cover
+http://*.three.example.com {
+ respond "three fallback"
+}
+
+# Has no wildcard coverage so it gets an AP
+foo.three.example.com {
+ respond "foo three"
+}
+----------
+{
+ "apps": {
+ "http": {
+ "servers": {
+ "srv0": {
+ "listen": [
+ ":443"
+ ],
+ "routes": [
+ {
+ "match": [
+ {
+ "host": [
+ "foo.three.example.com"
+ ]
+ }
+ ],
+ "handle": [
+ {
+ "handler": "subroute",
+ "routes": [
+ {
+ "handle": [
+ {
+ "body": "foo three",
+ "handler": "static_response"
+ }
+ ]
+ }
+ ]
+ }
+ ],
+ "terminal": true
+ },
+ {
+ "match": [
+ {
+ "host": [
+ "foo.one.example.com"
+ ]
+ }
+ ],
+ "handle": [
+ {
+ "handler": "subroute",
+ "routes": [
+ {
+ "handle": [
+ {
+ "body": "foo one",
+ "handler": "static_response"
+ }
+ ]
+ }
+ ]
+ }
+ ],
+ "terminal": true
+ },
+ {
+ "match": [
+ {
+ "host": [
+ "bar.one.example.com"
+ ]
+ }
+ ],
+ "handle": [
+ {
+ "handler": "subroute",
+ "routes": [
+ {
+ "handle": [
+ {
+ "body": "bar one",
+ "handler": "static_response"
+ }
+ ]
+ }
+ ]
+ }
+ ],
+ "terminal": true
+ },
+ {
+ "match": [
+ {
+ "host": [
+ "*.one.example.com"
+ ]
+ }
+ ],
+ "handle": [
+ {
+ "handler": "subroute",
+ "routes": [
+ {
+ "handle": [
+ {
+ "body": "one fallback",
+ "handler": "static_response"
+ }
+ ]
+ }
+ ]
+ }
+ ],
+ "terminal": true
+ },
+ {
+ "match": [
+ {
+ "host": [
+ "*.two.example.com"
+ ]
+ }
+ ],
+ "handle": [
+ {
+ "handler": "subroute",
+ "routes": [
+ {
+ "handle": [
+ {
+ "body": "two fallback",
+ "handler": "static_response"
+ }
+ ]
+ }
+ ]
+ }
+ ],
+ "terminal": true
+ }
+ ],
+ "automatic_https": {
+ "skip_certificates": [
+ "foo.one.example.com",
+ "bar.one.example.com"
+ ],
+ "prefer_wildcard": true
+ }
+ },
+ "srv1": {
+ "listen": [
+ ":80"
+ ],
+ "routes": [
+ {
+ "match": [
+ {
+ "host": [
+ "*.three.example.com"
+ ]
+ }
+ ],
+ "handle": [
+ {
+ "handler": "subroute",
+ "routes": [
+ {
+ "handle": [
+ {
+ "body": "three fallback",
+ "handler": "static_response"
+ }
+ ]
+ }
+ ]
+ }
+ ],
+ "terminal": true
+ }
+ ],
+ "automatic_https": {
+ "prefer_wildcard": true
+ }
+ }
+ }
+ },
+ "tls": {
+ "automation": {
+ "policies": [
+ {
+ "subjects": [
+ "foo.three.example.com"
+ ]
+ },
+ {
+ "subjects": [
+ "bar.one.example.com"
+ ],
+ "issuers": [
+ {
+ "email": "[email protected]",
+ "module": "acme"
+ },
+ {
+ "ca": "https://acme.zerossl.com/v2/DV90",
+ "email": "[email protected]",
+ "module": "acme"
+ }
+ ]
+ },
+ {
+ "subjects": [
+ "*.one.example.com",
+ "*.two.example.com"
+ ],
+ "issuers": [
+ {
+ "challenges": {
+ "dns": {
+ "provider": {
+ "name": "mock"
+ }
+ }
+ },
+ "module": "acme"
+ }
+ ]
+ }
+ ]
+ }
+ }
+ }
+} \ No newline at end of file