Skip to content

fix(s3): enforce presigned POST policy conditions (eq, starts-with, content-type) #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Meemaw wants to merge 1 commit into
base: main
Choose a base branch
from
+225 −34
Open

fix(s3): enforce presigned POST policy conditions (eq, starts-with, content-type) #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 81 additions & 30 deletions src/main/java/io/github/hectorvent/floci/services/s3/S3Controller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,16 @@
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Arrays;
import java.util.Optional;
import java.util.Set;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.jboss.logging.Logger;

/**
Expand All @@ -56,6 +58,8 @@ public class S3Controller {
.ofPattern("EEE, dd MMM yyyy HH:mm:ss z", Locale.US)
.withZone(ZoneId.of("GMT"));

private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();

private final S3Service s3Service;
private final S3SelectService s3SelectService;
private final RegionResolver regionResolver;
Expand Down Expand Up @@ -1555,10 +1559,10 @@ private Response handlePresignedPost(String bucket, String contentType, byte[] b
"Bucket POST must contain a file field.", 400);
}

// Validate content-length-range from policy if present
// Validate policy conditions if present
String policy = fields.get("policy");
if (policy != null && !policy.isEmpty()) {
validateContentLengthRange(policy, fileData.length);
validatePolicyConditions(policy, bucket, fields, fileData.length);
}

// Use Content-Type from form fields, fall back to file part Content-Type
Expand Down Expand Up @@ -1588,45 +1592,92 @@ private Response handlePresignedPost(String bucket, String contentType, byte[] b
.build();
}

private void validateContentLengthRange(String policyBase64, int contentLength) {
private void validatePolicyConditions(String policyBase64, String bucket,
Map<String, String> fields, int contentLength) {
try {
String decoded = new String(java.util.Base64.getDecoder().decode(policyBase64), StandardCharsets.UTF_8);
// Parse conditions array from the policy JSON to find content-length-range
int condIdx = decoded.indexOf("\"conditions\"");
if (condIdx < 0) {
return;
}
// Look for content-length-range condition: ["content-length-range", min, max]
String lower = decoded.toLowerCase(Locale.ROOT);
int rangeIdx = lower.indexOf("content-length-range");
if (rangeIdx < 0) {
byte[] decoded = java.util.Base64.getDecoder().decode(policyBase64);
JsonNode policy = OBJECT_MAPPER.readTree(decoded);
JsonNode conditions = policy.get("conditions");
if (conditions == null || !conditions.isArray()) {
return;
}
// Find the enclosing array bracket
int bracketStart = decoded.lastIndexOf('[', rangeIdx);
int bracketEnd = decoded.indexOf(']', rangeIdx);
if (bracketStart < 0 || bracketEnd < 0) {
return;
}
String rangeArray = decoded.substring(bracketStart, bracketEnd + 1);
// Extract min and max values
String[] tokens = rangeArray.replaceAll("[\\[\\]\"]", "").split(",");
if (tokens.length >= 3) {
long min = Long.parseLong(tokens[1].trim());
long max = Long.parseLong(tokens[2].trim());
if (contentLength < min || contentLength > max) {
throw new AwsException("EntityTooLarge",
"Your proposed upload exceeds the maximum allowed size.", 400);
for (JsonNode condition : conditions) {
if (condition.isObject()) {
validateExactMatchCondition(condition, bucket, fields);
} else if (condition.isArray()) {
validateArrayCondition(condition, bucket, fields, contentLength);
}
}
} catch (AwsException e) {
throw e;
} catch (Exception e) {
// If policy parsing fails, skip validation (match AWS lenient behavior for emulator)
LOG.debugv("Failed to parse presigned POST policy: {0}", e.getMessage());
}
}

private void validateExactMatchCondition(JsonNode condition, String bucket, Map<String, String> fields) {
Iterator<Map.Entry<String, JsonNode>> fieldIter = condition.fields();
while (fieldIter.hasNext()) {
Map.Entry<String, JsonNode> entry = fieldIter.next();
String fieldName = entry.getKey();
String expectedValue = entry.getValue().asText();
String actualValue;
if ("bucket".equals(fieldName)) {
actualValue = bucket;
} else {
actualValue = fields.get(fieldName);
}
if (actualValue == null || !actualValue.equals(expectedValue)) {
throw new AwsException("AccessDenied",
"Invalid according to Policy: Policy Condition failed: "
+ "[\"eq\", \"$" + fieldName + "\", \"" + expectedValue + "\"]", 403);
}
}
}

private void validateArrayCondition(JsonNode condition, String bucket,
Map<String, String> fields, int contentLength) {
if (condition.size() < 3) {
return;
}
String operator = condition.get(0).asText().toLowerCase(Locale.ROOT);
if ("content-length-range".equals(operator)) {
long min = condition.get(1).asLong();
long max = condition.get(2).asLong();
if (contentLength < min || contentLength > max) {
throw new AwsException("EntityTooLarge",
"Your proposed upload exceeds the maximum allowed size.", 400);
}
} else if ("eq".equals(operator)) {
String fieldRef = condition.get(1).asText();
String expectedValue = condition.get(2).asText();
String fieldName = fieldRef.startsWith("$") ? fieldRef.substring(1) : fieldRef;
String actualValue = resolveFieldValue(fieldName, bucket, fields);
if (actualValue == null || !actualValue.equals(expectedValue)) {
throw new AwsException("AccessDenied",
"Invalid according to Policy: Policy Condition failed: "
+ "[\"eq\", \"$" + fieldName + "\", \"" + expectedValue + "\"]", 403);
}
} else if ("starts-with".equals(operator)) {
String fieldRef = condition.get(1).asText();
String prefix = condition.get(2).asText();
String fieldName = fieldRef.startsWith("$") ? fieldRef.substring(1) : fieldRef;
String actualValue = resolveFieldValue(fieldName, bucket, fields);
if (actualValue == null || !actualValue.startsWith(prefix)) {
throw new AwsException("AccessDenied",
"Invalid according to Policy: Policy Condition failed: "
+ "[\"starts-with\", \"$" + fieldName + "\", \"" + prefix + "\"]", 403);
}
}
}

private static String resolveFieldValue(String fieldName, String bucket, Map<String, String> fields) {
if ("bucket".equals(fieldName)) {
return bucket;
}
return fields.get(fieldName);
}

private static String extractBoundary(String contentType) {
if (contentType == null) {
return null;
Expand Down
148 changes: 144 additions & 4 deletions src/test/java/io/github/hectorvent/floci/services/s3/S3PresignedPostIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ void presignedPostRejectsExceedingContentLength() {
.post("/" + BUCKET)
.then()
.statusCode(400)
.body(containsString("EntityTooLarge"));
.contentType("application/xml")
.body(hasXPath("/Error/Code", equalTo("EntityTooLarge")))
.body(hasXPath("/Error/Message", equalTo(
"Your proposed upload exceeds the maximum allowed size.")));
}

@Test
Expand All @@ -143,7 +146,10 @@ void presignedPostRequiresKeyField() {
.post("/" + BUCKET)
.then()
.statusCode(400)
.body(containsString("InvalidArgument"));
.contentType("application/xml")
.body(hasXPath("/Error/Code", equalTo("InvalidArgument")))
.body(hasXPath("/Error/Message", equalTo(
"Bucket POST must contain a field named 'key'.")));
}

@Test
Expand All @@ -157,7 +163,10 @@ void presignedPostRequiresFileField() {
.post("/" + BUCKET)
.then()
.statusCode(400)
.body(containsString("InvalidArgument"));
.contentType("application/xml")
.body(hasXPath("/Error/Code", equalTo("InvalidArgument")))
.body(hasXPath("/Error/Message", equalTo(
"Bucket POST must contain a file field.")));
}

@Test
Expand Down Expand Up @@ -227,7 +236,8 @@ void presignedPostToNonExistentBucketFails() {
.post("/nonexistent-presigned-bucket")
.then()
.statusCode(404)
.body(containsString("NoSuchBucket"));
.contentType("application/xml")
.body(hasXPath("/Error/Code", equalTo("NoSuchBucket")));
}

@Test
Expand Down Expand Up @@ -255,6 +265,117 @@ void presignedPostWithContentLengthWithinRange() {
.statusCode(204);
}

@Test
@Order(91)
void presignedPostRejectsContentTypeMismatch() {
String key = "uploads/ct-mismatch.png";
String fileContent = "not a real png";

String policy = buildPolicy(BUCKET, key, "image/png", 0, 10485760);
String policyBase64 = Base64.getEncoder().encodeToString(policy.getBytes(StandardCharsets.UTF_8));

given()
.multiPart("key", key)
.multiPart("Content-Type", "image/gif")
.multiPart("policy", policyBase64)
.multiPart("x-amz-algorithm", "AWS4-HMAC-SHA256")
.multiPart("x-amz-credential", "AKIAIOSFODNN7EXAMPLE/20260330/us-east-1/s3/aws4_request")
.multiPart("x-amz-date", AMZ_DATE_FORMAT.format(Instant.now()))
.multiPart("x-amz-signature", "dummysignature")
.multiPart("file", "ct-mismatch.png", fileContent.getBytes(StandardCharsets.UTF_8), "image/gif")
.when()
.post("/" + BUCKET)
.then()
.statusCode(403)
.contentType("application/xml")
.body(hasXPath("/Error/Code", equalTo("AccessDenied")))
.body(hasXPath("/Error/Message", equalTo(
"Invalid according to Policy: Policy Condition failed: "
+ "[\"eq\", \"$Content-Type\", \"image/png\"]")));
}

@Test
@Order(92)
void presignedPostRejectsKeyMismatch() {
String key = "uploads/wrong-key.txt";
String fileContent = "test content";

String policy = buildPolicy(BUCKET, "uploads/expected-key.txt", "text/plain", 0, 10485760);
String policyBase64 = Base64.getEncoder().encodeToString(policy.getBytes(StandardCharsets.UTF_8));

given()
.multiPart("key", key)
.multiPart("Content-Type", "text/plain")
.multiPart("policy", policyBase64)
.multiPart("x-amz-algorithm", "AWS4-HMAC-SHA256")
.multiPart("x-amz-credential", "AKIAIOSFODNN7EXAMPLE/20260330/us-east-1/s3/aws4_request")
.multiPart("x-amz-date", AMZ_DATE_FORMAT.format(Instant.now()))
.multiPart("x-amz-signature", "dummysignature")
.multiPart("file", "wrong-key.txt", fileContent.getBytes(StandardCharsets.UTF_8), "text/plain")
.when()
.post("/" + BUCKET)
.then()
.statusCode(403)
.contentType("application/xml")
.body(hasXPath("/Error/Code", equalTo("AccessDenied")))
.body(hasXPath("/Error/Message", equalTo(
"Invalid according to Policy: Policy Condition failed: "
+ "[\"eq\", \"$key\", \"uploads/expected-key.txt\"]")));
}

@Test
@Order(93)
void presignedPostWithStartsWithCondition() {
String key = "uploads/prefix-test.txt";
String fileContent = "starts-with test";

String policy = buildStartsWithPolicy(BUCKET, "uploads/", "text/", 0, 10485760);
String policyBase64 = Base64.getEncoder().encodeToString(policy.getBytes(StandardCharsets.UTF_8));

given()
.multiPart("key", key)
.multiPart("Content-Type", "text/plain")
.multiPart("policy", policyBase64)
.multiPart("x-amz-algorithm", "AWS4-HMAC-SHA256")
.multiPart("x-amz-credential", "AKIAIOSFODNN7EXAMPLE/20260330/us-east-1/s3/aws4_request")
.multiPart("x-amz-date", AMZ_DATE_FORMAT.format(Instant.now()))
.multiPart("x-amz-signature", "dummysignature")
.multiPart("file", "prefix-test.txt", fileContent.getBytes(StandardCharsets.UTF_8), "text/plain")
.when()
.post("/" + BUCKET)
.then()
.statusCode(204);
}

@Test
@Order(94)
void presignedPostRejectsStartsWithMismatch() {
String key = "other/wrong-prefix.txt";
String fileContent = "starts-with mismatch";

String policy = buildStartsWithPolicy(BUCKET, "uploads/", "text/", 0, 10485760);
String policyBase64 = Base64.getEncoder().encodeToString(policy.getBytes(StandardCharsets.UTF_8));

given()
.multiPart("key", key)
.multiPart("Content-Type", "text/plain")
.multiPart("policy", policyBase64)
.multiPart("x-amz-algorithm", "AWS4-HMAC-SHA256")
.multiPart("x-amz-credential", "AKIAIOSFODNN7EXAMPLE/20260330/us-east-1/s3/aws4_request")
.multiPart("x-amz-date", AMZ_DATE_FORMAT.format(Instant.now()))
.multiPart("x-amz-signature", "dummysignature")
.multiPart("file", "wrong-prefix.txt", fileContent.getBytes(StandardCharsets.UTF_8), "text/plain")
.when()
.post("/" + BUCKET)
.then()
.statusCode(403)
.contentType("application/xml")
.body(hasXPath("/Error/Code", equalTo("AccessDenied")))
.body(hasXPath("/Error/Message", equalTo(
"Invalid according to Policy: Policy Condition failed: "
+ "[\"starts-with\", \"$key\", \"uploads/\"]")));
}

@Test
@Order(100)
void cleanupBucket() {
Expand All @@ -264,6 +385,7 @@ void cleanupBucket() {
given().delete("/" + BUCKET + "/uploads/no-policy.txt");
given().delete("/" + BUCKET + "/uploads/typed-file.json");
given().delete("/" + BUCKET + "/uploads/within-range.txt");
given().delete("/" + BUCKET + "/uploads/prefix-test.txt");

given()
.when()
Expand All @@ -288,4 +410,22 @@ private String buildPolicy(String bucket, String key, String contentType, long m
}
""".formatted(expiration, bucket, key, contentType, minSize, maxSize);
}

private String buildStartsWithPolicy(String bucket, String keyPrefix, String contentTypePrefix,
long minSize, long maxSize) {
String expiration = Instant.now().plusSeconds(3600)
.atZone(ZoneOffset.UTC)
.format(DateTimeFormatter.ISO_INSTANT);
return """
{
"expiration": "%s",
"conditions": [
{"bucket": "%s"},
["starts-with", "$key", "%s"],
["starts-with", "$Content-Type", "%s"],
["content-length-range", %d, %d]
]
}
""".formatted(expiration, bucket, keyPrefix, contentTypePrefix, minSize, maxSize);
}
}