From 33a06f987d3da89b747bc7c76ae863ef98da7836 Mon Sep 17 00:00:00 2001 From: Gilles Filippini Date: Thu, 9 May 2024 23:28:34 +0200 Subject: [PATCH] Improve acme-challenge handling So that there is no need anymore for the Let's Encrypt companion to fiddle with vhosts nginx configuration. When `HTTPS_METHOD=nohttp` and the certificate is missing, enforce nohttp instead of switching to `HTTPS_METHOD=redirect`. --- nginx.tmpl | 18 ++++++++++++++---- test/test_fallback.py | 14 ++++++-------- test/test_ssl/test_noredirect.py | 4 ++-- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/nginx.tmpl b/nginx.tmpl index 2b73158..21431d0 100644 --- a/nginx.tmpl +++ b/nginx.tmpl @@ -642,7 +642,7 @@ proxy_set_header Proxy ""; {{- $default_https_exists := false }} {{- $http3_enabled := false }} {{- range $vhost := $globals.vhosts }} - {{- $http := or (ne $vhost.https_method "nohttp") (not $vhost.cert_ok) }} + {{- $http := ne $vhost.https_method "nohttp" }} {{- $https := ne $vhost.https_method "nohttps" }} {{- $http_exists = or $http_exists $http }} {{- $https_exists = or $https_exists $https }} @@ -716,7 +716,7 @@ server { {{ template "upstream" (dict "globals" $globals "Path" $path "VPath" $vpath) }} {{- end }} - {{- if and $vhost.cert_ok (eq $vhost.https_method "redirect") }} + {{- if and (or $vhost.cert_ok $globals.default_cert_ok) (eq $vhost.https_method "redirect") }} server { server_name {{ $hostname }}; {{- if $vhost.server_tokens }} @@ -757,11 +757,21 @@ server { {{- if $vhost.http2_enabled }} http2 on; {{- end }} - {{- if or (eq $vhost.https_method "nohttps") (not $vhost.cert_ok) (eq $vhost.https_method "noredirect") }} + {{- if or (eq $vhost.https_method "nohttps") (and (not $vhost.cert_ok) (not $globals.default_cert_ok)) (eq $vhost.https_method "noredirect") }} listen {{ $globals.external_http_port }} {{ $default_server }}; {{- if $globals.enable_ipv6 }} listen [::]:{{ $globals.external_http_port }} {{ $default_server }}; {{- end }} + + {{- if (eq $vhost.https_method "noredirect") }} + location /.well-known/acme-challenge/ { + auth_basic off; + allow all; + root /usr/share/nginx/html; + try_files $uri =404; + break; + } + {{- end }} {{- end }} {{- if ne $vhost.https_method "nohttps" }} listen {{ $globals.external_https_port }} ssl {{ $default_server }}; @@ -856,4 +866,4 @@ server { } {{- end }} } -{{- end }} \ No newline at end of file +{{- end }} diff --git a/test/test_fallback.py b/test/test_fallback.py index 1ee923a..0291595 100644 --- a/test/test_fallback.py +++ b/test/test_fallback.py @@ -44,7 +44,7 @@ CONNECTION_REFUSED_RE = re.compile("Connection refused") ("withdefault.yml", "https://https-only.nginx-proxy.test/", 200, None), ("withdefault.yml", "http://http-only.nginx-proxy.test/", 200, None), ("withdefault.yml", "https://http-only.nginx-proxy.test/", 503, None), - ("withdefault.yml", "http://missing-cert.nginx-proxy.test/", 200, None), + ("withdefault.yml", "http://missing-cert.nginx-proxy.test/", 301, None), ("withdefault.yml", "https://missing-cert.nginx-proxy.test/", 500, None), ("withdefault.yml", "http://unknown.nginx-proxy.test/", 503, None), ("withdefault.yml", "https://unknown.nginx-proxy.test/", 503, None), @@ -69,15 +69,13 @@ CONNECTION_REFUSED_RE = re.compile("Connection refused") ("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), + # Same as nohttp.yml, except there is a vhost with a missing cert. + # nohttp should be enforced in this case as well. + ("nohttp-with-missing-cert.yml", "http://https-only.nginx-proxy.test/", None, CONNECTION_REFUSED_RE), ("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", "http://missing-cert.nginx-proxy.test/", None, CONNECTION_REFUSED_RE), ("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", "http://unknown.nginx-proxy.test/", None, CONNECTION_REFUSED_RE), ("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), diff --git a/test/test_ssl/test_noredirect.py b/test/test_ssl/test_noredirect.py index 0f50063..1d956d1 100644 --- a/test/test_ssl/test_noredirect.py +++ b/test/test_ssl/test_noredirect.py @@ -19,9 +19,9 @@ def test_web2_HSTS_policy_is_inactive(docker_compose, nginxproxy): assert "Strict-Transport-Security" not in r.headers -def test_web3_acme_challenge_does_not_work(docker_compose, nginxproxy, acme_challenge_path): +def test_web3_acme_challenge_does_work(docker_compose, nginxproxy, acme_challenge_path): r = nginxproxy.get( f"http://web3.nginx-proxy.tld/{acme_challenge_path}", allow_redirects=False ) - assert r.status_code == 404 + assert r.status_code == 200