From 99e0a5b58c8e5a7b5f4399ca00cb2edb26a5326a Mon Sep 17 00:00:00 2001 From: yibole Date: Fri, 13 Feb 2026 16:40:12 -0800 Subject: [PATCH] use json writer --- services/cloudfront/pom.xml | 5 + .../internal/utils/SigningUtils.java | 113 ++++++++--- .../cloudfront/CloudFrontUtilitiesTest.java | 2 +- .../internal/utils/SigningUtilsTest.java | 176 ++++++++++++++++++ 4 files changed, 272 insertions(+), 24 deletions(-) create mode 100644 services/cloudfront/src/test/java/software/amazon/awssdk/services/cloudfront/internal/utils/SigningUtilsTest.java diff --git a/services/cloudfront/pom.xml b/services/cloudfront/pom.xml index 82ab05f9e456..624983098122 100644 --- a/services/cloudfront/pom.xml +++ b/services/cloudfront/pom.xml @@ -51,6 +51,11 @@ aws-xml-protocol ${awsjavasdk.version} + + software.amazon.awssdk + json-utils + ${awsjavasdk.version} + software.amazon.awssdk protocol-core diff --git a/services/cloudfront/src/main/java/software/amazon/awssdk/services/cloudfront/internal/utils/SigningUtils.java b/services/cloudfront/src/main/java/software/amazon/awssdk/services/cloudfront/internal/utils/SigningUtils.java index 407ecec7088d..4b99ae619b3d 100644 --- a/services/cloudfront/src/main/java/software/amazon/awssdk/services/cloudfront/internal/utils/SigningUtils.java +++ b/services/cloudfront/src/main/java/software/amazon/awssdk/services/cloudfront/internal/utils/SigningUtils.java @@ -34,6 +34,7 @@ import java.util.Base64; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.exception.SdkClientException; +import software.amazon.awssdk.protocols.jsoncore.JsonWriter; import software.amazon.awssdk.services.cloudfront.internal.auth.Pem; import software.amazon.awssdk.utils.IoUtils; import software.amazon.awssdk.utils.StringUtils; @@ -57,11 +58,29 @@ private SigningUtils() { * >Setting signed cookies using a canned policy. */ public static String buildCannedPolicy(String resourceUrl, Instant expirationDate) { - return "{\"Statement\":[{\"Resource\":\"" - + resourceUrl - + "\",\"Condition\":{\"DateLessThan\":{\"AWS:EpochTime\":" - + expirationDate.getEpochSecond() - + "}}}]}"; + Validate.notNull(resourceUrl, "resourceUrl must not be null"); + Validate.notNull(expirationDate, "expirationDate must not be null"); + validateInput(resourceUrl, "resourceUrl"); + + JsonWriter writer = JsonWriter.create(); + writer.writeStartObject() + .writeFieldName("Statement") + .writeStartArray() + .writeStartObject() + .writeFieldName("Resource") + .writeValue(resourceUrl) + .writeFieldName("Condition") + .writeStartObject() + .writeFieldName("DateLessThan") + .writeStartObject() + .writeFieldName("AWS:EpochTime") + .writeValue(expirationDate.getEpochSecond()) + .writeEndObject() + .writeEndObject() + .writeEndObject() + .writeEndArray() + .writeEndObject(); + return new String(writer.getBytes(), UTF_8); } /** @@ -75,24 +94,72 @@ public static String buildCannedPolicy(String resourceUrl, Instant expirationDat * >Setting signed cookies using a custom policy. */ public static String buildCustomPolicy(String resourceUrl, Instant activeDate, Instant expirationDate, - String ipAddress) { - return "{\"Statement\": [{" - + "\"Resource\":\"" - + resourceUrl - + "\"" - + ",\"Condition\":{" - + "\"DateLessThan\":{\"AWS:EpochTime\":" - + expirationDate.getEpochSecond() - + "}" - + (ipAddress == null - ? "" - : ",\"IpAddress\":{\"AWS:SourceIp\":\"" + ipAddress + "\"}" - ) - + (activeDate == null - ? "" - : ",\"DateGreaterThan\":{\"AWS:EpochTime\":" + activeDate.getEpochSecond() + "}" - ) - + "}}]}"; + String ipAddress) { + Validate.notNull(resourceUrl, "resourceUrl must not be null"); + Validate.notNull(expirationDate, "expirationDate must not be null"); + validateInput(resourceUrl, "resourceUrl"); + if (ipAddress != null) { + validateInput(ipAddress, "ipAddress"); + } + + JsonWriter writer = JsonWriter.create(); + writer.writeStartObject() + .writeFieldName("Statement") + .writeStartArray() + .writeStartObject() + .writeFieldName("Resource") + .writeValue(resourceUrl) + .writeFieldName("Condition") + .writeStartObject() + .writeFieldName("DateLessThan") + .writeStartObject() + .writeFieldName("AWS:EpochTime") + .writeValue(expirationDate.getEpochSecond()) + .writeEndObject(); + + if (ipAddress != null) { + writer.writeFieldName("IpAddress") + .writeStartObject() + .writeFieldName("AWS:SourceIp") + .writeValue(ipAddress) + .writeEndObject(); + } + + if (activeDate != null) { + writer.writeFieldName("DateGreaterThan") + .writeStartObject() + .writeFieldName("AWS:EpochTime") + .writeValue(activeDate.getEpochSecond()) + .writeEndObject(); + } + + writer.writeEndObject() + .writeEndObject() + .writeEndArray() + .writeEndObject(); + + return new String(writer.getBytes(), UTF_8); + } + + /** + * Validates that the input does not contain characters that could be used for JSON injection attacks. + * Double quotes, backslashes, and control characters should never appear in valid CloudFront resource URLs + * or IP addresses. + * + * @param input the input string to validate + * @param paramName the parameter name for error messages + * @throws IllegalArgumentException if the input contains invalid characters + */ + private static void validateInput(String input, String paramName) { + for (int i = 0; i < input.length(); i++) { + char c = input.charAt(i); + if (c == '"' || c == '\\' || Character.isISOControl(c)) { + throw new IllegalArgumentException( + paramName + " contains invalid characters. The character '" + c + "' at position " + i + + " is not allowed. URLs and IP addresses should be properly encoded and must not contain " + + "double quotes, backslashes, or control characters."); + } + } } /** diff --git a/services/cloudfront/src/test/java/software/amazon/awssdk/services/cloudfront/CloudFrontUtilitiesTest.java b/services/cloudfront/src/test/java/software/amazon/awssdk/services/cloudfront/CloudFrontUtilitiesTest.java index a07e26d1e0ce..8e69a92597a2 100644 --- a/services/cloudfront/src/test/java/software/amazon/awssdk/services/cloudfront/CloudFrontUtilitiesTest.java +++ b/services/cloudfront/src/test/java/software/amazon/awssdk/services/cloudfront/CloudFrontUtilitiesTest.java @@ -429,7 +429,7 @@ void getSignedURLWithCustomPolicy_policyResourceUrlShouldHandleVariousPatterns( StringJoiner conditions = new StringJoiner(",", "{", "}"); conditions.add("\"DateLessThan\":{\"AWS:EpochTime\":" + expiration.getEpochSecond() + "}"); - expectedPolicy.append("{\"Statement\": [{") + expectedPolicy.append("{\"Statement\":[{") .append("\"Resource\":\"").append(expectedResource).append("\",") .append("\"Condition\":").append(conditions) .append("}]}"); diff --git a/services/cloudfront/src/test/java/software/amazon/awssdk/services/cloudfront/internal/utils/SigningUtilsTest.java b/services/cloudfront/src/test/java/software/amazon/awssdk/services/cloudfront/internal/utils/SigningUtilsTest.java new file mode 100644 index 000000000000..c6c9011a492d --- /dev/null +++ b/services/cloudfront/src/test/java/software/amazon/awssdk/services/cloudfront/internal/utils/SigningUtilsTest.java @@ -0,0 +1,176 @@ +/* + * 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.services.cloudfront.internal.utils; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.time.Instant; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +class SigningUtilsTest { + + private static final Instant EXPIRATION = Instant.ofEpochSecond(1704067200); + private static final Instant ACTIVE_DATE = Instant.ofEpochSecond(1640995200); + private static final String VALID_URL = "https://d111111abcdef8.cloudfront.net/s3ObjectKey"; + private static final String VALID_IP = "192.168.1.0/24"; + + @Test + void buildCannedPolicy_withValidUrl_producesValidJson() { + String policy = SigningUtils.buildCannedPolicy(VALID_URL, EXPIRATION); + + assertThat(policy).contains("\"Resource\":\"" + VALID_URL + "\""); + assertThat(policy).contains("\"AWS:EpochTime\":" + EXPIRATION.getEpochSecond()); + // Verify it's valid JSON structure + assertThat(policy).startsWith("{"); + assertThat(policy).endsWith("}"); + } + + @Test + void buildCustomPolicy_withAllParameters_producesValidJson() { + String policy = SigningUtils.buildCustomPolicy(VALID_URL, ACTIVE_DATE, EXPIRATION, VALID_IP); + + assertThat(policy).contains("\"Resource\":\"" + VALID_URL + "\""); + assertThat(policy).contains("\"DateLessThan\""); + assertThat(policy).contains("\"DateGreaterThan\""); + assertThat(policy).contains("\"IpAddress\""); + assertThat(policy).contains("\"AWS:SourceIp\":\"" + VALID_IP + "\""); + } + + + @Test + void buildCannedPolicy_withDoubleQuoteInUrl_shouldRejectInput() { + String maliciousUrl = "https://example.com/file\",\"Resource\":\"*\",\"x\":\""; + + assertThatThrownBy(() -> SigningUtils.buildCannedPolicy(maliciousUrl, EXPIRATION)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("contains invalid characters") + .hasMessageContaining("resourceUrl"); + } + + @Test + void buildCannedPolicy_withBackslashInUrl_shouldRejectInput() { + String maliciousUrl = "https://example.com/file\\"; + + assertThatThrownBy(() -> SigningUtils.buildCannedPolicy(maliciousUrl, EXPIRATION)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("contains invalid characters"); + } + + @ParameterizedTest + @ValueSource(strings = { + "https://example.com/file\u0000", // null character + "https://example.com/file\n", // newline + "https://example.com/file\r", // carriage return + "https://example.com/file\t" // tab + }) + void buildCannedPolicy_withControlCharactersInUrl_shouldRejectInput(String maliciousUrl) { + assertThatThrownBy(() -> SigningUtils.buildCannedPolicy(maliciousUrl, EXPIRATION)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("contains invalid characters"); + } + + @Test + void buildCustomPolicy_withDoubleQuoteInUrl_shouldRejectInput() { + String maliciousUrl = "https://example.com/file\""; + + assertThatThrownBy(() -> SigningUtils.buildCustomPolicy(maliciousUrl, ACTIVE_DATE, EXPIRATION, VALID_IP)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("resourceUrl"); + } + + @Test + void buildCustomPolicy_withDoubleQuoteInIpAddress_shouldRejectInput() { + String maliciousIp = "192.168.1.0\",\"Resource\":\"*"; + + assertThatThrownBy(() -> SigningUtils.buildCustomPolicy(VALID_URL, ACTIVE_DATE, EXPIRATION, maliciousIp)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("ipAddress"); + } + + @Test + void buildCustomPolicyForSignedUrl_withDoubleQuoteInUrl_shouldRejectInput() { + String maliciousUrl = "https://example.com/file\""; + + assertThatThrownBy(() -> SigningUtils.buildCustomPolicyForSignedUrl(maliciousUrl, ACTIVE_DATE, EXPIRATION, VALID_IP)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("contains invalid characters"); + } + + @Test + void buildCustomPolicyForSignedUrl_withDoubleQuoteInIpRange_shouldRejectInput() { + String maliciousIp = "192.168.1.0\""; + + assertThatThrownBy(() -> SigningUtils.buildCustomPolicyForSignedUrl(VALID_URL, ACTIVE_DATE, EXPIRATION, maliciousIp)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("contains invalid characters"); + } + + // Null parameter validation tests + + @Test + void buildCannedPolicy_withNullUrl_shouldThrowNullPointerException() { + assertThatThrownBy(() -> SigningUtils.buildCannedPolicy(null, EXPIRATION)) + .isInstanceOf(NullPointerException.class) + .hasMessageContaining("resourceUrl"); + } + + @Test + void buildCannedPolicy_withNullExpiration_shouldThrowNullPointerException() { + assertThatThrownBy(() -> SigningUtils.buildCannedPolicy(VALID_URL, null)) + .isInstanceOf(NullPointerException.class) + .hasMessageContaining("expirationDate"); + } + + @Test + void buildCustomPolicy_withNullUrl_shouldThrowNullPointerException() { + assertThatThrownBy(() -> SigningUtils.buildCustomPolicy(null, ACTIVE_DATE, EXPIRATION, VALID_IP)) + .isInstanceOf(NullPointerException.class) + .hasMessageContaining("resourceUrl"); + } + + @Test + void buildCustomPolicy_withNullExpiration_shouldThrowNullPointerException() { + assertThatThrownBy(() -> SigningUtils.buildCustomPolicy(VALID_URL, ACTIVE_DATE, null, VALID_IP)) + .isInstanceOf(NullPointerException.class) + .hasMessageContaining("expirationDate"); + } + + // Valid edge cases that should still work + + @Test + void buildCannedPolicy_withWildcard_shouldSucceed() { + String policy = SigningUtils.buildCannedPolicy("*", EXPIRATION); + assertThat(policy).contains("\"Resource\":\"*\""); + } + + @Test + void buildCannedPolicy_withWildcardInPath_shouldSucceed() { + String url = "https://d111111abcdef8.cloudfront.net/*"; + String policy = SigningUtils.buildCannedPolicy(url, EXPIRATION); + assertThat(policy).contains("\"Resource\":\"" + url + "\""); + } + + @Test + void buildCannedPolicy_withQueryParameters_shouldSucceed() { + String url = "https://d111111abcdef8.cloudfront.net/file?param=value&other=123"; + String policy = SigningUtils.buildCannedPolicy(url, EXPIRATION); + assertThat(policy).contains("\"Resource\":\"" + url + "\""); + } + +}