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

KMS resource Key Policy propagation consistent fail #21225

Closed
AshMenhennett opened this issue Oct 9, 2021 · 15 comments · Fixed by #22093
Closed

KMS resource Key Policy propagation consistent fail #21225

AshMenhennett opened this issue Oct 9, 2021 · 15 comments · Fixed by #22093
Assignees
Labels
bug Addresses a defect in current functionality. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. service/kms Issues and PRs that pertain to the kms service.
Milestone

Comments

@AshMenhennett
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform CLI and Terraform AWS Provider Version

Terraforn Core: 1.0.5
AWS Provider: 3.62.0

Affected Resource(s)

  • aws_kms_key

Terraform Configuration Files

Same as per #20588 which isn't resolved in latest provider.

Workaround is to roll back to 3.52 which allows to create a key with policy having multiple statements.

Note: providing a policy that is equivalent to the default (non lock out) policy will create successfully, when adding an additional statement, provisioning will fail.

There are new comments on #20588 that provide additional detail.

Thanks for helping resolve.

Expected Behavior

I should be able to create a CMK key with a custom policy.

Actual Behavior

Provisioning fails with failure to propagate key policy.

Steps to Reproduce

Create a aws_key_resource with the policy attribute set to a policy with multiple statements.

References

@github-actions github-actions bot added needs-triage Waiting for first response or review from a maintainer. service/kms Issues and PRs that pertain to the kms service. labels Oct 9, 2021
@ewbankkit
Copy link
Contributor

func KeyPolicyPropagated(conn *kms.KMS, id, policy string) error {
checkFunc := func() (bool, error) {
output, err := finder.KeyPolicyByKeyIDAndPolicyName(conn, id, tfkms.PolicyNameDefault)
if tfresource.NotFound(err) {
return false, nil
}
if err != nil {
return false, err
}
equivalent, err := awspolicy.PoliciesAreEquivalent(aws.StringValue(output), policy)
if err != nil {
return false, err
}
return equivalent, nil
}
opts := tfresource.WaitOpts{
ContinuousTargetOccurence: 5,
MinTimeout: 1 * time.Second,
}
return tfresource.WaitUntil(KeyPolicyPropagationTimeout, checkFunc, opts)
}

Potentially caused by awspolicy.PoliciesAreEquivalent incorrectly returning false?

@justinretzolk justinretzolk added bug Addresses a defect in current functionality. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. and removed needs-triage Waiting for first response or review from a maintainer. labels Oct 12, 2021
@CrawX
Copy link

CrawX commented Oct 20, 2021

I'm facing this issue but can't downgrade to 3.52 because I need to use autoscaling_group_tag which has only been added in v3.56.0.

I attempted to debug the issue and in my case it was related to me using the following in my policy (note the boolean represented as an actual bool)

{
  "Condition": {
    "Bool": {
      "kms:GrantIsForAWSResource": true
    }
  }
}

I debugged awspolicy.PoliciesAreEquivalent and found that the equals implemetation fails, because the policy retrieved via the KMS APIs (the "theirs" side of the comparison) has kms:GrantIsForAWSResource set to "true" instead (boolean represented as string).

I changed my condition to the following:

{
  "Condition": {
    "Bool": {
      "kms:GrantIsForAWSResource": "true"
    }
  }
}

and it seems to work now. Terraform was also continously showing a diff before this change but I assumed this to be unrelated. Changing true to "true" has fixed this was well.

This might not be the cause in the case of @AshMenhennett but may be of help to others.
Cheers

@AshMenhennett
Copy link
Author

Hey @CrawX,

That's awesome to find workaround AND use the new features. I also appreciate that you took the time to debug it, as that's helpful us as well!

Seeing this thread looks like it's related, when I find some time I'll see if we can remedy as you have.

@ellaqezi
Copy link

ellaqezi commented Nov 30, 2021

Thanks to @CrawX's pointer, we also fixed our issue and now use hashicorp/aws v3.67.0. Here's the summary, instead of using

    principals {
      identifiers = [data.aws_caller_identity.current.user_id]
      type        = "AWS"
    }

