fix: adding missing pivot role permission to get key policy

[zsaltys]

Feature or Bugfix

• Bugfix

Detail

• getKeyPolicy permission is required by share manager. I'm not sure if it is required in 2.0 but it is definitely required for S3 bucket policy sharing. Though I suspect it should be needed for OS version to as for access points to work pivotRole needs to update KMS key policy right? Without this permission sharing manager fails to get the policy and fails. The only workaround is if you manually add the pivotRole to the KMS key policy.

Relates

N/A

Security

This change expands the pivot role with a new permission to get key policy. This is still following least permission principle.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[zsaltys]

@noah-paige @dlpzx please review

[noah-paige]

Looked through the latest code in v2m1m0 and it appears we do add kms:GetKeyPolicy along with other required permissions for the pivotRole IF it is auto-created via cdk along with the environment stack. We may have missed also adding to the pivotRole yaml template

It may work fine for newly created datasets since we do explicitly define the pivotRole permissions (including kms:GetKeyPolicy) in the dataset's KMS Key Policy but would require the manual workaround to add the pivot role to the key policy for imported buckets if not there already

I think the above changes look good to me - think it should only require the 1 line to add the kms:GetKeyPolicy in the pivotRole.yaml template but can also perform additional validation on this again on v2m1m0 before release - @dlpzx let me know thoughts and I can merge (++ thank you @zsaltys)

[dlpzx]

We were also surprised to see this permission issue in the latest validations that we did. Access point sharing was working for the validation of v2.0.0, so we are unsure of why this error started happening. In any case, as @noah-paige just said, we added the permission to the pivotRole-cdk but forgot the CloudFormation yaml template update. thank you @zsaltys I will merge and test it with v2m1m0 as it is already tested with the cdk version of it