Remove keyAuthorization field from the challenge response JWS token

[adferrand]

Fixes #6755.

POSTing the keyAuthorization in a JWS token when answering an ACME challenge, has been deprecated for some time now. Indeed, this is superfluous as the request is already authentified by the JWS signature.

Boulder still accepts to see this field in the JWS token, and ignore it. Pebble in non strict mode also. But Pebble in strict mode refuses the request, to prepare complete removal of this field in ACME v2.

Certbot still sends the keyAuthorization field. This PR removes it, and makes Certbot compliant with current ACME v2 protocol, and so Pebble in strict mode.

The implementation chosen here is done to reduce at most the surface of the API change, to mitigate risk of breakage in third-party applications that rely on acme. Indeed, the objects themselves see their API untouched: only the JSON serialization, done to build the JWS token, filter out the keyAuthorization field.

See also letsencrypt/pebble#192 for implementation details server side.

[bmw]

Doesn't this change also affect ACMEv1?

There are other ACME CAs out there using Certbot and I worry that this change might break things for them. Is there a way we can only make this change for ACMEv2? I'm happy to brainstorm with you on how to do this if you'd like.

[adferrand]

I choose this implementation because it has a very low impact on API, while making in sync the KeyAuthorizationResponseBody in sync with the ACME spec, that avoids also to leak the keyAuthorization accidentally in the log, in case of JSON dump, similarly to what is done on Boulder and Pebble.

However, if you have concerns on the retro-compatibility (but think on it twice in term of the impact on a deprecated protocol vs security related improvement), I have another proposition with API low impact.

It is to do like POST-as-GET, with dynamic fallback to older formats of the spec. From the lowest level of POST implementation in acme (both for V1 and V2):

• check if the body to be dumped is of type KeyAuthorizationResponseBody
• if so, try first with dump expurged of keyAuthorization
• expect a malformed error
• retry with dump including keyAuthorization
• die for any further error

For other bodies, normal process. And in any circumstances, do not log the keyAuthorization from the body dump (either because it is not present, or it is edited if so).

Formely, the spec says to send an empty {}, which is not the case here (we have still the others fields, like resource). But sending an empty JSON is refused by Boulder v1 mode, while juste removing keyAuthorization is accepted.