we now use

    principals {
      identifiers = [data.aws_caller_identity.current.arn]
      type        = "AWS"
    }

which allows us to avoid updates-in-place related to the principals block of our key policies like the one below.

~ Principal = {
        ~ AWS = "arn:aws:iam::***:user/foobar" -> "AY6MGURNFAFDLAID2CBNV"
}

So it looks like the validation between user_id and arn is either not working or does so after the 5m0s timeout.

@Zambonilli
Copy link
Contributor

As @CrawX mentioned study your plan differences closely on your key policy and try to remove them. We ended up having defined our KMS key policy with 2 AWS principal fields instead of a single AWS field as an array.

@ewbankkit
Copy link
Contributor

The failure to converge with a Condition with a non-quoted boolean has been addressed in hashicorp/[email protected] merged via #2206.
I have added an acceptance test for this case.

@github-actions github-actions bot added this to the v3.69.0 milestone Dec 7, 2021
@github-actions
Copy link

This functionality has been released in v3.69.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@trallnag
Copy link

Still occurring for me

@mijndert
Copy link

Same.

@mijndert
Copy link

mijndert commented Mar 7, 2022

Any update @ewbankkit?

resource "aws_ses_domain_identity" "this" {
  for_each      = toset(var.ses_domains)
  domain        = each.value
}

resource "aws_ses_domain_mail_from" "this" {
  for_each         = toset(var.ses_domains)
  domain           = each.value
  mail_from_domain = "aws.${each.value}"
}

resource "aws_kms_key" "mail-bounces" {
  description = "KMS key for SNS topic mail-bounces"
  enable_key_rotation = true
  policy = <<EOT
{
  "Statement": [
    {
      "Effect": "Allow",
      "Principal": {
        "Service": "ses.amazonaws.com"
      },
      "Action": [
        "kms:GenerateDataKey*",
        "kms:Decrypt"
      ],
      "Resource": "*"
    },
    {
      "Effect": "Allow",
      "Principal": { "AWS": "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root" },
      "Action": [
        "kms:Create*",
        "kms:Describe*",
        "kms:Enable*",
        "kms:List*",
        "kms:Put*",
        "kms:Update*",
        "kms:Revoke*",
        "kms:Disable*",
        "kms:Get*",
        "kms:Delete*",
        "kms:ScheduleKeyDeletion",
        "kms:CancelKeyDeletion"
      ],
      "Resource": "*"
    }
  ]
}
EOT
}
resource "aws_sns_topic" "bounces" {
  name              = "mail-bounces"
  kms_master_key_id = aws_kms_key.mail-bounces.key_id
}

resource "aws_ses_identity_notification_topic" "bounces" {
  for_each                 = toset(var.ses_domains)
  identity                 = each.value
  topic_arn                = aws_sns_topic.bounces.arn
  notification_type        = "Bounce"
  include_original_headers = true
}

Error:

╷
│ Error: error waiting for KMS Key (21b3e2f2-86b6-413c-b56c-cdc16eb8f71b) policy propagation: timeout while waiting for state to become 'TRUE' (last state: 'FALSE', timeout: 5m0s)
│ 
│   with module.ses.aws_kms_key.mail-bounces,
│   on ../../../modules/ses/main.tf line 13, in resource "aws_kms_key" "mail-bounces":
│   13: resource "aws_kms_key" "mail-bounces" {
│ 
╵

@lsacco-nutreense
Copy link

lsacco-nutreense commented Mar 15, 2022

Same for me to on v3.74.3.

@homer6
Copy link

homer6 commented Apr 12, 2022

Still occurs for me at v3.75.1

@homer6
Copy link

homer6 commented Apr 12, 2022

/reopen

@jkmerriam
Copy link

This is also occurring for me on hashicorp/aws v4.1

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. service/kms Issues and PRs that pertain to the kms service.
Projects
None yet