[enh] Certificate management integration (e.g. Let's Encrypt ...) #180

alexAubin · 2016-10-28T18:33:29Z

Hi guys,

here's a first proposition of integration of Let's Encrypt (and more generally certificate management).

It adds the following features :

Check certificate status

usage: yunohost domain cert-status [-h] [--full] [domainList [domainList ...]]

positional arguments:
  domainList  Domains to check

optional arguments:
  -h, --help  show this help message and exit
  --full      Show more details

Install Let's Encrypt certificates

usage: yunohost domain cert-install [-h] [--force] [--no-checks]
                                    [--self-signed]
                                    [domainList [domainList ...]]

positional arguments:
  domainList     Domains for which to install the certificates

optional arguments:
  -h, --help     show this help message and exit
  --force        Install even if current certificate is not self-signed
  --no-checks    Does not perform any check that your domain seems correcly
                 configured (DNS, reachability) before attempting to install.
                 (Not recommended)
  --self-signed  Install self-signed certificate instead of Let's Encrypt

Renew Let's Encrypt certificate

usage: yunohost domain cert-renew [-h] [--force] [--email] [--no-checks]
                                  [domainList [domainList ...]]

positional arguments:
  domainList   Domains for which to renew the certificates

optional arguments:
  -h, --help   show this help message and exit
  --force      Ignore the validity treshold (30 days)
  --email      Send an email to root with logs if some renewing fails
  --no-checks  Does not perform any check that your domain seems correcly
               configured (DNS, reachability) before attempting to renew. (Not
               recommended)

Some more details / explanations

Let's Encrypt certificates are managed using acme-tiny. The choice was driven by the fact that it is a lightweight module, somewhat easy to use, and can be installed through pip install acme-tiny ;
I edited the ssowatconf function to automatically authorize the ACME challenge uri /.well-known/blahif it is present in the nginx conf ;
cert-install automatically adds a small cron-job (2 lines of code), calling yunohost domain cert-renew --email every week, which will renew Let's Encrypt certificates which are about to expire, and send an email to root if some renewing fails ;
By default, cert-install and cert-renew include some checks that the domain seems to have a reasonable setup to attempt Let's Encrypt certificate installation (DNS poiting to global IP, + being able to GET a page) ;
I moved the generation of self-signed certificate into a new file, and cert-install can be used to regenerate a self-signed certificate (in case you don't want Let's Encrypt anymore for some reason). The short/mid-term goal is also to refactorize this part of code to get rid of os.system() calls and use the OpenSSL.crypto module instead.

There are more features we can think about including in the future, such as supporting other ACME authorities, supporting DNS-based challenge, set level of security (e.g. deactivate TLS v1.0 and 1.1), import certificates from non-ACME authorities, provide a diagnosis for domain (e.g. inform user a Let's Encrypt cert can be installed, display current SSL Labs rating) etc... But for now, let's focus on the basic stuff :)

Remaining work

How do we manage acme-tiny (and python-tabulate) as dependencies. At the moment this code does not work if you didn't do a pip install acme-tiny and pip install tabulate for example.
How do we make sure the transition from letsencrypt_ynh is smooth
???

Cheers !

… certificate install)

M5oul · 2016-10-29T09:11:55Z

sudo yunohost domain -h
    cert-install        Install Let's Encrypt certificates for given domains
                        (all by default).
    cert-status         List status of current certificates (all by default).
    list                List domains
    remove              Delete domains
    add                 Create a custom domain
    dns-conf            Generate DNS configuration for a domain
    cert-renew          Renew the Let's Encrypt certificates for given domains
                        (all by default).

Could cert-* sub-commands be grouped?
On the actionmap, there are all followed.
Why do they appear in this order?

M5oul · 2016-10-29T09:53:46Z

How do we manage acme-tiny (and python-tabulate) as dependencies. At the moment this code does not work if you didn't do a pip install acme-tiny and pip install tabulate for example.

As said on the chat room, there is three solutions:

Debian packages

That solution would be the best choice.
tabulate and acme-tiny Debian packages are available on Jessie backport and Debian Stretch.
May be we could install them using this or this solutions.

Subtrees

For now, we could add them as subtrees from respective repositories: tabulate and acme-tiny.
They need to move them to right place.

Pip

It's kind of ugly solution:

sudo pip install tabulate acme-tiny

I suggest to use subtrees solution for now, and Debian packages on Debian Stretch.

opi · 2016-10-29T14:39:36Z

src/yunohost/certificate.py

+
+def _install_cron() :
+
+    cron_job_file = "/etc/cron.weekly/certificateRenewer"


Maybe rename cron file to something more consistent with other yunohost cron. Currently we have /etc/cron.d/yunohost-applist-yunohost, /etc/cron.d/yunohost-firewall and /etc/cron.d/yunohost-dyndns.
I don't know if it worth to move this cron to /etc/cron.d as well.

I vote for /etc/cron.d/yunohost-certificate-renew ;)

opi · 2016-10-29T14:48:16Z

src/yunohost/certificate.py

