Add SSL support via ACME / LetsEncrypt #30

jasonpincin · 2016-09-30T01:08:08Z

This PR leverages dehydrated to request and renew certs. The dehydrated script offers a hook system which is used to send challenge tokens, certs, and keys to consul. Consul Template is used to generate the tokens, certs, and keys on disk from Consul Agent.

Only one Nginx container will be responsible for renewing certificates and writing to Consul. A simple consul lock acquisition is used to determine which container is the leader. All nginx containers, including the leader, watch consul for changes and generate the corresponding files as changes occur. After certs/keys are generated, Nginx is instructed to reload the config.

In order to activate this functionality, a single environment variable, ACME_DOMAIN must be passed to the Nginx container at runtime who's value is the domain the container(s) should request a certificate for. It's up to the user to have the domain resolve to the containers so that ACME challenges can be responded to.

By default, the LetsEncrypt staging endpoint will be used, which allows the developer to iterate and get this working without hitting the API limits imposed on the production endpoints. Once the user is ready to move to production, and get real certificates, a 2nd environment variable must be passed: ACME_ENV=production (this is consistent with NODE_ENV and others).

Further implementation details:

Unfortunately, consul-template does not support writing to a dynamic file name which is required for acme challenge responses, so we write to a temporary file consul-template, then spawn a process that generates the properly named token.
For startup purposes, there's now two nginx config templates. One with SSL directives and one without. The reload.sh script was modified to choose the correct one based on whether certs/keys are found on disk.
Port 443 is being exposed by default in docker-compose.yml.
An acme script was added (to /usr/local/bin) which contains container specific log for ACME operations. This works in conjunction with the dehydrated script and hooks (located at /etc/acme/dehydrated/hook.sh).
Three Container Pilot tasks are added when ACME is enabled. The logic for these tasks is contained within the acme script.
- acme-checkin - Renews consul session and attempts to become leader. Runs every 5m.
- acme-renew-certs - Acquires or renews certificate for ACME_DOMAIN. Runs every 12h.
- clean-unused-certs - Cleans old certs. Runs once per day.

Other miscellaneous improvements:

in order to reduce logging noise health check 200 responses are no longer logged
corrected memory limit in local-compose.yml
Upgraded to Consul v0.7.0 in Nginx, Consul, and example backend containers
jq was added to the container for more reliable JSON parsing in bash

…data to consul

tgross · 2016-09-30T14:02:28Z

bin/dehydrated

@@ -0,0 +1,1116 @@
+#!/usr/bin/env bash


Maybe rather than copying this or pulling it down in the Dockerfile, we could make this a git submodule?

Possible, but all we need is the script from that module, and we will want to pin it to a version. So the build will either curl -O a url (with version) or cloning a submodule, checking out the right version there, and copying the script from there to out bin path.

tgross · 2016-09-30T14:08:42Z

etc/dehydrated/hook.sh

@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+
+CONSUL_ROOT="http://localhost:8500/v1/kv/nginx"


The ContainerPilot configuration supports non-localhost Consul paths depending on the value of CONSUL_AGENT variable. We should probably try to translate the same configuration to bash here:

"consul": "{{ if .CONSUL_AGENT }}localhost{{ else }}{{ .CONSUL }}{{ end }}:8500",

Absolutely. I'll do this. This PR is very WIP, so some of what you see if just a result of me testing this out locally, outside of containers.

tgross · 2016-09-30T14:11:46Z

etc/dehydrated/hook.sh

@@ -0,0 +1,45 @@
+#!/usr/bin/env bash


A TODO as you refine this: given how many HTTP calls we have to make in this script, we might want to consider setting set -e here to fail fast or have a trap on the error so that we get some nice noisy logging or problems.

I didn't think set -e worked that way for bash function calls. I could be mistaken. This script defines a handful of functions. Then dehydrated sources this file and invokes those functions at the proper time. If set -e would work in this context, I'm not sure what impact it would have on dehydrated. Perhaps the better approach here would be to && the curl commands? Something like:

curl blah1 && curl blah2 && curl blah3

Thoughts?

Yeah I think you're right on that. Chaining the calls with && might be better.

tgross · 2016-09-30T14:13:57Z

etc/dehydrated/hook.sh

+function deploy_challenge {
+    local DOMAIN="${1}" TOKEN_FILENAME="${2}" TOKEN_VALUE="${3}"
+
+    curl -sX PUT -d "${TOKEN_FILENAME}" ${CONSUL_ROOT}/domains/${DOMAIN}/acme/challenge/token-filename > /dev/null


I'd suggest avoiding piping these to /dev/null particularly during the development process. Consul isn't very chatty as it is and it'd be good to know that something went wrong. Maybe we pipe them to a log function somewhere in this same script?

Agreed. I actually put those there during development because they were spamming true to STDOUT and cluttering up other stuff I was trying to read. You'd hope alarms would go to STDERR, but I know that's not always the case.

…main for now), return from hook funcs early on failure, limit retries on verifying challenge token has been distributed

