getsops / sops

Simple and flexible tool for managing secrets
https://getsops.io/
Mozilla Public License 2.0
16.29k stars 857 forks source link

add support for kms key aliases #381

Open snstanton opened 5 years ago

snstanton commented 5 years ago

It would be convenient be able to use a key alias as in: aws kms encrypt --key-id alias/my-key ... rather than having to provide the full arn.

autrilla commented 5 years ago

Duplicate of #366

snstanton commented 5 years ago

This isn't actually a duplicate of 366. I was talking about the AWS key alias mechanism (e.g. aws kms list-aliases) not just adding a string label in the .sops file. In some aws apis, you can specify either the ARN for the key or an alias like alias/mykey.

autrilla commented 5 years ago

Oh, cool, I didn't know that even existed! Reopening

rosscdh commented 4 years ago

This does not seem to be fixed in 3.3.0 as documented in changelog not in HEAD of master

func (key MasterKey) createSession() (*session.Session, error) {
    re := regexp.MustCompile(`^arn:aws[\w-]*:kms:(.+):[0-9]+:(key|alias)/.+$`)

requires the full arn (which defies the point of an alias?)

autrilla commented 4 years ago

I have no idea how that AWS feature works, but #415 shows you don't need to provide the full ARN. You need the account ID and the alias name. We still have to tell the AWS SDK that it's an alias somehow.

I'm not sure I understand why this is an issue for you. My guess (but it's just that, a guess) is that you're specifying the key through command line flags, which to me feels like doing it wrong. It is convenient for one-off operations and examples, but I don't think it really is how you should actually use sops. Using a .sops.yaml file is a better approach in my opinion.

rosscdh commented 4 years ago

Hey there thanks for the reply.

I'll check the sops yaml setup (am using it via the kustomize sops plugin FYI) but i suspect based on the code i pasted above the full arn will still be necessary

Also re the purpose of a kms alias: its point appears to be to not require the region nor acc id as per https://docs.aws.amazon.com/cli/latest/reference/kms/encrypt.html

--key-id (string)

A unique identifier for the customer master key (CMK).

To specify a CMK, use its key ID, Amazon Resource Name (ARN), alias name, or alias ARN. When using an alias name, prefix it with "alias/" . To specify a CMK in a different AWS account, you must use the key ARN or alias ARN.

For example:

Key ID: 1234abcd-12ab-34cd-56ef-1234567890ab
Key ARN: arn:aws:kms:us-east-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab
Alias name: alias/ExampleAlias
Alias ARN: arn:aws:kms:us-east-2:111122223333:alias/ExampleAlias

as you can see

aws kms encrypt \
    --key-id alias/ExampleAlias \
    --plaintext fileb://ExamplePlaintextFile \
    --output text \
    --query CiphertextBlob > C:\Temp\ExampleEncryptedFile.base64

is valid and then allows keys to be used independently of region and acc id

autrilla commented 4 years ago

I'll check the sops yaml setup (am using it via the kustomize sops plugin FYI) but i suspect based on the code i pasted above the full arn will still be necessary

I'm afraid so.

Also re the purpose of a kms alias: its point appears to be to not require the region nor acc id as per https://docs.aws.amazon.com/cli/latest/reference/kms/encrypt.html

--key-id (string)

A unique identifier for the customer master key (CMK).

To specify a CMK, use its key ID, Amazon Resource Name (ARN), alias name, or alias ARN. When using an alias name, prefix it with "alias/" . To specify a CMK in a different AWS account, you must use the key ARN or alias ARN.

For example:

Key ID: 1234abcd-12ab-34cd-56ef-1234567890ab
Key ARN: arn:aws:kms:us-east-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab
Alias name: alias/ExampleAlias
Alias ARN: arn:aws:kms:us-east-2:111122223333:alias/ExampleAlias

I see. I think it would indeed be nice to be able to provide the key in the same formats the AWS CLI takes, which we currently don't support.

rosscdh commented 4 years ago

@autrilla submitted a PR for review (needs tests but want to see if imp is ok first

https://github.com/mozilla/sops/pull/541

wwentland commented 3 years ago

It would be amazing if this could be implemented. Especially now that KMS supports multi-regional keys:

haarchri commented 2 years ago

we get the following error in decrypt process with alias

Group 0: FAILED
  arn:aws:kms:eu-central-1:123456789102:alias/sops-example: FAILED
    - | Error decrypting key: AccessDeniedException: The ciphertext
      | refers to a customer master key that does not exist, does
      | not exist in this region, or you are not allowed to access.
      |     status code: 400, request id:
      | 12411fe7-14f1-4dcc-xxe3-190bf941a7xx

cloudtrail:

"errorMessage": "User: arn:aws:sts::123456789102:assumed-role/sops is not authorized to perform: kms:Decrypt on resource: arn:aws:kms:eu-central-1:123456789102:key/xx930d41-d9a4-462a-8b61-12be1d944c8x because no identity-based policy allows the kms:Decrypt action",

our solution:

encrypt:

sops --kms arn:aws:kms:eu-central-1:123456789102:alias/sops-example --encrypt --encrypted-regex '^(users)$' test.yaml

decrypt:

sops --kms arn:aws:kms:eu-central-1:123456789102:alias/sops-example --decrypt test.yaml

we need to usekms:ResourceAliases in iam policy because it seems sops changes the alias/sops-example in decrypt process to the key-id

The kms:ResourceAliases condition is a condition of the resource, not the request. As such, a request that doesn't specify the alias can still satisfy the condition.

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Action": [
        "kms:Encrypt",
        "kms:Decrypt",
        "kms:ReEncrypt*",
        "kms:GenerateDataKey*",
        "kms:DescribeKey"
      ],
      "Resource": "*",
      "Condition": {
        "ForAnyValue:StringLike": {
          "kms:ResourceAliases": [
            "alias/sops-example"
          ]
        }
      }
    },
    {
      "Action": [
        "kms:Describe*",
        "kms:Get*",
        "kms:List*",
        "tag:GetResources"
      ],
      "Effect": "Allow",
      "Resource": "*"
    }
  ]
}

https://docs.amazonaws.cn/en_us/kms/latest/developerguide/alias-authorization.html

typekpb commented 1 year ago

as far as I've tested, this one is resolved in the meantime and should be closed.

dudicoco commented 1 year ago

@typekpb how did you test this? Doesn't work for me:

$ sops --encrypt -in-place secrets.yaml
Could not generate data key: [failed to encrypt new data key with master key "alias/example": Failed to create session: No valid ARN found in "alias/example"]

The code still only checks for an ARN: https://github.com/mozilla/sops/blob/1bb30e28b484dd0f8611ee1807766c9bbdc941ad/kms/keysource.go#L196-L201

haarchri commented 1 year ago

@dudicoco check my last comment for IAM policy - this is working

dudicoco commented 1 year ago

@haarchri this issue is specifically about using the alias without the full ARN - alias/example.

haarchri commented 1 year ago

using the arn of the alias is not an option ?

dudicoco commented 1 year ago

@haarchri for some it is, for others it's preferable to not use the full ARN

elovelan commented 1 year ago

This would make using multi-region keys considerably easier, especially in a DR scenario (not sure if KMS has ever gone down in a region but it's feasible).

I'll take a look at the code in #541 and see if I can make it work for all scenarios supported by the Encrypt API (i.e. add support for Key ID without ARN as well) and open a new PR.

Just to be clear: this PR might be a month or so away; I don't want to set expectations too high!

moshest commented 1 year ago

I'm not sure this is needed. You can use the alias name with the following ARN:

arn:aws:kms:${region}:${account_id}:alias/${alias_name}`

For example:

arn:aws:kms:us-east-1:1234567890:alias/my/key
dudicoco commented 1 year ago

I'm not sure this is needed. You can use the alias name with the following ARN:

arn:aws:kms:${region}:${account_id}:alias/${alias_name}`

For example:

arn:aws:kms:us-east-1:1234567890:alias/my/key

We know the full ARN can be used, it's all over the comments and in the original comment as well... We would like not to hardcode the ARN (especially the account ID) within the SOPS yaml file.

Geun-Oh commented 2 months ago

Hello everyone. I made a draft of it with core logic of supporting alias-based encryption.

1537

If anyone has idea to join it, feel free for it.

When the logic seems to be completed, I'll turn it to pr.