Skip to content

LG-10344 Opportunistically read multi-region KMS ciphertexts#9047

Merged
jmhooper merged 9 commits intomainfrom
jmhooper-opportunistically-kms-mr-read
Aug 23, 2023
Merged

LG-10344 Opportunistically read multi-region KMS ciphertexts#9047
jmhooper merged 9 commits intomainfrom
jmhooper-opportunistically-kms-mr-read

Conversation

@jmhooper
Copy link
Contributor

We are in the midst of a migration to a new KMS key that support multi-region operations. This migration involves the following steps:

  1. Start writing to new columns with the new key
  2. Start reading from the new columns if they have a value and using the new key to decrypt them
  3. Go through and convert old rows to the new key
  4. Drop the old column

This commit adds code and a feature flag to allow step 2. When then flag is enabled the IDP will try to use the multi-region column's ciphertext instead of the single-region ciphertext.

Note that KMS does not require a key ID for decryption. It can determine that based on the KMS ciphertext it receives for decrypt. As a result the multi-region KMS client can be used to decrypt multi-region and single-region cihpertexts.

We are in the midst of a migration to a new KMS key that support multi-region operations. This migration involves the following steps:

1. Start writing to new columns with the new key
2. Start reading from the new columns if they have a value and using the new key to decrypt them
3. Go through and convert old rows to the new key
4. Drop the old column

This commit adds code and a feature flag to allow step 2. When then flag is enabled the IDP will try to use the multi-region column's ciphertext instead of the single-region ciphertext.

Note that KMS does not require a key ID for decryption. It can determine that based on the KMS ciphertext it receives for decrypt. As a result the multi-region KMS client can be used to decrypt multi-region and single-region cihpertexts.

[skip changelog]
@jmhooper
Copy link
Contributor Author

I have this labelled WIP while I sort out tests

@jmhooper jmhooper marked this pull request as ready for review August 22, 2023 15:49
@jmhooper
Copy link
Contributor Author

I just got my tests updated. This should be ready for review

Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested locally with read flag enabled create user and log in/out, IdV, and reset password and recover PII with personal key.

@jmhooper jmhooper merged commit fdc81be into main Aug 23, 2023
@jmhooper jmhooper deleted the jmhooper-opportunistically-kms-mr-read branch August 23, 2023 15:28
jmhooper added a commit that referenced this pull request Sep 11, 2023
The `aws_kms_multi_region_read_enabled` was in place to toggle the changes introduced in #9047. Those changes have been deployed, enabled, and have been running without issue for some time.

This commit removes the tooling to toggle the feature since we will be using it going forward.

[skip changelog]
jmhooper added a commit that referenced this pull request Sep 11, 2023
The `aws_kms_multi_region_read_enabled` was in place to toggle the changes introduced in #9047. Those changes have been deployed, enabled, and have been running without issue for some time.

This commit removes the tooling to toggle the feature since we will be using it going forward.

[skip changelog]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants