nginx-proxy / acme-companion

Automated ACME SSL certificate generation for nginx-proxy
MIT License
7.37k stars 819 forks source link

Add support for default certificates signed by Let's Encrypt #1062

Open Exagone313 opened 10 months ago

Exagone313 commented 10 months ago

Hello,

Currently, acme-companion generates a self-signed certificate in /etc/nginx/certs/default.crt (CN=letsencrypt-nginx-proxy-companion). This certificate is used when a certificate is missing, e.g. when a container is down.

It should be possible to use a default certificate that is valid and signed by Let's Encrypt, once it is created by acme-companion.

A possible implementation is to follow standalone certificate creation steps to handle the default identifier in a special manner so that symbolic links are created to point default.crt (and default.key) to the appropriate standalone certificate.

Relates to #1061

vialcollet commented 3 months ago

This is creating huge issues with things like Bugcrowd's VDP program. They scan services and when a container is down, the error is served with the self-signed certificate. Ideally, when a container is down it should be served by the certificate corresponding to the requested URL.

DEFAULT_HOST does not currently work because the certificate would correspond to DEFAULT_HOST and not to VIRTUAL_HOST (or LETSENCRYPT_HOST).

I tried to do something like this but it doesn't solve the issue:

nginx-proxy docker compose

version: '3'

services:
  nginx-proxy:
    image: nginxproxy/nginx-proxy
    container_name: nginx-proxy
    ports:
      - "80:80"
      - "443:443"
    volumes:
      - conf:/etc/nginx/conf.d
      - vhost:/etc/nginx/vhost.d
      - html:/usr/share/nginx/html
      - certs:/etc/nginx/certs:ro
      - /var/run/docker.sock:/tmp/docker.sock:ro 
    restart: always
    environment:
      DEFAULT_HOST: mydomain.com
    networks:
      - webproxy

  acme-companion:
    image: nginxproxy/acme-companion
    container_name: nginx-proxy-acme
    environment:
      - DEFAULT_EMAIL=xxxx
    volumes_from:
      - nginx-proxy
    restart: always
    volumes:
      - certs:/etc/nginx/certs:rw
      - acme:/etc/acme.sh
      - /var/run/docker.sock:/var/run/docker.sock:ro
    networks:
      - webproxy

volumes:
  conf:
  vhost:
  html:
  certs:
  acme:

networks: 
  webproxy:
    name: webproxy
    external: true

default host docker compose

version: "3.8"

services:

  forbidden:
    image: nginx:alpine
    volumes:
      - ./nginx.conf:/etc/nginx/nginx.conf:ro
    networks:
      - webproxy
    environment:
      VIRTUAL_HOST: mydomain.com
      VIRTUAL_PORT: 80
      LETSENCRYPT_HOST: mydomain.com,app1.mydomain.com,app2.mydomain.com
      LETSENCRYPT_SINGLE_DOMAIN_CERTS: true
      LETSENCRYPT_EMAIL: xxxxx

networks: 
  webproxy:
    name: webproxy
    external: true

nginx.conf

events {
    worker_connections 1024;
}

http {
    server {
        listen 80;
        server_name app1.mydomain.com;

        root /usr/share/nginx/html;
        index index.html;

        location / {
            return 403 "Forbidden";
        }
    }
}

The idea was that if app1 stack is down, then the request fall backs to DEFAULT_HOST (mydomain.com) and is served with a certificate that would work with app1.mydomain.com. However it still generates a certificate warning... :(

Any ideas?

buchdag commented 3 months ago

@pini-gh I'd like you advice on this.

I feel like the creation of a default certificate by acme-companion might have been a mistake caused by my misunderstanding of the ACME RFC at the time, ie thinking that ACME HTTP challenge validation can in some case be attempted over HTTPS first, which is not true. With this in mind I'm not certain that the creation of a default self signed certificate for nginx-proxy is really desirable, as it appears to me to do little more than favor certificate errors by default.

Aside from the "should we automatically create a self signed default certificate" question, I think this issue should be easier and/or make more sense to fix on nginx-proxy's end, with something like a DEFAULT_CERT_NAME variable.

pini-gh commented 3 months ago

About the OP's question I think the @vialcollet's approach is correct. It should work after removing LETSENCRYPT_SINGLE_DOMAIN_CERTS: true from the default host docker compose file.

@buchdag I agree on the first part of your post. But I fail to see how a new variable DEFAULT_CERT_NAME would solve the present issue. Would you care to elaborate?

buchdag commented 3 months ago

The idea was to provide user a way to designate to nginx-proxy a "real" certificate created and handled by acme-companion to use as a default certificate, because it seemed easier to do something like this on nginx-proxy's template (very basic and incomplete example just so you can get the idea) :

{{- $_ := set $globals "customDefaultCertPath" := printf "/etc/nginx/certs/%s.crt" $globals.Env.DEFAULT_CERT_NAME }}
{{- $_ := set $globals "customDefaultKeyPath" := printf "/etc/nginx/certs/%s.key" $globals.Env.DEFAULT_CERT_NAME }}
{{- $_ := set $globals "custom_default_cert_ok" (and (exists customDefaultCertPath) (exists customDefaultKeyPath)) }}
{{- $_ := set $globals "default_cert_ok" (and (exists "/etc/nginx/certs/default.crt") (exists "/etc/nginx/certs/default.key")) }}

[...]

        {{- if $globals.custom_default_cert_ok }}
    ssl_certificate {{ $globals.defaultCertPath }};
    ssl_certificate_key {{ $globals.defaultKeyPath }};
        {{- else if $globals.default_cert_ok }}
    ssl_certificate /etc/nginx/certs/default.crt;
    ssl_certificate_key /etc/nginx/certs/default.key;
        {{- else }}

This should implement the feature asked by @Exagone313

pini-gh commented 3 months ago

This should implement the feature asked by @Exagone313

Indeed. Thanks for the details.

EDIT Thinking about it, relying on the fallback server to serve an error page when an app container is down implies that the default certificate is valid for every related app domain. It ends up with a bloated default certificate. Or it needs wildcard certificates.

Wouldn't it make sense to use the backup option of the upstream block's server statement instead, to proxypass to a server dedicated to this task? This way it wouldn't require an all domains certificate because the SSL termination would be handled by the app domain's dedicated server block. If there is a way to achieve this it feels like a better option to me.

buchdag commented 3 months ago

relying on the fallback server to serve an error page when an app container is down implies that the default certificate is valid for every related app domain. It ends up with a bloated default certificate. Or it needs wildcard certificates.

Agreed, I consider this to be an "easy" fix to those default cert issues more than a long term, scalable solution.

wouldn't it make sense to use the backup option of the upstream block's server statement instead

That's what crossed my mind too, I'll look into it.