From 45ce7d418e1229b53d9dae33b5bb6cc3916c7d51 Mon Sep 17 00:00:00 2001 From: Randy Fay Date: Wed, 16 Sep 2020 21:04:15 -0600 Subject: [PATCH] ddev-router should not accept hostnames it's not configured for, fixes #2345 (#2504) --- containers/ddev-router/Dockerfile | 2 +- containers/ddev-router/Procfile | 2 +- .../gen-cert-and-nginx-config.sh.tmpl | 38 ++++++++++ containers/ddev-router/gen-cert.sh.tmpl | 20 ----- containers/ddev-router/healthcheck.sh | 3 + containers/ddev-router/nginx.tmpl | 75 ++++++++++--------- pkg/ddevapp/ddevapp_test.go | 2 +- pkg/version/version.go | 2 +- 8 files changed, 85 insertions(+), 59 deletions(-) create mode 100644 containers/ddev-router/gen-cert-and-nginx-config.sh.tmpl delete mode 100644 containers/ddev-router/gen-cert.sh.tmpl diff --git a/containers/ddev-router/Dockerfile b/containers/ddev-router/Dockerfile index ca0d1b4b050..e6ab28e4492 100644 --- a/containers/ddev-router/Dockerfile +++ b/containers/ddev-router/Dockerfile @@ -8,7 +8,7 @@ ENV DOCKER_HOST unix:///tmp/docker.sock RUN apt-get -qq update && \ apt-get -qq install --no-install-recommends --no-install-suggests -y \ - ca-certificates certbot curl less python-certbot-nginx procps telnet vim wget && \ + ca-certificates certbot curl iputils-ping less python-certbot-nginx procps telnet vim wget && \ apt-get autoremove -y && \ apt-get clean -y && \ rm -rf /var/lib/apt/lists/* diff --git a/containers/ddev-router/Procfile b/containers/ddev-router/Procfile index a83442832e6..1b2f5e09267 100644 --- a/containers/ddev-router/Procfile +++ b/containers/ddev-router/Procfile @@ -1,2 +1,2 @@ nginx: nginx -dockergen: docker-gen -watch -only-exposed --notify-output -notify "chmod ugo+x /gen-cert.sh && /gen-cert.sh && sleep 1 && nginx -s reload" /app/gen-cert.sh.tmpl /gen-cert.sh +dockergen: docker-gen -watch -only-exposed --notify-output -notify "bash /gen-cert-and-nginx-config.sh" /app/gen-cert-and-nginx-config.sh.tmpl /gen-cert-and-nginx-config.sh diff --git a/containers/ddev-router/gen-cert-and-nginx-config.sh.tmpl b/containers/ddev-router/gen-cert-and-nginx-config.sh.tmpl new file mode 100644 index 00000000000..3dad4f6a8e3 --- /dev/null +++ b/containers/ddev-router/gen-cert-and-nginx-config.sh.tmpl @@ -0,0 +1,38 @@ +#!/bin/bash + +# This gets prprocessed by docker-gen into a script which generates needed +# mkcert certs and updates the nginx configs for all projects + +set -eu -o pipefail + +hostnames='{{ range $host, $containers := groupByMulti $ "Env.VIRTUAL_HOST" "," }}{{ trim $host }} {{ end }}' +echo "Processing certs and nginx for hostnames: $hostnames" + +# To redirect invalid hostnames, we need a list of http ports and https ports +httpports='80 +{{ range $port, $containers := groupByMulti $ "Env.HTTP_EXPOSE" "," }}{{ trim $port }} +{{ end }}' +echo "${httpports}" >/tmp/httpports.txt +httpsports='443 +{{ range $port, $containers := groupByMulti $ "Env.HTTPS_EXPOSE" "," }}{{ trim $port }} +{{ end }}' +echo "${httpsports}" >/tmp/httpsports.txt + +# Convert the lists into unique sets of listen directives in /tmp +awk -F: '$0 != "" {printf "\tlisten %s;\n", $1;}' /tmp/httpports.txt | sort -u >/tmp/http_ports.conf +awk -F: '$0 != "" {printf "\tlisten %s ssl http2;\n", $1;}' /tmp/httpsports.txt | sort -u >/tmp/https_ports.conf + + +if [ ! -z "${USE_LETSENCRYPT:-}" ]; then + for host in ${hostnames}; do + # certbot challenge can fail for many reasons, but don't let it break everything + certbot --nginx certonly -n --domain "${host}" --agree-tos --email "${LETSENCRYPT_EMAIL:-}" || true + done +fi + +mkcert -cert-file /etc/nginx/certs/master.crt -key-file /etc/nginx/certs/master.key $hostnames 127.0.0.1 localhost "*.ddev.site" + +# This is not recursive, as it executes completely different instructions. +# It's important for the nginx config creation and the nginx reload to take place after all cert +# activities are completed. +docker-gen -only-exposed -notify-output -notify "sleep 1 && nginx -s reload" /app/nginx.tmpl /etc/nginx/conf.d/default.conf diff --git a/containers/ddev-router/gen-cert.sh.tmpl b/containers/ddev-router/gen-cert.sh.tmpl deleted file mode 100644 index 772eb4a5b75..00000000000 --- a/containers/ddev-router/gen-cert.sh.tmpl +++ /dev/null @@ -1,20 +0,0 @@ -#!/bin/bash -set -eu -o pipefail -set -x - -if [ ! -z "${USE_LETSENCRYPT:-}" ]; then - hostnames="{{ range $host, $containers := groupByMulti $ "Env.VIRTUAL_HOST" "," }}{{ trim $host }} {{ end }}" - - for host in ${hostnames}; do - # certbot challenge can fail for many reasons, but don't let it break everything - certbot --nginx certonly -n --domain "${host}" --agree-tos --email "${LETSENCRYPT_EMAIL:-}" || true - done -fi - -# mkcert is fully capable of generating all needed names in a single container. -mkcert -cert-file /etc/nginx/certs/master.crt -key-file /etc/nginx/certs/master.key {{ range $host, $containers := groupByMulti $ "Env.VIRTUAL_HOST" "," }} {{ trim $host }} {{ end }} 127.0.0.1 localhost "*.ddev.local" "*.ddev.site" - -# This is not recursive, as it executes completely different instructions. -# It's important for the nginx config creation and the nginx reload to take place after all cert -# activities are completed. -docker-gen -only-exposed -notify-output -notify "sleep 1 && nginx -s reload" /app/nginx.tmpl /etc/nginx/conf.d/default.conf diff --git a/containers/ddev-router/healthcheck.sh b/containers/ddev-router/healthcheck.sh index 5e3678fa0ce..8706cbf7aa2 100644 --- a/containers/ddev-router/healthcheck.sh +++ b/containers/ddev-router/healthcheck.sh @@ -20,6 +20,9 @@ connect=false if nginx -t ; then config=true printf "nginx config: OK " +else + printf "nginx configuration invalid: $(nginx -t)" + exit 2 fi # Check our healthcheck endpoint diff --git a/containers/ddev-router/nginx.tmpl b/containers/ddev-router/nginx.tmpl index 4e48de6a211..32c09489a99 100644 --- a/containers/ddev-router/nginx.tmpl +++ b/containers/ddev-router/nginx.tmpl @@ -86,37 +86,12 @@ proxy_set_header Proxy ""; {{ $default_cert := trimSuffix ".crt" $default_cert }} {{ $default_cert := trimSuffix ".key" $default_cert }} server { - server_name _; # This is just an invalid value which will never trigger on a real hostname. - listen 80; - {{ if $enable_ipv6 }} - listen [::]:80; - {{ end }} - access_log /var/log/nginx/access.log vhost; - - error_page 503 @noupstream; - location @noupstream { - rewrite ^(.*)$ /503.html break; - root /app; - } - - location / { - return 503; - } + server_name _; # Catch-all for any server_name that is not matched in the config - ## provide a health check endpoint - location = /healthcheck { - access_log off; - return 200; - } -} + # Listen on all configured http ports + 80 (HTTPS_EXPOSE) + # So that we can redirect any server_name that is invalid -{{ if exists (printf "/etc/nginx/certs/%s.crt" $default_cert) }} -server { - server_name _; # This is just an invalid value which will never trigger on a real hostname. - listen 443 ssl http2; - {{ if $enable_ipv6 }} - listen [::]:443 ssl http2; - {{ end }} + include /tmp/http_ports.conf; access_log /var/log/nginx/access.log vhost; error_page 503 @noupstream; @@ -134,12 +109,42 @@ server { access_log off; return 200; } - - ssl_session_tickets off; - ssl_certificate /etc/nginx/certs/{{ (printf "%s.crt" $default_cert) }}; - ssl_certificate_key /etc/nginx/certs/{{ (printf "%s.key" $default_cert) }}; } -{{ end }} + + {{ if exists (printf "/etc/nginx/certs/%s.crt" $default_cert) }} + server { + server_name _; # Catch-all for any server_name that is not matched in the config + + # Listen on all configured https ports + 443 (HTTPS_EXPOSE) + # So that we can redirect any server_name that is invalid + include /tmp/https_ports.conf; + + {{ if $enable_ipv6 }} + listen [::]:443 ssl http2; + {{ end }} + access_log /var/log/nginx/access.log vhost; + + error_page 503 @noupstream; + location @noupstream { + rewrite ^(.*)$ /503.html break; + root /app; + } + + location / { + return 503; + } + + ## provide a health check endpoint + location = /healthcheck { + access_log off; + return 200; + } + + ssl_session_tickets off; + ssl_certificate /etc/nginx/certs/{{ (printf "%s.crt" $default_cert) }}; + ssl_certificate_key /etc/nginx/certs/{{ (printf "%s.key" $default_cert) }}; + } + {{ end }} {{ range $host, $containers := groupByMulti $ "Env.VIRTUAL_HOST" "," }} @@ -151,7 +156,7 @@ server { {{/* upstream {{ $upstream_name }} { */}} {{ range $container := $containers }} - {{/* This replaces the VIRTUAL_PORT var originally provided by jwilder/nginx-proxy. */}} + {{/* HTTP_EXPOSE replaces the VIRTUAL_PORT var originally provided by jwilder/nginx-proxy. */}} {{/* HTTP_EXPOSE provides comma-separated ports to serve for container. Ports can be defined as hostPort:containerPort */}} {{ $ports := coalesce $container.Env.HTTP_EXPOSE "80" }} {{ $ports := split $ports "," }} diff --git a/pkg/ddevapp/ddevapp_test.go b/pkg/ddevapp/ddevapp_test.go index a53b284c199..d9ca149f02f 100644 --- a/pkg/ddevapp/ddevapp_test.go +++ b/pkg/ddevapp/ddevapp_test.go @@ -3192,7 +3192,7 @@ func TestCustomCerts(t *testing.T) { }) stdout = strings.Trim(stdout, "\n") // This should be our regular wildcard cert - assert.Contains(stdout, "*.ddev.local\n*.ddev.site") + assert.Contains(stdout, "*.ddev.site") // Now stop it so we can install new custom cert. err = app.Stop(true, false) diff --git a/pkg/version/version.go b/pkg/version/version.go index c1bf5845ce5..4fb273f0bbf 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -63,7 +63,7 @@ var DBATag = "5" // Note that this can be overridden by make var RouterImage = "drud/ddev-router" // RouterTag defines the tag used for the router. -var RouterTag = "20200817_custom_certs" // Note that this can be overridden by make +var RouterTag = "20200909_ddev_router" // Note that this can be overridden by make var SSHAuthImage = "drud/ddev-ssh-agent"