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.66k stars 3.92k forks source link

Aws cdk don't provide a way to create secrets with specified values #5810

Closed aakrem closed 2 years ago

aakrem commented 4 years ago

In the link below it's specified in the docs that we can create secrets from a JSON file containing the values of the secrets.

aws secretsmanager create-secret --name MyTestDatabaseSecret \
    --description "My test database secret created with the CLI" \
    --secret-string file://mycreds.json

Unfortunately, I don't find a way to do this with aws cdk. The create-secret method doesn't accept values of the secrets themselves and code like below will autogenerate a secret.

    new secretsmanager.Secret(this, 'Secret', {
      description: 'secret description,
      secretName: 'secretName'
    });

Secrets manager docs from CLI: https://docs.aws.amazon.com/cli/latest/reference/secretsmanager/create-secret.html#examples

AWS CDK secrets manager docs: https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-secretsmanager.Secret.html

richardhboyd commented 4 years ago

+1 to this. I had to drop down to the L1 Construct to create a Secret with a specific value (i.e. a Lambda CR that creates an EC2 KeyPair, then put the SecretKey into SecretsManager via cfn)

Cloudrage commented 4 years ago

+1 too to create a SecretString in addition of the generateSecretString Props.

ilkomiliev commented 4 years ago

+1

trusanen-bpg commented 4 years ago

+1

richardhboyd commented 4 years ago

please don't add comments with just "+1". Use the đź‘Ť on the original issue so that we can better track the number of users who want this and it keeps the comment section clean for discussion of the issue.

gadams999 commented 4 years ago

+1 (yes, I did đź‘Ť the main ask too :)). For Greengrass we have the need to send secrets locally to the secrets manager services, and based on the edge use-case, need to provide specific values and not randomly generated ones.

bmVsc29u commented 4 years ago

we need this for the scenario:

hence, either cloudformation provide a way to inject secrets directly to secrets manager, or allow secrets manager to store access secret key via AccessKey's property would be accepted. But I am worry about the secrets may leave some foot prints through the secretString way.

njlynch commented 4 years ago

9594 closed this issue, but then #9610 reverted it. I wanted to share the rationale on this issue for those that have been waiting for the functionality.

Supplying the secretString directly in the CDK exposes the secret in all of the CDK outputs (cdk.out and outputs of commands like cdk synth), in the CloudFormation template itself (visible in the console and via the SDK/CLI), and increases the risk of a secret being committed to source. Even with appropriate warnings in the documentation, the risk of accidental leakage was deemed too high by the team.

The preferred guidance is to use the AWS SDK/CLI/console to store these secrets, and then import them into you CDK app.

If you still want to achieve the same effect, you can always either use escape hatches or write your own construct.

```ts // Escape hatch approach const secret = new secretsmanager.Secret(stack, 'Secret'); const cfnSecret = secret.construct.defaultChild as secretsmanager.CfnSecret; cfnSecret.generateSecretString = undefined; cfnSecret.secretString = 'mynotverysecuresecret'; ```
skyrpex commented 4 years ago

Libraries like react use abnormally long property names for properties that are insecure, like dangerouslySetInnerHTML. Maybe it would be a good idea to do something similar, if it's that dangerous.

ThomasSteinbach commented 4 years ago

The decision just to revert the ability to provide a secret string on secret creation is quite unsatisfying.

First the decision is not documented. It took me a while to get to the reasoning here, after I read the CDK API documentation, the CDK code and searching the open and closed issues. I was short before creating a feature request.

Furthermore the documentation explicitly says to use the aws_secretsmanager to store secret strings in AWS:

Parameters of type SecretString cannot be created directly from a CDK application; if you want to provision secrets automatically, use Secrets Manager Secrets (see the @aws-cdk/aws-secretsmanager package).

https://docs.aws.amazon.com/cdk/api/latest/docs/aws-ssm-readme.html#creating-new-ssm-parameters-in-your-cdk-app

So currently there seems no viable solution to store a secret string in AWS with CDK.

Instead of reverting the feature, wouldn't it be possible to implement another feature which let CDK hide sensitive content?

