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-rds): grantConnect for IAM authentication provides invalid permissions (surface DbiResourceId) #11851

Closed
henripoikonen opened this issue Dec 3, 2020 · 30 comments · Fixed by #25141
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@henripoikonen
Copy link

henripoikonen commented Dec 3, 2020

DatabaseInstance has a method grantConnect for granting connect access to instance using IAM based authentication.

However, the db resource ARN in the produced IAM policy is incorrect and doesn't work. Based on the documentation The format for the resource in the IAM policy should be: arn:aws:rds-db:region:account-id:dbuser:DbiResourceId/db-user-name

The actual resource produced by grantConnect is having format: arn:aws:rds:region:account-id:db:DBInstanceId. Also, the function doesn't provide any parameter to define the db username to be used in the policy.

Reproduction Steps

import { Stack, Construct, StackProps } from '@aws-cdk/core';
import {
  DatabaseInstance,
  DatabaseInstanceEngine,
  PostgresEngineVersion,
  Credentials,
} from '@aws-cdk/aws-rds';

import { IVpc } from '@aws-cdk/aws-ec2';
import { User } from '@aws-cdk/aws-iam';

export interface MyStackProps extends StackProps {
  vpc: IVpc;
}
export class MyStack extends Stack {
  constructor(scope: Construct, id: string, props: MyStackProps) {
    super(scope, id, props);

    const db = new DatabaseInstance(this, 'Instance', {
      engine: DatabaseInstanceEngine.postgres({ version: PostgresEngineVersion.VER_12_4 }),
      credentials: Credentials.fromGeneratedSecret('testuser'),
      vpc: props.vpc,
      port: 5432,
      iamAuthentication: true,
    });

    const user = new User(this, 'TestUser', {
      userName: 'testuser',
    });

    db.grantConnect(user);
  }
}

What did you expect to happen?

To create a IAM policy where the resource ARN would be according to the documentation i.e. arn:aws:rds-db:region:account-id:dbuser:DbiResourceId/db-user-name

What actually happened?

Instead of the correct policy, the generated template contains following definition:

           {
              "Action": "rds-db:connect",
              "Effect": "Allow",
              "Resource": {
                "Fn::Join": [
                  "",
                  [
                    "arn:",
                    {
                      "Ref": "AWS::Partition"
                    },
                    ":rds:",
                    {
                      "Ref": "AWS::Region"
                    },
                    ":",
                    {
                      "Ref": "AWS::AccountId"
                    },
                    ":db:",
                    {
                      "Ref": "InstanceC1063A87"
                    }
                  ]
                ]
              }
            }

In addition that the format of the ARN is incorrect, also wrong DB identifier is used. The template uses the DB Instance id but the correct identifier is the DB Resource id.

Environment

  • CDK CLI Version : 1.75.0
  • Framework Version: 1.75.0
  • Node.js Version: 12.18.3
  • OS : OSx
  • Language (Version): Typescript 4.1.2

Other

The support for the grantConnect was requested in this issue and added in this PR.

A comment in the original issue still stands i.e. that the DB Resource Id is not accessible in Cloudformation.


This is 🐛 Bug Report

@henripoikonen henripoikonen added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 3, 2020
@henripoikonen henripoikonen changed the title (awards): grantConnect for IAM authentication provides invalid permissions (aws-rds): grantConnect for IAM authentication provides invalid permissions Dec 3, 2020
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Dec 3, 2020
@skinny85 skinny85 added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Dec 3, 2020
@skinny85
Copy link
Contributor

skinny85 commented Dec 3, 2020

Thanks for reporting @henripoikonen .

@henripoikonen
Copy link
Author

Thanks for reporting @henripoikonen .

No problem! I would assume that this enhancement would need to be first implemented to Clouformation before a proper fix can be made?

@skinny85
Copy link
Contributor

skinny85 commented Dec 4, 2020

Thanks for reporting @henripoikonen .

No problem! I would assume that this enhancement would need to be first implemented to Clouformation before a proper fix can be made?

Yep, I think so - I don't see how we could construct the correct database instance resource ID without it.

@skinny85 skinny85 added the needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. label Dec 4, 2020
@jdvornek
Copy link
Contributor

jdvornek commented Jan 6, 2021

Seems like a similar issue exists for DatabaseProxy. Do you want a separate issue for that or is it easier to track it all here?

I was able to work around the invalid permissions in the current grantConnect() for DatabaseProxy with the following. Thought I'd leave it here in case it might be helpful to someone else.

