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

fix(lambda): support Lambda's new Invoke with Qualifier authorization strategy #19318

Merged

Conversation

madeline-k
Copy link
Contributor

@madeline-k madeline-k commented Mar 9, 2022

‼️ Lambda is changing their authorization strategy, which means that some behavior that was previously valid now results in access-denied errors.

Under the new behavior, customer lambda invocations will fail if the CDK generates a policy with an unqualified ARN as the resource, and the customer invokes lambda with the unqualified ARN and the Qualifier request parameter.

Example of an affected setup:

Statement: 
{
  Effect: "Allow",
  Action: "lambda:InvokeFunction",
  Resource: "arn:aws:lambda:...:function:MyFunction",
}

API Call:
lambda.Invoke({
  FunctionName: "MyFunction",
  Qualifier: "1234",
})

This Invoke call used to succeed, but under the new authorization strategy it will fail. The required statement to make the call succeed would be (note the qualified ARN):

{
  Effect: "Allow",
  Action: "lambda:InvokeFunction",
  Resource: "arn:aws:lambda:...:function:MyFunction:1234",
}

This PR aims to align the CDK with the new authorization strategy. The PR introduces changes to the grantInvoke() api on a lambda function. Now, when calling grantInvoke() on a lambda function, [ARN, ARN:*] is used in the identity policy, so that identities that are granted permission to invoke the Function may also invoke all of its Versions and Aliases.

When calling grantInvoke() on a lambda function Version or Alias, the generated identity policy will remain the same, and only include ARN:<version/alias> in the policy.

This is part of #19273


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Mar 9, 2022

@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Mar 9, 2022
@madeline-k madeline-k requested review from rix0rrr, kaizencc and a team March 9, 2022 23:12
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Mar 9, 2022
@madeline-k madeline-k added the pr/do-not-merge This PR should not be merged at this time. label Mar 10, 2022
@madeline-k
Copy link
Contributor Author

Adding do not merge label until we decide whether or not to Feature Flag this change

@kaizencc
Copy link
Contributor

If I'm not mistaken, when this is complete it should also close #17515.

@madeline-k
Copy link
Contributor Author

If I'm not mistaken, when this is complete it should also close #17515.

@kaizen3031593, It will not. But I am planning to do that in another PR. This PR only adds [ARN, ARN:*] when you call grantInvoke on a "regular" Function. Not on a Version or Alias.

In a separate PR, we will add the [ARN, ARN:*] permissions for calling grantInvoke on a Version, when it is used in a stepfunction task. That part will fix #17515.

On a somewhat related note - calling grantInvoke on an Alias will always remain the same, and only grant permissions to ARN:alias, because one of the purposes of an Alias is to restrict users to only have access to a specific version of the function.

@kaizencc kaizencc added the pr-linter/exempt-readme The PR linter will not require README changes label Mar 17, 2022
@kaizencc
Copy link
Contributor

I updated 1. API ref, but for 2 & 3 I am going to need some help concocting a coherent message. I'll take a stab at 3. and update the description to my liking.

Copy link
Contributor Author

@madeline-k madeline-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really like your updates to the PR description 👍

Thanks for fighting with the integ tests! Let me know if I can help

Comment on lines +68 to +69
* This property is for cdk modules to consume only. You should not need to use this property.
* Instead, use grantInvoke() directly.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

@kaizencc kaizencc removed the pr/do-not-merge This PR should not be merged at this time. label Mar 24, 2022
@kaizencc
Copy link
Contributor

Finally got the build working for this one. Some of the integ tests have changed assets, at this point, I think that we should be okay with it. The build fails without these changes.

@mergify
Copy link
Contributor

mergify bot commented Mar 24, 2022

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@rix0rrr rix0rrr changed the title feat(lambda): support Lambda's new IAM authorization behavior for identity policies generated by grantInvoke() fix(lambda): support Lambda's new IAM authorization behavior for identity policies generated by grantInvoke() Mar 24, 2022
@rix0rrr rix0rrr changed the title fix(lambda): support Lambda's new IAM authorization behavior for identity policies generated by grantInvoke() fix(lambda): support Lambda's new Invoke with Qualifier authorization strategy Mar 24, 2022
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: ebb9b53
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit d06b27f into aws:master Mar 24, 2022
@mergify
Copy link
Contributor

