From 48a385fcc4ec9f98cbc10ed4e5b68322ddc79fd6 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Tue, 28 Nov 2023 23:48:29 +1000 Subject: [PATCH 1/2] acme: return proper error on malformed account payload ACMEAccountService currently throws an uncaught exception if decode the account object payload fails. This results in the server responding 500 Internal Server Error. Respond with status 400 and a proper problem document instead. --- .../dogtagpki/acme/server/ACMEAccountService.java | 9 ++++++++- .../java/org/dogtagpki/acme/server/ACMEEngine.java | 12 ++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEAccountService.java b/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEAccountService.java index b4d43fcf567..38db0b2c3dc 100644 --- a/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEAccountService.java +++ b/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEAccountService.java @@ -17,6 +17,8 @@ import javax.ws.rs.core.Response.ResponseBuilder; import javax.ws.rs.core.UriInfo; +import com.fasterxml.jackson.core.JsonProcessingException; + import org.dogtagpki.acme.ACMEAccount; import org.dogtagpki.acme.ACMEHeader; import org.dogtagpki.acme.ACMENonce; @@ -74,7 +76,12 @@ public Response updateAccount(@PathParam("id") String accountID, JWS jws) throws String payload = new String(jws.getPayloadAsBytes(), "UTF-8"); logger.info("Payload: " + payload); - ACMEAccount update = ACMEAccount.fromJSON(payload); + ACMEAccount update; + try { + update = ACMEAccount.fromJSON(payload); + } catch (JsonProcessingException e) { + throw engine.createMalformedException(e.toString()); + } String newStatus = update.getStatus(); if (newStatus != null) { diff --git a/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEEngine.java b/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEEngine.java index 938c9b76f47..92a8baa99a5 100644 --- a/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEEngine.java +++ b/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEEngine.java @@ -728,6 +728,18 @@ public Exception createAccountDoesNotExistException(String accountID) { return new WebApplicationException(builder.build()); } + public Exception createMalformedException(String desc) { + ResponseBuilder builder = Response.status(Response.Status.BAD_REQUEST); + builder.type("application/problem+json"); + + ACMEError error = new ACMEError(); + error.setType("urn:ietf:params:acme:error:malformed"); + error.setDetail("Malformed request: " + desc); + builder.entity(error); + + return new WebApplicationException(builder.build()); + } + public void updateAccount(ACMEAccount account) throws Exception { database.updateAccount(account); } From ed3127762647d4960632380b6a1f6db87456c660 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Wed, 29 Nov 2023 00:09:06 +1000 Subject: [PATCH 2/2] acme: implement POST-as-GET for accounts Some ACME clients POST-as-GET the account resource, expecting to receive the account object (for an existing account). In particular, mod_md does this and certificate renewal fails when it cannot read or verify the account information. The ACME protocol does not explicitly require this behaviour. But on the other hand, it is not surprising that clients assume they can do it, and it arguably is surprising if an ACME server does not provide it. So let's implement it. The change itself is trivial: when payload is empty, POST-as-GET is implied (RFC 8555 section 6.3). In this case, return the ACMEAccount object (which we already have at hand) unchanged. --- .../acme/server/ACMEAccountService.java | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEAccountService.java b/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEAccountService.java index 38db0b2c3dc..8c9f704cf5e 100644 --- a/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEAccountService.java +++ b/base/acme/src/main/java/org/dogtagpki/acme/server/ACMEAccountService.java @@ -74,33 +74,40 @@ public Response updateAccount(@PathParam("id") String accountID, JWS jws) throws engine.validateJWS(jws, header.getAlg(), account.getJWK()); String payload = new String(jws.getPayloadAsBytes(), "UTF-8"); - logger.info("Payload: " + payload); - ACMEAccount update; - try { - update = ACMEAccount.fromJSON(payload); - } catch (JsonProcessingException e) { - throw engine.createMalformedException(e.toString()); + if (payload.isEmpty()) { + logger.info("Empty payload; treating as POST-as-GET"); } - String newStatus = update.getStatus(); - if (newStatus != null) { - logger.info("New status: " + newStatus); - account.setStatus(newStatus); - } + else { + logger.info("Payload: " + payload); - String[] newContact = update.getContact(); - if (newContact != null) { - logger.info("New contact:"); - for (String c : newContact) { - logger.info("- " + c); + ACMEAccount update; + try { + update = ACMEAccount.fromJSON(payload); + } catch (JsonProcessingException e) { + throw engine.createMalformedException(e.toString()); + } + + String newStatus = update.getStatus(); + if (newStatus != null) { + logger.info("New status: " + newStatus); + account.setStatus(newStatus); + } + + String[] newContact = update.getContact(); + if (newContact != null) { + logger.info("New contact:"); + for (String c : newContact) { + logger.info("- " + c); + } + account.setContact(newContact); } - account.setContact(newContact); - } - engine.updateAccount(account); + engine.updateAccount(account); - // TODO: if account is deactivated, cancel all account's pending operations + // TODO: if account is deactivated, cancel all account's pending operations + } // RFC 8555 Section 7.1.2.1 Orders List //