Skip to content

HDDS-15984. [STS] Improve s3:prefix Condition handling and reject unsupported AssumeRole parameters #10875

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
fmorg-git wants to merge 2 commits into
base: HDDS-13323-sts
Choose a base branch
from
Open

HDDS-15984. [STS] Improve s3:prefix Condition handling and reject unsupported AssumeRole parameters #10875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e560f0
improve s3:prefix Condition handling
Jul 26, 2026
dea43c1
reject unsupported AssumeRole parameters
Jul 26, 2026
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
7 changes: 6 additions & 1 deletion ...mmon/src/main/java/org/apache/hadoop/ozone/security/acl/iam/IamSessionPolicyResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,12 @@ private static Condition parsePrefixesFromConditions(JsonNode stmt) throws OMExc
ERROR_PREFIX + "Invalid Condition operator value structure - " + operatorValue, MALFORMED_POLICY_DOCUMENT);
}

final String keyName = operatorValue.fieldNames().hasNext() ? operatorValue.fieldNames().next() : null;
if (operatorValue.size() != 1) {
throw new OMException(
ERROR_PREFIX + "Only one Condition key is supported per operator", NOT_SUPPORTED_OPERATION);
}

final String keyName = operatorValue.fieldNames().next();
if (!"s3:prefix".equalsIgnoreCase(keyName)) {
throw new OMException(ERROR_PREFIX + "Unsupported Condition key name - " + keyName, NOT_SUPPORTED_OPERATION);
}
Expand Down
30 changes: 30 additions & 0 deletions .../src/test/java/org/apache/hadoop/ozone/security/acl/iam/TestIamSessionPolicyResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,36 @@ public void testUnsupportedConditionAttributeThrows() {
json, "IAM session policy: Unsupported Condition key name - aws:SourceArn", NOT_SUPPORTED_OPERATION);
}

@Test
public void testMultipleConditionKeysWithPrefixFirstThrows() {
final String json = "{\n" +
" \"Statement\": [{\n" +
" \"Effect\": \"Allow\",\n" +
" \"Action\": \"s3:ListBucket\",\n" +
" \"Resource\": \"arn:aws:s3:::b\",\n" +
" \"Condition\": { \"StringEquals\": { \"s3:prefix\": \"x\", \"aws:SourceIp\": \"1.2.3.4\" } }\n" +
" }]\n" +
"}";

expectResolveThrowsForBothAuthorizers(
json, "IAM session policy: Only one Condition key is supported per operator", NOT_SUPPORTED_OPERATION);
}

@Test
public void testMultipleConditionKeysWithUnsupportedKeyFirstThrows() {
final String json = "{\n" +
" \"Statement\": [{\n" +
" \"Effect\": \"Allow\",\n" +
" \"Action\": \"s3:ListBucket\",\n" +
" \"Resource\": \"arn:aws:s3:::b\",\n" +
" \"Condition\": { \"StringEquals\": { \"aws:SourceIp\": \"1.2.3.4\", \"s3:prefix\": \"x\" } }\n" +
" }]\n" +
"}";

expectResolveThrowsForBothAuthorizers(
json, "IAM session policy: Only one Condition key is supported per operator", NOT_SUPPORTED_OPERATION);
}

