Revert "Revert "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