Implement a post-renewal hook script #205
Conversation
whi-tw
force-pushed
the
post-renewal-command
branch
from
584128f to
e0d024a
Compare
whi-tw
force-pushed
the
post-renewal-command
branch
from
e0d024a to
8671ada
Compare
Sometimes, after a certificate is renewed, you may want to do something
with the new certificate. For example, you may want to change the
ownership of the certificate or key files, use these files for some
other cryptographic purpose (eg. creating a `.p12` file) or some other
action.
This change allows for a renewal script to be created by specifying
commands with the `step_acme_cert_post_renewal_commands` variable.
An example of this is for provisioning a certificate for UniFi's
Controller. The following configuration will update unifi's jks and
restart the service after the certificate is renewed:
```yaml
step_acme_cert_post_renewal_commands:
- openssl pkcs12 -export -in "${CERT_FILE}" -inkey "${KEY_FILE}" -out /etc/ssl/cert.p12 -name unifi -password pass:aircontrolenterprise
- keytool -importkeystore -deststorepass aircontrolenterprise -destkeypass aircontrolenterprise -destkeystore /usr/lib/unifi/data/keystore -srckeystore /etc/ssl/cert.p12 -srcstoretype PKCS12 -srcstorepass aircontrolenterprise -alias unifi
- systemctl restart unifi
```
In the example above, I am using `systemctl restart unifi` as the last
command, because I have experienced issues with
`systemctl try-reload-or-restart` for this specific service. For a more
'normal' service, the following should work:
```yaml
step_acme_cert_post_renewal_commands:
- do_something ${CERT_FILE} ${KEY_FILE}
step_acme_cert_renewal_reload_services:
- some_service
```
The variables `${STEP_CLI}`, `${CERT_FILE}`, and `${KEY_FILE}` are all
exported in the script by default, and are available for use in the
commands.
`systemctl try-reload-or-restart {{step_acme_cert_renewal_reload_services}}`
has been removed from the `ExecStart` command in the systemd unit file,
and is appended to the end of this post-renewal hook script.
If no `step_acme_cert_renewal_reload_services` or
`step_acme_cert_post_renewal_commands` are provided, the post-renew-hook
script is not created.
This change is backwards compatible, and will not break existing
configurations.
Signed-off-by: Tom Whitwell <[email protected]>
whi-tw
force-pushed
the
post-renewal-command
branch
from
8671ada to
9db3559
Compare
There was a problem hiding this comment.
Thanks you for opening this PR, and thank you for showing a potential use-case right away, I can definitely see the benefit here. Looks good to me for the most part. There's a few things that need to be addressed, but I am happy to merge this once that's done.
And thank you for the through and detailed PR, it makes for a much easier review process!
Aside from the stuff mentioned in the comments, I'd also like to see some molecule tests added for the post-hook script. For that to work the test scenario would have to go through a renewal though, not sure if that's implemented right now. I'll have to look into that
| @@ -9,7 +9,7 @@ Type=simple | |||
| Restart=always | |||
| RestartSec=1 | |||
| Environment=STEPPATH={{ step_cli_steppath }} | |||
| ExecStart={{ step_cli_executable_absolute.stdout }} ca renew {{ step_acme_cert_certfile_full.path }} {{ step_acme_cert_keyfile_full.path }} --daemon --force{% if step_acme_cert_renewal_when is defined %} --expires-in {{ step_acme_cert_renewal_when }}{% endif %}{% if step_acme_cert_renewal_reload_services %} --exec "systemctl try-reload-or-restart {{ step_acme_cert_renewal_reload_services | join(' ') }}"{% endif %} | |||
| ExecStart={{ step_cli_executable_absolute.stdout }} ca renew {{ step_acme_cert_certfile_full.path }} {{ step_acme_cert_keyfile_full.path }} --daemon --force{% if step_acme_cert_renewal_when is defined %} --expires-in {{ step_acme_cert_renewal_when }}{% endif %}{% if step_acme_cert_post_renewal_commands or step_acme_cert_renewal_reload_services %} --exec "{{ step_acme_cert_post_renewal_shell }} {{ step_cli_steppath }}/{{ step_acme_cert_renewal_service }}_post.sh"{% endif %} | |||
There was a problem hiding this comment.
Given that the script already has a shebang, why don't we make the script executable and call it directly here? So --exec /root/.step/renewal_post.sh instead of a call to the shell with parameters?
| dest: "{{step_cli_steppath}}/{{ step_acme_cert_renewal_service }}_post.sh" | ||
| owner: root | ||
| group: root | ||
| mode: 0744 |
There was a problem hiding this comment.
Any reason for going with 744 over 755 here? I'm more used to seeing the latter
| export STEP_CLI="{{ step_cli_executable_absolute.stdout }}" | ||
| export CERT_FILE="{{ step_acme_cert_certfile_full.path }}" | ||
| export KEY_FILE="{{ step_acme_cert_keyfile_full.path }}" |
There was a problem hiding this comment.
I don't know how I feel about these. I get that they're convenient shorthands and that we kind of need them (since we are using step_acme_key/certfile_*full* internally, so we can't just tell people to use step_came_cert_certfile.path or whatever), but i'm wondering whether there's a way to avoid adding these. Then again, I guess having them doesn't hurt either ...
| @@ -0,0 +1,12 @@ | |||
| #!{{ step_acme_cert_post_renewal_shell }} | |||
| ####### added by ansible: maxhoesel.smallstep.step_acme_cert - changes will be overwritten ####### | |||
| set -eu | |||
There was a problem hiding this comment.
Maybe also add set -o pipefail for good measure?
| @@ -0,0 +1,12 @@ | |||
| #!{{ step_acme_cert_post_renewal_shell }} | |||
| ####### added by ansible: maxhoesel.smallstep.step_acme_cert - changes will be overwritten ####### | |||
There was a problem hiding this comment.
Seems like a good place to include {{ ansible_managed }}
maxhoesel
added
pr-minor
What
Sometimes, after a certificate is renewed, you may want to do something with the new certificate. For example, you may want to change the ownership of the certificate or key files, use these files for some other cryptographic purpose (eg. creating a
.p12file) or some other action.This change allows for a renewal script to be created by specifying commands with the
step_acme_cert_post_renewal_commandsvariable.Usage
An example of this is for provisioning a certificate for UniFi's Controller. The following configuration will update unifi's jks and restart the service after the certificate is renewed:
In the example above, I am using
systemctl restart unifias the last command, because I have experienced issues withsystemctl try-reload-or-restartfor this specific service. For a 'more normal' service, the following should work:systemctl try-reload-or-restart {{step_acme_cert_renewal_reload_services}}has been removed from theExecStartcommand in the systemd unit file, and is appended to the end of this new post-renewal hook script.The variables
${STEP_CLI},${CERT_FILE}, and${KEY_FILE}are all exported in the script by default, and are available for use in the commands.If no
step_acme_cert_renewal_reload_servicesorstep_acme_cert_post_renewal_commandsare provided, the post-renew-hook script is not created.This change is backwards compatible, and will not break existing configurations.
How to review
General
Basic Usage
step_acme_cert_post_renewal_commands.step_acme_cert_renewal_reload_services'Migration' from previous version
step_acme_cert_renewal_reload_services