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.

Sign up for GitHub

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

Jump to bottom

security: Encrypt region boundary keys, Part 4 - KMS #3141

Merged
merged 54 commits into from Nov 13, 2020
Merged

security: Encrypt region boundary keys, Part 4 - KMS #3141

merged 54 commits into from Nov 13, 2020

Conversation

yiwu-arbug
Copy link
Contributor

@yiwu-arbug yiwu-arbug commented Nov 3, 2020 •
edited

What problem does this PR solve?

This is part 4 for adding TDE support to PD. pingcap/tidb#18262 This PR supports AWS KMS as master key.

What is changed and how it works?

The previous PR in the series support storing master key as a file. This is only suitable for testing. For production this is insecure. This PR supports using AWS KMS as master key. KMS is a service holds an encryption key (called a CMK), and provide API to encrypt and decrypt data using the encryption key, without exposing the key itself. We only use two of its interfaces:

  GenerateDataKey() (plaintextKey, ciphertextKey),
  Decrypt(ciphertextKey) plaintextKey,

GenerateDataKey() generates an encryption key from the CMK, returning both the plaintext key, and the key after encrypted by the CMK (the ciphertext key). Decrypt() can then used to decrypt the ciphertext key. We could use the KMS data key as our data key, but on PD restart we would need to decrypt the data keys one by one, and AWS KMS doesn't provide a batch API. Instead we use the KMS data key as our master key. So here's another layer of envelope encryption.

KMS CMK (hold by KMS, never exposed to us)
↓ encrypts
KMS data key (a.k.a PD master key)
↓ encrypts
PD data key

When PD persist data keys (after key rotation):

  1. Call GenerateDataKey to obtain a new KMS data key (plaintext and ciphertext key pair), use it to encrypt the PD data keys
  2. Store the encrypted data keys to etcd, along with the ciphertext key returned by KMS. discard the plaintext key.

When PD restart, or is notified of change to the keys through watcher:

  1. load the encrypted data keys from etcd, which contains the ciphertext key.
  2. call Decrypt to restore plaintext KMS data key from the ciphertext key.
  3. decrypt PD data keys using the plaintext key.

This PR only support AWS KMS, but in the future we need to support other KMS services (e.g. GCP KMS). The vender field in encryptionpb.MasterKey is used for forward compatibility - we will fail if the field is not "AWS".

Check List

Tests

  • New unit test
  • Manual test with sysbench on a tidb cluster, with KMS key.

Related changes

Release note

  • No release note

@yiwu-arbug
Encrypt region boundary keys, Part 2 - server changes
20c682d 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
update errno and config template
b6f7ba0 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix typo
0bf0304 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix tests
23954db 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix tests
2bad0b9 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix tests
f88d26a 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix tests
6f6d93a 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
address comment in tikv#2931
928de26 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix loadRegion
9b6bcfd 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
rename encryption_key_manager package
bdd5de4 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
Merge branch 'master' into enc_server
bbdb8ef
@yiwu-arbug
fix lint
90de5ef 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix lint
c16ecd8 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix comments
2428248 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix comment
ce83a42 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
use option pattern
073f13e 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
move loadRegion and saveRegion
c35f948 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
Merge branch 'master' into enc_server
06d8963
@yiwu-arbug
revert changes
551b3f9 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix doc
7051407 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
address comment
7201c37 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
Merge branch 'master' into enc_server
c9eb341
@yiwu-arbug
Merge remote-tracking branch 'origin/master' into enc_server
39f147e
@yiwu-arbug
Merge branch 'master' into enc_server
12f03c4
@yiwu-arbug
Merge branch 'master' into enc_server
f951278
@yiwu-arbug
key manager
ee2e0cd 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
add test and refactor
9e753b4 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
make EncryptionKeysPath a const
a6d1359 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix region_crypter key manager nil check
103a612 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
save conflict test
e9d810d 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
address comment
9a6020f 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
Merge branch 'master' into enc_kms
5a69760
@yiwu-arbug
fix error type
0321a80 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
fix test
312bb48 
Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
Merge branch 'master' into enc_kms
e90b370
@HunDunDM
Copy link
Member

HunDunDM commented Nov 9, 2020

/run-all-tests

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 9, 2020
Yisaer
Yisaer reviewed Nov 12, 2020
View reviewed changes
Copy link
Contributor

@Yisaer Yisaer left a comment

Choose a reason for hiding this comment

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

LGTM

@Yisaer
Copy link
Contributor

Yisaer commented Nov 12, 2020

/merge

@ti-chi-bot
Copy link
Member

@Yisaer: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the tidb-community-bots/prow-config repository.

@ti-chi-bot
Copy link
Member

@Yisaer: adding 'status/can-merge' to this PR must have 2 LGTMs

In response to this:

/merge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the tidb-community-bots/prow-config repository.

Yisaer
Yisaer approved these changes Nov 12, 2020
View reviewed changes
Copy link
Contributor

@Yisaer Yisaer left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 12, 2020
@Yisaer
Copy link
Contributor

Yisaer commented Nov 12, 2020

/merge

@ti-chi-bot
Copy link
Member

@Yisaer: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the tidb-community-bots/prow-config repository.

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 12, 2020
@ti-chi-bot
Copy link
Member

Can merge label has been added.

Git tree hash: 7480d5d

@yiwu-arbug
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

@yiwu-arbug: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the tidb-community-bots/prow-config repository.

@ti-chi-bot
Copy link
Member

@yiwu-arbug: you cannot merge your own PR.

In response to this:

/merge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the tidb-community-bots/prow-config repository.

@HunDunDM
Copy link
Member

/run-integration-ddl-test

@HunDunDM
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

@HunDunDM: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the tidb-community-bots/prow-config repository.

@HunDunDM
Copy link
Member

/run-integration-lightning-test

@ti-chi-bot ti-chi-bot merged commit 3c62563 into tikv:master Nov 13, 2020
@yiwu-arbug yiwu-arbug deleted the enc_kms branch November 13, 2020 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yisaer Yisaer Yisaer approved these changes

@HunDunDM HunDunDM HunDunDM approved these changes

Assignees
No one assigned
Labels
component/security Security logic. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@yiwu-arbug @HunDunDM @Yisaer @ti-chi-bot @nolouch