make policy checker check for arn rather than specific id, as to avoi…

…d unnessary calls to KMS (#70)

src/main/java/com/nike/cerberus/service/KmsPolicyService.java

@@ -25,6 +25,7 @@
 import com.amazonaws.auth.policy.actions.KMSActions;
 import com.amazonaws.auth.policy.internal.JsonPolicyReader;
 import com.amazonaws.services.kms.model.PutKeyPolicyRequest;
+import com.nike.cerberus.util.AwsIamRoleArnParser;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -64,13 +65,18 @@ public class KmsPolicyService {
 
     private final JsonPolicyReader policyReader;
 
+    private final AwsIamRoleArnParser awsIamRoleArnParser;
+
     @Inject
     public KmsPolicyService(@Named(ROOT_USER_ARN_PROPERTY) String rootUserArn,
                             @Named(ADMIN_ROLE_ARN_PROPERTY) String adminRoleArn,
-                            @Named(CMS_ROLE_ARN_PROPERTY) String cmsRoleArn) {
+                            @Named(CMS_ROLE_ARN_PROPERTY) String cmsRoleArn,
+                            AwsIamRoleArnParser awsIamRoleArnParser) {
+
         this.rootUserArn = rootUserArn;
         this.adminRoleArn = adminRoleArn;
         this.cmsRoleArn = cmsRoleArn;
+        this.awsIamRoleArnParser = awsIamRoleArnParser;
 
         PolicyReaderOptions policyReaderOptions = new PolicyReaderOptions();
         policyReaderOptions.setStripAwsPrincipalIdHyphensEnabled(false);
@@ -82,11 +88,10 @@ public KmsPolicyService(@Named(ROOT_USER_ARN_PROPERTY) String rootUserArn,
      * access the KMS key.
      *
      * @param policyJson - The KMS key policy as a String
-     * @param iamRoleArn - The IAM Role that is supposed to have decrypt permissions
      * @return true if the policy is valid, false if the policy contains an ID because the ARN had been deleted and recreated
      */
-    public boolean isPolicyValid(String policyJson, String iamRoleArn) {
-        return iamPrincipalCanAccessKey(policyJson, iamRoleArn) && cmsHasKeyDeletePermissions(policyJson);
+    public boolean isPolicyValid(String policyJson) {
+        return consumerPrincipalIsAnArnAndNotAnId(policyJson) && cmsHasKeyDeletePermissions(policyJson);
     }
 
     /**
@@ -97,16 +102,17 @@ public boolean isPolicyValid(String policyJson, String iamRoleArn) {
      * allow the ARN of the recreated principal instead of the ID of the deleted principal.
      *
      * @param policyJson - The KMS key policy as a String
-     * @param iamPrincipalArn - The IAM Role that is supposed to have decrypt permissions
      */
-    protected boolean iamPrincipalCanAccessKey(String policyJson, String iamPrincipalArn) {
+    protected boolean consumerPrincipalIsAnArnAndNotAnId(String policyJson) {
         try {
             Policy policy = policyReader.createPolicyFromJsonString(policyJson);
             return policy.getStatements()
                     .stream()
                     .anyMatch(statement ->
                             StringUtils.equals(statement.getId(), CERBERUS_CONSUMER_SID) &&
-                                    statementAppliesToPrincipal(statement, iamPrincipalArn));
+                                    statement.getPrincipals()
+                                            .stream()
+                                            .anyMatch(principal -> awsIamRoleArnParser.isRoleArn(principal.getId())));
         } catch (Exception e) {
             // if we can't deserialize we will assume policy has been corrupted manually and regenerate it
             logger.error("Failed to validate policy, did someone manually edit the kms policy?", e);

src/main/java/com/nike/cerberus/service/KmsService.java

@@ -217,7 +217,7 @@ public void validateKeyAndPolicy(AwsIamRoleKmsKeyRecord kmsKeyRecord, String iam
         try {
             String keyPolicy = getKmsKeyPolicy(awsKmsKeyArn, kmsCMKRegion);
 
-            if (!kmsPolicyService.isPolicyValid(keyPolicy, iamPrincipalArn)) {
+            if (!kmsPolicyService.isPolicyValid(keyPolicy)) {
                 logger.info("The KMS key: {} generated for IAM principal: {} contained an invalid policy, regenerating",
                         awsKmsKeyArn, iamPrincipalArn);
 

src/test/java/com/nike/cerberus/service/KmsPolicyServiceTest.java

@@ -6,6 +6,7 @@
 import com.amazonaws.auth.policy.actions.KMSActions;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.nike.cerberus.util.AwsIamRoleArnParser;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.assertj.core.util.Lists;
@@ -36,7 +37,7 @@ public void before() {
         String rootUserArn = "arn:aws:iam::1111111111:root";
         String adminRoleArn = "arn:aws:iam::1111111111:role/admin";
         String cmsRoleArn = "arn:aws:iam::1111111111:role/cms-iam-role";
-        kmsPolicyService = new KmsPolicyService(rootUserArn, adminRoleArn, cmsRoleArn);
+        kmsPolicyService = new KmsPolicyService(rootUserArn, adminRoleArn, cmsRoleArn, new AwsIamRoleArnParser());
         objectMapper = new ObjectMapper();
     }
 
@@ -62,7 +63,7 @@ public void test_that_isPolicyValid_returns_true_with_a_valid_policy() throws IO
                 .getResourceAsStream("com/nike/cerberus/service/valid-cerberus-iam-auth-kms-key-policy.json");
         String policyJsonAsString = IOUtils.toString(policy, "UTF-8");
 
-        assertTrue(kmsPolicyService.isPolicyValid(policyJsonAsString, CERBERUS_CONSUMER_IAM_ROLE_ARN));
+        assertTrue(kmsPolicyService.isPolicyValid(policyJsonAsString));
 
         policy.close();
     }
@@ -73,7 +74,7 @@ public void test_that_isPolicyValid_returns_false_with_an_invalid_policy() throw
                 .getResourceAsStream("com/nike/cerberus/service/invalid-cerberus-iam-auth-kms-key-policy.json");
         String policyJsonAsString = IOUtils.toString(policy, "UTF-8");
 
-        assertFalse(kmsPolicyService.isPolicyValid(policyJsonAsString, CERBERUS_CONSUMER_IAM_ROLE_ARN));
+        assertFalse(kmsPolicyService.isPolicyValid(policyJsonAsString));
 
         policy.close();
     }
@@ -84,14 +85,14 @@ public void test_that_isPolicyValid_returns_false_when_cms_does_not_have_delete_
                 .getResourceAsStream("com/nike/cerberus/service/invalid-cerberus-kms-key-policy-cms-cannot-delete.json");
         String policyJsonAsString = IOUtils.toString(policy, "UTF-8");
 
-        assertFalse(kmsPolicyService.isPolicyValid(policyJsonAsString, CERBERUS_CONSUMER_IAM_ROLE_ARN));
+        assertFalse(kmsPolicyService.isPolicyValid(policyJsonAsString));
 
         policy.close();
     }
 
     @Test
     public void test_that_isPolicyValid_returns_false_when_a_non_standard_policy_is_supplied() {
-        assertFalse(kmsPolicyService.isPolicyValid(null, CERBERUS_CONSUMER_IAM_ROLE_ARN));
+        assertFalse(kmsPolicyService.isPolicyValid(null));
     }
 
     @Test

src/test/java/com/nike/cerberus/service/KmsServiceTest.java

@@ -137,7 +137,7 @@ public void test_validatePolicy_validates_policy_when_validate_interval_has_pass
         when(result.getPolicy()).thenReturn(policy);
         when(client.getKeyPolicy(new GetKeyPolicyRequest().withKeyId(kmsKeyArn)
                 .withPolicyName("default"))).thenReturn(result);
-        when(kmsPolicyService.isPolicyValid(policy, kmsKeyArn)).thenReturn(true);
+        when(kmsPolicyService.isPolicyValid(policy)).thenReturn(true);
 
         AwsIamRoleKmsKeyRecord kmsKey = mock(AwsIamRoleKmsKeyRecord.class);
         when(kmsKey.getAwsIamRoleId()).thenReturn(awsIamRoleRecordId);
@@ -151,7 +151,7 @@ public void test_validatePolicy_validates_policy_when_validate_interval_has_pass
 
         verify(client, times(1)).getKeyPolicy(new GetKeyPolicyRequest().withKeyId(kmsKeyArn)
                 .withPolicyName("default"));
-        verify(kmsPolicyService, times(1)).isPolicyValid(policy, kmsKeyArn);
+        verify(kmsPolicyService, times(1)).isPolicyValid(policy);
     }
 
     @Test
@@ -172,7 +172,7 @@ public void test_validateKeyAndPolicy_validates_policy_when_validate_interval_ha
         kmsService.validateKeyAndPolicy(kmsKey, iamPrincipalArn);
 
         verify(kmsClientFactory, never()).getClient(anyString());
-        verify(kmsPolicyService, never()).isPolicyValid(anyString(), anyString());
+        verify(kmsPolicyService, never()).isPolicyValid(anyString());
     }
 
     @Test
@@ -201,7 +201,7 @@ public void test_validateKeyAndPolicy_does_not_throw_error_when_cannot_validate(
 
         kmsService.validateKeyAndPolicy(kmsKey, iamPrincipalArn);
 
-        verify(kmsPolicyService, never()).isPolicyValid(policy, iamPrincipalArn);
+        verify(kmsPolicyService, never()).isPolicyValid(policy);
         verify(client, never()).putKeyPolicy(anyObject());
     }