@Test
public void testUnsupportedEffectThrows() {
final String json = "{\n" +
Expand Down
161 changes: 140 additions & 21 deletions hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3sts/S3STSEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,34 @@
import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.STS_VALIDATION_ERROR;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
import java.io.IOException;
import java.io.StringWriter;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import javax.inject.Inject;
import javax.ws.rs.FormParam;
import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.core.Form;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
import javax.xml.bind.JAXBContext;
import javax.xml.bind.JAXBException;
import javax.xml.bind.Marshaller;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.Strings;
import org.apache.hadoop.ozone.audit.S3GAction;
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.AssumeRoleResponseInfo;
Expand Down Expand Up @@ -88,6 +95,19 @@ public class S3STSEndpoint extends S3STSEndpointBase {

private static final String EXPECTED_VERSION = "2011-06-15";

private static final String SIGV4_PARAM_PREFIX = "X-Amz-";

private static final Set<String> ASSUME_ROLE_ALLOWED_PARAMS = ImmutableSet.of(
"Action", "RoleArn", "RoleSessionName", "DurationSeconds", "Version", "Policy");

private static final String POLICY_ARNS_MEMBER_PREFIX = "PolicyArns.member.";
private static final String PROVIDED_CONTEXTS_MEMBER_PREFIX = "ProvidedContexts.member.";
private static final String TAGS_MEMBER_PREFIX = "Tags.member.";
private static final String TRANSITIVE_TAG_KEYS_MEMBER_PREFIX = "TransitiveTagKeys.member.";

private static final Set<String> AWS_VALID_ASSUME_ROLE_OPTIONAL_PARAMS = ImmutableSet.of(
"ExternalId", "SerialNumber", "SourceIdentity", "TokenCode");

// JAXBContext is relatively expensive to create and is threadsafe, so cache and reuse
private static final JAXBContext JAXB_CONTEXT;

Expand Down Expand Up @@ -128,35 +148,37 @@ public Response get(
@QueryParam("Version") String version,
@QueryParam("Policy") String awsIamSessionPolicy) throws OS3Exception {

return handleSTSRequest(action, roleArn, roleSessionName, durationSeconds, version, awsIamSessionPolicy);
return handleSTSRequest(
getQueryParameters().keySet(), action, roleArn, roleSessionName, durationSeconds, version, awsIamSessionPolicy);
}

/**
* STS endpoint that handles POST requests with form data.
* AWS STS typically uses POST requests with form-encoded parameters.
*
* @param action The STS action to perform
* @param roleArn The ARN of the role to assume
* @param roleSessionName Session name for the role
* @param durationSeconds Duration of the token validity
* @param version AWS STS API version
* @param form form-encoded request parameters
* @return Response containing STS response XML or error
*/
@POST
@Consumes(MediaType.APPLICATION_FORM_URLENCODED)

@ChenSammi ChenSammi Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like AWS STS support ”application/x-www-form-urlencoded“ and “application/x-amz-json-1.1”, we only support the first one?

@Produces(MediaType.APPLICATION_XML)
public Response post(
@FormParam("Action") String action,
@FormParam("RoleArn") String roleArn,
@FormParam("RoleSessionName") String roleSessionName,
@FormParam("DurationSeconds") Integer durationSeconds,
@FormParam("Version") String version,
@FormParam("Policy") String awsIamSessionPolicy) throws OS3Exception {

return handleSTSRequest(action, roleArn, roleSessionName, durationSeconds, version, awsIamSessionPolicy);
public Response post(Form form) throws OS3Exception {

@ChenSammi ChenSammi Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If form is null, can we just response with error directly?

final MultivaluedMap<String, String> formParams = form == null ? null : form.asMap();
final String action = formParams == null ? null : formParams.getFirst("Action");
final String roleArn = formParams == null ? null : formParams.getFirst("RoleArn");
final String roleSessionName = formParams == null ? null : formParams.getFirst("RoleSessionName");
final Integer durationSeconds =
parseIntegerOrNull(formParams == null ? null : formParams.getFirst("DurationSeconds"));
final String version = formParams == null ? null : formParams.getFirst("Version");
final String awsIamSessionPolicy = formParams == null ? null : formParams.getFirst("Policy");

final Set<String> formParamNames = formParams == null ? Collections.emptySet() : formParams.keySet();
return handleSTSRequest(
formParamNames, action, roleArn, roleSessionName, durationSeconds, version, awsIamSessionPolicy);
}

private Response handleSTSRequest(String action, String roleArn, String roleSessionName,
Integer durationSeconds, String version, String awsIamSessionPolicy) throws OS3Exception {
private Response handleSTSRequest(Set<String> paramNamesToValidate, String action, String roleArn,
String roleSessionName, Integer durationSeconds, String version, String awsIamSessionPolicy) throws OS3Exception {
final String requestId = requestIdentifier.getRequestId();
// NOTE: invalid, missing or unsupported actions are not added to the audit log
try {
Expand All @@ -170,7 +192,8 @@ private Response handleSTSRequest(String action, String roleArn, String roleSess

switch (action) {
case ASSUME_ROLE_ACTION:
return handleAssumeRole(roleArn, roleSessionName, durationSeconds, awsIamSessionPolicy, version, requestId);
return handleAssumeRole(
paramNamesToValidate, roleArn, roleSessionName, durationSeconds, awsIamSessionPolicy, version, requestId);
// These operations are not supported yet
case GET_SESSION_TOKEN_ACTION:
case ASSUME_ROLE_WITH_SAML_ACTION:
Expand All @@ -193,8 +216,8 @@ private Response handleSTSRequest(String action, String roleArn, String roleSess
}
}

private Response handleAssumeRole(String roleArn, String roleSessionName, Integer durationSeconds,
String awsIamSessionPolicy, String version, String requestId) throws OSTSException {
private Response handleAssumeRole(Set<String> paramNamesToValidate, String roleArn, String roleSessionName,
Integer durationSeconds, String awsIamSessionPolicy, String version, String requestId) throws OSTSException {
final String action = "AssumeRole";
final Map<String, String> auditParams = getAuditParameters();
S3STSUtils.addAssumeRoleAuditParams(
Expand All @@ -211,6 +234,16 @@ private Response handleAssumeRole(String roleArn, String roleSessionName, Intege
throw exception;
}

final AssumeRoleParamValidationResult assumeRoleParamValidationResult = validateAssumeRoleParameters(
paramNamesToValidate);
if (!assumeRoleParamValidationResult.getNotImplementedOptionalParams().isEmpty()) {
final OSTSException exception = new OSTSException(STS_UNSUPPORTED_OPERATION).withMessage(
"AssumeRole optional parameter(s) not implemented: " +
String.join(", ", assumeRoleParamValidationResult.getNotImplementedOptionalParams()));
getAuditLogger().logWriteFailure(buildAuditMessageForFailure(S3GAction.ASSUME_ROLE, auditParams, exception));
throw exception;
}

final Set<String> validationErrors = new HashSet<>();
int duration = durationSeconds == null ? S3STSUtils.DEFAULT_DURATION_SECONDS : durationSeconds;
try {
Expand Down Expand Up @@ -242,6 +275,11 @@ private Response handleAssumeRole(String roleArn, String roleSessionName, Intege
validationErrors.add(e.getMessage());
}

if (!assumeRoleParamValidationResult.getUnsupportedParams().isEmpty()) {
validationErrors.add("Unsupported AssumeRole parameter(s): " + String.join(", ",
assumeRoleParamValidationResult.getUnsupportedParams()));
}

final int numValidationErrors = validationErrors.size();
if (numValidationErrors > 0) {
//noinspection StringBufferReplaceableByString
Expand Down Expand Up @@ -303,6 +341,87 @@ private Response handleAssumeRole(String roleArn, String roleSessionName, Intege
}
}

private AssumeRoleParamValidationResult validateAssumeRoleParameters(Set<String> paramNamesToValidate) {
if (paramNamesToValidate == null || paramNamesToValidate.isEmpty()) {
return AssumeRoleParamValidationResult.empty();
}

final List<String> notImplementedOptionalParams = new ArrayList<>();
final List<String> unsupportedParams = new ArrayList<>();
for (String paramName : paramNamesToValidate) {
if (isAllowedAssumeRoleParameter(paramName)) {
continue;
}

if (isAwsValidButNotImplementedAssumeRoleParameter(paramName)) {
notImplementedOptionalParams.add(paramName);
} else {
unsupportedParams.add(paramName);
}
}

Collections.sort(notImplementedOptionalParams);
Collections.sort(unsupportedParams);
return new AssumeRoleParamValidationResult(notImplementedOptionalParams, unsupportedParams);
}

private static boolean isAllowedAssumeRoleParameter(String paramName) {
return StringUtils.isBlank(paramName)
|| ASSUME_ROLE_ALLOWED_PARAMS.contains(paramName)
|| Strings.CI.startsWith(paramName, SIGV4_PARAM_PREFIX);
}
Comment on lines +368 to +372

@ChenSammi ChenSammi Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1


private static boolean isAwsValidButNotImplementedAssumeRoleParameter(String paramName) {
if (StringUtils.isBlank(paramName)) {
return false;
}
if (AWS_VALID_ASSUME_ROLE_OPTIONAL_PARAMS.contains(paramName)) {
return true;
}

return Strings.CI.startsWith(paramName, POLICY_ARNS_MEMBER_PREFIX)
|| Strings.CI.startsWith(paramName, PROVIDED_CONTEXTS_MEMBER_PREFIX)
|| Strings.CI.startsWith(paramName, TAGS_MEMBER_PREFIX)
|| Strings.CI.startsWith(paramName, TRANSITIVE_TAG_KEYS_MEMBER_PREFIX);
}

private static Integer parseIntegerOrNull(String value) throws OSTSException {
if (StringUtils.isBlank(value)) {
return null;
}
try {
return Integer.parseInt(value);
} catch (NumberFormatException e) {
throw new OSTSException(STS_VALIDATION_ERROR)
.withMessage("1 validation error detected: Invalid Value: DurationSeconds must be a number");
}
}

private static final class AssumeRoleParamValidationResult {
private static final AssumeRoleParamValidationResult EMPTY = new AssumeRoleParamValidationResult(
Collections.emptyList(), Collections.emptyList());

private final List<String> notImplementedOptionalParams;
private final List<String> unsupportedParams;

private AssumeRoleParamValidationResult(List<String> notImplementedOptionalParams, List<String> unsupportedParams) {
this.notImplementedOptionalParams = notImplementedOptionalParams;
this.unsupportedParams = unsupportedParams;
}

private static AssumeRoleParamValidationResult empty() {
return EMPTY;
}

private List<String> getNotImplementedOptionalParams() {
return notImplementedOptionalParams;
}

private List<String> getUnsupportedParams() {
return unsupportedParams;
}
}

private String generateAssumeRoleResponse(String assumedRoleUserArn, AssumeRoleResponseInfo responseInfo,
String requestId) throws IOException {
final String accessKeyId = responseInfo.getAccessKeyId();
Expand Down
10 changes: 10 additions & 0 deletions hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3sts/S3STSEndpointBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import javax.inject.Inject;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MultivaluedHashMap;
import javax.ws.rs.core.MultivaluedMap;
import org.apache.hadoop.ozone.audit.AuditAction;
import org.apache.hadoop.ozone.audit.AuditEventStatus;
import org.apache.hadoop.ozone.audit.AuditLogger;
Expand Down Expand Up @@ -133,4 +135,12 @@ public void setSignatureInfo(SignatureInfo signatureInfo) {
protected Map<String, String> getAuditParameters() {
return AuditUtils.getAuditParameters(context);
}

protected MultivaluedMap<String, String> getQueryParameters() {
if (context == null || context.getUriInfo() == null) {
return new MultivaluedHashMap<>();
}
final MultivaluedMap<String, String> params = context.getUriInfo().getQueryParameters();
return params == null ? new MultivaluedHashMap<>() : params;
}
}
Loading