Skip to content
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

Fix race condition during ACME order finalization #4562

Merged
merged 2 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,6 @@ public void finalizeValidAuthorization() throws Exception {
Date expirationTime = engine.getPolicy().getValidAuthorizationExpirationTime(currentTime);
authorization.setExpirationTime(expirationTime);

engine.updateAuthorization(account, authorization);

logger.info("Updating pending orders");

Collection<ACMEOrder> orders =
Expand All @@ -129,6 +127,10 @@ public void finalizeValidAuthorization() throws Exception {
boolean allAuthorizationsValid = true;

for (String orderAuthzID : order.getAuthzIDs()) {
if (orderAuthzID.equals(authzID)) {
// We're about to set it to valid, so treat it as such
continue;
}

ACMEAuthorization authz = engine.getAuthorization(account, orderAuthzID);
if (authz.getStatus().equals("valid")) continue;
Expand All @@ -147,6 +149,13 @@ public void finalizeValidAuthorization() throws Exception {

engine.updateOrder(account, order);
}

// We defer the LDAP update until AFTER any order transitions
// occur. This avoids a race condition where clients that eagerly
// proceed to finalization when all Authorizations are "valid"
// experience finalization failure, because the __Order__ has not yet
// transition to "ready".
engine.updateAuthorization(account, authorization);
}

public void finalizeInvalidAuthorization(ACMEError err) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.ResponseBuilder;
import javax.ws.rs.core.UriInfo;

import org.dogtagpki.acme.ACMEAccount;
import org.dogtagpki.acme.ACMEError;
import org.dogtagpki.acme.ACMEHeader;
import org.dogtagpki.acme.ACMENonce;
import org.dogtagpki.acme.ACMEOrder;
Expand Down Expand Up @@ -66,8 +68,25 @@ public Response handlePOST(@PathParam("id") String orderID, JWS jws) throws Exce
ACMEOrder order = engine.getOrder(account, orderID);

if (!order.getStatus().equals("ready")) {
// TODO: generate proper exception
throw new Exception("Order not ready: " + orderID);

// RFC 8555 Section 7.4: Applying for Certificate Issuance
//
// A request to finalize an order will result in error if the order is
// not in the "ready" state. In such cases, the server MUST return a
// 403 (Forbidden) error with a problem document of type
// "orderNotReady". The client should then send a POST-as-GET request
// to the order resource to obtain its current state. The status of the
// order will indicate what action the client should take (see below).

ResponseBuilder builder = Response.status(Response.Status.FORBIDDEN);
builder.type("application/problem+json");

ACMEError error = new ACMEError();
error.setType("urn:ietf:params:acme:error:orderNotReady");
error.setDetail("Order not ready: " + orderID);
builder.entity(error);

throw new WebApplicationException(builder.build());
}

order.setStatus("processing");
Expand Down
Loading