letsencrypt: Add option to let Plinth "manage" certbot's renewal hooks
Created by: JoKeyser
Hi, I'm looking forward to the review, please let me know if/how I can help it go smoothly. And make sure to squash the commits, there's plenty of "internal" changes to this PR.
This PR adds optional "management via Plinth" of automatic renewals for LE certificates, see issue #881 (closed). The management option pertains exclusively the currently configured domain (see screenshot below). The main work is done in a new letsencrypt manage_hooks
subcommand, which checks/changes the certbot configuration file at /etc/letsencrypt/$CURRENT_DOMAIN.conf
. To do that I use the configobj
module, which would add a dependency: in Debian it seems a very common and sensible module, and it's what certbot uses as well for this file (though in python2).python3-configobj
, apparently in Ubuntu something else, since Travis can't find it? While that's all a bit annoying,
Why is this useful?
If this PR's management option is disabled (current master), LE certificates obtained with Plinth get renewed, but without pre-, post-, or renew-hooks. This can lead to use of expired certificates, as apache2's restart is not ensured. With the current PR, only the renew-hook have an actual effect, but also the pre- and post hooks point to Plinth's letsencrypt action, for potential future use.
For existing LE certificates (maybe obtained without Plinth), the management option needs manual enabling. If enabled, the renewal options are made compatible with the Plinth's LE setup (e.g. obtaining via certbot webroot method), and the previous config file is backed up - to be restored if management is disabled again. If a certificate is (re-)obtained with Plinth, the management option will be enabled as well.
In the future, the configured calls can be expanded to provide integration of LE certs with other Plinth modules. The letsencrypt action subcommands run_(pre|renewal|post)_hooks
already include a --modules
option, which could be used to traverse the passed modules for further hooks in their action scripts.
How to test this?
To superficially test the hook configuration, run e.g. certbot --dry-run renew --cert-name $CURRENT_DOMAIN
, this will however not call the renew-hook. To make a full test, edit the file /etc/letsencrypt/$CURRENT_DOMAIN.conf
and (temporarily) set renew_before_exiry = 100 days
or something. Then call certbot without --dry-run
. I've tested this in the vagrant image; the renew-hook indeed restarts apache2. (Testing LE stuff in the vagrant image cost me some nerves, I made a humble blog post about it.)
Some known (minor?) issues I'm aware of, for (future?) improvement
-
Probably worst, changing the domain name will not disable the configured hooks. However the letsencrypt hook commands won't execute if they're called with another domain than the currently configured one. Still, it's much cleaner to disable them, and the config module sends the respective signal already. I'll look into how to react to them in the coming days; tips welcome.Fixed in 066126bc. -
For each letsencrypt page load, the letsencrypt actionFixed in 688589fe.get_status
subcommand is called twice; once for the get_status() for the table overview, and once when calling the new 'manage_hooks` action. I'll look into this in the coming days. - The new
letsencrypt manage_hooks enable
subcommand could be made to tolerate changing/creating even missing or invalid config files, e.g. in case an admin made a mistake, and now wants Plinth to take over. The current implementation simply aborts if such unforeseen circumstances are detected. - Maybe we want to react to invalid certs, which would become possible with PR #883.
- The 'edited by Plinth' comment in the config file could state the date and time for more transparency?
- Maybe the checkbox should be remade with a django form for letsencrypt (the action buttons are nice like they are I think). But I think it's more sensible to think about that when implementing module integration, which will add more options.
- There should be some tests around the new letsencrypt actions.