If it becomes a problem on Boulder V2, we could pass to a three steps try, instead of two (empty, keyAuthorization expurged, with keyAuthorization, but hopefully at this time the ACME v1 will be totally removed, and we will not care about it anymore.

[adferrand]

However again on the security related issue, this process exposes Certbot to leak the keyAuthorization if by MITM attack it receives a malformed request error.

So considering that:

• the deprecation is one year old, like TLS-SNI-01
• TLS-SNI-01 removal next month will nonetheless force the migration to a recent ecosystem for acme clients and servers
• most used server (Boulder) accepts a request without keyAuthorization for both v1 and v2 protocol
• all of this is about mitigating security issues

I will still strongly militate for a clean removal without fallback.

[cpu]

There are other ACME CAs out there using Certbot and I worry that this change might break things for them.

There's no spec for "ACME v1", just a reference implementation (Boulder). I'm not sure when in the history of Boulder we removed processing a keyAuthorization in a POST body to a challenge but it has been at least 2 years.

Personally I think anyone that implemented an "ACME v1" server made a poor choice and there are two solutions available to them that wouldn't be a burden on Certbot: they could implement a protocol with a specification (ACMEv2), or they could invest the engineering effort required to track and adopt Boulder's changes as they happen.

Server operators that do neither of these things are guaranteed to have endless compatibility problems and I would hate to see the client ecosystem encourage more "ACME v1" development by working too hard to remain compatible.

[bmw]

@adferrand, you wrote a lot about your concerns about leaking the key authorization. What is the attack here?

Rather than falling back, I think my personally preferred behavior is to change Certbot's behavior based on the ACME version as suggested by jsha at #6755 (comment). I'd argue that "ACME v1" (however you want to define it) requires the keyAuthorzation while including for "ACME v2" is wrong.

@cpu,

There's no spec for "ACME v1", just a reference implementation (Boulder).

Right, but as you know, there has been a lot of client software implemented against this reference implementation. The other ACME CAs listed at https://en.wikipedia.org/wiki/Automated_Certificate_Management_Environment#CAs_&_PKIs_that_offer_ACME_certificates whose directory endpoint I've been able to query implement "ACME v1".

I'm not sure when in the history of Boulder we removed processing a keyAuthorization in a POST body to a challenge but it has been at least 2 years.

If that's the case, simply suddenly changing the behavior is honestly tempting to me, however, this does not seem to be documented at https://github.com/letsencrypt/boulder/blob/master/docs/acme-divergences.md so I wouldn't expect CAs other than boulder who implemented "ACME v1" to know this.

Personally I think anyone that implemented an "ACME v1" server made a poor choice and there are two solutions available to them that wouldn't be a burden on Certbot: they could implement a protocol with a specification (ACMEv2), or they could invest the engineering effort required to track and adopt Boulder's changes as they happen.

I agree this would be great but I don't think this happening.

Server operators that do neither of these things are guaranteed to have endless compatibility problems and I would hate to see the client ecosystem encourage more "ACME v1" development by working too hard to remain compatible.

I think this is likely particularly painful for Certbot. For better or worse, many CAs recommend people use Certbot as well. I've heard stories about ACME servers being implemented with the goal being that "it works with Certbot".

With that said, I personally think we're making progress here. We've been talking about deprecating and then removing our support for "ACME v1" to strongly discourage CAs from continuing to rely on this dated "spec".

[adferrand]

@bmw I wrote about security, because it seems to be an initial concern to remove it server side, but I may be wrong. Maybe @jsha or @cpu could confirm or not that it is the case.

For the other proposition I made, I prefer it over to decide depending on the acme version, because Boulder seems to support it with acme v1, so we will end up to remove this deprecated key most of the time, for any acme version used.

[adferrand]

On my previous post, for the concerns, I was thinking of this: letsencrypt/boulder#3526, that prevents to display the keyAuthorization value in the logs.

[jsha]

I haven't followed this whole thread, but it sounds like you're asking: Is the keyAuthorization a confidential value? If so, no it's not. The removal in letsencrypt/boulder#3526 was just to get closer to matching the spec, not to avoid writing anything to logs.

[adferrand]

OK, then the fallback is reasonable, and not insecure. I will do that. Are we agree on my approach, to rely on the malformed error instead of the acme version used? It will have also the benefit to cover servers that do not implement a recent draft of the acme v2 draft.

Once it becomes a RFC, we should remove the fallback.

[cpu]

The other ACME CAs listed at https://en.wikipedia.org/wiki/Automated_Certificate_Management_Environment#CAs_&_PKIs_that_offer_ACME_certificates whose directory endpoint I've been able to query implement "ACME v1".

I don't think these other CAs implement ACME. They implement something unspecified.

If that's the case, simply suddenly changing the behavior is honestly tempting to me, however, this does not seem to be documented at https://github.com/letsencrypt/boulder/blob/master/docs/acme-divergences.md so I wouldn't expect CAs other than boulder who implemented "ACME v1" to know this.

This isn't a specification and shouldn't be used by other CAs in place of one - it isn't intended for server developers. Arguably it isn't a divergence because we won't error if you do POST the keyAuthorization, its just ignored. What I'm trying to say is that if another CA implemented "ACME v1" then the way they are supposed to know about these changes is to closely follow Boulder/Certbot development. That's the trade-off of choosing ACME v1.

I've heard stories about ACME servers being implemented with the goal being that "it works with Certbot".

Right, and this is a chance to indicate that's not a strategy the team supports. It's your call if you do want to send a message that this is an acceptable way to implement ACME but I'm trying to convince you that it's a bad idea for the long term health of the ecosystem. One of the major goals of ACME is to promote a vendor neutral protocol that would let users switch server providers and clients without risking compatibility. Ossifying on what Certbot supported at a specific point in time for an unspecified protocol is the opposite of this.

We've been talking about deprecating and then removing our support for "ACME v1" to strongly discourage CAs from continuing to rely on this dated "spec".

I'm a big +1 for this because again, there is no spec! There's not even a single draft # that can be pointed to that approximates ACMEv1. It's a reference implementation spanning multiple drafts as a product of the co-evolution that occurred.

It sounds like I'm in the minority here so I won't keep beating on this dead horse. If you want to maintain the keyAuthorization challenge POST that's your call. Thanks for engaging with me on the discussion!

[adferrand]

@bmw you have heard of other ACME CA servers to develop by following the Certbot implementation. @cpu you are talking about not giving a bad signal to developer about maintaining deprecated and unspecified features, and also state that implementing ACME "v1" is in fact following Boulder/Certbot development.

As it is not a security update, there is no urge to process immediately. But it is clearly an occasion to be proactive and engage a migration path to incite other ACME CA server maintainers to clean their implementation.

Instead of waiting official RFC for ACME, or support forever keyAuthorization in the fear of breaking stuff, could we implement my proposition, advert in changelog that keyAuthorization is deprecated and should not be used anymore, then completely remove it in N+2 releases of Certbot ?

[bmw]

Thanks for engaging with me on the discussion!

I'm happy to respond to some/all of what you wrote or answer any questions if you'd like to talk about this more @cpu. I largely agree with you with my main hesitation here being suddenly making these changes without sufficient notice.

@adferrand, your approach sounds good to me.

[cpu]

@bmw If everyone is on board with @adferrand's plan I don't think we need to talk more. It sounds like a deprecation notice and delaying the removal a release or two would address your hesitation about a sudden change/sufficient notice and also result in it being removed in the near future so I'm 👍

[adferrand]

@bmw I implemented the new behavior with fallback. Changelog is also updated.

[bmw]

LGTM!