aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorFrancis Lavoie <[email protected]>2022-03-25 00:54:03 -0400
committerGitHub <[email protected]>2022-03-24 22:54:03 -0600
commita58f240d3ecbb59285303746406cab50217f8d24 (patch)
tree60da8ebee4a81e8ad608b56e8ab81b55101111a4
parent4b75f3e2f09b77b98783ea4d6602391c7283f984 (diff)
downloadcaddy-a58f240d3ecbb59285303746406cab50217f8d24.tar.gz
caddy-a58f240d3ecbb59285303746406cab50217f8d24.zip
httpcaddyfile: Fix #4640 (auto-HTTPS edgecase) (#4661)
Guh, this is complicated. Fixes #4640 This also follows up on #4398 (reverting it) which made a change that technically worked, but was incorrect. It changed the condition in `hostsFromKeysNotHTTP` from `&&` to `||`, but then the function no longer did what its name said it would do, and it would return hosts even if they were marked with `http://`, if they used a non-HTTP port. That wasn't the intent of it. The test added in there was kept though, because it is a valid usecase. The actual fix is to check _earlier_ whether all the addresses explicitly have `http://`, and if so we can short circuit and skip considering the rest.
-rw-r--r--caddyconfig/httpcaddyfile/directives.go13
-rw-r--r--caddyconfig/httpcaddyfile/httptype.go2
-rw-r--r--caddyconfig/httpcaddyfile/tlsapp.go6
-rw-r--r--caddytest/integration/caddyfile_adapt/tls_automation_policies_9.txt56
4 files changed, 75 insertions, 2 deletions
diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go
index 425bf192a..6b80e346d 100644
--- a/caddyconfig/httpcaddyfile/directives.go
+++ b/caddyconfig/httpcaddyfile/directives.go
@@ -494,7 +494,7 @@ func (sb serverBlock) hostsFromKeysNotHTTP(httpPort string) []string {
if addr.Host == "" {
continue
}
- if addr.Scheme != "http" || addr.Port != httpPort {
+ if addr.Scheme != "http" && addr.Port != httpPort {
hostMap[addr.Host] = struct{}{}
}
}
@@ -519,6 +519,17 @@ func (sb serverBlock) hasHostCatchAllKey() bool {
return false
}
+// isAllHTTP returns true if all sb keys explicitly specify
+// the http:// scheme
+func (sb serverBlock) isAllHTTP() bool {
+ for _, addr := range sb.keys {
+ if addr.Scheme != "http" {
+ return false
+ }
+ }
+ return true
+}
+
type (
// UnmarshalFunc is a function which can unmarshal Caddyfile
// tokens into zero or more config values using a Helper type.
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index d7716a42e..f5dd68a68 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -581,7 +581,7 @@ func (st *ServerType) serversFromPairings(
}
for _, addr := range sblock.keys {
- // if server only uses HTTPS port, auto-HTTPS will not apply
+ // if server only uses HTTP port, auto-HTTPS will not apply
if listenersUseAnyPortOtherThan(srv.Listen, httpPort) {
// exclude any hosts that were defined explicitly with "http://"
// in the key from automated cert management (issue #2998)
diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go
index daaec95df..76d7ebf36 100644
--- a/caddyconfig/httpcaddyfile/tlsapp.go
+++ b/caddyconfig/httpcaddyfile/tlsapp.go
@@ -101,6 +101,12 @@ func (st ServerType) buildTLSApp(
}
for _, sblock := range p.serverBlocks {
+ // check the scheme of all the site addresses,
+ // skip building AP if they all had http://
+ if sblock.isAllHTTP() {
+ continue
+ }
+
// get values that populate an automation policy for this block
ap, err := newBaseAutomationPolicy(options, warnings, true)
if err != nil {
diff --git a/caddytest/integration/caddyfile_adapt/tls_automation_policies_9.txt b/caddytest/integration/caddyfile_adapt/tls_automation_policies_9.txt
new file mode 100644
index 000000000..bd82e96c1
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/tls_automation_policies_9.txt
@@ -0,0 +1,56 @@
+# example from issue #4640
+http://foo:8447, http://127.0.0.1:8447 {
+ reverse_proxy 127.0.0.1:8080
+}
+----------
+{
+ "apps": {
+ "http": {
+ "servers": {
+ "srv0": {
+ "listen": [
+ ":8447"
+ ],
+ "routes": [
+ {
+ "match": [
+ {
+ "host": [
+ "foo",
+ "127.0.0.1"
+ ]
+ }
+ ],
+ "handle": [
+ {
+ "handler": "subroute",
+ "routes": [
+ {
+ "handle": [
+ {
+ "handler": "reverse_proxy",
+ "upstreams": [
+ {
+ "dial": "127.0.0.1:8080"
+ }
+ ]
+ }
+ ]
+ }
+ ]
+ }
+ ],
+ "terminal": true
+ }
+ ],
+ "automatic_https": {
+ "skip": [
+ "foo",
+ "127.0.0.1"
+ ]
+ }
+ }
+ }
+ }
+ }
+} \ No newline at end of file