Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS
Mirrored from https://github.com/Slazee/caddy.git
Synced Nov 13, 09:15 UTC
httpcaddyfile: Improve detection of indistinguishable TLS automation policies (#5120)
* httpcaddyfile: Skip some logic if auto_https off * Try removing this check altogether... * Refine test timeouts slightly, sigh * caddyhttp: Assume udp for unrecognized network type Seems like the reasonable thing to do if a plugin registers its own network type. * Add comment to document my lack of knowledge * Clean up and prepare to merge Add comments to try to explain what happened
Matt Holt committed
Oct 13, 2022 at 11:30 UTC
6bad878a22e048762262d6fabe2144cefaf4ca81
5 files changed
+56
-58
caddyconfig/httpcaddyfile/tlsapp.go
+44
-44
@@ -44,41 +44,32 @@ func (st ServerType) buildTLSApp(
44
if hp, ok := options["http_port"].(int); ok {
45
httpPort = strconv.Itoa(hp)
46
}
47
- httpsPort := strconv.Itoa(caddyhttp.DefaultHTTPSPort)
48
- if hsp, ok := options["https_port"].(int); ok {
49
- httpsPort = strconv.Itoa(hsp)
50
- }
47
autoHTTPS := "on"
48
if ah, ok := options["auto_https"].(string); ok {
49
autoHTTPS = ah
50
}
51
56
- // count how many server blocks have a TLS-enabled key with
57
- // no host, and find all hosts that share a server block with
58
- // a hostless key, so that they don't get forgotten/omitted
59
- // by auto-HTTPS (since they won't appear in route matchers)
60
- var serverBlocksWithTLSHostlessKey int
52
+ // find all hosts that share a server block with a hostless
53
+ // key, so that they don't get forgotten/omitted by auto-HTTPS
54
+ // (since they won't appear in route matchers)
55
httpsHostsSharedWithHostlessKey := make(map[string]struct{})
62
- for _, pair := range pairings {
63
- for _, sb := range pair.serverBlocks {
64
- for _, addr := range sb.keys {
65
- if addr.Host == "" {
66
- // this address has no hostname, but if it's explicitly set
67
- // to HTTPS, then we need to count it as being TLS-enabled
68
- if addr.Scheme == "https" || addr.Port == httpsPort {
69
- serverBlocksWithTLSHostlessKey++
70
- }
71
- // this server block has a hostless key, now
72
- // go through and add all the hosts to the set
73
- for _, otherAddr := range sb.keys {
74
- if otherAddr.Original == addr.Original {
75
- continue
76
- }
77
- if otherAddr.Host != "" && otherAddr.Scheme != "http" && otherAddr.Port != httpPort {
78
- httpsHostsSharedWithHostlessKey[otherAddr.Host] = struct{}{}
56
+ if autoHTTPS != "off" {
57
+ for _, pair := range pairings {
58
+ for _, sb := range pair.serverBlocks {
59
+ for _, addr := range sb.keys {
60
+ if addr.Host == "" {
61
+ // this server block has a hostless key, now
62
+ // go through and add all the hosts to the set
63
+ for _, otherAddr := range sb.keys {
64
+ if otherAddr.Original == addr.Original {
65
+ continue
66
+ }
67
+ if otherAddr.Host != "" && otherAddr.Scheme != "http" && otherAddr.Port != httpPort {
68
+ httpsHostsSharedWithHostlessKey[otherAddr.Host] = struct{}{}
69
+ }
70
}
71
+ break
72
}
81
- break
73
}
74
}
75
}
@@ -138,6 +129,19 @@ func (st ServerType) buildTLSApp(
129
issuers = append(issuers, issuerVal.Value.(certmagic.Issuer))
130
}
131
if ap == catchAllAP && !reflect.DeepEqual(ap.Issuers, issuers) {
132
+ // this more correctly implements an error check that was removed
133
+ // below; try it with this config:
134
+ //
135
+ // :443 {
136
+ // bind 127.0.0.1
137
+ // }
138
+ //
139
+ // :443 {
140
+ // bind ::1
141
+ // tls {
142
+ // issuer acme
143
+ // }
144
+ // }
145
return nil, warnings, fmt.Errorf("automation policy from site block is also default/catch-all policy because of key without hostname, and the two are in conflict: %#v != %#v", ap.Issuers, issuers)
146
}
147
ap.Issuers = issuers
@@ -180,29 +184,25 @@ func (st ServerType) buildTLSApp(
184
}
185
}
186
183
- // first make sure this block is allowed to create an automation policy;
184
- // doing so is forbidden if it has a key with no host (i.e. ":443")
187
+ // we used to ensure this block is allowed to create an automation policy;
188
+ // doing so was forbidden if it has a key with no host (i.e. ":443")
189
// and if there is a different server block that also has a key with no
190
// host -- since a key with no host matches any host, we need its
191
// associated automation policy to have an empty Subjects list, i.e. no
192
// host filter, which is indistinguishable between the two server blocks
193
// because automation is not done in the context of a particular server...
194
// this is an example of a poor mapping from Caddyfile to JSON but that's
191
- // the least-leaky abstraction I could figure out
192
- if len(sblockHosts) == 0 {
193
- if serverBlocksWithTLSHostlessKey > 1 {
194
- // this server block and at least one other has a key with no host,
195
- // making the two indistinguishable; it is misleading to define such
196
- // a policy within one server block since it actually will apply to
197
- // others as well
198
- 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")
199
- }
200
- if catchAllAP == nil {
201
- // this server block has a key with no hosts, but there is not yet
202
- // a catch-all automation policy (probably because no global options
203
- // were set), so this one becomes it
204
- catchAllAP = ap
205
- }
195
+ // the least-leaky abstraction I could figure out -- however, this check
196
+ // was preventing certain listeners, like those provided by plugins, from
197
+ // being used as desired (see the Tailscale listener plugin), so I removed
198
+ // the check: and I think since I originally wrote the check I added a new
199
+ // check above which *properly* detects this ambiguity without breaking the
200
+ // listener plugin; see the check above with a commented example config
201
+ if len(sblockHosts) == 0 && catchAllAP == nil {
202
+ // this server block has a key with no hosts, but there is not yet
203
+ // a catch-all automation policy (probably because no global options
204
+ // were set), so this one becomes it
205
+ catchAllAP = ap
206
}
207
208
// associate our new automation policy with this server block's hosts
caddytest/integration/reverseproxy_test.go
+2
-2
@@ -436,8 +436,8 @@ func TestReverseProxyHealthCheck(t *testing.T) {
436
437
health_uri /health
438
health_port 2021
439
- health_interval 2s
440
- health_timeout 5s
439
+ health_interval 10ms
440
+ health_timeout 100ms
441
}
442
}
443
`, "caddyfile")
go.mod
+3
-3
@@ -14,7 +14,7 @@ require (
14
github.com/google/uuid v1.3.0
15
github.com/klauspost/compress v1.15.11
16
github.com/klauspost/cpuid/v2 v2.1.1
17
- github.com/lucas-clemente/quic-go v0.29.1
17
+ github.com/lucas-clemente/quic-go v0.29.2
18
github.com/mholt/acmez v1.0.4
19
github.com/prometheus/client_golang v1.12.2
20
github.com/smallstep/certificates v0.22.1
@@ -87,8 +87,8 @@ require (
87
github.com/libdns/libdns v0.2.1 // indirect
88
github.com/manifoldco/promptui v0.9.0 // indirect
89
github.com/marten-seemann/qpack v0.2.1 // indirect
90
- github.com/marten-seemann/qtls-go1-18 v0.1.2 // indirect
91
- github.com/marten-seemann/qtls-go1-19 v0.1.0 // indirect
90
+ github.com/marten-seemann/qtls-go1-18 v0.1.3 // indirect
91
+ github.com/marten-seemann/qtls-go1-19 v0.1.1 // indirect
92
github.com/mattn/go-colorable v0.1.8 // indirect
93
github.com/mattn/go-isatty v0.0.13 // indirect
94
github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
go.sum
+6
-6
@@ -423,18 +423,18 @@ github.com/libdns/libdns v0.2.1 h1:Wu59T7wSHRgtA0cfxC+n1c/e+O3upJGWytknkmFEDis=
423
github.com/libdns/libdns v0.2.1/go.mod h1:yQCXzk1lEZmmCPa857bnk4TsOiqYasqpyOEeSObbb40=
424
github.com/lightstep/lightstep-tracer-common/golang/gogo v0.0.0-20190605223551-bc2310a04743/go.mod h1:qklhhLq1aX+mtWk9cPHPzaBjWImj5ULL6C7HFJtXQMM=
425
github.com/lightstep/lightstep-tracer-go v0.18.1/go.mod h1:jlF1pusYV4pidLvZ+XD0UBX0ZE6WURAspgAczcDHrL4=
426
-github.com/lucas-clemente/quic-go v0.29.1 h1:Z+WMJ++qMLhvpFkRZA+jl3BTxUjm415YBmWanXB8zP0=
427
-github.com/lucas-clemente/quic-go v0.29.1/go.mod h1:CTcNfLYJS2UuRNB+zcNlgvkjBhxX6Hm3WUxxAQx2mgE=
426
+github.com/lucas-clemente/quic-go v0.29.2 h1:O8Mt0O6LpvEW+wfC40vZdcw0DngwYzoxq5xULZNzSI8=
427
+github.com/lucas-clemente/quic-go v0.29.2/go.mod h1:g6/h9YMmLuU54tL1gW25uIi3VlBp3uv+sBihplIuskE=
428
github.com/lyft/protoc-gen-validate v0.0.13/go.mod h1:XbGvPuh87YZc5TdIa2/I4pLk0QoUACkjt2znoq26NVQ=
429
github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
430
github.com/manifoldco/promptui v0.9.0 h1:3V4HzJk1TtXW1MTZMP7mdlwbBpIinw3HztaIlYthEiA=
431
github.com/manifoldco/promptui v0.9.0/go.mod h1:ka04sppxSGFAtxX0qhlYQjISsg9mR4GWtQEhdbn6Pgg=
432
github.com/marten-seemann/qpack v0.2.1 h1:jvTsT/HpCn2UZJdP+UUB53FfUUgeOyG5K1ns0OJOGVs=
433
github.com/marten-seemann/qpack v0.2.1/go.mod h1:F7Gl5L1jIgN1D11ucXefiuJS9UMVP2opoCp2jDKb7wc=
434
-github.com/marten-seemann/qtls-go1-18 v0.1.2 h1:JH6jmzbduz0ITVQ7ShevK10Av5+jBEKAHMntXmIV7kM=
435
-github.com/marten-seemann/qtls-go1-18 v0.1.2/go.mod h1:mJttiymBAByA49mhlNZZGrH5u1uXYZJ+RW28Py7f4m4=
436
-github.com/marten-seemann/qtls-go1-19 v0.1.0 h1:rLFKD/9mp/uq1SYGYuVZhm83wkmU95pK5df3GufyYYU=
437
-github.com/marten-seemann/qtls-go1-19 v0.1.0/go.mod h1:5HTDWtVudo/WFsHKRNuOhWlbdjrfs5JHrYb0wIJqGpI=
434
+github.com/marten-seemann/qtls-go1-18 v0.1.3 h1:R4H2Ks8P6pAtUagjFty2p7BVHn3XiwDAl7TTQf5h7TI=
435
+github.com/marten-seemann/qtls-go1-18 v0.1.3/go.mod h1:mJttiymBAByA49mhlNZZGrH5u1uXYZJ+RW28Py7f4m4=
436
+github.com/marten-seemann/qtls-go1-19 v0.1.1 h1:mnbxeq3oEyQxQXwI4ReCgW9DPoPR94sNlqWoDZnjRIE=
437
+github.com/marten-seemann/qtls-go1-19 v0.1.1/go.mod h1:5HTDWtVudo/WFsHKRNuOhWlbdjrfs5JHrYb0wIJqGpI=
438
github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
439
github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcnceauSikq3lYCQ=
440
github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
modules/caddyhttp/server.go
+1
-3
@@ -500,14 +500,12 @@ func (s *Server) serveHTTP3(addr caddy.NetworkAddress, tlsCfg *tls.Config) error
500
switch addr.Network {
501
case "unix":
502
addr.Network = "unixgram"
503
- case "tcp":
504
- addr.Network = "udp"
503
case "tcp4":
504
addr.Network = "udp4"
505
case "tcp6":
506
addr.Network = "udp6"
507
default:
510
- return fmt.Errorf("unsure what network to use for HTTP/3 given network type: %s", addr.Network)
508
+ addr.Network = "udp" // TODO: Maybe a better default is to not enable HTTP/3 if we do not know the network?
509
}
510
511
lnAny, err := addr.Listen(s.ctx, 0, net.ListenConfig{})