From b20e609438bcf4955da36082c21b263a27f9aa6a Mon Sep 17 00:00:00 2001 From: Todd Lisonbee Date: Tue, 24 Oct 2017 14:13:40 -0700 Subject: [PATCH] Bug fix: role ARNs weren't being constructed properly when paths were being used (#29) * Bug fix: role ARNs weren't being constructed properly when paths were being used --- .../auth/aws/BaseAwsCredentialsProvider.java | 1 + .../InstanceRoleVaultCredentialsProvider.java | 132 ++++++++++++++++-- ...tanceRoleVaultCredentialsProviderTest.java | 89 ++++++++++++ 3 files changed, 209 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/nike/cerberus/client/auth/aws/BaseAwsCredentialsProvider.java b/src/main/java/com/nike/cerberus/client/auth/aws/BaseAwsCredentialsProvider.java index 91f3116..c972533 100644 --- a/src/main/java/com/nike/cerberus/client/auth/aws/BaseAwsCredentialsProvider.java +++ b/src/main/java/com/nike/cerberus/client/auth/aws/BaseAwsCredentialsProvider.java @@ -186,6 +186,7 @@ else if (expireDateTime.isBeforeNow()) { * AWS account ID used to auth with cerberus * @param iamRoleName * IAM role name used to auth with cerberus + * @deprecated no longer used, will be removed */ protected void getAndSetToken(final String accountId, final String iamRoleName) { final String iamRoleArn = String.format(IAM_ROLE_ARN_FORMAT, accountId, iamRoleName); diff --git a/src/main/java/com/nike/cerberus/client/auth/aws/InstanceRoleVaultCredentialsProvider.java b/src/main/java/com/nike/cerberus/client/auth/aws/InstanceRoleVaultCredentialsProvider.java index dc10953..c203d0f 100644 --- a/src/main/java/com/nike/cerberus/client/auth/aws/InstanceRoleVaultCredentialsProvider.java +++ b/src/main/java/com/nike/cerberus/client/auth/aws/InstanceRoleVaultCredentialsProvider.java @@ -17,6 +17,8 @@ package com.nike.cerberus.client.auth.aws; import com.amazonaws.AmazonClientException; +import com.amazonaws.regions.Region; +import com.amazonaws.regions.Regions; import com.amazonaws.util.EC2MetadataUtils; import com.google.gson.JsonSyntaxException; import com.nike.vault.client.UrlResolver; @@ -26,10 +28,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.HashSet; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; +import static com.nike.cerberus.client.auth.aws.StaticIamRoleVaultCredentialsProvider.IAM_ROLE_ARN_FORMAT; + /** * {@link VaultCredentialsProvider} implementation that uses the assigned role @@ -37,6 +42,10 @@ * response using KMS. If the assigned role has been granted the appropriate * provisioned for usage of Vault, it will succeed and have a token that can be * used to interact with Vault. + * + * This class uses the AWS Instance Metadata endpoint to look-up information automatically. + * + * @see AWS Instance Metadata */ public class InstanceRoleVaultCredentialsProvider extends BaseAwsCredentialsProvider { @@ -44,6 +53,8 @@ public class InstanceRoleVaultCredentialsProvider extends BaseAwsCredentialsProv public static final Pattern IAM_ARN_PATTERN = Pattern.compile("(arn\\:aws\\:iam\\:\\:)(?[0-9].*)(\\:.*)"); + private static final Pattern INSTANCE_PROFILE_ARN_PATTERN = Pattern.compile("arn:aws:iam::(.*?):instance-profile/(.*)"); + /** * Constructor to setup credentials provider using the specified * implementation of {@link UrlResolver} @@ -58,7 +69,7 @@ public InstanceRoleVaultCredentialsProvider(UrlResolver urlResolver) { * Constructor to setup credentials provider using the specified * implementation of {@link UrlResolver} * - * @param urlResolver Resolver for resolving the Cerberus URL + * @param urlResolver Resolver for resolving the Cerberus URL * @param xCerberusClientOverride - Overrides the default header value for the 'X-Cerberus-Client' header */ public InstanceRoleVaultCredentialsProvider(UrlResolver urlResolver, String xCerberusClientOverride) { @@ -75,15 +86,16 @@ public InstanceRoleVaultCredentialsProvider(UrlResolver urlResolver, String xCer @Override protected void authenticate() { try { + final String instanceProfileArn = getInstanceProfileArn(); final Set iamRoleSet = EC2MetadataUtils.getIAMSecurityCredentials().keySet(); - final String accountId = lookupAccountId(); + Region region = Regions.getCurrentRegion(); - for (final String iamRole : iamRoleSet) { + for (String iamRole : buildIamRoleArns(instanceProfileArn, iamRoleSet)) { try { - getAndSetToken(accountId, iamRole); + getAndSetToken(iamRole, region); return; } catch (VaultClientException sce) { - LOGGER.warn("Unable to acquire Vault token for IAM role: " + iamRole, sce); + LOGGER.warn("Unable to acquire Vault token for IAM role: " + iamRole + ", instance profile was " + instanceProfileArn, sce); } } } catch (AmazonClientException ace) { @@ -99,25 +111,119 @@ protected void authenticate() { * Parses and returns the AWS account ID from the instance profile ARN. * * @return AWS account ID + * @deprecated no longer used, will be removed */ protected String lookupAccountId() { - final EC2MetadataUtils.IAMInfo iamInfo = EC2MetadataUtils.getIAMInstanceProfileInfo(); + final Matcher matcher = IAM_ARN_PATTERN.matcher(getInstanceProfileArn()); + + if (matcher.matches()) { + final String accountId = matcher.group("accountId"); + if (StringUtils.isNotBlank(accountId)) { + return accountId; + } + } + + throw new VaultClientException("Unable to obtain AWS account ID from instance profile ARN."); + } + + protected String getInstanceProfileArn() { + EC2MetadataUtils.IAMInfo iamInfo = EC2MetadataUtils.getIAMInstanceProfileInfo(); if (iamInfo == null) { final String errorMessage = "No IAM Instance Profile assigned to running instance."; LOGGER.error(errorMessage); throw new VaultClientException(errorMessage); } + return iamInfo.instanceProfileArn; + } - final Matcher matcher = IAM_ARN_PATTERN.matcher(iamInfo.instanceProfileArn); + /** + * Build a set of IAM Role ARNs from the information collected from the meta-data endpoint + * + * @param instanceProfileArn the instance-profile ARN + * @param securityCredentialsKeySet a set of role names + * @return + */ + protected static Set buildIamRoleArns(String instanceProfileArn, Set securityCredentialsKeySet) { - if (matcher.matches()) { - final String accountId = matcher.group("accountId"); - if (StringUtils.isNotBlank(accountId)) { - return accountId; - } + final Set result = new HashSet<>(); + + final InstanceProfileInfo instanceProfileInfo = parseInstanceProfileArn(instanceProfileArn); + final String accountId = instanceProfileInfo.accountId; + final String path = parsePathFromInstanceProfileName(instanceProfileInfo.profileName); + + for (String roleName : securityCredentialsKeySet) { + result.add(buildRoleArn(accountId, path, roleName)); } - throw new VaultClientException("Unable to obtain AWS account ID from instance profile ARN."); + return result; + } + + /** + * Parse an instance-profile ARN into parts + */ + protected static InstanceProfileInfo parseInstanceProfileArn(String instanceProfileArn) { + if (instanceProfileArn == null) { + throw new VaultClientException("instanceProfileArn provided was null rather than valid arn"); + } + + InstanceProfileInfo info = new InstanceProfileInfo(); + Matcher matcher = INSTANCE_PROFILE_ARN_PATTERN.matcher(instanceProfileArn); + boolean found = matcher.find(); + if (!found) { + throw new VaultClientException(String.format( + "Failed to find account id and role / instance profile name from ARN: %s using pattern %s", + instanceProfileArn, INSTANCE_PROFILE_ARN_PATTERN.pattern())); + } + + info.accountId = matcher.group(1); + info.profileName = matcher.group(2); + + return info; + } + + /** + * Parse the path out of a instanceProfileName or return null for no path + * + * e.g. parse "foo/bar" out of "foo/bar/name" + */ + protected static String parsePathFromInstanceProfileName(String instanceProfileName) { + if (StringUtils.contains(instanceProfileName, "/")) { + return StringUtils.substringBeforeLast(instanceProfileName, "/"); + } else { + return null; + } + } + + /** + * Build a role arn from the supplied arguments + */ + protected static String buildRoleArn(String accountId, String path, String roleName) { + return String.format(IAM_ROLE_ARN_FORMAT, accountId, roleWithPath(path, roleName)); + } + + /** + * If a path is supplied, prepend it to the role name. + * + * e.g. roleWithPath(null, "foo") returns "foo". + * e.g. roleWithPath("bar", "foo") returns "bar/foo". + * e.g. roleWithPath("bar/more", "foo") returns "bar/more/foo". + */ + protected static String roleWithPath(String path, String role) { + if (StringUtils.isBlank(path)) { + return role; + } else { + return StringUtils.appendIfMissing(path, "/") + role; + } + } + + /** + * Bean for holding Instance Profile parse results + */ + protected static class InstanceProfileInfo { + /** AWS Account ID */ + String accountId; + /** Name found after "instance-profile/" in the instance profile ARN, includes paths e.g. "foo/bar/name" */ + String profileName; } } diff --git a/src/test/java/com/nike/cerberus/client/auth/aws/InstanceRoleVaultCredentialsProviderTest.java b/src/test/java/com/nike/cerberus/client/auth/aws/InstanceRoleVaultCredentialsProviderTest.java index 1be9b96..baa62f2 100644 --- a/src/test/java/com/nike/cerberus/client/auth/aws/InstanceRoleVaultCredentialsProviderTest.java +++ b/src/test/java/com/nike/cerberus/client/auth/aws/InstanceRoleVaultCredentialsProviderTest.java @@ -26,9 +26,11 @@ import com.nike.vault.client.auth.VaultCredentials; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; +import org.apache.commons.lang3.StringUtils; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.internal.util.collections.Sets; import org.powermock.core.classloader.annotations.PowerMockIgnore; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; @@ -37,8 +39,13 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Set; +import static com.nike.cerberus.client.auth.aws.InstanceRoleVaultCredentialsProvider.buildIamRoleArns; +import static com.nike.cerberus.client.auth.aws.InstanceRoleVaultCredentialsProvider.buildRoleArn; +import static com.nike.cerberus.client.auth.aws.StaticIamRoleVaultCredentialsProvider.IAM_ROLE_ARN_FORMAT; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.any; import static org.powermock.api.mockito.PowerMockito.mock; import static org.powermock.api.mockito.PowerMockito.mockStatic; @@ -142,4 +149,86 @@ private void mockGetIamInstanceProfileInfo(final String instanceProfileArn) { when(EC2MetadataUtils.getIAMInstanceProfileInfo()).thenReturn(iamInfo); } + @Test + public void test_buildIamRoleArns_with_path() { + String instanceProfileArn = "arn:aws:iam::1234567890123:instance-profile/brewmaster/foo/brewmaster-foo-cerberus"; + Set roles = Sets.newSet("brewmaster-foo-cerberus"); + + Set results = buildIamRoleArns(instanceProfileArn, roles); + assertEquals(1, results.size()); + String result = results.iterator().next(); + + assertEquals("arn:aws:iam::1234567890123:role/brewmaster/foo/brewmaster-foo-cerberus", result); + } + + @Test + public void test_buildIamRoleArns_with_CloudFormation_style_names() { + String instanceProfileArn = "arn:aws:iam::1234567890123:instance-profile/foo-cerberus-LKSJDFIWERWER"; + Set roles = Sets.newSet("foo-cerberus-SDFLKJRWE234"); + + Set results = buildIamRoleArns(instanceProfileArn, roles); + assertEquals(1, results.size()); + String result = results.iterator().next(); + + assertEquals("arn:aws:iam::1234567890123:role/foo-cerberus-SDFLKJRWE234", result); + } + + @Test + public void test_buildIamRoleArns_CloudFormation_style_names_with_paths() { + String instanceProfileArn = "arn:aws:iam::1234567890123:instance-profile/brewmaster/foo/foo-cerberus-LKSJDFIWERWER"; + Set roles = Sets.newSet("foo-cerberus-SDFLKJRWE234"); + + Set results = buildIamRoleArns(instanceProfileArn, roles); + assertEquals(1, results.size()); + String result = results.iterator().next(); + + assertEquals("arn:aws:iam::1234567890123:role/brewmaster/foo/foo-cerberus-SDFLKJRWE234", result); + } + + @Test + public void test_parseInstanceProfileArn_with_path() { + String instanceProfileArn = "arn:aws:iam::1234567890123:instance-profile/brewmaster/foo/brewmaster-foo-cerberus"; + InstanceRoleVaultCredentialsProvider.InstanceProfileInfo info = InstanceRoleVaultCredentialsProvider.parseInstanceProfileArn(instanceProfileArn); + assertEquals("1234567890123", info.accountId); + assertEquals("brewmaster/foo/brewmaster-foo-cerberus", info.profileName); + } + + @Test + public void test_parseInstanceProfileArn_without_path() { + String instanceProfileArn = "arn:aws:iam::1234567890123:instance-profile/foo-cerberus"; + InstanceRoleVaultCredentialsProvider.InstanceProfileInfo info = InstanceRoleVaultCredentialsProvider.parseInstanceProfileArn(instanceProfileArn); + assertEquals("1234567890123", info.accountId); + assertEquals("foo-cerberus", info.profileName); + } + + @Test + public void test_parsePathFromInstanceProfileName() { + String instanceProfileName = "brewmaster/foo/brewmaster-foo-cerberus"; + String path = InstanceRoleVaultCredentialsProvider.parsePathFromInstanceProfileName(instanceProfileName); + assertEquals("brewmaster/foo", path); + } + + @Test + public void test_buildRoleArn_without_path() { + String accountId = "1234567890123"; + assertEquals("arn:aws:iam::1234567890123:role/foo", buildRoleArn(accountId, "", "foo")); + } + + @Test + public void test_buildRoleArn_with_null_path() { + String accountId = "1234567890123"; + assertEquals("arn:aws:iam::1234567890123:role/foo", buildRoleArn(accountId, null, "foo")); + } + + @Test + public void test_buildRoleArn_with_path() { + String accountId = "1234567890123"; + assertEquals("arn:aws:iam::1234567890123:role/bar/foo", buildRoleArn(accountId, "bar", "foo")); + } + + @Test + public void test_buildRoleArn_with_longer_path() { + String accountId = "1234567890123"; + assertEquals("arn:aws:iam::1234567890123:role/bar/more/foo", buildRoleArn(accountId, "bar/more", "foo")); + } } \ No newline at end of file