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

allow certificates to be expanded #117

Closed
wants to merge 1 commit into from

Conversation

ymarkus
Copy link

@ymarkus ymarkus commented Jun 26, 2020

After being a bit disappointed that I can't append to certs with this role, I thought why not try to fix it myself.

This is the solution I came up with. It should fix multiple open issues, such as #113 and #43 (which is stale). PR #50 already proposed a solution which was rejected by @geerlingguy with the reason "I don't generally like to pollute managed directories with files to track state, and that's what this seems to be doing (maintaining a list of domains in a file in the letsencrypt directories)."

This PR works without creating any files and uses only certbot, grep and awk.

Short summary of how it works:

  1. Get list of existing certs and their domains (using certbot)
  2. Parse that output as preperation for ansible (using grep & awk)
  3. Check the cert with its domains that is supposed to exist against the list from above
  4. If there is no match, create (or expand) the cert

Tested on a NixOS host running ansible 2.9.9 and a debian 10 client.

Potential issue: This PR does not check for domains that were removed from the cert. I'm sure this is not a problem, as the function is not present anyway.

@ymarkus
Copy link
Author

ymarkus commented Jun 26, 2020

Had to fix some smaller fixes the check marked, like a conditional being ... == false

@MonsieurV
Copy link

The implementation seems great for me: it uses the Certbot CLI to get the current state (list of domains), and compare it to the intended state.

Maybe one culprit: if they are multiple certificates, I'm not sure the grep and awk (as currently implemented) will extract the right domain list from certbot certificates output. Shouldn't it be able to sort out the specific domain list from multiple certificate entries?


Example n°1: one certificate

The certbot certificate output for an unique certificate entry (with multiple domains) is like the following:

Saving debug log to /var/log/letsencrypt/letsencrypt.log

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Found the following certs:
  Certificate Name: www.example.com
    Serial Number: 441319d7aa87ad735838dff43791b4d208b
    Domains: www.example.com admin.example.com api.example.com blog.example.com example.com
    Expiry Date: 2020-08-04 11:56:17+00:00 (VALID: 58 days)
    Certificate Path: /etc/letsencrypt/live/www.example.com/fullchain.pem
    Private Key Path: /etc/letsencrypt/live/www.example.com/privkey.pem
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

With the string parsing | grep "Domains:" | awk '{ gsub(/ Domains: /,""); print }' applied, it gives:

www.example.com admin.example.com api.example.com blog.example.com example.com

which is perfect.


Example n°2: multiple certificates

The certbot certificate output for 3 certificate entries (each with multiple domains) is like the following (source):

Saving debug log to /var/log/letsencrypt/letsencrypt.log

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Found the following certs:
  Certificate Name: www.example.com
    Serial Number: 441319d7aa87ad735838dff43791b4d208b
    Domains: www.example.com admin.example.com api.example.com blog.example.com example.com
    Expiry Date: 2020-08-04 11:56:17+00:00 (VALID: 58 days)
    Certificate Path: /etc/letsencrypt/live/www.example.com/fullchain.pem
    Private Key Path: /etc/letsencrypt/live/www.example.com/privkey.pem
  Certificate Name: ansible.example.com
    Serial Number: 441319d7aa87ad735838dff43791b4d203a
    Domains: ansible.example.com ansible1.example.com ansible2.example.com 
    Expiry Date: 2020-08-04 11:56:17+00:00 (VALID: 58 days)
    Certificate Path: /etc/letsencrypt/live/ansible.example.com/fullchain.pem
    Private Key Path: /etc/letsencrypt/live/ansible.example.com/privkey.pem
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

With the string parsing | grep "Domains:" | awk '{ gsub(/ Domains: /,""); print }' applied, it gives:

www.example.com admin.example.com api.example.com blog.example.com example.com
ansible.example.com ansible1.example.com ansible2.example.com

which doesn't give the expected result.

For this case, we could:

  1. create a more advanced string parsing by first selecting the certificate name
  2. use --cert-name CLI parameter, for eg. certbot certificate --cert-name=www.exemple.com

@ymarkus
Copy link
Author

ymarkus commented Jul 4, 2020

Yes, that is why I'm using the letsencrypt_certs.stdout_lines in a loop after that. Then (in your example) it would first check against the first line www.example.com admin.example.com api.example.com blog.example.com example.com and after that against the second line ansible.example.com ansible1.example.com ansible2.example.com. I've tested it with up to 4 unique certificates that each have a lot of unique domain names and it works flawlessly.

I think using your options is not a viable solution, as changing the order would result in this script trying to generate new certs, even though they are the same, see example below

- domains:
    - example.com
    - example.net
    - example.de
- domains:
    - ansible.example.com
    - ansible.example.net
    - ansible.example.de

and

- domains:
    - example.net
    - example.com
    - example.de
- domains:
    - ansible.example.net
    - ansible.example.com
    - ansible.example.de

would be regarded as different, even though they're the same...

@stale
Copy link

stale bot commented Oct 3, 2020

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark pull requests as stale.

@stale stale bot added the stale label Oct 3, 2020
@stale
Copy link

stale bot commented Nov 2, 2020

This pull request has been closed due to inactivity. If you feel this is in error, please reopen the pull request or file a new PR with the relevant details.

@stale stale bot closed this Nov 2, 2020
@reikjarloekl
Copy link

@geerlingguy such a pitty IMHO that this was not merged. Would have been very useful to me multiple times already.

@ymarkus
Copy link
Author

ymarkus commented Feb 23, 2021

You could just patch it locally and use it. That's what I'm doing.

@aded-cmcc
Copy link

Is there any reason why this PR was not applied?

@geerlingguy
Copy link
Owner

In terms of the first change (addition of --expand), that can easily be overridden in your playbook: https://github.com/geerlingguy/ansible-role-certbot/blob/master/defaults/main.yml#L20

@aded-cmcc
Copy link

In terms of the first change (addition of --expand), that can easily be overridden in your playbook: https://github.com/geerlingguy/ansible-role-certbot/blob/master/defaults/main.yml#L20

Hi @geerlingguy , thanks for your answer and thanks for your roles, they are very useful! :-)
We can certainly override the var certbot_create_command, but we also need the check on domain list as @ymarkus did, right? BTW the code has slightly changed in the meantime, you added pre/post hooks so the only task with when to be updated is Generate new certificate if one doesn't exist.

@ymarkus ymarkus deleted the append-cert branch December 31, 2021 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants