feat(rds): change excludeCharacters for Cluster, Instance and Databas…

…eSecret

Change the default excludeCharacters for Cluster,
Instance and DatabaseSecret to the character set " %+~`#$&*()|[]{}:;<>?!'/@\"\\",
as the previous set ('"@/\\')
had a tendency to generate problematic passwords that wouldn't work in the shell,
or with services like DMS.
Do the same for single- and multi-user rotations in Cluster and Instance as well.
Also allow passing a custom excludeCharacters for Credentials and SnapshotCredentials,
and also in addSingleUserRotation and addMultiUserRotation.

Fixes aws#4144

BREAKING CHANGE: the default excludeCharacters set for Instance, Cluster and DatabaseSecret is now " %+~`#$&*()|[]{}:;<>?!'/@\"\\"
* **rds**: the default excludeCharacters for addSingleUserRotation and addMultiUserRotation is now " %+~`#$&*()|[]{}:;<>?!'/@\"\\"
* **rds**: Instance.addSingleUserRotation now takes options as the first argument, instead of just Duration
* **rds**: Cluster.addSingleUserRotation now takes options as the first argument, instead of just Duration

packages/@aws-cdk/aws-rds/README.md

@@ -194,7 +194,10 @@ const address = instance.instanceEndpoint.socketAddress;   // "HOSTNAME:PORT"
 When the master password is generated and stored in AWS Secrets Manager, it can be rotated automatically:
 
 ```ts
-instance.addRotationSingleUser(); // Will rotate automatically after 30 days
+instance.addRotationSingleUser({
+  automaticallyAfter: cdk.Duration.days(7), // defaults to 30 days
+  excludeCharacters: '!@#$%^&*', // defaults to the set " %+~`#$&*()|[]{}:;<>?!'/@\"\\"
+});
 ```
 
 [example of setting up master password rotation for a cluster](test/integ.cluster-rotation.lit.ts)
@@ -213,6 +216,7 @@ It's also possible to create user credentials together with the instance/cluster
 const myUserSecret = new rds.DatabaseSecret(this, 'MyUserSecret', {
   username: 'myuser',
   masterSecret: instance.secret,
+  excludeCharacters: '{}[]()\'"/\\', // defaults to the set " %+~`#$&*()|[]{}:;<>?!'/@\"\\"
 });
 const myUserSecretAttached = myUserSecret.attach(instance); // Adds DB connections information in the secret
 

packages/@aws-cdk/aws-rds/lib/cluster.ts

@@ -10,8 +10,8 @@ import { DatabaseClusterAttributes, IDatabaseCluster } from './cluster-ref';
 import { DatabaseSecret } from './database-secret';
 import { Endpoint } from './endpoint';
 import { IParameterGroup } from './parameter-group';
-import { applyRemovalPolicy, defaultDeletionProtection, setupS3ImportExport } from './private/util';
-import { BackupProps, Credentials, InstanceProps, PerformanceInsightRetention, RotationMultiUserOptions } from './props';
+import { applyRemovalPolicy, DEFAULT_PASSWORD_EXCLUDE_CHARS, defaultDeletionProtection, setupS3ImportExport } from './private/util';
+import { BackupProps, Credentials, InstanceProps, PerformanceInsightRetention, RotationSingleUserOptions, RotationMultiUserOptions } from './props';
 import { DatabaseProxy, DatabaseProxyOptions, ProxyTarget } from './proxy';
 import { CfnDBCluster, CfnDBClusterProps, CfnDBInstance } from './rds.generated';
 import { ISubnetGroup, SubnetGroup } from './subnet-group';
@@ -493,6 +493,7 @@ export class DatabaseCluster extends DatabaseClusterNew {
       credentials = Credentials.fromSecret(new DatabaseSecret(this, 'Secret', {
         username: credentials.username,
         encryptionKey: credentials.encryptionKey,
+        excludeCharacters: credentials.excludeCharacters,
       }));
     }
     const secret = credentials.secret;
