-
Notifications
You must be signed in to change notification settings - Fork 0
WIP: Kms encryption #1
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
Conversation
also split the Proposal into sub-sections.
| replaces: | ||
| - "" | ||
| superseded-by: | ||
| - "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
openshift#1682 needs to be in here
| ## Summary | ||
|
|
||
| Provide a user-configurable interface to support encryption of data stored in | ||
| etcd using a supported Key Management Service (KMS). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to link https://kubernetes.io/docs/tasks/administer-cluster/kms-provider/ in here.
| However, aescbc and aesgcm, which are supported encryption technologies today | ||
| available in OpenShift do not protect against online host compromise i.e. in | ||
| such cases, attackers can decrypt encrypted data from etcd using local keys, | ||
| KMS managed keys protects against such scenarios. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
| KMS managed keys protects against such scenarios. | |
| KMS managed keys protects against such scenarios, since they are stored and managed by externally |
| Status | ||
| * As a cluster admin, I want to be able to switch to a different KMS plugin, | ||
| i.e. from AWS to a pre-installed Vault, by performing a single configuration | ||
| change without needing to perform any other manual intervention |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| change without needing to perform any other manual intervention | |
| change without needing to perform any other manual intervention and migration |
| ### Goals | ||
|
|
||
| * Users have an easy to use interface to configure KMS encryption | ||
| * Users will configure OpenShift clusters to use a specific KMS key, created by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Users will configure OpenShift clusters to use a specific KMS key, created by | |
| * Users will configure OpenShift clusters to use a predefined set of KMS providers |
|
|
||
| The keyController will continue managing encryption key secrets as it does | ||
| today. The difference is that for the KMS encryption provider, the encryption | ||
| key secret contents will be empty. This secret must be empty because when the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secret contents will not be empty. It will store the hash of kms configuration (including the status field as well) to detect any changes.
This will also resolve your TODO comment:
TODO: how do KMS plugins determine `key_id`? It mustn't be the same as i.e.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secret contents will not be empty.
The encryption key secret contents will be empty in tech-preview, and we'll possibly write some contents in it for the GA release... no?
Even if this is the case though I still need to update this paragraph to clarify this.
It will store the hash of kms configuration (including the status field as well) to detect any changes.
Is this so we can detect rotation of key materials (i.e when key is rotated without key_id change)? Otherwise I don't think we need it 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 2 dimensions.
- key_controller needs to detect the changes in KMS Config such as ARN, region, mode, etc. https://github.com/openshift/api/blob/8691c3014652a8ced9d3304efb4a0cd76c3a35b2/config/v1/types_kmsencryption.go#L7-L22 in order to trigger new secret creation. (key_controller achieves this for other encryption types by checking the key. But in KMS case, there is no key).
- key_controller needs to detect the changes in KMS Status https://github.com/kubernetes/kubernetes/blob/ae60c554f8cc073e3ae78e004208ce92c6e4cae4/staging/src/k8s.io/kms/pkg/service/interface.go#L49 to detect the changes in KeyID (which means key is rotated).
We'll merge them somehow to detect the changes. In any case, secret won't be empty.
| #### Key Rotation and Data Migration | ||
|
|
||
| Keys can be rotated in the following ways: | ||
| * Automatic periodic key rotation by the KMS, following user provided rotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned that automatic key rotation does not change the overall kms configuration. Therefore, we can't detect key is rotated. Other than that, this makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I need to update the EP to cover this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, we agreed upon how we'll move forward. So I'm up to you about expressing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and thank you for your comments :)
| Keys can be rotated in the following ways: | ||
| * Automatic periodic key rotation by the KMS, following user provided rotation | ||
| policy in the KMS itself | ||
| * The user creates a new KMS key, and updates the KMS section of the APIServer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * The user creates a new KMS key, and updates the KMS section of the APIServer | |
| * The user creates a new KMS key, and updates the KMS section of the APIServer (without deleting the old one) |
| configured with the previous key. The two pods must run in parallel until | ||
| migration to the new key is complete. | ||
|
|
||
| TODO: how do KMS plugins determine `key_id`? It mustn't be the same as i.e. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment above.
| - Describe which OCP teams are likely to be called upon in case of escalation with one of the failure modes | ||
| and add them as reviewers to this enhancement. | ||
|
|
||
| ## Support Procedures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed previously, we need a section about listing the failures and the actions.
|
I'm closing this, as the main purpose is to drop comments. |
|
I'll review your comments today. For posterity, here's the draft PR where we'll continue the conversation: openshift#1872 |
SSIA