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 KMS plugin registry #49742

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -12,14 +12,17 @@ go_library(
name = "go_default_library",
srcs = [
"config.go",
"plugins.go",
"types.go",
],
tags = ["automanaged"],
deps = [
"//vendor/github.com/ghodss/yaml:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage/value:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage/value/encrypt/aes:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage/value/encrypt/envelope:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage/value/encrypt/identity:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage/value/encrypt/secretbox:go_default_library",
],
Expand All @@ -33,5 +36,6 @@ go_test(
deps = [
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage/value:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage/value/encrypt/envelope:go_default_library",
],
)
Expand Up @@ -30,14 +30,17 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apiserver/pkg/storage/value"
aestransformer "k8s.io/apiserver/pkg/storage/value/encrypt/aes"
"k8s.io/apiserver/pkg/storage/value/encrypt/envelope"
"k8s.io/apiserver/pkg/storage/value/encrypt/identity"
"k8s.io/apiserver/pkg/storage/value/encrypt/secretbox"
)

const (
aesCBCTransformerPrefixV1 = "k8s:enc:aescbc:v1:"
aesGCMTransformerPrefixV1 = "k8s:enc:aesgcm:v1:"
secretboxTransformerPrefixV1 = "k8s:enc:secretbox:v1:"
aesCBCTransformerPrefixV1 = "k8s:enc:aescbc:v1:"
aesGCMTransformerPrefixV1 = "k8s:enc:aesgcm:v1:"
secretboxTransformerPrefixV1 = "k8s:enc:secretbox:v1:"
kmsTransformerPrefixV1 = "k8s:enc:kms:v1:"
cloudProvidedKMSTransformerPrefixV1 = "k8s:enc:cloudprovidedkms:v1:"
)

// GetTransformerOverrides returns the transformer overrides by reading and parsing the encryption provider configuration file
Expand Down Expand Up @@ -144,6 +147,38 @@ func GetPrefixTransformers(config *ResourceConfig) ([]value.PrefixTransformer, e
found = true
}

if provider.KMS != nil {
if found == true {
return nil, fmt.Errorf("more than one provider specified in a single element, should split into different list elements")
}
f, err := os.Open(provider.KMS.ConfigFile)
Copy link
Member

Choose a reason for hiding this comment

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

It feels wrong that we are trying to open/read the file here. It would seem better to just pass this location down to the factory and let it do the read. Especially as in something like a hybrid case this could be a key into a config server or be a file+coordinate into a larger config file. We should need to know or care about such things. However if we insist on opening+reading the file then we end up having to care about these things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The admissions factory used this format, which is why I stuck with this. We can change this in a follow-up.
Once this PR is merged, I'll open a new one to change this and remove the reflect mentioned in your 2nd comment, along with some other cleanup.

Copy link
Contributor

Choose a reason for hiding this comment

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

It feels wrong that we are trying to open/read the file here. It would seem better to just pass this location down to the factory and let it do the read. Especially as in something like a

To make this API congruent to the admission config, there will be a choice of separate file or embedded content depending the sensitivity (which we don't know since its pluggable) of the config in question.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An approach which I've proposed earlier, and I've seen used in the cloudproviders package:
Have a field config map[string]interface{} on the KMSConfig struct. Send it to the plugin factory. The plugin factory would use the mapstructure library to parse it into a struct safely. This library is used in certain cloudproviders like openstack.
So this way you can support both config: and configfile: attributes on the kms configuration.

May not be acceptable though, due to a point in the flow where the config is unstructured (it will still be read in a structured form later on). Just putting this out here.

Copy link
Member

Choose a reason for hiding this comment

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

The reader interface lets you load a structured config from an external file or an inlined config. All our config files should be structured and versioned (see scheduler config, ABAC config, admission config, etc)

if err != nil {
return nil, fmt.Errorf("error opening KMS provider configuration file %q: %v", provider.KMS.ConfigFile, err)
}
defer f.Close()
envelopeService, pluginFound, err := KMSPluginRegistry.getPlugin(provider.KMS.Name, f)
if err != nil {
return nil, fmt.Errorf("could not configure KMS plugin %q, %v", provider.KMS.Name, err)
}
if pluginFound == false {
return nil, fmt.Errorf("KMS plugin %q not found", provider.KMS.Name)
}
transformer, err = getEnvelopePrefixTransformer(provider.KMS.CoreKMSConfig, envelopeService, kmsTransformerPrefixV1)
found = true
}

if provider.CloudProvidedKMS != nil {
if found == true {
return nil, fmt.Errorf("more than one provider specified in a single element, should split into different list elements")
}
envelopeService, err := KMSPluginRegistry.getCloudProvidedPlugin(provider.CloudProvidedKMS.Name)
if err != nil {
return nil, fmt.Errorf("could not configure Cloud-Provided KMS plugin %q, %v", provider.CloudProvidedKMS.Name, err)
}
transformer, err = getEnvelopePrefixTransformer(provider.CloudProvidedKMS.CoreKMSConfig, envelopeService, cloudProvidedKMSTransformerPrefixV1)
found = true
}

if err != nil {
return result, err
}
Expand Down Expand Up @@ -258,3 +293,16 @@ func GetSecretboxPrefixTransformer(config *SecretboxConfig) (value.PrefixTransfo
}
return result, nil
}

