search

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.36k stars 3.77k forks source link

Wrong policy action when create an AwsCustomResource #4533

Open jagu-sayan opened 4 years ago

jagu-sayan commented 4 years ago

Hello, When I try to create the AwsCustomResource show as example in the documentation : https://docs.aws.amazon.com/cdk/api/latest/docs/custom-resources-readme.html, I got a cloudformation error.

Reproduction Steps

import { AwsCustomResource } from "@aws-cdk/custom-resources";
const app = new App();
const verifyDomainIdentity = new AwsCustomResource(app, 'VerifyDomainIdentity', {
  onCreate: {
    service: 'SES',
    action: 'verifyDomainIdentity',
    parameters: {
      Domain: 'example.com'
    },
    physicalResourceIdPath: 'VerificationToken' // Use the token returned by the call as physical id
  }
});

Error Log

Failed to create resource. User: arn:aws:sts::***:assumed-role/***-***/***-***-*** is not authorized to perform: ses:VerifyDomainIdentity

Environment

Other

When I look the cloudformation created by cdk, the Action property is wrong. I got `"Action": "email:VerifyDomainIdentity" instead of "Action": "ses:VerifyDomainIdentity"

I think mapping define here is not correct for all services.

Workaround

import { AwsCustomResource } from "@aws-cdk/custom-resources";
const app = new App();
const verifyDomainIdentity = new AwsCustomResource(app, 'VerifyDomainIdentity', {
  onCreate: {
    service: 'SES',
    action: 'verifyDomainIdentity',
    parameters: {
      Domain: 'example.com'
    },
    physicalResourceIdPath: 'VerificationToken' // Use the token returned by the call as physical id
  },
  policyStatements : [
    new PolicyStatement({
      resources : ['*'],
      actions   : ['ses:VerifyDomainIdentity'],
    }),
  ],
});

This is :bug: Bug Report

svenrienstra commented 4 years ago

The problem seems to originate in the SDK, the prefix used comes from this file: https://github.com/aws/aws-sdk-js/blob/master/apis/metadata.json

In there the prefix for SES is defined as email. Not sure what this prefix is used for in the SDK, but it might need fixing in the SDK instead of CDK.

SomayaB commented 4 years ago

Thanks for reporting this! We are looking into it. Someone will update when there is movement.

vipingoel commented 3 years ago

This issue is present for some more APIs of node js sdk.

We are trying to create inventory configuration on s3 using AwsCustomResource with policy statements fromSdkCalls. It needs "s3:PutInventoryConfiguration" action but fromSdkCalls generates "s3:putBucketInventoryConfiguration".

Same is the case for s3:CreateJob (IAM action) vs s3control:CreateJob (sdk method).

I guess this is dumb to just use the sdk method. There should be a sort of mapping between the two or sdk methods should exactly map with the actions.

rrrix commented 3 years ago

Is there an open ticket in the AWS Javascript SDK GitHub repository for this issue? I don't see one linked above.

I just happened to need to Verify both an SES domain and SES Email Identity, so I thought I would use the handy dandy example for AwsCustomResource in the documentation: https://docs.aws.amazon.com/cdk/api/latest/docs/custom-resources-readme.html#examples-1 

const verifyDomainIdentity = new AwsCustomResource(this, 'VerifyDomainIdentity', {
  onCreate: {
    service: 'SES',
    action: 'verifyDomainIdentity',
    parameters: {
      Domain: 'example.com'
    },
    physicalResourceId: PhysicalResourceId.fromResponse('VerificationToken') // Use the token returned by the call as physical id
  },
  policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

new route53.TxtRecord(this, 'SESVerificationRecord', {
  zone,
  recordName: `_amazonses.example.com`,
  values: [verifyDomainIdentity.getResponseField('VerificationToken')]
});

It's funny because the example in the docs for AwsCustomResource is broken (but generally works for almost all other services and actions I've tried).

Turns out the generated SDK metadata file is completely wrong for SES. It generates the following IAM Policy:

  AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E:
    Type: AWS::IAM::Policy
    Properties:
      PolicyDocument:
        Statement:
          - Action: email:VerifyDomainIdentity
            Effect: Allow
            Resource: "*"
          - Action: email:VerifyEmailIdentity
            Effect: Allow
            Resource: "*"
          - Action: email:DeleteIdentity
            Effect: Allow
            Resource: "*"
        Version: "2012-10-17"
      PolicyName: AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E
      Roles:
        - Ref: AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2

That should obviously be ses:VerifyDomainIdentity, ses:VerifyEmailIdentity and ses:DeleteIdentity ("email" is not an AWS Service 😄 )

If the AWS JS SDK is going to continue to be wrong, and it's too difficult to get the appropriate fixes applied to the AWS JS SDK (which admittedly is a total beast and has a zillion moving parts), then perhaps the AWS CDK needs some sort of "custom override" mapping for known issues (like SES, and @vipingoel's example)?

Also, this is probably something which should have more tests for scenarios like this.

rrrix commented 3 years ago

I figured it out. Houston, we have a problem. 🚀 💥

The referenced metadata.json file from the AWS SDK isn't supposed to be used for IAM policies. It's used for AWS SDK API Client code generation, AWS API Model file references, and the distribution (build-time) processes.

Firstly, it's important to know exactly where in the AWS JS SDK this file is used:

aws-sdk-js/dist-tools/service-collector.js

apis/metadata.json is used by the aws-sdk-js build tools when generating the release versions of the SDK.

The apis/metadata.json file is not used by the aws-sdk-js itself during runtime.

Lets use SES as an example, since that's why I'm going down this rabbit hole.

The sdk-api-metadata.json (metadata.json) file from the AWS JS SDK has this for SES (Simple Email Service):

  "ses": {
    "prefix": "email",
    "name": "SES",
    "cors": true
  },

Lets break this down, line by line:

  1. Line 1. The key: "ses": {
    1. This is basically basically a standardized ServiceName.toLowerCase() that can then be used to reference the aws-sdk-js/clients/ses.js file, used later for minification to aws-sdk-js/dist/aws-sdk.js.
  2. Line 2, The prefix: "prefix": "email",
    1. Firstly, "prefix" is actually meant as "HTTP API endpoint prefix".
    2. This is the DNS Subdomain that the client uses to actually invoke the HTTP API's.
    3. For an example, see the SES Model file: aws-sdk-js/apis/email-2010-12-01.normal.json:
{
  "version": "2.0",
  "metadata": {
    "apiVersion": "2010-12-01",
    "endpointPrefix": "email",
    "protocol": "query",
    "serviceAbbreviation": "Amazon SES",
    "serviceFullName": "Amazon Simple Email Service",
    "serviceId": "SES",
    "signatureVersion": "v4",
    "signingName": "ses",
    "uid": "email-2010-12-01",
    "xmlNamespace": "http://ses.amazonaws.com/doc/2010-12-01/"
  },

This is most obvious when looking at a raw HTTP GET / POST example, from the SES Documentation:

POST / HTTP/1.1
Host: email.us-west-2.amazonaws.com
Content-Type: application/x-www-form-urlencoded
Date: Tue, 25 May 2010 21:20:27 +0000
Content-Length: 174

Action=SendRawEmail
&Destinations.member.1=allan%40example.com
&RawMessage.Data=RnJvbTp1c2VyQGV4YW1wbGUuY29tDQpTdWJqZWN0OiBUZXN0DQoNCk1lc3 ...

⚠️ Note the email prefix in Host: email.us-west-2.amazonaws.com HTTP Header.

  1. Line 3, The Name. "name": "SES",. This is obvious :)
  2. Line 4, "cors": true
    1. This is what initially gave me a hint that this file is not for IAM 😄 IAM Doesn't care about CORS. HTTP API's do, however.

Unfortunately, I don't have a magic recommendation, fix or obvious alternative to suggest. But one thing is for certain - this metadata never going to change in the AWS JavaScript SDK into a form that the AwsCustomResource logic needs (which is a ServiceName to IAM Action mapping).

rrrix commented 3 years ago

As a refresher, here's the problem function:

/**
 * Transform SDK service/action to IAM action using metadata from aws-sdk module.
 * Example: CloudWatchLogs with putRetentionPolicy => logs:PutRetentionPolicy
 *
 * TODO: is this mapping correct for all services?
 */
function awsSdkToIamAction(service: string, action: string): string {
  const srv = service.toLowerCase();
  const iamService = (awsSdkMetadata[srv] && awsSdkMetadata[srv].prefix) || srv;
  const iamAction = action.charAt(0).toUpperCase() + action.slice(1);
  return `${iamService}:${iamAction}`;
}

Side-note: that TODO is overdue.

I loaded the metadata file into a node REPL and produced a side-by-side diff of the Metadata Keys (awsSdkMetadata[srv]) and the associated prefix (awsSdkMetadata[srv].prefix):

In practical terms, this means that if the prefix exists (the value on the right) the awsSdkToIamAction() function will use that value instead of the lower-case service name on the left.

| **Metadata Keys**              |  **Metadata Prefixes**         |
|-----------------------------------------------------------------|
| acmpca                          |  acm-pca                      |
| applicationautoscaling          |  application-autoscaling      |
| applicationinsights             |  application-insights         |
| augmentedairuntime              |  sagemaker-a2i-runtime        |
| autoscalingplans                |  autoscaling-plans            |
| cloudwatch                      |  monitoring                   |
| cloudwatchevents                |  events                       |
| cloudwatchlogs                  |  logs                         |
| codegurureviewer                |  codeguru-reviewer            |
| codestarconnections             |  codestar-connections         |
| codestarnotifications           |  codestar-notifications       |
| cognitoidentity                 |  cognito-identity             |
| cognitoidentityserviceprovider  |  cognito-idp                  |
| cognitosync                     |  cognito-sync                 |
| computeoptimizer                |  compute-optimizer            |
| configservice                   |  config                       |
| costexplorer                    |  ce                           |
| directoryservice                |  ds                           |
| dynamodbstreams                 |  streams.dynamodb             |
| ec2instanceconnect              |  ec2-instance-connect         |
| efs                             |  elasticfilesystem            |
| elasticinference                |  elastic-inference            |
| elb                             |  elasticloadbalancing         |
| elbv2                           |  elasticloadbalancingv2       |
| emr                             |  elasticmapreduce             |
| forecastqueryservice            |  forecastquery                |
| forecastservice                 |  forecast                     |
| iot1clickdevicesservice         |  iot1click-devices            |
| iot1clickprojects               |  iot1click-projects           |
| iotdata                         |  iot-data                     |
| ioteventsdata                   |  iotevents-data               |
| iotjobsdataplane                |  iot-jobs-data                |
| kinesisvideoarchivedmedia       |  kinesis-video-archived-media |
| kinesisvideomedia               |  kinesis-video-media          |
| kinesisvideosignalingchannels   |  kinesis-video-signaling      |
| lexmodelbuildingservice         |  lex-models                   |
| lexruntime                      |  runtime.lex                  |
| licensemanager                  |  license-manager              |
| marketplacecatalog              |  marketplace-catalog          |
| marketplaceentitlementservice   |  entitlement.marketplace      |
| marketplacemetering             |  meteringmarketplace          |
| mediapackagevod                 |  mediapackage-vod             |
| mediastoredata                  |  mediastore-data              |
| migrationhub                    |  AWSMigrationHub              |
| migrationhubconfig              |  migrationhub-config          |
| mturk                           |  mturk-requester              |
| personalizeevents               |  personalize-events           |
| personalizeruntime              |  personalize-runtime          |
| pinpointemail                   |  pinpoint-email               |
| pinpointsmsvoice                |  sms-voice                    |
| qldbsession                     |  qldb-session                 |
| rdsdataservice                  |  rds-data                     |
| resourcegroups                  |  resource-groups              |
| sagemakerruntime                |  runtime.sagemaker            |
| serverlessapplicationrepository |  serverlessrepo               |
| servicequotas                   |  service-quotas               |
| ses                             |  email                        |
| simpledb                        |  sdb                          |
| ssooidc                         |  sso-oidc                     |
| stepfunctions                   |  states                       |
| transcribeservice               |  transcribe                   |
| wafregional                     |  waf-regional                 |

62 lines.

Here's the node.js script used to produce the above output:

// metadata-keys-prefixes.js
const {execSync} = require('child_process');
const {writeFileSync} = require('fs');

const metadata = require('aws-sdk/apis/metadata.json');
const keys = Object.keys(metadata).sort();
const prefixKeys = keys.map(k => metadata[k].prefix ?? k);

writeFileSync('./metadata-keys.txt', keys.join('\n'), {encoding: 'utf8'});
writeFileSync('./metadata-prefixes.txt', prefixKeys.join('\n'), {encoding: 'utf8'});

try {
  execSync('diff metadata-keys.txt metadata-prefixes.txt -dty -W 80 --suppress-common-lines > metadata-keys-prefixes.diff', {encoding: 'utf8'});
} catch (error) {
  // diff will exit 1 which makes execSync throw a fit
}
rrrix commented 3 years ago

Perhaps we can use the JavaScript-encoded IAM Policy reference from the IAM Policy Generator instead or in combination to the AWS JS SDK API Metadata?

https://awspolicygen.s3.amazonaws.com/js/policies.js 

    'Amazon SES': {
      'StringPrefix': 'ses',
      'Actions': ['CloneReceiptRuleSet', 'CreateConfigurationSet', 'CreateConfigurationSetEventDestination', 'CreateConfigurationSetTrackingOptions', 'CreateCustomVerificationEmailTemplate', 'CreateReceiptFilter', 'CreateReceiptRule', 'CreateReceiptRuleSet', 'CreateTemplate', 'DeleteConfigurationSet', 'DeleteConfigurationSetEventDestination', 'DeleteConfigurationSetTrackingOptions', 'DeleteCustomVerificationEmailTemplate', 'DeleteIdentity', 'DeleteIdentityPolicy', 'DeleteReceiptFilter', 'DeleteReceiptRule', 'DeleteReceiptRuleSet', 'DeleteTemplate', 'DeleteVerifiedEmailAddress', 'DescribeActiveReceiptRuleSet', 'DescribeConfigurationSet', 'DescribeReceiptRule', 'DescribeReceiptRuleSet', 'GetAccountSendingEnabled', 'GetCustomVerificationEmailTemplate', 'GetIdentityDkimAttributes', 'GetIdentityMailFromDomainAttributes', 'GetIdentityNotificationAttributes', 'GetIdentityPolicies', 'GetIdentityVerificationAttributes', 'GetSendQuota', 'GetSendStatistics', 'GetTemplate', 'ListConfigurationSets', 'ListCustomVerificationEmailTemplates', 'ListIdentities', 'ListIdentityPolicies', 'ListReceiptFilters', 'ListReceiptRuleSets', 'ListTemplates', 'ListVerifiedEmailAddresses', 'PutIdentityPolicy', 'ReorderReceiptRuleSet', 'SendBounce', 'SendBulkTemplatedEmail', 'SendCustomVerificationEmail', 'SendEmail', 'SendRawEmail', 'SendTemplatedEmail', 'SetActiveReceiptRuleSet', 'SetIdentityDkimEnabled', 'SetIdentityFeedbackForwardingEnabled', 'SetIdentityHeadersInNotificationsEnabled', 'SetIdentityMailFromDomain', 'SetIdentityNotificationTopic', 'SetReceiptRulePosition', 'TestRenderTemplate', 'UpdateAccountSendingEnabled', 'UpdateConfigurationSetEventDestination', 'UpdateConfigurationSetReputationMetricsEnabled', 'UpdateConfigurationSetSendingEnabled', 'UpdateConfigurationSetTrackingOptions', 'UpdateCustomVerificationEmailTemplate', 'UpdateReceiptRule', 'UpdateTemplate', 'VerifyDomainDkim', 'VerifyDomainIdentity', 'VerifyEmailAddress', 'VerifyEmailIdentity'],
      'ARNFormat': 'arn:aws:ses:<region>:<account_ID>:<arn_type>/<resource_id>',
      'ARNRegex': '^arn:aws:ses:.+:[0-9]+:.+',
      'conditionKeys': ['ses:FeedbackAddress', 'ses:FromAddress', 'ses:FromDisplayName', 'ses:Recipients'],
      'HasResource': true,
    },
    // ... 
    'Amazon S3': {
      'StringPrefix': 's3',
      'Actions': ['AbortMultipartUpload', 'BypassGovernanceRetention', 'CreateAccessPoint', 'CreateBucket', 'CreateJob', 'DeleteAccessPoint', 'DeleteAccessPointPolicy', 'DeleteBucket', 'DeleteBucketPolicy', 'DeleteBucketWebsite', 'DeleteJobTagging', 'DeleteObject', 'DeleteObjectTagging', 'DeleteObjectVersion', 'DeleteObjectVersionTagging', 'DescribeJob', 'GetAccelerateConfiguration', 'GetAccessPoint', 'GetAccessPointPolicy', 'GetAccessPointPolicyStatus', 'GetAccountPublicAccessBlock', 'GetAnalyticsConfiguration', 'GetBucketAcl', 'GetBucketCORS', 'GetBucketLocation', 'GetBucketLogging', 'GetBucketNotification', 'GetBucketObjectLockConfiguration', 'GetBucketPolicy', 'GetBucketPolicyStatus', 'GetBucketPublicAccessBlock', 'GetBucketRequestPayment', 'GetBucketTagging', 'GetBucketVersioning', 'GetBucketWebsite', 'GetEncryptionConfiguration', 'GetInventoryConfiguration', 'GetJobTagging', 'GetLifecycleConfiguration', 'GetMetricsConfiguration', 'GetObject', 'GetObjectAcl', 'GetObjectLegalHold', 'GetObjectRetention', 'GetObjectTagging', 'GetObjectTorrent', 'GetObjectVersion', 'GetObjectVersionAcl', 'GetObjectVersionForReplication', 'GetObjectVersionTagging', 'GetObjectVersionTorrent', 'GetReplicationConfiguration', 'HeadBucket', 'ListAccessPoints', 'ListAllMyBuckets', 'ListBucket', 'ListBucketMultipartUploads', 'ListBucketVersions', 'ListJobs', 'ListMultipartUploadParts', 'ObjectOwnerOverrideToBucketOwner', 'PutAccelerateConfiguration', 'PutAccessPointPolicy', 'PutAccountPublicAccessBlock', 'PutAnalyticsConfiguration', 'PutBucketAcl', 'PutBucketCORS', 'PutBucketLogging', 'PutBucketNotification', 'PutBucketObjectLockConfiguration', 'PutBucketPolicy', 'PutBucketPublicAccessBlock', 'PutBucketRequestPayment', 'PutBucketTagging', 'PutBucketVersioning', 'PutBucketWebsite', 'PutEncryptionConfiguration', 'PutInventoryConfiguration', 'PutJobTagging', 'PutLifecycleConfiguration', 'PutMetricsConfiguration', 'PutObject', 'PutObjectAcl', 'PutObjectLegalHold', 'PutObjectRetention', 'PutObjectTagging', 'PutObjectVersionAcl', 'PutObjectVersionTagging', 'PutReplicationConfiguration', 'ReplicateDelete', 'ReplicateObject', 'ReplicateTags', 'RestoreObject', 'UpdateJobPriority', 'UpdateJobStatus'],
      'ARNFormat': 'arn:aws:s3:::<bucket_name>/<key_name>',
      'ARNRegex': '^arn:aws:s3:::.+',
      'conditionKeys': ['aws:RequestTag/${TagKey}', 'aws:ResourceTag/${TagKey}', 'aws:TagKeys', 's3:AccessPointNetworkOrigin', 's3:DataAccessPointAccount', 's3:DataAccessPointArn', 's3:ExistingJobOperation', 's3:ExistingJobPriority', 's3:ExistingObjectTag/<key>', 's3:JobSuspendedCause', 's3:LocationConstraint', 's3:RequestJobOperation', 's3:RequestJobPriority', 's3:RequestObjectTag/<key>', 's3:RequestObjectTagKeys', 's3:VersionId', 's3:authType', 's3:delimiter', 's3:locationconstraint', 's3:max-keys', 's3:object-lock-legal-hold', 's3:object-lock-mode', 's3:object-lock-remaining-retention-days', 's3:object-lock-retain-until-date', 's3:prefix', 's3:signatureAge', 's3:signatureversion', 's3:versionid', 's3:x-amz-acl', 's3:x-amz-content-sha256', 's3:x-amz-copy-source', 's3:x-amz-grant-full-control', 's3:x-amz-grant-read', 's3:x-amz-grant-read-acp', 's3:x-amz-grant-write', 's3:x-amz-grant-write-acp', 's3:x-amz-metadata-directive', 's3:x-amz-server-side-encryption', 's3:x-amz-server-side-encryption-aws-kms-key-id', 's3:x-amz-storage-class', 's3:x-amz-website-redirect-location'],
      'HasResource': true,
    },

Un-minified: https://github.com/rrrix/aws-iam-reference/blob/master/var/policies.js

I found a little tool that extracts every IAM Action and puts it into a text file: https://github.com/rrrix/aws-iam-reference/blob/master/all-actions.txt

Forked from https://github.com/rvedotrc/aws-iam-reference (which uses an outdated js file reference from the "old" policygen).

joehillen commented 3 years ago

I just ran into this using the service called "ResourceGroupsTaggingAPI" in Javascript, but the IAM action prefix is "tag". Is there a workaround for this that doesn't allow all actions?

NGL321 commented 2 years ago

Escalating to p1 as this has been pending and blocking for too long

adilosa commented 2 years ago

Summary: We need API Prefix to tell CloudFormation what API to call for the custom resource, and need the IAM prefix to create permissions. These are often not the same value. Further, there is no authoritative mapping between Service Name, API Prefix, and IAM Action Prefix.

(in fact there does not appear to even be a definitive way to list services, or a definitive name for what to call a service)

Proposed Solution: Short of AWS providing a definitive list of their services, I think the only way to handle this will be to maintain a mapping in code somewhere. The policies.json file seems to be the only source for IAM Action prefixes so most likely that will have to be incorporated.

Options:

Source Service Name Service Code HTTP API Prefix IAM Action Prefix Caveats
metadata ✅1 ❌ ✅ ❌ Designed for internal JS SDK code generation, not as a service list
policies.json ✅2 ❌ ❌ ✅ Unclear how publicly supported/documented this is, although people seem to know about it.
SSM ✅3 ✅1 ✅ ❌ Not available statically, requires 2N+1 calls to SSM. May also be regional.
list-services ✅4 ✅2 ❌ ❌ Only services integrated with service-quotas API

Deceptively, the various service names/codes are often subtly different between sources - preventing a trivial mapping. However, there's enough overlap by coincidence that most of the services appear to work.

Examples: Source Service Name Service Code HTTP API Prefix IAM Action Prefix
metadata SES email
policies.json Amazon SES ses
SSM Amazon Simple Email Service (SES) ses email
list-services Amazon Simple Email Service(Amazon SES) ses
Source Service Name Service Code HTTP API Prefix IAM Action Prefix
metadata ResourceGroupsTaggingAPI resourcegroupstaggingap (*this appears to be an error? 'tagging' is correct)
policies.json Amazon Resource Group Tagging API tag
SSM AWS Resource Groups Tagging API resourcegroupstaggingapi tagging
list-services
juliagrigni commented 1 year ago

I'm not sure if this is an issue people are still struggling with, but here's a potential workaround I found (I did not write it) that works for me:

Create a policy with the action as it should be in IAM (ex: 'ses:verifyEmailIdentity'), then attach that policy to your custom resource, instead of having it create the policy using fromSdkCalls.

I'm sorry if this was already common knowledge/unhelpful, but I hope it's helpful to someone as a workaround in the meantime!