Skip to content

Commit

Permalink
fix(codebuild): Secret env variable as token from another account fai…
Browse files Browse the repository at this point in the history 
…ls on Key decryption (aws#14483)

fixes aws#14477


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@Kruspe @hollanddd
Kruspe authored and hollanddd committed Aug 26, 2021
1 parent d009913 commit 895b8d7
Show file tree
Hide file tree
Showing 6 changed files with 338 additions and 38 deletions.
101 changes: 69 additions & 32 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as iam from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import * as s3 from '@aws-cdk/aws-s3';
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
import { ArnComponents, Aws, Duration, IResource, Lazy, Names, PhysicalName, Resource, SecretValue, Stack, Token, TokenComparison, Tokenization } from '@aws-cdk/core';
import { Aws, Duration, IResource, Lazy, Names, PhysicalName, Reference, Resource, SecretValue, Stack, Token, TokenComparison, Tokenization } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { IArtifacts } from './artifacts';
import { BuildSpec } from './build-spec';
Expand Down Expand Up @@ -764,59 +764,96 @@ export class Project extends ProjectBase {

// save SecretsManager env variables
if (envVariable.type === BuildEnvironmentVariableType.SECRETS_MANAGER) {
if (Token.isUnresolved(envVariableValue)) {
// the value of the property can be a complex string, separated by ':';
// see https://docs.aws.amazon.com/codebuild/latest/userguide/build-spec-ref.html#build-spec.env.secrets-manager
const secretArn = envVariableValue.split(':')[0];

// if we are passed a Token, we should assume it's the ARN of the Secret
// (as the name would not work anyway, because it would be the full name, which CodeBuild does not support)
secretsManagerIamResources.add(secretArn);
} else {
// check if the provided value is a full ARN of the Secret
let parsedArn: ArnComponents | undefined;
try {
parsedArn = stack.parseArn(envVariableValue, ':');
} catch (e) {}
const secretSpecifier: string = parsedArn ? parsedArn.resourceName : envVariableValue;
// We have 3 basic cases here of what envVariableValue can be:
// 1. A string that starts with 'arn:' (and might contain Token fragments).
// 2. A Token.
// 3. A simple value, like 'secret-id'.
if (envVariableValue.startsWith('arn:')) {
const parsedArn = stack.parseArn(envVariableValue, ':');
if (!parsedArn.resourceName) {
throw new Error('SecretManager ARN is missing the name of the secret: ' + envVariableValue);
}

// the value of the property can be a complex string, separated by ':';
// see https://docs.aws.amazon.com/codebuild/latest/userguide/build-spec-ref.html#build-spec.env.secrets-manager
const secretName = secretSpecifier.split(':')[0];
const secretIamResourceName = parsedArn
// If we were given an ARN, we don't' know whether the name is full, or partial,
// as CodeBuild supports both ARN forms.
// Because of that, follow the name with a '*', which works for both
? `${secretName}*`
// If we were given just a name, it must be partial, as CodeBuild doesn't support providing full names.
// In this case, we need to accommodate for the generated suffix in the IAM resource name
: `${secretName}-??????`;
const secretName = parsedArn.resourceName.split(':')[0];
secretsManagerIamResources.add(stack.formatArn({
service: 'secretsmanager',
resource: 'secret',
resourceName: secretIamResourceName,
// since we don't know whether the ARN was full, or partial
// (CodeBuild supports both),
// stick a "*" at the end, which makes it work for both
resourceName: `${secretName}*`,
sep: ':',
// if we were given an ARN, we need to use the provided partition/account/region
partition: parsedArn?.partition,
account: parsedArn?.account,
region: parsedArn?.region,
partition: parsedArn.partition,
account: parsedArn.account,
region: parsedArn.region,
}));
// if secret comes from another account, SecretsManager will need to access
// KMS on the other account as well to be able to get the secret
if (parsedArn && parsedArn.account && Token.compareStrings(parsedArn.account, stack.account) === TokenComparison.DIFFERENT) {
if (parsedArn.account && Token.compareStrings(parsedArn.account, stack.account) === TokenComparison.DIFFERENT) {
kmsIamResources.add(stack.formatArn({
service: 'kms',
resource: 'key',
// We do not know the ID of the key, but since this is a cross-account access,
// the key policies have to allow this access, so a wildcard is safe here
resourceName: '*',
sep: '/',
// if we were given an ARN, we need to use the provided partition/account/region
partition: parsedArn.partition,
account: parsedArn.account,
region: parsedArn.region,
}));
}
} else if (Token.isUnresolved(envVariableValue)) {
// the value of the property can be a complex string, separated by ':';
// see https://docs.aws.amazon.com/codebuild/latest/userguide/build-spec-ref.html#build-spec.env.secrets-manager
let secretArn = envVariableValue.split(':')[0];

// parse the Token, and see if it represents a single resource
// (we will assume it's a Secret from SecretsManager)
const fragments = Tokenization.reverseString(envVariableValue);
if (fragments.tokens.length === 1) {
const resolvable = fragments.tokens[0];
if (Reference.isReference(resolvable)) {
// check the Stack the resource owning the reference belongs to
const resourceStack = Stack.of(resolvable.target);
if (Token.compareStrings(stack.account, resourceStack.account) === TokenComparison.DIFFERENT) {
// since this is a cross-account access,
// add the appropriate KMS permissions
kmsIamResources.add(stack.formatArn({
service: 'kms',
resource: 'key',
// We do not know the ID of the key, but since this is a cross-account access,
// the key policies have to allow this access, so a wildcard is safe here
resourceName: '*',
sep: '/',
partition: resourceStack.partition,
account: resourceStack.account,
region: resourceStack.region,
}));

// Work around a bug in SecretsManager -
// when the access is cross-environment,
// Secret.secretArn returns a partial ARN!
// So add a "*" at the end, so that the permissions work
secretArn = `${secretArn}-??????`;
}
}
}

// if we are passed a Token, we should assume it's the ARN of the Secret
// (as the name would not work anyway, because it would be the full name, which CodeBuild does not support)
secretsManagerIamResources.add(secretArn);
} else {
// the value of the property can be a complex string, separated by ':';
// see https://docs.aws.amazon.com/codebuild/latest/userguide/build-spec-ref.html#build-spec.env.secrets-manager
const secretName = envVariableValue.split(':')[0];
secretsManagerIamResources.add(stack.formatArn({
service: 'secretsmanager',
resource: 'secret',
resourceName: `${secretName}-??????`,
sep: ':',
}));
}
}
}
Expand Down
236 changes: 236 additions & 0 deletions packages/@aws-cdk/aws-codebuild/test/test.project.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1466,6 +1466,242 @@ export = {

test.done();
},

'can be provided as a SecretArn of a new Secret, with its physical name set, created in a different account'(test: Test) {
// GIVEN
const app = new cdk.App();
const secretStack = new cdk.Stack(app, 'SecretStack', {
env: { account: '012345678912' },
});
const stack = new cdk.Stack(app, 'ProjectStack', {
env: { account: '123456789012' },
});

// WHEN
const secret = new secretsmanager.Secret(secretStack, 'Secret', { secretName: 'secret-name' });
new codebuild.PipelineProject(stack, 'Project', {
environmentVariables: {
'ENV_VAR1': {
type: codebuild.BuildEnvironmentVariableType.SECRETS_MANAGER,
value: secret.secretArn,
},
},
});

// THEN
expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', {
'Environment': {
'EnvironmentVariables': [
{
'Name': 'ENV_VAR1',
'Type': 'SECRETS_MANAGER',
'Value': {
'Fn::Join': ['', [
'arn:',
{ 'Ref': 'AWS::Partition' },
':secretsmanager:',
{ 'Ref': 'AWS::Region' },
':012345678912:secret:secret-name',
]],
},
},
],
},
}));

expect(stack).to(haveResourceLike('AWS::IAM::Policy', {
'PolicyDocument': {
'Statement': arrayWith({
'Action': 'secretsmanager:GetSecretValue',
'Effect': 'Allow',
'Resource': {
'Fn::Join': ['', [
'arn:',
{ 'Ref': 'AWS::Partition' },
':secretsmanager:',
{ 'Ref': 'AWS::Region' },
':012345678912:secret:secret-name-??????',
]],
},
}),
},
}));

expect(stack).to(haveResourceLike('AWS::IAM::Policy', {
'PolicyDocument': {
'Statement': arrayWith({
'Action': 'kms:Decrypt',
'Effect': 'Allow',
'Resource': {
'Fn::Join': ['', [
'arn:',
{ 'Ref': 'AWS::Partition' },
':kms:',
{ 'Ref': 'AWS::Region' },
':012345678912:key/*',
]],
},
}),
},
}));

test.done();
},

