fix: Don't create fallback http(s) server when http(s) disabled

Before, a fallback http server was created to handle requests for unknown virtual hosts even when `HTTPS_METHOD=nohttp`. (In this case, all http vhosts would be unknown.) Likewise, a catch-all fallback https server was still created even if `HTTPS_METHOD=nohttps`. Now the fallback servers are created only if needed. This brings the behavior in line with the documentation and user expectation. It will also make it easier to implement a planned feature: different servers on different ports.
nginx-proxy · Mar 14, 2023 · 74d565d · 74d565d
1 parent 008fe8a
commit 74d565d
Show file tree

Hide file tree

Showing 8 changed files with 193 additions and 49 deletions.
diff --git a/nginx.tmpl b/nginx.tmpl
@@ -20,6 +20,7 @@
 {{- $_ := set $globals "access_log" (or (and (not $globals.Env.DISABLE_ACCESS_LOGS) "access_log /var/log/nginx/access.log vhost;") "") }}
 {{- $_ := set $globals "enable_ipv6" (parseBool (coalesce $globals.Env.ENABLE_IPV6 "false")) }}
 {{- $_ := set $globals "ssl_policy" (or ($globals.Env.SSL_POLICY) "Mozilla-Intermediate") }}
+{{- $_ := set $globals "vhosts" (dict) }}
 {{- $_ := set $globals "networks" (dict) }}
 # Networks available to the container running docker-gen (which are assumed to
 # match the networks available to the container running nginx):
@@ -310,22 +311,80 @@ proxy_set_header X-Original-URI $request_uri;
 proxy_set_header Proxy "";
 {{- end }}
 