// getEnvelopePrefixTransformer returns a prefix transformer from the provided config.
// envelopeService is used as the root of trust.
func getEnvelopePrefixTransformer(config *CoreKMSConfig, envelopeService envelope.Service, prefix string) (value.PrefixTransformer, error) {
envelopeTransformer, err := envelope.NewEnvelopeTransformer(envelopeService, config.CacheSize, aestransformer.NewCBCTransformer)
if err != nil {
return value.PrefixTransformer{}, err
}
return value.PrefixTransformer{
Transformer: envelopeTransformer,
Prefix: []byte(prefix + config.Name + ":"),
}, nil
}
Expand Up @@ -18,18 +18,27 @@ package encryptionconfig

import (
"bytes"
"encoding/base64"
"fmt"
"io"
"os"
"strings"
"testing"

"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apiserver/pkg/storage/value"
"k8s.io/apiserver/pkg/storage/value/encrypt/envelope"
)

const (
sampleText = "abcdefghijklmnopqrstuvwxyz"

sampleContextText = "0123456789"

// Modify these in all configurations if changed
testEnvelopeServiceConfigPath = "testproviderconfig"
testEnvelopeServiceProviderName = "testprovider"

correctConfigWithIdentityFirst = `
kind: EncryptionConfig
apiVersion: v1
Expand All @@ -45,6 +54,13 @@ resources:
secret: c2VjcmV0IGlzIHNlY3VyZQ==
- name: key2
secret: dGhpcyBpcyBwYXNzd29yZA==
- kms:
name: testprovider
configfile: testproviderconfig
cachesize: 10
- cloudprovidedkms:
name: testprovider
cachesize: 10
- aescbc:
keys:
- name: key1
Expand Down Expand Up @@ -74,12 +90,19 @@ resources:
keys:
- name: key1
secret: YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXoxMjM0NTY=
- kms:
name: testprovider
configfile: testproviderconfig
cachesize: 10
- aescbc:
keys:
- name: key1
secret: c2VjcmV0IGlzIHNlY3VyZQ==
- name: key2
secret: dGhpcyBpcyBwYXNzd29yZA==
- cloudprovidedkms:
name: testprovider
cachesize: 10
- identity: {}
`

Expand All @@ -96,7 +119,14 @@ resources:
secret: c2VjcmV0IGlzIHNlY3VyZQ==
- name: key2
secret: dGhpcyBpcyBwYXNzd29yZA==
- kms:
name: testprovider
configfile: testproviderconfig
cachesize: 10
- identity: {}
- cloudprovidedkms:
name: testprovider
cachesize: 10
- secretbox:
keys:
- name: key1
Expand All @@ -120,12 +150,19 @@ resources:
keys:
- name: key1
secret: YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXoxMjM0NTY=
- cloudprovidedkms:
name: testprovider
cachesize: 10
- aescbc:
keys:
- name: key1
secret: c2VjcmV0IGlzIHNlY3VyZQ==
- name: key2
secret: dGhpcyBpcyBwYXNzd29yZA==
- kms:
name: testprovider
configfile: testproviderconfig
cachesize: 10
- identity: {}
- aesgcm:
keys:
Expand All @@ -135,6 +172,75 @@ resources:
secret: dGhpcyBpcyBwYXNzd29yZA==
`

correctConfigWithKMSFirst = `
kind: EncryptionConfig
apiVersion: v1
resources:
- resources:
- secrets
providers:
- kms:
name: testprovider
configfile: testproviderconfig
cachesize: 10
- secretbox:
keys:
- name: key1
secret: YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXoxMjM0NTY=
- aescbc:
keys:
- name: key1
secret: c2VjcmV0IGlzIHNlY3VyZQ==
- name: key2
secret: dGhpcyBpcyBwYXNzd29yZA==
- identity: {}
- cloudprovidedkms:
name: testprovider
cachesize: 10
- aesgcm:
keys:
- name: key1
secret: c2VjcmV0IGlzIHNlY3VyZQ==
- name: key2
secret: dGhpcyBpcyBwYXNzd29yZA==
`

correctConfigWithCloudProvidedKMSFirst = `
kind: EncryptionConfig
apiVersion: v1
resources:
- resources:
- secrets
providers:
- cloudprovidedkms:
name: testprovider
cachesize: 10
- kms:
name: testprovider
configfile: testproviderconfig
cachesize: 10
- secretbox:
keys:
- name: key1
secret: YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXoxMjM0NTY=
- aescbc:
keys:
- name: key1
secret: c2VjcmV0IGlzIHNlY3VyZQ==
- name: key2
secret: dGhpcyBpcyBwYXNzd29yZA==
- identity: {}
- cloudprovidedkms:
name: testprovider
cachesize: 10
- aesgcm:
keys:
- name: key1
secret: c2VjcmV0IGlzIHNlY3VyZQ==
- name: key2
secret: dGhpcyBpcyBwYXNzd29yZA==
`

incorrectConfigNoSecretForKey = `
kind: EncryptionConfig
apiVersion: v1
Expand Down Expand Up @@ -165,11 +271,48 @@ resources:
`
)

// testEnvelopeService is a mock envelope service which can be used to simulate remote Envelope services
// for testing of the envelope transformer with other transformers.
type testEnvelopeService struct {
disabled bool
}

func (t *testEnvelopeService) Decrypt(data string) ([]byte, error) {
if t.disabled {
return nil, fmt.Errorf("Envelope service was disabled")
}
return base64.StdEncoding.DecodeString(data)
}

func (t *testEnvelopeService) Encrypt(data []byte) (string, error) {
if t.disabled {
return "", fmt.Errorf("Envelope service was disabled")
}
return base64.StdEncoding.EncodeToString(data), nil
}

func (t *testEnvelopeService) SetDisabledStatus(status bool) {
t.disabled = status
}

var _ envelope.Service = &testEnvelopeService{}

func TestEncryptionProviderConfigCorrect(t *testing.T) {
// Creates two transformers with different ordering of identity and AES transformers.
// Transforms data using one of them, and tries to untransform using both of them.
// Repeats this for both the possible combinations.
os.OpenFile(testEnvelopeServiceConfigPath, os.O_CREATE, 0666)
defer os.Remove(testEnvelopeServiceConfigPath)
KMSPluginRegistry.Register(testEnvelopeServiceProviderName, func(config io.Reader) (envelope.Service, error) {
return &testEnvelopeService{}, nil
})
KMSPluginRegistry.RegisterCloudProvidedKMSPlugin(func(name string) (envelope.Service, error) {
if name == testEnvelopeServiceProviderName {
return &testEnvelopeService{}, nil
}
return nil, fmt.Errorf("no such cloud provided KMS plugin registered: %q", name)
})

// Creates compound/prefix transformers with different ordering of available transformers.
// Transforms data using one of them, and tries to untransform using the others.
// Repeats this for all possible combinations.
identityFirstTransformerOverrides, err := ParseEncryptionConfiguration(strings.NewReader(correctConfigWithIdentityFirst))
if err != nil {
t.Fatalf("error while parsing configuration file: %s.\nThe file was:\n%s", err, correctConfigWithIdentityFirst)
Expand All @@ -190,11 +333,23 @@ func TestEncryptionProviderConfigCorrect(t *testing.T) {
t.Fatalf("error while parsing configuration file: %s.\nThe file was:\n%s", err, correctConfigWithSecretboxFirst)
}

kmsFirstTransformerOverrides, err := ParseEncryptionConfiguration(strings.NewReader(correctConfigWithKMSFirst))
if err != nil {
t.Fatalf("error while parsing configuration file: %s.\nThe file was:\n%s", err, correctConfigWithKMSFirst)
}

cloudProvidedKMSFirstTransformerOverrides, err := ParseEncryptionConfiguration(strings.NewReader(correctConfigWithCloudProvidedKMSFirst))
if err != nil {
t.Fatalf("error while parsing configuration file: %s.\nThe file was:\n%s", err, correctConfigWithCloudProvidedKMSFirst)
}

// Pick the transformer for any of the returned resources.
identityFirstTransformer := identityFirstTransformerOverrides[schema.ParseGroupResource("secrets")]
aesGcmFirstTransformer := aesGcmFirstTransformerOverrides[schema.ParseGroupResource("secrets")]
aesCbcFirstTransformer := aesCbcFirstTransformerOverrides[schema.ParseGroupResource("secrets")]
secretboxFirstTransformer := secretboxFirstTransformerOverrides[schema.ParseGroupResource("secrets")]
kmsFirstTransformer := kmsFirstTransformerOverrides[schema.ParseGroupResource("secrets")]
cloudProvidedKMSFirstTransformer := cloudProvidedKMSFirstTransformerOverrides[schema.ParseGroupResource("secrets")]

context := value.DefaultContext([]byte(sampleContextText))
originalText := []byte(sampleText)
Expand All @@ -207,6 +362,8 @@ func TestEncryptionProviderConfigCorrect(t *testing.T) {
{aesCbcFirstTransformer, "aesCbcFirst"},
{secretboxFirstTransformer, "secretboxFirst"},
{identityFirstTransformer, "identityFirst"},
{kmsFirstTransformer, "kmsFirst"},
{cloudProvidedKMSFirstTransformer, "cloudProvidedKMSFirst"},
}

for _, testCase := range transformers {
Expand Down