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

Enable Let's Encrypt to detect updated site_hosts #630

Merged
merged 2 commits into from Jan 23, 2017

Conversation

Projects
None yet
2 participants
@fullyint
Copy link
Member

fullyint commented Aug 5, 2016

Update CSRs and certs

The primary problem to solve: If a user has already created Let's Encrypt CSRs and certs then modifies site_hosts, the LE role will fail to update the CSRs/certs.

If an LE cert exists, it will not update if not older than 60 days. If a CSR exists, it will not be recreated because the creates parameter specifies the CSR file, and "when it already exists, this step will not be run."

This PR updates the LE role to detect changes to site_hosts and update the CSR. The PR also updates the renew-certs.py to detect changes to the CSR and update the cert, even if younger than 60 days.

The CSR updates become idempotent via a new rsync like Trellis already uses for the .env file (details on rationale of using rsync for idempotence).

Add acme challenge location to conf

Edit: The remainder of this comment is now obsolete after internal discussion and a decision to avoid the replace module.

If a user has just updated to a version of Trellis that includes the LE role, the LE role's Test Acme Challenges would fail because the wordpress-setup role hasn't yet had a chance to insert include acme-challenge-location.conf; into the conf. This challenge location could also be missing for users with current Trellis with ssl enabled with a non-LE provider. This missing challenge location has required users to run the wordpress role first to update their conf before running the letsencrypt role, an unexpected hassle.

This PR updates the LE role to detect the missing challenge location and inserts it into the conf so that users no longer must run the wordpress role first. This feature is convenient but relies on the replace module, which introduces some risk. There are some reassurances, however.

  • The replace task will not even run if the challenge location is already in the conf.
  • If an unforeseen regex failure messes up the conf, the user can fix it by running the wordpress tag, which they would have had to do anyway without this attempted replace. Then the challenge location will be in the conf and the replace won't ever run again for that user/project.
  • The replace regex is designed to remove nothing. It should return everything matched, just adding the challenge location conf.

Explaining some regex (suggestions welcome)
example: (listen 80;(?:[^\{]*\n)*)(.*return 301 \$scheme:\/\/.*\$request_uri;)

  • (listen 80;(?:[^\{]*\n)*) -- backref 1
    • listen 80; -- this portion of the match must begin with listen 80;
    • (?:[^\{]*\n)*
      • The ?: prevents the creation of a separate backref for this (?:pattern).
      • The [^\{]*\n will match any character(s) -- except { -- followed by a newline. Excluding the { prevents matching across server/location blocks. For example, we wouldn't want a match to start at the listen 80; from the "ssl redirect server block" and finish on the return 301 in the following "general redirects server block."
      • The final * lets there be multiple instances of [^\{]*\n
  • (.*return 301 \$scheme:\/\/.*\$request_uri;) -- backref 2

The match patterns appear applicable to the conf for the past year (back to ff0fbff, June 30, 2015).

Handlers

Edit: The handler adjustment separated out to #722.

@fullyint

This comment has been minimized.

Copy link
Member

fullyint commented Dec 31, 2016

After generous and painstaking review from @swalkinshaw, I think this is ready.

This PR originally intended to solve the problem that CSRs and certs would not detect changes in site_hosts and would fail to update.

Upon further review, I see that other crucial changes would go undetected, but should trigger cert regeneration, i.e., changes in any of the following:

  • var: site_hosts (the original issue of this PR)
  • var: letsencrypt_intermediate_cert_sha256sum
  • var: acme_tiny_commit
  • var: letsencrypt_ca
  • file: /etc/nginx/ssl/letsencrypt/{{ site }}.key
  • file: /var/lib/letsencrypt/account.key

The revised 2nd commit hashes the combination of the above, per site, inserting a corresponding 7-digit ID into CSR and cert filenames. If any item above changes, the required filename will change, triggering creation of new CSRs and certs (if they don't already exist with this ID).

Edit: I edited the 2nd commit, adjusting this line in defaults/main.yml to accommodate a wordpress_sites list with a mix of SSL enabled/disabled sites:

-letsencrypt_cert_ids: "{ {% for item in generate_cert_ids.results %}'{{ item.item.key }}':'{{ item.stdout }}', {% endfor %} }"
+letsencrypt_cert_ids: "{ {% for item in generate_cert_ids.results if not item | skipped %}'{{ item.item.key }}':'{{ item.stdout }}', {% endfor %} }"

How to update

New servers

New servers provisioned with Trellis (after this PR is merged) will accommodate changes to site_hosts or any of the above items.

Existing servers that do NOT need to add hosts to site_hosts

  1. Update trellis (get changes from this PR)
  2. Run ansible-playbook server.yml -e env=<environment> --tags letsencrypt

Then, any future changes to site_hosts etc. will work no problem for this project.

These steps should also work for existing servers that need to REMOVE hosts from site_hosts.

Existing servers that DO need to add hosts to site_hosts

  1. Update trellis (get changes from this PR)
  2. Set ssl enabled: false for all sites in group_vars/<environment>/wordpress_sites.yml
  3. Run ansible-playbook server.yml -e env=<environment> --tags wordpress
  4. Set ssl enabled: true for applicable sites in group_vars/<environment>/wordpress_sites.yml
  5. Run ansible-playbook server.yml -e env=<environment> --tags letsencrypt

Then, any future changes to site_hosts etc. should work no problem for this project.

Existing servers that use non-Let's Encrypt certs and are transitioning to Let's Encrypt certs will probably need to use these 5 steps.

@swalkinshaw

This comment has been minimized.

Copy link
Member

swalkinshaw commented Jan 22, 2017

Let's go forward with this 👍

Needs changelog + maybe docs?

fullyint added some commits Aug 4, 2016

Use IDs in filenames to ensure CSRs and certs update when needed
Each ID is a hash of the combination of the following:
* var: site_hosts
* var: letsencrypt_intermediate_cert_sha256sum
* var: acme_tiny_commit
* var: letsencrypt_ca
* file: /etc/nginx/ssl/letsencrypt/{{ site }}.key
* file: /var/lib/letsencrypt/account.key
@swalkinshaw

This comment has been minimized.

Copy link
Member

swalkinshaw commented Jan 23, 2017

Awesome work :shipit:

@fullyint fullyint merged commit 85a11bf into master Jan 23, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@fullyint fullyint deleted the letsencrypt branch Jan 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment