Normalize URLs during signing (except for S3). (#3534)

During signing when generating the canonical request, request paths are supposed to be normalized to remove . and .. path components. We were not doing this before, making it possible to construct requests that fail signature validation.

Author: millems

.changes/next-release/bugfix-AWSSDKforJavav2-d06ff61.json

@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "contributor": "",
+    "description": "Fixed an issue that could result in signature mismatch exceptions when requests included . or .."
+}

core/auth/src/main/java/software/amazon/awssdk/auth/signer/AwsSignerExecutionAttribute.java

@@ -58,6 +58,12 @@ public final class AwsSignerExecutionAttribute extends SdkExecutionAttribute {
      */
     public static final ExecutionAttribute<Boolean> SIGNER_DOUBLE_URL_ENCODE = new ExecutionAttribute<>("DoubleUrlEncode");
 
+    /**
+     * The key to specify whether to normalize the resource path during signing.
+     */
+    public static final ExecutionAttribute<Boolean> SIGNER_NORMALIZE_PATH =
+        new ExecutionAttribute<>("NormalizePath");
+
     /**
      * The key to specify the expiration time when pre-signing aws requests.
      */

core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java

@@ -24,8 +24,11 @@
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
+import java.util.SortedMap;
+import java.util.TreeMap;
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.auth.credentials.AwsCredentials;
 import software.amazon.awssdk.auth.credentials.AwsSessionCredentials;
@@ -41,6 +44,7 @@
 import software.amazon.awssdk.core.internal.util.HttpChecksumUtils;
 import software.amazon.awssdk.core.signer.Presigner;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
+import software.amazon.awssdk.http.SdkHttpRequest;
 import software.amazon.awssdk.utils.BinaryUtils;
 import software.amazon.awssdk.utils.Logger;
 import software.amazon.awssdk.utils.Pair;
@@ -81,6 +85,7 @@ protected SdkHttpFullRequest.Builder doSign(SdkHttpFullRequest request,
                                                 ContentChecksum contentChecksum) {
 
         SdkHttpFullRequest.Builder mutableRequest = request.toBuilder();
+
         AwsCredentials sanitizedCredentials = sanitizeCredentials(signingParams.awsCredentials());
         if (sanitizedCredentials instanceof AwsSessionCredentials) {
             addSessionCredentials(mutableRequest, (AwsSessionCredentials) sanitizedCredentials);
@@ -97,9 +102,11 @@ protected SdkHttpFullRequest.Builder doSign(SdkHttpFullRequest request,
         putChecksumHeader(signingParams.checksumParams(), contentChecksum.contentFlexibleChecksum(),
                 mutableRequest, contentChecksum.contentHash());
 
-        CanonicalRequest canonicalRequest = createCanonicalRequest(mutableRequest,
+        CanonicalRequest canonicalRequest = createCanonicalRequest(request,
+                                                                   mutableRequest,
                                                                    contentChecksum.contentHash(),
-                                                                   signingParams.doubleUrlEncode());
+                                                                   signingParams.doubleUrlEncode(),
+                                                                   signingParams.normalizePath());
 
         String canonicalRequestString = canonicalRequest.string();
         String stringToSign = createStringToSign(canonicalRequestString, requestParams);
@@ -138,8 +145,11 @@ protected SdkHttpFullRequest.Builder doPresign(SdkHttpFullRequest request,
         // Add the important parameters for v4 signing
         String contentSha256 = calculateContentHashPresign(mutableRequest, signingParams);
 
-        CanonicalRequest canonicalRequest = createCanonicalRequest(mutableRequest, contentSha256,
-                                                                   signingParams.doubleUrlEncode());
+        CanonicalRequest canonicalRequest = createCanonicalRequest(request,
+                                                                   mutableRequest,
+                                                                   contentSha256,
+                                                                   signingParams.doubleUrlEncode(),
+                                                                   signingParams.normalizePath());
 
         addPreSignInformationToRequest(mutableRequest, canonicalRequest, sanitizedCredentials,
                                        requestParams, expirationInSeconds);
@@ -237,10 +247,12 @@ protected final byte[] deriveSigningKey(AwsCredentials credentials, Instant sign
      * .amazon.com/general/latest/gr/sigv4-create-canonical-request.html to
      * generate the canonical request.
      */
-    private CanonicalRequest createCanonicalRequest(SdkHttpFullRequest.Builder request,
+    private CanonicalRequest createCanonicalRequest(SdkHttpFullRequest request,
+                                                    SdkHttpFullRequest.Builder requestBuilder,
                                                     String contentSha256,
-                                                    boolean doubleUrlEncode) {
-        return new CanonicalRequest(request, contentSha256, doubleUrlEncode);
+                                                    boolean doubleUrlEncode,
+                                                    boolean normalizePath) {
+        return new CanonicalRequest(request, requestBuilder, contentSha256, doubleUrlEncode, normalizePath);
     }
 
     /**
@@ -251,6 +263,8 @@ private CanonicalRequest createCanonicalRequest(SdkHttpFullRequest.Builder reque
     private String createStringToSign(String canonicalRequest,
                                       Aws4SignerRequestParams requestParams) {
 
+        LOG.debug(() -> "AWS4 Canonical Request: " + canonicalRequest);
+
         String requestHash = BinaryUtils.toHex(hash(canonicalRequest));
 
         String stringToSign = requestParams.getSigningAlgorithm() +
@@ -330,7 +344,7 @@ private void addPreSignInformationToRequest(SdkHttpFullRequest.Builder mutableRe
      * @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(char ch) {
         return ch == ' ' || ch == '\t' || ch == '\n' || ch == '\u000b' || ch == '\r' || ch == '\f';
     }
 
@@ -402,8 +416,15 @@ protected <B extends Aws4SignerParams.Builder> B extractSignerParams(B paramsBui
                      .signingRegion(executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNING_REGION))
                      .timeOffset(executionAttributes.getAttribute(AwsSignerExecutionAttribute.TIME_OFFSET));
 
-        if (executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNER_DOUBLE_URL_ENCODE) != null) {
-            paramsBuilder.doubleUrlEncode(executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNER_DOUBLE_URL_ENCODE));
+        Boolean doubleUrlEncode = executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNER_DOUBLE_URL_ENCODE);
+        if (doubleUrlEncode != null) {
+            paramsBuilder.doubleUrlEncode(doubleUrlEncode);
+        }
+
+        Boolean normalizePath =
+            executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNER_NORMALIZE_PATH);
+        if (normalizePath != null) {
+            paramsBuilder.normalizePath(normalizePath);
         }
         ChecksumSpecs checksumSpecs = executionAttributes.getAttribute(RESOLVED_CHECKSUM_SPECS);
         if (checksumSpecs != null && checksumSpecs.algorithm() != null) {
@@ -453,30 +474,41 @@ private SdkChecksum createSdkChecksumFromParams(T signingParams, SdkHttpFullRequ
         return null;
     }
 
-    private class CanonicalRequest {
-        private final SdkHttpFullRequest.Builder request;
+    static final class CanonicalRequest {
+        private final SdkHttpFullRequest request;
+        private final SdkHttpFullRequest.Builder requestBuilder;
         private final String contentSha256;
         private final boolean doubleUrlEncode;
+        private final boolean normalizePath;
 
         private String canonicalRequestString;
         private StringBuilder signedHeaderStringBuilder;
         private List<Pair<String, List<String>>> canonicalHeaders;
         private String signedHeaderString;
 
-        private CanonicalRequest(SdkHttpFullRequest.Builder request, String contentSha256, boolean doubleUrlEncode) {
+        CanonicalRequest(SdkHttpFullRequest request,
+                         SdkHttpFullRequest.Builder requestBuilder,
+                         String contentSha256,
+                         boolean doubleUrlEncode,
+                         boolean normalizePath) {
             this.request = request;
+            this.requestBuilder = requestBuilder;
             this.contentSha256 = contentSha256;
             this.doubleUrlEncode = doubleUrlEncode;
+            this.normalizePath = normalizePath;
         }
 
         public String string() {
             if (canonicalRequestString == null) {
                 StringBuilder canonicalRequest = new StringBuilder(512);
-                canonicalRequest.append(request.method().toString())
+                canonicalRequest.append(requestBuilder.method().toString())
                                 .append(SignerConstant.LINE_SEPARATOR);
-                addCanonicalizedResourcePath(canonicalRequest, request.encodedPath(), doubleUrlEncode);
+                addCanonicalizedResourcePath(canonicalRequest,
+                                             request,
+                                             doubleUrlEncode,
+                                             normalizePath);
                 canonicalRequest.append(SignerConstant.LINE_SEPARATOR);
-                addCanonicalizedQueryString(canonicalRequest, request);
+                addCanonicalizedQueryString(canonicalRequest, requestBuilder);
                 canonicalRequest.append(SignerConstant.LINE_SEPARATOR);
                 addCanonicalizedHeaderString(canonicalRequest, canonicalHeaders());
                 canonicalRequest.append(SignerConstant.LINE_SEPARATOR)
@@ -488,6 +520,82 @@ public String string() {
             return canonicalRequestString;
         }
 
+        private void addCanonicalizedResourcePath(StringBuilder result,
+                                                  SdkHttpRequest request,
+                                                  boolean urlEncode,
+                                                  boolean normalizePath) {
+            String path = normalizePath ? request.getUri().normalize().getRawPath()
+                                        : request.encodedPath();
+
+            if (StringUtils.isEmpty(path)) {
+                result.append("/");
+                return;
+            }
+
+            if (urlEncode) {
+                path = SdkHttpUtils.urlEncodeIgnoreSlashes(path);
+            }
+
+            if (!path.startsWith("/")) {
+                result.append("/");
+            }
+            result.append(path);
+
+            // Normalization can leave a trailing slash at the end of the resource path,
+            // even if the input path doesn't end with one. Example input: /foo/bar/.
+            // Remove the trailing slash if the input path doesn't end with one.
+            boolean trimTrailingSlash = normalizePath &&
+                                        path.length() > 1 &&
+                                        !request.encodedPath().endsWith("/") &&
+                                        result.charAt(result.length() - 1) == '/';
+            if (trimTrailingSlash) {
+                result.setLength(result.length() - 1);
+            }
+        }
+
+        /**
+         * Examines the specified query string parameters and returns a
+         * canonicalized form.
+         * <p>
+         * The canonicalized query string is formed by first sorting all the query
+         * string parameters, then URI encoding both the key and value and then
+         * joining them, in order, separating key value pairs with an '&amp;'.
+         *
+         * @return A canonicalized form for the specified query string parameters.
+         */
+        private void addCanonicalizedQueryString(StringBuilder result, SdkHttpRequest.Builder httpRequest) {
+
+            SortedMap<String, List<String>> sorted = new TreeMap<>();
+
+            /**
+             * Signing protocol expects the param values also to be sorted after url
+             * encoding in addition to sorted parameter names.
+             */
+            httpRequest.forEachRawQueryParameter((key, values) -> {
+                if (StringUtils.isEmpty(key)) {
+                    // Do not sign empty keys.
+                    return;
+                }
+
+                String encodedParamName = SdkHttpUtils.urlEncode(key);
+
+                List<String> encodedValues = new ArrayList<>(values.size());
+                for (String value : values) {
+                    String encodedValue = SdkHttpUtils.urlEncode(value);
+
+                    // Null values should be treated as empty for the purposes of signing, not missing.
+                    // For example "?foo=" instead of "?foo".
+                    String signatureFormattedEncodedValue = encodedValue == null ? "" : encodedValue;
+
+                    encodedValues.add(signatureFormattedEncodedValue);
+                }
+                Collections.sort(encodedValues);
+                sorted.put(encodedParamName, encodedValues);
+            });
+
+            SdkHttpUtils.flattenQueryParameters(result, sorted);
+        }
+
         public StringBuilder signedHeaderStringBuilder() {
             if (signedHeaderStringBuilder == null) {
                 signedHeaderStringBuilder = new StringBuilder();
@@ -505,7 +613,7 @@ public String signedHeaderString() {
 
         private List<Pair<String, List<String>>> canonicalHeaders() {
             if (canonicalHeaders == null) {
-                canonicalHeaders = canonicalizeSigningHeaders(request);
+                canonicalHeaders = canonicalizeSigningHeaders(requestBuilder);
             }
             return canonicalHeaders;
         }

core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAwsSigner.java

@@ -21,11 +21,6 @@
 import java.security.DigestInputStream;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.SortedMap;
-import java.util.TreeMap;
 import javax.crypto.Mac;
 import javax.crypto.spec.SecretKeySpec;
 import software.amazon.awssdk.annotations.SdkInternalApi;
@@ -38,10 +33,8 @@
 import software.amazon.awssdk.core.signer.Signer;
 import software.amazon.awssdk.http.ContentStreamProvider;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
-import software.amazon.awssdk.http.SdkHttpRequest;
 import software.amazon.awssdk.utils.BinaryUtils;
 import software.amazon.awssdk.utils.StringUtils;
-import software.amazon.awssdk.utils.http.SdkHttpUtils;
 
 /**
  * Abstract base class for AWS signing protocol implementations. Provides
@@ -219,49 +212,6 @@ byte[] hash(byte[] data) throws SdkClientException {
         return hash(data, null);
     }
 
-    /**
-     * Examines the specified query string parameters and returns a
-     * canonicalized form.
-     * <p>
-     * The canonicalized query string is formed by first sorting all the query
-     * string parameters, then URI encoding both the key and value and then
-     * joining them, in order, separating key value pairs with an '&amp;'.
-     *
-     * @return A canonicalized form for the specified query string parameters.
-     */
-    protected void addCanonicalizedQueryString(StringBuilder result, SdkHttpRequest.Builder httpRequest) {
-
-        SortedMap<String, List<String>> sorted = new TreeMap<>();
-
-        /**
-         * Signing protocol expects the param values also to be sorted after url
-         * encoding in addition to sorted parameter names.
-         */
-        httpRequest.forEachRawQueryParameter((key, values) -> {
-            if (StringUtils.isEmpty(key)) {
-                // Do not sign empty keys.
-                return;
-            }
-
-            String encodedParamName = SdkHttpUtils.urlEncode(key);
-
-            List<String> encodedValues = new ArrayList<>(values.size());
-            for (String value : values) {
-                String encodedValue = SdkHttpUtils.urlEncode(value);
-
-                // Null values should be treated as empty for the purposes of signing, not missing.
-                // For example "?foo=" instead of "?foo".
-                String signatureFormattedEncodedValue = encodedValue == null ? "" : encodedValue;
-
-                encodedValues.add(signatureFormattedEncodedValue);
-            }
-            Collections.sort(encodedValues);
-            sorted.put(encodedParamName, encodedValues);
-        });
-
-        SdkHttpUtils.flattenQueryParameters(result, sorted);
-    }
-
     protected InputStream getBinaryRequestPayloadStream(ContentStreamProvider streamProvider) {
         try {
             if (streamProvider == null) {
@@ -278,31 +228,6 @@ protected InputStream getBinaryRequestPayloadStream(ContentStreamProvider stream
         }
     }
 
-    protected void addCanonicalizedResourcePath(StringBuilder result, String resourcePath, boolean urlEncode) {
-        if (StringUtils.isEmpty(resourcePath)) {
-            result.append("/");
-        } else {
-            String value = urlEncode ? SdkHttpUtils.urlEncodeIgnoreSlashes(resourcePath) : resourcePath;
-            if (value.startsWith("/")) {
-                result.append(value);
-            } else {
-                result.append("/").append(value);
-            }
-        }
-    }
-
-    protected String getCanonicalizedEndpoint(SdkHttpFullRequest request) {
-        String endpointForStringToSign = StringUtils.lowerCase(request.host());
-
-        // Omit the port from the endpoint if we're using the default port for the protocol. Some HTTP clients (ie. Apache) don't
-        // allow you to specify it in the request, so we're standardizing around not including it. See SdkHttpRequest#port().
-        if (!SdkHttpUtils.isUsingStandardPort(request.protocol(), request.port())) {
-            endpointForStringToSign += ":" + request.port();
-        }
-
-        return endpointForStringToSign;
-    }
-
     /**
      * Loads the individual access key ID and secret key from the specified credentials, trimming any extra whitespace from the
      * credentials.