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

Add note about not supporting customer managed keys #1025

Conversation

mikaylathompson
Copy link
Collaborator

@mikaylathompson mikaylathompson commented Sep 27, 2024

Description

It came up in PE review that we don't support customer managed keys within the migration infrastructure. Adding a note about it to the SECURITY.md caveats section.

Issues Resolved

[List any issues this PR will resolve]

Is this a backport? If so, please add backport PR # and/or commits #

Testing

[Please provide details of testing done: unit testing, integration testing and manual testing]

Check List

  • New functionality includes testing
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

codecov bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.31%. Comparing base (d0d78b0) to head (52446ed).
Report is 15 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1025      +/-   ##
============================================
+ Coverage     80.24%   80.31%   +0.07%     
- Complexity     2723     2726       +3     
============================================
  Files           365      366       +1     
  Lines         13618    13617       -1     
  Branches        942      942              
============================================
+ Hits          10928    10937       +9     
+ Misses         2115     2106       -9     
+ Partials        575      574       -1     
Flag Coverage Δ
gradle-test 78.30% <ø> (+0.08%) ⬆️
python-test 90.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

SECURITY.md Outdated
@@ -23,3 +23,6 @@ If you are concerned about this scenario, we recommend fully mitigating it by pu
The output tuples, available on the shared EFS volume via the Migration Console, contain the exact requests and responses received from both the source and target clusters with the headers and the body of the messages. The Authorization header is present on SigV4 signed requests and those using basic authorization, and with basic authorization credentials can be extracted from the header value. These values are often essential for debugging and so are not censored from the output.

If you use basic authorization credentials, ensure that access to your output tuples is protected similarly to the credentials themselves.

### Customer Managed Keys are not supported by the migration infrastructure
We are able to migrate data to and from clusters with customer managed keys, but data in the intermediary stages (on Kafka, EFS volume, ephemeral storage on ECS) is stored with KMS managed keys.
Copy link
Member

Choose a reason for hiding this comment

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

with KMS managed keys -> with AWS managed keys

Copy link
Collaborator

@gregschohn gregschohn Sep 27, 2024

Choose a reason for hiding this comment

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

I would rewrite the section to drop what the source and target clusters might be using.

Each of the AWS services that are interacting with data will encrypt all data being stored at rest. While those services can support performing the encryption via a KMS Key, the CDK deployment option doesn't have the ability to set a customer key for any of those services. That will leave all of the data at rest encrypted, but not under the control of a customer's KMS Key. See the links below for more details on forthcoming support:

https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html
#1026

Signed-off-by: Mikayla Thompson <[email protected]>
Signed-off-by: Mikayla Thompson <[email protected]>
SECURITY.md Outdated
Each of the AWS services that are interacting with data will encrypt all data being stored at rest. While the services themselves can support performing the encryption via a KMS Key, the CDK deployment option of Migration Assistant doesn't have the ability to set a customer key for any of those services. That will leave all of the data at rest encrypted, but not under the control of a customer's KMS Key. See the links below for more details on forthcoming support:

https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html
#1026
Copy link
Member

Choose a reason for hiding this comment

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

What is 1026 here?

Signed-off-by: Mikayla Thompson <[email protected]>
Each of the AWS services that are interacting with data will encrypt all data being stored at rest. While the services themselves can support performing the encryption via a KMS Key, the CDK deployment option of Migration Assistant doesn't have the ability to set a customer key for any of those services. That will leave all of the data at rest encrypted, but not under the control of a customer's KMS Key. See the links below for more details on forthcoming support:

https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html
[Issue #1026](https://github.com/opensearch-project/opensearch-migrations/issues/1026)
Copy link
Member

Choose a reason for hiding this comment

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

nit: shows up on the same line, having another newline may be clearer

@mikaylathompson mikaylathompson merged commit a529050 into opensearch-project:main Sep 30, 2024
14 checks passed
@mikaylathompson mikaylathompson deleted the note-about-customer-keys branch September 30, 2024 17:03
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.

3 participants