mergify bot commented Mar 24, 2022

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify bot pushed a commit that referenced this pull request Mar 31, 2022
… `currentVersion` (#19464)

‼️ Lambda is changing their authorization strategy, which means that some behavior that was previously valid now results in `access-denied` errors.

Under the new behavior, customer lambda invocations will fail if the CDK generates a policy with an unqualified ARN as the resource, and the customer invokes lambda with the unqualified ARN and the `Qualifier` request parameter. 

Example of an affected setup:

```
Statement: 
{
  Effect: "Allow",
  Action: "lambda:InvokeFunction",
  Resource: "arn:aws:lambda:...:function:MyFunction",
}

API Call:
lambda.Invoke({
  FunctionName: "MyFunction",
  Qualifier: "1234",
})
```

This `Invoke` call *used* to succeed, but under the new authorization strategy it will fail. The required statement to make the call succeed would be (note the qualified ARN):

```
{
  Effect: "Allow",
  Action: "lambda:InvokeFunction",
  Resource: "arn:aws:lambda:...:function:MyFunction:1234",
}
```

This PR aims to warn users who could be using an affected setup. Users will receive the a warning message under the following circumstances:

- they grant `lambda:InvokeFunction` to an unqualified function arn
- they call `lambda.currentVersion` somewhere in their code

This is part of #19273. Related is #19318.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
StevePotter pushed a commit to StevePotter/aws-cdk that referenced this pull request Apr 27, 2022
…tion strategy (aws#19318)

‼️ Lambda is changing their authorization strategy, which means that some behavior that was previously valid now results in `access-denied` errors.

Under the new behavior, customer lambda invocations will fail if the CDK generates a policy with an unqualified ARN as the resource, and the customer invokes lambda with the unqualified ARN and the `Qualifier` request parameter. 

Example of an affected setup:

```
Statement: 
{
  Effect: "Allow",
  Action: "lambda:InvokeFunction",
  Resource: "arn:aws:lambda:...:function:MyFunction",
}

API Call:
lambda.Invoke({
  FunctionName: "MyFunction",
  Qualifier: "1234",
})
```

This `Invoke` call *used* to succeed, but under the new authorization strategy it will fail. The required statement to make the call succeed would be (note the qualified ARN):

```
{
  Effect: "Allow",
  Action: "lambda:InvokeFunction",
  Resource: "arn:aws:lambda:...:function:MyFunction:1234",
}
```

This PR aims to align the CDK with the new authorization strategy. The PR introduces changes to the `grantInvoke()` api on a lambda function. Now, when calling `grantInvoke()` on a lambda function, `[ARN, ARN:*]` is used in the identity policy, so that identities that are granted permission to invoke the Function may also invoke all of its Versions and Aliases. 

When calling `grantInvoke()` on a lambda function Version or Alias, the generated identity policy will remain the same, and only include `ARN:<version/alias>` in the policy.

This is part of aws#19273


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
StevePotter pushed a commit to StevePotter/aws-cdk that referenced this pull request Apr 27, 2022
… `currentVersion` (aws#19464)

‼️ Lambda is changing their authorization strategy, which means that some behavior that was previously valid now results in `access-denied` errors.

Under the new behavior, customer lambda invocations will fail if the CDK generates a policy with an unqualified ARN as the resource, and the customer invokes lambda with the unqualified ARN and the `Qualifier` request parameter. 

Example of an affected setup:

```
Statement: 
{
  Effect: "Allow",
  Action: "lambda:InvokeFunction",
  Resource: "arn:aws:lambda:...:function:MyFunction",
}

API Call:
lambda.Invoke({
  FunctionName: "MyFunction",
  Qualifier: "1234",
})
```

This `Invoke` call *used* to succeed, but under the new authorization strategy it will fail. The required statement to make the call succeed would be (note the qualified ARN):

```
{
  Effect: "Allow",
  Action: "lambda:InvokeFunction",
  Resource: "arn:aws:lambda:...:function:MyFunction:1234",
}
```

This PR aims to warn users who could be using an affected setup. Users will receive the a warning message under the following circumstances:

- they grant `lambda:InvokeFunction` to an unqualified function arn
- they call `lambda.currentVersion` somewhere in their code

This is part of aws#19273. Related is aws#19318.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda contribution/core This is a PR that came from AWS. pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exempt-readme The PR linter will not require README changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants