From e138a28973aeadb9902195ca889147a2705f43e4 Mon Sep 17 00:00:00 2001 From: Ninad Page Date: Tue, 22 Dec 2020 03:27:21 +0100 Subject: [PATCH] feat: add general support for isBinary for all backends (#585) * feat: add general support for isBinary for all backends - Add support for `isBinary` in `KVBackend` + tests - Remove specific implementation of `isBinary` from Azure Key Vault & GCP Secrets Manager backends - Update description for `isBinary` field in the CRD to remove Azure-specific details - Update docs * chore: add test for isBinary explicitly set to false --- README.md | 44 ++++++---- ...ernetes-client.io_externalsecrets_crd.yaml | 7 +- .../hello-service-external-secret-gcp.yml | 2 +- .../hello-service-external-secret-vault.yml | 4 + lib/backends/azure-keyvault-backend.js | 7 +- lib/backends/gcp-secrets-manager-backend.js | 8 +- lib/backends/kv-backend.js | 13 +++ lib/backends/kv-backend.test.js | 80 ++++++++++++++++++- 8 files changed, 130 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index e720f338..a3c36241 100644 --- a/README.md +++ b/README.md @@ -443,23 +443,6 @@ spec: name: password ``` -Due to the way Azure handles binary files, you need to explicitly let the ExternalSecret know that the secret is binary. -You can do that with the `isBinary` field on the key. This is necessary for certificates and other secret binary files. - -```yml -apiVersion: kubernetes-client.io/v1 -kind: ExternalSecret -metadata: - name: hello-keyvault-service -spec: - backendType: azureKeyVault - keyVaultName: hello-world - data: - - key: hello-service/credentials - name: password - isBinary: true -``` - ### Alibaba Cloud KMS Secret Manager kubernetes-external-secrets supports fetching secrets from [Alibaba Cloud KMS Secret Manager](https://www.alibabacloud.com/help/doc-detail/152001.htm) @@ -623,6 +606,33 @@ To retrieve an individual secret's content, use the following where "mysecret" i The secrets will persist even if the helm installation is removed, although they will no longer sync to Google Secret Manager. +## Binary Secrets +Most backends do not treat binary secrets any differently than text secrets. Since you typically store a binary secret as a base64-encoded string in the backend, you need to explicitly let the ExternalSecret know that the secret is binary, otherwise it will be encoded in base64 again. +You can do that with the `isBinary` field on the key. This is necessary for certificates and other secret binary files. + +```yml +apiVersion: kubernetes-client.io/v1 +kind: ExternalSecret +metadata: + name: hello-service +spec: + backendType: anySupportedBackend + # ... + data: + - key: hello-service/archives/secrets_zip + name: secrets.zip + isBinary: true # Default: false + # also works with `property` + - key: hello-service/certificates + name: cert.p12 + property: cert.p12 + isBinary: true +``` + +AWS Secrets Manager is a notable exception to this. If you create/update a secret using [SecretBinary](https://docs.aws.amazon.com/secretsmanager/latest/apireference/API_CreateSecret.html#API_CreateSecret_RequestSyntax) parameter of the API, then AWS API will return the secret data as `SecretBinary` in the response and ExternalSecret will handle it accordingly. In that case, you do not need to use the `isBinary` field. + +Note that `SecretBinary` parameter is not available when using the AWS Secrets Manager console. For any binary secrets (represented by a base64-encoded strings) created/updated via the AWS console, or stored in key-value pairs instead of text strings, you can just use the `isBinary` field explicitly as above. + ## Metrics kubernetes-external-secrets exposes the following metrics over a prometheus endpoint: diff --git a/charts/kubernetes-external-secrets/crds/kubernetes-client.io_externalsecrets_crd.yaml b/charts/kubernetes-external-secrets/crds/kubernetes-client.io_externalsecrets_crd.yaml index 0040a8ab..5e115872 100644 --- a/charts/kubernetes-external-secrets/crds/kubernetes-client.io_externalsecrets_crd.yaml +++ b/charts/kubernetes-external-secrets/crds/kubernetes-client.io_externalsecrets_crd.yaml @@ -83,9 +83,10 @@ spec: description: Property to extract if secret in backend is a JSON object isBinary: description: >- - You must set this to true if configuring an item for a binary file stored in Azure KeyVault. - Azure automatically base64 encodes binary files and setting this to true ensures External Secrets - does not base64 encode the base64 encoded binary files. + Whether the backend secret shall be treated as binary data + represented by a base64-encoded string. You must set this to true + for any base64-encoded binary data in the backend - to ensure it + is not encoded in base64 again. Default is false. type: boolean required: - name diff --git a/examples/hello-service-external-secret-gcp.yml b/examples/hello-service-external-secret-gcp.yml index 2f5907a7..ec50ed17 100644 --- a/examples/hello-service-external-secret-gcp.yml +++ b/examples/hello-service-external-secret-gcp.yml @@ -15,5 +15,5 @@ spec: property: value # Version of the secret (default: 'latest') version: 1 - # If the secret is encoded in base64 then decodes it (default: false) + # If the secret is already encoded in base64, then sends it unchanged (default: false) isBinary: false diff --git a/examples/hello-service-external-secret-vault.yml b/examples/hello-service-external-secret-vault.yml index 88491a1d..6e517fe5 100644 --- a/examples/hello-service-external-secret-vault.yml +++ b/examples/hello-service-external-secret-vault.yml @@ -11,3 +11,7 @@ spec: - name: password key: secret/data/hello-service/password property: password + - name: cert.p12 + key: secret/data/hello-service/certificates + property: cert.p12 + isBinary: true # defaults to false diff --git a/lib/backends/azure-keyvault-backend.js b/lib/backends/azure-keyvault-backend.js index 7e6827bf..450e2b56 100644 --- a/lib/backends/azure-keyvault-backend.js +++ b/lib/backends/azure-keyvault-backend.js @@ -26,18 +26,13 @@ class AzureKeyVaultBackend extends KVBackend { * Get secret property value from Azure Key Vault. * @param {string} key - Key used to store secret property value in Azure Key Vault. * @param {string} specOptions.keyVaultName - Name of the azure key vault - * @param {string} keyOptions.isBinary - Does the secret contain a binary? Set to "true" to handle as binary. Does not work with "property" * @returns {Promise} Promise object representing secret property value. */ - async _get ({ key, keyOptions, specOptions: { keyVaultName } }) { + async _get ({ key, specOptions: { keyVaultName } }) { const client = this._keyvaultClient({ keyVaultName }) this._logger.info(`fetching secret ${key} from Azure KeyVault ${keyVaultName}`) const secret = await client.getSecret(key) - // Handle binary files, since the Azure client does not - if (keyOptions && keyOptions.isBinary) { - return Buffer.from(secret.value, 'base64') - } return secret.value } } diff --git a/lib/backends/gcp-secrets-manager-backend.js b/lib/backends/gcp-secrets-manager-backend.js index 26db525b..232fd0eb 100644 --- a/lib/backends/gcp-secrets-manager-backend.js +++ b/lib/backends/gcp-secrets-manager-backend.js @@ -44,13 +44,7 @@ class GCPSecretsManagerBackend extends KVBackend { const [version] = await this._client.accessSecretVersion({ name: 'projects/' + projectId + '/secrets/' + key + '/versions/' + secretVersion }) - const secret = version.payload.data.toString('utf8') - // Handle binary files - this is useful when you've stored a base64 encoded string - if (keyOptions && keyOptions.isBinary) { - return Buffer.from(secret, 'base64') - } - - return secret + return version.payload.data.toString('utf8') } } diff --git a/lib/backends/kv-backend.js b/lib/backends/kv-backend.js index 63b92eac..3bece062 100644 --- a/lib/backends/kv-backend.js +++ b/lib/backends/kv-backend.js @@ -20,6 +20,8 @@ class KVBackend extends AbstractBackend { * @param {string} data[].name - Kubernetes Secret property name. * @param {string} data[].property - If the backend secret is an * object, this is the property name of the value to use. + * @param {string} data[].isBinary - If the backend secret shall be treated + * as binary data represented by a base64-encoded string. Defaults to false. * @param {Object} specOptions - Options set on spec level. * @returns {Promise} Promise object representing secret property values. */ @@ -28,6 +30,7 @@ class KVBackend extends AbstractBackend { const { name, property = null, key, ...keyOptions } = dataItem const plainOrObjValue = await this._get({ key, keyOptions, specOptions }) const shouldParseValue = 'property' in dataItem + const isBinary = 'isBinary' in dataItem && dataItem.isBinary === true let value = plainOrObjValue if (shouldParseValue) { @@ -48,6 +51,16 @@ class KVBackend extends AbstractBackend { value = parsedValue[property] } + if (isBinary) { + // value in the backend is binary data which is already encoded in base64. + if (typeof value === 'string') { + // Skip this step if the value from the backend is not a string (e.g., AWS + // SecretsManager will already return a `Buffer` with base64 encoding if the + // secret contains `SecretBinary` instead of `SecretString`). + value = Buffer.from(value, 'base64') + } + } + return { [name]: value } })) } diff --git a/lib/backends/kv-backend.test.js b/lib/backends/kv-backend.test.js index 6cb08437..267c95a4 100644 --- a/lib/backends/kv-backend.test.js +++ b/lib/backends/kv-backend.test.js @@ -103,6 +103,42 @@ describe('kv-backend', () => { expect(secretPropertyValues).deep.equals([{ fakePropertyName1: 'fakePropertyValue1' }, { fakePropertyName2: 'fakePropertyValue2' }]) }) + it('handles binary values', async () => { + kvBackend._get.onCall(0).resolves('YmluYXJ5Cg==') // base64 of binary + kvBackend._get.onCall(1).resolves('stringPropertyValue1') + kvBackend._get.onCall(2).resolves('{"stringPropertyKey2": "stringPropertyValue2", "binaryPropertyKey2": "YmluYXJ5Cg=="}') + kvBackend._get.onCall(3).resolves('{"stringPropertyKey2": "stringPropertyValue2", "binaryPropertyKey2": "YmluYXJ5Cg=="}') + + const secretPropertyValues = await kvBackend._fetchDataValues({ + data: [{ + key: 'binaryPropertyKey1', + name: 'binaryPropertyName1', + isBinary: true + }, { + key: 'stringPropertyKey1', + name: 'stringPropertyName1' + // isBinary: false + }, { + key: 'jsonProperties', + name: 'stringPropertyName2', + // isBinary: false, + property: 'stringPropertyKey2' + }, { + key: 'jsonProperties', + name: 'binaryPropertyName2', + isBinary: true, + property: 'binaryPropertyKey2' + }] + }) + + expect(secretPropertyValues).deep.equals( + [{ binaryPropertyName1: Buffer.from('YmluYXJ5Cg==', 'base64') }, // base64 of binary (unchanged) + { stringPropertyName1: 'stringPropertyValue1' }, + { stringPropertyName2: 'stringPropertyValue2' }, + { binaryPropertyName2: Buffer.from('YmluYXJ5Cg==', 'base64') } // base64 of binary (unchanged) + ]) + }) + it('fetches secret property values using the specified role', async () => { kvBackend._get.onFirstCall().resolves('fakePropertyValue1') kvBackend._get.onSecondCall().resolves('fakePropertyValue2') @@ -422,8 +458,11 @@ describe('kv-backend', () => { }) describe('base64 encoding', () => { - it('handles json objects', async () => { + beforeEach(() => { kvBackend._get = sinon.stub() + }) + + it('handles json objects', async () => { kvBackend._get .resolves(JSON.stringify({ textProperty: 'text', @@ -446,5 +485,44 @@ describe('kv-backend', () => { jsonProperty: 'eyJzb21lS2V5Ijp7Im15VGV4dCI6InRleHQifX0=' // base 64 value of: {"someKey":{"myText":"text"}} }) }) + + it('handles different types of binary data returned by backends', async () => { + kvBackend._get.onCall(0).resolves(Buffer.from('YmluYXJ5Cg==', 'base64')) // base64 of binary as a Buffer + kvBackend._get.onCall(1).resolves(Buffer.from('YmluYXJ5Cg==', 'base64')) // base64 of binary as a Buffer + kvBackend._get.onCall(2).resolves('YmluYXJ5Cg==') // base64 of binary as String + kvBackend._get.onCall(3).resolves('test') + // e.g. AWS Secrets Manager will return `SecretBinary` as a Buffer and `SecretString` as a String + + const manifestData = await kvBackend.getSecretManifestData({ + spec: { + data: [{ + key: 'binaryPropertyKey1', + name: 'binaryPropertyName1', + isBinary: true + }, { + key: 'binaryPropertyKey2', + name: 'binaryPropertyName2' + // isBinary: false, but will have no impact if the backend returns a Buffer + }, { + key: 'stringPropertyKey3', + name: 'stringPropertyName3' + // isBinary: false, + // must be set to true to ensure base64-encoded string in the backend + // is not encoded in base64 again + }, { + key: 'stringPropertyKey4', + name: 'stringPropertyName4', + isBinary: false // explicitly set false + }] + } + }) + + expect(manifestData).deep.equals({ + binaryPropertyName1: 'YmluYXJ5Cg==', // base64 of binary (unchanged) + binaryPropertyName2: 'YmluYXJ5Cg==', // base64 of binary (unchanged) + stringPropertyName3: 'WW1sdVlYSjVDZz09', // base64 of base64 of binary + stringPropertyName4: 'dGVzdA==' // base64 of test + }) + }) }) })