From 51bac076c733fbcdc5a6852363d88c15162f5bf7 Mon Sep 17 00:00:00 2001 From: Eric Horst Date: Thu, 23 Jul 2020 10:59:11 -0700 Subject: [PATCH 1/3] fix(vault-backend): token ttl conditional renew --- README.md | 2 ++ config/environment.js | 2 ++ config/index.js | 7 ++++++- lib/backends/vault-backend.js | 17 ++++++++++++----- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 8b64b7bf..ef261911 100644 --- a/README.md +++ b/README.md @@ -352,6 +352,8 @@ spec: If you use Vault Namespaces (a Vault Enterprise feature) you can set the namespace to interact with via the `VAULT_NAMESPACE` environment variable. +The Vault token obtained by Kubernetes authentication will be renewed as needed. By default the token will be renewed three poller intervals (POLLER_INTERVAL_MILLISECONDS) before the token TTL expires. The default should be acceptable in most cases but the token renew threshold can also be customized by setting the `VAULT_TOKEN_RENEW_THRESHOLD` environment variable. The token renew threshold value is specified in seconds and tokens with remaining TTL less than this number of seconds will be renewed. In order to minimize token renewal load on the Vault server it is suggested that Kubernetes auth tokens issued by Vault have a TTL of at least ten times the poller interval so that they are renewed less frequently. A longer token TTL results in a lower token renewal load on Vault. + If Vault uses a certificate issued by a self-signed CA you will need to provide that certificate: ```sh diff --git a/config/environment.js b/config/environment.js index 37aa6e7a..46f73500 100644 --- a/config/environment.js +++ b/config/environment.js @@ -19,6 +19,7 @@ if (environment === 'development') { const vaultEndpoint = process.env.VAULT_ADDR || 'http://127.0.0.1:8200' // Grab the vault namespace from the environment const vaultNamespace = process.env.VAULT_NAMESPACE || null +const vaultTokenRenewThreshold = process.env.VAULT_TOKEN_RENEW_THRESHOLD || null const pollerIntervalMilliseconds = process.env.POLLER_INTERVAL_MILLISECONDS ? Number(process.env.POLLER_INTERVAL_MILLISECONDS) : 10000 @@ -40,6 +41,7 @@ const customResourceManagerDisabled = 'DISABLE_CUSTOM_RESOURCE_MANAGER' in proce module.exports = { vaultEndpoint, vaultNamespace, + vaultTokenRenewThreshold, environment, pollerIntervalMilliseconds, metricsPort, diff --git a/config/index.js b/config/index.js index 5e85d872..b5b7d02f 100644 --- a/config/index.js +++ b/config/index.js @@ -77,7 +77,12 @@ if (envConfig.vaultNamespace) { } } const vaultClient = vault(vaultOptions) -const vaultBackend = new VaultBackend({ client: vaultClient, logger }) +// The Vault token is renewed only during polling, not asynchronously. The default tokenRenewThreshold +// is three times larger than the pollerInterval so that the token is renewed before it +// expires and with at least one remaining poll opportunty to retry renewal if it fails. +const vaultTokenRenewThreshold = envConfig.vaultTokenRenewThreshold + ? Number(envConfig.vaultTokenRenewThreshold) : 3 * envConfig.pollerIntervalMilliseconds / 1000 +const vaultBackend = new VaultBackend({ client: vaultClient, tokenRenewThreshold: vaultTokenRenewThreshold, logger }) const azureKeyVaultBackend = new AzureKeyVaultBackend({ credential: azureConfig.azureKeyVault(), logger diff --git a/lib/backends/vault-backend.js b/lib/backends/vault-backend.js index 5c07813b..5f93f30d 100644 --- a/lib/backends/vault-backend.js +++ b/lib/backends/vault-backend.js @@ -7,11 +7,13 @@ class VaultBackend extends KVBackend { /** * Create Vault backend. * @param {Object} client - Client for interacting with Vault. + * @param {Number} tokenRenewThreshold - tokens are renewed when ttl reaches this threshold * @param {Object} logger - Logger for logging stuff. */ - constructor ({ client, logger }) { + constructor ({ client, tokenRenewThreshold, logger }) { super({ logger }) this._client = client + this._tokenRenewThreshold = tokenRenewThreshold } /** @@ -40,15 +42,20 @@ class VaultBackend extends KVBackend { if (!this._client.token) { const jwt = this._fetchServiceAccountToken() this._logger.debug('fetching new token from vault') - const vault = await this._client.kubernetesLogin({ + await this._client.kubernetesLogin({ mount_point: vaultMountPoint, role: vaultRole, jwt: jwt }) - this._client.token = vault.auth.client_token } else { - this._logger.debug('renewing existing token from vault') - this._client.tokenRenewSelf() + this._logger.debug('checking vault token expiry') + const tokenStatus = await this._client.tokenLookupSelf() + this._logger.debug(`vault token valid for ${tokenStatus.data.ttl} seconds, renews at ${this._tokenRenewThreshold}`) + + if (Number(tokenStatus.data.ttl) <= this._tokenRenewThreshold) { + this._logger.debug('renewing vault token') + await this._client.tokenRenewSelf() + } } this._logger.debug(`reading secret key ${key} from vault`) From 06e9afcccffbfd39c4b3524507e15ff1515c1dc3 Mon Sep 17 00:00:00 2001 From: Eric Horst Date: Mon, 27 Jul 2020 18:09:28 -0700 Subject: [PATCH 2/3] fix(vault-backend): grr, trailing whitespace --- config/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/index.js b/config/index.js index b5b7d02f..5a7bd2f0 100644 --- a/config/index.js +++ b/config/index.js @@ -80,7 +80,7 @@ const vaultClient = vault(vaultOptions) // The Vault token is renewed only during polling, not asynchronously. The default tokenRenewThreshold // is three times larger than the pollerInterval so that the token is renewed before it // expires and with at least one remaining poll opportunty to retry renewal if it fails. -const vaultTokenRenewThreshold = envConfig.vaultTokenRenewThreshold +const vaultTokenRenewThreshold = envConfig.vaultTokenRenewThreshold ? Number(envConfig.vaultTokenRenewThreshold) : 3 * envConfig.pollerIntervalMilliseconds / 1000 const vaultBackend = new VaultBackend({ client: vaultClient, tokenRenewThreshold: vaultTokenRenewThreshold, logger }) const azureKeyVaultBackend = new AzureKeyVaultBackend({ From 445b10f47672ed0e16e5f0b78d9734f245f67ddd Mon Sep 17 00:00:00 2001 From: Eric Horst Date: Wed, 29 Jul 2020 19:35:35 -0700 Subject: [PATCH 3/3] fix(vault-backend): add and correct tests --- lib/backends/vault-backend.test.js | 51 ++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/lib/backends/vault-backend.test.js b/lib/backends/vault-backend.test.js index aeee96c6..9e4ae9b4 100644 --- a/lib/backends/vault-backend.test.js +++ b/lib/backends/vault-backend.test.js @@ -34,11 +34,24 @@ describe('VaultBackend', () => { } } + const vaultTokenRenewThreshold = 30 + const mockTokenLookupResultMustRenew = { + data: { + ttl: 15 + } + } + const mockTokenLookupResultNoRenew = { + data: { + ttl: 60 + } + } + beforeEach(() => { clientMock = sinon.mock() vaultBackend = new VaultBackend({ client: clientMock, + tokenRenewThreshold: vaultTokenRenewThreshold, logger }) }) @@ -46,6 +59,7 @@ describe('VaultBackend', () => { describe('_get', () => { beforeEach(() => { clientMock.read = sinon.stub().returns(kv2Secret) + clientMock.tokenLookupSelf = sinon.stub().returns(mockTokenLookupResultMustRenew) clientMock.tokenRenewSelf = sinon.stub().returns(true) clientMock.kubernetesLogin = sinon.stub().returns({ auth: { @@ -133,8 +147,9 @@ describe('VaultBackend', () => { expect(secretPropertyValue).equals(quotedSecretValue) }) - it('returns secret property value after renewing token if a token exists', async () => { + it('returns secret property value after renewing token if a token exists that needs renewal', async () => { clientMock.token = 'an-existing-token' + clientMock.tokenLookupSelf = sinon.stub().returns(mockTokenLookupResultMustRenew) const secretPropertyValue = await vaultBackend._get({ specOptions: { @@ -147,7 +162,10 @@ describe('VaultBackend', () => { // No logging into Vault... sinon.assert.notCalled(clientMock.kubernetesLogin) - // ... but renew the token instead ... + // ... but check the token instead ... + sinon.assert.calledOnce(clientMock.tokenLookupSelf) + + // ... then renew the token ... sinon.assert.calledOnce(clientMock.tokenRenewSelf) // ... then we fetch the secret ... @@ -156,11 +174,40 @@ describe('VaultBackend', () => { // ... and expect to get its proper value expect(secretPropertyValue).equals(quotedSecretValue) }) + + it('returns secret property value if a token exists that does not need renewal', async () => { + clientMock.token = 'an-existing-token' + clientMock.tokenLookupSelf = sinon.stub().returns(mockTokenLookupResultNoRenew) + + const secretPropertyValue = await vaultBackend._get({ + specOptions: { + vaultMountPoint: mountPoint, + vaultRole: role + }, + key: secretKey + }) + + // No logging into Vault... + sinon.assert.notCalled(clientMock.kubernetesLogin) + + // ... but check the token instead ... + sinon.assert.calledOnce(clientMock.tokenLookupSelf) + + // ... and token does not need renewal ... + sinon.assert.notCalled(clientMock.tokenRenewSelf) + + // ... then we fetch the secret ... + sinon.assert.calledWith(clientMock.read, secretKey) + + // ... and expect to get its proper value + expect(secretPropertyValue).equals(quotedSecretValue) + }) }) describe('getSecretManifestData', () => { beforeEach(() => { clientMock.read = sinon.stub().returns(kv2Secret) + clientMock.tokenLookupSelf = sinon.stub().returns(mockTokenLookupResultMustRenew) clientMock.tokenRenewSelf = sinon.stub().returns(true) clientMock.kubernetesLogin = sinon.stub().returns({ auth: { client_token: '1234' } })