Kralizek / AWSSecretsManagerConfigurationExtensions

This repository contains a provider for Microsoft.Extensions.Configuration that retrieves secrets stored in AWS Secrets Manager.
MIT License
219 stars 43 forks source link

Specify --version-stage AWSCURRENT in GetSecretValue #75

Closed avalara-stephen-hickey closed 1 year ago

avalara-stephen-hickey commented 1 year ago

Background

There are times when it's required to specify the version stage when performing secretsmanager:GetSecretValue.

In my case, when accessing a secret in a different AWS account, that has been shared to a credential I've grabbed from the CredentialProfileStoreChain, I am only able to access the secret when specifying --version-stage AWSCURRENT.

aws secretsmanager get-secret-value \
  --region us-west-2 \
  --secret-id arn:aws:secretsmanager:us-west-2:{redacted}:secret:{redacted} \
  --profile sharedsecretprofile \
  --version-stage AWSCURRENT

In the previous code block, when specifying --version-stage AWSCURRENT, I am able to retrieve the secret. Without it, I get the error

An error occurred (AccessDeniedException) when calling the GetSecretValue operation: User: arn:aws:sts::{redacted}:assumed-role/{redacted}/{redacted} is not authorized to perform: secretsmanager:GetSecretValue on resource: arn:aws:secretsmanager:us-west-2:{redacted}:secret:{redacted} because no resource-based policy allows the secretsmanager:GetSecretValue action

The AWS documentation says it is set to AWSCURRENT by default, but that is clearly not true based on the previous example.

Proposal

Add VersionStage = "AWSCURRENT" to the GetSecretValueRequest.

I believe this change won't have any affect on any users as not specifying the stage should have the same affect as specifying AWSCURRENT as the stage.

Changes

Update SecretsManagerConfigurationProvider#L218 from

var secretValue = await Client.GetSecretValueAsync(new GetSecretValueRequest { SecretId = secret.ARN }, cancellationToken).ConfigureAwait(false);

to

var secretValue = await Client.GetSecretValueAsync(new GetSecretValueRequest { SecretId = secret.ARN, VersionStage = "AWSCURRENT" }, cancellationToken).ConfigureAwait(false);

I'd be happy to make this change if it's decided the change is okay.

Kralizek commented 1 year ago

I'm not sure this is the best approach. I can't understand the side effects for other users of this library.

@normj maybe could shed some lights on this?

Alternatively, I'd rather add to the configuration a delegate that allows the customization of the request before its sent.

I know the configuration is getting a bit crowded with many properties, but I'm really afraid of a change that could cause issues to other users.

normj commented 1 year ago

That is interesting that going across accounts is requiring AWSCURRENT to be set. For my own education I would be curious on how the resource policy is configured, is it using a specific secret arn or wildcard and does that make a difference.

I don't think @avalara-stephen-hickey suggestion will have any negative side effects for other users but there is the question do you want provide the ability for more customizations in the version id and stage.

Kralizek commented 1 year ago

Thank you @normj for your input.

I see the request customization delegate a way to solve this issue and potentially handle future needs like the one you mention.

Anyway, maybe it's a policy configuration error. @avalara-stephen-hickey would it be ok for you to share a sanitized version of your policy?

avalara-stephen-hickey commented 1 year ago

I appreciate the quick response!

As far as I understand, there wouldn't be any affect on other users since this is supposed to be the default behavior anyway. But I understand your fear that it may affect other users because it's documented as the default behavior but somehow is also required in this scenario. My company has documented this behavior and attaches the following note to every secret they share.

Ensure that the development team is specifying version-state as AWSCURRENT. AWS docs state that if not specified it defaults to that version, however it needs to be explicitly stated for this to work.

A configurator option will solve my need, but I think to have long-term benefit, the action would require a context of what secret is being requested so that a user could change the version stage per secret.

Here are all the roles and policies related to what I'm experiencing.

AWS Account 1 (where the secret is)