'can be provided as a SecretArn of a Secret imported by name in a different account'(test: Test) {
// GIVEN
const app = new cdk.App();
const secretStack = new cdk.Stack(app, 'SecretStack', {
env: { account: '012345678912' },
});
const stack = new cdk.Stack(app, 'ProjectStack', {
env: { account: '123456789012' },
});

// WHEN
const secret = secretsmanager.Secret.fromSecretNameV2(secretStack, 'Secret', 'secret-name');
new codebuild.PipelineProject(stack, 'Project', {
environmentVariables: {
'ENV_VAR1': {
type: codebuild.BuildEnvironmentVariableType.SECRETS_MANAGER,
value: `${secret.secretArn}:json-key`,
},
},
});

// THEN
expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', {
'Environment': {
'EnvironmentVariables': [
{
'Name': 'ENV_VAR1',
'Type': 'SECRETS_MANAGER',
'Value': {
'Fn::Join': ['', [
'arn:',
{ 'Ref': 'AWS::Partition' },
':secretsmanager:',
{ 'Ref': 'AWS::Region' },
':012345678912:secret:secret-name:json-key',
]],
},
},
],
},
}));

expect(stack).to(haveResourceLike('AWS::IAM::Policy', {
'PolicyDocument': {
'Statement': arrayWith({
'Action': 'secretsmanager:GetSecretValue',
'Effect': 'Allow',
'Resource': {
'Fn::Join': ['', [
'arn:',
{ 'Ref': 'AWS::Partition' },
':secretsmanager:',
{ 'Ref': 'AWS::Region' },
':012345678912:secret:secret-name*',
]],
},
}),
},
}));

expect(stack).to(haveResourceLike('AWS::IAM::Policy', {
'PolicyDocument': {
'Statement': arrayWith({
'Action': 'kms:Decrypt',
'Effect': 'Allow',
'Resource': {
'Fn::Join': ['', [
'arn:',
{ 'Ref': 'AWS::Partition' },
':kms:',
{ 'Ref': 'AWS::Region' },
':012345678912:key/*',
]],
},
}),
},
}));

test.done();
},

'can be provided as a SecretArn of a Secret imported by complete ARN from a different account'(test: Test) {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'ProjectStack', {
env: { account: '123456789012' },
});
const secretArn = 'arn:aws:secretsmanager:us-west-2:901234567890:secret:mysecret-123456';

// WHEN
const secret = secretsmanager.Secret.fromSecretCompleteArn(stack, 'Secret', secretArn);
new codebuild.PipelineProject(stack, 'Project', {
environmentVariables: {
'ENV_VAR1': {
type: codebuild.BuildEnvironmentVariableType.SECRETS_MANAGER,
value: secret.secretArn,
},
},
});

// THEN
expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', {
'Environment': {
'EnvironmentVariables': [
{
'Name': 'ENV_VAR1',
'Type': 'SECRETS_MANAGER',
'Value': secretArn,
},
],
},
}));

expect(stack).to(haveResourceLike('AWS::IAM::Policy', {
'PolicyDocument': {
'Statement': arrayWith({
'Action': 'secretsmanager:GetSecretValue',
'Effect': 'Allow',
'Resource': `${secretArn}*`,
}),
},
}));

expect(stack).to(haveResourceLike('AWS::IAM::Policy', {
'PolicyDocument': {
'Statement': arrayWith({
'Action': 'kms:Decrypt',
'Effect': 'Allow',
'Resource': 'arn:aws:kms:us-west-2:901234567890:key/*',
}),
},
}));

test.done();
},

'should fail when the parsed Arn does not contain a secret name'(test: Test) {
// GIVEN
const stack = new cdk.Stack();

// WHEN
test.throws(() => {
new codebuild.PipelineProject(stack, 'Project', {
environmentVariables: {
'ENV_VAR1': {
type: codebuild.BuildEnvironmentVariableType.SECRETS_MANAGER,
value: 'arn:aws:secretsmanager:us-west-2:123456789012:secret',
},
},
});
}, /SecretManager ARN is missing the name of the secret:/);

test.done();
},
},

'should fail creating when using a secret value in a plaintext variable'(test: Test) {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-secretsmanager/lib/secret.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ export class Secret extends SecretBase {
public readonly secretName = parseSecretName(scope, secretArn);
protected readonly autoCreatePolicy = false;
public get secretFullArn() { return secretArnIsPartial ? undefined : secretArn; }
}(scope, id);
}(scope, id, { environmentFromArn: secretArn });
}

public readonly encryptionKey?: kms.IKey;
Expand Down
Loading

0 comments on commit 895b8d7

Please sign in to comment.