Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Exagone313 opened this issue Oct 20, 2023 · 6 comments
Open

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

Exagone313 opened this issue Oct 20, 2023 · 6 comments
Labels
kind/feature-request Issue requesting a new feature

Comments

@Exagone313
Copy link

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

@buchdag buchdag added the kind/feature-request Issue requesting a new feature label Dec 10, 2023
@vialcollet
Copy link

vialcollet commented May 13, 2024

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
Copy link
Member

buchdag commented May 13, 2024

@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
Copy link
Contributor

pini-gh commented May 13, 2024

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
Copy link
Member

buchdag commented May 14, 2024

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
Copy link
Contributor

pini-gh commented May 14, 2024

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
Copy link
Member

buchdag commented May 14, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature-request Issue requesting a new feature
Projects
None yet
Development

No branches or pull requests

4 participants