aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.72k stars 3.94k forks source link

(aws-ssm): Generated ssm:GetParameters policy contains double forward slashes in resource ARN #26990

Closed hssyoo closed 1 year ago

hssyoo commented 1 year ago

Describe the bug

A CodeBuild Project can take a prop environmentVariables, which is of type BuildEnvironmentVariable. BuildEnvironmentVariable has a prop value, which can take SSM Parameter names. Per the docs:

For SSM parameter variables, pass the name of the parameter here (parameterName property of IParameter).

I'm trying to create a StringParameter in my stack with the name /foo/bar. Let's call it fooBar:

const fooBar = new StringParameter(
  this,
  'FooBar',
    {
      stringValue: 'baz',
      parameterName: '/foo/bar',
    }
);

And when I'm defining my CodeBuild Project prop's environmentVariables, I want to pass fooBar.parameterName (per the docs) into the value field:

{
  FOOBAR_ENV_VAR: {
    value: fooBar.parameterName,
    type: BuildEnvironmentVariableType.PARAMETER_STORE,
  }
}

I'm able to successfully build and deploy my app, but when I check the ssm:GetParameters policy attached to the project role, I see that the resource ARN contains double forward slashes:

{
  "Action": "ssm:GetParameters",
  "Resource": [
    "arn:aws:ssm:us-west-2:00000000000:parameter//foo/bar",
  ],
  "Effect": "Allow"
}

This causes my CodeBuild job to fail since the project role has been given permissions to the wrong resource:

[Container] 2023/09/01 23:15:58 Phase context status code: Decrypted Variables Error Message: 
AccessDeniedException: User: arn:aws:sts::00000000000:assumed-role/Project-role is not authorized to perform: 
ssm:GetParameters on resource: arn:aws:ssm:us-west-2:00000000000:parameter/foo/bar because no identity-based 
policy allows the ssm:GetParameters action

I noticed that when the CDK serializes environment variables, it has logic that strips SSM parameter names of the leading slash if it contains one. However, because I'm passing in the parameterName property of a construct, the string value is a reference to the parameter resource name and not the actual name itself. One can see it in their generated CloudFormation template:

         "Fn::Join": [
          "",
          [
           "arn:",
           {
            "Ref": "AWS::Partition"
           },
           ":ssm:us-west-2:00000000000:parameter/arn:",
           {
            "Ref": "AWS::Partition"
           },
           ":ssm:us-west-2:00000000000:parameter",
           {
            "Ref": "FooBar022DCA45"
           }
          ]
         ]

Because the value is a reference and not the name itself (/foo/bar), the leading slash is not detected. This results in a malformed resource ARN at runtime, leading to the bad policy.

Expected Behavior

The project role should have a policy attached that has the correct resource ARN for the string parameters without the double slash:

{
  "Action": "ssm:GetParameters",
  "Resource": [
    "arn:aws:ssm:us-west-2:00000000000:parameter/foo/bar",
  ],
  "Effect": "Allow"
}

Current Behavior

It contains double slashes, resulting in a failed CodeBuild job:

{
  "Action": "ssm:GetParameters",
  "Resource": [
    "arn:aws:ssm:us-west-2:00000000000:parameter//foo/bar",
  ],
  "Effect": "Allow"
}

Reproduction Steps

Create a StringParameter in your stack with a parameterName that contains a forward slash:

const fooBar = new StringParameter(
  this,
  'FooBar',
  {
    stringValue: 'baz',
    parameterName: '/foo/bar',
  }
);

Create a CodeBuild Project that passes in fooBar.parameterName as the value for BuildEnvironmentVariableType when defining the Project's environmentVariables property:

const project = new codebuild.Project(
  this,
  'TestProject',
  {
    // other props you need
    environmentVariables: {
      FOOBAR_ENV_VAR: {
        value: fooBar.parameterName,
        type: BuildEnvironmentVariableType.PARAMETER_STORE
      }
    }
    // other props you need
  }

Possible Solution

The root cause is that when removing leading forward slashes from the parameter name, it's not taken into account that the value could be an unresolved reference to the parameter name and not the actual parameter name itself. This results in the actual parameter name never being stripped of its leading forward slash.

I'm not sure about a fix but a workaround is to pass the parameter name as a hardcoded string rather than using the parameterName property of a StringParameter. While this is a straightforward workaround, I'd love to see a long-term fix since from an interface-perspective, it's awkward to not be able to reference the StringParameter construct that I made that already encapsulates and exposes the parameter name. It's also inconsistent with the docs since it specifically says to use the parameterName property as the value.

Additional Information/Context

No response

CDK CLI Version

2.77.0

Framework Version

No response

Node.js Version

18.16.3

OS

Amazon Linux 2

Language

Typescript

Language Version

No response

Other information

No response

msambol commented 1 year ago

@peterwoodworth I can take this.

peterwoodworth commented 1 year ago

This is happening because the StringParameter.parameterName is a token that the value isn't known until deploy time. Is it possible for you to refactor your code such that you're passing in the same value to the parameterName and value?

@msambol I'm not sure which solution you would have in mind for this, but if you have one in mind that fixes it then go for it, but I'm not sure how we'd handle this on our end.

msambol commented 1 year ago

@peterwoodworth I spent some time on it..couldn't think of anything.

github-actions[bot] commented 1 year ago

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.