njlynch commented 4 years ago

So currently there seems no viable solution to store a secret string in AWS with CDK.

As stated in https://github.com/aws/aws-cdk/issues/5810#issuecomment-672736662, there is an option (using escape hatches); we intentionally want to keep some friction here given the potentially dangerous consequences of this approach. The other approach is to use the Secrets Manager via the console, SDK, or CLI to store the secret, then import the secret using the CDK.

trusanen-bpg commented 4 years ago

As stated in #5810 (comment), there is an option (using escape hatches); we intentionally want to keep some friction here given the potentially dangerous consequences of this approach. The other approach is to use the Secrets Manager via the console, SDK, or CLI to store the secret, then import the secret using the CDK.

There is still a valid need for this feature in the Secret-construct, even though there are potential security issues as well.

Suppose the following use-case: One would like to provision an RDS instance from a snapshot that includes a master user with some specific initial password. The master user credentials need to be passed automatically to some service that accesses the database.

A secure system can be achieved by utilizing secret rotation, but it would still require passing the initial password in CDK. Currently this can be done with escape hatches like you mentioned, but it's not very pretty or convenient. This could be a feature of SecretRotation as well.

Cloudrage commented 4 years ago

So currently there seems no viable solution to store a secret string in AWS with CDK.

As stated in #5810 (comment), there is an option (using escape hatches); we intentionally want to keep some friction here given the potentially dangerous consequences of this approach. The other approach is to use the Secrets Manager via the console, SDK, or CLI to store the secret, then import the secret using the CDK.

Not agree with that, in my case, I often use Secrets Manager to store multiple key/value for 1 use-case, don't want to create xxx SSM Parameter Store (like I do few years ago before SecretsManager was released). In fact, I prefere store values in one "vault" instead of creating a lot of parameters.

