Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Commit

Permalink
@Transational on DB update methods + validate keys
Browse files Browse the repository at this point in the history 
* Add transactional annotations where missing on methods that update records in the database
* Validate that KMS keys are usable during authentication. If not, then delete the key record
  • Loading branch information
@sdford
sdford committed Jul 13, 2017
1 parent bff872b commit 25d0168
Show file tree
Hide file tree
Showing 5 changed files with 214 additions and 13 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/nike/cerberus/service/AuthenticationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ protected String getKeyId(IamPrincipalCredentials credentials) {
kmsKeyRecord = kmsKey.get();
kmsKeyId = kmsKeyRecord.getAwsKmsKeyId();
kmsService.validatePolicy(kmsKeyRecord, iamPrincipalArn);
kmsService.validateKmsKeyIsUsable(kmsKeyRecord, iamPrincipalArn);
}

return kmsKeyId;
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/com/nike/cerberus/service/CleanUpService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.nike.cerberus.record.AwsIamRoleKmsKeyRecord;
import com.nike.cerberus.record.AwsIamRoleRecord;
import com.nike.cerberus.util.DateTimeSupplier;
import org.mybatis.guice.transactional.Transactional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -102,7 +103,7 @@ protected void cleanUpInactiveAndOrphanedKmsKeys(final int kmsKeysInactiveAfterN
kmsKeyArn, kmsKeyRegion, kmsKeyRecord.getLastValidatedTs());
kmsService.validatePolicyAllowsCMSToDeleteCMK(kmsKeyArn, kmsKeyRegion);
kmsService.scheduleKmsKeyDeletion(kmsKeyArn, kmsKeyRegion, SOONEST_A_KMS_KEY_CAN_BE_DELETED);
awsIamRoleDao.deleteKmsKeyById(kmsKeyRecord.getId());
kmsService.deleteKmsKeyById(kmsKeyRecord.getId());
TimeUnit.SECONDS.sleep(sleepInSeconds);
} catch (InterruptedException ie) {
logger.error("Timeout between KMS key deletion was interrupted", ie);
Expand All @@ -120,6 +121,7 @@ protected void cleanUpInactiveAndOrphanedKmsKeys(final int kmsKeysInactiveAfterN
/**
* Delete all IAM role records that are no longer associated with an SDB.
*/
@Transactional
protected void cleanUpOrphanedIamRoles() {

// get orphaned iam role ids
Expand Down
75 changes: 66 additions & 9 deletions src/main/java/com/nike/cerberus/service/KmsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
import com.amazonaws.services.kms.model.CreateAliasRequest;
import com.amazonaws.services.kms.model.CreateKeyRequest;
import com.amazonaws.services.kms.model.CreateKeyResult;
import com.amazonaws.services.kms.model.DescribeKeyRequest;
import com.amazonaws.services.kms.model.GetKeyPolicyRequest;
import com.amazonaws.services.kms.model.KeyMetadata;
import com.amazonaws.services.kms.model.KeyState;
import com.amazonaws.services.kms.model.KeyUsageType;
import com.amazonaws.services.kms.model.NotFoundException;
import com.amazonaws.services.kms.model.PutKeyPolicyRequest;
Expand All @@ -35,6 +37,7 @@
import com.nike.cerberus.record.AwsIamRoleKmsKeyRecord;
import com.nike.cerberus.util.DateTimeSupplier;
import com.nike.cerberus.util.UuidSupplier;
import org.apache.commons.lang3.StringUtils;
import org.mybatis.guice.transactional.Transactional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -173,6 +176,11 @@ public void updateKmsKey(final String awsIamRoleId,
awsIamRoleDao.updateIamRoleKmsKey(updatedKmsKeyRecord);
}

@Transactional
public void deleteKmsKeyById(final String kmsKeyId) {
awsIamRoleDao.deleteKmsKeyById(kmsKeyId);
}

protected String getAliasName(String awsIamRoleKmsKeyId) {
return String.format(KMS_ALIAS_FORMAT, awsIamRoleKmsKeyId);
}
Expand Down Expand Up @@ -220,7 +228,7 @@ public void validatePolicy(AwsIamRoleKmsKeyRecord kmsKeyRecord, String iamPrinci
logger.warn("Failed to validate KMS policy because the KMS key did not exist, but the key record did." +
"Deleting the key record to prevent this from failing again: keyId: {} for IAM principal: {} in region: {}",
awsKmsKeyArn, iamPrincipalArn, kmsCMKRegion, nfe);
awsIamRoleDao.deleteKmsKeyById(kmsKeyRecord.getId());
deleteKmsKeyById(kmsKeyRecord.getId());
} catch (AmazonServiceException e) {
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);
Expand All @@ -230,13 +238,13 @@ public void validatePolicy(AwsIamRoleKmsKeyRecord kmsKeyRecord, String iamPrinci
/**
* Validate the the CMS IAM role has permissions to ScheduleKeyDeletion and CancelKeyDeletion for the give CMK
* if not, then update the policy to give the appropriate permissions.
* @param awsKmsKeyArn - The ARN of the CMK to validate
* @param awsKmsKeyId - The ARN of the CMK to validate
* @param kmsCMKRegion - The region that the CMK exists in
*/
protected void validatePolicyAllowsCMSToDeleteCMK(String awsKmsKeyArn, String kmsCMKRegion) {
protected void validatePolicyAllowsCMSToDeleteCMK(String awsKmsKeyId, String kmsCMKRegion) {

try {
String policyJson = getKmsKeyPolicy(awsKmsKeyArn, kmsCMKRegion);
String policyJson = getKmsKeyPolicy(awsKmsKeyId, kmsCMKRegion);

if (!kmsPolicyService.cmsHasKeyDeletePermissions(policyJson)) {
// Overwrite the policy statement for CMS only, instead of regenerating the entire policy because regenerating
Expand All @@ -247,25 +255,58 @@ protected void validatePolicyAllowsCMSToDeleteCMK(String awsKmsKeyArn, String km
// of and ARN, rendering the policy invalid. So delete the consumer statement here just in case
String updatedPolicyWithNoConsumer = kmsPolicyService.removeConsumerPrincipalFromPolicy(updatedPolicy);

updateKmsKeyPolicy(updatedPolicyWithNoConsumer, awsKmsKeyArn, kmsCMKRegion);
updateKmsKeyPolicy(updatedPolicyWithNoConsumer, awsKmsKeyId, kmsCMKRegion);
}
} catch (AmazonServiceException ase) {
logger.error("Failed to validate that CMS can delete the given KMS key, ARN: {}, region: {}", awsKmsKeyArn, kmsCMKRegion, ase);
logger.error("Failed to validate that CMS can delete the given KMS key, ARN: {}, region: {}", awsKmsKeyId, kmsCMKRegion, ase);
} catch (IllegalArgumentException iae) {
logger.error("Failed to add ScheduleKeyDeletion to key policy for CMK: {}, because the policy does" +
"not contain any allow statements for the CMS role", awsKmsKeyArn, iae);
"not contain any allow statements for the CMS role", awsKmsKeyId, iae);
}

}

/**
* Validate that the KMS key is usable (i.e. the key is not disabled or scheduled for deletion).
*
* If the key is not usable, then delete the DB record of the key, so that the caller will be given a new KMS key
* the next time they authenticate.
*
* @param kmsKeyRecord - Record of the KMS key to validate
* @param iamPrincipalArn - ARN of the authenticating IAM principal
*/
protected void validateKmsKeyIsUsable(AwsIamRoleKmsKeyRecord kmsKeyRecord, String iamPrincipalArn) {

String kmsCMKRegion = kmsKeyRecord.getAwsRegion();
String awsKmsKeyArn = kmsKeyRecord.getAwsKmsKeyId();

if (kmsKeyIsDisabledOrScheduledForDeletion(awsKmsKeyArn, kmsCMKRegion)) {
logger.warn("The KMS key ID: {} for IAM principal: {} is disabled or scheduled for deletion. Deleting the" +
"key record to prevent this from failing again: keyId: {} for IAM principal: {} in region: {}.",
awsKmsKeyArn, iamPrincipalArn, kmsCMKRegion);
deleteKmsKeyById(kmsKeyRecord.getId());
}
}

/**
* Return true if the KMS key is disabled or scheduled for deletion, false if not.
*/
protected boolean kmsKeyIsDisabledOrScheduledForDeletion(String awsKmsKeyId, String kmsCMKRegion) {

String keyState = getKmsKeyState(awsKmsKeyId, kmsCMKRegion);

return StringUtils.equals(keyState, KeyState.PendingDeletion.toString()) ||
StringUtils.equals(keyState, KeyState.Disabled.toString());
}

/**
* Gets the KMS key policy from AWS for the given CMK
*/
protected String getKmsKeyPolicy(String awsKmsKeyArn, String kmsCMKRegion) {
protected String getKmsKeyPolicy(String kmsKeyId, String kmsCMKRegion) {

AWSKMSClient kmsClient = kmsClientFactory.getClient(kmsCMKRegion);

GetKeyPolicyRequest request = new GetKeyPolicyRequest().withKeyId(awsKmsKeyArn).withPolicyName("default");
GetKeyPolicyRequest request = new GetKeyPolicyRequest().withKeyId(kmsKeyId).withPolicyName("default");

return kmsClient.getKeyPolicy(request).getPolicy();
}
Expand All @@ -284,6 +325,22 @@ protected void updateKmsKeyPolicy(String updatedPolicyJson, String awsKmsKeyArn,
);
}

/**
* Get the state of the KMS key
* @param kmsKeyId - The AWS KMS Key ID
* @param region - The KMS key region
* @return - KMS key state
*/
protected String getKmsKeyState(String kmsKeyId, String region) {

AWSKMSClient kmsClient = kmsClientFactory.getClient(region);
DescribeKeyRequest request = new DescribeKeyRequest().withKeyId(kmsKeyId);

return kmsClient.describeKey(request)
.getKeyMetadata()
.getKeyState();
}

/**
* Delete a CMK in AWS
* @param kmsKeyId - The AWS KMS Key ID
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/nike/cerberus/service/CleanUpServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void test_that_cleanUpInactiveAndOrphanedKmsKeys_succeeds() {
cleanUpService.cleanUpInactiveAndOrphanedKmsKeys(inactivePeriod, 0);

verify(awsIamRoleDao).getInactiveOrOrphanedKmsKeys(inactiveCutoffDate);
verify(awsIamRoleDao).deleteKmsKeyById(keyRecordId);
verify(kmsService).deleteKmsKeyById(keyRecordId);
verify(kmsService).scheduleKmsKeyDeletion(awsKeyId, keyRegion, SOONEST_A_KMS_KEY_CAN_BE_DELETED);
}

Expand Down
145 changes: 143 additions & 2 deletions src/test/java/com/nike/cerberus/service/KmsServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,15 @@

import com.amazonaws.AmazonServiceException;
import com.amazonaws.services.kms.AWSKMSClient;
import com.amazonaws.services.kms.model.*;
import com.amazonaws.services.kms.model.CreateAliasRequest;
import com.amazonaws.services.kms.model.CreateKeyRequest;
import com.amazonaws.services.kms.model.CreateKeyResult;
import com.amazonaws.services.kms.model.DescribeKeyResult;
import com.amazonaws.services.kms.model.GetKeyPolicyRequest;
import com.amazonaws.services.kms.model.GetKeyPolicyResult;
import com.amazonaws.services.kms.model.KeyMetadata;
import com.amazonaws.services.kms.model.KeyState;
import com.amazonaws.services.kms.model.KeyUsageType;
import com.nike.backstopper.exception.ApiException;
import com.nike.cerberus.aws.KmsClientFactory;
import com.nike.cerberus.dao.AwsIamRoleDao;
Expand All @@ -17,7 +25,15 @@
import java.util.Optional;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.*;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.anyObject;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class KmsServiceTest {

Expand Down Expand Up @@ -221,4 +237,129 @@ public void test_updateKmsKey_fails_when_record_not_found() {

kmsService.updateKmsKey(iamRoleId, awsRegion, user, dateTime, dateTime);
}

@Test
public void test_getKmsKeyState_happy() {
String awsRegion = "aws region";

String kmsKeyId = "kms key id";
String state = "state";
AWSKMSClient kmsClient = mock(AWSKMSClient.class);
when(kmsClientFactory.getClient(awsRegion)).thenReturn(kmsClient);
when(kmsClient.describeKey(anyObject())).thenReturn(
new DescribeKeyResult()
.withKeyMetadata(
new KeyMetadata()
.withKeyState(state)));

String result = kmsService.getKmsKeyState(kmsKeyId, awsRegion);

assertEquals(state, result);
}

@Test
public void test_validateKmsKeyIsUsable_returns_true_when_state_is_pending_deletion() {
String keyId = "key id";
String awsRegion = "aws region";

AWSKMSClient kmsClient = mock(AWSKMSClient.class);
when(kmsClientFactory.getClient(awsRegion)).thenReturn(kmsClient);
when(kmsClient.describeKey(anyObject())).thenReturn(
new DescribeKeyResult()
.withKeyMetadata(
new KeyMetadata()
.withKeyState(KeyState.PendingDeletion)));

boolean result = kmsService.kmsKeyIsDisabledOrScheduledForDeletion(keyId, awsRegion);

assertTrue(result);
}

@Test
public void test_validateKmsKeyIsUsable_return_true_when_state_is_disabled() {
String keyId = "key id";
String awsRegion = "aws region";

AWSKMSClient kmsClient = mock(AWSKMSClient.class);
when(kmsClientFactory.getClient(awsRegion)).thenReturn(kmsClient);
when(kmsClient.describeKey(anyObject())).thenReturn(
new DescribeKeyResult()
.withKeyMetadata(
new KeyMetadata()
.withKeyState(KeyState.Disabled)));

boolean result = kmsService.kmsKeyIsDisabledOrScheduledForDeletion(keyId, awsRegion);

assertTrue(result);
}

@Test
public void test_validateKmsKeyIsUsable_returns_false_when_state_is_not_deletion_or_disabled() {
String keyId = "key id";
String awsRegion = "aws region";

AWSKMSClient kmsClient = mock(AWSKMSClient.class);
when(kmsClientFactory.getClient(awsRegion)).thenReturn(kmsClient);
when(kmsClient.describeKey(anyObject())).thenReturn(
new DescribeKeyResult()
.withKeyMetadata(
new KeyMetadata()
.withKeyState(KeyState.Enabled)));

boolean result = kmsService.kmsKeyIsDisabledOrScheduledForDeletion(keyId, awsRegion);

assertFalse(result);
}

@Test
public void test_validateKmsKeyIsUsable_deletes_kms_key_when_not_usable() {

String id = "id";
String awsKmsKeyArn = "aws kms key arn";
String iamPrincipalArn = "arn";
String awsRegion = "aws region";

AwsIamRoleKmsKeyRecord kmsKey = mock(AwsIamRoleKmsKeyRecord.class);
when(kmsKey.getId()).thenReturn(id);
when(kmsKey.getAwsKmsKeyId()).thenReturn(awsKmsKeyArn);
when(kmsKey.getAwsRegion()).thenReturn(awsRegion);

AWSKMSClient kmsClient = mock(AWSKMSClient.class);
when(kmsClientFactory.getClient(awsRegion)).thenReturn(kmsClient);
when(kmsClient.describeKey(anyObject())).thenReturn(
new DescribeKeyResult()
.withKeyMetadata(
new KeyMetadata()
.withKeyState(KeyState.PendingDeletion)));

kmsService.validateKmsKeyIsUsable(kmsKey, iamPrincipalArn);

verify(awsIamRoleDao, times(1)).deleteKmsKeyById(id);
}

@Test
public void test_validateKmsKeyIsUsable_does_not_delete_kms_key_when_usable() {

String id = "id";
String awsKmsKeyArn = "aws kms key arn";
String iamPrincipalArn = "arn";
String awsRegion = "aws region";

AwsIamRoleKmsKeyRecord kmsKey = mock(AwsIamRoleKmsKeyRecord.class);
when(kmsKey.getId()).thenReturn(id);
when(kmsKey.getAwsKmsKeyId()).thenReturn(awsKmsKeyArn);
when(kmsKey.getAwsRegion()).thenReturn(awsRegion);

AWSKMSClient kmsClient = mock(AWSKMSClient.class);
when(kmsClientFactory.getClient(awsRegion)).thenReturn(kmsClient);
when(kmsClient.describeKey(anyObject())).thenReturn(
new DescribeKeyResult()
.withKeyMetadata(
new KeyMetadata()
.withKeyState(KeyState.Enabled)));

kmsService.validateKmsKeyIsUsable(kmsKey, iamPrincipalArn);

verify(awsIamRoleDao, never()).deleteKmsKeyById(id);
}
}

0 comments on commit 25d0168

Please sign in to comment.