(aws-rds): Password length is not passed through to rotation application

[ben-eb]

Describe the bug

Hello,

We have an issue with the automatic secret rotation functionality provided by the addRotationMultiUser method.

From a casual glance of the codebase (and the generated CloudFormation template) it would seem that the default behaviour of CDK is to create passwords that are a maximum of 30 characters in length (and this cannot be changed). Indeed, there is a comment explaining the reason:

https://github.com/aws/aws-cdk/blob/7caab7d9690cfcc8a750800686d96d0f7f857fa9/packages/aws-cdk-lib/aws-rds/lib/database-secret.ts#L86

This is relevant to us because we are needing to create a database link from an Oracle database server back to RDS and cannot use more than 30 characters.

However, further automatic updates to this secret (possibly on initial creation also but we have not verified this) will discard this length requirement, resetting it to the default.

We traced this back to https://github.com/aws-samples/aws-secrets-manager-rotation-lambdas/blob/67a98d2745b08bc748d17dc0469e48199c039789/SecretsManagerRDSPostgreSQLRotationMultiUser/lambda_function.py#L128 which does not allow us to pass through a maximum password length (so it defaults to 32 instead).

There is an open issue for tracking this bug at https://github.com/aws-samples/aws-secrets-manager-rotation-lambdas/issues/51.

Expected Behavior

We would expect the length requirement for the default rotation function to be parameterised so that the maximum password length is passed through.

Indeed, this could unblock support for longer passwords than 32 characters.

Current Behavior

The password length is not passed through, so the password is 32 characters instead of 30.

Reproduction Steps

stack.ts

import { Fn, Stack } from "aws-cdk-lib";
import { Construct } from "constructs";
import {
  AuroraPostgresEngineVersion,
  ClusterInstance,
  DatabaseCluster,
  DatabaseClusterEngine,
  DatabaseSecret,
} from "aws-cdk-lib/aws-rds";
import {
  InstanceClass,
  InstanceSize,
  InstanceType,
  Vpc,
} from "aws-cdk-lib/aws-ec2";

export class TestStack extends Stack {
  constructor(scope: Construct, id: string) {
    super(scope, id);

    const vpc = Vpc.fromVpcAttributes(this, "primary-vpc", {
      availabilityZones: ["az1"],
      vpcId: Fn.importValue("TestVpc"),
      privateSubnetIds: ["A", "B"].map((id) =>
        Fn.importValue(`Test-Subnet-${id}`),
      ),
    });

    const instanceProps = {
      instanceType: InstanceType.of(InstanceClass.T3, InstanceSize.MEDIUM),
    };

    const database = new DatabaseCluster(this, "database", {
      engine: DatabaseClusterEngine.auroraPostgres({
        version: AuroraPostgresEngineVersion.VER_11_19,
      }),
      vpc,
      writer: ClusterInstance.provisioned("Instance1", instanceProps),
      readers: [ClusterInstance.provisioned("Instance2", instanceProps)],
    });

    const secret = new DatabaseSecret(this, "secret", {
      username: "test",
      secretName: "testuser",
      excludeCharacters: "{}[]()'\"/\\",
    });

    database.addRotationMultiUser("rotation", {
      secret: secret.attach(database),
    });
  }
}

bin.ts

#!/usr/bin/env node
import * as cdk from "aws-cdk-lib";
import { TestStack } from "./stack";

const app = new cdk.App();

new TestStack(app, "test-stack");

cdk.json

{
  "app": "npx ts-node --prefer-ts-exts bin.ts"
}

Running cdk synth will produce, among other things, a AWS::Serverless::Application resource that does not have the secret length as a parameter.

Possible Solution

The rotation application sample would need updating to pass through the length parameter.

Additional Information/Context

No response

CDK CLI Version

2.102.0 (build 2abc59a)

Framework Version

No response

Node.js Version

v18.18.2

OS

Mac OS X

Language

TypeScript

Language Version

TypeScript 5.2.2

Other information

No response

[msambol]

@peterwoodworth I can take this.

[msambol]

I investigated this. One issue I see is PasswordLength is not supported here in CFN: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-secretsmanager-rotationschedule-hostedrotationlambda.html. I can parameterize the password length on creation but I think it will break during rotation. Waiting for input from the CDK team before I proceed.

[indrora]

The CDK uses the underlying SecretsManager::Secret parameters: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-secretsmanager-secret-generatesecretstring.html (mapped in the CDK as https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_secretsmanager.SecretStringGenerator.html).

I was initially incredulous that content out of aws-samples/ would be used in production like this, but it does appear that this is the case: https://docs.aws.amazon.com/secretsmanager/latest/userguide/reference_available-rotation-templates.html#sar-template-oracle-singleuser is the actual live reference for the lambda. Honestly, this is a massive failure on the part of the CloudFormation/SecretsManager team as this should be in a top level AWS organization (aws-samples is generally provided without any level of support).

This appears to be a problem with the AWS Secrets Manager lambda, not the CDK itself.

[msambol]

Yeah, from what I'm reading here, automatic rotation generates a new password with 32 chars every time. But you can generate a secret string with a custom length. Feels like automatic rotation is missing a feature :)

[indrora]

Since it's a problem on CloudFormation/Secrets Manager, I'm going to close this ticket.

[github-actions[bot]]

⚠️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.

[msambol]

@indrora I agree but is it possible to open a TT to see if the team can update the automatic rotation functionality to include password length as a parameter?

[peterwoodworth]

We can absolutely do that, and should do that for customers when applicable. @indrora can you please follow up internally?

Also I don't think this will be the best move here, but @msambol creating issues in the cloudformation coverage roadmap does automatically create tickets internally (though i'm not sure how to follow up on those on my end)

[msambol]

@peterwoodworth Happy to do so in that repo—let me know if you want to go that route or TT.

[msambol]

@peterwoodworth ^ I created a new issue in the other repo.

[ashishdhingra]

Closing the issue in favor of https://github.com/aws-cloudformation/cloudformation-coverage-roadmap/issues/1824.

[github-actions[bot]]

⚠️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.