diff options
author | Harald Musum <musum@verizonmedia.com> | 2023-01-04 00:04:14 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-01-04 00:04:14 +0100 |
commit | c6d23e8452a1e779bb788744745ae6269015e125 (patch) | |
tree | eb87693c4aacb4a560a43bd4a84eb8947e404519 /controller-api | |
parent | 21a63553b099e37f480ea3b4b1a65862388e3655 (diff) |
Revert "Hardening validation of user provided IAM role"
Diffstat (limited to 'controller-api')
3 files changed, 6 insertions, 85 deletions
diff --git a/controller-api/pom.xml b/controller-api/pom.xml index 73e4522c521..c7ac5d3518a 100644 --- a/controller-api/pom.xml +++ b/controller-api/pom.xml @@ -81,28 +81,6 @@ <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 eb4bd2e2289..fba361f9223 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,9 +1,7 @@ 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; @@ -12,6 +10,7 @@ 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"); @@ -57,25 +56,14 @@ public class ArchiveAccess { return new ArchiveAccess(awsRole(), Optional.empty()); } - private static final Pattern ACCOUNT_ID_PATTERN = Pattern.compile("\\d{12}"); - private static void validateAWSIAMRole(String role) { + 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())); + } 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 deleted file mode 100644 index 87e02793361..00000000000 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/tenant/ArchiveAccessTest.java +++ /dev/null @@ -1,45 +0,0 @@ -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 |