From e62de48ba91d12b6ee6616a3346a9399a1b85568 Mon Sep 17 00:00:00 2001 From: Shaun Ford Date: Wed, 26 Apr 2017 22:41:18 -0700 Subject: [PATCH 1/3] Avoid hitting API limit on KMS policy validation --- README.md | 27 ++++----- gradle.properties | 2 +- gradle/develop.gradle | 4 +- .../com/nike/cerberus/dao/AwsIamRoleDao.java | 4 ++ .../cerberus/mapper/AwsIamRoleMapper.java | 2 + .../record/AwsIamRoleKmsKeyRecord.java | 16 +++++- .../service/AuthenticationService.java | 33 +++++++++-- .../com/nike/cerberus/service/KmsService.java | 28 ++++++++++ .../nike/cerberus/mapper/AwsIamRoleMapper.xml | 22 +++++++- ...ast_validated_field_to_aws_iam_kms_key.sql | 5 ++ .../nike/cerberus/dao/AwsIamRoleDaoTest.java | 10 ++++ .../service/AuthenticationServiceTest.java | 56 +++++++++++++++++++ .../nike/cerberus/service/KmsServiceTest.java | 50 +++++++++++++++++ 13 files changed, 233 insertions(+), 26 deletions(-) create mode 100644 src/main/resources/com/nike/cerberus/migration/V1.4.0.0__add_last_validated_field_to_aws_iam_kms_key.sql diff --git a/README.md b/README.md index 7e810c928..539c3fd20 100644 --- a/README.md +++ b/README.md @@ -59,19 +59,20 @@ That will setup the default policy and generate a token for CMS and output: There are a few parameters that need to be configured for CMS to run properly, they are defined in this table. -property | required | notes ---------------------------- | -------- | ---------- -JDBC.url | Yes | The JDBC url for the mysql db -JDBC.username | Yes | The JDBC user name for the mysql db -JDBC.password | Yes | The JDBC JDBC.password for the mysql db -root.user.arn | Yes | The arn for the root AWS user, needed to make the KMS keys deletable. -admin.role.arn | Yes | The arn for an AWS user, needed to make the KMS keys deletable. -cms.role.arn | Yes | The arn for the Instance profile for CMS instances, so they can admin KMS keys that they create. -cms.admin.group | Yes | Group that user can be identified by to get admin privileges, currently this just enables users to access `/v1/metadata` see API.md -cms.admin.roles | No | Comma seperated list of ARNs that can auth and access admin endpoints. -cms.auth.connector | Yes | The user authentication connector implementation to use for user auth. -cms.user.token.ttl.override | No | By default user tokens are created with a TTL of 1h, you can override that with this param -cms.iam.token.ttl.override | No | By default IAM tokens are created with a TTL of 1h, you can override that with this param +property | required | notes +--------------------------- | -------- | ---------- +JDBC.url | Yes | The JDBC url for the mysql db +JDBC.username | Yes | The JDBC user name for the mysql db +JDBC.password | Yes | The JDBC JDBC.password for the mysql db +root.user.arn | Yes | The arn for the root AWS user, needed to make the KMS keys deletable. +admin.role.arn | Yes | The arn for an AWS user, needed to make the KMS keys deletable. +cms.role.arn | Yes | The arn for the Instance profile for CMS instances, so they can admin KMS keys that they create. +cms.admin.group | Yes | Group that user can be identified by to get admin privileges, currently this just enables users to access `/v1/metadata` see API.md +cms.admin.roles | No | Comma seperated list of ARNs that can auth and access admin endpoints. +cms.auth.connector | Yes | The user authentication connector implementation to use for user auth. +cms.user.token.ttl.override | No | By default user tokens are created with a TTL of 1h, you can override that with this param +cms.iam.token.ttl.override | No | By default IAM tokens are created with a TTL of 1h, you can override that with this param +cms.kms.policy.validation.interval.millis.override | No | By default CMS validates KMS key policies no more than once per minute, you can override that with this param For local dev see `Running CMS Locally`. diff --git a/gradle.properties b/gradle.properties index 48451129d..24e72c056 100644 --- a/gradle.properties +++ b/gradle.properties @@ -14,6 +14,6 @@ # limitations under the License. # -version=0.17.0 +version=0.18.0 groupId=com.nike.cerberus artifactId=cms diff --git a/gradle/develop.gradle b/gradle/develop.gradle index c7a6f31bd..855cd4471 100644 --- a/gradle/develop.gradle +++ b/gradle/develop.gradle @@ -18,8 +18,8 @@ import org.apache.tools.ant.taskdefs.condition.Os import groovyx.net.http.RESTClient import static groovyx.net.http.ContentType.* -def dashboardRelease = 'v0.12.0' -def vaultVersion = "0.6.4" +def dashboardRelease = 'v1.0.0' +def vaultVersion = "0.7.0" buildscript { apply from: file('gradle/buildscript.gradle'), to: buildscript diff --git a/src/main/java/com/nike/cerberus/dao/AwsIamRoleDao.java b/src/main/java/com/nike/cerberus/dao/AwsIamRoleDao.java index 118a87510..c5f25014b 100644 --- a/src/main/java/com/nike/cerberus/dao/AwsIamRoleDao.java +++ b/src/main/java/com/nike/cerberus/dao/AwsIamRoleDao.java @@ -76,4 +76,8 @@ public Optional getKmsKey(final String awsIamRoleId, fin public int createIamRoleKmsKey(final AwsIamRoleKmsKeyRecord record) { return awsIamRoleMapper.createIamRoleKmsKey(record); } + + public int updateIamRoleKmsKey(final AwsIamRoleKmsKeyRecord record) { + return awsIamRoleMapper.updateIamRoleKmsKey(record); + } } diff --git a/src/main/java/com/nike/cerberus/mapper/AwsIamRoleMapper.java b/src/main/java/com/nike/cerberus/mapper/AwsIamRoleMapper.java index d6aedddeb..c15fd88b9 100644 --- a/src/main/java/com/nike/cerberus/mapper/AwsIamRoleMapper.java +++ b/src/main/java/com/nike/cerberus/mapper/AwsIamRoleMapper.java @@ -49,4 +49,6 @@ int deleteIamRolePermission(@Param("safeDepositBoxId") String safeDepositBoxId, List getIamRolePermissions(@Param("safeDepositBoxId") String safeDepositBoxId); int deleteIamRolePermissions(@Param("safeDepositBoxId") String safeDepositBoxId); + + int updateIamRoleKmsKey(@Param("record") AwsIamRoleKmsKeyRecord record); } diff --git a/src/main/java/com/nike/cerberus/record/AwsIamRoleKmsKeyRecord.java b/src/main/java/com/nike/cerberus/record/AwsIamRoleKmsKeyRecord.java index b1c644999..666a4aeb9 100644 --- a/src/main/java/com/nike/cerberus/record/AwsIamRoleKmsKeyRecord.java +++ b/src/main/java/com/nike/cerberus/record/AwsIamRoleKmsKeyRecord.java @@ -40,6 +40,8 @@ public class AwsIamRoleKmsKeyRecord { private OffsetDateTime lastUpdatedTs; + private OffsetDateTime lastValidatedTs; + public String getId() { return id; } @@ -112,6 +114,15 @@ public AwsIamRoleKmsKeyRecord setLastUpdatedTs(OffsetDateTime lastUpdatedTs) { return this; } + public OffsetDateTime getLastValidatedTs() { + return lastValidatedTs; + } + + public AwsIamRoleKmsKeyRecord setLastValidatedTs(OffsetDateTime lastValidatedTs) { + this.lastValidatedTs = lastValidatedTs; + return this; + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -124,11 +135,12 @@ public boolean equals(Object o) { Objects.equals(createdBy, that.createdBy) && Objects.equals(lastUpdatedBy, that.lastUpdatedBy) && Objects.equals(createdTs, that.createdTs) && - Objects.equals(lastUpdatedTs, that.lastUpdatedTs); + Objects.equals(lastUpdatedTs, that.lastUpdatedTs) && + Objects.equals(lastValidatedTs, that.lastValidatedTs); } @Override public int hashCode() { - return Objects.hash(id, awsIamRoleId, awsRegion, awsKmsKeyId, createdBy, lastUpdatedBy, createdTs, lastUpdatedTs); + return Objects.hash(id, awsIamRoleId, awsRegion, awsKmsKeyId, createdBy, lastUpdatedBy, createdTs, lastUpdatedTs, lastValidatedTs); } } diff --git a/src/main/java/com/nike/cerberus/service/AuthenticationService.java b/src/main/java/com/nike/cerberus/service/AuthenticationService.java index 59196b17b..87308b4cb 100644 --- a/src/main/java/com/nike/cerberus/service/AuthenticationService.java +++ b/src/main/java/com/nike/cerberus/service/AuthenticationService.java @@ -57,9 +57,14 @@ import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang3.StringUtils; import org.apache.http.HttpStatus; +import org.joda.time.Interval; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.nio.ByteBuffer; import java.nio.charset.Charset; +import java.time.OffsetDateTime; +import java.time.temporal.ChronoUnit; import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -73,13 +78,17 @@ @Singleton public class AuthenticationService { + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + public static final String SYSTEM_USER = "system"; public static final String ADMIN_GROUP_PROPERTY = "cms.admin.group"; public static final String ADMIN_IAM_ROLES_PROPERTY = "cms.admin.roles"; public static final String USER_TOKEN_TTL_OVERRIDE = "cms.user.token.ttl.override"; public static final String IAM_TOKEN_TTL_OVERRIDE = "cms.iam.token.ttl.override"; + public static final String KMS_POLICY_VALIDATION_INTERVAL_OVERRIDE = "cms.kms.policy.validation.interval.millis.override"; public static final String LOOKUP_SELF_POLICY = "lookup-self"; public static final String DEFAULT_TOKEN_TTL = "1h"; + public static final Integer DEFAULT_KMS_VALIDATION_INTERVAL = 6000; // in milliseconds private final SafeDepositBoxDao safeDepositBoxDao; private final AwsIamRoleDao awsIamRoleDao; @@ -107,6 +116,10 @@ public class AuthenticationService { @Named(IAM_TOKEN_TTL_OVERRIDE) String iamTokenTTL = DEFAULT_TOKEN_TTL; + @Inject(optional=true) + @Named(KMS_POLICY_VALIDATION_INTERVAL_OVERRIDE) + Integer kmsKeyPolicyValidationInterval = DEFAULT_KMS_VALIDATION_INTERVAL; + @Inject public AuthenticationService(final SafeDepositBoxDao safeDepositBoxDao, final AwsIamRoleDao awsIamRoleDao, @@ -203,7 +216,7 @@ public IamRoleAuthResponse authenticate(IamPrincipalCredentials credentials) { return authenticate(credentials, vaultAuthPrincipalMetadata); } - public IamRoleAuthResponse authenticate(IamPrincipalCredentials credentials, Map vaultAuthPrincipalMetadata) { + private IamRoleAuthResponse authenticate(IamPrincipalCredentials credentials, Map vaultAuthPrincipalMetadata) { final String keyId; try { keyId = getKeyId(credentials); @@ -360,7 +373,7 @@ private Set buildPolicySet(final String iamRoleArn) { * @param credentials IAM role credentials * @return KMS Key id */ - private String getKeyId(IamPrincipalCredentials credentials) { + protected String getKeyId(IamPrincipalCredentials credentials) { final Optional iamRole = awsIamRoleDao.getIamRole(credentials.getIamPrincipalArn()); if (!iamRole.isPresent()) { @@ -374,14 +387,24 @@ private String getKeyId(IamPrincipalCredentials credentials) { final Optional kmsKey = awsIamRoleDao.getKmsKey(iamRole.get().getId(), credentials.getRegion()); final String kmsKeyId; + final AwsIamRoleKmsKeyRecord kmsKeyRecord; + final OffsetDateTime now = dateTimeSupplier.get(); if (!kmsKey.isPresent()) { kmsKeyId = kmsService.provisionKmsKey(iamRole.get().getId(), credentials.getIamPrincipalArn(), - credentials.getRegion(), SYSTEM_USER, dateTimeSupplier.get()); + credentials.getRegion(), SYSTEM_USER, now); } else { - kmsKeyId = kmsKey.get().getAwsKmsKeyId(); + kmsKeyRecord = kmsKey.get(); + kmsKeyId = kmsKeyRecord.getAwsKmsKeyId(); String keyRegion = credentials.getRegion(); - kmsService.validatePolicy(kmsKeyId, credentials.getIamPrincipalArn(), keyRegion); + if (ChronoUnit.MILLIS.between(kmsKeyRecord.getLastValidatedTs(), now) >= kmsKeyPolicyValidationInterval) { + try { + kmsService.validatePolicy(kmsKeyId, credentials.getIamPrincipalArn(), keyRegion); + kmsService.updateKmsKey(kmsKeyRecord, SYSTEM_USER, now, now); + } catch (ApiException ae) { + logger.warn("Could not validate KMS policy. API limit may have been reached for validate call."); + } + } } return kmsKeyId; diff --git a/src/main/java/com/nike/cerberus/service/KmsService.java b/src/main/java/com/nike/cerberus/service/KmsService.java index 041befe51..819b225d8 100644 --- a/src/main/java/com/nike/cerberus/service/KmsService.java +++ b/src/main/java/com/nike/cerberus/service/KmsService.java @@ -39,6 +39,7 @@ import javax.inject.Inject; import javax.inject.Singleton; import java.time.OffsetDateTime; +import java.util.Optional; /** * Abstracts interactions with the AWS KMS service. @@ -111,12 +112,39 @@ public String provisionKmsKey(final String iamRoleId, awsIamRoleKmsKeyRecord.setLastUpdatedBy(user); awsIamRoleKmsKeyRecord.setCreatedTs(dateTime); awsIamRoleKmsKeyRecord.setLastUpdatedTs(dateTime); + awsIamRoleKmsKeyRecord.setLastValidatedTs(dateTime); awsIamRoleDao.createIamRoleKmsKey(awsIamRoleKmsKeyRecord); return result.getKeyMetadata().getArn(); } + @Transactional + public void updateKmsKey(final AwsIamRoleKmsKeyRecord awsIamRoleKmsKeyRecord, + final String user, + final OffsetDateTime dateTime, + final OffsetDateTime lastValidatedTs) { + final Optional kmsKey = + awsIamRoleDao.getKmsKey(awsIamRoleKmsKeyRecord.getAwsIamRoleId(), awsIamRoleKmsKeyRecord.getAwsRegion()); + + if (!kmsKey.isPresent()) { + throw ApiException.newBuilder() + .withApiErrors(DefaultApiError.ENTITY_NOT_FOUND) + .withExceptionMessage("Unable to update a KMS key that does not exist.") + .build(); + } + + AwsIamRoleKmsKeyRecord kmsKeyRecord = kmsKey.get(); + + AwsIamRoleKmsKeyRecord updatedKmsKeyRecord = new AwsIamRoleKmsKeyRecord(); + updatedKmsKeyRecord.setAwsIamRoleId(kmsKeyRecord.getAwsIamRoleId()); + updatedKmsKeyRecord.setLastUpdatedBy(user); + updatedKmsKeyRecord.setLastUpdatedTs(dateTime); + updatedKmsKeyRecord.setLastValidatedTs(lastValidatedTs); + updatedKmsKeyRecord.setAwsRegion(kmsKeyRecord.getAwsRegion()); + awsIamRoleDao.updateIamRoleKmsKey(updatedKmsKeyRecord); + } + protected String getAliasName(String awsIamRoleKmsKeyId) { return String.format(KMS_ALIAS_FORMAT, awsIamRoleKmsKeyId); } diff --git a/src/main/resources/com/nike/cerberus/mapper/AwsIamRoleMapper.xml b/src/main/resources/com/nike/cerberus/mapper/AwsIamRoleMapper.xml index 76aefde56..f0a8d5db2 100644 --- a/src/main/resources/com/nike/cerberus/mapper/AwsIamRoleMapper.xml +++ b/src/main/resources/com/nike/cerberus/mapper/AwsIamRoleMapper.xml @@ -57,7 +57,8 @@ CREATED_BY, LAST_UPDATED_BY, CREATED_TS, - LAST_UPDATED_TS + LAST_UPDATED_TS, + LAST_VALIDATED_TS FROM AWS_IAM_ROLE_KMS_KEY WHERE @@ -75,7 +76,8 @@ CREATED_BY, LAST_UPDATED_BY, CREATED_TS, - LAST_UPDATED_TS + LAST_UPDATED_TS, + LAST_VALIDATED_TS ) VALUES ( #{record.id}, @@ -85,7 +87,8 @@ #{record.createdBy}, #{record.lastUpdatedBy}, #{record.createdTs}, - #{record.lastUpdatedTs} + #{record.lastUpdatedTs}, + #{record.lastValidatedTs} ) @@ -160,6 +163,19 @@ AWS_IAM_ROLE_ID = #{record.awsIamRoleId} + + UPDATE + AWS_IAM_ROLE_KMS_KEY + SET + LAST_UPDATED_BY = #{record.lastUpdatedBy}, + LAST_UPDATED_TS = #{record.lastUpdatedTs}, + LAST_VALIDATED_TS = #{record.lastValidatedTs} + WHERE + AWS_IAM_ROLE_ID = #{record.awsIamRoleId} + AND + AWS_REGION = #{record.awsRegion} + + DELETE FROM AWS_IAM_ROLE_PERMISSIONS diff --git a/src/main/resources/com/nike/cerberus/migration/V1.4.0.0__add_last_validated_field_to_aws_iam_kms_key.sql b/src/main/resources/com/nike/cerberus/migration/V1.4.0.0__add_last_validated_field_to_aws_iam_kms_key.sql new file mode 100644 index 000000000..6d01a5cbc --- /dev/null +++ b/src/main/resources/com/nike/cerberus/migration/V1.4.0.0__add_last_validated_field_to_aws_iam_kms_key.sql @@ -0,0 +1,5 @@ +ALTER TABLE AWS_IAM_ROLE_KMS_KEY + ADD COLUMN LAST_VALIDATED_TS DATETIME NOT NULL; + +UPDATE AWS_IAM_ROLE_KMS_KEY + SET LAST_VALIDATED_TS = LAST_UPDATED_TS; \ No newline at end of file diff --git a/src/test/java/com/nike/cerberus/dao/AwsIamRoleDaoTest.java b/src/test/java/com/nike/cerberus/dao/AwsIamRoleDaoTest.java index dc2cdfa9a..78a150c68 100644 --- a/src/test/java/com/nike/cerberus/dao/AwsIamRoleDaoTest.java +++ b/src/test/java/com/nike/cerberus/dao/AwsIamRoleDaoTest.java @@ -247,4 +247,14 @@ public void createIamRoleKmsKey_returns_record_count() { assertThat(actualCount).isEqualTo(recordCount); } + + @Test + public void updateIamRoleKmsKey_returns_record_count() { + final int recordCount = 1; + when(awsIamRoleMapper.updateIamRoleKmsKey(awsIamRoleKmsKeyRecord)).thenReturn(recordCount); + + final int actualCount = subject.updateIamRoleKmsKey(awsIamRoleKmsKeyRecord); + + assertThat(actualCount).isEqualTo(recordCount); + } } \ No newline at end of file diff --git a/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java b/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java index 279f14921..d87540443 100644 --- a/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java +++ b/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java @@ -22,21 +22,32 @@ import com.nike.cerberus.aws.KmsClientFactory; import com.nike.cerberus.dao.AwsIamRoleDao; import com.nike.cerberus.dao.SafeDepositBoxDao; +import com.nike.cerberus.domain.IamPrincipalCredentials; +import com.nike.cerberus.record.AwsIamRoleKmsKeyRecord; +import com.nike.cerberus.record.AwsIamRoleRecord; import com.nike.cerberus.security.VaultAuthPrincipal; import com.nike.cerberus.util.AwsIamRoleArnParser; import com.nike.cerberus.util.DateTimeSupplier; import com.nike.vault.client.VaultAdminClient; +import org.joda.time.DateTime; import org.junit.Before; import org.junit.Test; import org.mockito.InjectMocks; import org.mockito.Mock; +import java.time.OffsetDateTime; +import java.time.ZoneOffset; import java.util.Map; +import java.util.Optional; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; /** @@ -104,4 +115,49 @@ public void tests_that_generateCommonVaultPrincipalAuthMetadata_contains_expecte assertTrue(result.containsKey(VaultAuthPrincipal.METADATA_KEY_IS_ADMIN)); } + @Test + public void tests_that_getKeyId_only_validates_kms_policy_one_time_within_interval() { + + String principalArn = "principal arn"; + String region = "region"; + String iamRoleId = "iam role id"; + String kmsId = "kms id"; + String keyId = "key id"; + + // ensure that validate interval is passed + OffsetDateTime dateTime = OffsetDateTime.of(2016, 1, 1, 1, 1, 1, 1, ZoneOffset.UTC); + OffsetDateTime now = OffsetDateTime.now(); + + IamPrincipalCredentials iamPrincipalCredentials = new IamPrincipalCredentials(); + iamPrincipalCredentials.setIamPrincipalArn(principalArn); + iamPrincipalCredentials.setRegion(region); + + AwsIamRoleRecord awsIamRoleRecord = new AwsIamRoleRecord().setAwsIamRoleArn(principalArn); + awsIamRoleRecord.setAwsIamRoleArn(principalArn); + awsIamRoleRecord.setId(iamRoleId); + when(awsIamRoleDao.getIamRole(principalArn)).thenReturn(Optional.of(awsIamRoleRecord)); + + AwsIamRoleKmsKeyRecord awsIamRoleKmsKeyRecord = new AwsIamRoleKmsKeyRecord(); + awsIamRoleKmsKeyRecord.setId(kmsId); + awsIamRoleKmsKeyRecord.setAwsKmsKeyId(keyId); + awsIamRoleKmsKeyRecord.setLastValidatedTs(dateTime); + + when(awsIamRoleDao.getKmsKey(iamRoleId, region)).thenReturn(Optional.of(awsIamRoleKmsKeyRecord)); + + when(dateTimeSupplier.get()).thenReturn(now); + + authenticationService.getKeyId(iamPrincipalCredentials); + + // verify validate is called once interval has passed + verify(kmsService, times(1)).validatePolicy(keyId, principalArn, region); + + // reset interval + awsIamRoleKmsKeyRecord.setLastValidatedTs(now); + + // verify validate is not called when interval has not passed + authenticationService.getKeyId(iamPrincipalCredentials); + authenticationService.getKeyId(iamPrincipalCredentials); + verify(kmsService, times(1)).validatePolicy(keyId, principalArn, region); + } + } \ No newline at end of file diff --git a/src/test/java/com/nike/cerberus/service/KmsServiceTest.java b/src/test/java/com/nike/cerberus/service/KmsServiceTest.java index 6b6bc64fd..fc19d08bb 100644 --- a/src/test/java/com/nike/cerberus/service/KmsServiceTest.java +++ b/src/test/java/com/nike/cerberus/service/KmsServiceTest.java @@ -2,6 +2,7 @@ import com.amazonaws.services.kms.AWSKMSClient; import com.amazonaws.services.kms.model.*; +import com.nike.backstopper.exception.ApiException; import com.nike.cerberus.aws.KmsClientFactory; import com.nike.cerberus.dao.AwsIamRoleDao; import com.nike.cerberus.record.AwsIamRoleKmsKeyRecord; @@ -10,6 +11,7 @@ import org.junit.Test; import java.time.OffsetDateTime; +import java.util.Optional; import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.*; @@ -81,6 +83,7 @@ public void test_provisionKmsKey() { awsIamRoleKmsKeyRecord.setLastUpdatedBy(user); awsIamRoleKmsKeyRecord.setCreatedTs(dateTime); awsIamRoleKmsKeyRecord.setLastUpdatedTs(dateTime); + awsIamRoleKmsKeyRecord.setLastValidatedTs(dateTime); verify(awsIamRoleDao).createIamRoleKmsKey(awsIamRoleKmsKeyRecord); } @@ -109,4 +112,51 @@ public void test_validatePolicy() { verify(client, times(1)).getKeyPolicy(new GetKeyPolicyRequest().withKeyId(keyId).withPolicyName("default")); verify(kmsPolicyService, times(1)).isPolicyValid(policy, iamRoleArn); } + + @Test + public void test_updateKmsKey() { + + String iamRoleId = "role-id"; + String awsRegion = "aws-region"; + String user = "user"; + OffsetDateTime dateTime = OffsetDateTime.now(); + + AwsIamRoleKmsKeyRecord awsIamRoleKmsKeyRecord = new AwsIamRoleKmsKeyRecord(); + awsIamRoleKmsKeyRecord.setAwsRegion(awsRegion); + awsIamRoleKmsKeyRecord.setAwsIamRoleId(iamRoleId); + + AwsIamRoleKmsKeyRecord dbRecord = new AwsIamRoleKmsKeyRecord(); + dbRecord.setAwsRegion(awsRegion); + dbRecord.setAwsIamRoleId(iamRoleId); + dbRecord.setLastValidatedTs(OffsetDateTime.now()); + when(awsIamRoleDao.getKmsKey(iamRoleId, awsRegion)).thenReturn(Optional.of(dbRecord)); + + kmsService.updateKmsKey(awsIamRoleKmsKeyRecord, user, dateTime, dateTime); + + AwsIamRoleKmsKeyRecord expected = new AwsIamRoleKmsKeyRecord(); + expected.setAwsIamRoleId(iamRoleId); + expected.setLastUpdatedBy(user); + expected.setLastUpdatedTs(dateTime); + expected.setLastValidatedTs(dateTime); + expected.setAwsRegion(awsRegion); + + verify(awsIamRoleDao).updateIamRoleKmsKey(expected); + } + + @Test(expected = ApiException.class) + public void test_updateKmsKey_fails_when_record_not_found() { + + String iamRoleId = "role-id"; + String awsRegion = "aws-region"; + String user = "user"; + OffsetDateTime dateTime = OffsetDateTime.now(); + + AwsIamRoleKmsKeyRecord awsIamRoleKmsKeyRecord = new AwsIamRoleKmsKeyRecord(); + awsIamRoleKmsKeyRecord.setAwsRegion(awsRegion); + awsIamRoleKmsKeyRecord.setAwsIamRoleId(iamRoleId); + + when(awsIamRoleDao.getKmsKey(iamRoleId, awsRegion)).thenReturn(Optional.empty()); + + kmsService.updateKmsKey(awsIamRoleKmsKeyRecord, user, dateTime, dateTime); + } } \ No newline at end of file From 3f8b67e0520abe1a1f63ca9b8b18deae7f72914f Mon Sep 17 00:00:00 2001 From: Shaun Ford Date: Thu, 27 Apr 2017 13:43:26 -0700 Subject: [PATCH 2/3] Refactor KMS policy validation timeout logic --- .../service/AuthenticationService.java | 16 +-- .../com/nike/cerberus/service/KmsService.java | 97 ++++++++++++------ .../service/AuthenticationServiceTest.java | 21 ++-- .../nike/cerberus/service/KmsServiceTest.java | 98 +++++++++++++++---- 4 files changed, 157 insertions(+), 75 deletions(-) diff --git a/src/main/java/com/nike/cerberus/service/AuthenticationService.java b/src/main/java/com/nike/cerberus/service/AuthenticationService.java index 87308b4cb..574e6cf26 100644 --- a/src/main/java/com/nike/cerberus/service/AuthenticationService.java +++ b/src/main/java/com/nike/cerberus/service/AuthenticationService.java @@ -85,10 +85,8 @@ public class AuthenticationService { public static final String ADMIN_IAM_ROLES_PROPERTY = "cms.admin.roles"; public static final String USER_TOKEN_TTL_OVERRIDE = "cms.user.token.ttl.override"; public static final String IAM_TOKEN_TTL_OVERRIDE = "cms.iam.token.ttl.override"; - public static final String KMS_POLICY_VALIDATION_INTERVAL_OVERRIDE = "cms.kms.policy.validation.interval.millis.override"; public static final String LOOKUP_SELF_POLICY = "lookup-self"; public static final String DEFAULT_TOKEN_TTL = "1h"; - public static final Integer DEFAULT_KMS_VALIDATION_INTERVAL = 6000; // in milliseconds private final SafeDepositBoxDao safeDepositBoxDao; private final AwsIamRoleDao awsIamRoleDao; @@ -116,10 +114,6 @@ public class AuthenticationService { @Named(IAM_TOKEN_TTL_OVERRIDE) String iamTokenTTL = DEFAULT_TOKEN_TTL; - @Inject(optional=true) - @Named(KMS_POLICY_VALIDATION_INTERVAL_OVERRIDE) - Integer kmsKeyPolicyValidationInterval = DEFAULT_KMS_VALIDATION_INTERVAL; - @Inject public AuthenticationService(final SafeDepositBoxDao safeDepositBoxDao, final AwsIamRoleDao awsIamRoleDao, @@ -396,15 +390,7 @@ protected String getKeyId(IamPrincipalCredentials credentials) { } else { kmsKeyRecord = kmsKey.get(); kmsKeyId = kmsKeyRecord.getAwsKmsKeyId(); - String keyRegion = credentials.getRegion(); - if (ChronoUnit.MILLIS.between(kmsKeyRecord.getLastValidatedTs(), now) >= kmsKeyPolicyValidationInterval) { - try { - kmsService.validatePolicy(kmsKeyId, credentials.getIamPrincipalArn(), keyRegion); - kmsService.updateKmsKey(kmsKeyRecord, SYSTEM_USER, now, now); - } catch (ApiException ae) { - logger.warn("Could not validate KMS policy. API limit may have been reached for validate call."); - } - } + kmsService.validatePolicy(kmsKeyRecord, credentials.getIamPrincipalArn()); } return kmsKeyId; diff --git a/src/main/java/com/nike/cerberus/service/KmsService.java b/src/main/java/com/nike/cerberus/service/KmsService.java index 819b225d8..418475792 100644 --- a/src/main/java/com/nike/cerberus/service/KmsService.java +++ b/src/main/java/com/nike/cerberus/service/KmsService.java @@ -26,11 +26,13 @@ import com.amazonaws.services.kms.model.KeyMetadata; import com.amazonaws.services.kms.model.KeyUsageType; import com.amazonaws.services.kms.model.PutKeyPolicyRequest; +import com.google.inject.name.Named; import com.nike.backstopper.exception.ApiException; import com.nike.cerberus.aws.KmsClientFactory; import com.nike.cerberus.dao.AwsIamRoleDao; import com.nike.cerberus.error.DefaultApiError; import com.nike.cerberus.record.AwsIamRoleKmsKeyRecord; +import com.nike.cerberus.util.DateTimeSupplier; import com.nike.cerberus.util.UuidSupplier; import org.mybatis.guice.transactional.Transactional; import org.slf4j.Logger; @@ -39,8 +41,11 @@ import javax.inject.Inject; import javax.inject.Singleton; import java.time.OffsetDateTime; +import java.time.temporal.ChronoUnit; import java.util.Optional; +import static com.nike.cerberus.service.AuthenticationService.SYSTEM_USER; + /** * Abstracts interactions with the AWS KMS service. */ @@ -50,6 +55,9 @@ public class KmsService { private final Logger logger = LoggerFactory.getLogger(this.getClass()); private static final String KMS_ALIAS_FORMAT = "alias/cerberus/%s"; + public static final String KMS_POLICY_VALIDATION_INTERVAL_OVERRIDE = "cms.kms.policy.validation.interval.millis.override"; + public static final Integer DEFAULT_KMS_VALIDATION_INTERVAL = 6000; // in milliseconds + private final AwsIamRoleDao awsIamRoleDao; @@ -59,15 +67,23 @@ public class KmsService { private final KmsPolicyService kmsPolicyService; + private final DateTimeSupplier dateTimeSupplier; + + @com.google.inject.Inject(optional=true) + @Named(KMS_POLICY_VALIDATION_INTERVAL_OVERRIDE) + Integer kmsKeyPolicyValidationInterval = DEFAULT_KMS_VALIDATION_INTERVAL; + @Inject public KmsService(final AwsIamRoleDao awsIamRoleDao, final UuidSupplier uuidSupplier, final KmsClientFactory kmsClientFactory, - final KmsPolicyService kmsPolicyService) { + final KmsPolicyService kmsPolicyService, + final DateTimeSupplier dateTimeSupplier) { this.awsIamRoleDao = awsIamRoleDao; this.uuidSupplier = uuidSupplier; this.kmsClientFactory = kmsClientFactory; this.kmsPolicyService = kmsPolicyService; + this.dateTimeSupplier = dateTimeSupplier; } /** @@ -119,13 +135,21 @@ public String provisionKmsKey(final String iamRoleId, return result.getKeyMetadata().getArn(); } + /** + * Updates the KMS CMK record for the specified IAM role and region + * @param awsIamRoleId The IAM role that this CMK will be associated with + * @param awsRegion The region to provision the key in + * @param user The user requesting it + * @param lastedUpdatedTs The date when the record was last updated + * @param lastValidatedTs The date when the record was last validated + */ @Transactional - public void updateKmsKey(final AwsIamRoleKmsKeyRecord awsIamRoleKmsKeyRecord, + public void updateKmsKey(final String awsIamRoleId, + final String awsRegion, final String user, - final OffsetDateTime dateTime, + final OffsetDateTime lastedUpdatedTs, final OffsetDateTime lastValidatedTs) { - final Optional kmsKey = - awsIamRoleDao.getKmsKey(awsIamRoleKmsKeyRecord.getAwsIamRoleId(), awsIamRoleKmsKeyRecord.getAwsRegion()); + final Optional kmsKey = awsIamRoleDao.getKmsKey(awsIamRoleId, awsRegion); if (!kmsKey.isPresent()) { throw ApiException.newBuilder() @@ -139,7 +163,7 @@ public void updateKmsKey(final AwsIamRoleKmsKeyRecord awsIamRoleKmsKeyRecord, AwsIamRoleKmsKeyRecord updatedKmsKeyRecord = new AwsIamRoleKmsKeyRecord(); updatedKmsKeyRecord.setAwsIamRoleId(kmsKeyRecord.getAwsIamRoleId()); updatedKmsKeyRecord.setLastUpdatedBy(user); - updatedKmsKeyRecord.setLastUpdatedTs(dateTime); + updatedKmsKeyRecord.setLastUpdatedTs(lastedUpdatedTs); updatedKmsKeyRecord.setLastValidatedTs(lastValidatedTs); updatedKmsKeyRecord.setAwsRegion(kmsKeyRecord.getAwsRegion()); awsIamRoleDao.updateIamRoleKmsKey(updatedKmsKeyRecord); @@ -162,34 +186,51 @@ protected String getAliasName(String awsIamRoleKmsKeyId) { * statement has been deleted the ARN is replaced by the ID. We can validate that principal matches an ARN pattern * or recreate the policy. * - * @param keyId - The CMK Id to validate the policies on. + * @param kmsKeyRecord - The CMK record to validate policy on * @param iamPrincipalArn - The principal ARN that should have decrypt permission - * @param kmsCMKRegion - The region that the key was provisioned for */ - public void validatePolicy(String keyId, String iamPrincipalArn, String kmsCMKRegion) { + public void validatePolicy(AwsIamRoleKmsKeyRecord kmsKeyRecord, String iamPrincipalArn) { + + if (! kmsPolicyNeedsValidation(kmsKeyRecord)) { + return; + } + + String kmsCMKRegion = kmsKeyRecord.getAwsRegion(); + String awsKmsKeyArn = kmsKeyRecord.getAwsKmsKeyId(); AWSKMSClient kmsClient = kmsClientFactory.getClient(kmsCMKRegion); - GetKeyPolicyResult policyResult = null; try { - policyResult = kmsClient.getKeyPolicy(new GetKeyPolicyRequest().withKeyId(keyId).withPolicyName("default")); + GetKeyPolicyResult policyResult = kmsClient.getKeyPolicy(new GetKeyPolicyRequest().withKeyId(awsKmsKeyArn).withPolicyName("default")); + + if (!kmsPolicyService.isPolicyValid(policyResult.getPolicy(), iamPrincipalArn)) { + logger.info("The KMS key: {} generated for IAM principal: {} contained an invalid policy, regenerating", + awsKmsKeyArn, iamPrincipalArn); + String updatedPolicy = kmsPolicyService.generateStandardKmsPolicy(iamPrincipalArn); + kmsClient.putKeyPolicy(new PutKeyPolicyRequest() + .withKeyId(awsKmsKeyArn) + .withPolicyName("default") + .withPolicy(updatedPolicy) + ); + } + + // update last validated timestamp + OffsetDateTime now = dateTimeSupplier.get(); + updateKmsKey(kmsKeyRecord.getAwsIamRoleId(), kmsCMKRegion, SYSTEM_USER, now, now); } catch (AmazonServiceException e) { - throw ApiException.newBuilder() - .withApiErrors(DefaultApiError.FAILED_TO_VALIDATE_KMS_KEY_POLICY) - .withExceptionCause(e) - .withExceptionMessage( - String.format("Failed to validate KMS key policy for keyId: " + - "%s for IAM principal: %s in region: %s", keyId, iamPrincipalArn, kmsCMKRegion)) - .build(); + logger.warn(String.format("Failed to validate KMS policy for keyId: %s for IAM principal: %s in region: %s. API limit" + + " may have been reached for validate call.", awsKmsKeyArn, iamPrincipalArn, kmsCMKRegion), e); } + } - if (!kmsPolicyService.isPolicyValid(policyResult.getPolicy(), iamPrincipalArn)) { - logger.info("The KMS key: {} generated for IAM principal: {} contained an invalid policy, regenerating", - keyId, iamPrincipalArn); - String updatedPolicy = kmsPolicyService.generateStandardKmsPolicy(iamPrincipalArn); - kmsClient.putKeyPolicy(new PutKeyPolicyRequest() - .withKeyId(keyId) - .withPolicyName("default") - .withPolicy(updatedPolicy) - ); - } + /** + * Determines if given KMS policy should be validated + * @param kmsKeyRecord - KMS key record to check for validation + * @return True if needs validation, False if not + */ + protected boolean kmsPolicyNeedsValidation(AwsIamRoleKmsKeyRecord kmsKeyRecord) { + + OffsetDateTime now = dateTimeSupplier.get(); + long timeSinceLastValidatedInMillis = ChronoUnit.MILLIS.between(kmsKeyRecord.getLastValidatedTs(), now); + + return timeSinceLastValidatedInMillis >= kmsKeyPolicyValidationInterval; } } diff --git a/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java b/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java index d87540443..be1b993f2 100644 --- a/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java +++ b/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java @@ -121,8 +121,8 @@ public void tests_that_getKeyId_only_validates_kms_policy_one_time_within_interv String principalArn = "principal arn"; String region = "region"; String iamRoleId = "iam role id"; - String kmsId = "kms id"; - String keyId = "key id"; + String kmsKeyId = "kms id"; + String cmkId = "key id"; // ensure that validate interval is passed OffsetDateTime dateTime = OffsetDateTime.of(2016, 1, 1, 1, 1, 1, 1, ZoneOffset.UTC); @@ -138,26 +138,19 @@ public void tests_that_getKeyId_only_validates_kms_policy_one_time_within_interv when(awsIamRoleDao.getIamRole(principalArn)).thenReturn(Optional.of(awsIamRoleRecord)); AwsIamRoleKmsKeyRecord awsIamRoleKmsKeyRecord = new AwsIamRoleKmsKeyRecord(); - awsIamRoleKmsKeyRecord.setId(kmsId); - awsIamRoleKmsKeyRecord.setAwsKmsKeyId(keyId); + awsIamRoleKmsKeyRecord.setId(kmsKeyId); + awsIamRoleKmsKeyRecord.setAwsKmsKeyId(cmkId); awsIamRoleKmsKeyRecord.setLastValidatedTs(dateTime); when(awsIamRoleDao.getKmsKey(iamRoleId, region)).thenReturn(Optional.of(awsIamRoleKmsKeyRecord)); when(dateTimeSupplier.get()).thenReturn(now); - authenticationService.getKeyId(iamPrincipalCredentials); + String result = authenticationService.getKeyId(iamPrincipalCredentials); // verify validate is called once interval has passed - verify(kmsService, times(1)).validatePolicy(keyId, principalArn, region); - - // reset interval - awsIamRoleKmsKeyRecord.setLastValidatedTs(now); - - // verify validate is not called when interval has not passed - authenticationService.getKeyId(iamPrincipalCredentials); - authenticationService.getKeyId(iamPrincipalCredentials); - verify(kmsService, times(1)).validatePolicy(keyId, principalArn, region); + assertEquals(cmkId, result); + verify(kmsService, times(1)).validatePolicy(awsIamRoleKmsKeyRecord, principalArn); } } \ No newline at end of file diff --git a/src/test/java/com/nike/cerberus/service/KmsServiceTest.java b/src/test/java/com/nike/cerberus/service/KmsServiceTest.java index fc19d08bb..575ca9242 100644 --- a/src/test/java/com/nike/cerberus/service/KmsServiceTest.java +++ b/src/test/java/com/nike/cerberus/service/KmsServiceTest.java @@ -1,16 +1,19 @@ package com.nike.cerberus.service; +import com.amazonaws.AmazonServiceException; import com.amazonaws.services.kms.AWSKMSClient; import com.amazonaws.services.kms.model.*; import com.nike.backstopper.exception.ApiException; import com.nike.cerberus.aws.KmsClientFactory; import com.nike.cerberus.dao.AwsIamRoleDao; import com.nike.cerberus.record.AwsIamRoleKmsKeyRecord; +import com.nike.cerberus.util.DateTimeSupplier; import com.nike.cerberus.util.UuidSupplier; import org.junit.Before; import org.junit.Test; import java.time.OffsetDateTime; +import java.time.ZoneOffset; import java.util.Optional; import static org.junit.Assert.assertEquals; @@ -22,6 +25,7 @@ public class KmsServiceTest { private UuidSupplier uuidSupplier; private KmsClientFactory kmsClientFactory; private KmsPolicyService kmsPolicyService; + private DateTimeSupplier dateTimeSupplier; private KmsService kmsService; @@ -31,7 +35,9 @@ public void setup() { uuidSupplier = mock(UuidSupplier.class); kmsClientFactory = mock(KmsClientFactory.class); kmsPolicyService = mock(KmsPolicyService.class); - kmsService = new KmsService(awsIamRoleDao, uuidSupplier, kmsClientFactory, kmsPolicyService); + dateTimeSupplier = mock(DateTimeSupplier.class); + + kmsService = new KmsService(awsIamRoleDao, uuidSupplier, kmsClientFactory, kmsPolicyService, dateTimeSupplier); } @Test @@ -93,24 +99,88 @@ public void test_getAliasName() { } @Test - public void test_validatePolicy() { + public void test_validatePolicy_validates_policy_when_validate_interval_has_passed() { + String kmsKeyArn = "kms key arn"; + String awsIamRoleRecordId = "aws iam role record id"; + String kmsCMKRegion = "kmsCMKRegion"; + String policy = "policy"; + OffsetDateTime lastValidated = OffsetDateTime.of(2016, 1, 1, 1, 1, + 1, 1, ZoneOffset.UTC); + OffsetDateTime now = OffsetDateTime.now(); + + AWSKMSClient client = mock(AWSKMSClient.class); + when(kmsClientFactory.getClient(kmsCMKRegion)).thenReturn(client); + + GetKeyPolicyResult result = mock(GetKeyPolicyResult.class); + when(result.getPolicy()).thenReturn(policy); + when(client.getKeyPolicy(new GetKeyPolicyRequest().withKeyId(kmsKeyArn) + .withPolicyName("default"))).thenReturn(result); + when(kmsPolicyService.isPolicyValid(policy, kmsKeyArn)).thenReturn(true); + + AwsIamRoleKmsKeyRecord kmsKey = mock(AwsIamRoleKmsKeyRecord.class); + when(kmsKey.getAwsIamRoleId()).thenReturn(awsIamRoleRecordId); + when(kmsKey.getAwsKmsKeyId()).thenReturn(kmsKeyArn); + when(kmsKey.getAwsRegion()).thenReturn(kmsCMKRegion); + when(kmsKey.getLastValidatedTs()).thenReturn(lastValidated); + when(awsIamRoleDao.getKmsKey(awsIamRoleRecordId, kmsCMKRegion)).thenReturn(Optional.of(kmsKey)); + + when(dateTimeSupplier.get()).thenReturn(now); + kmsService.validatePolicy(kmsKey, kmsKeyArn); + + verify(client, times(1)).getKeyPolicy(new GetKeyPolicyRequest().withKeyId(kmsKeyArn) + .withPolicyName("default")); + verify(kmsPolicyService, times(1)).isPolicyValid(policy, kmsKeyArn); + } + + @Test + public void test_validatePolicy_validates_policy_when_validate_interval_has_not_passed() { + String awsKmsKeyArn = "aws kms key arn"; + String iamPrincipalArn = "arn"; + String awsIamRoleRecordId = "aws iam role record id"; + String kmsCMKRegion = "kmsCMKRegion"; + OffsetDateTime now = OffsetDateTime.now(); + + AwsIamRoleKmsKeyRecord kmsKey = mock(AwsIamRoleKmsKeyRecord.class); + when(kmsKey.getAwsKmsKeyId()).thenReturn(awsKmsKeyArn); + when(kmsKey.getAwsIamRoleId()).thenReturn(awsIamRoleRecordId); + when(kmsKey.getAwsRegion()).thenReturn(kmsCMKRegion); + when(kmsKey.getLastValidatedTs()).thenReturn(now); + + when(dateTimeSupplier.get()).thenReturn(now); + kmsService.validatePolicy(kmsKey, iamPrincipalArn); + + verify(kmsClientFactory, never()).getClient(anyString()); + verify(kmsPolicyService, never()).isPolicyValid(anyString(), anyString()); + } + + @Test + public void test_validatePolicy_does_not_throw_error_when_cannot_validate() { String keyId = "key-id"; - String iamRoleArn = "arn"; + String iamPrincipalArn = "arn"; String kmsCMKRegion = "kmsCMKRegion"; String policy = "policy"; + OffsetDateTime lastValidated = OffsetDateTime.of(2016, 1, 1, 1, 1, + 1, 1, ZoneOffset.UTC); + OffsetDateTime now = OffsetDateTime.now(); + when(dateTimeSupplier.get()).thenReturn(now); + + AwsIamRoleKmsKeyRecord kmsKey = mock(AwsIamRoleKmsKeyRecord.class); + when(kmsKey.getAwsKmsKeyId()).thenReturn(keyId); + when(kmsKey.getAwsIamRoleId()).thenReturn(iamPrincipalArn); + when(kmsKey.getAwsRegion()).thenReturn(kmsCMKRegion); + when(kmsKey.getLastValidatedTs()).thenReturn(lastValidated); AWSKMSClient client = mock(AWSKMSClient.class); when(kmsClientFactory.getClient(kmsCMKRegion)).thenReturn(client); GetKeyPolicyResult result = mock(GetKeyPolicyResult.class); when(result.getPolicy()).thenReturn(policy); - when(client.getKeyPolicy(new GetKeyPolicyRequest().withKeyId(keyId).withPolicyName("default"))).thenReturn(result); - when(kmsPolicyService.isPolicyValid(policy, iamRoleArn)).thenReturn(true); + when(client.getKeyPolicy(new GetKeyPolicyRequest().withKeyId(keyId).withPolicyName("default"))).thenThrow(AmazonServiceException.class); - kmsService.validatePolicy(keyId, iamRoleArn, kmsCMKRegion); + kmsService.validatePolicy(kmsKey, iamPrincipalArn); - verify(client, times(1)).getKeyPolicy(new GetKeyPolicyRequest().withKeyId(keyId).withPolicyName("default")); - verify(kmsPolicyService, times(1)).isPolicyValid(policy, iamRoleArn); + verify(kmsPolicyService, never()).isPolicyValid(policy, iamPrincipalArn); + verify(client, never()).putKeyPolicy(anyObject()); } @Test @@ -121,17 +191,13 @@ public void test_updateKmsKey() { String user = "user"; OffsetDateTime dateTime = OffsetDateTime.now(); - AwsIamRoleKmsKeyRecord awsIamRoleKmsKeyRecord = new AwsIamRoleKmsKeyRecord(); - awsIamRoleKmsKeyRecord.setAwsRegion(awsRegion); - awsIamRoleKmsKeyRecord.setAwsIamRoleId(iamRoleId); - AwsIamRoleKmsKeyRecord dbRecord = new AwsIamRoleKmsKeyRecord(); dbRecord.setAwsRegion(awsRegion); dbRecord.setAwsIamRoleId(iamRoleId); dbRecord.setLastValidatedTs(OffsetDateTime.now()); when(awsIamRoleDao.getKmsKey(iamRoleId, awsRegion)).thenReturn(Optional.of(dbRecord)); - kmsService.updateKmsKey(awsIamRoleKmsKeyRecord, user, dateTime, dateTime); + kmsService.updateKmsKey(iamRoleId, awsRegion, user, dateTime, dateTime); AwsIamRoleKmsKeyRecord expected = new AwsIamRoleKmsKeyRecord(); expected.setAwsIamRoleId(iamRoleId); @@ -151,12 +217,8 @@ public void test_updateKmsKey_fails_when_record_not_found() { String user = "user"; OffsetDateTime dateTime = OffsetDateTime.now(); - AwsIamRoleKmsKeyRecord awsIamRoleKmsKeyRecord = new AwsIamRoleKmsKeyRecord(); - awsIamRoleKmsKeyRecord.setAwsRegion(awsRegion); - awsIamRoleKmsKeyRecord.setAwsIamRoleId(iamRoleId); - when(awsIamRoleDao.getKmsKey(iamRoleId, awsRegion)).thenReturn(Optional.empty()); - kmsService.updateKmsKey(awsIamRoleKmsKeyRecord, user, dateTime, dateTime); + kmsService.updateKmsKey(iamRoleId, awsRegion, user, dateTime, dateTime); } } \ No newline at end of file From 525fa3d702e3906a910f74e4bfae7b139a17eb66 Mon Sep 17 00:00:00 2001 From: Shaun Ford Date: Thu, 27 Apr 2017 14:36:01 -0700 Subject: [PATCH 3/3] Update README --- README.md | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 539c3fd20..9bdb15e23 100644 --- a/README.md +++ b/README.md @@ -61,18 +61,22 @@ There are a few parameters that need to be configured for CMS to run properly, t property | required | notes --------------------------- | -------- | ---------- -JDBC.url | Yes | The JDBC url for the mysql db -JDBC.username | Yes | The JDBC user name for the mysql db -JDBC.password | Yes | The JDBC JDBC.password for the mysql db -root.user.arn | Yes | The arn for the root AWS user, needed to make the KMS keys deletable. -admin.role.arn | Yes | The arn for an AWS user, needed to make the KMS keys deletable. -cms.role.arn | Yes | The arn for the Instance profile for CMS instances, so they can admin KMS keys that they create. -cms.admin.group | Yes | Group that user can be identified by to get admin privileges, currently this just enables users to access `/v1/metadata` see API.md -cms.admin.roles | No | Comma seperated list of ARNs that can auth and access admin endpoints. -cms.auth.connector | Yes | The user authentication connector implementation to use for user auth. -cms.user.token.ttl.override | No | By default user tokens are created with a TTL of 1h, you can override that with this param -cms.iam.token.ttl.override | No | By default IAM tokens are created with a TTL of 1h, you can override that with this param -cms.kms.policy.validation.interval.millis.override | No | By default CMS validates KMS key policies no more than once per minute, you can override that with this param +JDBC.url | Yes | The JDBC url for the mysql db +JDBC.username | Yes | The JDBC user name for the mysql db +JDBC.password | Yes | The JDBC JDBC.password for the mysql db +root.user.arn | Yes | The arn for the root AWS user, needed to make the KMS keys deletable. +admin.role.arn | Yes | The arn for an AWS user, needed to make the KMS keys deletable. +cms.role.arn | Yes | The arn for the Instance profile for CMS instances, so they can admin KMS keys that they create. +cms.admin.group | Yes | Group that user can be identified by to get admin privileges, currently this just enables users to access `/v1/metadata` see API.md +cms.admin.roles | No | Comma separated list of ARNs that can auth and access admin endpoints. +cms.auth.connector | Yes | The user authentication connector implementation to use for user auth. +cms.user.token.ttl.override | No | By default user tokens are created with a TTL of 1h, you can override that with this param +cms.iam.token.ttl.override | No | By default IAM tokens are created with a TTL of 1h, you can override that with this param +cms.kms.policy.validation.interval.millis.override | No | By default CMS validates KMS key policies no more than once per minute, you can override that with this param + +KMS Policies are bound to IAM Principal IDs rather than ARNs themselves. Because of this, we validate the policy at authentication time +to ensure that if an IAM role has been deleted and re-created, that we grant access to the new principal ID. +The API limit for this call is low, so the `cms.kms.policy.validation.interval.millis.override` property is used to throttle this validation. For local dev see `Running CMS Locally`.