function fixedGrantConnect(rds_proxy, grantee, db_user) {
  let parsed_arn = Arn.parse(rds_proxy.dbProxyArn);
  parsed_arn.service = 'rds-db';
  parsed_arn.resource =
    Fn.join(':',
            [
              'dbuser',
              Fn.select(6, Fn.split(':', rds_proxy.dbProxyArn))
            ]);
  parsed_arn.resourceName = db_user;
  return Grant.addToPrincipal({
                                grantee,
                                actions:      ['rds-db:connect'],
                                resourceArns: [Arn.format(parsed_arn, Stack.of(rds_proxy))]
                              });
}

@skinny85
Copy link
Contributor

skinny85 commented Jan 6, 2021

@jdvornek is this a problem with the code we have from #12243 ?

@jdvornek
Copy link
Contributor

jdvornek commented Jan 6, 2021

@skinny85 that's right. In grantConnect the Grant resource is created referencing the proxy ARN, but I believe it should be something like arn:aws:rds-db:us-east-2:1234567890:dbuser:prx-ABCDEFGHIJKL01234/db_user per the documentation.

@skinny85
Copy link
Contributor

skinny85 commented Jan 6, 2021

Damn. Any chance of a PR fixing this, using the code from your workaround? 🙂 https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md

@jdvornek
Copy link
Contributor

jdvornek commented Jan 8, 2021

@skinny85 #12416 is in for the DatabaseProxy fix. To keep things separate I opened a new issue for that as the solution has no bearing on grantConnect for DatabaseInstance or DatabaseCluster.

@skinny85
Copy link
Contributor

skinny85 commented Jan 8, 2021

@jdvornek thanks, that's awesome!

@underbluewaters
Copy link

I'm not using the database proxy, but having problems granting permissions to directly access an RDS instance.

This:

    const role = new iam.Role(this, "MaintenanceRole", {
      assumedBy: new ServicePrincipal("ecs-tasks.amazonaws.com"),
    });
    asset.repository.grantPull(role);
    props.db.grantConnect(role);

    const taskDefinition = new ecs.FargateTaskDefinition(
      this,
      "SeaSketchMaintenanceFargateTaskDef",
      {
        cpu: 256,
        executionRole: role,
        memoryLimitMiB: 512,
      }
    );

Yields something like this, which appears useless:

    {
        "Effect": "Allow",
        "Action": [
            "rds-db:connect",
        ],
        "Resource": [
            "arn:aws:rds:us-west-2:123430260133:db:si1b6m5gggg9ypx"
        ]
    },

@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Mar 20, 2021
@skinny85 skinny85 removed the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Mar 20, 2021
@skinny85
Copy link
Contributor

Yes @underbluewaters . The problem is that we're lacking a feature in CloudFormation to get the Database instance resource ID, which is needed to correctly create this policy, as has been said in #11851 (comment).

@Plasma
Copy link
Contributor

Plasma commented Apr 13, 2021

Just hit this issue myself, looking forward to a fix, thank you!

@mjgp2
Copy link

mjgp2 commented May 6, 2021

Is it possible for the CDK team to raise this with the CF team? I mean this is really broken.

@jdvornek
Copy link
Contributor

jdvornek commented May 6, 2021

I believe the ResourceID is available from the cli, which should mean that you can use a custom resource to get the required fields to generate the policy.

https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/UsingWithRDS.IAMDBAuth.IAMPolicy.html

@jdvornek
Copy link
Contributor

jdvornek commented May 7, 2021

This works for an Aurora cluster, I believe. Obviously it needs to be extended for instances, etc, but maybe this will get someone unblocked and it seems easy enough to swap out once a proper fix is available.

const someCluster = new rds.DatabaseCluster();
const someDatabaseUser = 'test';
const someRole = new iam.Role();
const dbResourceId = new custom.AwsCustomResource(this, 'MembershipDBResourceID', {
  onCreate: {
    service: 'RDS',
    action: 'describeDBClusters',
    parameters: {
      DBClusterIdentifier: someCluster.clusterIdentifier
    },
    physicalResourceId: custom.PhysicalResourceId.fromResponse('DBClusters.0.DbClusterResourceId')
  },
  policy: custom.AwsCustomResourcePolicy.fromSdkCalls({
    resources: custom.AwsCustomResourcePolicy.ANY_RESOURCE
  })
});
const scopeStack = cdk.Stack.of(this);
const resourceId = dbResourceId.getResponseField('DBClusters.0.DbClusterResourceId');
const userArn = scopeStack.formatArn({
                                       service: 'rds-db',
                                       resource: 'dbuser',
                                       resourceName: `${resourceId}/${someDatabaseUser}`,
                                       sep: ':',
                                     });
iam.Grant.addToPrincipal({
  grantee: someRole,
  actions: ["rds-db:connect"],
  resourceArns: [userArn]
});

