Skip to content

letsencrypt: Add option to let Plinth "manage" certbot's renewal hooks

Sunil Mohan Adapa requested to merge JoKeyser:renewalLEcerts into master

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 python3-configobj, apparently in Ubuntu something else, since Travis can't find it? While that's all a bit annoying, it seems a very common and sensible module, and it's what certbot uses as well for this file (though in python2).

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 action 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. Fixed in 688589fe.
  • 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.

Finally, a screenshot of the additional option: lerenewalhooks

Merge request reports

Loading