Skip to content

Commit

Permalink
feat(lambda): warn if you use function.grantInvoke while also using…
Browse files Browse the repository at this point in the history
… `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*
  • Loading branch information
kaizencc authored and Stephen Potter committed Apr 27, 2022
1 parent 3d9b742 commit 65648de
Show file tree
Hide file tree
Showing 3 changed files with 198 additions and 2 deletions.
46 changes: 45 additions & 1 deletion packages/@aws-cdk/aws-lambda/lib/function-base.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as cloudwatch from '@aws-cdk/aws-cloudwatch';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import { ArnFormat, ConstructNode, IResource, Resource, Token } from '@aws-cdk/core';
import { Annotations, ArnFormat, ConstructNode, IResource, Resource, Token } from '@aws-cdk/core';
import { AliasOptions } from './alias';
import { Architecture } from './architecture';
import { EventInvokeConfig, EventInvokeConfigOptions } from './event-invoke-config';
Expand All @@ -12,6 +12,10 @@ import { CfnPermission } from './lambda.generated';
import { Permission } from './permission';
import { addAlias, flatMap } from './util';

// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
// eslint-disable-next-line no-duplicate-imports, import/order
import { Construct } from '@aws-cdk/core';

export interface IFunction extends IResource, ec2.IConnectable, iam.IGrantable {

/**
Expand Down Expand Up @@ -274,12 +278,45 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC

private _latestVersion?: LatestVersion;

/**
* Flag to delay adding a warning message until current version is invoked.
* @internal
*/
protected _warnIfCurrentVersionCalled: boolean = false;

/**
* Mapping of invocation principals to grants. Used to de-dupe `grantInvoke()` calls.
* @internal
*/
protected _invocationGrants: Record<string, iam.Grant> = {};

/**
* A warning will be added to functions under the following conditions:
* - permissions that include `lambda:InvokeFunction` are added to the unqualified function.
* - function.currentVersion is invoked before or after the permission is created.
*
* This applies only to permissions on Lambda functions, not versions or aliases.
* This function is overridden as a noOp for QualifiedFunctionBase.
*/
public considerWarningOnInvokeFunctionPermissions(scope: Construct, action: string) {
const affectedPermissions = ['lambda:InvokeFunction', 'lambda:*', 'lambda:Invoke*'];
if (affectedPermissions.includes(action)) {
if (scope.node.tryFindChild('CurrentVersion')) {
this.warnInvokeFunctionPermissions(scope);
} else {
this._warnIfCurrentVersionCalled = true;
}
}
}

protected warnInvokeFunctionPermissions(scope: Construct): void {
Annotations.of(scope).addWarning([
"AWS Lambda has changed their authorization strategy, which may cause client invocations using the 'Qualifier' parameter of the lambda function to fail with Access Denied errors.",
"If you are using a lambda Version or Alias, make sure to call 'grantInvoke' or 'addPermission' on the Version or Alias, not the underlying Function",
'See: https://github.com/aws/aws-cdk/issues/19273',
].join('\n'));
}

/**
* Adds a permission to the Lambda resource policy.
* @param id The id for the permission construct
Expand All @@ -296,6 +333,8 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC
const action = permission.action ?? 'lambda:InvokeFunction';
const scope = permission.scope ?? this;

this.considerWarningOnInvokeFunctionPermissions(scope, action);

new CfnPermission(scope, id, {
action,
principal,
Expand Down Expand Up @@ -554,6 +593,11 @@ export abstract class QualifiedFunctionBase extends FunctionBase {
...options,
});
}

public considerWarningOnInvokeFunctionPermissions(_scope: Construct, _action: string): void {
// noOp
return;
}
}

/**
Expand Down
4 changes: 4 additions & 0 deletions packages/@aws-cdk/aws-lambda/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,10 @@ export class Function extends FunctionBase {
return this._currentVersion;
}

if (this._warnIfCurrentVersionCalled) {
this.warnInvokeFunctionPermissions(this);
};

this._currentVersion = new Version(this, 'CurrentVersion', {
lambda: this,
...this.currentVersionOptions,
Expand Down
150 changes: 149 additions & 1 deletion packages/@aws-cdk/aws-lambda/test/function.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as path from 'path';
import { Match, Template } from '@aws-cdk/assertions';
import { Annotations, Match, Template } from '@aws-cdk/assertions';
import { ProfilingGroup } from '@aws-cdk/aws-codeguruprofiler';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as efs from '@aws-cdk/aws-efs';
Expand Down Expand Up @@ -435,6 +435,154 @@ describe('function', () => {
// THEN
Template.fromStack(stack).resourceCountIs('AWS::Lambda::Permission', 0);
});

describe('annotations on different IFunctions', () => {
let stack: cdk.Stack;
let fn: lambda.Function;
let warningMessage: string;
beforeEach(() => {
warningMessage = 'AWS Lambda has changed their authorization strategy';
stack = new cdk.Stack();
fn = new lambda.Function(stack, 'MyLambda', {
code: lambda.Code.fromAsset(path.join(__dirname, 'my-lambda-handler')),
handler: 'index.handler',
runtime: lambda.Runtime.PYTHON_3_6,
});
});

describe('permissions on functions', () => {
test('without lambda:InvokeFunction', () => {
// WHEN
fn.addPermission('MyPermission', {
action: 'lambda.GetFunction',
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
});

// Simulate a workflow where a user has created a currentVersion with the intent to invoke it later.
fn.currentVersion;

// THEN
Annotations.fromStack(stack).hasNoWarning('/Default/MyLambda', Match.stringLikeRegexp(warningMessage));
});

describe('with lambda:InvokeFunction', () => {
test('without invoking currentVersion', () => {
// WHEN
fn.addPermission('MyPermission', {
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
});

// THEN
Annotations.fromStack(stack).hasNoWarning('/Default/MyLambda', Match.stringLikeRegexp(warningMessage));
});

test('with currentVersion invoked first', () => {
// GIVEN
// Simulate a workflow where a user has created a currentVersion with the intent to invoke it later.
fn.currentVersion;

// WHEN
fn.addPermission('MyPermission', {
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
});

// THEN
Annotations.fromStack(stack).hasWarning('/Default/MyLambda', Match.stringLikeRegexp(warningMessage));
});

test('with currentVersion invoked after permissions created', () => {
// WHEN
fn.addPermission('MyPermission', {
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
});

// Simulate a workflow where a user has created a currentVersion after adding permissions to the function.
fn.currentVersion;

// THEN
Annotations.fromStack(stack).hasWarning('/Default/MyLambda', Match.stringLikeRegexp(warningMessage));
});

test('multiple currentVersion calls does not result in multiple warnings', () => {
// WHEN
fn.currentVersion;

fn.addPermission('MyPermission', {
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
});

fn.currentVersion;

// THEN
const warns = Annotations.fromStack(stack).findWarning('/Default/MyLambda', Match.stringLikeRegexp(warningMessage));
expect(warns).toHaveLength(1);
});
});
});

test('permission on versions', () => {
// GIVEN
const version = new lambda.Version(stack, 'MyVersion', {
lambda: fn.currentVersion,
});

// WHEN
version.addPermission('MyPermission', {
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
});

// THEN
Annotations.fromStack(stack).hasNoWarning('/Default/MyVersion', Match.stringLikeRegexp(warningMessage));
});

test('permission on latest version', () => {
// WHEN
fn.latestVersion.addPermission('MyPermission', {
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
});

// THEN
// cannot add permissions on latest version, so no warning necessary
Annotations.fromStack(stack).hasNoWarning('/Default/MyLambda/$LATEST', Match.stringLikeRegexp(warningMessage));
});

describe('permission on alias', () => {
test('of current version', () => {
// GIVEN
const version = new lambda.Version(stack, 'MyVersion', {
lambda: fn.currentVersion,
});
const alias = new lambda.Alias(stack, 'MyAlias', {
aliasName: 'alias',
version,
});

// WHEN
alias.addPermission('MyPermission', {
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
});

// THEN
Annotations.fromStack(stack).hasNoWarning('/Default/MyAlias', Match.stringLikeRegexp(warningMessage));
});

test('of latest version', () => {
// GIVEN
const alias = new lambda.Alias(stack, 'MyAlias', {
aliasName: 'alias',
version: fn.latestVersion,
});

// WHEN
alias.addPermission('MyPermission', {
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
});

// THEN
Annotations.fromStack(stack).hasNoWarning('/Default/MyAlias', Match.stringLikeRegexp(warningMessage));
});
});
});
});

test('Lambda code can be read from a local directory via an asset', () => {
Expand Down

0 comments on commit 65648de

Please sign in to comment.