Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
hssyoo opened this issue Sep 3, 2023 · 4 comments
Labels
@aws-cdk/aws-ssm Related to AWS Systems Manager bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@hssyoo
Copy link

hssyoo commented Sep 3, 2023

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

@hssyoo hssyoo added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 3, 2023
@github-actions github-actions bot added the @aws-cdk/aws-ssm Related to AWS Systems Manager label Sep 3, 2023
@hssyoo hssyoo changed the title (aws-ssm): (Generated ssm:GetParameters policy contains double forward slashes in resource ARN) (aws-ssm): Generated ssm:GetParameters policy contains double forward slashes in resource ARN Sep 3, 2023
@msambol
Copy link
Contributor

msambol commented Sep 4, 2023

@peterwoodworth I can take this.

@peterwoodworth
Copy link
Contributor

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.

@peterwoodworth peterwoodworth added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Sep 5, 2023
@msambol
Copy link
Contributor

msambol commented Sep 5, 2023

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

@peterwoodworth peterwoodworth added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Sep 5, 2023
@github-actions
Copy link

github-actions bot commented Sep 8, 2023

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.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ssm Related to AWS Systems Manager bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

3 participants