HDDS-15984. [STS] Improve s3:prefix Condition handling and reject unsupported AssumeRole parameters - #10875
HDDS-15984. [STS] Improve s3:prefix Condition handling and reject unsupported AssumeRole parameters#10875fmorg-git wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens validation in the S3 STS (AssumeRole) surface by rejecting previously-ignored unsupported parameters, and improves IAM session policy parsing to reject invalid s3:prefix condition shapes instead of silently accepting/ignoring them.
Changes:
- Add explicit validation for AssumeRole parameter names (GET query params and POST form params), distinguishing “AWS-valid but not implemented” vs “unsupported”.
- Update the STS POST endpoint to consume a form payload as a single entity and validate only body parameters (ignoring query string params for POST).
- Reject IAM session policies that specify multiple Condition keys per operator (e.g.,
StringEqualscontaining boths3:prefixand another key).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3sts/TestS3STSEndpoint.java | Adds unit tests covering new AssumeRole parameter validation behavior for GET and POST. |
| hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3sts/S3STSEndpointBase.java | Adds a safe helper to access query parameters from the request context. |
| hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3sts/S3STSEndpoint.java | Implements AssumeRole parameter validation (allowed vs unsupported vs not-implemented AWS params) and updates POST handling to consume Form. |
| hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/security/acl/iam/TestIamSessionPolicyResolver.java | Adds test cases ensuring multiple Condition keys (including ordering variations) are rejected. |
| hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/iam/IamSessionPolicyResolver.java | Enforces “exactly one Condition key per operator” in s3:prefix condition parsing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static boolean isAllowedAssumeRoleParameter(String paramName) { | ||
| return StringUtils.isBlank(paramName) | ||
| || ASSUME_ROLE_ALLOWED_PARAMS.contains(paramName) | ||
| || Strings.CI.startsWith(paramName, SIGV4_PARAM_PREFIX); | ||
| } |
| @FormParam("Policy") String awsIamSessionPolicy) throws OS3Exception { | ||
|
|
||
| return handleSTSRequest(action, roleArn, roleSessionName, durationSeconds, version, awsIamSessionPolicy); | ||
| public Response post(Form form) throws OS3Exception { |
There was a problem hiding this comment.
If form is null, can we just response with error directly?
| * @return Response containing STS response XML or error | ||
| */ | ||
| @POST | ||
| @Consumes(MediaType.APPLICATION_FORM_URLENCODED) |
There was a problem hiding this comment.
It looks like AWS STS support ”application/x-www-form-urlencoded“ and “application/x-amz-json-1.1”, we only support the first one?
There was a problem hiding this comment.
per my investigation, AWS STS and IAM strictly use the AWS Query Protocol (which has application/x-www-form-urlencoded as the content type). Only newer services like Amazon Cognito, Amazon DynamoDB, Amazon SQS, etc support AWS JSON RPC 1.1. (not sure if you saw SQS and thought it was STS).
Please describe your PR in detail:
Separately, currently unsupported AssumeRole api parameters are being ignored rather than being rejected. This ticket will improve that as well.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15984
How was this patch tested?
unit tests and smoke tests