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

Feature Request: Create "Encrypted" Variants of Some Policy Templates #1959

Open
chrisoverzero opened this issue Mar 10, 2021 · 0 comments
Open

Comments

@chrisoverzero
Copy link
Contributor

chrisoverzero commented Mar 10, 2021

Describe your idea/feature/enhancement

When using KMS with various AWS resources, it can be undesirable to give a Function (or a State Machine – anything which can take a SAM Policy Template, I guess) blanket kms:Decrypt for a particular key in order to augment, say, AWSSecretsManagerGetSecretValuePolicy, DynamoDBCrudPolicy or the like. Through KMS, IAM has Conditions which can read the encryption context of a particular request, narrowing the scope of the permissions. As an example:

- AWSSecretsManagerGetSecretValuePolicy:
    SecretArn: !Ref MySecret

…transforms into (effectively):

{
  "Statement": [
    {
      "Effect": "Allow",
      "Action": [
        "secretsmanager:GetSecretValue"
      ],
      "Resource": { "Ref": "MySecret" }
    }
  ]
}

…with the user of the Policy Template also having to add KMSDecryptPolicy to the same resource when the Secret is encrypted with a CMK. These could be combined into one while narrowing the scope of the KMS permissions like this:

- Encrypted-AWSSecretsManagerGetSecretValuePolicy: # This name is very much not a recommendation.
    SecretArn: !Ref MySecret
    KeyId: !Ref MyKey

…transforming into (effectively):

{
  "Statement": [
    {
      "Effect": "Allow",
      "Action": [
        "secretsmanager:GetSecretValue"
      ],
      "Resource": { "Ref": "MySecret" }
    },
    {
      "Effect": "Allow",
      "Action": [
        "kms:Decrypt"
      ],
      "Resource": {
        "Fn::Sub": [
          "arn:${AWS::Partition}:kms:${AWS::Region}:${AWS::AccountId}:key/${keyId}",
          {
            "keyId": { "Ref": "KeyId" }
          }
        ]
      },
      "Condition": {
        "StringEquals": {
          "kms:ViaService": { "Fn::Sub": "secretsmanager.${AWS::Region}.${AWS::URLSuffix}" }
        },
        "ForAnyValue:ArnEquals": {
          "kms:EncryptionContext:SecretARN": { "Ref": "MySecret" }
        }
      }
    }
  ]
}

Knowing the context in which the key will be used allows the permissions to be narrowed greatly.

This would also provide ergonomics where it would be silly to provide every possible IAM permission as a separate SAM Policy Template. For example, to publish to an encrypted SNS topic, a Function requires:

  • kms:Encrypt
  • kms:GenerateDataKey
  • sns:Publish

…for only two of which does a SAM Policy Template exist. A template for kms:GenerateDataKey has been requested, but matching the template to the use case would obviate such requests:

- Encrypted-SNSPublishMessagePolicy:
    TopicName: !GetAtt MyTopic.TopicName
    KeyId: !Ref MyKey

…the transformation of which, I hope, is straightforward. (I'm getting tired of doing it by hand. 😝)

Proposal

When I am able, I can go through the existing SAM Policy Templates to audit which would benefit from an "Encrypted" variant. I can then list which kms: permissions such a variant would use, then really dig into the documentation and list the encryption contexts each of those operations accepts. From that point, it's only a matter of plugging them into the document which defines the policy templates.

Oh, and naming. We'd have to decide on naming. Possibly the hardest part.

Things to consider:
[x] The SAM documentation will need to be updated

Additional Details

I wonder if some kind of smart use of Fn::If or its like would allow KeyId to become an optional parameter to the existing templates. Where leaving it out would have the template transform exactly as it does today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants