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

Decode base64 K8s secrets #508

Merged
merged 2 commits into from
Mar 21, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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/
3 changes: 3 additions & 0 deletions deploy/config/k8s/k8s-secret.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 3 additions & 0 deletions deploy/config/openshift/k8s-secret.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions deploy/dev/config/k8s/secrets-provider-init-container.sh.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions deploy/dev/config/k8s/secrets-provider-k8s-rotation.sh.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions deploy/policy/load_policies.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions deploy/policy/templates/conjur-secrets.template.sh.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ cat << EOL
- !variable var with spaces
- !variable var+with+pluses
- !variable umlaut
- !variable encoded
- !variable url
- !variable username
- !variable password
Expand Down
16 changes: 10 additions & 6 deletions deploy/utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions pkg/log/messages/error_messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
59 changes: 40 additions & 19 deletions pkg/secrets/k8s_secrets_storage/provide_conjur_secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package k8ssecretsstorage
import (
"bytes"
"context"
"encoding/base64"
"fmt"
"strings"

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
87 changes: 62 additions & 25 deletions pkg/secrets/k8s_secrets_storage/provide_conjur_secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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"),
},
},
{
Expand Down Expand Up @@ -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)
}
Expand Down