For example, a "secret" with vpc keys (yeah, it's not possible with CDK to retreieve PrivateSubnets #9891 for example); so I've create 1 secret with @ll informations about my vpc/subnets (cidr, IDs...). Another example with SMTP Secrets, store values like endpoint/port/user/domain but don't want to generate a string for the pwd, it's added by a CustomResource. Same for a KeyPair.

So, in my code I have to always put a parameter like : generateStringKey: 'WaitingForSecretStringPropsInCDK' Because I don't want to generate a string.

cmwd commented 3 years ago

Using ECS with a private repository is another example when this would be useful. The ContainerImage.fromRegistry method requires a Secret instance and this makes it hard to provide an authentication token.

h3smith commented 3 years ago

Using ECS with a private repository is another example when this would be useful. The ContainerImage.fromRegistry method requires a Secret instance and this makes it hard to provide an authentication token.

To echo @cmwd — if you are using Fargate/ECS and private repos, you need to supply a Secret value held in SSM to grab the credentials. This adds an extra step into the process to automate deployment with CDK.

Dzhuneyt commented 3 years ago

Using ECS with a private repository is another example when this would be useful. The ContainerImage.fromRegistry method requires a Secret instance and this makes it hard to provide an authentication token.

To echo @cmwd — if you are using Fargate/ECS and private repos, you need to supply a Secret value held in SSM to grab the credentials. This adds an extra step into the process to automate deployment with CDK.

I have the same scenario and was frustrated by this in the beginning, but on a second thought, it is much more secure to generate your GitHub Personal Access Key manually and create the SSM manually and hardcode a reference to it inside your codebase, instead of going the "tempting" and "easy" way of hardcoding the token itself in your codebase. It's like putting your .env files into source control, but much more dangerous. Don't do it. Yes, your infrastructure is ephemeral nowadays, using tools like CDK or Terraform, but your personal access keys are not. Replacing them is difficult, once compromised, and reverting damages is even more difficult. Would you hardcode your password inside the source code? Probably not.

On the other hand though, I'm sure there are valid use cases where you might want to use SSM and provide their values in plain text. Something outside the scope of GitHub Access Tokens.

E.g IPs that point to other parts of the infrastructure, API keys, password salts (not actual passwords), etc.

TikiTDO commented 3 years ago

@Dzhuneyt There's no need to hard-code anything in your code base though. My approach is when I need to update a secret I can create a temporary file and store it in a location outside of the code repo. When the deployer detects that this file is present it is first GPG encrypted using a SecretManger password. That encrypted file is uploaded as an asset, and once it's stored in the target bucket it is decrypted, parsed, and written to another SecretManager value using an S3 onCreate handler. It's a huge pain to maintain all this, but it's certainly doable and reasonably secure.

This has the secondary benefit that you can trivially let people upload new secrets, without giving them any additional dashboard access which you then need to manage and audit. Unfortunately all this involves a lot of song and dance which an official solution could easily hide away.

RaulProenza commented 3 years ago

@Dzhuneyt There's no need to hard-code anything in your code base though. My approach is when I need to update a secret I can create a temporary file and store it in a location outside of the code repo. When the deployer detects that this file is present it is first GPG encrypted using a SecretManger password. That encrypted file is uploaded as an asset, and once it's stored in the target bucket it is decrypted, parsed, and written to another SecretManager value using an S3 onCreate handler. It's a huge pain to maintain all this, but it's certainly doable and reasonably secure.

This has the secondary benefit that you can trivially let people upload new secrets, without giving them any additional dashboard access which you then need to manage and audit. Unfortunately all this involves a lot of song and dance which an official solution could easily hide away.

HI @Dzhuneyt! Can you share some details about your solution design? Did you use a white paper or howtos that you can share as reference to achieve the same?

TikiTDO commented 3 years ago

@RaulProenza I think you meant that question for me.

I did not use any tutorial, just a lot of custom code. There was a mix of s3Deployment.BucketDeployment docs, using s3Assets.Source docs, configuring it to abuse the bundling parameter docs, which would do a few aws-cli and gpg commands on the build machine, and a few custom lambdas to parse the secret after it's uploaded to s3. Looking back at it, the gpg stuff was probably unnecessary, as long as you use a proper encryption key on your uploaded resource.

I can't share the code since it was all written for a client, but if you spend some time looking at the documentation for all of the above features you should be able to replicate something like this with some work.

sunshineo commented 3 years ago

Everyone take a guess on what people will do when they cannot save a pre-existing secret in AWS Secret Manager: ... ... ... My answer: they will save it in AWS System Manager Parameter Store

richardhboyd commented 3 years ago

@njlynch Can we re-open this? it's a real pain that people are going to resort to less secure alternatives as workarounds. There's no reason that I cannot specify

const secret = new secretsmanager.Secret(stack, 'Secret', value: Fn.getAtt(myCustomResourceFunction, 'KeyPair'));

This would allow me to store a secret in SecretsManager and wouldn't leave any confidential information in the template. We're forcing users to use L1 resources and escape hatches because we think they might do something dumb, but we don't try to stop them from spinning up 100 p4d.24xlarge instances.

njlynch commented 3 years ago

Thanks everyone for continuing to :+1: and comment on this issue with your use cases. When we reverted the initial implementation, we were worried about the potential for accidentally leaking secrets when specifying the initial string value, and wanted to intentionally add a little friction to make it more likely a user doing it knew the consequences.

Given the feedback on this issue, I'm going to re-open.

One possible compromise that still addresses the original concerns would be to re-enable passing in a secretString, but require that the value was a Token. This means that creating a secret for an IAM user's credentials (@bmVsc29u 's use case) or from the output of a custom resource (@richardhboyd 's example) would be supported, but just providing a hard-coded 'p@ssw0rd' would still require some degree of work-around.

I would love to get feedback on if the above solves (most) use cases, or if there's still a strong need to hard-code an insecure plaintext string password.

aax commented 3 years ago

Would passing a stack parameter value into a secret be considered secure? I guess that way it will also never get coded into the template or anywhere in cdk.out. Would this also work with the approach described by @njlynch ?

fkjellman commented 3 years ago

Thanks everyone for continuing to đź‘Ť and comment on this issue with your use cases. When we reverted the initial implementation, we were worried about the potential for accidentally leaking secrets when specifying the initial string value, and wanted to intentionally add a little friction to make it more likely a user doing it knew the consequences.

Given the feedback on this issue, I'm going to re-open.

One possible compromise that still addresses the original concerns would be to re-enable passing in a secretString, but require that the value was a Token. This means that creating a secret for an IAM user's credentials (@bmVsc29u 's use case) or from the output of a custom resource (@richardhboyd 's example) would be supported, but just providing a hard-coded 'p@ssw0rd' would still require some degree of work-around.

I would love to get feedback on if the above solves (most) use cases, or if there's still a strong need to hard-code an insecure plaintext string password.

@njlynch This is exactly what I'm looking for, any progress or eta updates on this issue?

Thanks in advance!

clafollett commented 3 years ago

Thanks everyone for continuing to đź‘Ť and comment on this issue with your use cases. When we reverted the initial implementation, we were worried about the potential for accidentally leaking secrets when specifying the initial string value, and wanted to intentionally add a little friction to make it more likely a user doing it knew the consequences.

Given the feedback on this issue, I'm going to re-open.

Much appreciated. I've just run into this issue myself in spinning up new S3 Buckets, Users, Policies and IAM CfnAccessKey. I was hoping to create the Keys and then store the generated secrets in SecretManager so we could grab them from the Console to put into our 3rd Party vendors system for bucket integration. I would definitely be interested in seeing a solution for this. Here is an example usage:

import * as cdk from '@aws-cdk/core';
import * as iam from '@aws-cdk/aws-iam';
import * as s3 from '@aws-cdk/aws-s3';
import * as sm from '@aws-cdk/aws-secretsmanager';

export interface S3AssetsBucketStackProps extends cdk.StackProps {
  lambdaTimeout?: number;
  lambdaThrottleSize?: number;

}

export class S3AssetsBucketStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: S3AssetsBucketStackProps) {
    super(scope, id, props);

