Skip to content

Commit

Permalink
Fix race condition during ACME authz polling
Browse files Browse the repository at this point in the history
Previously after creating an ACME order the client would call
ACMEAuthorizationService to poll the status of the authorization.
Initially the authorization did not have any challenges, so this
service would create the challenges for it. In subsequent calls
this service would just return the status of the authorization.

When the client completes a challenge, the ACMEChallengeProcessor
will update the authorization by removing the old challenges and
adding the new ones. Since these operations are not atomic there
is a risk that after the old challenges are removed the client
will call the ACMEAuthorizationService and create new challenges
which will never be completed by the client.

To avoid the problem, the code that creates the challenges has
been moved from ACMEAuthorizationService into ACMENewOrderService
so the challenges can only be created just once when the order is
initially created.

The LDAPDatabase.addAuthorization() has also been updated to add
the challenges after adding the authorization.
  • Loading branch information
edewata committed Sep 14, 2023
1 parent 02148f2 commit ee746e2
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,10 @@ public void addAuthorization(ACMEAuthorization authorization) throws Exception {
+ "," + RDN_AUTHORIZATION + "," + baseDN;
LDAPEntry entry = new LDAPEntry(dn, attrSet);
ldapAdd(entry);

for (ACMEChallenge challenge : authorization.getChallenges()) {
addChallenge(authorization.getAccountID(), challenge);
}
}

public void addChallenge(String accountID, ACMEChallenge challenge)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
package org.dogtagpki.acme.server;

import java.net.URI;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Collection;

import javax.ws.rs.POST;
Expand All @@ -20,14 +18,12 @@
import javax.ws.rs.core.Response.ResponseBuilder;
import javax.ws.rs.core.UriInfo;

import org.apache.commons.codec.binary.Base64;
import org.dogtagpki.acme.ACMEAccount;
import org.dogtagpki.acme.ACMEAuthorization;
import org.dogtagpki.acme.ACMEChallenge;
import org.dogtagpki.acme.ACMEHeader;
import org.dogtagpki.acme.ACMENonce;
import org.dogtagpki.acme.JWS;
import org.dogtagpki.acme.validator.ACMEValidator;

/**
* @author Endi S. Dewata
Expand Down Expand Up @@ -69,30 +65,8 @@ public Response handlePOST(@PathParam("id") String authzID, JWS jws) throws Exce
String authorizationStatus = authorization.getStatus();
logger.info("Authorization status: " + authorizationStatus);

// generate 128-bit token with JSS
// TODO: make it configurable

byte[] bytes = new byte[16];
SecureRandom random = SecureRandom.getInstance("pkcs11prng", "Mozilla-JSS");
random.nextBytes(bytes);
String token = Base64.encodeBase64URLSafeString(bytes);
logger.info("Token: " + token);

Collection<ACMEChallenge> challenges = authorization.getChallenges();

if (challenges == null || challenges.size() <= 0) {
logger.info("Creating new challenges");
challenges = new ArrayList<>();

for (ACMEValidator validator : engine.getValidators()) {
ACMEChallenge challenge = validator.createChallenge(authzID, token);
challenges.add(challenge);
}

authorization.setChallenges(challenges);
engine.updateAuthorization(account, authorization);
}

logger.info("Challenges:");
for (ACMEChallenge challenge : challenges) {
logger.info("- " + challenge.getType() + ": " + challenge.getStatus());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
package org.dogtagpki.acme.server;

import java.net.URI;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Date;

import javax.ws.rs.POST;
Expand All @@ -18,14 +20,17 @@
import javax.ws.rs.core.Response.ResponseBuilder;
import javax.ws.rs.core.UriInfo;

import org.apache.commons.codec.binary.Base64;
import org.dogtagpki.acme.ACMEAccount;
import org.dogtagpki.acme.ACMEAuthorization;
import org.dogtagpki.acme.ACMEChallenge;
import org.dogtagpki.acme.ACMEHeader;
import org.dogtagpki.acme.ACMEIdentifier;
import org.dogtagpki.acme.ACMENonce;
import org.dogtagpki.acme.ACMEOrder;
import org.dogtagpki.acme.JWS;
import org.dogtagpki.acme.ValidationResult;
import org.dogtagpki.acme.validator.ACMEValidator;

/**
* @author Endi S. Dewata
Expand Down Expand Up @@ -67,6 +72,15 @@ public Response createNewOrder(JWS jws) throws Exception {
ACMEOrder request = ACMEOrder.fromJSON(payload);
ArrayList<String> authzIDs = new ArrayList<>();

// generate 128-bit token for authorization challenges
// TODO: make it configurable

byte[] bytes = new byte[16];
SecureRandom random = SecureRandom.getInstance("pkcs11prng", "Mozilla-JSS");
random.nextBytes(bytes);
String token = Base64.encodeBase64URLSafeString(bytes);
logger.info("Token: " + token);

logger.info("Generating authorization for each identifiers");
for (ACMEIdentifier identifier : request.getIdentifiers()) {

Expand Down Expand Up @@ -114,6 +128,16 @@ public Response createNewOrder(JWS jws) throws Exception {
authorization.setIdentifier(identifier);
authorization.setWildcard(wildcard);

Collection<ACMEChallenge> challenges = new ArrayList<>();

for (ACMEValidator validator : engine.getValidators()) {
ACMEChallenge challenge = validator.createChallenge(authzID, token);
logger.info(" - challenge ID: " + challenge.getID());
challenges.add(challenge);
}

authorization.setChallenges(challenges);

authorization.setStatus("pending");

Date expirationTime = engine.getPolicy().getPendingAuthorizationExpirationTime(currentTime);
Expand Down

0 comments on commit ee746e2

Please sign in to comment.