+def _dns_ip_match_public_ip(public_ip, domain) :
+
+    try :
+        r = requests.get("http://dns-api.org/A/"+domain)


Please use https url ;)

I don't know if using 'dnspython' lib is easy, but I'd like to avoid using a third party API. Note that 'dnspython' is package in Debian's python-dns.

alexAubin · 2016-10-29T15:37:45Z

@M5oul

Could cert-* sub-commands be grouped?
On the actionmap, there are all followed.
Why do they appear in this order?

That seems related to the yaml file being loaded in a dictionnary in moulinette ? See here ?

Psycojoker · 2016-10-30T03:55:29Z

src/yunohost/certificate.py

+    else :
+        return True
+
+def _domain_is_accessible_through_HTTP(ip, domain) :


This test looks really weak, I don't think that this is the good way to do that but I don't have any other in mind right now.

opi · 2016-11-23T13:21:54Z

src/yunohost/certificate.py

@@ -731,13 +731,7 @@ def _dns_ip_match_public_ip(public_ip, domain):

 def _domain_is_accessible_through_HTTP(ip, domain):
    try:
-        # Check HTTP reachability
        requests.head("http://" + ip, headers={"Host": domain})


no matter what domain value is, this requests will not return any exception ;)

Maybe we should request /yunohost/admin or /yunohost/api to be sure that domain & ip will match. See https://dev.yunohost.org/issues/638 for example

As discussed on the chat, even requesting /yunohost/admin or /yunohost/api doesn't give a clear indication. Properly diagnosing the status/configuration of a domain seems to be a complicated problem 😛. Let's agree that these are some minimal checks for now, and can be refined later.

opi · 2016-11-23T13:24:05Z

src/yunohost/certificate.py

+        if ("urn:acme:error:rateLimited" in str(e)) :
+            raise MoulinetteError(errno.EINVAL,  m18n.n('certmanager_hit_rate_limit', domain=domain))
+        else :
+            raise


shouldn't we raise a MoulinetteError, with something like "Unknown error: ..." ?

Indeed. Fixed in e66a708.

…lready existing in nginx

Psycojoker · 2016-11-24T23:24:14Z

Warning: we need to merge this into unstable, not into testing (we can still have the discussion and review here)

zamentur · 2016-11-25T20:04:56Z

I have done a test with the presence of letsencrypt_ynh it was ok, the webadmin ask me to uninstall it before. For the cli I have add the answer:

root@yunohost:/vagrant/yunohost# yunohost domain cert-install testle4.grimaud.me
Error: certmanager_old_letsencrypt_app_detected

I have done a test with nginx disable:

root@yunohost:/vagrant/yunohost# yunohost domain cert-install testle4.exemple.com
Error: Certificate installation for testle4.exemple.com failed !
Error: [Errno 22] certmanager_domain_http_not_working

M5oul · 2016-11-25T20:44:58Z

Why displaying WARNING for self-signed certif?

yunohost domain cert-status
certificates: 
  le.moul.re: 
    CA_type: Self-signed
    summary: WARNING
    validity: 3649

M5oul

It works fine on my side.
I tested main commands and usage (not every stranges cases and details).
It could be merge.

M5oul · 2016-11-25T22:26:39Z

The A test check failed for a domain names I used.
With host command the DNS was spread on the DNS resolver of the VM.
Finally, I succeed to install the cert with yunohost domain cert-install --no-check --force.
But, from web admin interface, I could stayed blocked as the button was disabled and I couldn't bypassed it.

alexAubin · 2016-11-26T02:56:11Z

Why displaying WARNING for self-signed certif?

This is with the assumption that domains in Yunohost are supposed to be suitable for "public" use, not for a private use (i.e. just you, + maybe a few friends ?). If your domain is for private use, good for you : add the cert exception popping in your browser, or download the CA certificate - done, ignore the warning.

But if your domain is meant to be visited by some other, nontechnical people, (95% of the use case IMHO ?) you don't want them to run into the spooky warning of all modern browser about self-signed cert, and there's no reason to have a self-cert if you can install a LE cert with one command or two clicks. Hence the choice to report a warning. The meaning of the warning is not really explicit in the CLI, but it is a bit more in the web interface.

alexAubin · 2016-11-26T03:11:39Z

The A test check failed for a domain names I used.
With host command the DNS was spread on the DNS resolver of the VM.
Finally, I succeed to install the cert with yunohost domain cert-install --no-check --force.
But, from web admin interface, I could stayed blocked as the button was disabled and I couldn't bypassed it.

To me that sounds related to the propagation to the FFDN resolvers. We might actually have a lot of impatient user who run into this if they want their cert ASAP after setting up their domain 😛.

On the CLI you should have seen this message : "The DNS 'A' record for domain {domain:s} is different from this server IP. If you recently modified your A record, please wait for it to propagate (some DNS propagation checkers are available online). (If you know what you are doing, use --no-checks to disable those checks.)".

On the web interface, we could change the message to tell the user to try again later ? But that's in another PR anyway.

(Pro Tip ™️ : https://dnschecker.org/ allows to see the worldwide propagation of a DNS change)

alexduf · 2016-11-29T10:43:24Z

well done @alexAubin and all! I am super excited to this PR merged ! :)

First draft of certificate management integration (e.g. Let's Encrypt…

35fa386

… certificate install)

alexAubin mentioned this pull request Oct 28, 2016

WIP Letsencrypt integration #114

Closed

opi reviewed Oct 29, 2016

View reviewed changes

Psycojoker added 7 commits October 30, 2016 04:22

[mod] remove unused imports

d47f591

[mod] autopep8

bec8f63

[mod] trailing spaces

32bf742

[mod] pep8

02fc92d

[mod] remove useless imports

6a1727d

[mod] pep8

ef62877

[mod] more verbose error

aff4dc4

Psycojoker reviewed Oct 30, 2016

View reviewed changes

Psycojoker added 15 commits October 30, 2016 04:55

[mod] more pythonic and explicit tests with more verbose errors

917c230

[mod] more pythonic code

29f5f2d

[mod] remove useless ()

39aaa17

[fix] correctly handle all cases

7dbbd7f

[mod] simplier condition

ea179b1

[fix] uses https

9a4dbd5

[mod] more pythonic code

ac9f61c

[mod] uses + for strings

ac90152

[mod] pep8

cd21edb

[mod] lisibility

a7b9226

[mod] remove useless ()

dd893b0

[mod] more pythonic string concatenation

34f9890

[mod] avoid useless indentation

55d007f

[mod] remove useless variable

e8e07d4

[mod] uses logger string concatenation api

5615e3f

Adding an option to use the staging Let's Encrypt CA, sort of a dry-run

ed16cd7

opi reviewed Nov 23, 2016

View reviewed changes

alexAubin added 2 commits November 23, 2016 11:46

Misc tweaks on exceptions

e66a708

More exception handling, this time for previous acme challenge conf a…

195c675

…lready existing in nginx

alexAubin changed the title ~~Certificate management integration (e.g. Let's Encrypt ...)~~ [enh] Certificate management integration (e.g. Let's Encrypt ...) Nov 24, 2016

alexAubin added 2 commits November 24, 2016 10:40

Daily renew cron job instead of weekly

1b20899

pep8

ddcc57e

M5oul changed the base branch from testing to unstable November 24, 2016 23:31

zamentur approved these changes Nov 25, 2016

View reviewed changes

M5oul approved these changes Nov 25, 2016

View reviewed changes

Psycojoker added 6 commits November 28, 2016 14:32

[mod] no space before a ? in english

7f046f0

[mod] create list in one step

5b73ab4

[fix] avoid reverse order log display on web admin

86eb9a2

[mod] pep8

2f4f254

[mod] merge double with

4a9b89d

[mod] style

f0c2914

Psycojoker added the medium/standard decision label Nov 28, 2016

Psycojoker approved these changes Nov 28, 2016

View reviewed changes

Psycojoker merged commit 34ca628 into YunoHost:unstable Nov 28, 2016

M5oul added this to the 2.5.0 milestone Dec 11, 2016

This was referenced May 24, 2018

Let's Encrypt support YunoHost/issues#2

Closed

Let's Encrypt certificate installations on IPv6-only instances YunoHost/issues#769

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[enh] Certificate management integration (e.g. Let's Encrypt ...) #180

[enh] Certificate management integration (e.g. Let's Encrypt ...) #180

alexAubin commented Oct 28, 2016

M5oul commented Oct 29, 2016

M5oul commented Oct 29, 2016

opi Oct 29, 2016

opi Oct 29, 2016

opi Oct 29, 2016

alexAubin commented Oct 29, 2016

Psycojoker Oct 30, 2016

opi Nov 23, 2016

alexAubin Nov 23, 2016

opi Nov 23, 2016

opi Nov 23, 2016

alexAubin Nov 23, 2016

Psycojoker commented Nov 24, 2016

zamentur commented Nov 25, 2016

M5oul commented Nov 25, 2016

M5oul left a comment •

edited

Loading

M5oul commented Nov 25, 2016

alexAubin commented Nov 26, 2016

alexAubin commented Nov 26, 2016

alexduf commented Nov 29, 2016


		def _install_cron() :

		cron_job_file = "/etc/cron.weekly/certificateRenewer"

[enh] Certificate management integration (e.g. Let's Encrypt ...) #180

[enh] Certificate management integration (e.g. Let's Encrypt ...) #180

Conversation

alexAubin commented Oct 28, 2016

M5oul commented Oct 29, 2016

M5oul commented Oct 29, 2016

Debian packages

Subtrees

Pip

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexAubin commented Oct 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Psycojoker commented Nov 24, 2016

zamentur commented Nov 25, 2016

M5oul commented Nov 25, 2016

M5oul left a comment • edited Loading

Choose a reason for hiding this comment

M5oul commented Nov 25, 2016

alexAubin commented Nov 26, 2016

alexAubin commented Nov 26, 2016

alexduf commented Nov 29, 2016

M5oul left a comment •

edited

Loading