    const assetBucket = new s3.Bucket(this, 'CmsAssetsBucket', {
      autoDeleteObjects: false,
      blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
      enforceSSL: true,
      encryption: s3.BucketEncryption.S3_MANAGED,
      removalPolicy: cdk.RemovalPolicy.RETAIN,
      cors: [
        {
          allowedHeaders: ['*'],
          allowedOrigins: ['*'],
          allowedMethods: [
            s3.HttpMethods.DELETE,
            s3.HttpMethods.GET,
            s3.HttpMethods.PUT
          ]          
        }
      ]
    });

    const cmsUser = new iam.User(this, 'CmsAssetsUser', { userName: 'cms-assets' });
    const cmsUserAccessKey = new iam.CfnAccessKey(this, 'CmsAssetsAccessKey', { userName: cmsUser.userName });
    const cmsUserPolicyDoc = iam.PolicyDocument.fromJson(
      {
        'Version': '2012-10-17',
        'Statement': [
          {
            'Action': [
              's3:DeleteObject',
              's3:ListBucket',
              's3:GetObject',
              's3:GetBucketLocation',
              's3:PutObject',
              's3:PutObjectAcl'
            ],
            'Effect': 'Allow',
            'Resource': [
              assetBucket.bucketArn,
              `${assetBucket.bucketArn}/*`
            ]
          },
          {
            'Action': [
              'rekognition:DetectLabels',
              'rekognition:DetectModerationLabels'
            ],
            'Effect': 'Allow',
            'Resource': '*'
          }
        ]
      }
    );

    const cmsUserPolicy = new iam.Policy(this, 'CmsUserPolicy', { document: cmsUserPolicyDoc });
    cmsUserPolicy.attachToUser(cmsUser);

    new sm.Secret(this, 'CmsUserAccessKey', {
      description: 'The IAM Access Key for the CMS Assets Bucket User',
      // Set Secret String from Access Key instance here
    });
  }
}
njlynch commented 3 years ago

Thanks again for all the feedback and :+1:s. As I've received no counter-points or complaints, I think we can proceed with the above approach of allowing Tokened plaintext secrets.