// Policy arn:aws:iam::1:policy/policy
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "VisualEditor0",
            "Effect": "Allow",
            "Action": [
                "secretsmanager:GetResourcePolicy",
                "secretsmanager:GetSecretValue",
                "secretsmanager:DescribeSecret",
                "secretsmanager:ListSecretVersionIds"
            ],
            "Resource": "arn:aws:secretsmanager:us-west-2:1:secret:secret"
        },
        {
            "Sid": "VisualEditor1",
            "Effect": "Allow",
            "Action": [
                "kms:GetPublicKey",
                "kms:ListKeyPolicies",
                "kms:ListRetirableGrants",
                "kms:GetKeyPolicy",
                "kms:ListResourceTags",
                "kms:ListGrants",
                "kms:GetParametersForImport",
                "kms:DescribeCustomKeyStores",
                "kms:ListKeys",
                "secretsmanager:GetRandomPassword",
                "kms:GetKeyRotationStatus",
                "kms:ListAliases",
                "kms:DescribeKey",
                "secretsmanager:ListSecrets"
            ],
            "Resource": "arn:aws:kms:us-west-2:1:key/key"
        }
    ]
}

// Role arn:aws:iam::1:role/role
// Trusted relationships
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Principal": {
        "Service": "ec2.amazonaws.com"
      },
      "Action": "sts:AssumeRole"
    },
    {
        "Effect": "Allow",
        "Principal": {
            "AWS": "arn:aws:iam::2:role/role2"
        },
        "Action": "sts:AssumeRole"
    }
  ]
}

AWS Account 2 (the account that I'm accessing the secret from)

// Role arn:aws:iam::2:role/role2
// Trusted relationships
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "Service": "ec2.amazonaws.com",
                "AWS": "arn:aws:sts::2:assumed-role/{redacted}/me@mycompany.com"
            },
            "Action": "sts:AssumeRole"
        }
    ]
}

// Policy arn:aws:iam::2:policy/policy2
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": "secretsmanager:GetSecretValue",
            "Resource": "arn:aws:secretsmanager:us-west-2:1:secret:secret"
        },
        {
            "Effect": "Allow",
            "Action": "kms:Decrypt",
            "Resource": "arn:aws:kms:us-west-2:1:key/key"
        },
        {
            "Effect": "Allow",
            "Action": "sts:AssumeRole",
            "Resource": "arn:aws:iam::1:role/role"
        }
    ]
}

Here's an example of me assuming and using the role in account 2 and the result.

var chain = new Amazon.Runtime.CredentialManagement.CredentialProfileStoreChain();

// %USERPROFILE%/.aws/credentials looks like
// [sharedsecretprofile]
// role_arn = arn:aws:iam::2:role/role2
// source_profile = default
// toolkit_artifact_guid = {redacted}

if (!chain.TryGetAWSCredentials("sharedsecretprofile", out var credentials))
{
    return;
}

var client = new AmazonSecretsManagerClient(credentials);
// This returns the secret result as I would expect.
var test = await client.GetSecretValueAsync(new GetSecretValueRequest
{
    SecretId = secretId,
    VersionStage = "AWSCURRENT"
});

// This throws, error below.
var test2 = await client.GetSecretValueAsync(new GetSecretValueRequest
{
    SecretId = secretId
});

Error

User: arn:aws:sts::2:assumed-role/role2/aws-dotnet-sdk-session-{redacted} is not authorized to perform: secretsmanager:GetSecretValue on resource: arn:aws:secretsmanager:us-west-2:1:secret:secret because no resource-based policy allows the secretsmanager:GetSecretValue action
avalara-stephen-hickey commented 1 year ago

Here's a proposed change, let me know what you think.

This is the "add to the configuration a delegate that allows the customization of the request before its sent" approach.

avalara-stephen-hickey commented 1 year ago

@Kralizek thanks for merging the change! What is your release cadence for minor versions so I can know when to expect to include it?

Kralizek commented 1 year ago

I wanted to check the other PR and then release a 2.0

@avalara-stephen-hickey

Kralizek commented 1 year ago

@avalara-stephen-hickey I've done of move around of the commits so that the PR you worked on can be released as 1.7 without waiting for me to release the 2.0. Would you be ok fetching the artifacts and try them before I publish an actual release?

avalara-stephen-hickey commented 1 year ago

Yeah, I'll look into that today.

Kralizek commented 1 year ago

Yeah, I'll look into that today.

Thanks. I'll let you know when it's ready and add a comment to the build in AppVeyor.

Kralizek commented 1 year ago

Here you go.

https://www.nuget.org/packages/Kralizek.Extensions.Configuration.AWSSecretsManager/1.7.0

avalara-stephen-hickey commented 1 year ago

I've confirmed 1.7.0 solves the issue I was having. Thank you for your time and help!

Kralizek commented 1 year ago

Great to hear. Thank you for your help!

Would you mind giving a look at #80 and see if you like what I'm cooking there?