+{{- /*
+     * Precompute some information about each vhost.  This is done early because
+     * the creation of fallback servers depends on DEFAULT_HOST, HTTPS_METHOD,
+     * and whether there are any missing certs.
+     */}}
+{{- range $vhost, $containers := groupByMulti $globals.containers "Env.VIRTUAL_HOST" "," }}
+    {{- $vhost := trim $vhost }}
+    {{- if not $vhost }}
+        {{- /* Ignore containers with VIRTUAL_HOST set to the empty string. */}}
+        {{- continue }}
+    {{- end }}
+    {{- $certName := first (groupByKeys $containers "Env.CERT_NAME") }}
+    {{- $vhostCert := closest (dir "/etc/nginx/certs") (printf "%s.crt" $vhost) }}
+    {{- $vhostCert = trimSuffix ".crt" $vhostCert }}
+    {{- $vhostCert = trimSuffix ".key" $vhostCert }}
+    {{- $cert := or $certName $vhostCert }}
+    {{- $cert_ok := and (ne $cert "") (exists (printf "/etc/nginx/certs/%s.crt" $cert)) (exists (printf "/etc/nginx/certs/%s.key" $cert)) }}
+    {{- $default := eq $globals.Env.DEFAULT_HOST $vhost }}
+    {{- $https_method := or (first (groupByKeys $containers "Env.HTTPS_METHOD")) $globals.Env.HTTPS_METHOD "redirect" }}
+    {{- $_ := set $globals.vhosts $vhost (dict "cert" $cert "cert_ok" $cert_ok "containers" $containers "default" $default "https_method" $https_method) }}
+{{- end }}
+
+{{- /*
+     * If needed, create a catch-all fallback server to send an error code to
+     * clients that request something from an unknown vhost.
+     */}}
+{{- block "fallback_server" $globals }}
+    {{- $globals := . }}
+    {{- $http_exists := false }}
+    {{- $https_exists := false }}
+    {{- $default_http_exists := false }}
+    {{- $default_https_exists := false }}
+    {{- range $vhost := $globals.vhosts }}
+        {{- $http := or (ne $vhost.https_method "nohttp") (not $vhost.cert_ok) }}
+        {{- $https := ne $vhost.https_method "nohttps" }}
+        {{- $http_exists = or $http_exists $http }}
+        {{- $https_exists = or $https_exists $https }}
+        {{- $default_http_exists = or $default_http_exists (and $http $vhost.default) }}
+        {{- $default_https_exists = or $default_https_exists (and $https $vhost.default) }}
+    {{- end }}
+    {{- $fallback_http := and $http_exists (not $default_http_exists) }}
+    {{- $fallback_https := and $https_exists (not $default_https_exists) }}
+    {{- /*
+         * If there are no vhosts at all, create fallbacks for both plain http
+         * and https so that clients get something more useful than a connection
+         * refused error.
+         */}}
+    {{- if and (not $http_exists) (not $https_exists) }}
+        {{- $fallback_http = true }}
+        {{- $fallback_https = true }}
+    {{- end }}
+    {{- if or $fallback_http $fallback_https }}
 server {
     server_name _; # This is just an invalid value which will never trigger on a real hostname.
     server_tokens off;
-    listen {{ $globals.external_http_port }};
-    listen {{ $globals.external_https_port }} ssl http2;
-{{- if $globals.enable_ipv6 }}
-    listen [::]:{{ $globals.external_http_port }};
-    listen [::]:{{ $globals.external_https_port }} ssl http2;
-{{- end }}
+        {{- if $fallback_http }}
+    listen {{ $globals.external_http_port }} default_server;
+                {{- if $globals.enable_ipv6 }}
+    listen [::]:{{ $globals.external_http_port }} default_server;
+                {{- end }}
+        {{- end }}
+        {{- if $fallback_https }}
+    listen {{ $globals.external_https_port }} ssl http2 default_server;
+            {{- if $globals.enable_ipv6 }}
+    listen [::]:{{ $globals.external_https_port }} ssl http2 default_server;
+            {{- end }}
+        {{- end }}
     {{ $globals.access_log }}
-{{- if $globals.default_cert_ok }}
+        {{- if $globals.default_cert_ok }}
     ssl_session_cache shared:SSL:50m;
     ssl_session_tickets off;
     ssl_certificate /etc/nginx/certs/default.crt;
     ssl_certificate_key /etc/nginx/certs/default.key;
-{{- else }}
+        {{- else }}
     # No default.crt certificate found for this vhost, so force nginx to emit a
     # TLS error if the client connects via https.
     {{- /* See the comment in the main `server` directive for rationale. */}}
@@ -336,17 +395,19 @@ server {
     if ($https) {
         return 444;
     }
-{{- end }}
+        {{- end }}
     return 503;
 }
+    {{- end }}
+{{- end }}
 
-{{- range $host, $containers := groupByMulti $globals.containers "Env.VIRTUAL_HOST" "," }}
+{{- range $host, $vhost := $globals.vhosts }}
+    {{- $cert := $vhost.cert }}
+    {{- $cert_ok := $vhost.cert_ok }}
+    {{- $containers := $vhost.containers }}
+    {{- $default_server := when $vhost.default "default_server" "" }}
+    {{- $https_method := $vhost.https_method }}
 
-    {{- $host := trim $host }}
-    {{- if not $host }}
-        {{- /* Ignore containers with VIRTUAL_HOST set to the empty string. */}}
-        {{- continue }}
-    {{- end }}
     {{- $is_regexp := hasPrefix "~" $host }}
     {{- $upstream_name := when (or $is_regexp $globals.sha1_upstream_name) (sha1 $host) $host }}
 
@@ -366,22 +427,12 @@ server {
 {{ template "upstream" (dict "globals" $globals "Upstream" $upstream "Containers" $containers) }}
     {{- end }}
 
-    {{- $default_host := or ($globals.Env.DEFAULT_HOST) "" }}
-    {{- $default_server := index (dict $host "" $default_host "default_server") $host }}
-
     {{- /*
          * Get the SERVER_TOKENS defined by containers w/ the same vhost,
          * falling back to "".
          */}}
     {{- $server_tokens := trim (or (first (groupByKeys $containers "Env.SERVER_TOKENS")) "") }}
 
-
-    {{- /*
-         * Get the HTTPS_METHOD defined by containers w/ the same vhost, falling
-         * back to "redirect".
-         */}}
-    {{- $https_method := or (first (groupByKeys $containers "Env.HTTPS_METHOD")) (or $globals.Env.HTTPS_METHOD "redirect") }}
-
     {{- /*
          * Get the SSL_POLICY defined by containers w/ the same vhost, falling
          * back to empty string (use default).
@@ -397,27 +448,6 @@ server {
     {{- /* Get the VIRTUAL_ROOT By containers w/ use fastcgi root */}}
     {{- $vhost_root := or (first (groupByKeys $containers "Env.VIRTUAL_ROOT")) "/var/www/public" }}
 
-
-    {{- /* Get the first cert name defined by containers w/ the same vhost */}}
-    {{- $certName := (first (groupByKeys $containers "Env.CERT_NAME")) }}
-
-    {{- /* Get the best matching cert by name for the vhost. */}}
-    {{- $vhostCert := (closest (dir "/etc/nginx/certs") (printf "%s.crt" $host))}}
-
-    {{- /*
-         * vhostCert is actually a filename so remove any suffixes since they
-         * are added later.
-         */}}
-    {{- $vhostCert := trimSuffix ".crt" $vhostCert }}
-    {{- $vhostCert := trimSuffix ".key" $vhostCert }}
-
-    {{- /*
-         * Use the cert specified on the container or fallback to the best vhost
-         * match.
-         */}}
-    {{- $cert := (coalesce $certName $vhostCert) }}
-    {{- $cert_ok := and (ne $cert "") (exists (printf "/etc/nginx/certs/%s.crt" $cert)) (exists (printf "/etc/nginx/certs/%s.key" $cert)) }}
-
     {{- if and $cert_ok (eq $https_method "redirect") }}
 server {
     server_name {{ $host }};

diff --git a/test/test_fallback.data/nohttp-on-app.yml b/test/test_fallback.data/nohttp-on-app.yml
@@ -0,0 +1,16 @@
+services:
+  sut:
+    image: nginxproxy/nginx-proxy:test
+    volumes:
+      - /var/run/docker.sock:/tmp/docker.sock:ro
+      - ./withdefault.certs:/etc/nginx/certs:ro
+    environment:
+      HTTPS_METHOD: redirect
+  https-only:
+    image: web
+    expose:
+      - "82"
+    environment:
+      WEB_PORTS: "82"
+      HTTPS_METHOD: nohttp
+      VIRTUAL_HOST: https-only.nginx-proxy.test
diff --git a/test/test_fallback.data/nohttp-with-missing-cert.yml b/test/test_fallback.data/nohttp-with-missing-cert.yml
@@ -0,0 +1,22 @@
+services:
+  sut:
+    image: nginxproxy/nginx-proxy:test
+    volumes:
+      - /var/run/docker.sock:/tmp/docker.sock:ro
+      - ./withdefault.certs:/etc/nginx/certs:ro
+    environment:
+      HTTPS_METHOD: nohttp
+  https-only:
+    image: web
+    expose:
+      - "82"
+    environment:
+      WEB_PORTS: "82"
+      VIRTUAL_HOST: https-only.nginx-proxy.test
+  missing-cert:
+    image: web
+    expose:
+      - "84"
+    environment:
+      WEB_PORTS: "84"
+      VIRTUAL_HOST: missing-cert.nginx-proxy.test
diff --git a/test/test_fallback.data/nohttp.yml b/test/test_fallback.data/nohttp.yml
@@ -0,0 +1,15 @@
+services:
+  sut:
+    image: nginxproxy/nginx-proxy:test
+    volumes:
+      - /var/run/docker.sock:/tmp/docker.sock:ro
+      - ./withdefault.certs:/etc/nginx/certs:ro
+    environment:
+      HTTPS_METHOD: nohttp
+  https-only:
+    image: web
+    expose:
+      - "82"
+    environment:
+      WEB_PORTS: "82"
+      VIRTUAL_HOST: https-only.nginx-proxy.test
diff --git a/test/test_fallback.data/nohttps-on-app.yml b/test/test_fallback.data/nohttps-on-app.yml
@@ -0,0 +1,15 @@
+services:
+  sut:
+    image: nginxproxy/nginx-proxy:test
+    volumes:
+      - /var/run/docker.sock:/tmp/docker.sock:ro
+    environment:
+      HTTPS_METHOD: redirect
+  http-only:
+    image: web
+    expose:
+      - "83"
+    environment:
+      WEB_PORTS: "83"
+      HTTPS_METHOD: nohttps
+      VIRTUAL_HOST: http-only.nginx-proxy.test
diff --git a/test/test_fallback.data/nohttps.yml b/test/test_fallback.data/nohttps.yml
@@ -0,0 +1,14 @@
+services:
+  sut:
+    image: nginxproxy/nginx-proxy:test
+    volumes:
+      - /var/run/docker.sock:/tmp/docker.sock:ro
+    environment:
+      HTTPS_METHOD: nohttps
+  http-only:
+    image: web
+    expose:
+      - "83"
+    environment:
+      WEB_PORTS: "83"
+      VIRTUAL_HOST: http-only.nginx-proxy.test
diff --git a/test/test_fallback.py b/test/test_fallback.py
@@ -33,6 +33,7 @@ def _get(url):
 
 
 INTERNAL_ERR_RE = re.compile("TLSV1_ALERT_INTERNAL_ERROR")
+CONNECTION_REFUSED_RE = re.compile("Connection refused")
 
 
 @pytest.mark.parametrize("compose_file,url,want_code,want_err_re", [
@@ -58,6 +59,36 @@ def _get(url):
     ("nodefault.yml", "https://missing-cert.nginx-proxy.test/", None, INTERNAL_ERR_RE),
     ("nodefault.yml", "http://unknown.nginx-proxy.test/", 503, None),
     ("nodefault.yml", "https://unknown.nginx-proxy.test/", None, INTERNAL_ERR_RE),
+    # HTTPS_METHOD=nohttp on nginx-proxy, HTTPS_METHOD unset on the app container.
+    ("nohttp.yml", "http://https-only.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
+    ("nohttp.yml", "https://https-only.nginx-proxy.test/", 200, None),
+    ("nohttp.yml", "http://unknown.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
+    ("nohttp.yml", "https://unknown.nginx-proxy.test/", 503, None),
+    # HTTPS_METHOD=redirect on nginx-proxy, HTTPS_METHOD=nohttp on the app container.
+    ("nohttp-on-app.yml", "http://https-only.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
+    ("nohttp-on-app.yml", "https://https-only.nginx-proxy.test/", 200, None),
+    ("nohttp-on-app.yml", "http://unknown.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
+    ("nohttp-on-app.yml", "https://unknown.nginx-proxy.test/", 503, None),
+    # Same as nohttp.yml, except there is a vhost with a missing cert.  This causes its
+    # HTTPS_METHOD=nohttp setting to effectively become HTTPS_METHOD=noredirect.  This means that
+    # there will be a plain http server solely to support that vhost, so http requests to other
+    # vhosts get a 503, not a connection refused error.
+    ("nohttp-with-missing-cert.yml", "http://https-only.nginx-proxy.test/", 503, None),
+    ("nohttp-with-missing-cert.yml", "https://https-only.nginx-proxy.test/", 200, None),
+    ("nohttp-with-missing-cert.yml", "http://missing-cert.nginx-proxy.test/", 200, None),
+    ("nohttp-with-missing-cert.yml", "https://missing-cert.nginx-proxy.test/", 500, None),
+    ("nohttp-with-missing-cert.yml", "http://unknown.nginx-proxy.test/", 503, None),
+    ("nohttp-with-missing-cert.yml", "https://unknown.nginx-proxy.test/", 503, None),
+    # HTTPS_METHOD=nohttps on nginx-proxy, HTTPS_METHOD unset on the app container.
+    ("nohttps.yml", "http://http-only.nginx-proxy.test/", 200, None),
+    ("nohttps.yml", "https://http-only.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
+    ("nohttps.yml", "http://unknown.nginx-proxy.test/", 503, None),
+    ("nohttps.yml", "https://unknown.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
+    # HTTPS_METHOD=redirect on nginx-proxy, HTTPS_METHOD=nohttps on the app container.
+    ("nohttps-on-app.yml", "http://http-only.nginx-proxy.test/", 200, None),
+    ("nohttps-on-app.yml", "https://http-only.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
+    ("nohttps-on-app.yml", "http://unknown.nginx-proxy.test/", 503, None),
+    ("nohttps-on-app.yml", "https://unknown.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
 ])
 def test_fallback(get, url, want_code, want_err_re):
     if want_err_re is None:

diff --git a/test/test_ssl/test_nohttp.py b/test/test_ssl/test_nohttp.py
@@ -1,9 +1,10 @@
 import pytest
+import requests
 
 
-def test_web2_http_is_not_forwarded(docker_compose, nginxproxy):
-    r = nginxproxy.get("http://web2.nginx-proxy.tld/", allow_redirects=False)
-    assert r.status_code == 503
+def test_web2_http_is_connection_refused(docker_compose, nginxproxy):
+    with pytest.raises(requests.exceptions.RequestException, match="Connection refused"):
+        nginxproxy.get("http://web2.nginx-proxy.tld/")
 
 
 def test_web2_https_is_forwarded(docker_compose, nginxproxy):