I can't provide a concrete ETA for an implementation. However, I'd be happy to work with a member of the community if someone wants to contribute a solution. #9594 could be used as the basis for the implementation, with a few minor updates to introduce the restriction of Tokenized input and some additional docs + tests.

damian-keska commented 3 years ago

I have similar case as @clafollett. I need to store IAM user credentials for Jenkins deployment in the AWS Secrets Manager. Additionally all passwords from AWS SM are synchronized to Jenkins password manager automatically, so for me it is natural way to just put IAM credentials into Jenkins using AWS SM.

@njlynch Do you have an example of https://github.com/aws/aws-cdk/issues/5810#issuecomment-672736662 workaround for Python?

SleeperSmith commented 3 years ago

Ah here we go. Another one of those problems fabricated by CDK. Lovely.

Just feed in the secrets via CloudFormation parameters with NoEcho option. Use a dummy value if you can't feed in optional values during CICD process and change it afterwards. Otherwise feed it once on first deploy.

But hey, feel free to sit around and assume CDK users are idiots who's going to hard code password into templates when using Secrets Manager. Better remove the environment variables in lambda/task definition while at it. You never know when people are going to hard code passwords in there.

@damian-keska-f @clafollett just use the L1 to create the secret and pass in the value. It works perfectly fine. You wasting your time screwing around with the L2 (as usual). https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-secretsmanager.CfnSecret.html

TikiTDO commented 3 years ago

@SleeperSmith That can be dangerous depending on how you use the secret. Refer from this block from the CloudFormation docs:

We strongly recommend against including NoEcho parameters, or any sensitive data, in resource 
properties that are part of a resource's primary identifier.

When a NoEcho parameter is included in a property that forms a primary resource identifier, CloudFormation
may use the actual plaintext value in the primary resource identifier. This resource ID may appear in any 
derived outputs or destinations.

To determine which resource properties comprise a resource type's primary identifier, refer to the resource
reference documentation for that resource. In the Return values section, the Ref function return value 
represents the resource properties that comprise the resource type's primary identifier.

My recommendation is to use application layer encryption with a password stored in secret manager. Here's an example:

    // You must create this beforehand
    const yourPasswordSecretArn =
      "arn:aws::secretsmanager:us-east-1:123456789012:secret:example/MyExample"

    // Fetch this value sometime before you start uploading
    const yourPassword = getFromCertificatManager(yourPasswordSecretArn)

    // Get the unecrypted secret value
    const yourSecret = fs.readFileSync("/path/to/unencrypted.secret")

    // Create a place for your uploaded secret to live
    const uploadedSecert = new cdk.aws_secretsmanager.Secret(this, "UploadedSecret", {})

    // This is where your encrypted secret is uploaded before it is parsed
    const secretBucket = new cdk.aws_s3.Bucket(this, "SecretBucket", {
      /* ... */
    })

    // This function decrypts your secret on the server
    const decryptAndStoreLambda = new cdk.aws_lambda.Function(this, "DecryptAndStoreFunction", {
      code: cdk.aws_lambda.Code.fromAsset("/path/to/decryptAndStore.js"),
      handler: "decryptAndStore.handler",
      runtime: cdk.aws_lambda.Runtime.NODEJS_14_X,
    })

    // Your lambda should be able to write the secert to it's new home
    uploadedSecert.grantWrite(decryptAndStoreLambda)

    // Your lambda will need to be able to decrypt the secert
    uploadedSecert.addToResourcePolicy(
      new cdk.aws_iam.PolicyStatement({
        actions: ["secretsmanager:GetSecretValue"],
        effect: cdk.aws_iam.Effect.ALLOW,
        resources: [yourPasswordSecretArn],
      }),
    )

    // Ensure your lambda is called whe the secret is uploaded
    secretBucket.addEventNotification(
      cdk.aws_s3.EventType.OBJECT_CREATED_PUT,
      new cdk.aws_s3_notifications.LambdaDestination(decryptAndStoreLambda),
    )

    // Encrypt your seceret before it gets uploaded
    const deploymentAsset = cdk.aws_s3_deployment.Source.asset("/path/to/empty/directory", {
      bundling: {
        command: ["sh", "-c", 'echo "Docker build not supported. Please install go."'],
        image: cdk.DockerImage.fromRegistry("alpine"),
        local: {
          tryBundle(outputDir: string, options: BundlingOptions) {
            fs.writeFileSync(
              path.join(outputDir, "encrypted.secret"),
              encryptYourSecret(yourSecret, yourPassword),
            )
            return true
          },
        },
      },
    })

    // Upload your secret
    new cdk.aws_s3_deployment.BucketDeployment(this, "SecretUpload", {
      destinationBucket: secretBucket,
      sources: [deploymentAsset],
    })

