simp_le client running as unprivileged user #18
Conversation
|
@jaywink this is a major change to the role internals, but I think the simp_le client is a better fit for our needs:
@robbyoconnor this role does not yet implement the simplifications from #15 - I will rebase as soon as it landed in master. It makes the cert.yml file much better to read, so I'd like to wait for your PR. |
|
@gronke no problem -- I just added travis CI tests for your nginx role |
|
@robbyoconnor this one is now rebased on top of #15 - your changes will be applied too (and GitHub should automatically mark your Pull-Request as merged) when this one is merged. Successfully tested compatibility of existing certificates created with certbot. The affected certificate permissions get updated, so that the |
|
👍 though it will not close.. |
|
Well this is indeed interesting, thanks @gronke . While I look into simp_le client, one question immediately creeps into my mind: how do you plan we offer cert renewal without shutting down the web server if we're going to be binding another service to port 80? Personally, while I understand the "no root" importance, I think even more important would be to enable renewing certs with zero downtime. I cringe whenever I run the role as a bunch of small sites have some seconds of possible requests going to By keeping Apache/nginx in play we already had some kind of plan to provide this renewal (comments in #5 ) - but I'm not entirely sure how we can do that if we start using the Python server on port 80? Any ideas on that or maybe I'm missing something. Thanks for your work. |
| @@ -52,28 +50,38 @@ Tested with the following: | |||
|
|
|||
| * `letsencrypt_domain` - Domain the certificate is for. | |||
| * `letsencrypt_email` - Your email as certificate owner. | |||
| * `letsencrypt_tos_sha256` - Provide the SHA256 hash of the latest Terms-of-Service you agreed to | |||
There was a problem hiding this comment.
Since it's in the defaults, no need to mention it here - nice section related to it in the readme which could also have this variable name.
There was a problem hiding this comment.
Sorry. The plan was to remove it from the default, so that the user needs to confirm the terms first. I think it's a good idea to use the hash of the document for that. It's important to leave a hint in the Readme and provide proper error messages when the hash is invalid or missing.
There was a problem hiding this comment.
I'd still like this default to be enabled in the defaults. The rationale is that the role should make things as easy as possible to use. Requiring the user to add extra variables to their role that could be provided as defaults IMHO is going backwards. The current role already has --agree-terms etc. The readme can clearly state which ToS the user will be accepting by using the role, and note that the user can change which ToS is being accepted via the variables.
There was a problem hiding this comment.
LetsEncrypt TOS handling can be discussed in a separate issue. To keep the previous behavior of --agree-terms being enabled by default, we can just set the default to the hash of the current terms of service document and make a new release whenever this document was updated.
| roles: | ||
| - role: ansible-letsencrypt | ||
| letsencrypt_user: "le-client-user" |
There was a problem hiding this comment.
Hmm this isn't mentioned in the readme required but is mentioned as letsencrypt in the defaults?
There was a problem hiding this comment.
The intention was to show how to override. But it was more confusing than explaining.
done
| letsencrypt_account_file: "{{ '/etc/letsencrypt/accounts/simp_le-' + ('staging' if letsencrypt_staging else 'production') + '.json' }}" | ||
| # ACME challenge server | ||
| letsencrypt_challenge_url: "https://{{ 'acme-staging' if letsencrypt_staging else 'acme-v01' }}.api.letsencrypt.org/directory" | ||
| # certbot custom arguments -- see: https://github.com/kuba/simp_le |
| letsencrypt_challenge_url: "https://{{ 'acme-staging' if letsencrypt_staging else 'acme-v01' }}.api.letsencrypt.org/directory" | ||
| # certbot custom arguments -- see: https://github.com/kuba/simp_le | ||
| letsencrypt_args: [] | ||
| letsencrypt_combined_args: [] |
There was a problem hiding this comment.
This is probably here by accident.
| # certbot custom arguments -- see: https://github.com/kuba/simp_le | ||
| letsencrypt_args: [] | ||
| letsencrypt_combined_args: [] | ||
| # default arguments passed to certbot |
| when: _letsencrypt_apt_packages is defined | ||
|
|
||
| - name: Dependencies are installed with RPM | ||
| apt: |
There was a problem hiding this comment.
.. you catched me pushing untested code 🙈
|
|
||
| # user and group configuration | ||
|
|
||
| - name: letsencrypt_group exits |
|
|
||
| # python webserver systemd wrapper | ||
|
|
||
| - name: webserver systemd service is installed |
There was a problem hiding this comment.
I may be horribly wrong, but AFAIK systemd is not supported under 15.04. This role needs to target also the still supported 14.04 LTS, which uses Upstart scripts.
There was a problem hiding this comment.
When ansible_service_mgr == "upstart" an upstart job will be deployed instead of a systemd service. Tested on 14.04.
|
|
||
| # virtualenv and python dependencies | ||
|
|
||
| - name: virtualenv environment exists |
There was a problem hiding this comment.
There is actually no need to create the virtualenv - it will be created by the next task pip, see docs.
| - name: Operating system dependencies | ||
| apt: name={{ item }} state=present | ||
|
|
||
| # dependencies |
There was a problem hiding this comment.
I guess the user needs to copy the vars from the correct file? Might need a readme entry for that.
Btw, we could look up machine facts to see whether it's Ubuntu, Debian or RPM based - though I have no idea how reliable that would be - especially the third.
There was a problem hiding this comment.
That's exactly how it's done here: tasks/main.yml#L9-L10
|
Since this PR needs a bit more discussion and I would like to do a release asap, preferably before introducing major rewrites. Should I merge in #15 before the release or shall we keep it in here? |
|
I had a few small changes on top of #15 applied during the rebase that removed unused variables. Please make sure to test the role before doing a release. The shutdown problem remains the same as with the standalone certbot version. The SimpleHTTPServer is just a replacement to provide the same feature. Since we now export the ACME challenge to a definable directory, it should be easy to configure the webserver accordingly. The question is when the configuration happens. IMO it should be done in the role configuring the webserver - and suddenly we're in the middle of a chicken and egg dilemma. Do we first configure the server without SSL, run LetsEncrypt and then modify the server configuration again? What about the idempotence? I'm sure this discussion would quickly exceed the scope of this PR. Anyway, we do not make the problem more complicated than it was before. |
|
My main point of concern is that AFAIK there is no way to keep for example Apache running (on port 80) while launching SimpleHTTPServer on port 80. Or am I wrong? Because this would automatically mean that existing sites (other sites too, all vhosts) always need to shut down for some seconds to allow binding the python web server to port 80. I know this change will not change anything regarding this - we already require downtime. But it might make fixing the downtime issue more difficult. We already have two choices how to not require downtime by dropping If we accept this change, can we in the future drop the python web server and serve whatever simp_le needs using Apache/nginx? This way we could use ad hoc vhosts and not require shutting down the web servers at all. Any other ideas how to use simp_le without needing to shut down Apache? |
|
Does it have to be Port 80? |
No, there are other types of challenge validation methods defined in the ACME specification. The only mechanism implemented in certbot today is Simple HTTP, which requires to be served from port 80. |
Yes. We need to introduce two more conditions to skip starting and stopping the Python server when not running in standalone mode. If that's the case, we expect the currently installed webserver to serve locations in I'd like to prevent us from messing with existing vhost configs, that were probably also maintained by Ansible. We could provide examples in the Readme how to configure popular webservers to serve static assets from the webroot location. With this being setup there is really no downtime - not even a risk of breaking a production server's config by accident with this role. |
|
@jaywink all your feedback is now addressed in this branch. Would you please give it a new review iteration? |
|
@gronke sorry for being silent. Will check these asap and then do a few test runs 👍 Btw, one thing I'm a bit worried about, digging into simp_le. The last commit to master is in April, the last github activity of the author in June, and for example in issues some are already worrying it's not maintained any more. I tried reaching out in the FreeNode #simp_le chat room given as support channel, the author isn't there and the only person who replied didn't know anything. Do you have any information on whether the project is still active? I know even if it goes out of maintenance someone might fork it, but still, something worth keeping in mind. I'm just worried hooking this role into something that might suddenly stop working (maybe LE changes something upstream) and the bug fixes end up taking a long time to resolve as people start pondering which fork to adopt. Anyway, will give this branch a test run tomorrow or as soon as possible. |
|
@jaywink no worries. Take all the time you need to review this branch! The main reason for picking simp_le was that it's the simplest client that is recommended by LetsEncrypt. It does less magic than then the official client and probably does not need so frequent updates for that reason. Here we use it for
Each of the above steps is well documented and easy to break out. Generating the key-pair could be for example easily done by calling OpenSSL directly. I'd be up to also remove the dependency on simp_le in the future and doing it all directly in Ansible. |
|
I've decided to do a one last release with the current master code now - let that be the last certbot release and then will get back to testing this. If simpl_le ends up not being active any more and no one maintains a fork, can always backtrack then, but let's push this soon to master to solve the root problem for now. Will be back in a few days. |
|
@jaywink did simp_le work for you? I wonder how I can help getting one more version with certbot released before we switch to simp_le (and maybe go further in the future to replace this with an Ansible python module). Let me know if there's anything I can do to support you. |
|
@gronke while working on other stuff I've been keeping an eye on simp_le and to be honest, it's looking more like an abandoned project as time goes by. See this issue. It's basically broken on default settings since August and the author hasn't even stepped in to say anything - actually it looks like the author hasn't done anything publicly in GitHub since June. Basing the role on something that is possibly a dead project feels wrong to me. Also, something that hit me when looking at that hash issue. If we use simp_le, it means as soon as LetsEncrypt introduces a change to their ToS, anybody using this role will suddenly start getting failures - right? At least that is what it looks like from the issue. This was not an issue with the old implementation. Granted, having no-root option is a heavy gain. I'm afraid I would like to wait with this for the author of simp_le to return or to see a stable fork of the project. I assume you're already using this version so hopefully that wont harm your usage of the role. If simp_le does not activate, maybe some other non-root enabling project could be considered? There seems to be a few. Sorry to keep you waiting. |
Yes, probably it is abandoned. But it's simple enough to be integrated into this repository quickly. I do not expect the ACME-challenge process to change dramatically in the future for http based authentication. We use the simp_le client for this features, which I'd anyway would like to re-implement in Ansible:
Everything beyond that is done in pure Ansible already. Would you feel better forking simp_le before using it as dependency?
The ToS hash is also included in certbot. If we do not upgrade the version manually in this repository, the ToS-Hash would also not be updated (automatically). IMO we should not read the ToS for our users. They should make sure themselves that they agree to LetsEncrypt ToS if they want to use their service. Therefore I would like make providing the hash as role parameter mandatory anyway (in a later pull-request).
Our next goal could be to write a helper that looks up certificate expiration dates and provide that information back to Ansible. In addition to that we would need to talk to the ACME API and writing the challenge response file structure. Using OpenSSL to generate the certificates with a command task is the easiest step. We could use simp_le as Python reference implementation. |
|
As discussed with @jaywink offline, I will fork this project with the aim to target generic services providing ACME-challenges (like LetsEncrypt) and an approach entirely based on Ansible tasks instead of third-party helper scripts. |
|
I'm confused... |
|
To ensure the stability and future compatibility of this role, we decided to remain using certbot as ACME client here. I'm going to fork this role and spike on a solution that does not use any third party tool at all. The first step towards this goal is the branch using the simp_le client instead of certbot, because it already replaced some of the tasks that certbot still does here - for instance running the webserver to serve the acme challenge response. @robbyoconnor your changes should be merged independently of this (now closed) pull-request and @jaywink requested some changes over there. |
|
@gronke great work! I made changes over there on my PR. |
|
@robbyoconnor yep, you were 8 minutes faster 🍻 |
fixes #17
closes #1
letsencrypt_tos_sha256ToDo
Ubuntu andCentOSmerge with [RDY] Make the entirety of the command args configurable #15