Update getCanonicalizedHeaderString to follow SigV4 spec

Simon Krueger · Simon Krueger · commit 632fea82eae1 · 2021-05-26T19:46:45.000-07:00
- Update to handle leading and trailing whitespace in header values - Update to handle multiple header values See Step 4 of https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html Without this change a request to S3 with multiple header values or a header value with leading and trailing spaces (e.g., ``` Header1: Cat\n Header1: Dog\n Header2: "a b c" \n ``` ) that is signed by the AwsS3V4Signer will receive a 403 Forbidden response with an Error Code of SignatureDoesNotMatch from S3.
diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-74f2d55.json b/.changes/next-release/bugfix-AWSSDKforJavav2-74f2d55.json
@@ -0,0 +1,6 @@
+{
+    "category": "AWS SDK for Java v2", 
+    "contributor": "skrueger", 
+    "type": "bugfix", 
+    "description": "Fix bug in SigV4 canonical request creation to handle multiple header values and leading & trailing whitespace in header values correctly."
+}
diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java
@@ -304,7 +304,7 @@ private void addPreSignInformationToRequest(SdkHttpFullRequest.Builder mutableRe
         mutableRequest.putRawQueryParameter(SignerConstant.X_AMZ_CREDENTIAL, signingCredentials);
     }
 
-    private Map<String, List<String>> canonicalizeSigningHeaders(Map<String, List<String>> headers) {
+    static Map<String, List<String>> canonicalizeSigningHeaders(Map<String, List<String>> headers) {
         Map<String, List<String>> result = new TreeMap<>();
 
         for (Map.Entry<String, List<String>> header : headers.entrySet()) {
@@ -319,48 +319,73 @@ private Map<String, List<String>> canonicalizeSigningHeaders(Map<String, List<St
         return result;
     }
 
-    private String getCanonicalizedHeaderString(Map<String, List<String>> canonicalizedHeaders) {
+    /**
+     * Step 4 of the AWS Signature version 4 calculation.
+     * Refer to
+     * https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html
+     */
+    static String getCanonicalizedHeaderString(Map<String, List<String>> canonicalizedHeaders) {
         StringBuilder buffer = new StringBuilder();
 
         canonicalizedHeaders.forEach((headerName, headerValues) -> {
-            for (String headerValue : headerValues) {
+            if (headerValues.size() > 0) {
                 appendCompactedString(buffer, headerName);
-                buffer.append(":");
-                if (headerValue != null) {
-                    appendCompactedString(buffer, headerValue);
+                buffer.append(':');
+                boolean headerValueAppended = false;
+                // Append a comma-separated list of values for that header.
+                // Do not sort the values in headers that have multiple values.
+                for (int i = 0; i < headerValues.size(); i++) {
+                    String headerValue = headerValues.get(i);
+                    if (headerValue != null) {
+                        if (headerValueAppended) {
+                            buffer.append(',');
+                        }
+                        appendCompactedString(buffer, headerValue);
+                        headerValueAppended = true;
+                    }
                 }
-                buffer.append("\n");
+
+                buffer.append('\n');
             }
         });
 
         return buffer.toString();
     }
 
     /**
-     * This method appends a string to a string builder and collapses contiguous
-     * white space is a single space.
+     * This method appends a modified version of the source string to the destination string builder.
+     * The source string's leading and trailing whitespace is removed and
+     * and all contiguous white space not at the beginning or end is collapsed into a single space.
+     *
+     * e.g.,
+     * Given a source String of "   a    b    c   ",
+     * "a b c" is appended to the destination StringBuilder.
      *
      * This is equivalent to:
-     *      destination.append(source.replaceAll("\\s+", " "))
+     * <code>
+     *      destination.append(source.replaceAll("(^\\s*|\\s*$)", "").replaceAll("\\s+", " "))
+     * </code>
      * but does not create a Pattern object that needs to compile the match
      * string; it also prevents us from having to make a Matcher object as well.
-     *
      */
-    private void appendCompactedString(final StringBuilder destination, final String source) {
+    static void appendCompactedString(final StringBuilder destination, final String source) {
+        boolean appendedNonWhitespaceChar = false;
         boolean previousIsWhiteSpace = false;
         int length = source.length();
 
         for (int i = 0; i < length; i++) {
             char ch = source.charAt(i);
             if (isWhiteSpace(ch)) {
-                if (previousIsWhiteSpace) {
-                    continue;
+                if (appendedNonWhitespaceChar) {
+                    previousIsWhiteSpace = true;
                 }
-                destination.append(' ');
-                previousIsWhiteSpace = true;
             } else {
+                if (previousIsWhiteSpace) {
+                    destination.append(' ');
+                    previousIsWhiteSpace = false;
+                }
                 destination.append(ch);
-                previousIsWhiteSpace = false;
+                appendedNonWhitespaceChar = true;
             }
         }
     }
@@ -373,7 +398,7 @@ private void appendCompactedString(final StringBuilder destination, final String
      * @param ch the character to be tested
      * @return true if the character is white  space, false otherwise.
      */
-    private boolean isWhiteSpace(final char ch) {
+    private static boolean isWhiteSpace(final char ch) {
         return ch == ' ' || ch == '\t' || ch == '\n' || ch == '\u000b' || ch == '\r' || ch == '\f';
     }
 
diff --git a/core/auth/src/test/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4SignerTest.java b/core/auth/src/test/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4SignerTest.java
@@ -0,0 +1,83 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+package software.amazon.awssdk.auth.signer.internal;
+
+import org.junit.Test;
+
+import java.util.*;
+
+import static org.junit.Assert.assertEquals;
+
+public class AbstractAws4SignerTest {
+
+    @Test
+    public void testAppendCompactedString() {
+        StringBuilder destination = new StringBuilder();
+        AbstractAws4Signer.appendCompactedString(destination, "    a   b   c  ");
+        String expected = "a b c";
+        assertEquals(expected, destination.toString());
+    }
+
+    @Test
+    public void testGetCanonicalizedHeaderString_SpecExample() {
+        Map<String, List<String>> headers = new LinkedHashMap<>();
+        headers.put("Host", Arrays.asList("iam.amazonaws.com"));
+        headers.put("Content-Type", Arrays.asList("application/x-www-form-urlencoded; charset=utf-8"));
+        headers.put("My-header1", Arrays.asList("    a   b   c  "));
+        headers.put("X-Amz-Date", Arrays.asList("20150830T123600Z"));
+        headers.put("My-Header2", Arrays.asList("    \"a   b   c\"  "));
+
+        Map<String, List<String>> canonicalizeSigningHeaders =
+                AbstractAws4Signer.canonicalizeSigningHeaders(headers);
+
+        String actual = AbstractAws4Signer.getCanonicalizedHeaderString(canonicalizeSigningHeaders);
+
+        String expected = String.join("\n",
+                "content-type:application/x-www-form-urlencoded; charset=utf-8",
+                "host:iam.amazonaws.com",
+                "my-header1:a b c",
+                "my-header2:\"a b c\"",
+                "x-amz-date:20150830T123600Z",
+                "");
+
+        assertEquals(expected, actual);
+    }
+
+    @Test
+    public void testGetCanonicalizedHeaderString_MultipleHeaderValuess() {
+        Map<String, List<String>> headers = new LinkedHashMap<>();
+        headers.put("Host", Arrays.asList("iam.amazonaws.com"));
+        headers.put("Content-Type", Arrays.asList("application/x-www-form-urlencoded; charset=utf-8"));
+        headers.put("My-header1", Arrays.asList("    a   b   c  "));
+        headers.put("X-Amz-Date", Arrays.asList("20150830T123600Z"));
+        headers.put("My-Header1", Arrays.asList("    \"a   b   c\"  "));
+
+        Map<String, List<String>> canonicalizeSigningHeaders =
+                AbstractAws4Signer.canonicalizeSigningHeaders(headers);
+
+        String actual = AbstractAws4Signer.getCanonicalizedHeaderString(canonicalizeSigningHeaders);
+
+        String expected = String.join("\n",
+                "content-type:application/x-www-form-urlencoded; charset=utf-8",
+                "host:iam.amazonaws.com",
+                "my-header1:a b c,\"a b c\"",
+                "x-amz-date:20150830T123600Z",
+                "");
+
+        assertEquals(expected, actual);
+    }
+
+}