This way you know that the only person that has access to your secret is someone with read access to secret manager.

You could even go further, and store a public key locally in a .gitignore'd location, then store a private key somewhere on AWS to decrypt.

damian-keska commented 3 years ago

I know it is probably not a best solution, but working one.

credentials = CfnAccessKey(self, id=USER_ACCESS_KEY_OBJECT_ID, user_name=user.user_name)

CfnSecret(
    self,
    id=SECRET_OBJECT_ID,
    name=SECRET_OBJECT_ID,
    description='API access',
    secret_string=json.dumps({
        'API_USER': access_key.ref,
        'API_PASSPHRASE': access_key.attr_secret_access_key
    })
)
makemefeelgr8 commented 3 years ago

How come there is a way to create a secret, but no easy way to set it's value? How do you expect one to even use this feature? Like, create a secret using CDK and then open AWS website to fill in the value? Really? Don't you guys have APIs?

Let me show you how your competitors do it: https://docs.microsoft.com/en-us/azure/devops/pipelines/tasks/deploy/azure-key-vault?view=azure-devops

Just check this out! az keyvault secret set --vault-name 'ContosoKeyVault' --name 'SQLPassword' --value 'Pa$$w0rd'

Now that's a solid reason to choose Azure over AWS.

TikiTDO commented 3 years ago

@makemefeelgr8 Make a secret. Make a lambda. Give it a role with write access to the secret. Pass the secret ARN as an ENV variable. Start the lambda to populate the secret.

Also, your example comparing apples and oranges. CDK is not the CLI interface. It's a template generator.

If you want to use the AWS CLI like in your example, the command is aws secretmanager put-secret-value, you can get the docs here.

dbartholomae commented 3 years ago

I'm coming at this from a perspective of using Mozilla Sops which allows to store the encrypted secrets in the same repo as the rest of the infra. Both CI as well as devs who should be able to update the secrets get access to the KMS key used for encryption. @njlynch what would be the preferred way from your perspective for a workflow like this? If I can create secrets from tokens, then this might be one of the few use cases for CfnParameter, as the goal here explicitly is to not have the value during the Synth step?

If this makes sense, I'm happy to try my hand at a PR for the token support.

makemefeelgr8 commented 3 years ago

Make a secret. Make a lambda. Give it a role with write access to the secret. Pass the secret ARN as an ENV variable. Start the lambda to populate the secret.

So, basically, what you're suggesting is to spend few days on research and write like 500 lines of code to just store a setting securely. Really? I mean, you're joking, right? Isn't cdk supposed to automate things, to make deployment easier? How about you do it instead? Like, write a method that makes all those dirty hacks you mentioned- creates lambdas and whatnot. And just expose the method to the user!

Here is a no-brainer:

new secretsmanager.Secret(this, 'Secret', {
      secretName: 'secretName',
      secretValue: value
    });

Why should an end user care about writing lambda functions if all they want is to store 1 value securely?!

And don't even start talking about security. Let me tell you what an average user would do in case they can't get it to work. Noone is going to spend days figuring out how to implement all those hacks you've mentioned. They'll just hardcode a value in code. Yes, that's right! And add a TODO comment with a link to this issue on github: "todo: store password in aws (if they ever implement support for it)".

Man, I really wish devs would care about user feedback.

disordered commented 3 years ago

Make a secret. Make a lambda. Give it a role with write access to the secret. Pass the secret ARN as an ENV variable. Start the lambda to populate the secret.

