roots / trellis

WordPress LEMP stack with PHP 8.2, Composer, WP-CLI and more
https://roots.io/trellis/
MIT License
2.51k stars 607 forks source link

SSL certificates refactor #1310

Open swalkinshaw opened 3 years ago

swalkinshaw commented 3 years ago

Newer replacement for https://github.com/roots/trellis/pull/896/ with many of the same goals:

Todo:

cc @TangRufus

tangrufus commented 3 years ago

Question: Does certbot support https://roots.io/docs/trellis/master/ssl/#multiple-servers ?

swalkinshaw commented 3 years ago

Not sure if there's a better way, copying /etc/letsencrypt/accounts across servers would work.

tangrufus commented 3 years ago

Are we going backwards if we merge all ssl providers into 1 role? With ansible collections, shouldn't we separate them into individual roles and let users install the roles they actually use?


Question: For self-signed certs on local vagrant VMs, which certs should the user trust on the host machines? (Something like $ step ca root root.crt?)

swalkinshaw commented 3 years ago

Are we going backwards if we merge all ssl providers into 1 role? With ansible collections, shouldn't we separate them into individual roles and let users install the roles they actually use?

Tough question... entirely depends on how we want to structure this. With 1 role, they can just change their site config to use acme or manual. With multiple roles, they'd have to do the same and change their playbook to reference a different role.

Does ansible collections actually change much? Couldn't we achieve the same thing now with galaxy roles if we wanted?

This PR really only has two "roles" and the manual is very simple. But ideally we make it easy for someone to replace our SSL certificate implementation with another one. Maybe another ACME client, or something different entirely.

Question: For self-signed certs on local vagrant VMs, which certs should the user trust on the host machines? (Something like $ step ca root root.crt?)

Yeah you need to download the root cert locally and there's a few ways we can make that easier:

  1. use Trellis to fetch it to the host computer
  2. use step ca bootstrap --ca-url https://{site_dev_url}:8443 --fingerprint {FINGERPRINT_THAT_TRELLIS_OUTPUTS}

and then

  1. run step certificate install --all {cert_path}
tangrufus commented 3 years ago

This PR really only has two "roles" and the manual is very simple. But ideally we make it easy for someone to replace our SSL certificate implementation with another one.

Agree.

Yeah you need to download the root cert locally and there's a few ways we can make that easier: ...

Let make a trellis-cli command for it when this is merged.

swalkinshaw commented 2 years ago

@TangRufus I think this is ready. Other than general fixes + cleanup, there's three main updates:

  1. it ensures any existing renew cron is disabled
  2. I've removed any built-in DNS challenge functionality and made it easier for anyone to provide a custom challenge task file. In the future we can properly build out the DNS plugin functionality if we want
  3. https://github.com/roots/trellis-cli/pull/311 exists to easily trust the root cert
craigpearson commented 2 years ago

Hey I've been watching the progress on this, good work! One thing that's always been a bit of a bug bear with Trellis and multisite is that the SSL configuration is pretty much tied to a global config. This is great for standard sites, but limits flexibility with multisite setups.

In some Trellis mulitisite setups I've moved the SSL config to be URL specific, this would allow for configurations such as:

wordpress_sites:
  example.com:
    site_hosts:
      - canonical: example.com
        redirects:
          - www.example.com
        ssl:
           provider: manual # Example where you supply a certificate
           cert_file: X # Stored in the recommended group_vars/files/cert.vault.(crt/pem/yml)
           key_file: X
      - canonical: test.com
        redirects:
          - www.test.com
        ssl:
           provider: letsencrypt # Default certificate behaviour
     - canonical: foo.com
        redirects:
          - www.foo.com
        ssl:
           provider: none # SSL termination is handled elsewhere such as at a load balancer

This would allow users to be more granular in their approach to SSL certs per domain on multisite installs. Additionally, this approach could be seen as an override, so you could still set SSL vars as a global WP rule, but on a domain per domain basis override where desired.

My question is, if I was to create a separate PR of how this might look, do you see there being any issues with this approach? Appreciate that this may not of be interest as it's quite a marginalised use case, but I work towards this anyway, so maybe it's beneficial upstream too?

tangrufus commented 2 years ago

I support @craigpearson’s suggestion.

To discuss: Is it possible (or, how much work) to support ssl settings in redirect domains?

site_hosts:
      - canonical: acme-letsencrypt.com
        redirects:
          - www.no-ssl.com
          - *.wildcard-ssl.com
          - 3rd-party-ssl-provider.com
          - acme-but-not-letsencrypt.com

We might need to support client certificates for each of them as well.

Of course, we don’t need to implement all these in 1 PR.

What do you think, @swalkinshaw ?

swalkinshaw commented 2 years ago

Is it possible (or, how much work) to support ssl settings in redirect domains?

Is this separate from @craigpearson's suggestion? Or just reframing his proposal? Not sure if you mean SSL settings per individual redirect host.

@craigpearson I'm not against your feature idea but I do imagine it's quite rare. For that reason I'd rather not sacrifice too much to support it, but it would be nice to know up front if it's even compatible with this proposed PR as is right now.

The main issue I can see is complexity for downstream consumers of SSL certificates (like Nginx confs) and not necessarily generating certs for different domains. Right now the Nginx conf template is structured around a "flattened" list of hosts; they're all treated together which wouldn't work if we wanted to vary HTTPS settings per host.