diff --git a/hadoop-ozone/common/pom.xml b/hadoop-ozone/common/pom.xml index 79039d218d72..75740bd1380e 100644 --- a/hadoop-ozone/common/pom.xml +++ b/hadoop-ozone/common/pom.xml @@ -31,6 +31,10 @@ com.fasterxml.jackson.core jackson-annotations + + com.fasterxml.jackson.core + jackson-core + com.fasterxml.jackson.core jackson-databind diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/iam/IamSessionPolicyResolver.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/iam/IamSessionPolicyResolver.java index 415ae5e8af75..6afe7ebee404 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/iam/IamSessionPolicyResolver.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/iam/IamSessionPolicyResolver.java @@ -29,13 +29,17 @@ import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.WRITE; import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.WRITE_ACL; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import java.io.IOException; import java.util.Collections; import java.util.EnumSet; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Map; @@ -78,6 +82,9 @@ * value is case-sensitive per the * AWS spec. *

+ * The only supported Statement elements are Sid, Effect, Action, Resource, and Condition. Duplicate JSON object keys + * and unsupported Statement elements will throw OMException with MALFORMED_POLICY_DOCUMENT. + *

* If a (currently) unsupported S3 action is requested, such as s3:GetAccelerateConfiguration, * it will be silently ignored. Similarly, if an invalid S3 action is requested, it will be silently ignored. *

