From 231c3030ed26f95113dec682d7ebf2c9ef7ce16f Mon Sep 17 00:00:00 2001 From: Glen Johnson Date: Mon, 20 Mar 2023 13:16:39 -0600 Subject: [PATCH] Decode base64 secrets --- pkg/log/messages/error_messages.go | 3 + .../provide_conjur_secrets.go | 59 +++++++++---- .../provide_conjur_secrets_test.go | 87 +++++++++++++------ 3 files changed, 105 insertions(+), 44 deletions(-) diff --git a/pkg/log/messages/error_messages.go b/pkg/log/messages/error_messages.go index 40a81b9d..7dbff696 100644 --- a/pkg/log/messages/error_messages.go +++ b/pkg/log/messages/error_messages.go @@ -84,3 +84,6 @@ const CSPFK058E string = "CSPFK058E Could not rename temporary file '%s' to '%s' const CSPFK059E string = "CSPFK059E Could not delete temporary file '%s'. Truncated file." const CSPFK060E string = "CSPFK060E Could not delete temporary file '%s'. File may be left on disk." const CSPFK061E string = "CSPFK061E Could not write content to temporary file for '%s'" + +// Secrets Decoding +const CSPFK064E string = "CSPFK064E Failed to decode secret '%s' with '%s' content-type. Reason: %s" diff --git a/pkg/secrets/k8s_secrets_storage/provide_conjur_secrets.go b/pkg/secrets/k8s_secrets_storage/provide_conjur_secrets.go index 4eefef8a..f08819a3 100644 --- a/pkg/secrets/k8s_secrets_storage/provide_conjur_secrets.go +++ b/pkg/secrets/k8s_secrets_storage/provide_conjur_secrets.go @@ -3,6 +3,7 @@ package k8ssecretsstorage import ( "bytes" "context" + "encoding/base64" "fmt" "strings" @@ -344,25 +345,7 @@ func (p K8sProvider) updateRequiredK8sSecrets( spanCtx, span := tracer.Start(p.traceContext, "Update K8s Secrets") defer span.End() - // Create a map of entries to be added to the 'Data' fields of each - // K8s Secret. Each entry will map an application secret name to - // a value retrieved from Conjur. - newSecretData := map[string]map[string][]byte{} - for variableID, secretValue := range conjurSecrets { - dests := p.secretsState.updateDestinations[variableID] - for _, dest := range dests { - k8sSecretName := dest.k8sSecretName - secretName := dest.secretName - // If there are no data entries for this K8s Secret yet, initialize - // its map of data entries. - if newSecretData[k8sSecretName] == nil { - newSecretData[k8sSecretName] = map[string][]byte{} - } - newSecretData[k8sSecretName][secretName] = secretValue - } - // Null out the secret value - conjurSecrets[variableID] = []byte{} - } + newSecretData := p.createSecretData(conjurSecrets) // Update K8s Secrets with the retrieved Conjur secrets for k8sSecretName, secretData := range newSecretData { @@ -401,3 +384,41 @@ func (p K8sProvider) updateRequiredK8sSecrets( return updated, nil } + +// createSecretData creates a map of entries to be added to the 'Data' fields +// of each K8s Secret. Each entry will map an application secret name to a +// value retrieved from Conjur. If a secret has a 'base64' content type, the +// resulting secret value will be decoded. +func (p K8sProvider) createSecretData(conjurSecrets map[string][]byte) map[string]map[string][]byte { + secretData := map[string]map[string][]byte{} + for variableID, secretValue := range conjurSecrets { + dests := p.secretsState.updateDestinations[variableID] + for _, dest := range dests { + k8sSecretName := dest.k8sSecretName + secretName := dest.secretName + // If there are no data entries for this K8s Secret yet, initialize + // its map of data entries. + if secretData[k8sSecretName] == nil { + secretData[k8sSecretName] = map[string][]byte{} + } + + // Check if the secret value should be decoded in this K8s Secret + if dest.contentType == "base64" { + decodedSecretValue, err := base64.StdEncoding.DecodeString(string(secretValue)) + if err != nil { + // Log the error as a warning but still provide the original secret value + p.log.warn(messages.CSPFK064E, secretName, dest.contentType, err.Error()) + secretData[k8sSecretName][secretName] = secretValue + } else { + secretData[k8sSecretName][secretName] = decodedSecretValue + } + } else { + secretData[k8sSecretName][secretName] = secretValue + } + } + // Null out the secret value + conjurSecrets[variableID] = []byte{} + } + + return secretData +} diff --git a/pkg/secrets/k8s_secrets_storage/provide_conjur_secrets_test.go b/pkg/secrets/k8s_secrets_storage/provide_conjur_secrets_test.go index 0a4f4eb4..a384f475 100644 --- a/pkg/secrets/k8s_secrets_storage/provide_conjur_secrets_test.go +++ b/pkg/secrets/k8s_secrets_storage/provide_conjur_secrets_test.go @@ -139,17 +139,28 @@ func assertErrorLogged(msg string, args ...interface{}) assertFunc { } } -func assertInfoLogged(expected bool, msg string, args ...interface{}) assertFunc { +func assertLogged(expected bool, logLevel string, msg string, args ...interface{}) assertFunc { return func(t *testing.T, mocks testMocks, updated bool, err error, desc string) { - infoStr := fmt.Sprintf(msg, args...) + logStr := fmt.Sprintf(msg, args...) var logDesc string if expected { - logDesc = ", expected info log to contain: " + logDesc = fmt.Sprintf(", expected %s log to contain: ", logLevel) } else { - logDesc = ", expected info log NOT to contain: " + logDesc = fmt.Sprintf(", expected %s log NOT to contain: ", logLevel) + } + newDesc := desc + logDesc + logStr + switch logLevel { + case "debug": + assert.Equal(t, expected, mocks.logger.DebugWasLogged(logStr), newDesc) + case "info": + assert.Equal(t, expected, mocks.logger.InfoWasLogged(logStr), newDesc) + case "warn": + assert.Equal(t, expected, mocks.logger.WarningWasLogged(logStr), newDesc) + case "error": + assert.Equal(t, expected, mocks.logger.ErrorWasLogged(logStr), newDesc) + default: + assert.Fail(t, "Invalid log level: "+logLevel) } - newDesc := desc + logDesc + infoStr - assert.Equal(t, expected, mocks.logger.InfoWasLogged(infoStr), newDesc) } } @@ -369,25 +380,50 @@ func TestProvide(t *testing.T) { }, requiredSecrets: []string{"k8s-secret1", "k8s-secret2"}, asserts: []assertFunc{ - // TODO - uncomment when decoding is implemented - // assertSecretsUpdated( - // expectedK8sSecrets{ - // "k8s-secret1": { - // "test-decoding": "decoded-value-1", - // "test-decoding2": "decoded-value-2", - // }, - // "k8s-secret2": { - // "test-still-encoded": "ZGVjb2RlZC12YWx1ZS0x", - // "test-still-encoded2": "ZGVjb2RlZC12YWx1ZS0y", - // }, - // }, - // expectedMissingValues{}, - // false, - // ), - assertInfoLogged(true, messages.CSPFK022I, "test-decoding", "k8s-secret1"), - assertInfoLogged(true, messages.CSPFK022I, "test-decoding2", "k8s-secret1"), - assertInfoLogged(false, messages.CSPFK022I, "test-still-encoded", "k8s-secret2"), - assertInfoLogged(false, messages.CSPFK022I, "test-still-encoded2", "k8s-secret2"), + assertSecretsUpdated( + expectedK8sSecrets{ + "k8s-secret1": { + "test-decoding": "decoded-value-1", + "test-decoding2": "decoded-value-2", + }, + "k8s-secret2": { + "test-still-encoded": "ZGVjb2RlZC12YWx1ZS0x", + "test-still-encoded2": "ZGVjb2RlZC12YWx1ZS0y", + }, + }, + expectedMissingValues{}, + false, + ), + assertLogged(true, "info", messages.CSPFK022I, "test-decoding", "k8s-secret1"), + assertLogged(true, "info", messages.CSPFK022I, "test-decoding2", "k8s-secret1"), + assertLogged(false, "info", messages.CSPFK022I, "test-still-encoded", "k8s-secret2"), + assertLogged(false, "info", messages.CSPFK022I, "test-still-encoded2", "k8s-secret2"), + }, + }, + { + desc: "Plaintext secret with base64 content-type returns the secret as is", + k8sSecrets: k8sStorageMocks.K8sSecrets{ + "k8s-secret1": { + "conjur-map": { + "secret1": map[string]interface{}{ + "id": "conjur/var/path1", + "content-type": "base64", + }, + }, + }, + }, + requiredSecrets: []string{"k8s-secret1"}, + asserts: []assertFunc{ + assertSecretsUpdated( + expectedK8sSecrets{ + "k8s-secret1": { + "secret1": "secret-value1", + }, + }, + expectedMissingValues{}, + false, + ), + assertLogged(true, "warn", messages.CSPFK064E, "secret1", "base64", "illegal base64 data"), }, }, { @@ -560,6 +596,7 @@ func TestProvide(t *testing.T) { mocks := newTestMocks() mocks.setPermissions(tc.denyConjurRetrieve, tc.denyK8sRetrieve, tc.denyK8sUpdate) + for secretName, secretData := range tc.k8sSecrets { mocks.kubeClient.AddSecret(secretName, secretData) }