Merge pull request #25383 from vespa-engine/revert-25378-bjorncs/hardened-arn-validation

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