So, basically, what you're suggesting is to spend few days on research and write like 500 lines of code to just store a setting securely. Really? I mean, you're joking, right? Isn't cdk supposed to automate things, to make deployment easier? How about you do it instead? Like, write a method that makes all those dirty hacks you mentioned- creates lambdas and whatnot. And just expose the method to the user!

Here is a no-brainer:

new secretsmanager.Secret(this, 'Secret', {
      secretName: 'secretName',
      secretValue: value
    });

Why should an end user care about writing lambda functions if all they want is to store 1 value securely?!

And don't even start talking about security. Let me tell you what an average user would do in case they can't get it to work. Noone is going to spend days figuring out how to implement all those hacks you've mentioned. They'll just hardcode a value in code. Yes, that's right! And add a TODO comment with a link to this issue on github: "todo: store password in aws (if they ever implement support for it)".

Man, I really wish devs would care about user feedback.

Why are you so confrontational?

If you don't care about leaking secret values in Cfn template, there is a workaround available: https://github.com/aws/aws-cdk/issues/5810#issuecomment-914054341

TikiTDO commented 3 years ago

@makemefeelgr8 First off, chill the hell out. I took the time out of my day to answer your question, and I don't see why I that deserves some random person whining at me that it's too much work. If you don't like the answer then submit a PR. That's what I do whenever there's a feature missing in a framework I use. It's far more effective than complaining.

Second, you can scroll up a few comments where I literally have a snippet of almost all the code you need to do this. It's far from 500 lines, and it certainly doesn't need "a few days of research."

Third, you're working on an cloud based infrastructure. Research is literally part of the job, as is coding. If you don't like this then you might be in the wrong line of work.

Fourth, as someone else has already pointed out, there is a perfectly serviceable workaround if you're fine with having secrets in your CF template. What more, this workaround was posted in the comment prior to yours, so you could have saved a lot of time if you did a few minutes of research.

Fifth, the reason that they made it hard to put secrets in the template is to avoid exactly the scenario you outlined.

Sixth, this repo literally has thousands of issues open, with PRs getting merged constantly. Clearly they care about user feedback enough to run a huge open source project to simplify a process that used to be much, much more painful than it is now. I understand being annoyed that things don't get fixed as quickly as you'd like, but they are running a huge system used by countless organizations big and small. A single user complaining at the bottom of a nearly 2 year old issue probably isn't going to be super high priority.

I for one appreciate the fact that the rest of us have at least some say in the process.

makemefeelgr8 commented 3 years ago

@TikiTDO First off, chill the hell out.

I'm not that good at English, sorry about that. Somehow, it sounded perfectly normal in my head.

If you don't like the answer then submit a PR.

I totally would, if it was a free open source project without a multi-billion corporation behind it. AWS is neither free, nor cheap. People are paying them a ton of money every day. So, how about they use some of those funds to actually hire developers and fix those "thousands of issues"? As you've mentioned, this one is almost 2 year old, and it have not even been addressed. So, this made me kinda disappointed. At the very least, they could have just closed it, with a comment like "will not implement".

hoegertn commented 3 years ago

I'm coming at this from a perspective of using Mozilla Sops which allows to store the encrypted secrets in the same repo as the rest of the infra. Both CI as well as devs who should be able to update the secrets get access to the KMS key used for encryption. @njlynch what would be the preferred way from your perspective for a workflow like this? If I can create secrets from tokens, then this might be one of the few use cases for CfnParameter, as the goal here explicitly is to not have the value during the Synth step?

If this makes sense, I'm happy to try my hand at a PR for the token support.

That's exactly the use case I have written https://github.com/taimos/secretsmanager-versioning for. Also, you need to think about versioning, what this does for you, to make sure to handle updates to secrets correctly.

jlk commented 2 years ago

The "escape hatch" listed above no longer works.

const cfnSecret = secret.construct.defaultChild as secretsManager.CfnSecret;

results in

Property 'construct' does not exist on type 'Secret'.

What does seem to work is to instead use

