From 6bb27c48b6096b969433f8e38383a21c28468b9b Mon Sep 17 00:00:00 2001 From: Sharon Nam Date: Wed, 19 Jul 2023 09:43:26 -0700 Subject: [PATCH] Allow usage of service principals for KMS grants --- .changelog/32595.txt | 3 ++ internal/service/kms/grant.go | 26 ++++++++++------- internal/service/kms/grant_test.go | 47 ++++++++++++++++++++++++++++++ internal/verify/validate.go | 21 +++++++++++++ internal/verify/validate_test.go | 38 ++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 10 deletions(-) create mode 100644 .changelog/32595.txt diff --git a/.changelog/32595.txt b/.changelog/32595.txt new file mode 100644 index 00000000000..9ddae06b2ab --- /dev/null +++ b/.changelog/32595.txt @@ -0,0 +1,3 @@ +```release-note:feature +resource/aws_kms_grant: Allow usage of service principal as grantee and revoker. +``` \ No newline at end of file diff --git a/internal/service/kms/grant.go b/internal/service/kms/grant.go index d4b7f0520f1..1d19522c481 100644 --- a/internal/service/kms/grant.go +++ b/internal/service/kms/grant.go @@ -90,10 +90,13 @@ func ResourceGrant() *schema.Resource { Computed: true, }, "grantee_principal": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: verify.ValidARN, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.Any( + verify.ValidARN, + verify.ValidServicePrincipal, + ), }, "key_id": { Type: schema.TypeString, @@ -122,10 +125,13 @@ func ResourceGrant() *schema.Resource { ForceNew: true, }, "retiring_principal": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - ValidateFunc: verify.ValidARN, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: validation.Any( + verify.ValidARN, + verify.ValidServicePrincipal, + ), }, }, } @@ -331,13 +337,13 @@ func findGrantByTwoPartKeyWithRetry(ctx context.Context, conn *kms.KMS, keyID, g } if principal := aws.StringValue(grant.GranteePrincipal); principal != "" { - if !arn.IsARN(principal) { + if !arn.IsARN(principal) && !verify.IsServicePrincipal(principal) { return retry.RetryableError(fmt.Errorf("grantee principal (%s) is invalid. Perhaps the principal has been deleted or recreated", principal)) } } if principal := aws.StringValue(grant.RetiringPrincipal); principal != "" { - if !arn.IsARN(principal) { + if !arn.IsARN(principal) && !verify.IsServicePrincipal(principal) { return retry.RetryableError(fmt.Errorf("retiring principal (%s) is invalid. Perhaps the principal has been deleted or recreated", principal)) } } diff --git a/internal/service/kms/grant_test.go b/internal/service/kms/grant_test.go index e27a580751a..a2b77edff8b 100644 --- a/internal/service/kms/grant_test.go +++ b/internal/service/kms/grant_test.go @@ -277,6 +277,41 @@ func TestAccKMSGrant_crossAccountARN(t *testing.T) { }) } +func TestAccKMSGrant_service(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_kms_grant.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + servicePrincipal := "dynamodb.us-west-1.amazonaws.com" //lintignore:AWSAT003 + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, kms.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckGrantDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccGrantConfig_service(rName, "\"Encrypt\", \"Decrypt\"", servicePrincipal), + Check: resource.ComposeTestCheckFunc( + testAccCheckGrantExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "grantee_principal", servicePrincipal), + resource.TestCheckResourceAttr(resourceName, "retiring_principal", servicePrincipal), + resource.TestCheckResourceAttr(resourceName, "operations.#", "2"), + resource.TestCheckTypeSetElemAttr(resourceName, "operations.*", "Encrypt"), + resource.TestCheckTypeSetElemAttr(resourceName, "operations.*", "Decrypt"), + resource.TestCheckResourceAttrPair(resourceName, "key_id", "aws_kms_key.test", "key_id"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"grant_token", "retire_on_delete"}, + }, + }, + }) +} + func testAccCheckGrantDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).KMSConn(ctx) @@ -496,3 +531,15 @@ resource "aws_kms_grant" "test" { } `, rName, operations)) } + +func testAccGrantConfig_service(rName string, operations string, servicePrincipal string) string { + return acctest.ConfigCompose(testAccGrantConfig_base(rName), fmt.Sprintf(` +resource "aws_kms_grant" "test" { + name = %[1]q + key_id = aws_kms_key.test.key_id + operations = [%[2]s] + grantee_principal = %[3]q + retiring_principal = %[3]q +} +`, rName, operations, servicePrincipal)) +} diff --git a/internal/verify/validate.go b/internal/verify/validate.go index 8d9a9e9e27b..696ed21fcfc 100644 --- a/internal/verify/validate.go +++ b/internal/verify/validate.go @@ -27,6 +27,9 @@ var accountIDRegexp = regexp.MustCompile(`^(aws|aws-managed|third-party|\d{12}|c var partitionRegexp = regexp.MustCompile(`^aws(-[a-z]+)*$`) var regionRegexp = regexp.MustCompile(`^[a-z]{2}(-[a-z]+)+-\d$`) +// validates all listed in https://gist.github.com/shortjared/4c1e3fe52bdfa47522cfe5b41e5d6f22 +var servicePrincipalRegexp = regexp.MustCompile(`^([a-z0-9-]+\.){1,4}(amazonaws|amazon)\.com$`) + func Valid4ByteASN(v interface{}, k string) (ws []string, errors []error) { value := v.(string) @@ -479,3 +482,21 @@ func ValidAnyDiag(validators ...schema.SchemaValidateDiagFunc) schema.SchemaVali return results } } + +func ValidServicePrincipal(v interface{}, k string) (ws []string, errors []error) { + value := v.(string) + + if value == "" { + return ws, errors + } + + if !IsServicePrincipal(value) { + errors = append(errors, fmt.Errorf("%q (%s) is an invalid Service Principal: invalid prefix value (expecting to match regular expression: %s)", k, value, servicePrincipalRegexp)) + } + + return ws, errors +} + +func IsServicePrincipal(value string) (valid bool) { + return servicePrincipalRegexp.MatchString(value) +} diff --git a/internal/verify/validate_test.go b/internal/verify/validate_test.go index 0b5b20616c5..3d715f5b170 100644 --- a/internal/verify/validate_test.go +++ b/internal/verify/validate_test.go @@ -737,3 +737,41 @@ func TestFloatGreaterThan(t *testing.T) { } } } + +func TestValidServicePrincipal(t *testing.T) { + t.Parallel() + + v := "" + _, errors := ValidServicePrincipal(v, "test.google.com") + if len(errors) != 0 { + t.Fatalf("%q should not be validated as an Service Principal name: %q", v, errors) + } + + validNames := []string{ + "a4b.amazonaws.com", + "appstream.application-autoscaling.amazonaws.com", + "alexa-appkit.amazon.com", + "member.org.stacksets.cloudformation.amazonaws.com", + "vpc-flow-logs.amazonaws.com", + "logs.eu-central-1.amazonaws.com", + } + for _, v := range validNames { + _, errors := ValidServicePrincipal(v, "arn") + if len(errors) != 0 { + t.Fatalf("%q should be a valid Service Principal: %q", v, errors) + } + } + + invalidNames := []string{ + "test.google.com", + "transfer.amz.com", + "test", + "testwithwildcard*", + } + for _, v := range invalidNames { + _, errors := ValidServicePrincipal(v, "arn") + if len(errors) == 0 { + t.Fatalf("%q should be an invalid Service Principal", v) + } + } +}