This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch 4.1.x-fixes in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 71bc5cda37be17b54b234818cbf816586216cb8b Author: Naveed Khan <[email protected]> AuthorDate: Thu Oct 29 14:23:07 2026 +0530 require a signed digest for any http signature message body (#3632) * require a signed digest for any http signature message body * only skip the digest requirement for gets and error responses without a body * pass a null body in the default required header not present tests (cherry picked from commit 23e0bc0c34497473394b9220f77d5ec4def45d85) --- .../rs/security/httpsignature/MessageVerifier.java | 8 +- .../filters/AbstractSignatureInFilter.java | 14 +- .../httpsignature/MessageVerifierTest.java | 8 +- .../security/httpsignature/SpecExamplesTest.java | 176 +++++++++++++++++++++ 4 files changed, 193 insertions(+), 13 deletions(-) diff --git a/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/MessageVerifier.java b/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/MessageVerifier.java index c07381047dd..0402c81602d 100644 --- a/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/MessageVerifier.java +++ b/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/MessageVerifier.java @@ -134,10 +134,12 @@ public class MessageVerifier { // Now dynamically check to see whether the "digest" header is required or not. It's not required // for the service case on a GET/HEAD request as there is no request, and also not required on - // the client side unless it is an OK response that is not 204 + // the client side unless it is an OK response that is not 204. It's still required in these cases + // if there is a message body Integer status = (Integer)m.get(Message.RESPONSE_CODE); - if ((requestor && (status < 200 || status >= 300 || status == 204)) - || (!requestor && ("GET".equalsIgnoreCase(method) || "HEAD".equalsIgnoreCase(method)))) { + boolean noBodyExpected = (requestor && (status < 200 || status >= 300 || status == 204)) + || (!requestor && ("GET".equalsIgnoreCase(method) || "HEAD".equalsIgnoreCase(method))); + if (noBodyExpected && (messageBody == null || messageBody.length == 0)) { signedHeaders.remove("digest"); } diff --git a/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/AbstractSignatureInFilter.java b/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/AbstractSignatureInFilter.java index 597759eed39..491eaa9543f 100644 --- a/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/AbstractSignatureInFilter.java +++ b/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/AbstractSignatureInFilter.java @@ -70,16 +70,18 @@ abstract class AbstractSignatureInFilter { return messageBody; } - // Verify digest if we have the appropriate header and a non-empty request body. Note that it is up to - // the MessageVerifier configuration, or the HTTPSignatureConstants.RSSEC_HTTP_SIGNATURE_IN_HEADERS - // configuration to require that the digest is signed (and hence present) - if (entityStream != null && headers.containsKey("Digest")) { - LOG.fine("Digesting message body"); + // Always read the body so the MessageVerifier can require a signed digest when one is present + if (entityStream != null) { try { messageBody = IOUtils.readBytesFromStream(entityStream); } catch (IOException e) { - throw new DigestFailureException("failed to validate the digest", e); + throw new DigestFailureException("failed to read the message body", e); } + } + + // Verify digest if we have the appropriate header + if (messageBody != null && headers.containsKey("Digest")) { + LOG.fine("Digesting message body"); DigestVerifier digestVerifier = new DigestVerifier(); try { diff --git a/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/MessageVerifierTest.java b/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/MessageVerifierTest.java index 876df0681ad..002000f5caf 100644 --- a/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/MessageVerifierTest.java +++ b/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/MessageVerifierTest.java @@ -230,7 +230,7 @@ public class MessageVerifierTest { Collections.singletonList("test")); headerVerifier.setSecurityProvider(new MockSecurityProvider()); headerVerifier.setAlgorithmProvider(new MockAlgorithmProvider()); - headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), MESSAGE_BODY.getBytes()); + headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), null); } @Test @@ -243,7 +243,7 @@ public class MessageVerifierTest { MessageVerifier headerVerifier = new MessageVerifier(keyId -> keyPair.getPublic()); headerVerifier.setSecurityProvider(new MockSecurityProvider()); headerVerifier.setAlgorithmProvider(new MockAlgorithmProvider()); - headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), MESSAGE_BODY.getBytes()); + headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), null); } @Test @@ -270,7 +270,7 @@ public class MessageVerifierTest { Collections.singletonList("test")); headerVerifier.setSecurityProvider(new MockSecurityProvider()); headerVerifier.setAlgorithmProvider(new MockAlgorithmProvider()); - headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), MESSAGE_BODY.getBytes()); + headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), null); } @Test(expected = InvalidDataToVerifySignatureException.class) @@ -282,7 +282,7 @@ public class MessageVerifierTest { MessageVerifier headerVerifier = new MessageVerifier(keyId -> keyPair.getPublic()); headerVerifier.setSecurityProvider(new MockSecurityProvider()); headerVerifier.setAlgorithmProvider(new MockAlgorithmProvider()); - headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), MESSAGE_BODY.getBytes()); + headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), null); } private static void createAndAddSignature(Map<String, List<String>> headers) throws IOException { diff --git a/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java b/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java index 8f5bbe898dd..6da1dcb59a4 100644 --- a/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java +++ b/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java @@ -41,8 +41,11 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; +import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.client.ClientRequestContext; +import jakarta.ws.rs.client.ClientResponseContext; import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.core.MultivaluedHashMap; import jakarta.ws.rs.core.MultivaluedMap; @@ -56,7 +59,9 @@ import org.apache.cxf.phase.Phase; import org.apache.cxf.phase.PhaseInterceptor; import org.apache.cxf.phase.PhaseInterceptorChain; import org.apache.cxf.phase.PhaseManager; +import org.apache.cxf.rs.security.httpsignature.exception.SignatureException; import org.apache.cxf.rs.security.httpsignature.filters.CreateSignatureInterceptor; +import org.apache.cxf.rs.security.httpsignature.filters.VerifySignatureClientFilter; import org.apache.cxf.rs.security.httpsignature.filters.VerifySignatureFilter; import org.apache.cxf.rs.security.httpsignature.provider.KeyProvider; import org.apache.cxf.rs.security.httpsignature.provider.MockAlgorithmProvider; @@ -68,6 +73,8 @@ import org.junit.Test; import org.mockito.Mockito; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; /** * Some examples from the Appendix C of the spec. @@ -342,6 +349,175 @@ public class SpecExamplesTest { }); } + @Test + public void filterUnsignedBodyOnDeleteTest() throws IOException { + URI uri = URI.create("https://www.example.com/foo/1";); + String method = "DELETE"; + + // The client signs a DELETE without a body, so no digest is added or signed + Map<String, List<String>> headers = new HashMap<>(); + headers.put("Host", Collections.singletonList("example.com")); + headers.put("Date", Collections.singletonList("Sun, 05 Jan 2014 21:31:40 GMT")); + MessageSigner messageSigner = new MessageSigner(keyProvider, "Test", + Arrays.asList("(request-target)", "host", "date")); + messageSigner.sign(headers, "/foo/1", method); + + MultivaluedMap<String, String> requestStringHeaders = new MultivaluedHashMap<>(); + headers.forEach((header, value) -> requestStringHeaders.add(header, value.get(0))); + + // A body can not be added to the signed request without a signed digest + AtomicReference<Exception> result = new AtomicReference<>(); + doInPhaseInterceptor(new MessageImpl(), () -> { + ContainerRequestContext containerRequestContext = + getContainerRequestContextMock(uri, method, requestStringHeaders); + InputStream stream = new ByteArrayInputStream("{\"all\": true}".getBytes(StandardCharsets.UTF_8)); + Mockito.when(containerRequestContext.getEntityStream()).thenReturn(stream); + result.set(runFilter(() -> createVerifySignatureFilter().filter(containerRequestContext))); + }); + assertTrue(String.valueOf(result.get()), result.get() instanceof BadRequestException); + + // The signed request without a body is still accepted + result.set(null); + doInPhaseInterceptor(new MessageImpl(), () -> { + ContainerRequestContext containerRequestContext = + getContainerRequestContextMock(uri, method, requestStringHeaders); + Mockito.when(containerRequestContext.getEntityStream()).thenReturn(new ByteArrayInputStream(new byte[0])); + result.set(runFilter(() -> createVerifySignatureFilter().filter(containerRequestContext))); + }); + assertNull(result.get()); + } + + @Test + public void clientFilterUnsignedResponseBodyTest() throws IOException { + // The service signs a response without a body, so no digest is added or signed + Map<String, List<String>> headers = new HashMap<>(); + headers.put("Date", Collections.singletonList("Sun, 05 Jan 2014 21:31:40 GMT")); + MessageSigner messageSigner = new MessageSigner(keyProvider, "Test"); + messageSigner.sign(headers, "", ""); + + MultivaluedMap<String, String> responseHeaders = new MultivaluedHashMap<>(); + headers.forEach((header, value) -> responseHeaders.add(header, value.get(0))); + + // A body can not be added to the signed response without a signed digest + Message message = new MessageImpl(); + message.put(Message.REQUESTOR_ROLE, Boolean.TRUE); + message.put(Message.RESPONSE_CODE, 200); + AtomicReference<Exception> result = new AtomicReference<>(); + doInPhaseInterceptor(message, () -> { + ClientResponseContext responseContext = Mockito.mock(ClientResponseContext.class); + Mockito.when(responseContext.getHeaders()).thenReturn(responseHeaders); + InputStream stream = new ByteArrayInputStream("{\"all\": true}".getBytes(StandardCharsets.UTF_8)); + Mockito.when(responseContext.getEntityStream()).thenReturn(stream); + + MessageVerifier messageVerifier = new MessageVerifier(keyId -> publicKey); + messageVerifier.setSecurityProvider(new MockSecurityProvider()); + messageVerifier.setAlgorithmProvider(new MockAlgorithmProvider()); + VerifySignatureClientFilter verifySignatureFilter = new VerifySignatureClientFilter(); + verifySignatureFilter.setMessageVerifier(messageVerifier); + + result.set(runFilter(() -> verifySignatureFilter.filter(null, responseContext))); + }); + assertTrue(String.valueOf(result.get()), result.get() instanceof SignatureException); + } + + @Test + public void filterUnsignedBodyOnGetTest() throws IOException { + URI uri = URI.create("https://www.example.com/foo/1";); + String method = "GET"; + + // The client signs a GET without a body, so no digest is added or signed + Map<String, List<String>> headers = new HashMap<>(); + headers.put("Host", Collections.singletonList("example.com")); + headers.put("Date", Collections.singletonList("Sun, 05 Jan 2014 21:31:40 GMT")); + MessageSigner messageSigner = new MessageSigner(keyProvider, "Test", + Arrays.asList("(request-target)", "host", "date")); + messageSigner.sign(headers, "/foo/1", method); + + MultivaluedMap<String, String> requestStringHeaders = new MultivaluedHashMap<>(); + headers.forEach((header, value) -> requestStringHeaders.add(header, value.get(0))); + + // A body can not be added to the signed GET request without a signed digest + AtomicReference<Exception> result = new AtomicReference<>(); + doInPhaseInterceptor(new MessageImpl(), () -> { + ContainerRequestContext containerRequestContext = + getContainerRequestContextMock(uri, method, requestStringHeaders); + InputStream stream = new ByteArrayInputStream("{\"all\": true}".getBytes(StandardCharsets.UTF_8)); + Mockito.when(containerRequestContext.getEntityStream()).thenReturn(stream); + result.set(runFilter(() -> createVerifySignatureFilter().filter(containerRequestContext))); + }); + assertTrue(String.valueOf(result.get()), result.get() instanceof BadRequestException); + + // The signed GET request without a body is still accepted + result.set(null); + doInPhaseInterceptor(new MessageImpl(), () -> { + ContainerRequestContext containerRequestContext = + getContainerRequestContextMock(uri, method, requestStringHeaders); + Mockito.when(containerRequestContext.getEntityStream()).thenReturn(new ByteArrayInputStream(new byte[0])); + result.set(runFilter(() -> createVerifySignatureFilter().filter(containerRequestContext))); + }); + assertNull(result.get()); + } + + @Test + public void clientFilterUnsignedErrorResponseBodyTest() throws IOException { + // The service signs a response without a body, so no digest is added or signed + Map<String, List<String>> headers = new HashMap<>(); + headers.put("Date", Collections.singletonList("Sun, 05 Jan 2014 21:31:40 GMT")); + MessageSigner messageSigner = new MessageSigner(keyProvider, "Test"); + messageSigner.sign(headers, "", ""); + + MultivaluedMap<String, String> responseHeaders = new MultivaluedHashMap<>(); + headers.forEach((header, value) -> responseHeaders.add(header, value.get(0))); + + for (int status : new int[] {204, 404}) { + // A body can not be added to a signed non-2xx or 204 response without a signed digest + assertTrue(verifyResponse(responseHeaders, status, "{\"all\": true}") instanceof SignatureException); + + // The signed response without a body is still accepted + assertNull(verifyResponse(responseHeaders, status, "")); + } + } + + private Exception verifyResponse(MultivaluedMap<String, String> responseHeaders, int status, String body) { + Message message = new MessageImpl(); + message.put(Message.REQUESTOR_ROLE, Boolean.TRUE); + message.put(Message.RESPONSE_CODE, status); + AtomicReference<Exception> result = new AtomicReference<>(); + doInPhaseInterceptor(message, () -> { + ClientResponseContext responseContext = Mockito.mock(ClientResponseContext.class); + Mockito.when(responseContext.getHeaders()).thenReturn(responseHeaders); + InputStream stream = new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8)); + Mockito.when(responseContext.getEntityStream()).thenReturn(stream); + + MessageVerifier messageVerifier = new MessageVerifier(keyId -> publicKey); + messageVerifier.setSecurityProvider(new MockSecurityProvider()); + messageVerifier.setAlgorithmProvider(new MockAlgorithmProvider()); + VerifySignatureClientFilter verifySignatureFilter = new VerifySignatureClientFilter(); + verifySignatureFilter.setMessageVerifier(messageVerifier); + + result.set(runFilter(() -> verifySignatureFilter.filter(null, responseContext))); + }); + return result.get(); + } + + private VerifySignatureFilter createVerifySignatureFilter() { + MessageVerifier messageVerifier = new MessageVerifier(keyId -> publicKey); + messageVerifier.setSecurityProvider(new MockSecurityProvider()); + messageVerifier.setAlgorithmProvider(new MockAlgorithmProvider()); + VerifySignatureFilter verifySignatureFilter = new VerifySignatureFilter(); + verifySignatureFilter.setMessageVerifier(messageVerifier); + return verifySignatureFilter; + } + + private static Exception runFilter(Runnable r) { + try { + r.run(); + return null; + } catch (Exception ex) { + return ex; + } + } + private ClientRequestContext getClientRequestContextMock(URI uri, String method, MultivaluedMap<String, Object> requestHeaders) { ClientRequestContext requestContext = Mockito.mock(ClientRequestContext.class);
