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

apigatewayv2: incorrect arn function causing unwanted behavior #33218

Closed
1 task
IkeNefcy opened this issue Jan 28, 2025 · 7 comments · Fixed by #33100
Closed
1 task

apigatewayv2: incorrect arn function causing unwanted behavior #33218

IkeNefcy opened this issue Jan 28, 2025 · 7 comments · Fixed by #33100
Assignees
Labels
@aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@IkeNefcy
Copy link
Contributor

IkeNefcy commented Jan 28, 2025

Describe the bug

In API gateway when building a websocket, the current function for arnForExecuteApi is too similar to the original function created for rest API.

The function would produce an arn like this (with default / no args)
"Resource": "arn:aws:execute-api:us-east-1:acc:apiId/*/*/*",

which is an issue because there are too many asterisks. When using an IAM authorizer on the websocket API, having too many wildcards like this causes the IAM Authorizer to deny the iam role from calling the API. You need to have the exact number of wild cards.

Websocket ARNs are structured differently than REST APIs
They look like this:
arn:aws:execute-api:us-east-1:acc:apiId/stage/$connect

This means to get all stages and all routes, you need an arn like this :
"Resource": "arn:aws:execute-api:us-east-1:acc:apiId/*/*",
in IAM.

The only way around this would be if users were trimming off the last 2 characters, but ideally the CDK should produce this expected output instead with only 2 /* wildcards.

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

Calling arnForExecuteApi for a websocket API should return an ARN in this structure
arn:aws:execute-api:us-east-1:acc:apiId/*/*

Current Behavior

The function arnForExecuteApi produces
arn:aws:execute-api:us-east-1:acc:apiId/*/*/*

Reproduction Steps

Create websocket API in CDK

const websocketApi = new apigw.WebSocketApi(stack, 'webocket-api');

call arnForExecuteApi to build arn

new iam.Role(stack, 'test-iam-role', {
  assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
  inlinePolicies: {
    webSocketAccess: new iam.PolicyDocument({
      statements: [
        new iam.PolicyStatement({
          actions: ['execute-api:Invoke'],
          resources: [
            websocketApi.arnForExecuteApi(),
            websocketApi.arnForExecuteApi('connect', 'prod'),
          ],
        }),
      ],
    }),
  },
});

Possible Solution

