-
Notifications
You must be signed in to change notification settings - Fork 18k
Proposal: implementation strategy for ACME RFC8555 in x/crypto/acme #33229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Why not both? In the go mod world, it's been formalized that if you make an incompatible revision of a library, it has to live at This related to #21324 - with go mod support becoming mainstream in 1.13, I think it's valuable for the |
Another trouble with /v1 and /v2 I forgot to mention is autocert: it would need to import both and use one or the other based on the ACME version. It complicates autocert code too much imho. Alternatively, you'd need to make separate /v2/autocert. Even more maintenance hell. |
Regarding this con in the first plan:
has there been positive confirmation that any of these CA's implementation of "ACME v1" function with the existing codebase? I don't think it can be taken on faith or based on limited public documentation. There's no specification for ACME v1 and no single draft that can be identified as a closest match. I don't think losing compatibility with these CAs should be considered as a con unless it can be confirmed they were compatible to begin with. |
I just tried now with the first in the list using a small cmd line tool which calls
Can try with some others but it's missing the point a bit. The list I composed is just a sample. Plus, some private CAs like I mentioned. |
@x1ddos: Why is it missing the point? Your con says:
I think establishing whether it was compatible or not to begin with is important in understanding the cost of breaking the compatibility. Since there is no specification its very difficult to ascertain compatibility without experimentation. Entrust's documentation specifically assumes Certbot, and I suspect other pre-RFC 8555 CAs used the "it works with Certbot so it's ACME v1 compatible" approach to interop. I'm convinced that Buypass' pre-RFC 8555 ACME endpoint is compatible but are the others? Breaking compatibility with 1 CA is a different level of compromise than breaking compatibility with 5+. |
|
Indeed. So the compatibility being lost is primarily hypothetical 👍
Perhaps, but like with HTTP for RFC 8555 there is standards text that can be used to mediate compliance discussions as a point of common reference. There is no such thing for ACME v1. There is no way to determine if a change requested by a server operator for compliance is a one-off request for a specific implementation, or a deviation from a standard that would affect multiple implementations without experimentation.
It seems like we're in agreement then: to know how to evaluate the hacks required means needing to understand the CAs that are compatible with this package now, and how many will be compatible based on proposed changes to the package. |
@cpu this is a nice theory. Unfortunately, practice tells me a different story which is why I expressed my preference towards option 2. |
@cpu I think the point here is not to say that the library shouldn't upgrade to ACMEv2, it is to say, should it do so in a way that breaks people with no notice. I would go so far to say if a package broke me with no notice on a major thing like this I would have trust issues with the package. With that in mind, it does not seem unreasonable to me to try to maintain backward compatibility with the current behavior for some period of time. To be clear personally, I do not like the idea of a merged implementation as is represented in option #2. With that said, it does sound like it doesn't break anyone (which I think is important) and gets Let's Encrypt ACMEv2 in the library in time for their migration from their "past ACME implementation" to the 8555 compatible one. I would like to recommend that if that approach is taken it comes with a statement that the prior implementation will be dropped at some specific and notified point of time so a cleaner and more maintainable library is in place long term. |
In Go, there is always an implicit compatibility target, which is what currently works. I agree that there is no such a thing as ACMEv1 to target, so now that there is a specified ACME protocol we shouldn't spend any more effort chasing compatibility with pre-RFC CAs. But that's different from changing behavior in such a way that breaks what used to work before the change. As reflected by both the the Go 1 Compatibility Promise and by modules' semantic versioning (although this package is not bound by either), the ecosystem really values this kind of stability. That doesn't mean we need to keep supporting "ACMEv1", but precisely the servers that already work, because that's what users will have tested and developed an expectation for. Considering this, option 2 here is generally preferable, if the API complexity cost is moderate. It looks like there wouldn't be more than a couple of v1-only and v2-only methods, which looks acceptable. I like the Discover based autodetection. As an implementation strategy, I'd like to see v2 code in separate files as much as possible, even if it means more code duplication. That should make it easier to rip out v1 eventually, and will ensure the v2 implementation is good enough to stand on its own. |
Ah, about using modules versioning for this: there are two ways to do it, either bumping all of x/crypto to v2, causing an import path change that is not necessary for most packages, or splitting x/crypto/acme into its own module, and then bumping that. Nested modules are a lot of complexity, they carve a hole in the parent module, and interact weirdly with it. To maintain compatibility we'd probably have to keep the current acme package in x/crypto in its current state, but we'd probably not want to have two different autocert versions, and I am not sure we can redirect from one to the other. I also worry that v2 would be less discoverable than v0 because the latter would be what projects already have in their go.mod. It's a judgement call, but I think this complexity is more than that of supporting both protocols in the same package. |
Just chiming in that we are starting to see failures from this now: This message https://community.letsencrypt.org/t/important-notice-to-acme-client-developers-regarding-acme-v1-deprecation/100795 points me to a list of platforms that support this new protocol here https://github.com/letsencrypt/website/blob/master/data/clients.json and autocert seems to be the only golang option and it does not support ACMEv2. If we are starting to see rolling outages in prod it would be nice to have this fixed ASAP (or at least have some supported alternative for the time being). |
Ah just saw golang/crypto@0e8c3a9 thanks for that! We will rebuild our binaries at that commit. Thanks for the prompt fixes. Maybe this issue can be closed? |
This got implemented. Closing. 🙌 |
The goal of this proposal is to agree on the implementation details of RFC8555 in #21081.
The acme package currently implements an approximation of draft-02, aka "ACMEv1 API".
The RFC8555 is the final version of the ACME protocol, aka "ACMEv2 API", based on draft-18. Let's Encrypt announced End of Life of their ACMEv1 in https://community.letsencrypt.org/t/end-of-life-plan-for-acmev1/88430 where first deadline is Nov 2019 past which no new account registrations will be allowed.
Note that we have 2 packages in scope:
While this issue is mostly concerned with the
acme
package,autocert
package will also be impacted in different ways based on this proposal resolution although it should be a relatively easy set of changes since most of its functionality is automated and unexported.There are currently two conflicting proposals. I will list pros & cons for each one as I see them.
1. Replace
golang.org/x/crypto/acme
with a strict RFC8555 implementation, essentially removing all remnants of draft-02.An example of this is https://golang.org/cl/86635.
Pros:
to the current acme package and split up in smaller CLs to make it possible
for a reasonable human review
to possibly improve although I don't have concrete shiny examples
Cons:
implemented RFC8555 yet; at the time of writing:
2. Add RFC855 support to the existing acme package without breaking its users and their issuance using ACME CAs
The major exported changes would be the following:
acme.Client
methodsCreateOrder
andFinalizeOrder
to implement order-based issuance flow which is preferred in the RFC, as opposed to the pre-authorization flow using the existingAuthorize
method; the pre-authorization flow is optional in RFC and in fact Let's Encrypt does not provide it in their ACMEv2 but some other CAs doOrder
to accomodate the new order-based issuance flow and new fields in the existing exported structs such as those in acme.Directory in https://go-review.googlesource.com/c/crypto/+/182937/2/acme/types.go; I expect them to have valid zero values or be populated by the acme client accordinglykid
andurl
fields; although most changesshould be transparent, they may require some new
acme.Client
methods to accommodate the new JWS/JWK formatsI do not expect for the existing exported methods of acme.Client to be removed or their signatures changed. Their implementation will branch according to the endpoint in use: ACMEv1 or ACMEv2. This is possible because the Client performs directory fetch automatically upon first network round-trip for any client call whenever required using its Discover method. It can thus identify which version of ACME protocol is in use at the right time.
Later on, we can amend the acme client and remove ACMEv1 support as more CAs catch up with the RFC compliance.
Pros:
Cons
which most likely means more complicated code until the support for non-compliant CAs
is dropped completely
A non-exhaustive list of alternatives I've been thinking of:
acme changes alone justify this.
as before: feels like a step backwards considering go mod.
who's still stuck with non-RFC compliant CAs still run. This fork will unlikely get future
improvements unless someone will have time to backport the changes.
Personally, I'm in favour of (2), i.e. add RFC support to the existing acme client and keep it working with all major ACME CAs out there until reasonably possible without too much of an overhead.
The text was updated successfully, but these errors were encountered: