This is an automated email from the ASF dual-hosted git repository.

coheigea pushed a commit to branch 3.4.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git


The following commit(s) were added to refs/heads/3.4.x-fixes by this push:
     new f5e7b2d  [CXF-8535] Query missing from signature request-target AND 
also to add digest only if request has body (#869)
f5e7b2d is described below

commit f5e7b2dee697dc200ec7121b41ea4bbd9530a04a
Author: AnilKumarHurkadli <[email protected]>
AuthorDate: Tue Nov 30 14:42:37 2021 +0530

    [CXF-8535] Query missing from signature request-target AND also to add 
digest only if request has body (#869)
    
    * [CXF-8535] Query missing from signature request-target AND also to add 
digest only if request has body
    
    * Message body available in Filter is used and added as an argument which 
is passed through
    
    * Added Test case for not throwing exception having null body Delete Method
    
    * Fix to avoid decoding of the path parameters by using rawpath instead of 
path otherwise will result in invalid signatures
    
    * Fixing the test case failure in systests/rs-security by adding required 
conditions in MessageVerifier
    
    Co-authored-by: Anilkumar Hurkadli <[email protected]>
---
 .../rs/security/httpsignature/MessageVerifier.java | 14 ++---
 .../filters/AbstractSignatureInFilter.java         |  6 ++-
 .../filters/VerifySignatureClientFilter.java       |  2 +-
 .../filters/VerifySignatureFilter.java             |  7 +--
 .../httpsignature/utils/SignatureHeaderUtils.java  |  2 +-
 .../httpsignature/MessageVerifierTest.java         | 61 ++++++++++++++++------
 .../security/httpsignature/SpecExamplesTest.java   | 13 +++--
 7 files changed, 68 insertions(+), 37 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 06bef9a..50094f5 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
@@ -18,11 +18,7 @@
  */
 package org.apache.cxf.rs.security.httpsignature;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
+import java.util.*;
 import java.util.logging.Logger;
 
 import org.apache.cxf.common.logging.LogUtils;
@@ -101,7 +97,8 @@ public class MessageVerifier {
         this.algorithmProvider = Objects.requireNonNull(algorithmProvider, 
"algorithm provider cannot be null");
     }
 
-    public void verifyMessage(Map<String, List<String>> messageHeaders, String 
method, String uri, Message m) {
+    public void verifyMessage(Map<String, List<String>> messageHeaders, String 
method,
+                              String uri, Message m, byte[] messageBody) {
         SignatureHeaderUtils.inspectMessageHeaders(messageHeaders);
         inspectMissingSignatureHeader(messageHeaders);
 
@@ -123,7 +120,10 @@ public class MessageVerifier {
             if (!(requestor || 
signedHeaders.contains(HTTPSignatureConstants.REQUEST_TARGET))) {
                 signedHeaders.add(HTTPSignatureConstants.REQUEST_TARGET);
             }
-            if (!signedHeaders.contains("digest")) {
+
+            if (!signedHeaders.contains("digest") && ((messageBody != null && 
messageBody.length > 0)
+                    || ("POST".equalsIgnoreCase(method) || 
"PUT".equalsIgnoreCase(method)
+                    || "PATCH".equalsIgnoreCase(method)))) {
                 signedHeaders.add("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 5b51883..8da49b1 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
@@ -99,7 +99,7 @@ abstract class AbstractSignatureInFilter {
     }
 
     protected void verifySignature(MultivaluedMap<String, String> headers, 
String uriPath,
-                                   String httpMethod) {
+                                   String httpMethod, byte[] messageBody) {
         if (!enabled) {
             LOG.fine("Verify signature filter is disabled");
             return;
@@ -111,7 +111,9 @@ abstract class AbstractSignatureInFilter {
 
         LOG.fine("Starting filter message verification process");
         try {
-            messageVerifier.verifyMessage(headers, httpMethod, uriPath, 
PhaseInterceptorChain.getCurrentMessage());
+            messageVerifier.verifyMessage(headers, httpMethod, uriPath,
+                    PhaseInterceptorChain.getCurrentMessage(),
+                    messageBody);
         } catch (DifferentAlgorithmsException | InvalidSignatureHeaderException
             | InvalidDataToVerifySignatureException | InvalidSignatureException
             | MultipleSignatureHeaderException | 
MissingSignatureHeaderException ex) {
diff --git 
a/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/VerifySignatureClientFilter.java
 
b/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/VerifySignatureClientFilter.java
index 1e88130..f3432dc 100644
--- 
a/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/VerifySignatureClientFilter.java
+++ 
b/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/VerifySignatureClientFilter.java
@@ -44,7 +44,7 @@ public class VerifySignatureClientFilter extends 
AbstractSignatureInFilter imple
             responseContext.setEntityStream(new 
ByteArrayInputStream(messageBody));
         }
 
-        verifySignature(responseContext.getHeaders(), "", "");
+        verifySignature(responseContext.getHeaders(), "", "", messageBody);
     }
 
     @Override
diff --git 
a/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/VerifySignatureFilter.java
 
b/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/VerifySignatureFilter.java
index c70c5e4..e4d9d01 100644
--- 
a/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/VerifySignatureFilter.java
+++ 
b/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/VerifySignatureFilter.java
@@ -28,7 +28,6 @@ import javax.ws.rs.container.ContainerRequestFilter;
 import javax.ws.rs.ext.Provider;
 
 import org.apache.cxf.rs.security.httpsignature.utils.SignatureHeaderUtils;
-
 /**
  * RS CXF container Filter which verifies the Digest header, and then extracts 
signature data from the context
  * and sends it to the message verifier
@@ -36,17 +35,15 @@ import 
org.apache.cxf.rs.security.httpsignature.utils.SignatureHeaderUtils;
 @Provider
 @Priority(Priorities.AUTHENTICATION)
 public class VerifySignatureFilter extends AbstractSignatureInFilter 
implements ContainerRequestFilter {
-
     @Override
     public void filter(ContainerRequestContext requestCtx) {
         byte[] messageBody = verifyDigest(requestCtx.getHeaders(), 
requestCtx.getEntityStream());
         if (messageBody != null) {
             requestCtx.setEntityStream(new ByteArrayInputStream(messageBody));
         }
-
         verifySignature(requestCtx.getHeaders(),
-                
SignatureHeaderUtils.createRequestTarget(requestCtx.getUriInfo().getAbsolutePath()),
-                        requestCtx.getMethod());
+                
SignatureHeaderUtils.createRequestTarget(requestCtx.getUriInfo().getRequestUri()),
+                        requestCtx.getMethod(), messageBody);
     }
 
     @Override
diff --git 
a/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/utils/SignatureHeaderUtils.java
 
b/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/utils/SignatureHeaderUtils.java
index 661b81c..88fa37f 100644
--- 
a/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/utils/SignatureHeaderUtils.java
+++ 
b/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/utils/SignatureHeaderUtils.java
@@ -104,7 +104,7 @@ public final class SignatureHeaderUtils {
 
     public static String createRequestTarget(URI uri) {
         StringBuilder stringBuilder = new StringBuilder();
-        stringBuilder.append(uri.getPath());
+        stringBuilder.append(uri.getRawPath());
 
         if (uri.getRawQuery() != null) {
             stringBuilder.append('?');
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 873ccf3..876df06 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
@@ -49,8 +49,10 @@ import org.junit.Test;
 public class MessageVerifierTest {
     private static final String KEY_ID = "testVerifier";
     private static final String METHOD = "GET";
+    private static final String DELETE_METHOD = "DELETE";
     private static final String URI = "/test/signature";
     private static final String KEY_PAIR_GENERATOR_ALGORITHM = "RSA";
+    private static final String MESSAGE_BODY = "Hello";
 
     private static MessageSigner messageSigner;
     private static MessageVerifier messageVerifier;
@@ -78,14 +80,21 @@ public class MessageVerifierTest {
     public void validUnalteredRequest() throws IOException {
         Map<String, List<String>> headers = createMockHeaders();
         createAndAddSignature(headers);
-        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
+    }
+
+    @Test
+    public void nullBodyRequest() throws IOException {
+        Map<String, List<String>> headers = createMockHeaders();
+        createAndAddSignature(headers);
+        messageVerifier.verifyMessage(headers, DELETE_METHOD, URI, new 
MessageImpl(), null);
     }
 
     @Test
     public void validUnalteredRequestWithoutBody() throws IOException {
         Map<String, List<String>> headers = createMockHeaders();
         createAndAddSignature(headers);
-        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test
@@ -94,7 +103,7 @@ public class MessageVerifierTest {
         createAndAddSignature(headers);
         headers.put("Test", Collections.singletonList("value"));
         headers.put("Test2", Collections.singletonList("value2"));
-        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test(expected = DifferentAlgorithmsException.class)
@@ -104,7 +113,7 @@ public class MessageVerifierTest {
         String signature = headers.get("Signature").get(0);
         signature = signature.replaceFirst("algorithm=\"rsa-sha256", 
"algorithm=\"hmac-sha256");
         headers.replace("Signature", Collections.singletonList(signature));
-        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test(expected = InvalidDataToVerifySignatureException.class)
@@ -112,7 +121,7 @@ public class MessageVerifierTest {
         Map<String, List<String>> headers = createMockHeaders();
         createAndAddSignature(headers);
         headers.remove("Content-Length");
-        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test(expected = InvalidSignatureException.class)
@@ -122,7 +131,7 @@ public class MessageVerifierTest {
         String signature = headers.get("Signature").get(0);
         signature = signature.replaceFirst("signature=\".{10}", 
"signature=\"AAAAAAAAAA");
         headers.replace("Signature", Collections.singletonList(signature));
-        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test(expected = InvalidSignatureHeaderException.class)
@@ -132,13 +141,13 @@ public class MessageVerifierTest {
         String signature = headers.get("Signature").get(0);
         signature = signature.replaceFirst(",signature", ",signature2");
         headers.replace("Signature", Collections.singletonList(signature));
-        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test(expected = MissingSignatureHeaderException.class)
     public void missingSignatureHeaderFails() {
         Map<String, List<String>> headers = createMockHeaders();
-        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test(expected = MultipleSignatureHeaderException.class)
@@ -149,7 +158,7 @@ public class MessageVerifierTest {
         List<String> signatureList = new ArrayList<>(headers.get("Signature"));
         signatureList.add(signature);
         headers.put("Signature", signatureList);
-        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        messageVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test
@@ -166,7 +175,7 @@ public class MessageVerifierTest {
         MessageVerifier hmacMessageVerifier =
             new MessageVerifier(keyId -> secretKey, null, keyId -> 
"hmac-sha256", Collections.emptyList());
         hmacMessageVerifier.setAddDefaultRequiredHeaders(false);
-        hmacMessageVerifier.verifyMessage(headers, METHOD, URI, new 
MessageImpl());
+        hmacMessageVerifier.verifyMessage(headers, METHOD, URI, new 
MessageImpl(), MESSAGE_BODY.getBytes());
     }
 
     @Test
@@ -180,7 +189,7 @@ public class MessageVerifierTest {
         headerVerifier.setAddDefaultRequiredHeaders(false);
         headerVerifier.setSecurityProvider(new MockSecurityProvider());
         headerVerifier.setAlgorithmProvider(new MockAlgorithmProvider());
-        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test(expected = InvalidDataToVerifySignatureException.class)
@@ -194,7 +203,7 @@ public class MessageVerifierTest {
         headerVerifier.setAddDefaultRequiredHeaders(false);
         headerVerifier.setSecurityProvider(new MockSecurityProvider());
         headerVerifier.setAlgorithmProvider(new MockAlgorithmProvider());
-        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test(expected = InvalidDataToVerifySignatureException.class)
@@ -207,7 +216,7 @@ public class MessageVerifierTest {
         headerVerifier.setAddDefaultRequiredHeaders(false);
         headerVerifier.setSecurityProvider(new MockSecurityProvider());
         headerVerifier.setAlgorithmProvider(new MockAlgorithmProvider());
-        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test
@@ -221,7 +230,7 @@ public class MessageVerifierTest {
                                                              
Collections.singletonList("test"));
         headerVerifier.setSecurityProvider(new MockSecurityProvider());
         headerVerifier.setAlgorithmProvider(new MockAlgorithmProvider());
-        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test
@@ -234,7 +243,21 @@ 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());
+        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
+    }
+
+    @Test
+    public void 
nullBodyDelMethodShouldNotThrowInvalidDataToVerifySignatureException() throws 
IOException {
+        Map<String, List<String>> headers = createMockHeaders();
+        headers.put("Test", Collections.singletonList("value"));
+        headers.put(HTTPSignatureConstants.REQUEST_TARGET, 
Collections.singletonList("12345"));
+        createAndAddSignatureDeleteMethod(headers);
+
+        MessageVerifier headerVerifier = new MessageVerifier(keyId -> 
keyPair.getPublic());
+        headerVerifier.setAddDefaultRequiredHeaders(true);
+        headerVerifier.setSecurityProvider(new MockSecurityProvider());
+        headerVerifier.setAlgorithmProvider(new MockAlgorithmProvider());
+        headerVerifier.verifyMessage(headers, DELETE_METHOD, URI, new 
MessageImpl(), null);
     }
 
     @Test(expected = InvalidDataToVerifySignatureException.class)
@@ -247,7 +270,7 @@ public class MessageVerifierTest {
                                                              
Collections.singletonList("test"));
         headerVerifier.setSecurityProvider(new MockSecurityProvider());
         headerVerifier.setAlgorithmProvider(new MockAlgorithmProvider());
-        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl());
+        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     @Test(expected = InvalidDataToVerifySignatureException.class)
@@ -259,13 +282,17 @@ 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());
+        headerVerifier.verifyMessage(headers, METHOD, URI, new MessageImpl(), 
MESSAGE_BODY.getBytes());
     }
 
     private static void createAndAddSignature(Map<String, List<String>> 
headers) throws IOException {
         messageSigner.sign(headers, URI, METHOD);
     }
 
+    private static void createAndAddSignatureDeleteMethod(Map<String, 
List<String>> headers) throws IOException {
+        messageSigner.sign(headers, URI, DELETE_METHOD);
+    }
+
     private static Map<String, List<String>> createMockHeaders() {
         Map<String, List<String>> headers = new HashMap<>();
         headers.put("Host", Collections.singletonList("example.org"));
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 d44c8cd..d18fdd6 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
@@ -66,6 +66,8 @@ import static org.junit.Assert.assertEquals;
  */
 public class SpecExamplesTest {
 
+    private static final String MESSAGE_BODY = "Hello";
+
     private static KeyProvider keyProvider;
     private static PublicKey publicKey;
     private Bus bus;
@@ -124,7 +126,8 @@ public class SpecExamplesTest {
         messageVerifier.setAddDefaultRequiredHeaders(false);
         messageVerifier.setSecurityProvider(new MockSecurityProvider());
         messageVerifier.setAlgorithmProvider(new MockAlgorithmProvider());
-        messageVerifier.verifyMessage(headers, "POST", 
"/foo?param=value&pet=dog", new MessageImpl());
+        messageVerifier.verifyMessage(headers, "POST", 
"/foo?param=value&pet=dog",
+                new MessageImpl(), MESSAGE_BODY.getBytes());
     }
 
     @Test
@@ -197,7 +200,8 @@ public class SpecExamplesTest {
         messageVerifier.setAddDefaultRequiredHeaders(false);
         messageVerifier.setSecurityProvider(new MockSecurityProvider());
         messageVerifier.setAlgorithmProvider(new MockAlgorithmProvider());
-        messageVerifier.verifyMessage(headers, "POST", 
"/foo?param=value&pet=dog", new MessageImpl());
+        messageVerifier.verifyMessage(headers, "POST", 
"/foo?param=value&pet=dog",
+                new MessageImpl(), MESSAGE_BODY.getBytes());
     }
 
     @Test
@@ -272,7 +276,8 @@ public class SpecExamplesTest {
         MessageVerifier messageVerifier = new MessageVerifier(keyId -> 
publicKey);
         messageVerifier.setSecurityProvider(new MockSecurityProvider());
         messageVerifier.setAlgorithmProvider(new MockAlgorithmProvider());
-        messageVerifier.verifyMessage(headers, "POST", 
"/foo?param=value&pet=dog", new MessageImpl());
+        messageVerifier.verifyMessage(headers, "POST", 
"/foo?param=value&pet=dog",
+                new MessageImpl(), MESSAGE_BODY.getBytes());
     }
 
     @Test
@@ -344,7 +349,7 @@ public class SpecExamplesTest {
                                                                        
requestStringHeaders) {
         ContainerRequestContext containerRequestContext = 
Mockito.mock(ContainerRequestContext.class);
         UriInfo uriInfo = Mockito.mock(UriInfo.class);
-        Mockito.when(uriInfo.getAbsolutePath()).thenReturn(uri);
+        Mockito.when(uriInfo.getRequestUri()).thenReturn(uri);
         Mockito.when(containerRequestContext.getUriInfo()).thenReturn(uriInfo);
         Mockito.when(containerRequestContext.getMethod()).thenReturn(method);
         
Mockito.when(containerRequestContext.getHeaders()).thenReturn(requestStringHeaders);

Reply via email to