diff options
author | Bjørn Christian Seime <bjorncs@yahooinc.com> | 2023-01-03 16:51:24 +0100 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@yahooinc.com> | 2023-01-03 16:51:24 +0100 |
commit | 3e051557b91d0cedd6bff859f49e0609567a4741 (patch) | |
tree | c7bce8460f0c5915915fbc7c4618fffda58c1097 | |
parent | 6f57128414995c8fad752dc7e20bd4ed98eaea16 (diff) |
Hardening validation of user provided IAM role
3 files changed, 85 insertions, 6 deletions
diff --git a/controller-api/pom.xml b/controller-api/pom.xml index c7ac5d3518a..73e4522c521 100644 --- a/controller-api/pom.xml +++ b/controller-api/pom.xml @@ -81,6 +81,28 @@ <artifactId>annotations</artifactId> <version>9.0.4</version> </dependency> + <dependency> + <artifactId>aws-java-sdk-core</artifactId> + <groupId>com.amazonaws</groupId> + <exclusions> + <exclusion> + <groupId>org.apache.httpcomponents</groupId> + <artifactId>httpclient</artifactId> + </exclusion> + <exclusion> + <groupId>com.fasterxml.jackson.dataformat</groupId> + <artifactId>*</artifactId> + </exclusion> + <exclusion> + <groupId>com.fasterxml.jackson.core</groupId> + <artifactId>*</artifactId> + </exclusion> + <exclusion> + <groupId>commons-logging</groupId> + <artifactId>*</artifactId> + </exclusion> + </exclusions> + </dependency> <!-- test --> diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/ArchiveAccess.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/ArchiveAccess.java index fba361f9223..eb4bd2e2289 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/ArchiveAccess.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/ArchiveAccess.java @@ -1,7 +1,9 @@ package com.yahoo.vespa.hosted.controller.tenant; +import com.amazonaws.arn.Arn; import com.yahoo.text.Text; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -10,7 +12,6 @@ import java.util.stream.Collectors; public class ArchiveAccess { - private static final Pattern VALID_AWS_ARCHIVE_ACCESS_ROLE_PATTERN = Pattern.compile("arn:aws:iam::\\d{12}:.+"); private static final Pattern VALID_GCP_ARCHIVE_ACCESS_MEMBER_PATTERN = Pattern.compile("(?<prefix>[a-zA-Z]+):.+"); private static final Set<String> gcpMemberPrefixes = Set.of("user", "serviceAccount", "group", "domain"); @@ -56,14 +57,25 @@ public class ArchiveAccess { return new ArchiveAccess(awsRole(), Optional.empty()); } - private void validateAWSIAMRole(String role) { - if (!VALID_AWS_ARCHIVE_ACCESS_ROLE_PATTERN.matcher(role).matches()) { - throw new IllegalArgumentException(Text.format("Invalid archive access role '%s': Must match expected pattern: '%s'", - awsRole.get(), VALID_AWS_ARCHIVE_ACCESS_ROLE_PATTERN.pattern())); - } + private static final Pattern ACCOUNT_ID_PATTERN = Pattern.compile("\\d{12}"); + private static void validateAWSIAMRole(String role) { if (role.length() > 100) { throw new IllegalArgumentException("Invalid archive access role too long, must be 100 or less characters"); } + try { + var arn = Arn.fromString(role); + if (!arn.getPartition().equals("aws")) throw new IllegalArgumentException("Partition must be 'aws'"); + if (!arn.getService().equals("iam")) throw new IllegalArgumentException("Service must be 'iam'"); + var resourceType = arn.getResource().getResourceType(); + if (resourceType == null) throw new IllegalArgumentException("Missing resource type - must be 'role' or 'user'"); + if (!List.of("user", "role").contains(resourceType)) + throw new IllegalArgumentException("Invalid resource type - must be either a 'role' or 'user'"); + var accountId = arn.getAccountId(); + if (!ACCOUNT_ID_PATTERN.matcher(accountId).matches()) + throw new IllegalArgumentException("Account id must be a 12-digit number"); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException(Text.format("Invalid archive access IAM role '%s': %s", role, e.getMessage())); + } } private void validateGCPMember(String member) { diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/tenant/ArchiveAccessTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/tenant/ArchiveAccessTest.java new file mode 100644 index 00000000000..87e02793361 --- /dev/null +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/tenant/ArchiveAccessTest.java @@ -0,0 +1,45 @@ +package com.yahoo.vespa.hosted.controller.tenant;// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +import com.yahoo.text.Text; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * @author bjorncs + */ +class ArchiveAccessTest { + + @Test + void validatesUserProvidedIamRole() { + assertValidIamRole("arn:aws:iam::012345678912:user/foo"); + assertValidIamRole("arn:aws:iam::012345678912:role/foo"); + + assertInvalidIamRole("arn:aws:iam::012345678912:foo/foo", "Invalid resource type - must be either a 'role' or 'user'"); + assertInvalidIamRole("arn:aws:iam::012345678912:foo", "Missing resource type - must be 'role' or 'user'"); + assertInvalidIamRole("arn:aws:iam::012345678912:role", "Missing resource type - must be 'role' or 'user'"); + assertInvalidIamRole("arn:aws:iam::012345678912:", "Malformed ARN - no resource specified"); + assertInvalidIamRole("arn:aws:iam::01234567891:user/foo", "Account id must be a 12-digit number"); + assertInvalidIamRole("arn:gcp:iam::012345678912:user/foo", "Partition must be 'aws'"); + assertInvalidIamRole("uri:aws:iam::012345678912:user/foo", "Malformed ARN - doesn't start with 'arn:'"); + assertInvalidIamRole("arn:aws:s3:::mybucket", "Service must be 'iam'"); + assertInvalidIamRole("", "Malformed ARN - doesn't start with 'arn:'"); + assertInvalidIamRole("foo", "Malformed ARN - doesn't start with 'arn:'"); + } + + private static void assertValidIamRole(String role) { assertDoesNotThrow(() -> archiveAccess(role)); } + + private static void assertInvalidIamRole(String role, String expectedMessage) { + var t = assertThrows(IllegalArgumentException.class, () -> archiveAccess(role)); + var expectedPrefix = Text.format("Invalid archive access IAM role '%s': ", role); + System.out.println(t.getMessage()); + assertTrue(t.getMessage().startsWith(expectedPrefix), role); + assertEquals(expectedMessage, t.getMessage().substring(expectedPrefix.length())); + } + + private static ArchiveAccess archiveAccess(String iamRole) { return new ArchiveAccess().withAWSRole(iamRole); } + +}
\ No newline at end of file |