@@ -141,14 +148,20 @@ public static Set resolve(String policyJson, Strin final Set statements = parseJsonAndRetrieveStatements(policyJson); for (JsonNode stmt : statements) { + validateSupportedStatementFields(stmt); validateEffectInJsonStatement(stmt); - final Set actions = readStringOrArray(stmt.get("Action")); - final Set resources = readStringOrArray(stmt.get("Resource")); + final Set actions = readRequiredStringOrArray(stmt.get("Action"), "Action"); + final Set resources = readRequiredStringOrArray(stmt.get("Resource"), "Resource"); // Parse prefixes from conditions, if any final Condition condition = parsePrefixesFromConditions(stmt); + // An empty s3:prefix array matches no prefixes and therefore grants no access (AWS behavior). + if (condition != null && condition.prefixes.isEmpty()) { + continue; + } + // Map actions to S3Action enum if possible final Set mappedS3Actions = mapPolicyActionsToS3Actions(actions); if (mappedS3Actions.isEmpty()) { @@ -203,6 +216,10 @@ private static void validateInputParameters(String policyJson, String volumeName * Parses IAM session policy and retrieve the statement(s). */ private static Set parseJsonAndRetrieveStatements(String policyJson) throws OMException { + // Jackson's tree model silently collapses duplicate keys (last value wins), which could let a caller smuggle + // broader permissions than intended. Detect them up front so we can reject them with the exact field name. + checkForDuplicateFields(policyJson); + final JsonNode root; try { root = MAPPER.readTree(policyJson); @@ -223,9 +240,84 @@ private static Set parseJsonAndRetrieveStatements(String policyJson) t } else { statements.add(statementsNode); } + if (statements.isEmpty()) { + throw new OMException(ERROR_PREFIX + "No Statement(s) found in policy", MALFORMED_POLICY_DOCUMENT); + } return statements; } + /** + * Detects duplicate JSON object keys at any nesting level in a single streaming pass, reporting the offending + * field name directly. Structural JSON problems are ignored here and surfaced by the subsequent tree parse. + */ + private static void checkForDuplicateFields(String policyJson) throws OMException { + try (JsonParser parser = MAPPER.getFactory().createParser(policyJson)) { + checkForDuplicateFields(parser); + } catch (OMException e) { + throw e; + } catch (IOException e) { + // Structural JSON problems are surfaced by the subsequent tree parse with a clearer message. + } + } + + private static void checkForDuplicateFields(JsonParser parser) throws IOException { + JsonToken token = parser.currentToken(); + if (token == null) { + token = parser.nextToken(); + } + + if (token == JsonToken.START_OBJECT) { + final Set fieldNames = new HashSet<>(); + while (parser.nextToken() == JsonToken.FIELD_NAME) { + final String fieldName = parser.currentName(); + if (!fieldNames.add(fieldName)) { + throw new OMException( + ERROR_PREFIX + "Duplicate field '" + fieldName + "' in session policy", MALFORMED_POLICY_DOCUMENT); + } + parser.nextToken(); + checkForDuplicateFields(parser); + } + } else if (token == JsonToken.START_ARRAY) { + JsonToken element; + while ((element = parser.nextToken()) != null && element != JsonToken.END_ARRAY) { + checkForDuplicateFields(parser); + } + } + } + + /** + * Ensures statements contain only the IAM policy elements supported by the STS session policy subset. + */ + private static void validateSupportedStatementFields(JsonNode statement) throws OMException { + if (!statement.isObject()) { + throw new OMException( + ERROR_PREFIX + "Invalid Statement in JSON policy (must be an Object) - " + statement, + MALFORMED_POLICY_DOCUMENT); + } + + final Iterator fieldNames = statement.fieldNames(); + while (fieldNames.hasNext()) { + final String fieldName = fieldNames.next(); + if (!isSupportedStatementField(fieldName)) { + throw new OMException( + ERROR_PREFIX + "Unsupported statement element - " + fieldName, MALFORMED_POLICY_DOCUMENT); + } + } + } + + private static boolean isSupportedStatementField(String fieldName) { + switch (fieldName) { + case "Sid": + case "Effect": + case "Action": + case "Resource": + case "Condition": + return true; + default: + return false; + } + } + /** * Parses Effect from IAM session policy and ensures it is valid and supported. */ @@ -248,28 +340,87 @@ private static void validateEffectInJsonStatement(JsonNode statement) throws OME } /** - * Reads a JsonNode and converts to a Set of String, if the node represents - * a textual value or an array of textual values. Otherwise, returns - * an empty List. + * Reads a required String or String array JSON policy element. */ - private static Set readStringOrArray(JsonNode node) { + private static Set readRequiredStringOrArray(JsonNode node, String fieldName) throws OMException { if (node == null || node.isMissingNode() || node.isNull()) { - return Collections.emptySet(); + throw new OMException(ERROR_PREFIX + "No " + fieldName + "(s) found in policy", MALFORMED_POLICY_DOCUMENT); } if (node.isTextual()) { return Collections.singleton(node.asText()); } if (node.isArray()) { final Set set = new HashSet<>(); - node.forEach(n -> { - if (n.isTextual()) { - set.add(n.asText()); + for (JsonNode n : node) { + if (!n.isTextual()) { + throw invalidStringOrArray(fieldName, node); } - }); + set.add(n.asText()); + } + if (set.isEmpty()) { + throw new OMException(ERROR_PREFIX + "No " + fieldName + "(s) found in policy", MALFORMED_POLICY_DOCUMENT); + } return set; } - return Collections.emptySet(); + throw invalidStringOrArray(fieldName, node); + } + + private static OMException invalidStringOrArray(String fieldName, JsonNode node) { + return new OMException( + ERROR_PREFIX + "Invalid " + fieldName + " in JSON policy (must be a String or Array of Strings) - " + node, + MALFORMED_POLICY_DOCUMENT); + } + + /** + * Reads and validates an s3:prefix condition value per AWS IAM session policy behavior. + *

+ * Rejects {@code null}, objects (such as {@code {}}), and arrays whose elements are not all + * strings or not all numbers/booleans. Scalar numbers and booleans are coerced to strings + * (for example {@code 123} becomes {@code "123"}). An empty array matches no prefixes and + * causes the statement to grant no access. + */ + private static Set readConditionPrefixValue(JsonNode node) throws OMException { + if (node == null || node.isMissingNode() || node.isNull()) { + throw invalidConditionPrefixValue(node); + } + if (node.isTextual() || node.isNumber() || node.isBoolean()) { + return Collections.singleton(node.asText()); + } + if (node.isArray()) { + if (node.isEmpty()) { + return Collections.emptySet(); + } + boolean allTextual = true; + boolean allNumber = true; + boolean allBoolean = true; + for (final JsonNode element : node) { + if (!element.isTextual()) { + allTextual = false; + } + if (!element.isNumber()) { + allNumber = false; + } + if (!element.isBoolean()) { + allBoolean = false; + } + } + if (!allTextual && !allNumber && !allBoolean) { + throw invalidConditionPrefixValue(node); + } + final Set prefixes = new HashSet<>(); + node.forEach(n -> prefixes.add(n.asText())); + return prefixes; + } + + throw invalidConditionPrefixValue(node); + } + + private static OMException invalidConditionPrefixValue(JsonNode node) { + return new OMException( + ERROR_PREFIX + "Invalid s3:prefix in Condition (must be a String, Number, Boolean, or homogeneous " + + "Array of Strings, Numbers, or Booleans) - " + node, + MALFORMED_POLICY_DOCUMENT); } /** @@ -319,7 +470,7 @@ private static Condition parsePrefixesFromConditions(JsonNode stmt) throws OMExc throw new OMException(ERROR_PREFIX + "Unsupported Condition key name - " + keyName, NOT_SUPPORTED_OPERATION); } - final Set prefixes = readStringOrArray(operatorValue.get(keyName)); + final Set prefixes = readConditionPrefixValue(operatorValue.get(keyName)); condition = new Condition(operator, prefixes); } @@ -596,23 +747,23 @@ private static void processResourceTypeAny(String volumeName, AuthorizerType aut addAclsForObj(objToAclsMap, volumeObj, action.volumePerms); addAclsForObj(objToAclsMap, bucketObj, action.bucketPerms); - if (condition != null && condition.prefixes != null && !condition.prefixes.isEmpty() && - action == S3Action.LIST_BUCKET) { - + if (condition != null && action == S3Action.LIST_BUCKET) { // Ensure the volume and bucket get the action addActionForKind(objToActionsMap, action, volumeObj, bucketObj, null); - for (String prefix : condition.prefixes) { - // If operator is StringEquals, ignore wildcard prefixes - this is AWS behavior - if (STRING_EQUALS.equals(condition.operator) && hasWildcard(prefix)) { - continue; - } + if (condition.prefixes != null && !condition.prefixes.isEmpty()) { + for (String prefix : condition.prefixes) { + // If operator is StringEquals, ignore wildcard prefixes - this is AWS behavior + if (STRING_EQUALS.equals(condition.operator) && hasWildcard(prefix)) { + continue; + } - final IOzoneObj listObj = createObjectResourcesFromConditionPrefix( - volumeName, authorizerType, ResourceSpec.any(), prefix, objToAclsMap, EnumSet.of(READ)); - addActionForKind(objToActionsMap, action, null, null, listObj); + final IOzoneObj listObj = createObjectResourcesFromConditionPrefix( + volumeName, authorizerType, ResourceSpec.any(), prefix, objToAclsMap, EnumSet.of(READ)); + addActionForKind(objToActionsMap, action, null, null, listObj); + } } - } else { + } else if (condition == null) { addAclsForObj(objToAclsMap, keyObj, action.objectPerms); addActionForKind(objToActionsMap, action, volumeObj, bucketObj, keyObj); } diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/security/acl/iam/TestIamSessionPolicyResolver.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/security/acl/iam/TestIamSessionPolicyResolver.java index 2902aa4fba09..54e3be080eaf 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/security/acl/iam/TestIamSessionPolicyResolver.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/security/acl/iam/TestIamSessionPolicyResolver.java @@ -197,6 +197,17 @@ public void testInvalidJsonWithoutStatementThrows() { json, "IAM session policy: Invalid policy JSON - missing Statement", MALFORMED_POLICY_DOCUMENT); } + @Test + public void testInvalidJsonWithEmptyStatementArrayThrows() { + final String json = "{\n" + + " \"Version\": \"2012-10-17\",\n" + + " \"Statement\": []\n" + + "}"; + + expectResolveThrowsForBothAuthorizers( + json, "IAM session policy: No Statement(s) found in policy", MALFORMED_POLICY_DOCUMENT); + } + @Test public void testInvalidEffectThrows() { final String json = "{\n" + @@ -210,6 +221,18 @@ public void testInvalidEffectThrows() { expectResolveThrowsForBothAuthorizers( json, "IAM session policy: Invalid Effect in JSON policy (must be a String) - [\"Allow\"]", MALFORMED_POLICY_DOCUMENT); + + final String jsonWithNull = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": null,\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket1\"\n" + + " }]\n" + + "}"; + + expectResolveThrowsForBothAuthorizers( + jsonWithNull, "IAM session policy: Invalid Effect in JSON policy (must be a String) - null", + MALFORMED_POLICY_DOCUMENT); } @Test @@ -225,6 +248,265 @@ public void testMissingEffectInStatementThrows() { json, "IAM session policy: Effect is missing from JSON policy", MALFORMED_POLICY_DOCUMENT); } + @Test + public void testDuplicateStatementKeysThrow() { + final String duplicateActionGetThenStar = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Action\": \"s3:*\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket1/*\"\n" + + " }]\n" + + "}"; + final String duplicateActionStarThenGet = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:*\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket1/*\"\n" + + " }]\n" + + "}"; + final String duplicateEffect = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Effect\": \"Deny\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket1/*\"\n" + + " }]\n" + + "}"; + final String duplicateResource = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket1/*\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket2/*\"\n" + + " }]\n" + + "}"; + + expectDuplicateFieldThrowsForBothAuthorizers(duplicateActionGetThenStar, "Action"); + expectDuplicateFieldThrowsForBothAuthorizers(duplicateActionStarThenGet, "Action"); + expectDuplicateFieldThrowsForBothAuthorizers(duplicateEffect, "Effect"); + expectDuplicateFieldThrowsForBothAuthorizers(duplicateResource, "Resource"); + } + + @Test + public void testDuplicateNestedConditionKeysThrow() { + final String duplicateS3Prefix = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket1\",\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": \"team/*\", \"s3:prefix\": \"other/*\" } }\n" + + " }]\n" + + "}"; + + expectDuplicateFieldThrowsForBothAuthorizers(duplicateS3Prefix, "s3:prefix"); + } + + @Test + public void testDuplicateConditionAtStatementLevelThrows() { + final String duplicateConditionStringEqualsThenStringLike = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket1\",\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": \"team/*\" } },\n" + + " \"Condition\": { \"StringLike\": { \"s3:prefix\": \"other/*\" } }\n" + + " }]\n" + + "}"; + final String duplicateConditionStringLikeThenStringEquals = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket1\",\n" + + " \"Condition\": { \"StringLike\": { \"s3:prefix\": \"other/*\" } },\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": \"team/*\" } }\n" + + " }]\n" + + "}"; + + expectDuplicateFieldThrowsForBothAuthorizers(duplicateConditionStringEqualsThenStringLike, "Condition"); + expectDuplicateFieldThrowsForBothAuthorizers(duplicateConditionStringLikeThenStringEquals, "Condition"); + } + + @Test + public void testInvalidStatementElementThrows() { + final String statementScalar = "{\n" + + " \"Statement\": \"not-an-object\"\n" + + "}"; + final String statementArrayWithNonObject = "{\n" + + " \"Statement\": [\"not-an-object\"]\n" + + "}"; + + expectResolveThrowsForBothAuthorizers( + statementScalar, "IAM session policy: Invalid Statement in JSON policy (must be an Object) - \"not-an-object\"", + MALFORMED_POLICY_DOCUMENT); + expectResolveThrowsForBothAuthorizers( + statementArrayWithNonObject, + "IAM session policy: Invalid Statement in JSON policy (must be an Object) - \"not-an-object\"", + MALFORMED_POLICY_DOCUMENT); + } + + @Test + public void testMissingActionInStatementThrows() { + final String json = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket1/*\"\n" + + " }]\n" + + "}"; + + expectResolveThrowsForBothAuthorizers( + json, "IAM session policy: No Action(s) found in policy", MALFORMED_POLICY_DOCUMENT); + } + + @Test + public void testMissingResourceInStatementThrows() { + final String json = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:GetObject\"\n" + + " }]\n" + + "}"; + + expectResolveThrowsForBothAuthorizers( + json, "IAM session policy: No Resource(s) found in policy", MALFORMED_POLICY_DOCUMENT); + } + + @Test + public void testNullResourceInStatementThrows() { + final String json = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Resource\": null\n" + + " }]\n" + + "}"; + + expectResolveThrowsForBothAuthorizers( + json, "IAM session policy: No Resource(s) found in policy", MALFORMED_POLICY_DOCUMENT); + } + + @Test + public void testInvalidResourceInStatementThrows() { + final String json = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Resource\": \"INVALID\"\n" + + " }]\n" + + "}"; + + expectResolveThrowsForBothAuthorizers( + json, "IAM session policy: Unsupported Resource Arn - INVALID", NOT_SUPPORTED_OPERATION); + } + + @Test + public void testUnsupportedStatementElementsThrow() { + final Set unsupportedStatementElements = strSet("NotAction", "NotResource", "Principal"); + for (String unsupportedStatementElement : unsupportedStatementElements) { + final String json = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket1/*\",\n" + + " \"" + unsupportedStatementElement + "\": \"ignored\"\n" + + " }]\n" + + "}"; + + expectResolveThrowsForBothAuthorizers( + json, "IAM session policy: Unsupported statement element - " + unsupportedStatementElement, + MALFORMED_POLICY_DOCUMENT); + } + } + + @Test + public void testInvalidActionShapeThrows() { + final String actionObject = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": {\"Name\":\"s3:GetObject\"},\n" + + " \"Resource\": \"arn:aws:s3:::bucket1/*\"\n" + + " }]\n" + + "}"; + final String actionArrayWithNonString = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": [\"s3:GetObject\", 1],\n" + + " \"Resource\": \"arn:aws:s3:::bucket1/*\"\n" + + " }]\n" + + "}"; + final String emptyActionArray = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": [],\n" + + " \"Resource\": \"arn:aws:s3:::bucket1/*\"\n" + + " }]\n" + + "}"; + final String nullAction = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": null,\n" + + " \"Resource\": \"arn:aws:s3:::bucket1/*\"\n" + + " }]\n" + + "}"; + + expectResolveThrowsForBothAuthorizers( + actionObject, "IAM session policy: Invalid Action in JSON policy (must be a String or Array of Strings) - " + + "{\"Name\":\"s3:GetObject\"}", MALFORMED_POLICY_DOCUMENT); + expectResolveThrowsForBothAuthorizers( + actionArrayWithNonString, + "IAM session policy: Invalid Action in JSON policy (must be a String or Array of Strings) - " + + "[\"s3:GetObject\",1]", MALFORMED_POLICY_DOCUMENT); + expectResolveThrowsForBothAuthorizers( + emptyActionArray, "IAM session policy: No Action(s) found in policy", MALFORMED_POLICY_DOCUMENT); + expectResolveThrowsForBothAuthorizers( + nullAction, "IAM session policy: No Action(s) found in policy", MALFORMED_POLICY_DOCUMENT); + } + + @Test + public void testInvalidResourceShapeThrows() { + final String resourceObject = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Resource\": {\"Arn\":\"arn:aws:s3:::bucket1/*\"}\n" + + " }]\n" + + "}"; + final String resourceArrayWithNonString = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Resource\": [\"arn:aws:s3:::bucket1/*\", 1]\n" + + " }]\n" + + "}"; + final String emptyResourceArray = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Resource\": []\n" + + " }]\n" + + "}"; + final String nullResource = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Resource\": null\n" + + " }]\n" + + "}"; + + expectResolveThrowsForBothAuthorizers( + resourceObject, "IAM session policy: Invalid Resource in JSON policy (must be a String or " + + "Array of Strings) - {\"Arn\":\"arn:aws:s3:::bucket1/*\"}", MALFORMED_POLICY_DOCUMENT); + expectResolveThrowsForBothAuthorizers( + resourceArrayWithNonString, + "IAM session policy: Invalid Resource in JSON policy (must be a String or Array of Strings) - " + + "[\"arn:aws:s3:::bucket1/*\",1]", MALFORMED_POLICY_DOCUMENT); + expectResolveThrowsForBothAuthorizers( + emptyResourceArray, "IAM session policy: No Resource(s) found in policy", MALFORMED_POLICY_DOCUMENT); + expectResolveThrowsForBothAuthorizers( + nullResource, "IAM session policy: No Resource(s) found in policy", MALFORMED_POLICY_DOCUMENT); + } + @Test public void testInvalidNumberOfConditionsThrows() { final String json = "{\n" + @@ -303,6 +585,140 @@ public void testInvalidConditionAttributeStructureThrows() { MALFORMED_POLICY_DOCUMENT); } + @Test + public void testInvalidS3PrefixConditionValueThrows() { + final String nullPrefix = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"arn:aws:s3:::b\",\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": null } }\n" + + " }]\n" + + "}"; + final String objectPrefix = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"arn:aws:s3:::b\",\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": {} } }\n" + + " }]\n" + + "}"; + final String mixedStringAndNumberArray = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"arn:aws:s3:::b\",\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": [\"team/*\", 1] } }\n" + + " }]\n" + + "}"; + + final String invalidPrefixMessagePrefix = "IAM session policy: Invalid s3:prefix in Condition (must be a " + + "String, Number, Boolean, or homogeneous Array of Strings, Numbers, or Booleans) - "; + + expectResolveThrowsForBothAuthorizers(nullPrefix, invalidPrefixMessagePrefix + "null", MALFORMED_POLICY_DOCUMENT); + expectResolveThrowsForBothAuthorizers(objectPrefix, invalidPrefixMessagePrefix + "{}", MALFORMED_POLICY_DOCUMENT); + expectResolveThrowsForBothAuthorizers( + mixedStringAndNumberArray, invalidPrefixMessagePrefix + "[\"team/*\",1]", MALFORMED_POLICY_DOCUMENT); + } + + @Test + public void testAcceptedS3PrefixConditionValueCoercion() throws OMException { + final String numericScalar = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"arn:aws:s3:::my-bucket\",\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": 123 } }\n" + + " }]\n" + + "}"; + final String booleanScalar = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"arn:aws:s3:::my-bucket\",\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": true } }\n" + + " }]\n" + + "}"; + final String numericArray = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"arn:aws:s3:::my-bucket\",\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": [123] } }\n" + + " }]\n" + + "}"; + + final Set numericScalarNative = resolve(numericScalar, VOLUME, NATIVE); + final Set numericScalarRanger = resolve(numericScalar, VOLUME, RANGER); + assertThat(numericScalarNative).containsExactlyInAnyOrder( + new OzoneGrant(objSet(volume(), prefix("my-bucket", "123")), acls(READ), strSet("ListBucket")), + new OzoneGrant(objSet(bucket("my-bucket")), acls(READ, LIST), strSet("ListBucket"))); + assertThat(numericScalarRanger).containsExactlyInAnyOrder( + new OzoneGrant(objSet(volume(), key("my-bucket", "123")), acls(READ), strSet("ListBucket")), + new OzoneGrant(objSet(bucket("my-bucket")), acls(READ, LIST), strSet("ListBucket"))); + + final Set booleanScalarNative = resolve(booleanScalar, VOLUME, NATIVE); + final Set booleanScalarRanger = resolve(booleanScalar, VOLUME, RANGER); + assertThat(booleanScalarNative).containsExactlyInAnyOrder( + new OzoneGrant(objSet(volume(), prefix("my-bucket", "true")), acls(READ), strSet("ListBucket")), + new OzoneGrant(objSet(bucket("my-bucket")), acls(READ, LIST), strSet("ListBucket"))); + assertThat(booleanScalarRanger).containsExactlyInAnyOrder( + new OzoneGrant(objSet(volume(), key("my-bucket", "true")), acls(READ), strSet("ListBucket")), + new OzoneGrant(objSet(bucket("my-bucket")), acls(READ, LIST), strSet("ListBucket"))); + + final Set numericArrayNative = resolve(numericArray, VOLUME, NATIVE); + final Set numericArrayRanger = resolve(numericArray, VOLUME, RANGER); + assertThat(numericArrayNative).containsExactlyInAnyOrder( + new OzoneGrant(objSet(volume(), prefix("my-bucket", "123")), acls(READ), strSet("ListBucket")), + new OzoneGrant(objSet(bucket("my-bucket")), acls(READ, LIST), strSet("ListBucket"))); + assertThat(numericArrayRanger).containsExactlyInAnyOrder( + new OzoneGrant(objSet(volume(), key("my-bucket", "123")), acls(READ), strSet("ListBucket")), + new OzoneGrant(objSet(bucket("my-bucket")), acls(READ, LIST), strSet("ListBucket"))); + } + + @Test + public void testEmptyS3PrefixConditionArrayDoesNotGrantAccess() throws OMException { + final String emptyPrefixArrayOnAnyResource = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"*\",\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": [] } }\n" + + " }]\n" + + "}"; + final String emptyPrefixArrayOnBucket = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:ListBucket\",\n" + + " \"Resource\": \"arn:aws:s3:::my-bucket\",\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": [] } }\n" + + " }]\n" + + "}"; + + assertThat(resolve(emptyPrefixArrayOnAnyResource, VOLUME, NATIVE)).isEmpty(); + assertThat(resolve(emptyPrefixArrayOnAnyResource, VOLUME, RANGER)).isEmpty(); + assertThat(resolve(emptyPrefixArrayOnBucket, VOLUME, NATIVE)).isEmpty(); + assertThat(resolve(emptyPrefixArrayOnBucket, VOLUME, RANGER)).isEmpty(); + } + + @Test + public void testEmptyS3PrefixConditionArrayWithMultipleActionsAndResourcesDoesNotGrantAccess() throws OMException { + final String json = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": [\"s3:ListBucket\", \"s3:PutObject\", \"s3:DeleteObject\"],\n" + + " \"Resource\": [\n" + + " \"arn:aws:s3:::my-bucket\",\n" + + " \"arn:aws:s3:::my-bucket/*\"\n" + + " ],\n" + + " \"Condition\": { \"StringEquals\": { \"s3:prefix\": [] } }\n" + + " }]\n" + + "}"; + + assertThat(resolve(json, VOLUME, NATIVE)).isEmpty(); + assertThat(resolve(json, VOLUME, RANGER)).isEmpty(); + } + @Test public void testInvalidJsonThrows() { final String invalidJson = "{[{{}]\"\""; @@ -2454,6 +2870,12 @@ private static void expectResolveThrowsForBothAuthorizers(String json, String ex expectResolveThrows(json, RANGER, expectedMessage, expectedCode); } + private static void expectDuplicateFieldThrowsForBothAuthorizers(String json, String duplicateFieldName) { + expectResolveThrowsForBothAuthorizers( + json, "IAM session policy: Duplicate field '" + duplicateFieldName + "' in session policy", + MALFORMED_POLICY_DOCUMENT); + } + /** * Ensure resources containing wildcards in buckets throw an Exception * when the OzoneNativeAuthorizer is used. diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3AssumeRoleRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3AssumeRoleRequest.java index f465dcaaf795..3ae775b716b3 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3AssumeRoleRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3AssumeRoleRequest.java @@ -471,14 +471,13 @@ public void testValidRoleSessionNameMinLengthBoundary() throws IOException { @Test public void testAssumeRoleWithSessionPolicyPresent() throws IOException { - final String sessionPolicy = "{\"Version\":\"2012-10-17\",\"Statement\":[]}"; final OMRequest omRequest = baseOmRequestBuilder() .setAssumeRoleRequest( AssumeRoleRequest.newBuilder() .setRoleArn(ROLE_ARN_1) .setRoleSessionName(SESSION_NAME) .setDurationSeconds(3600) - .setAwsIamSessionPolicy(sessionPolicy) + .setAwsIamSessionPolicy(AWS_IAM_POLICY) .setRequestId(REQUEST_ID) ).build(); @@ -491,6 +490,40 @@ public void testAssumeRoleWithSessionPolicyPresent() throws IOException { assertMarkForAuditCalled(requestWithCredentials); } + @Test + public void testMalformedSessionPolicyDoesNotIssueCredentials() throws IOException { + final String sessionPolicy = "{\n" + + " \"Statement\": [{\n" + + " \"Effect\": \"Allow\",\n" + + " \"Action\": \"s3:GetObject\",\n" + + " \"Action\": \"s3:*\",\n" + + " \"Resource\": \"arn:aws:s3:::bucket1/*\"\n" + + " }]\n" + + "}"; + final OMRequest omRequest = baseOmRequestBuilder() + .setAssumeRoleRequest( + AssumeRoleRequest.newBuilder() + .setRoleArn(ROLE_ARN_1) + .setRoleSessionName(SESSION_NAME) + .setDurationSeconds(3600) + .setAwsIamSessionPolicy(sessionPolicy) + .setRequestId(REQUEST_ID) + ).build(); + + final S3AssumeRoleRequest request = new S3AssumeRoleRequest(omRequest, CLOCK); + final OMRequest preExecutedRequest = request.preExecute(ozoneManager); + final S3AssumeRoleRequest requestWithCredentials = new S3AssumeRoleRequest(preExecutedRequest, CLOCK); + final OMClientResponse response = requestWithCredentials.validateAndUpdateCache(ozoneManager, context); + final OMResponse omResponse = response.getOMResponse(); + + assertThat(omResponse.getStatus()).isEqualTo(Status.MALFORMED_POLICY_DOCUMENT); + assertThat(omResponse.getMessage()).isEqualTo("IAM session policy: Duplicate field 'Action' in session policy"); + assertThat(omResponse.hasAssumeRoleResponse()).isFalse(); + verify(accessAuthorizer, never()).generateAssumeRoleSessionPolicy( + any(org.apache.hadoop.ozone.security.acl.AssumeRoleRequest.class)); + assertMarkForAuditCalled(requestWithCredentials); + } + @Test public void testGetSessionPolicyUsesDefaultVolumeWhenMultiTenantDisabled() throws Exception { when(ozoneManager.isS3MultiTenancyEnabled()).thenReturn(false);