… at runtime

…ponsible for renewing certs)

…eaning certificates (if leader). Also added a co-process responsible for writing acme challenge tokens as well as certs provided by the leader (leader uses this too)

…s (default to staging so as not to exceed limits in prod)

jasonpincin · 2016-09-30T21:22:18Z

etc/containerpilot.json

                  "--domain", "{{ .ACME_DOMAIN }}",
                  "--hook", "/etc/dehydrated/hook.sh",
-                  "--config", "/etc/dehydrated/config"],
+                  "--config", "/etc/dehydrated/config.{{ .ACME_ENV }}"],


Is there a good way to default this to staging in the case it's omitted from docker-compose.yml? cc @tgross

…o etc/acme

…pting ACME challenge

…on. Add init command to be used by health check to insure certs are installed before ssl health check is a success. This may change in the future, see: TritonDataCenter/containerpilot#227

…llenge

…sed on existance of keys

…init, for startup reasons

…ginx logging.

tgross · 2016-10-07T13:33:48Z

etc/nginx/nginx-ssl.conf.ctmpl

+            alias /var/www/acme/challenge;
+        }
+        {{ end }}
+        listen 443 ssl;


Should these be under separate server blocks? With this setup we're accepting requests for the default (_) hostname on port 80 and our secured domain on port 443 together. I think we want to have the port 80 server block only serve the items we absolutely need it to and then redirect to the port 443 server block for everything else.

Yep, good catch. Will do that.

tgross · 2016-10-07T13:34:44Z

local-compose.yml

        service: nginx
    build: .
-    mem_limit: 128g
+    mem_limit: 128m


Thanks for catching that one. 😊

tgross · 2016-10-07T13:36:06Z

README.md

+
+### Configuring LetsEncrypt (ACME)
+
+Setting the `ACME_DOMAIN` environment variable will enable LetsEncrypt within


No need to hand-wrap markdown at 80 cols.

tgross · 2016-10-07T13:36:45Z

README.md

+`ACME_ENV` environment variable to `production`. 
+
+You must ensure the domain resolves to your Nginx containers so that they can
+respond to the ACME http challenges.


Maybe we should link to a doc showing that you can CNAME to your Triton CNS service name, for Triton users?

tgross · 2016-10-07T14:39:47Z

etc/acme/dehydrated/hook.sh

+    local DOMAIN="${1}" TOKEN_FILENAME="${2}" TOKEN_VALUE="${3}"
+
+    curl -sX DELETE ${CONSUL_KEY_ROOT}/acme/challenge/token-filename | log
+    test ${PIPESTATUS[0]} -eq 0 || (log "${FUNCNAME} failed" && return)


These are the result of the discussion we had here, right? This makes this kinda hard to find the logic amongst all the error handling. I think we could clean this up if we use set -o pipefail and a trap in this script for errors.

Right. No objections to changing it to pipefail. Will be cleaner. For the same reason in the last discussion though, I'm not sure about using a trap due to the fact that this script is sourced by dehydrated. I don't want to exit on errors in this script. I want dehydrated to capture the error and do whatever it needs to do.

Sure, but doesn't dehydrated itself have set -euo pipefail though? It looks to me like if we're returning anything other than 0 from the function then we're just bailing out.

Edit for clarity: the only reason I'm suggesting a trap at all is to massage the error messages. ContainerPilot will log things with the appropriate timestamps and ERROR block, etc. so if you're happy with that then so am I.

…ocation. This prevents empty cert/key files from being written and simplifies logic pertaining to their existence

tgross · 2016-10-07T20:43:56Z

LGTM.

jasonpincin added 4 commits September 29, 2016 20:48

bringing in dehydrated to manage acme cert request/renews

bdf50c0

config and hooks for dehydrated, hooks will propogate challenge/cert …

9596d9b

…data to consul

cannot generate dynamic filenames from consul-template, so hacking it

974ddbd

respond to acme challenges if acme is enabled

9b4d272

tgross reviewed Sep 30, 2016

View reviewed changes

jasonpincin added 12 commits September 30, 2016 14:38

Respect CONSUL_AGENT and CONSUL env vars, simplify kv structure (1 do…

7b1cc78

…main for now), return from hook funcs early on failure, limit retries on verifying challenge token has been distributed

adding ssl cert and acme challenge token templates

1b01db8

corrected paths, removed consul host from config file to be specified…

e73717e

… at runtime

moved acme watch file

ddf9fc5