const cfnSecret = secret.node.defaultChild as secretsManager.CfnSecret;

So the updated hack would be:

    const secret = new secretsManager.Secret(this, 'Secret')
    const cfnSecret = secret.node.defaultChild as secretsManager.CfnSecret;
    cfnSecret.generateSecretString = undefined;
    cfnSecret.secretString = 'mynotverysecuresecret';

AWS folks - appreciate the friendly engagement on this issue, but I have to stress how frustrating this is to us. We're trying to do the right thing and store data in secrets in a programmatic way, and the response from CDK feels like "lol nope."

makemefeelgr8 commented 2 years ago

Dirty hacks we've used have just stopped working... Will this get addressed, like, ever? Any updates? Anyone? Please? @otaviomacedo @kaizen3031593 @rix0rrr @skinny85 @peterwoodworth @njlynch @madeline-k @ryparker

kellertk commented 2 years ago

Just to copy what @njlynch said above, it's very easy to accidentally leak your secrets when specifying the value, because the value will be present in the generated CloudFormation templates, outputs of commands, and others. I won't rehash the discussion here, but the team does not plan on changing this in the future.

Of course, CDK always lets you go down to the L1 construct if you want to. Here's an example stack for CDK v2:

import { Stack, StackProps } from 'aws-cdk-lib';
import { Construct } from 'constructs';
import { CfnSecret } from 'aws-cdk-lib/aws-secretsmanager';

export class TestStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);
    const mySecret = 'I-promise-I-will-not-put-my-secrets-in-my-source-code';
    new CfnSecret(this, 'my-very-insecure-secret', {
      description: 'This secret is visible in the cloudformation template!',
      secretString: mySecret,
    });
  }
}

This should produce:

Resources:
  myveryinsecuresecret:
    Type: AWS::SecretsManager::Secret
    Properties:
      Description: This secret is visible in the cloudformation template!
      SecretString: I-promise-I-will-not-put-my-secrets-in-my-source-code
    Metadata:
      aws:cdk:path: TestStack/my-very-insecure-secret
github-actions[bot] commented 2 years ago

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

makemefeelgr8 commented 2 years ago

@njlynch Man, you're my hero! The bug was hanging there for 2 years and you've just fixed it. Thanks!

kellertk commented 2 years ago

Hi! After discussing this further internally we've decided to go ahead and provide an Tokenized implementation as discussed above. Note that there are still some potential sharp edges here and you should always use care when working with secret values, but pull #18098 should cover many use cases.

github-actions[bot] commented 2 years ago

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

dmschauer commented 1 year ago

For anyone stumbling across this and using the Python version of the AWS CDK: you need to use the CfnSecretClass instead of the SecretClass if you want to do it the "insecure" way. There you will find a secret_stringproperty https://docs.aws.amazon.com/cdk/api/v1/python/aws_cdk.aws_secretsmanager/CfnSecret.html

For example in this code, I have a file called secrets_config.json that holds the secret values and read the values from there to store them in Secrets Manager. The file isn't checked into Git (it's in .gitignore) and users of the repository are instructed to create the file before executing cdk deploy

import json
import os

from aws_cdk import Stack, aws_secretsmanager
from constructs import Construct

class SecretsStack(Stack):
    def __init__(
        self,
        scope: Construct,
        stack_name: str,
        construct_id: str,
        secret_name: str,
        **kwargs,
    ) -> None:
        super().__init__(scope, stack_name=stack_name, id=construct_id, **kwargs)

        # Read secret values from the config file
        with open(os.path.join(os.path.dirname(__file__), "secrets_config.json"), "r") as file:
            secrets = json.load(file)

        secret = aws_secretsmanager.CfnSecret(  # noqa: F841
            self, id=secret_name, name=secret_name, secret_string=json.dumps(secrets)
        )
nickwillan commented 6 months ago

I've created this open-source CDK Construct you can use to embed encrypted ciphertext into your code, and it will securely decrypt and set a value in the secret without exposing the secret to the stack output: https://github.com/nickwillan/cdk-encrypted-secret. I hope this helps.