That yields

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Action": "rds-db:connect",
            "Resource": "arn:aws:rds-db:<region>:<account>:dbuser:cluster-0123456789/test",
            "Effect": "Allow"
        }
    ]
}

@alex9311
Copy link
Contributor

alex9311 commented Aug 18, 2021

thanks for the workarounds @jdvornek and @cloventt, lifesavers. Confirmed these work for postgres db instances.

function grantIamAuth(
  scope: cdk.Stack,
  db: rds.DatabaseInstance,
  grantPrincipal: iam.IPrincipal
) {
  const context = cdke._98point6Stack.getContext(scope)

  // https://github.com/aws/aws-cdk/issues/11851
  const dbResourceId = new custom.AwsCustomResource(
    scope,
    `PostgresDBResourceIdCR-${grantPrincipal}`,
    {
      onCreate: {
        service: 'RDS',
        action: 'describeDBInstances',
        parameters: {
          DBInstanceIdentifier: db.instanceIdentifier,
        },
        physicalResourceId: custom.PhysicalResourceId.fromResponse(
          'DBInstances.0.DbiResourceId'
        ),
        outputPath: 'DBInstances.0.DbiResourceId',
      },
      policy: custom.AwsCustomResourcePolicy.fromSdkCalls({
        resources: custom.AwsCustomResourcePolicy.ANY_RESOURCE,
      }),
    }
  )
  const resourceId = dbResourceId.getResponseField(
    'DBInstances.0.DbiResourceId'
  )

  const dbUserArn = `arn:aws:rds-db:${my_region}:${my_account_num}:dbuser:${resourceId}/${my_iam_user}`
  iam.Grant.addToPrincipal({
    grantee: grantPrincipal,
    actions: ['rds-db:connect'],
    resourceArns: [dbUserArn],
  })
}

@skinny85 skinny85 changed the title (aws-rds): grantConnect for IAM authentication provides invalid permissions (aws-rds): grantConnect for IAM authentication provides invalid permissions (surface DbiResourceId) Dec 10, 2021
@skinny85 skinny85 removed their assignment Dec 15, 2021
mergify bot pushed a commit that referenced this issue Mar 8, 2022
Add notes in the docs that `grantConnect()` does not work currently.

Reference: #11851

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TheRealAmazonKendra pushed a commit to TheRealAmazonKendra/aws-cdk that referenced this issue Mar 11, 2022
Add notes in the docs that `grantConnect()` does not work currently.

Reference: aws#11851

----

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

We were bitten by a hidden issue when updating CDK. Because we had not specified an onUpdate handler, we received an error that the DbiResourceId did not exist:

CustomResource attribute error: Vendor response doesn't contain DBInstances.0.DbiResourceId key in object

To fix this we duplicated the code to add an onUpdate handler:

    new customresource.AwsCustomResource(this, 'RdsInstanceResourceId', {
      onCreate: {
        service: 'RDS',
        action: 'describeDBInstances',
        parameters: {
          DBInstanceIdentifier: this.rdsInstance.instanceIdentifier,
        },
        physicalResourceId: customresource.PhysicalResourceId.fromResponse('DBInstances.0.DbiResourceId'),
        outputPath: 'DBInstances.0.DbiResourceId',
      },
      onUpdate: {
        service: 'RDS',
        action: 'describeDBInstances',
        parameters: {
          DBInstanceIdentifier: this.rdsInstance.instanceIdentifier,
        },
        physicalResourceId: customresource.PhysicalResourceId.fromResponse('DBInstances.0.DbiResourceId'),
        outputPath: 'DBInstances.0.DbiResourceId',
      },
      policy: customresource.AwsCustomResourcePolicy.fromSdkCalls({
        resources: customresource.AwsCustomResourcePolicy.ANY_RESOURCE,
      }),
    });

@rittneje
Copy link

@cloventt The docs for AwsCustomResource say that onCreate will call onUpdate by default. So probably you only need to specify onUpdate. https://docs.aws.amazon.com/cdk/api/v1/docs/@aws-cdk_custom-resources.AwsCustomResourceProps.html#oncreate

@skinny85
Copy link
Contributor

Would someone be interested in contributing their Custom Resource that does this back to the CDK Construct Library for RDS?

@alex9311
Copy link
Contributor

@skinny85 I would be interested. Is there a good example I can use to follow? Maybe #12416 ?

@skinny85
Copy link
Contributor

@alex9311 yes, #12416 should be helpful, as well as the code in this comment of this issue: #11851 (comment).

@mkhraisha
Copy link

is this fixed by #12416?

if so can we update the docs to reflect that the grantConnect now functions correctly?

@skinny85
Copy link
Contributor

is this fixed by #12416?

if so can we update the docs to reflect that the grantConnect now functions correctly?

No. #12416 is about grantConnect() in the Proxy resource, while this issue is about grantConnect() in the DatabaseInstance resource.

@sander-su
Copy link

sander-su commented Nov 18, 2022

As aws-cloudformation/cloudformation-coverage-roadmap#105 has been released some days ago does this means we can expect a fix soon? @skinny85

@comcalvi comcalvi removed the needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. label Jan 27, 2023
@ryanchamp-ICE
Copy link

So it looks like skinny85 doesn't work for Amazon any more, who would be the person to give an update on this particular issue?

@drankard
Copy link

Is this still stalling ?

@mergify mergify bot closed this as completed in #25141 Apr 20, 2023
mergify bot pushed a commit that referenced this issue Apr 20, 2023
The [IAM policy for IAM database access](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/UsingWithRDS.IAMDBAuth.IAMPolicy.html) takes the form of:

```
arn:aws:rds-db:region:account-id:dbuser:DbiResourceId/db-user-name
```

Following aws-cloudformation/cloudformation-coverage-roadmap#105, this change updates the ARN used in `grantConnect` to this format.

Additionally, update the signature of `grantConnect` to take an optional `dbUser`, which is defaulted to the master username of the database, obtained via the available `Secret`.

This signature change also matches `grantConnect` in `DatabaseProxy`. See #12416.

Closes #11851.

----

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

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Jimlinz added a commit to linz/bde-fdw-rds that referenced this issue Apr 21, 2023
RDS resource id is needed to allow iam login. This value can't be called directly due to a known AWS bug. A suggested workaround is to call custom resource aws/aws-cdk#11851
Jimlinz added a commit to linz/bde-fdw-rds that referenced this issue Apr 21, 2023
RDS resource id is needed to allow iam login. This value can't be called directly due to a known AWS bug. A suggested workaround is to call custom resource aws/aws-cdk#11851
kodiakhq bot pushed a commit to linz/bde-fdw-rds that referenced this issue Apr 25, 2023
* chore: Minor tidy ups

- Added security group in context. These are automatically generated during cdk synth or cdk deploy and will generally speed things up.
- Rename rds_init lambda function to rds_init_script
- Increase rds storage from 10GB to 100GB, scalable up to 300GB
- Remove unused Class
- Deflate zip package

* feat: Ensure cdk can be updated

The rds_init_script was set up to only run once during cdk deploy. This becomes a problem during stack update (i.e. subsequent cdk deploy creates cloudformation changeset) because some schemas already exist in the fdw rds. This commit ensures schemas are deleted (if exists) so they can be created again during an update.

* chore: Rebase branch onto Master

Resolve merge conflict resulting from dependabot updates.

* fix: Remove use_remote_estimate

Tests seem to show worse performance with this enabled.

* fix: Remove iam group, add tailored iam user policy

Although IAM group is a tidy way of assigning permission to users needing similar access, it can be a security flaw in our application. IAM policy needs to be specific to individual user to prevent them from connecting to the rds instance as another user; specifically, it stops users from calling `generate-db-auth-token` of another known user and use their token to connect to the bde analytics instance.

* fix: use the "rds-db" prefix and resource id

* fix: Type checking for AWS secrets

As per example (https://awslabs.github.io/aws-lambda-powertools-python/2.13.0/utilities/parameters/#fetching-secrets) typing for get_secrets should be set to Any.

* fix: Remove todo list and fix formatting

* feat: Get AWS account ID from boto3

Avoid hard coding AWS account ID in resource_arn, making stack deployment account agnostic.

* feat: Create schema for each user

The purpose of this stack is to allow user to query bde data via fdw (remote to avoid tempering with production db). For this to be useful, user should be able to run some queries and save their changes locally. This commit creates a schema for each user upon user / role creation. It provides user with liberal privileges within their own schema.

* fix: fetch_size sql syntax

This option needs to be `ADD`(ed) first. SET is for altering existing value.

* fix: Parse account id from cdk.json

* fix: Attach managed policy to deploy lambda in vpc

Deploying lambda inside VPC requires permissions outlined in AWSLambdaVPCAccessExecutionRole managed policy.

* fix: Use / attach correct policies

Correct policies needed for lambda to create users. Policy path needs to begin and end with a /

* fix: Custom resource to acquire rds resource id

RDS resource id is needed to allow iam login. This value can't be called directly due to a known AWS bug. A suggested workaround is to call custom resource aws/aws-cdk#11851

* fix: Type checking

---------

Co-authored-by: AmrouEmad <73799995+AmrouEmad@users.noreply.github.com>
@mkhraisha
Copy link

I see this is closed has it been fixed?
if so should update the docs here: grantConnect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.