@@ -530,11 +531,8 @@ export class DatabaseCluster extends DatabaseClusterNew {
 
   /**
    * Adds the single user rotation of the master password to this cluster.
-   *
-   * @param [automaticallyAfter=Duration.days(30)] Specifies the number of days after the previous rotation
-   * before Secrets Manager triggers the next automatic rotation.
    */
-  public addRotationSingleUser(automaticallyAfter?: Duration): secretsmanager.SecretRotation {
+  public addRotationSingleUser(options: RotationSingleUserOptions = {}): secretsmanager.SecretRotation {
     if (!this.secret) {
       throw new Error('Cannot add single user rotation for a cluster without secret.');
     }
@@ -547,11 +545,12 @@ export class DatabaseCluster extends DatabaseClusterNew {
 
     return new secretsmanager.SecretRotation(this, id, {
       secret: this.secret,
-      automaticallyAfter,
       application: this.singleUserRotationApplication,
       vpc: this.vpc,
       vpcSubnets: this.vpcSubnets,
       target: this,
+      ...options,
+      excludeCharacters: options.excludeCharacters ?? DEFAULT_PASSWORD_EXCLUDE_CHARS,
     });
   }
 
@@ -563,9 +562,9 @@ export class DatabaseCluster extends DatabaseClusterNew {
       throw new Error('Cannot add multi user rotation for a cluster without secret.');
     }
     return new secretsmanager.SecretRotation(this, id, {
-      secret: options.secret,
+      ...options,
+      excludeCharacters: options.excludeCharacters ?? DEFAULT_PASSWORD_EXCLUDE_CHARS,
       masterSecret: this.secret,
-      automaticallyAfter: options.automaticallyAfter,
       application: this.multiUserRotationApplication,
       vpc: this.vpc,
       vpcSubnets: this.vpcSubnets,

packages/@aws-cdk/aws-rds/lib/database-secret.ts

@@ -1,6 +1,7 @@
 import * as kms from '@aws-cdk/aws-kms';
 import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
 import { Aws, Construct } from '@aws-cdk/core';
+import { DEFAULT_PASSWORD_EXCLUDE_CHARS } from './private/util';
 
 /**
  * Construction properties for a DatabaseSecret.
@@ -24,6 +25,13 @@ export interface DatabaseSecretProps {
    * @default - no master secret information will be included
    */
   readonly masterSecret?: secretsmanager.ISecret;
+
+  /**
+   * Characters to not include in the generated password.
+   *
+   * @default " %+~`#$&*()|[]{}:;<>?!'/@\"\\"
+   */
+  readonly excludeCharacters?: string;
 }
 
 /**
@@ -43,7 +51,7 @@ export class DatabaseSecret extends secretsmanager.Secret {
           masterarn: props.masterSecret?.secretArn,
         }),
         generateStringKey: 'password',
-        excludeCharacters: '"@/\\',
+        excludeCharacters: props.excludeCharacters ?? DEFAULT_PASSWORD_EXCLUDE_CHARS,
       },
     });
   }

packages/@aws-cdk/aws-rds/lib/instance.ts

@@ -11,8 +11,8 @@ import { Endpoint } from './endpoint';
 import { IInstanceEngine } from './instance-engine';
 import { IOptionGroup } from './option-group';
 import { IParameterGroup } from './parameter-group';
-import { applyRemovalPolicy, defaultDeletionProtection, engineDescription, setupS3ImportExport } from './private/util';
-import { Credentials, PerformanceInsightRetention, RotationMultiUserOptions, SnapshotCredentials } from './props';
+import { applyRemovalPolicy, DEFAULT_PASSWORD_EXCLUDE_CHARS, defaultDeletionProtection, engineDescription, setupS3ImportExport } from './private/util';
+import { Credentials, PerformanceInsightRetention, RotationMultiUserOptions, RotationSingleUserOptions, SnapshotCredentials } from './props';
 import { DatabaseProxy, DatabaseProxyOptions, ProxyTarget } from './proxy';
 import { CfnDBInstance, CfnDBInstanceProps } from './rds.generated';
 import { ISubnetGroup, SubnetGroup } from './subnet-group';
@@ -851,10 +851,10 @@ abstract class DatabaseInstanceSource extends DatabaseInstanceNew implements IDa
   /**
    * Adds the single user rotation of the master password to this instance.
    *
-   * @param [automaticallyAfter=Duration.days(30)] Specifies the number of days after the previous rotation
-   * before Secrets Manager triggers the next automatic rotation.
+   * @param options the options for the rotation,
+   *                if you want to override the defaults
    */
-  public addRotationSingleUser(automaticallyAfter?: Duration): secretsmanager.SecretRotation {
+  public addRotationSingleUser(options: RotationSingleUserOptions = {}): secretsmanager.SecretRotation {
     if (!this.secret) {
       throw new Error('Cannot add single user rotation for an instance without secret.');
     }
@@ -867,11 +867,12 @@ abstract class DatabaseInstanceSource extends DatabaseInstanceNew implements IDa
 
     return new secretsmanager.SecretRotation(this, id, {
       secret: this.secret,
-      automaticallyAfter,
       application: this.singleUserRotationApplication,
       vpc: this.vpc,
       vpcSubnets: this.vpcPlacement,
       target: this,
+      ...options,
+      excludeCharacters: options.excludeCharacters ?? DEFAULT_PASSWORD_EXCLUDE_CHARS,
     });
   }
 
@@ -883,9 +884,9 @@ abstract class DatabaseInstanceSource extends DatabaseInstanceNew implements IDa
       throw new Error('Cannot add multi user rotation for an instance without secret.');
     }
     return new secretsmanager.SecretRotation(this, id, {
-      secret: options.secret,
+      ...options,
+      excludeCharacters: options.excludeCharacters ?? DEFAULT_PASSWORD_EXCLUDE_CHARS,
       masterSecret: this.secret,
-      automaticallyAfter: options.automaticallyAfter,
       application: this.multiUserRotationApplication,
       vpc: this.vpc,
       vpcSubnets: this.vpcPlacement,
@@ -948,6 +949,7 @@ export class DatabaseInstance extends DatabaseInstanceSource implements IDatabas
       credentials = Credentials.fromSecret(new DatabaseSecret(this, 'Secret', {
         username: credentials.username,
         encryptionKey: credentials.encryptionKey,
+        excludeCharacters: credentials.excludeCharacters,
       }));
     }
     const secret = credentials.secret;
@@ -1026,6 +1028,7 @@ export class DatabaseInstanceFromSnapshot extends DatabaseInstanceSource impleme
       secret = new DatabaseSecret(this, 'Secret', {
         username: credentials.username,
         encryptionKey: credentials.encryptionKey,
+        excludeCharacters: credentials.excludeCharacters,
       });
     }
 

packages/@aws-cdk/aws-rds/lib/private/util.ts

@@ -3,6 +3,16 @@ import * as s3 from '@aws-cdk/aws-s3';
 import { Construct, CfnDeletionPolicy, CfnResource, RemovalPolicy } from '@aws-cdk/core';
 import { IEngine } from '../engine';
 
+/**
+ * The default set of characters we exclude from generated passwords for database users.
+ * It's a combination of characters that have a tendency to cause problems in shell scripts,
+ * some engine-specific characters (for example, Oracle doesn't like '@' in its passwords),
+ * and some that trip up other services, like DMS.
+ *
+ * This constant is private to the RDS module.
+ */
+export const DEFAULT_PASSWORD_EXCLUDE_CHARS = " %+~`#$&*()|[]{}:;<>?!'/@\"\\";
+
 /** Common base of `DatabaseInstanceProps` and `DatabaseClusterBaseProps` that has only the S3 props */
 export interface DatabaseS3ImportExportProps {
   readonly s3ImportRole?: iam.IRole;

packages/@aws-cdk/aws-rds/lib/props.ts

@@ -134,6 +134,14 @@ export interface CredentialsFromUsernameOptions {
    * @default - default master key
    */
   readonly encryptionKey?: kms.IKey;
+
+  /**
+   * The characters to exclude from the generated password.
+   * Has no effect if {@link password} has been provided.
+   *
+   * @default - the DatabaseSecret default exclude character set (" %+~`#$&*()|[]{}:;<>?!'/@\"\\")
+   */
+  readonly excludeCharacters?: string;
 }
 
 /**
@@ -146,7 +154,10 @@ export abstract class Credentials {
    * If no password is provided, one will be generated and stored in SecretsManager.
    */
   public static fromUsername(username: string, options: CredentialsFromUsernameOptions = {}): Credentials {
-    return { username, password: options.password, encryptionKey: options.encryptionKey };
+    return {
+      ...options,
+      username,
+    };
   }
 
   /**
@@ -197,6 +208,34 @@ export abstract class Credentials {
    * @default - none
    */
   public abstract readonly secret?: secretsmanager.Secret;
+
+  /**
+   * The characters to exclude from the generated password.
+   * Only used if {@link generatePassword} if true.
+   *
+   * @default - the DatabaseSecret default exclude character set (" %+~`#$&*()|[]{}:;<>?!'/@\"\\")
+   */
+  public abstract readonly excludeCharacters?: string;
+}
+
+/**
+ * Options used in the {@link SnapshotCredentials.fromGeneratedPassword} method.
+ */
+export interface SnapshotCredentialsFromGeneratedPasswordOptions {
+  /**
+   * KMS encryption key to encrypt the generated secret.
+   *
+   * @default - default master key
+   */
+  readonly encryptionKey?: kms.IKey;
+
+  /**
+   * The characters to exclude from the generated password.
+   * Has no effect if {@link password} has been provided.
+   *
+   * @default - the DatabaseSecret default exclude character set (" %+~`#$&*()|[]{}:;<>?!'/@\"\\")
+   */
+  readonly excludeCharacters?: string;
 }
 
 /**
@@ -208,8 +247,8 @@ export abstract class SnapshotCredentials {
    *
    * Note - The username must match the existing master username of the snapshot.
    */
-  public static fromGeneratedPassword(username: string, encryptionKey?: kms.IKey): SnapshotCredentials {
-    return { generatePassword: true, username, encryptionKey };
+  public static fromGeneratedPassword(username: string, options: SnapshotCredentialsFromGeneratedPasswordOptions = {}): SnapshotCredentials {
+    return { generatePassword: true, username, encryptionKey: options.encryptionKey, excludeCharacters: options.excludeCharacters };
   }
 
   /**
@@ -275,12 +314,46 @@ export abstract class SnapshotCredentials {
    * @default - none
    */
   public abstract readonly secret?: secretsmanager.Secret;
+
+  /**
+   * The characters to exclude from the generated password.
+   * Only used if {@link generatePassword} if true.
+   *
+   * @default - the DatabaseSecret default exclude character set (" %+~`#$&*()|[]{}:;<>?!'/@\"\\")
+   */
+  public abstract readonly excludeCharacters?: string;
+}
+
+/**
+ * Properties common to single-user and multi-user rotation options.
+ */
+interface CommonRotationUserOptions {
+  /**
+   * Specifies the number of days after the previous rotation
+   * before Secrets Manager triggers the next automatic rotation.
+   *
+   * @default - 30 days
+   */
+  readonly automaticallyAfter?: Duration;
+
+  /**
+   * Specifies characters to not include in generated passwords.
+   *
+   * @default " %+~`#$&*()|[]{}:;<>?!'/@\"\\"
+   */
+  readonly excludeCharacters?: string;
+}
+
+/**
+ * Options to add the multi user rotation
+ */
+export interface RotationSingleUserOptions extends CommonRotationUserOptions {
 }
 
 /**
  * Options to add the multi user rotation
  */
-export interface RotationMultiUserOptions {
+export interface RotationMultiUserOptions extends CommonRotationUserOptions {
   /**
    * The secret to rotate. It must be a JSON string with the following format:
    * ```
@@ -296,14 +369,6 @@ export interface RotationMultiUserOptions {
    * ```
    */
   readonly secret: secretsmanager.ISecret;
-
-  /**
-   * Specifies the number of days after the previous rotation before
-   * Secrets Manager triggers the next automatic rotation.
-   *
-   * @default Duration.days(30)
-   */
-  readonly automaticallyAfter?: Duration;
 }
 
 /**

packages/@aws-cdk/aws-rds/test/integ.cluster-rotation.lit.expected.json

@@ -594,7 +594,7 @@
           ]
         },
         "GenerateSecretString": {
-          "ExcludeCharacters": "\"@/\\",
+          "ExcludeCharacters": " %+~`#$&*()|[]{}:;<>?!'/@\"\\",
           "GenerateStringKey": "password",
           "PasswordLength": 30,
           "SecretStringTemplate": "{\"username\":\"admin\"}"
@@ -807,7 +807,8 @@
               "DatabaseRotationSingleUserSecurityGroupAC6E0E73",
               "GroupId"
             ]
-          }
+          },
+          "excludeCharacters": " %+~`#$&*()|[]{}:;<>?!'/@\"\\"
         }
       }
     }

packages/@aws-cdk/aws-rds/test/integ.instance-s3.expected.json

@@ -530,7 +530,7 @@
           ]
         },
         "GenerateSecretString": {
-          "ExcludeCharacters": "\"@/\\",
+          "ExcludeCharacters": " %+~`#$&*()|[]{}:;<>?!'/@\"\\",
           "GenerateStringKey": "password",
           "PasswordLength": 30,
           "SecretStringTemplate": "{\"username\":\"admin\"}"

packages/@aws-cdk/aws-rds/test/integ.instance.lit.expected.json

@@ -549,7 +549,7 @@
           ]
         },
         "GenerateSecretString": {
-          "ExcludeCharacters": "\"@/\\",
+          "ExcludeCharacters": " %+~`#$&*()|[]{}:;<>?!'/@\"\\",
           "GenerateStringKey": "password",
           "PasswordLength": 30,
           "SecretStringTemplate": "{\"username\":\"syscdk\"}"
@@ -832,6 +832,7 @@
             ]
           },
           "functionName": "awscdkrdsinstanceInstanceRotationSingleUserAFE3C214",
+          "excludeCharacters": " %+~`#$&*()|[]{}:;<>?!'/@\"\\",
           "vpcSubnetIds": {
             "Fn::Join": [
               "",

packages/@aws-cdk/aws-rds/test/integ.proxy.ts

@@ -10,7 +10,9 @@ const vpc = new ec2.Vpc(stack, 'vpc', { maxAzs: 2 });
 const dbInstance = new rds.DatabaseInstance(stack, 'dbInstance', {
   engine: rds.DatabaseInstanceEngine.POSTGRES,
   instanceType: ec2.InstanceType.of(ec2.InstanceClass.BURSTABLE3, ec2.InstanceSize.MEDIUM),
-  credentials: rds.Credentials.fromUsername('master'),
+  credentials: rds.Credentials.fromUsername('master', {
+    excludeCharacters: '"@/\\',
+  }),
   vpc,
 });
 

packages/@aws-cdk/aws-rds/test/test.cluster.ts

@@ -184,6 +184,7 @@ export = {
       engine: DatabaseClusterEngine.AURORA_MYSQL,
       credentials: {
         username: 'admin',
+        excludeCharacters: '"@/\\',
       },
       instanceProps: {
         instanceType: ec2.InstanceType.of(ec2.InstanceClass.BURSTABLE2, ec2.InstanceSize.SMALL),