terraform-aws-modules/terraform-aws-kms

enable_key_rotation should be automatically set to false for SIGN_VERIFY keys

gilbahat opened this issue · 7 comments

gilbahat commented

Description

key rotation is invalid for keys of type SIGN_VERIFY. when enable_key_rotation is not actively set to false for such a key, the provider will throw an error:

│ Error: creating KMS Key (redacted): key rotation: UnsupportedOperationException:
│
│ with module.kms.aws_kms_key.this[0],
│ on .terraform/modules/kms/main.tf line 8, in resource "aws_kms_key" "this":
│ 8: resource "aws_kms_key" "this" {

  • ✋ I have searched the open/closed issues and my issue is not listed.

Versions

  • Module version [Required]:
    1.5.0

  • Terraform version:

✗ terraform -version aws:Distribution.AdministratorAccess
Terraform v1.5.3
on darwin_amd64

  • provider registry.terraform.io/hashicorp/aws v4.67.0
  • provider registry.terraform.io/hashicorp/random v3.5.1
  • provider registry.terraform.io/olivr/gpg v0.2.1

Reproduction Code [Required]

module "kms" {
source = "terraform-aws-modules/kms/aws"

description = "Cosign signing key"
key_usage = "SIGN_VERIFY"
customer_master_key_spec = "RSA_4096"

Aliases

aliases = ["alias/redacted"]

}

Steps to reproduce the behavior:

terraform apply

Expected behavior

KMS key created

Actual behavior

│ Error: creating KMS Key (redacted): key rotation: UnsupportedOperationException:
│
│ with module.kms.aws_kms_key.this[0],
│ on .terraform/modules/kms/main.tf line 8, in resource "aws_kms_key" "this":
│ 8: resource "aws_kms_key" "this" {

Additional context

from https://docs.aws.amazon.com/kms/latest/developerguide/rotate-keys.html :
"The Key rotation tab appears only on the detail page of symmetric encryption KMS keys with key material that AWS KMS generated (the Origin is AWS_KMS), including multi-Region symmetric encryption KMS keys."

bryantbiggs commented

I'm not quite following - this UnsupportedOperationException means the API is stating its not a supported operation, which is outside the scope of the module

gilbahat commented

I'm not quite following - this UnsupportedOperationException means the API is stating its not a supported operation, which is outside the scope of the module

this happens because of poor default value for enable_key_rotation in the module: it defaults to true regardless of whether key rotation is supported or not.

I would expect the module to do two things:

  1. set the default according to the value of key_usage: true if ENCRYPT_DECRYPT but false if SIGN_VERIFY
  2. throw a clear and meaningful error message if the user attempts to set SIGN_VERIFY and enable_key_rotation - "asymmetric keys do not support rotation"
bryantbiggs commented

throw a clear and meaningful error message

Terraform does not support this. The module is designed with a set of default values and assumptions, but deviating from the defaults will require users to set the values correctly per the underlying AWS API

gilbahat commented

throw a clear and meaningful error message

Terraform does not support this. The module is designed with a set of default values and assumptions, but deviating from the defaults will require users to set the values correctly per the underlying AWS API

is this about the error message or the default value? because throwing an error seems supportable, looking something like this:

validation {
condition = var.key_usage == 'SIGN_VERIFY' && var.enable_key_rotation
error_message = "Asymmetric keys do not support rotation"
}

as for the variable - here's a possible way to support this: make the variable a tri-state one instead of true/false and do not use it directly. in the module execution, if set to 'default' and not true or false, look up the value of key_usage and derive the right default. e.g. (pseudocode)

real_key_rotation = var.enable_key_rotation == 'default' ? lookup(truefalsemap, var.key_usage) : toboolean(var.enable_key_rotation)

github-actions commented

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

github-actions commented

This issue was automatically closed because of stale in 10 days

github-actions commented

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.