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

Encryption of shoot secrets in etcd #1066

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@michael-engler
Copy link

commented Jun 4, 2019

What this PR does / why we need it:
In a nutshell, this PR addresses an automatic encryption of shoot secrets according to https://kubernetes.io/docs/tasks/administer-cluster/encrypt-data/

The protection of shoot cluster secrets against eavesdropping and unauthorised access is of utmost importance. As of today, Gardener already protects the transmission of secrets by applying transport encryption via TLS between the API server and the kubelet(s). While encryption in transit is already addressed, this PR provides a data-at-rest protection of Kubernetes secrets. More clearly, shoot secrets that are stored in etcd need to be encrypted prior to storing them in etcd and need to be decrypted after retrieving them from etcd. From Kubernetes version 1.13.0 onward, the kube-apiserver process supports the encryption of secret data at rest.

This PR uses the Kubernetes feature automatically for shoots created by Gardener. Each shoot cluster with Kubernetes version 1.13.0 receives an EncryptionConfiguration with a random shoot-specific encryption key. The EncryptionConfiguration is realised itself as a Kubernetes secret which is created during the shoot reconciliation in the seed cluster (in the shoot namespace) and is mounted to the API server pods. All shoot secrets are rewritten to ensure that they are encrypted as specified in the EncryptionConfiguration.

More clearly, the EncryptionConfiguration is written during the first reconciliation run in a passive state (meaning that "identity" is the first entry in the list of encryption providers, and "aescbc" is the second entry). In this state, the key is already known to each apiserver process (and could be used for decrypting a secret that was encrypted with that key) but secrets are not yet actively encrypted. The next subsequent reconciliation run re-orders the encryption providers and thereby activates the encryption.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:


@michael-engler michael-engler requested a review from gardener/gardener-maintainers as a code owner Jun 4, 2019

Show resolved Hide resolved pkg/controllermanager/controller/shoot/shoot_control_reconcile.go Outdated
Show resolved Hide resolved pkg/controllermanager/controller/shoot/shoot_control_reconcile.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/util.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/util_test.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/util_test.go Outdated
Show resolved Hide resolved pkg/operation/hybridbotanist/controlplane.go Outdated
Show resolved Hide resolved ...ts/seed-controlplane/charts/kube-apiserver/templates/kube-apiserver.yaml

@rfranzke rfranzke self-assigned this Jun 5, 2019

@adracus
Copy link
Member

left a comment

Quick remark: Please don't reply 'Ok' to items you resolved - We get an email for each of these replies. Rather just resolve the conversation. We're still able to see what you've resolved and what not.

Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionkeys_test.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionkeys_test.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/controllermanager/controller/shoot/shoot_control_reconcile.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionkeys.go
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionkeys_test.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/etcdencryption_suite_test.go
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionconfiguration_test.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated

@michael-engler michael-engler force-pushed the michael-engler:EtcdEncryption branch 3 times, most recently from 9357a8f to 6e99e6d Jun 26, 2019

@michael-engler michael-engler force-pushed the michael-engler:EtcdEncryption branch 3 times, most recently from 8ac6911 to e8169e8 Jul 4, 2019

func (b *Botanist) writeEncryptionConfigurationSecretToSeed(ctx context.Context, ecYamlBytes []byte, forcePlaintextSecrets bool) error {
secretObj := b.createEncryptionConfigurationSecret(common.EtcdEncryptionSecretName, b.Shoot.SeedNamespace, ecYamlBytes, forcePlaintextSecrets)
return kutil.CreateOrUpdate(ctx, b.K8sSeedClient.Client(), secretObj, func() error {
secretObj.Data[common.EtcdEncryptionSecretFileName] = ecYamlBytes

This comment has been minimized.

Copy link
@adracus

adracus Jul 4, 2019

Member

This code is duplicative: You've got the same thing above. I'd suggest having a method called UpdateETCDEncryptionConfigurationSecret that takes in a secret and the required data (here I'd also suggest to hand in the encryption configuration object and not the ecYamlBytes) and then update the secret. Then you can re-use this function here as well.

This comment has been minimized.

Copy link
@michael-engler

michael-engler Jul 5, 2019

Author

I encapsulated the common parts already in createEncryptionConfigurationSecret. The differences (e.g. the OwnerReference, names, namespaces) are in writeEncryptionConfigurationSecretToSeed respectively writeEncryptionConfigurationSecretToGarden.

This comment has been minimized.

Copy link
@adracus

adracus Jul 8, 2019

Member

yes, that's why I'm recommending to factor out the common part into one method that can then be used by both.

This comment has been minimized.

Copy link
@michael-engler

michael-engler Jul 9, 2019

Author

there are no common parts left

*metav1.NewControllerRef(b.Shoot.Info, gardenv1beta1.SchemeGroupVersion.WithKind("Shoot")),
}
return kutil.CreateOrUpdate(ctx, b.K8sGardenClient.Client(), secretObj, func() error {
secretObj.Data[common.EtcdEncryptionSecretFileName] = ecYamlBytes

This comment has been minimized.

Copy link
@adracus

adracus Jul 4, 2019

Member

This would also benefit from the function mentioned above.

Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionconfiguration.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionkeys.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionkeys.go
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated

@michael-engler michael-engler force-pushed the michael-engler:EtcdEncryption branch 2 times, most recently from ddf491c to 880fd72 Jul 4, 2019

Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go Outdated
Show resolved Hide resolved pkg/operation/botanist/etcdencryption.go
func (b *Botanist) writeEncryptionConfigurationSecretToSeed(ctx context.Context, ecYamlBytes []byte, forcePlaintextSecrets bool) error {
secretObj := b.createEncryptionConfigurationSecret(common.EtcdEncryptionSecretName, b.Shoot.SeedNamespace, ecYamlBytes, forcePlaintextSecrets)
return kutil.CreateOrUpdate(ctx, b.K8sSeedClient.Client(), secretObj, func() error {
secretObj.Data[common.EtcdEncryptionSecretFileName] = ecYamlBytes

This comment has been minimized.

Copy link
@adracus

adracus Jul 8, 2019

Member

yes, that's why I'm recommending to factor out the common part into one method that can then be used by both.

Show resolved Hide resolved pkg/operation/botanist/waiter.go
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionconfiguration.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionconfiguration.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionconfiguration.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionconfiguration_test.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/encryptionkeys.go Outdated
Show resolved Hide resolved pkg/operation/etcdencryption/types.go Outdated

@michael-engler michael-engler force-pushed the michael-engler:EtcdEncryption branch 4 times, most recently from f3965ac to c6343df Jul 9, 2019

@michael-engler michael-engler force-pushed the michael-engler:EtcdEncryption branch from c6343df to aca6be1 Jul 11, 2019

@michael-engler

This comment has been minimized.

Copy link
Author

commented Jul 16, 2019

The EncryptionConfiguration is written as secret to the seed and additionally is synced to the Garden. Having the key stored in two separate places was done on purpose to alleviate the risk of loosing an EncryptionConfiguration key as long as no current backup of the seed's etcd has been done. This approach was recommended by @marwinski and discussed with @rfranzke .

I understand that you @adracus are thinking about removing this part ... and of course the code gets simpler if this functionality is removed again ... and of course it is absolutely your choice as Gardener code owner. But I would like to ask you to weigh this code simplification against a potentially higher risk of loosing an encryption key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.