diff --git a/API.md b/API.md index f0996f132..e2671f05f 100644 --- a/API.md +++ b/API.md @@ -1514,7 +1514,7 @@ This endpoint does not return any secret data but can be used by Cerberus admins ### Trigger Scheduled Job [POST] -Manually trigger a job, e.g. ExpiredTokenCleanUpJob, HystrixMetricsProcessingJob, KmsKeyCleanUpJob, KpiMetricsProcessingJob. +Manually trigger a job, e.g. ExpiredTokenCleanUpJob, HystrixMetricsProcessingJob, InactiveKmsKeyCleanUpJob, KpiMetricsProcessingJob. A 400 response code is given if the job wasn't found. + Request diff --git a/gradle/check.gradle b/gradle/check.gradle index 4eb70172d..a5268e3e2 100644 --- a/gradle/check.gradle +++ b/gradle/check.gradle @@ -38,12 +38,14 @@ jacoco { toolVersion = "0.7.7.201606060606" reportsDir = file("$buildDir/reports/jacoco") } + jacocoTestReport { reports { xml.enabled true csv.enabled false } } + test.finalizedBy(project.tasks.jacocoTestReport) task findbugsHtml { @@ -57,4 +59,4 @@ task findbugsHtml { } findbugsMain.finalizedBy findbugsHtml -tasks.coveralls.dependsOn check \ No newline at end of file +tasks.coveralls.dependsOn check diff --git a/src/main/java/com/nike/cerberus/domain/AuthKmsKeyMetadata.java b/src/main/java/com/nike/cerberus/domain/AuthKmsKeyMetadata.java index 0b18e387e..eedd22f34 100644 --- a/src/main/java/com/nike/cerberus/domain/AuthKmsKeyMetadata.java +++ b/src/main/java/com/nike/cerberus/domain/AuthKmsKeyMetadata.java @@ -1,6 +1,7 @@ package com.nike.cerberus.domain; import java.time.OffsetDateTime; +import java.util.Objects; public class AuthKmsKeyMetadata { @@ -64,4 +65,23 @@ public AuthKmsKeyMetadata setLastValidatedTs(OffsetDateTime lastValidatedTs) { this.lastValidatedTs = lastValidatedTs; return this; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + AuthKmsKeyMetadata that = (AuthKmsKeyMetadata) o; + return Objects.equals(awsIamRoleArn, that.awsIamRoleArn) && + Objects.equals(awsKmsKeyId, that.awsKmsKeyId) && + Objects.equals(awsRegion, that.awsRegion) && + Objects.equals(createdTs, that.createdTs) && + Objects.equals(lastUpdatedTs, that.lastUpdatedTs) && + Objects.equals(lastValidatedTs, that.lastValidatedTs); + } + + @Override + public int hashCode() { + + return Objects.hash(awsIamRoleArn, awsKmsKeyId, awsRegion, createdTs, lastUpdatedTs, lastValidatedTs); + } } diff --git a/src/main/java/com/nike/cerberus/hystrix/HystrixKmsClient.java b/src/main/java/com/nike/cerberus/hystrix/HystrixKmsClient.java index 244cec7d1..7a2218149 100644 --- a/src/main/java/com/nike/cerberus/hystrix/HystrixKmsClient.java +++ b/src/main/java/com/nike/cerberus/hystrix/HystrixKmsClient.java @@ -12,6 +12,8 @@ import com.amazonaws.services.kms.model.EncryptResult; import com.amazonaws.services.kms.model.GetKeyPolicyRequest; import com.amazonaws.services.kms.model.GetKeyPolicyResult; +import com.amazonaws.services.kms.model.ListKeysRequest; +import com.amazonaws.services.kms.model.ListKeysResult; import com.amazonaws.services.kms.model.PutKeyPolicyRequest; import com.amazonaws.services.kms.model.PutKeyPolicyResult; import com.amazonaws.services.kms.model.ScheduleKeyDeletionRequest; @@ -81,6 +83,11 @@ public PutKeyPolicyResult putKeyPolicy(PutKeyPolicyRequest request) { return execute("KmsPutKeyPolicy", () -> client.putKeyPolicy(request)); } + public ListKeysResult listKeys(ListKeysRequest request) { + // Default AWS limit was X as of July 2018 + return execute("ListKeysRequest", () -> client.listKeys(request)); + } + /** * Execute a function that returns a value in a ThreadPool unique to that command. */ diff --git a/src/main/java/com/nike/cerberus/jobs/KmsKeyCleanUpJob.java b/src/main/java/com/nike/cerberus/jobs/InactiveKmsKeyCleanUpJob.java similarity index 80% rename from src/main/java/com/nike/cerberus/jobs/KmsKeyCleanUpJob.java rename to src/main/java/com/nike/cerberus/jobs/InactiveKmsKeyCleanUpJob.java index 8f660df00..bf84a9bc0 100644 --- a/src/main/java/com/nike/cerberus/jobs/KmsKeyCleanUpJob.java +++ b/src/main/java/com/nike/cerberus/jobs/InactiveKmsKeyCleanUpJob.java @@ -24,7 +24,10 @@ import javax.inject.Inject; import javax.inject.Named; -public class KmsKeyCleanUpJob extends LockingJob { +/** + * Scans through the data store and deletes in-active KMS CMKs + */ +public class InactiveKmsKeyCleanUpJob extends LockingJob { private final Logger logger = LoggerFactory.getLogger(getClass()); @@ -35,10 +38,10 @@ public class KmsKeyCleanUpJob extends LockingJob { private final int pauseTimeInSeconds; @Inject - public KmsKeyCleanUpJob(CleanUpService cleanUpService, - @Named("cms.jobs.KmsCleanUpJob.deleteKmsKeysOlderThanNDays") + public InactiveKmsKeyCleanUpJob(CleanUpService cleanUpService, + @Named("cms.jobs.KmsCleanUpJob.deleteKmsKeysOlderThanNDays") int expirationPeriodInDays, - @Named("cms.jobs.KmsCleanUpJob.batchPauseTimeInSeconds") + @Named("cms.jobs.KmsCleanUpJob.batchPauseTimeInSeconds") int pauseTimeInSeconds) { this.cleanUpService = cleanUpService; diff --git a/src/main/java/com/nike/cerberus/jobs/OrphanedKmsKeyCleanUpJob.java b/src/main/java/com/nike/cerberus/jobs/OrphanedKmsKeyCleanUpJob.java new file mode 100644 index 000000000..62ada5f89 --- /dev/null +++ b/src/main/java/com/nike/cerberus/jobs/OrphanedKmsKeyCleanUpJob.java @@ -0,0 +1,146 @@ +package com.nike.cerberus.jobs; + +import com.amazonaws.regions.Regions; +import com.google.common.collect.Sets; +import com.nike.cerberus.domain.AuthKmsKeyMetadata; +import com.nike.cerberus.service.KmsService; +import org.apache.commons.lang.StringUtils; + +import javax.inject.Inject; +import javax.inject.Named; +import java.util.*; +import java.util.stream.Collectors; + +import static com.nike.cerberus.service.KmsService.SOONEST_A_KMS_KEY_CAN_BE_DELETED; + +/** + * This scans through all KMS keys it can access in all regions and parses the Key Policies to see if it was a key + * that was created by CMS for the current running env. If it is a key that was created by itself for this env, + * it cross references it with the data store to check if it is orphaned. + * If it is orphaned (Created by CMS for the current env, but not in the database), it schedules it for deletion. + * + * Orphaned keys can be created due to a race condition from lazily creating KMS CMKs for auth. + */ +public class OrphanedKmsKeyCleanUpJob extends LockingJob { + + private final KmsService kmsService; + private final boolean isDeleteOrphanKeysInDryMode; + private final String environmentName; + + @Inject + public OrphanedKmsKeyCleanUpJob(KmsService kmsService, + @Named("cms.kms.delete_orphaned_keys_job.dry_mode") + boolean isDeleteOrphanKeysInDryMode, + @Named("cms.env.name") String environmentName) { + + this.kmsService = kmsService; + this.isDeleteOrphanKeysInDryMode = isDeleteOrphanKeysInDryMode; + this.environmentName = environmentName; + } + + @Override + protected void executeLockableCode() { + + log.info("Fetching the the keys that are in the database"); + List authKmsKeyMetadataList = kmsService.getAuthenticationKmsMetadata(); + + Map> orphanedKeysByRegion = new HashMap<>(); + + // For each region that has KMS + Arrays.stream(Regions.values()).forEach(region -> { + String regionName = region.getName(); + + // skip china + if (regionName.startsWith("cn")) { + log.debug("KMS isn't in china, skipping..."); + return; + } + // skip us gov + if (regionName.startsWith("us-gov")) { + log.debug("Cerberus isn't in us-gov, as z requires special credentials, skipping..."); + return; + } + + orphanedKeysByRegion.put(regionName, processRegion(authKmsKeyMetadataList, regionName)); + }); + + logCompleteSummary(orphanedKeysByRegion); + } + + /** + * Downloads all the KMS CMK policies for the keys in the given region to determine what keys it created and compares + * that set to the set of keys it has in the datastore to find and delete orphaned keys + * + * @param authKmsKeyMetadataList The kms metadata from the data store + * @param regionName The region to process and delete orphaned keys in + * @return The set or orphaned keys it found and processed + */ + protected Set processRegion(List authKmsKeyMetadataList, String regionName) { + log.info("Processing region: {}", regionName); + // Get the KMS Key Ids that are in the db for the current region + Set currentKmsCmkIdsForRegion = authKmsKeyMetadataList.stream() + .filter(authKmsKeyMetadata -> StringUtils.equalsIgnoreCase(authKmsKeyMetadata.getAwsRegion(), regionName)) + .map(authKmsKeyMetadata -> { + String fullArn = authKmsKeyMetadata.getAwsKmsKeyId(); + return fullArn.split("/")[1]; + }) + .collect(Collectors.toSet()); + + log.info("Fetching all KMS CMK ids keys for the region: {}", regionName); + Set allKmsCmkIdsForRegion = kmsService.getKmsKeyIdsForRegion(regionName); + log.info("Found {} keys to process for region: {}", allKmsCmkIdsForRegion.size(), regionName); + + log.info("Filtering out the keys that were not created by this environment"); + Set kmsCmksCreatedByKmsService = kmsService.filterKeysCreatedByKmsService(allKmsCmkIdsForRegion, regionName); + log.info("Found {} keys to created by this environment process for region: {}", kmsCmksCreatedByKmsService.size(), regionName); + + log.info("Calculating difference between the set of keys created by this env to the set of keys in the db"); + Set orphanedKmsKeysForRegion = Sets.difference(kmsCmksCreatedByKmsService, currentKmsCmkIdsForRegion); + log.info("Found {} keys that were orphaned for region: {}", orphanedKmsKeysForRegion.size(), regionName); + + logRegionSummary(regionName, kmsCmksCreatedByKmsService, currentKmsCmkIdsForRegion, orphanedKmsKeysForRegion); + + // Delete the orphaned keys + if (! isDeleteOrphanKeysInDryMode) { + orphanedKmsKeysForRegion.forEach(kmsCmkId -> kmsService + .scheduleKmsKeyDeletion(kmsCmkId, regionName, SOONEST_A_KMS_KEY_CAN_BE_DELETED)); + } + + return orphanedKmsKeysForRegion; + } + + /** + * Logs the summary of actions taken for a given region + */ + private void logRegionSummary(String regionName, + Set kmsCmksCreatedByKmsService, + Set currentKmsCmkIdsForRegion, + Set orphanedKmsKeysForRegion) { + + log.info("---------- Orphan KMS Key cleanup job summary for region: {} ------------", regionName); + log.debug("The following keys where determined to be created by CMS service for env: {}", environmentName); + kmsCmksCreatedByKmsService.forEach(log::debug); + log.debug("The following keys were in the data-store for this region"); + currentKmsCmkIdsForRegion.forEach(log::debug); + log.info("The following keys were determined to be orphaned and have been scheduled for deletion? {}", !isDeleteOrphanKeysInDryMode); + orphanedKmsKeysForRegion.forEach(log::info); + log.info("--------------------------------------------------------------------------------"); + + } + + /** + * Logs a summary for all regions + */ + private void logCompleteSummary(Map> orphanedKeysByRegion) { + log.info("----------- Orphan Kms Key Cleanup Job summary ------------"); + orphanedKeysByRegion.forEach((regionName, keys) -> { + log.info("Region: {}, number of orphaned keys: {}", regionName, keys.size()); + }); + if (isDeleteOrphanKeysInDryMode) { + log.info("The job was in dry mode so the keys were not deleted"); + } else { + log.info("The job was not in dry mode so the keys have been scheduled for deletion"); + } + log.info("--------------------------------------------------------------------------------"); + } +} diff --git a/src/main/java/com/nike/cerberus/service/KmsPolicyService.java b/src/main/java/com/nike/cerberus/service/KmsPolicyService.java index 76d07d0c5..6950d31c3 100644 --- a/src/main/java/com/nike/cerberus/service/KmsPolicyService.java +++ b/src/main/java/com/nike/cerberus/service/KmsPolicyService.java @@ -105,7 +105,7 @@ public boolean isPolicyValid(String policyJson) { */ protected boolean consumerPrincipalIsAnArnAndNotAnId(String policyJson) { try { - Policy policy = policyReader.createPolicyFromJsonString(policyJson); + Policy policy = getPolicyFromPolicyString(policyJson); return policy.getStatements() .stream() .anyMatch(statement -> @@ -127,7 +127,7 @@ protected boolean consumerPrincipalIsAnArnAndNotAnId(String policyJson) { */ protected boolean cmsHasKeyDeletePermissions(String policyJson) { try { - Policy policy = policyReader.createPolicyFromJsonString(policyJson); + Policy policy = getPolicyFromPolicyString(policyJson); return policy.getStatements() .stream() .anyMatch(statement -> @@ -151,7 +151,7 @@ protected boolean cmsHasKeyDeletePermissions(String policyJson) { * @return - The updated JSON KMS policy containing a regenerated statement for CMS */ protected String overwriteCMSPolicy(String policyJson) { - Policy policy = policyReader.createPolicyFromJsonString(policyJson); + Policy policy = getPolicyFromPolicyString(policyJson); removeStatementFromPolicy(policy, CERBERUS_MANAGEMENT_SERVICE_SID); Collection statements = policy.getStatements(); statements.add(generateStandardCMSPolicyStatement()); @@ -169,7 +169,7 @@ protected String overwriteCMSPolicy(String policyJson) { * @return - The updated key policy JSON */ protected String removeConsumerPrincipalFromPolicy(String policyJson) { - Policy policy = policyReader.createPolicyFromJsonString(policyJson); + Policy policy = getPolicyFromPolicyString(policyJson); removeStatementFromPolicy(policy, CERBERUS_CONSUMER_SID); return policy.toJson(); } @@ -259,4 +259,9 @@ public String generateStandardKmsPolicy(String iamRoleArn) { return kmsPolicy.toJson(); } + + + public Policy getPolicyFromPolicyString(String jsonString) { + return policyReader.createPolicyFromJsonString(jsonString); + } } diff --git a/src/main/java/com/nike/cerberus/service/KmsService.java b/src/main/java/com/nike/cerberus/service/KmsService.java index 6da8d08e6..cece6186c 100644 --- a/src/main/java/com/nike/cerberus/service/KmsService.java +++ b/src/main/java/com/nike/cerberus/service/KmsService.java @@ -17,17 +17,12 @@ package com.nike.cerberus.service; import com.amazonaws.AmazonServiceException; +import com.amazonaws.auth.policy.Policy; +import com.amazonaws.auth.policy.Statement; +import com.amazonaws.services.kms.AWSKMS; import com.amazonaws.services.kms.AWSKMSClient; -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.KeyUsageType; -import com.amazonaws.services.kms.model.NotFoundException; -import com.amazonaws.services.kms.model.PutKeyPolicyRequest; -import com.amazonaws.services.kms.model.ScheduleKeyDeletionRequest; -import com.amazonaws.services.kms.model.Tag; +import com.amazonaws.services.kms.model.*; +import com.google.common.collect.ImmutableSet; import com.google.inject.name.Named; import com.netflix.hystrix.exception.HystrixRuntimeException; import com.nike.backstopper.exception.ApiException; @@ -49,13 +44,12 @@ import javax.inject.Singleton; import java.time.OffsetDateTime; import java.time.temporal.ChronoUnit; -import java.util.LinkedList; -import java.util.List; -import java.util.Optional; +import java.util.*; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import static com.nike.cerberus.service.AuthenticationService.SYSTEM_USER; +import static com.nike.cerberus.service.KmsPolicyService.CERBERUS_MANAGEMENT_SERVICE_SID; /** * Abstracts interactions with the AWS KMS service. @@ -261,7 +255,7 @@ public List getAuthenticationKmsMetadata() { .setAwsRegion(key.getAwsRegion()) .setCreatedTs(key.getCreatedTs()) .setLastUpdatedTs(key.getLastUpdatedTs()) - .setLastUpdatedTs(key.getLastUpdatedTs()); + .setLastValidatedTs(key.getLastValidatedTs()); awsIamRoleDao.getIamRoleById(key.getAwsIamRoleId()) .ifPresent(awsIamRoleRecord -> metadata.setAwsIamRoleArn(awsIamRoleRecord.getAwsIamRoleArn())); @@ -428,14 +422,24 @@ protected String getKmsKeyState(String kmsKeyId, String region) { * @param kmsKeyId - The AWS KMS Key ID * @param region - The KMS key region */ - protected void scheduleKmsKeyDeletion(String kmsKeyId, String region, Integer pendingWindowInDays) { + public void scheduleKmsKeyDeletion(String kmsKeyId, String region, Integer pendingWindowInDays) { + + logger.info("Scheduling kms cmk id: {} in region: {} for deletion in {} days", kmsKeyId, region, pendingWindowInDays); final AWSKMSClient kmsClient = kmsClientFactory.getClient(region); final ScheduleKeyDeletionRequest scheduleKeyDeletionRequest = new ScheduleKeyDeletionRequest() .withKeyId(kmsKeyId) .withPendingWindowInDays(pendingWindowInDays); - kmsClient.scheduleKeyDeletion(scheduleKeyDeletionRequest); + try { + kmsClient.scheduleKeyDeletion(scheduleKeyDeletionRequest); + } catch (KMSInvalidStateException e) { + if (e.getErrorMessage().contains("pending deletion")) { + logger.warn("The key: {} in region: {} is already pending deletion", kmsKeyId, region); + } else { + throw e; + } + } } /** @@ -463,4 +467,119 @@ protected boolean kmsPolicyNeedsValidation(AwsIamRoleKmsKeyRecord kmsKeyRecord, return needValidation; } + + /** + * Attempts to download the policy, if something goes wrong, it returns an empty optional. + * @param kmsCmkId The KMS CMK that you want the default policy for + * @param regionName The region that the KMS CMK resides in. + * @return The policy if it can successfully be fetched. + */ + protected Optional downloadPolicy(String kmsCmkId, String regionName, int retryCount) { + final Set unretryableErrors = ImmutableSet.of( + "NotFoundException", + "InvalidArnException", + "AccessDeniedException" + ); + Policy policy = null; + try { + policy = kmsPolicyService.getPolicyFromPolicyString(getKmsKeyPolicy(kmsCmkId, regionName)); + return Optional.of(policy); + } catch (AWSKMSException | HystrixRuntimeException e) { + String errorCode = e instanceof AWSKMSException ? ((AWSKMSException) e).getErrorCode() : e.getMessage(); + logger.error("Failed to download policy, error code: {}", errorCode); + if (! unretryableErrors.contains(errorCode) && retryCount < 10) { + try { + Thread.sleep(500 ^ (retryCount + 1)); + } catch (InterruptedException e1) { + return Optional.empty(); + } + return downloadPolicy(kmsCmkId, regionName, retryCount + 1); + } + } + return Optional.ofNullable(policy); + } + + /** + * Gets all the KMS CMK ids for a given region + * @param regionName The region in which you want all the KMS CMK ids + * @return A list of of the KMS CMK ids for the requested region. + */ + public Set getKmsKeyIdsForRegion(String regionName) { + AWSKMS kms = kmsClientFactory.getClient(regionName); + + Set kmsKeyIdsForRegion = new HashSet<>(); + + String marker = null; + do { + logger.debug("Fetching keys for region: {} and marker: {}", regionName, marker); + ListKeysRequest listKeysRequest = new ListKeysRequest(); + if (marker != null) { + listKeysRequest.withMarker(marker); + } + ListKeysResult listKeysResult = kms.listKeys(listKeysRequest); + listKeysResult.getKeys().forEach(keyListEntry -> kmsKeyIdsForRegion.add(keyListEntry.getKeyId())); + marker = listKeysResult.getNextMarker(); + } while (marker != null); + + return kmsKeyIdsForRegion; + } + + /** + * Downloads the KMS CMK policies to determine if the key was created by this. + * + * @param allKmsCmkIdsForRegion The list of all the KMS Key. + * @param regionName The region. + * @return The list of KMS Keys that were created by this. + */ + public Set filterKeysCreatedByKmsService(Set allKmsCmkIdsForRegion, String regionName) { + Set keysCreatedByThis = new HashSet<>(); + int numberOfKeys = allKmsCmkIdsForRegion.size(); + String[] kmsCmkIds = allKmsCmkIdsForRegion.toArray(new String[numberOfKeys]); + + logger.info("Preparing to download and analyze {} key policies in region: {} to " + + "determine what keys where created by this env", numberOfKeys, regionName); + + for (int i = 0; i < numberOfKeys; i++) { + logger.debug("Processed {}% of keys for region: {}", Math.ceil((double) i / numberOfKeys * 100), regionName); + String kmsCmkId = kmsCmkIds[i]; + if (wasKeyCreatedByKmsService(kmsCmkId,regionName)) { + keysCreatedByThis.add(kmsCmkId); + } + } + + return keysCreatedByThis; + } + + /** + * Checks the KMS CMK policy to see if it was created by this environments cms cluster. + * @param kmsCmkId The KMS CMK id. + * @param regionName The region. + * @return true if the kms key was created by CMS for the current env + */ + private boolean wasKeyCreatedByKmsService(String kmsCmkId, String regionName) { + boolean wasCreatedByThis = false; + + logger.debug("Downloading policy for key: {} in region: {}", kmsCmkId, regionName); + + Optional policyOptional = downloadPolicy(kmsCmkId, regionName, 0); + + if (policyOptional.isPresent()) { + Policy policy = policyOptional.get(); + if (policy.getStatements().size() == 4) { + Optional cmsStatement = policy.getStatements().stream() + .filter(statement -> statement.getId().equals(CERBERUS_MANAGEMENT_SERVICE_SID)) + .findFirst(); + + if (cmsStatement.isPresent()) { + wasCreatedByThis = cmsStatement.get().getPrincipals().stream() + .anyMatch(principal -> principal.getId().contains(environmentName)); + logger.debug("detected cms policy, was created by this env: {}", wasCreatedByThis); + } + } + } else { + logger.warn("Failed to fetch policy for key: {} in region: {}, skipping...", kmsCmkId, regionName); + } + + return wasCreatedByThis; + } } diff --git a/src/main/java/com/nike/cerberus/service/MetricsService.java b/src/main/java/com/nike/cerberus/service/MetricsService.java index e0a752c1b..d980bc963 100644 --- a/src/main/java/com/nike/cerberus/service/MetricsService.java +++ b/src/main/java/com/nike/cerberus/service/MetricsService.java @@ -25,7 +25,6 @@ import com.signalfx.codahale.metrics.SettableDoubleGauge; import com.signalfx.codahale.metrics.SettableLongGauge; import com.signalfx.codahale.reporter.MetricMetadata; -import com.signalfx.codahale.reporter.MetricMetadataImpl; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,7 +48,7 @@ public MetricsService(CodahaleMetricsCollector metricsCollector, if (signalFxReporterFactory != null) { this.metricMetadata = signalFxReporterFactory.getReporter(metricsCollector.getMetricRegistry()).getMetricMetadata(); } else { - metricMetadata = new MetricMetadataImpl(); + metricMetadata = null; } } @@ -99,6 +98,16 @@ public Counter getOrCreateCounter(String name, Map dimensions) { } private M getOrCreate(MetricBuilder builder, String metricName, Map dimensions) { + + if (metricMetadata == null) { + if (metricsCollector.getMetricRegistry().getMetrics().containsKey(metricName)) { + return (M) metricsCollector.getMetricRegistry().getMetrics().get(metricName); + } else { + M metric = builder.newMetric(); + return metricsCollector.getMetricRegistry().register(metricName, metric); + } + } + MetricMetadata.BuilderTagger builderTagger = metricMetadata .forBuilder(builder) .withMetricName(metricName); diff --git a/src/main/resources/cms.conf b/src/main/resources/cms.conf index b4f3f2af9..4ce04b9b2 100644 --- a/src/main/resources/cms.conf +++ b/src/main/resources/cms.conf @@ -97,17 +97,24 @@ hystrix.threadpool.KmsScheduleKeyDeletion.coreSize=5 # Default AWS limit was 30 as of Aug 2017 hystrix.threadpool.KmsGetKeyPolicy.coreSize=10 +hystrix.command.KmsGetKeyPolicy.execution.isolation.thread.timeoutInMilliseconds=3000 # Default AWS limit was 5 as of Aug 2017 hystrix.threadpool.KmsPutKeyPolicy.coreSize=5 +# Default AWS limit was 30 as of Aug 2017 +hystrix.threadpool.ListKeysRequest.coreSize=5 +hystrix.command.ListKeysRequest.execution.isolation.thread.timeoutInMilliseconds=10000 # Application name cms.app.name=cms +# Kms Cleanup Job Config +cms.kms.delete_orphaned_keys_job.dry_mode=true + # Scheduled job configuration cms.jobs.enabled=true -cms.jobs.initialDelay=1 +cms.jobs.initialDelay=0 cms.jobs.initialDelayTimeUnits=minutes cms.jobs.configuredJobs=[ { @@ -123,11 +130,17 @@ cms.jobs.configuredJobs=[ "repeatTimeUnit": "seconds" }, { - "jobClassName": "KmsKeyCleanUpJob", + "jobClassName": "InactiveKmsKeyCleanUpJob", "repeatCount": -1, # repeat indefinitely "repeatInterval": 1, "repeatTimeUnit": "days" }, + { + "jobClassName": "OrphanedKmsKeyCleanUpJob", + "repeatCount": -1, + "repeatInterval": 15, + "repeatTimeUnit": "days" + } { "jobClassName": "KpiMetricsProcessingJob", "repeatCount": -1, # repeat indefinitely @@ -170,4 +183,4 @@ cms.iam.token.ttl=1h # For example: # When true, if an SDB grants access to AD group 'Lst-foo', then users in group 'Lst-Foo' will not have access # When false, if an SDB grants access to AD group 'Lst-foo', then users in group 'Lst-Foo' will have access -cms.user.groups.caseSensitive=true \ No newline at end of file +cms.user.groups.caseSensitive=true diff --git a/src/test/java/com/nike/cerberus/jobs/OrphanedKmsKeyCleanUpJobTest.java b/src/test/java/com/nike/cerberus/jobs/OrphanedKmsKeyCleanUpJobTest.java new file mode 100644 index 000000000..387914538 --- /dev/null +++ b/src/test/java/com/nike/cerberus/jobs/OrphanedKmsKeyCleanUpJobTest.java @@ -0,0 +1,80 @@ +package com.nike.cerberus.jobs; + +import com.amazonaws.regions.Regions; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.nike.cerberus.domain.AuthKmsKeyMetadata; +import com.nike.cerberus.service.KmsService; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; + +import java.util.List; +import java.util.Set; + +import static com.nike.cerberus.service.KmsService.SOONEST_A_KMS_KEY_CAN_BE_DELETED; +import static junit.framework.TestCase.assertEquals; +import static org.mockito.Matchers.anyList; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.*; +import static org.mockito.MockitoAnnotations.initMocks; + +public class OrphanedKmsKeyCleanUpJobTest { + + @Mock + private KmsService kmsService; + + private final String ENVIRONMENT = "unit-test"; + + private OrphanedKmsKeyCleanUpJob job; + + @Before + public void before() { + initMocks(this); + job = new OrphanedKmsKeyCleanUpJob(kmsService, false, ENVIRONMENT); + } + + @Test + public void test_that_executeLockableCode_iterates_over_available_regions_and_calls_process_region_for_each() { + OrphanedKmsKeyCleanUpJob spyJob = spy(job); + doReturn(ImmutableSet.of("kms-cmk-id-2")).when(spyJob).processRegion(anyList(), anyString()); + spyJob.executeLockableCode(); + verify(spyJob, times(Regions.values().length - 2)).processRegion(anyList(), anyString()); + } + + @Test + public void test_that_processRegion_obeys_dry_mode_flag() { + String region = "us-west-2"; + List authKmsKeyMetadataList = ImmutableList.of( + new AuthKmsKeyMetadata() + .setAwsRegion(region) + .setAwsKmsKeyId("arn:aws:kms:us-west-2:111122223333:key/kms-cmk-id-1") + ); + Set kmsKeyIdsForRegion = ImmutableSet.of("kms-cmk-id-1", "kms-cmk-id-2", "kms-cmk-id-3", "kms-cmk-id-4"); + Set keysCreatedByKmsService = ImmutableSet.of("kms-cmk-id-1", "kms-cmk-id-2"); + when(kmsService.getKmsKeyIdsForRegion(region)).thenReturn(kmsKeyIdsForRegion); + when(kmsService.filterKeysCreatedByKmsService(kmsKeyIdsForRegion, region)).thenReturn(keysCreatedByKmsService); + + Set keys = job.processRegion(authKmsKeyMetadataList, region); + + verify(kmsService, times(1)) + .scheduleKmsKeyDeletion("kms-cmk-id-2", region, SOONEST_A_KMS_KEY_CAN_BE_DELETED); + + assertEquals(1, keys.size()); + + reset(kmsService); + + when(kmsService.getKmsKeyIdsForRegion(region)).thenReturn(kmsKeyIdsForRegion); + when(kmsService.filterKeysCreatedByKmsService(kmsKeyIdsForRegion, region)).thenReturn(keysCreatedByKmsService); + + job = new OrphanedKmsKeyCleanUpJob(kmsService, true, ENVIRONMENT); + + Set keys2 = job.processRegion(authKmsKeyMetadataList, region); + + verify(kmsService, never()) + .scheduleKmsKeyDeletion(anyString(), anyString(), anyInt()); + + assertEquals(1, keys2.size()); + } + +} diff --git a/src/test/java/com/nike/cerberus/service/KmsServiceTest.java b/src/test/java/com/nike/cerberus/service/KmsServiceTest.java index b33de622b..039cc0cc6 100644 --- a/src/test/java/com/nike/cerberus/service/KmsServiceTest.java +++ b/src/test/java/com/nike/cerberus/service/KmsServiceTest.java @@ -1,6 +1,9 @@ package com.nike.cerberus.service; import com.amazonaws.AmazonServiceException; +import com.amazonaws.auth.policy.Policy; +import com.amazonaws.auth.policy.Principal; +import com.amazonaws.auth.policy.Statement; import com.amazonaws.services.kms.AWSKMSClient; import com.amazonaws.services.kms.model.CreateAliasRequest; import com.amazonaws.services.kms.model.CreateKeyRequest; @@ -12,11 +15,15 @@ import com.amazonaws.services.kms.model.KeyState; import com.amazonaws.services.kms.model.KeyUsageType; import com.amazonaws.services.kms.model.Tag; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.nike.backstopper.exception.ApiException; import com.nike.cerberus.aws.KmsClientFactory; import com.nike.cerberus.dao.AwsIamRoleDao; +import com.nike.cerberus.domain.AuthKmsKeyMetadata; import com.nike.cerberus.record.AwsIamRoleKmsKeyRecord; +import com.nike.cerberus.record.AwsIamRoleRecord; import com.nike.cerberus.util.AwsIamRoleArnParser; import com.nike.cerberus.util.DateTimeSupplier; import com.nike.cerberus.util.Slugger; @@ -26,17 +33,17 @@ import java.time.OffsetDateTime; import java.time.ZoneOffset; +import java.time.temporal.ChronoUnit; +import java.util.LinkedList; +import java.util.List; import java.util.Optional; +import java.util.Set; +import static com.nike.cerberus.service.KmsPolicyService.CERBERUS_MANAGEMENT_SERVICE_SID; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; 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; +import static org.mockito.Mockito.*; public class KmsServiceTest { @@ -293,4 +300,109 @@ public void test_getKmsKeyState_happy() { assertEquals(state, result); } + @Test + public void test_deleteKmsKeyById_proxies_the_dao() { + String key = "the-key-id"; + + kmsService.deleteKmsKeyById(key); + + verify(awsIamRoleDao, times(1)).deleteKmsKeyById(key); + } + + @Test + public void test_that_getAuthenticationKmsMetadata_returns_empty_list_when_there_are_no_keys() { + when(awsIamRoleDao.getAllKmsKeys()).thenReturn(Optional.empty()); + + assertEquals(new LinkedList(), kmsService.getAuthenticationKmsMetadata()); + } + + @Test + public void test_that_getAuthenticationKmsMetadata_returns_AuthKmsKeyMetadata_from_dao_data() { + OffsetDateTime create = OffsetDateTime.now().plus(5, ChronoUnit.MINUTES); + OffsetDateTime update = OffsetDateTime.now().plus(3, ChronoUnit.MINUTES); + OffsetDateTime validate = OffsetDateTime.now().plus(7, ChronoUnit.MINUTES); + List keyRecords = ImmutableList.of( + new AwsIamRoleKmsKeyRecord() + .setAwsIamRoleId("iam-role-id") + .setAwsKmsKeyId("key-id") + .setAwsRegion("us-west-2") + .setCreatedTs(create) + .setLastUpdatedTs(update) + .setLastValidatedTs(validate) + ); + + List expected = ImmutableList.of( + new AuthKmsKeyMetadata() + .setAwsIamRoleArn("iam-role-arn") + .setAwsKmsKeyId("key-id") + .setAwsRegion("us-west-2") + .setCreatedTs(create) + .setLastUpdatedTs(update) + .setLastValidatedTs(validate) + ); + + when(awsIamRoleDao.getAllKmsKeys()).thenReturn(Optional.ofNullable(keyRecords)); + when(awsIamRoleDao.getIamRoleById("iam-role-id")).thenReturn(Optional.of( + new AwsIamRoleRecord().setAwsIamRoleArn("iam-role-arn") + )); + + assertArrayEquals(expected.toArray(), kmsService.getAuthenticationKmsMetadata().toArray()); + } + + @Test + public void test_that_filterKeysCreatedByKmsService_filters_out_keys_that_do_not_contain_expected_arn_prefix() { + + Policy policyThatShouldBeInSet = new Policy().withStatements( + new Statement(Statement.Effect.Allow) + .withId(CERBERUS_MANAGEMENT_SERVICE_SID) + .withPrincipals(new Principal("arn:aws:iam:123456:role/" + ENV + "-cms-role-alk234khsdf")), + + new Statement(Statement.Effect.Allow), + new Statement(Statement.Effect.Allow), + new Statement(Statement.Effect.Allow) + ); + + Policy policyThatShouldNotBeInSet = new Policy().withStatements( + new Statement(Statement.Effect.Allow) + .withId(CERBERUS_MANAGEMENT_SERVICE_SID) + .withPrincipals(new Principal("arn:aws:iam:123456:role/prod-cms-role-alk234khsdf")), + + new Statement(Statement.Effect.Allow), + new Statement(Statement.Effect.Allow), + new Statement(Statement.Effect.Allow) + ); + + Policy policyThatWasntCreatedByCms = new Policy().withStatements( + new Statement(Statement.Effect.Allow) + .withId("foo-bar") + .withPrincipals(new Principal("arn:aws:iam:123456:role/" + ENV + "-cms-role-alk234khsdf")) + ); + + KmsService kmsServiceSpy = spy(kmsService); + + Set allKmsCmkIdsForRegion = ImmutableSet.of( + "key1", + "key2", + "key3", + "key4", + "key5" + ); + + String region = "us-west-2"; + + Set expectedKeys = ImmutableSet.of( + "key3" + ); + + doReturn(Optional.of(policyThatShouldNotBeInSet)).when(kmsServiceSpy).downloadPolicy("key1", region, 0); + doReturn(Optional.of(policyThatShouldNotBeInSet)).when(kmsServiceSpy).downloadPolicy("key2", region, 0); + doReturn(Optional.of(policyThatShouldBeInSet)).when(kmsServiceSpy).downloadPolicy("key3", region, 0); + doReturn(Optional.of(policyThatShouldNotBeInSet)).when(kmsServiceSpy).downloadPolicy("key4", region, 0); + doReturn(Optional.of(policyThatWasntCreatedByCms)).when(kmsServiceSpy).downloadPolicy("key5", region, 0); + + Set actual = kmsServiceSpy.filterKeysCreatedByKmsService(allKmsCmkIdsForRegion, region); + + assertEquals(expectedKeys, actual); + } + } \ No newline at end of file