aarongable / draft-acme-ari

Internet Draft for the Automated Certificate Management Environment (ACME) Renewal Information (ARI) Extension
Other
5 stars 7 forks source link

Support `replaces` without `renewal-info` resource #72

Closed Radranic closed 2 months ago

Radranic commented 2 months ago

From a reading of the draft there is no explicit indication that a server supports the replaces tag on new orders.

I've been told that it should be related to the renewal-info resource, but from what I read of the draft that relation is not explicit.

I also know this isn't just my understanding of the issue as at least one client (https://github.com/rmbolger/Posh-ACME/releases/tag/v4.24.0) that has added support for ARI is sending the replaces value to services without the renewal-info field.

While a solution to this misunderstand could be to make the relation more explicit, I would prefer if the support for the replaces field could be advertised independently so that a server could choose to support this without explicitly needing to have a renewal-info endpoint if the server doesn't care when a renewal happens.

rmbolger commented 2 months ago

(https://github.com/rmbolger/Posh-ACME/releases/tag/v4.24.0) that has added support for ARI is sending the replaces value to services without the renewal-info field.

Posh-ACME has indeed added support for ARI, but it shouldn't be sending the replaces value for new orders unless the server's directory endpoint has the renewalInfo endpoint.

https://github.com/rmbolger/Posh-ACME/blob/6b0b3c1d584d0551ce724c2eba23f66d3476c539/Posh-ACME/Public/New-PAOrder.ps1#L201-L205

Radranic commented 2 months ago

Is that a confirmation that there is no intent to be able to support the replaces value without including the renewalInfo endpoint?

rmbolger commented 2 months ago

I would think that any ACME CA supporting the replaces value would implicitly be supporting the renewalInfo field since they're ultimately both part of the ARI spec. I don't see how else a client would know it was supported.

mholt commented 2 months ago

Also, when replaces is set, the server SHOULD do some strict verifications to make sure the account is authorized to indicate a certificate is replaced, etc. So it (likely) follows that it should not be set if ARI is not supported at all. Setting it when it is not intended is grounds for errors.

Radranic commented 2 months ago

I would think that any ACME CA supporting the replaces value would implicitly be supporting the renewalInfo field since they're ultimately both part of the ARI spec. I don't see how else a client would know it was supported.

I am asking for this to be added. A way to indicate replaces is supported without needing renewalInfo.

Also, when replaces is set, the server SHOULD do some strict verifications to make sure the account is authorized to indicate a certificate is replaced, etc. So it (likely) follows that it should not be set if ARI is not supported at all. Setting it when it is not intended is grounds for errors.

I have never disagreed with the validation requirements.

I am asking why both features of ARI are bound into the same setting? Why can they not be both separate features? Or maybe at least a way in the renewalInfo to indicate "this server does not care when you renew your cert".

The way ARI is now, a server can't make use of order tracking via the replace without having to have some kind of logic to decide what "good" renewal info times are, even if the server is under no load constraints.

Maybe add a field to the meta section of the directory similar to the externalAccountRequired. If it does not exist, replace is not supported, if it does exist and is true, then orders can be replaced.

aarongable commented 2 months ago

Fundamentally, the purpose of this draft is not to enable order tracking, it's to enable rapid replacement of certificates. If someone is considering implementing just the "replaces" field of this spec without implementing the renewal-info endpoint, I would encourage them to instead implement both.

At this time I do not intend to create a second separate indicator for whether or not the server supports the "replaces" field.

I will update the document to indicate that clients SHOULD NOT include the "replaces" field in new-order requests if the server does not advertise a renewal-info endpoint in its directory.

Radranic commented 2 months ago

I get how the renewalInfo aims for those goals, but it's unfortunate that a useful feature such as replaces can't be used explicitly independently.

From reading it I guess my only solution is to intentionally make an invalid renewalInfo or suggestedWindow inside it and hope that clients follow the "SHOULD make its own determination".

I don't think Posh-ACME does proper checking on the suggested time in case the end is before the start, so an invalid response is probably the better bet.

Thank you for the conversation.

rmbolger commented 2 months ago

@Radranic If you think there's a problem with Posh-ACME's ARI implementation that should be fixed, feel free to create an issue over there. But we shouldn't clutter the RFC draft repo with that discussion.

Radranic commented 2 months ago

@rmbolger, I don't have any problem with the implementation. I was just mentioning that an example current implementation doesn't do something in that is marked as SHOULD for ARI, which limits ways to get around the "must have renewalInfo" requirement.

I suspect a server that is intentionally trying to trigger failure case behavior of the RFC draft to trigger specific behaviors of a client isn't something that anyone here wants to specifically fix, so I'm not worried about the behavior.