arnForExecuteApi is currently written with 3 arguments as inputs, and the /* values are filled in when using no arguments. The arguments are method, path and stage. But these arguments are keywords used in REST API and are not appropriate for Websockets which are fundamentally different. In a websocket API there are stages and Routes. So the function needs to be edited to only use these 2 args, in which case using it with no args will default to 2 x /* values tagged onto the arn.

Additional Information/Context

No response

CDK CLI Version

Any / Latest

Framework Version

No response

Node.js Version

v18.18.0

OS

macOS Sequoia (15.2)

Language

TypeScript

Language Version

5.6.3

Other information

More in depth on the issue here #33100 (wrote the PR first)

@IkeNefcy IkeNefcy added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 28, 2025
@github-actions github-actions bot added the @aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 label Jan 28, 2025
@pahud pahud self-assigned this Jan 29, 2025
@pahud
Copy link
Contributor

pahud commented Jan 29, 2025

Hi,

This makes sense to me and thank you for your detailed feedback.

When using an IAM authorizer on the websocket API, having too many wildcards like this causes the IAM Authorizer to deny the iam role from calling the API

Are you able to share the full error messages? I checked the document and found this
https://docs.aws.amazon.com/securityhub/latest/userguide/iam-controls.html

And I agree this should be avoided. Thank you for your PR.

@pahud pahud added p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jan 29, 2025
@pahud pahud removed their assignment Jan 29, 2025
@pahud pahud added the effort/medium Medium work item – several days of effort label Jan 29, 2025
@IkeNefcy
Copy link
Contributor Author

It's simply an access denied, I'll get the API-G log

@IkeNefcy
Copy link
Contributor Author

    "error": {
        "message": "User: arn:aws:sts::acc:assumed-role/role_assumed_via_cognito_id_pool/CognitoIdentityCredentials is not authorized to perform: execute-api:Invoke on resource: arn:aws:execute-api:us-east-1:acc:apiId/prod/$connect",
        "messageString": ""
        User: too many details for gh,
        "responseType": "ACCESS_DENIED",
        "validationErrorString": "-"
    },

@IkeNefcy
Copy link
Contributor Author

IkeNefcy commented Jan 29, 2025

If I build my websocket app with vite and test connecting, the console outputs that it cannot connect.
If I simply delete the 2 characters in iam /* so there are only 2 wild cards, it works right away, too many details in the success API-G log to share, but the key is that it shows "status": "200", after.
I have also verified this with a cloud support engineer, mentioned in the PR (I wrote the PR first so it's more detailed)

@IkeNefcy
Copy link
Contributor Author

I scanned the document you sent and oddly I don't see any mention of this behavior, perhaps something that could be added / clarified. The closest match is
[IAM.21] IAM customer managed policies that you create should not allow wildcard actions for services
But this is only for wildcard actions, which in this case I do not have, I'm only using the action
'execute-api:Invoke', and nothing else. It's the wildcards in the resource arn that is causing the no auth via authorizer.

@pahud
Copy link
Contributor

pahud commented Jan 29, 2025

The log is very helpful. I'll bring this up to the team for visibility. Please feel free to continue your PR. The team would review your PR when it's ready.

@gracelu0 gracelu0 self-assigned this Jan 29, 2025
@mergify mergify bot closed this as completed in #33100 Jan 31, 2025
mergify bot pushed a commit that referenced this issue Jan 31, 2025
…33100)

fixes: #33218

### Reason for this change

In websocket APIs in `aws-apigatewayv2`, the function arnForExecuteApi has essentially the same exact functionality as a REST API, which is not appropriate for Websockets which are fundamentally different. 

The way I found this issue was I used arnForExecuteApi to put the arn of the api into an IAM Role. The reason for this was because I was trying to use an IAM authorizer, which from a React standpoint meant signing iam credentials from my Cognito id pool using Amplify lib. When doing this I used arnForExecuteApi from CDK to write the policy, I did not include any arguments, just the default. 

The issue was that this was not working. I spent time diving deep on the issue in case it was the method in which I was signing the credentials, since I was not too familiar with this process. I also got the assistance of a Cloud Support Engineer from AWS to try and identify the problem. 

Shout-out Mike Sacks.

The problem ended up being that that the resource policy was not correct. The policy that was generated by the function arnForExecuteApi was 

```
"Resource": "arn:aws:execute-api:us-east-1:acc:apiId/*/*/*",
```

This is because the function itself has 3 values, stage, method and path, so when all are left in default states, this indicates `all` or `*`. So when adding each value at default you get `/*/*/*`, 3 x /*. 

This is an issue because Websocket arns are not structured like this, and as it turns out **iam prevents access if you have too many wild cards than applicable**. This means the reason for getting access denied was not because of my signed url, but because having 1 extra /* means that you no longer have permissions. 

Websocket arns are structured like this 

```
arn:aws:execute-api:us-east-1:acc:apiId/*/$connect
```

In this example, * is the stage (this is what it shows on the console) and $connect is the `route`. 
You can add as many routes as you want, but the main ones by default are $connect, $disconnect and $default for no matching route. So if I want to grant an IAM role to have access to all routes and all stages, I would use this:

```
"Resource": "arn:aws:execute-api:us-east-1:acc:apiId/*/*",
```

Note 2 x /* instead of 3. 
Simply changing this by hand (deleting 2 characters) was enough to get the websocket to begin connecting via my signed url. 

### Description of changes

A re-write of the function for creating the arn. This is implemented as arnForExecuteApiV2, the original function has been changes to include the deprecated tag. This is to avoid making a breaking change since the new function only has 2 args and the original had 3.

```ts
  /**
   * Get the "execute-api" ARN.
   *
   * @default - The default behavior applies when no specific route, or stage is provided.
   * In this case, the ARN will cover all routes, and all stages of this API.
   * Specifically, if 'route' is not specified, it defaults to '*', representing all routes.
   * If 'stage' is not specified, it also defaults to '*', representing all stages.
   */
  public arnForExecuteApiV2(route?: string, stage?: string): string {
    return Stack.of(this).formatArn({
      service: 'execute-api',
      resource: this.apiId,
      arnFormat: ArnFormat.SLASH_RESOURCE_NAME,
      resourceName: `${stage ?? '*'}/${route ?? '*'}`,
    });
  }
```

I removed "Method" and "Path" entirely since these are not even appropriate to use as terms for websockets. I used Route instead.

### Description of how you validated changes

Updated Tests, there were 4 tests before:
* get arnForExecuteApi
  * is now using route, and intentionally uses a route with no `$` to check that the `$` is being added correctly. 
* get arnForExecuteApi with default values
  * is now using route
* get arnForExecuteApi with ANY method (removed)
  * doesn't make any sense here because "ANY" is not a term used with websockets, and method does not exist. Thus, removed this test
* throws when call arnForExecuteApi method with specifing a string that does not start with / for the path argument. (removed)
  * This test is checking for a specific format for path, which is not needed since the route can be anything. Also path does not exist.

This leaves 2 total tests now.

Added a new integ function, `integ.api-grant-invoke.ts` and used --update-on-failed with my personal account to bootstrap new snapshots to match. For this test I included an iam role and 2 arns, one with default settings and one with 
`.arnForExecuteApi('connect', 'prod')`
Intentionally left off the `$` to check that it's being added.

All tests and integ are passing.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

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

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants