Support ACME for certificate provisioning#4384
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4384 +/- ##
===========================================
- Coverage 73.7% 73.58% -0.12%
===========================================
Files 300 301 +1
Lines 29705 29822 +117
Branches 4882 4894 +12
===========================================
+ Hits 21895 21946 +51
- Misses 6385 6446 +61
- Partials 1425 1430 +5 |
|
Note for reviewers (@richvdh , @erikjohnston ): this removes the I'm guessing I'll need to add another newsfile, or split that into a different PR. More info about why plain DHE is bad: https://weakdh.org/sysadmin.html |
richvdh
left a comment
There was a problem hiding this comment.
This is looking nice now.
Something I don't quite understand here. I thought you needed an account key to get certs from LE, and that you were supposed to reuse the same account key for subsequent renewals. I'm not seeing anything about that here... what am I missing?
As for the dh_params thing... I would love it if it could be pulled out to a completely separate PR, but if that's a bit of a faff I won't argue.
CI is sad, at least partially about dh_params.
Co-Authored-By: hawkowl <hawkowl@atleastfornow.net>
@richvdh This is because it registers and uses a consistent |
|
@richvdh I'll pull the |
…/matrix-org/synapse into hawkowl/acme-portable-certificates
synapse/config/tls.py
Outdated
| ) | ||
| self.acme_port = acme_config.get("port", 8449) | ||
| self.acme_bind_addresses = acme_config.get("bind_addresses", ["127.0.0.1"]) | ||
| self.acme_reprovision_threshold = acme_config.get("reprovision_threshold", 10) |
There was a problem hiding this comment.
LetsEncrypt as the main cert provider via ACME suggest renewal after 60 days (at 30 days to go). They currently send emails to warn of un-renewed certs at 20 and 10 days, so this may want to be about 30 days as a default.
synapse/config/tls.py
Outdated
| acme_config = config.get("acme", {}) | ||
| self.acme_enabled = acme_config.get("enabled", False) | ||
| self.acme_url = acme_config.get( | ||
| "url", "https://acme-staging.api.letsencrypt.org/directory" |
There was a problem hiding this comment.
Don't forget to set the default to be the live endpoint
There was a problem hiding this comment.
Also, do we want to consider using the v2 API now , if it's a simple change in our use of the library, to prevent us having to move to v2 when they decommission (timeline unspecified, so if it's a lot of work it is definitelyworth punting down the road)
There was a problem hiding this comment.
txacme does not yet support v2. We can probably put a little bit of work in to make it support it, later.
There was a problem hiding this comment.
WRT live endpoint -- I think it should be the staging one by default, since the real one has rate limits, and we dont' want someone to get accidentally blacklisted while setting up their server, I think.
There was a problem hiding this comment.
that feels like a lower risk then them wondering why their cert doesn't work, tbh. The fact that the whole thing is disabled by default is enough of a safetynet imho.
richvdh
left a comment
There was a problem hiding this comment.
lgtm otherwise, modulo @michaelkaye 's comments
Co-Authored-By: hawkowl <hawkowl@atleastfornow.net>
…/matrix-org/synapse into hawkowl/acme-portable-certificates
No description provided.