From 231c3030ed26f95113dec682d7ebf2c9ef7ce16f Mon Sep 17 00:00:00 2001 From: Glen Johnson Date: Mon, 20 Mar 2023 13:16:39 -0600 Subject: [PATCH 1/2] 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) } From 4274d140b2ed95240a51973bced10b105ffe091f Mon Sep 17 00:00:00 2001 From: Glen Johnson Date: Tue, 21 Mar 2023 10:27:00 -0600 Subject: [PATCH 2/2] Update k8s and k8s-rotation dev environments with encoded secret --- .gitignore | 4 ++-- deploy/config/k8s/k8s-secret.yml | 3 +++ deploy/config/openshift/k8s-secret.yml | 3 +++ .../k8s/secrets-provider-init-container.sh.yml | 5 +++++ .../k8s/secrets-provider-k8s-rotation.sh.yml | 5 +++++ deploy/policy/load_policies.sh | 1 + .../templates/conjur-secrets.template.sh.yml | 1 + deploy/utils.sh | 16 ++++++++++------ 8 files changed, 30 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index e7f26f4b..be06f493 100644 --- a/.gitignore +++ b/.gitignore @@ -12,5 +12,5 @@ junit.xml # Temporary directory to store the CyberArk proxy CA certificate build_ca_certificate/ -# Ignore generated policy files -deploy/policy/generated/ +# Ignore generated policy files and manifests +deploy/**/generated/ diff --git a/deploy/config/k8s/k8s-secret.yml b/deploy/config/k8s/k8s-secret.yml index 86b0b315..680e2b90 100644 --- a/deploy/config/k8s/k8s-secret.yml +++ b/deploy/config/k8s/k8s-secret.yml @@ -9,4 +9,7 @@ stringData: var_with_spaces: secrets/var with spaces var_with_pluses: secrets/var+with+pluses var_with_umlaut: secrets/umlaut + var_with_encoded: + id: secrets/encoded + content-type: base64 non-conjur-key: some-value diff --git a/deploy/config/openshift/k8s-secret.yml b/deploy/config/openshift/k8s-secret.yml index 86b0b315..680e2b90 100644 --- a/deploy/config/openshift/k8s-secret.yml +++ b/deploy/config/openshift/k8s-secret.yml @@ -9,4 +9,7 @@ stringData: var_with_spaces: secrets/var with spaces var_with_pluses: secrets/var+with+pluses var_with_umlaut: secrets/umlaut + var_with_encoded: + id: secrets/encoded + content-type: base64 non-conjur-key: some-value diff --git a/deploy/dev/config/k8s/secrets-provider-init-container.sh.yml b/deploy/dev/config/k8s/secrets-provider-init-container.sh.yml index 10a8876b..7c5fb79c 100755 --- a/deploy/dev/config/k8s/secrets-provider-init-container.sh.yml +++ b/deploy/dev/config/k8s/secrets-provider-init-container.sh.yml @@ -46,6 +46,11 @@ spec: secretKeyRef: name: test-k8s-secret key: var_with_umlaut + - name: VARIABLE_WITH_ENCODED_SECRET + valueFrom: + secretKeyRef: + name: test-k8s-secret + key: var_with_encoded - name: NON_CONJUR_SECRET valueFrom: secretKeyRef: diff --git a/deploy/dev/config/k8s/secrets-provider-k8s-rotation.sh.yml b/deploy/dev/config/k8s/secrets-provider-k8s-rotation.sh.yml index aa73be79..026e077f 100755 --- a/deploy/dev/config/k8s/secrets-provider-k8s-rotation.sh.yml +++ b/deploy/dev/config/k8s/secrets-provider-k8s-rotation.sh.yml @@ -69,6 +69,11 @@ spec: secretKeyRef: name: test-k8s-secret key: var_with_umlaut + - name: VARIABLE_WITH_ENCODED_SECRET + valueFrom: + secretKeyRef: + name: test-k8s-secret + key: var_with_encoded - name: NON_CONJUR_SECRET valueFrom: secretKeyRef: diff --git a/deploy/policy/load_policies.sh b/deploy/policy/load_policies.sh index a49d139b..c49c3e57 100755 --- a/deploy/policy/load_policies.sh +++ b/deploy/policy/load_policies.sh @@ -34,6 +34,7 @@ conjur variable set -i secrets/test_secret -v "some-secret" conjur variable set -i "secrets/var with spaces" -v "some-secret" conjur variable set -i "secrets/var+with+pluses" -v "some-secret" conjur variable set -i "secrets/umlaut" -v "some-secret" +conjur variable set -i "secrets/encoded" -v "c2VjcmV0LXZhbHVl" # == secret-value conjur variable set -i secrets/url -v "postgresql://test-app-backend.app-test.svc.cluster.local:5432" conjur variable set -i secrets/username -v "some-user" conjur variable set -i secrets/password -v "7H1SiSmYp@5Sw0rd" diff --git a/deploy/policy/templates/conjur-secrets.template.sh.yml b/deploy/policy/templates/conjur-secrets.template.sh.yml index f76552e0..27f65449 100755 --- a/deploy/policy/templates/conjur-secrets.template.sh.yml +++ b/deploy/policy/templates/conjur-secrets.template.sh.yml @@ -12,6 +12,7 @@ cat << EOL - !variable var with spaces - !variable var+with+pluses - !variable umlaut + - !variable encoded - !variable url - !variable username - !variable password diff --git a/deploy/utils.sh b/deploy/utils.sh index c718c234..708fb0bf 100644 --- a/deploy/utils.sh +++ b/deploy/utils.sh @@ -332,6 +332,7 @@ deploy_chart() { } set_config_directory_path() { + export DEV_CONFIG_DIR="dev/config/k8s" export CONFIG_DIR="config/k8s" if [[ "$PLATFORM" = "openshift" ]]; then export CONFIG_DIR="config/openshift" @@ -380,8 +381,9 @@ deploy_init_env() { echo "Running Deployment Manifest" if [[ "$DEV" = "true" ]]; then - ./dev/config/k8s/secrets-provider-init-container.sh.yml > ./dev/config/k8s/secrets-provider-init-container.yml - $cli_with_timeout apply -f ./dev/config/k8s/secrets-provider-init-container.yml + mkdir -p $DEV_CONFIG_DIR/generated + $DEV_CONFIG_DIR/secrets-provider-init-container.sh.yml > $DEV_CONFIG_DIR/generated/secrets-provider-init-container.yml + $cli_with_timeout apply -f $DEV_CONFIG_DIR/generated/secrets-provider-init-container.yml $cli_with_timeout "get pods --namespace=$APP_NAMESPACE_NAME --selector app=init-env --no-headers | wc -l" else @@ -407,8 +409,9 @@ deploy_k8s_rotation_env() { echo "Running Deployment Manifest" if [[ "$DEV" = "true" ]]; then - ./dev/config/k8s/secrets-provider-k8s-rotation.sh.yml > ./dev/config/k8s/secrets-provider-k8s-rotation.yml - $cli_with_timeout apply -f ./dev/config/k8s/secrets-provider-k8s-rotation.yml + mkdir -p $DEV_CONFIG_DIR/generated + $DEV_CONFIG_DIR/secrets-provider-k8s-rotation.sh.yml > $DEV_CONFIG_DIR/generated/secrets-provider-k8s-rotation.yml + $cli_with_timeout apply -f $DEV_CONFIG_DIR/generated/secrets-provider-k8s-rotation.yml $cli_with_timeout "get pods --namespace=$APP_NAMESPACE_NAME --selector app=test-app --no-headers | wc -l" else @@ -649,8 +652,9 @@ deploy_push_to_file() { deployment_name="test-env" if [[ "$DEV" = "true" ]]; then - "./dev/config/k8s/$dev_yaml_file_name.sh.yml" > "./dev/config/k8s/$dev_yaml_file_name.yml" - $cli_with_timeout apply -f "./dev/config/k8s/$dev_yaml_file_name.yml" + mkdir -p $DEV_CONFIG_DIR/generated + "$DEV_CONFIG_DIR/$dev_yaml_file_name.sh.yml" > "$DEV_CONFIG_DIR/generated/$dev_yaml_file_name.yml" + $cli_with_timeout apply -f "$DEV_CONFIG_DIR/generated/$dev_yaml_file_name.yml" $cli_with_timeout "get pods --namespace=$APP_NAMESPACE_NAME --selector app=$deployment_name --no-headers | wc -l" else