Bug 1444814
Summary: | certbot: error: argument --pre-hook: expected one argument | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora EPEL | Reporter: | Brian J. Murrell <brian> | ||||
Component: | certbot | Assignee: | James Hogarth <james.hogarth> | ||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | urgent | Docs Contact: | |||||
Priority: | unspecified | ||||||
Version: | epel7 | CC: | alexey, bmw, igeorgex, itamar, james.hogarth, mharris, nb, nick, rbu, rhbz | ||||
Target Milestone: | --- | ||||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | If docs needed, set a value | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2017-09-11 14:39:54 UTC | Type: | Bug | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Attachments: |
|
Description
Brian J. Murrell
2017-04-24 11:22:52 UTC
Any ideas here? This is preventing (automatic) renewals from happening and I don't want my certs to expire. The quoting done in the sysconfig file is incorrect and should be done in the systemd unit file itself. As indicated in https://www.freedesktop.org/software/systemd/man/systemd.service.html#Command%20lines > Double quotes ("…") and single quotes ('…') may be used, in which case > everything until the next matching quote becomes part of the same argument. > Quotes themselves are removed The default variables containing only quotes (PRE_HOOK="''") are expanded into nothing I'll attach a patch file correcting the systemd unit by using the ${} syntax to force forming a single argument from the hooks variables, which seems to work even with empty variables. As a workaround, I'm having success by setting "/bin/true" as the XXX_HOOK parameters in the sysconfig file for hooks that I'm not using, with the current unit file Created attachment 1275745 [details]
patch to the unit file using ${} syntax
The result of that patch, when applied here manually seems to yield better results. Thanks much! I can reproduce the issue and confirm the workaround mentioned in https://bugzilla.redhat.com/show_bug.cgi?id=1444814#c2 appears to work. Same problem here, the patch from comment #3 works and solved the problem. Thanks for the investigation work, not sure then why it worked on my testing here but then that was on Fedora and I, obviously wrongly, assumed the same behaviour on systemd for them both. This issue I have is that I wanted to include the --pre-hook (and post etc) in the certbot renew ExecStart directly, but with "" in sysconfig there was no value provided to that which then gets an error. When I tested with "''" in sysconfig then '' was provided as the value which then worked. I'll do another verify on AWS on both fedora and centos and then issue the update to fix this by default as part of 0.14.0 Right after doing some testing with the certbot 0.14.0 build in AWS trying to have --post-hook in the ExecStart by default just isn't going to work ... Leave it empty and it complains it needs an argument, make the argument '' and it complains there is no executable it can run. So what I'm going to move to is having the line: ExecStart=/usr/bin/certbot renew $PRE_HOOK $POST_HOOK $RENEW_HOOK $CERTBOT_ARGS Then in sysconfig the various hooks will be populated as: POST_HOOK="" To make use of the hook, and this will be a comment you can just uncomment in sysconfig, the full argument will need to be set POST_HOOK="--post-hook 'systemctl restart httpd'" I just tested this setup and it worked correctly. Please pay attention to your configs to merge it okay and make it happy. That seems counter-intuitive and error-prone. At that point I would consider just dropping the *_HOOK args and letting people put whatever they need in the CERTBOT_ARGS variable. What do you think of using the same logic as for the current workaround and having /bin/true as the default value instead ? This also has the benefit of allowing the use of the ${VARIABLE} syntax sparing us from having to remember to escape things in the sysconfig file. I did think about dropping the *_HOOK env vars ... but I liked the clarity of having them there ... plus it would be even more of a drastic change from 0.12.0 I'm good with defaulting them to /bin/true though as the workaround to make it all happy. I'll add that to a fresh build over the weekend and include it as part of the 0.14.x package. certbot-0.14.1-2.fc26 python-acme-0.14.1-1.fc26 python-certbot-apache-0.14.1-1.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-64270716a0 certbot-0.14.1-2.fc26 python-acme-0.14.1-1.fc26 python-certbot-apache-0.14.1-1.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-64270716a0 certbot-0.14.1-2.fc25 python-acme-0.14.1-1.fc25 python-certbot-apache-0.14.1-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-f1071b956e certbot-0.14.1-2.fc25 python-acme-0.14.1-1.fc25 python-certbot-apache-0.14.1-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-f1071b956e certbot-0.14.1-2.fc24 python-acme-0.14.1-1.fc24 python-certbot-apache-0.14.1-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-c651872919 certbot-0.14.1-2.fc24 python-acme-0.14.1-1.fc24 python-certbot-apache-0.14.1-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-c651872919 certbot-0.14.1-2.el7 python-acme-0.14.1-1.el7 python-certbot-apache-0.14.1-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-acdfbe8cfa certbot-0.14.1-2.el7 python-acme-0.14.1-1.el7 python-certbot-apache-0.14.1-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-acdfbe8cfa Apologies for the delay but there was breakage shown in the tests with augeas 1.7+ I'd appreciate if you can test the new 0.14.1 build and if it meets your expectations provide karma on the builds so we can get it out to branches ASAP certbot-0.14.1-2.fc26, python-acme-0.14.1-1.fc26, python-certbot-apache-0.14.1-1.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-64270716a0 certbot-0.14.1-2.el7, python-acme-0.14.1-1.el7, python-certbot-apache-0.14.1-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-acdfbe8cfa certbot-0.14.1-2.fc24, python-acme-0.14.1-1.fc24, python-certbot-apache-0.14.1-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-c651872919 certbot-0.14.1-2.fc25, python-acme-0.14.1-1.fc25, python-certbot-apache-0.14.1-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-f1071b956e I'm the technical lead on Certbot and there's actually another substantial problem that's made worse by the changes here. If you run something like: ``` certbot certonly --standalone --pre-hook "systemctl stop nginx" --post-hook "systemctl start nginx" ``` These values get saved in /etc/letsencrypt/renewal/domain.conf like: ``` ... [renewalparams] authenticator = standalone post_hook = systemctl start nginx pre_hook = systemctl stop nginx ... ``` Normally when you run `certbot renew`, these hooks are reused when it comes time to renew that certificate, however, if your certificates are renewed by https://src.fedoraproject.org/cgit/rpms/certbot.git/tree/certbot-renew-systemd.service?id=ef6c3a40fba22c36585912c447c8ef0c1db3650d, these hooks are ignored and /etc/letsencrypt/renewal/domain.conf becomes: ``` ... [renewalparams] authenticator = standalone post_hook = /bin/true pre_hook = /bin/true renew_hook = /bin/true ... ``` Previously, certbot-renew.service would fail about an invalid argument but if the user had scripted their own renewal, it can actually break things by overwriting their hooks they may have been relying on. It also removes the ability for users to define different hooks for each certificate. I'm happy to open another bug report about this but I was directed here and this is definitely related. The easiest course of action is to remove the *_HOOK vars entirely but there are other options as well. This seems to be a problem indeed for users that have setup hooks when first getting a certificate. However, neither certbot --help certonly or the online docs mention this behavior, or even that the --*-hook options are available for any other command than renew. The closest I found to a hint that this would work is this paragraph documenting the renew command: > For more fine-grained control, you can renew individual lineages with the > `certonly` subcommand. Hooks are available to run commands before and > after renewal; see https://certbot.eff.org/docs/using.html#renewal for > more information on these. And also this issue: https://github.com/certbot/certbot/issues/4103 So it seems reasonable to assume, at least for now, that no hooks have been setup by most users when getting the certificates in the first place, since they wouldn't know it is possible. So overriding them wouldn't be an issue. As a user, I would argue for the complete renewal of the sysconfig file anyway, and leave only a $CERTBOT_ARGS variable in the systemd unit file to be overriden by a systemd partial unit if the user wants special parameters. Maybe it could be migrated with a postinst scriptlet that would move any hooks defined by /etc/sysconfig/certbot into the config files in /etc/letsencrypt (without overriding existing ones), to ensure compatibility when the variables are removed ? In any case, you should probably be documenting that behavior, it is quite useful to have hooks by domain ! So, I've given this a lot of thought and I do like splitting out and specifying the hooks in sysconfig for the considerable number of situations that a global hook makes sense and it looks cleaner with the different sections so far easier to read. However I would like to be able to support per certificate configuration of hooks, as I can see a lot of potential in that. In light of the upstream developer feedback highlighting the known issues I've made my decision and the package is going to work as follows, now that I've carried out some testing. The sysconfig put in place will have it split out as previous, but the values of PRE_HOOK, POST_HOOK and RENEW_HOOK will default to "" so that there's no argument at all provided to certbot renew by default, which permits the per certificate configuration. The examples in the sysconfig that can be uncommented will include the --post-hook etc to make it clear they need to be included. There will also be a note in the sysconfig pointing out per certificate config for hooks is possible. See here for what I mean: https://src.fedoraproject.org/cgit/rpms/certbot.git/tree/certbot-sysconfig-certbot I have tested this and it does work as expected. I am keeping with the sysconfig file as that's what the vast majority of people expect on a Red Hat style system and requiring that people use systemctl edit to add a [Service] override for Environment=CERTBOT_ARGS="--post-hook 'foo'" is not very intuitive to most. I feel that this will provide maximum flexibility and the greatest ease of use. I do apologise for the need to update any configurations when this goes stable, but this interface will remain stable going forwards unless something really unexpected happens. I hope you understand and appreciate my reasoning on this. certbot-0.14.1-3.el7 python-acme-0.14.1-1.el7 python-certbot-apache-0.14.1-1.el7 python-certbot-nginx-0.14.1-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-acdfbe8cfa certbot-0.14.1-3.fc26 python-acme-0.14.1-1.fc26 python-certbot-apache-0.14.1-1.fc26 python-certbot-nginx-0.14.1-1.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-64270716a0 certbot-0.14.1-3.fc24 python-acme-0.14.1-1.fc24 python-certbot-apache-0.14.1-1.fc24 python-certbot-nginx-0.14.1-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-c651872919 certbot-0.14.1-3.fc25 python-acme-0.14.1-1.fc25 python-certbot-apache-0.14.1-1.fc25 python-certbot-nginx-0.14.1-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-f1071b956e The problem is that 'certbot renew' handles all and any installed cert Therefore put a hook to systemd is a good idea inly if you have just one use of all the serts. But if you have, for example, http and email servers - you have to run different hooks on different certificates. It's not a case for systemd. IMHO, the approach to handle it: 1. Keep just one CERTBOT_ARGS in syscinfig. This is pretty enough to pass any extra paraveters. 2. Clearly describe certbot behaviour on relation between cert generation hooks and renewal config in certbot's configuration. To explain that cortbot remembers the last hook in /etc/letsencrypt/renewal/domain.conf and the hook SHOULD be set there and MAY be overridden in sysconfig. Think, it's useful to remove hook refresh on 'renew' action just for simple reason: the probabilty we need to use different hook on renewal seems very low. certbot-0.14.1-3.fc26, python-acme-0.14.1-1.fc26, python-certbot-apache-0.14.1-1.fc26, python-certbot-nginx-0.14.1-1.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-64270716a0 certbot-0.14.1-3.fc24, python-acme-0.14.1-1.fc24, python-certbot-apache-0.14.1-1.fc24, python-certbot-nginx-0.14.1-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-c651872919 certbot-0.14.1-3.el7, python-acme-0.14.1-1.el7, python-certbot-apache-0.14.1-1.el7, python-certbot-nginx-0.14.1-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-acdfbe8cfa certbot-0.14.1-3.fc25, python-acme-0.14.1-1.fc25, python-certbot-apache-0.14.1-1.fc25, python-certbot-nginx-0.14.1-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-f1071b956e certbot-0.14.1-3.fc24, python-acme-0.14.1-1.fc24, python-certbot-apache-0.14.1-1.fc24, python-certbot-nginx-0.14.1-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report. certbot-0.14.1-3.fc25, python-acme-0.14.1-1.fc25, python-certbot-apache-0.14.1-1.fc25, python-certbot-nginx-0.14.1-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report. certbot-0.14.1-3.el7, python-acme-0.14.1-1.el7, python-certbot-apache-0.14.1-1.el7, python-certbot-nginx-0.14.1-1.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report. certbot-0.14.1-3.fc26, python-acme-0.14.1-1.fc26, python-certbot-apache-0.14.1-1.fc26, python-certbot-nginx-0.14.1-1.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report. |