accept source and destination as args, remove source when done

a59c152

Insure directories needed for acme/ssl operation are present

6def330

renamed env var to ACME_DOMAIN, supporting single domain for now

9aa587a

do not need trailing /

f113253

Added script for managing Consul sessions (for use with leader lock)

a67a567

Added script for managing leader aquisition (determines container res…

ae56880

…ponsible for renewing certs)

Added tasks for renewing consol session, claiming leader, renewing/cl…

4d67190

…eaning certificates (if leader). Also added a co-process responsible for writing acme challenge tokens as well as certs provided by the leader (leader uses this too)

Support toggling ACME/LetsEncrypt between staging/production endpoint…

893e1c9

…s (default to staging so as not to exceed limits in prod)

jasonpincin commented Sep 30, 2016

View reviewed changes

jasonpincin added 12 commits September 30, 2016 17:32

fetch dehydrated at docker build time

35cedcd

If ACME_ENV not set, default to staging.

3039fc3

consolidated acme related tasks into bin/acme. Moved etc/dehydrated t…

562ee7e

…o etc/acme

Corrected template formatting errors

c5e3485

Fixed typo. Added jq.

42f92ef

Corrected commands to add jq

9f7e18a

Verify challenge token is written to all nginx containers before acce…

b65eafc

…pting ACME challenge

No limit to consul-template restarts. Formatting fixes.

b48b811

Defined dehydrated WELLKNOWN

f8f384a

Moved ENV VAR lookups into acme script to declutter containerpilot.js…

7122b63

…on. Add init command to be used by health check to insure certs are installed before ssl health check is a success. This may change in the future, see: TritonDataCenter/containerpilot#227

Upgraded consul to 0.7.0. Adjusted options.

079f977

Insure all nginx containers have challenge token before accepting cha…

56a39e1

…llenge

jasonpincin added 12 commits October 4, 2016 18:06

check that key exists before writing it

f1dfdfb

Add SSL interface. Increasedd acme-checkin frequency from 2.5 mins to 5,

99141c4

Handle dehydrated flags in acme script

0b2a71e

Add SSL directives to Nginx config if ACME_DOMAIN env var set

04feeb3

Remove debugging messages

6aac101

Upgrade consul agent to 0.7.0

b51807e

Seperate template for ssl enabled nginx for startup reasons

55b893f

Reload will choose whether to use ssl template or non-ssl template ba…

c1bf4ab

…sed on existance of keys

Perform reload as part of consul-template key writes instead of acme …

0ac4ba6

…init, for startup reasons

Expose port 443. Correct mem_limit.

0d2b2c8

Mention ACME support in README

0019d51

Clean up logs my omitting sucesful (200) health check requests from n…

3416757

…ginx logging.

jasonpincin changed the title ~~[WIP] Add SSL support via ACME / LetsEncrypt~~ Add SSL support via ACME / LetsEncrypt Oct 6, 2016

Updated example backend to Consul v0.7.0

6025b0f

tgross reviewed Oct 7, 2016

View reviewed changes

local-compose.yml

service: nginx

build: .

mem_limit: 128g

mem_limit: 128m

Copy link

Contributor

tgross Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that one. 😊

tgross reviewed Oct 7, 2016

View reviewed changes

jasonpincin added 6 commits October 7, 2016 10:39

Un-handwrapped text

7709242

Link to document on using your own domain with Triton CNS

5b1c69a

Improve readability of code by using pipefail

cdfaf0f

Redirect http to https (except for health check) when SSL is enabled

8febef6

Include host header in ssl health check

f4f21b2

Write certs/keys to temporary location before copying them to final l…

894f7e7

…ocation. This prevents empty cert/key files from being written and simplifies logic pertaining to their existence

jasonpincin merged commit e132eff into autopilotpattern:master Oct 10, 2016

jasonpincin mentioned this pull request Oct 13, 2016

Add Let's Encrypt support #25

Closed

misterbisson mentioned this pull request Oct 31, 2016

Patterns for injecting SSL certificate and key #38

Open

		@@ -0,0 +1,45 @@
		#!/usr/bin/env bash

		CONSUL_ROOT="http://localhost:8500/v1/kv/nginx"


		### Configuring LetsEncrypt (ACME)

		Setting the `ACME_DOMAIN` environment variable will enable LetsEncrypt within

Add SSL support via ACME / LetsEncrypt #30

Add SSL support via ACME / LetsEncrypt #30

Uh oh!

Conversation

jasonpincin commented Sep 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross Oct 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross Oct 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross commented Oct 7, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jasonpincin commented Sep 30, 2016 •

edited

Loading

tgross Oct 7, 2016 •

edited

Loading

tgross Oct 7, 2016 •

edited

Loading