mesosphere / marathon-lb

Marathon-lb is a service discovery & load balancing tool for DC/OS
Apache License 2.0
449 stars 300 forks source link

Dynamic SSL certificates for port 443/HTTPS #307

Open JayH5 opened 8 years ago

JayH5 commented 8 years ago

It's not currently possible to dynamically specify SSL certificates for the port 443 frontend. HAPROXY_{n}_SSL_CERT only sets the cert for the frontend for the app's service port. Certificates for port 443 must be set using --ssl-certs and require a complete restart of marathon-lb.

There is at least one TODO note for this functionality in the source code here.

This is functionality that we (Praekelt.org) need as we're trying to build a system to automatically fetch certificates for public-facing apps from Let's Encrypt.

I've come up with a couple of ways to achieve this and was halfway through writing up a PR but thought I should probably explain my thoughts/ideas and get some feedback before moving forward.

Option 1: add a new HAPROXY_{n}_VHOST_SSL_CERT label

The existing HAPROXY_{n}_SSL_CERT label does a very specific thing that some users may rely on. It also works together with the HAPROXY_{n}_BIND_OPTIONS label. For this reason I'd suggest a new label, say, HAPROXY_{n}_VHOST_SSL_CERT, so called because it applies to the vhost entry in the port 443 binding. This label's value would be in the same format as HAPROXY_{n}_SSL_CERT -- a comma-separated list of paths to certificate (.pem) files.

Certs added in this new label would simply be added to the bind declaration in HAPROXY_HTTPS_FRONTEND_HEAD after those already specified in --ssl-certs.

To do this I'd also suggest a new template, HAPROXY_HTTPS_FRONTEND_HEAD_CRT. This is a small template that defines how certs are added to HAPROXY_HTTPS_FRONTEND_HEAD. It could just look something like this:

 \
    crt {sslCert}

So an escape + newline + crt declaration. The crts could all be on one line but I expect there to be a lot of them after this change and so it would probably be better to put each on its own line.

The {sslCerts} variable in the HAPROXY_HTTPS_FRONTEND_HEAD template would then be populated with the list of both the certs from --ssl-certs and HAPROXY_{n}_VHOST_SSL_CERT values from all apps -- each element of this list run through the HAPROXY_HTTPS_FRONTEND_HEAD_CRT template and the strings appended.

Option 2: repurpose HAPROXY_{n}_SSL_CERT

The other option is just to use the existing HAPROXY_{n}_SSL_CERT label. We could keep the same behaviour for the service port frontend, but also use the same certificates in the port 443 frontend.

One argument for this is that the existing behaviour seems to me to be a very niche use-case. I could be missing something here, but it seems unlikely that people are using addresses like https://myservice.example.com:10025 to access their services via marathon-lb with HTTPS? It would be good to limit the number of labels used by marathon-lb and people are likely to get HAPROXY_{n}_SSL_CERT and HAPROXY_{n}_VHOST_SSL_CERT mixed up.

On the other hand, this could break existing things that I'm not aware of, so option 1 is probably my preferred option.

Further work: HAProxy's crt-list

It could be possible to use HAProxy's crt-list option and put the list of certificates in a separate file. This is tempting because having a long list of certs with \ characters could get messy.

I would suggest that maybe this could be a second iteration... maybe adding something like a --haproxy-crt-list flag to go along with --haproxy-map that would switch on the separate crt-list file.

Using the SNI filters available with that option would be tricky because we can't exactly map vhosts to certs when it is possible to specify multiple of both for each app port. It's an optimisation to think about maybe, though. The SNI filters are optional.

Other options:

Any suggestions? Thoughts?

brndnmtthws commented 8 years ago

I'd love it if we could also integrate with the secrets management API in DC/OS.

vixns commented 7 years ago

As you can read at http://cbonte.github.io/haproxy-dconv/1.7/configuration.html#5.1-crt, haproxy supports passing a directory as crt parameter.

Since version 1.7.0, multiple certs (rsa,dsa,ecdsa) per domain are supported and autodetected. haproxy also detects ocsp, issuer and sctl files.

So HAPROXY_{n}_SSL_CERT is no longer required and (may|should) be deprecated, passing a directory to --ssl-certs is sufficient.

JayH5 commented 7 years ago

Yep, been meaning to say, HAProxy will load certificates if you give it a directory of them. This works even in 1.6 (although I'm not sure about the different kinds of certs). The missing piece of the puzzle was getting HAProxy to reload, as changes to certificates in the directory won't be reflected after startup. Since #312, it's possible to reload HAProxy via an HTTP endpoint even if its config file hasn't changed.

Shameless plug: After piecing all this together we wrote marathon-acme to issue Let's Encrypt certificates for Marathon apps and trigger marathon-lb reloads automatically.

As described above, HAPROXY_{n}_SSL_CERT in its current form does not control the certificates used for port 443 (unlike --ssl-certs). There could be people using this behaviour (although the use cases I can think of are quite obscure) so I wouldn't be in a rush to remove it.