From 94a27611614413deedccffc1a2b0f190acb25f03 Mon Sep 17 00:00:00 2001 From: Daniel Peng Date: Fri, 25 Sep 2026 01:32:50 -0400 Subject: [PATCH] feat: honor KEY_PROVIDER_URL scheme and validate at config load Ticket: WCN-1927 --- README.md | 6 +- docker-compose.yml | 1 + .../keyProviderClient.test.ts | 25 +++++ src/__tests__/config.test.ts | 96 +++++++++++++++---- .../keyProviderClient/keyProviderClient.ts | 22 ++--- src/initConfig.ts | 32 +++++++ 6 files changed, 152 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index d1f00c66..e19c2289 100644 --- a/README.md +++ b/README.md @@ -125,6 +125,7 @@ BITGO_ENV=test \ APP_MODE=advanced-wallet-manager \ ADVANCED_WALLET_MANAGER_PORT=3080 \ KEY_PROVIDER_URL=http://localhost:3000 \ +ALLOW_PLAINTEXT_KEY_PROVIDER=true \ npm start ``` @@ -175,8 +176,11 @@ curl -X POST http://localhost:3081/ping/advancedWalletManager | `ADVANCED_WALLET_MANAGER_PORT` | Port to listen on | `3080` | ❌ | | `KEY_PROVIDER_URL` | URL to your key provider API implementation | - | ✅ | | `SIGNING_MODE` | Delegates key generation and signing to key provider (`local` or `external`) | `local` | ❌ | +| `ALLOW_PLAINTEXT_KEY_PROVIDER` | Allow an `http://` `KEY_PROVIDER_URL`/`BACKUP_KMS_URL` when `TLS_MODE=disabled`. **Development only** — private keys are sent unencrypted | `false` | ❌ | > **Note:** The `KEY_PROVIDER_URL` points to your implementation of the key provider API interface. You must implement this interface to connect your KMS/HSM. See [Prerequisites](#prerequisites) for the specification and examples. +> +> The URL scheme is honored as configured and must be `http://` or `https://`. `https://` is required when `TLS_MODE=mtls`; `http://` is rejected unless `ALLOW_PLAINTEXT_KEY_PROVIDER=true`. ### Master Express Settings @@ -305,7 +309,7 @@ podman run -d \ -e TLS_MODE=mtls \ -e SERVER_TLS_KEY_PATH=/app/certs/advanced-wallet-manager-key.pem \ -e SERVER_TLS_CERT_PATH=/app/certs/advanced-wallet-manager-cert.pem \ - -e KEY_PROVIDER_URL=host.containers.internal:3000 \ + -e KEY_PROVIDER_URL=https://host.containers.internal:3000 \ -e NODE_ENV=development \ -e CLIENT_CERT_ALLOW_SELF_SIGNED=true \ advanced-wallet-manager diff --git a/docker-compose.yml b/docker-compose.yml index b9d05857..1b0ffd98 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -24,6 +24,7 @@ services: # Key provider settings (required) - KEY_PROVIDER_URL=http://172.20.0.1:3000 # UPDATE TO YOUR OWN key provider URL + - ALLOW_PLAINTEXT_KEY_PROVIDER=true # Development only: permits http:// KEY_PROVIDER_URL with TLS_MODE=disabled - KEY_PROVIDER_SERVER_CERT_ALLOW_SELF_SIGNED=true # Optional key provider TLS settings (uncomment if using mTLS with key provider) diff --git a/src/__tests__/api/advancedWalletManager/keyProviderClient.test.ts b/src/__tests__/api/advancedWalletManager/keyProviderClient.test.ts index 418320bf..9a0caae1 100644 --- a/src/__tests__/api/advancedWalletManager/keyProviderClient.test.ts +++ b/src/__tests__/api/advancedWalletManager/keyProviderClient.test.ts @@ -224,3 +224,28 @@ describe('KeyProviderClient.sign', () => { }); }); }); + +describe('KeyProviderClient URL scheme', () => { + afterEach(() => nock.cleanAll()); + + it('should not downgrade an https:// URL when TLS is disabled', async () => { + const keyProviderUrl = 'https://key-provider.invalid:8443'; + const params = { coin: 'hteth', source: 'user' as const, type: 'independent' as const }; + const mockResponse = { pub: 'xpub661MyMwAq', ...params }; + const client = new KeyProviderClient({ + appMode: AppMode.ADVANCED_WALLET_MANAGER, + signingMode: SigningMode.LOCAL, + port: 0, + bind: 'localhost', + timeout: 60000, + httpLoggerFile: '', + keyProviderUrl, + tlsMode: TlsMode.DISABLED, + clientCertAllowSelfSigned: true, + }); + + const httpsNock = nock(keyProviderUrl).post('/key/generate').reply(200, mockResponse); + await client.generateKey(params); + httpsNock.done(); + }); +}); diff --git a/src/__tests__/config.test.ts b/src/__tests__/config.test.ts index f57b8e08..f3fbf5b5 100644 --- a/src/__tests__/config.test.ts +++ b/src/__tests__/config.test.ts @@ -23,6 +23,8 @@ describe('Configuration', () => { delete process.env.APP_MODE; delete process.env.BITGO_APP_MODE; delete process.env.KEY_PROVIDER_URL; + delete process.env.ALLOW_PLAINTEXT_KEY_PROVIDER; + delete process.env.BACKUP_KMS_URL; delete process.env.ADVANCED_WALLET_MANAGER_URL; delete process.env.AWM_SERVER_CA_CERT_PATH; delete process.env.TLS_MODE; @@ -90,7 +92,7 @@ describe('Configuration', () => { }); it('should use default configuration when minimal environment variables are set', () => { - process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; process.env.SERVER_TLS_KEY = mockTlsKey; process.env.SERVER_TLS_CERT = mockTlsCert; process.env.KEY_PROVIDER_CLIENT_TLS_KEY = mockClientTlsKey; @@ -106,7 +108,7 @@ describe('Configuration', () => { cfg.bind.should.equal('localhost'); cfg.tlsMode.should.equal(TlsMode.MTLS); cfg.timeout.should.equal(305 * 1000); - cfg.keyProviderUrl.should.equal('http://localhost:3000'); + cfg.keyProviderUrl.should.equal('https://localhost:3000'); cfg.serverTlsKey!.should.equal(mockTlsKey); cfg.serverTlsCert!.should.equal(mockTlsCert); } @@ -114,7 +116,7 @@ describe('Configuration', () => { it('should read port from environment variable', () => { process.env.ADVANCED_WALLET_MANAGER_PORT = '4000'; - process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; process.env.SERVER_TLS_KEY = mockTlsKey; process.env.SERVER_TLS_CERT = mockTlsCert; process.env.KEY_PROVIDER_CLIENT_TLS_KEY = mockClientTlsKey; @@ -127,14 +129,14 @@ describe('Configuration', () => { isAdvancedWalletManagerConfig(cfg).should.be.true(); if (isAdvancedWalletManagerConfig(cfg)) { cfg.port.should.equal(4000); - cfg.keyProviderUrl.should.equal('http://localhost:3000'); + cfg.keyProviderUrl.should.equal('https://localhost:3000'); cfg.serverTlsKey!.should.equal(mockTlsKey); cfg.serverTlsCert!.should.equal(mockTlsCert); } }); it('should read the recovery mode from the env', () => { - process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; process.env.SERVER_TLS_KEY = mockTlsKey; process.env.SERVER_TLS_CERT = mockTlsCert; process.env.KEY_PROVIDER_CLIENT_TLS_KEY = mockClientTlsKey; @@ -149,7 +151,7 @@ describe('Configuration', () => { }); it('should read TLS mode from environment variables', () => { - process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; process.env.SERVER_TLS_KEY = mockTlsKey; process.env.SERVER_TLS_CERT = mockTlsCert; process.env.KEY_PROVIDER_CLIENT_TLS_KEY = mockClientTlsKey; @@ -165,7 +167,7 @@ describe('Configuration', () => { isAdvancedWalletManagerConfig(cfg).should.be.true(); if (isAdvancedWalletManagerConfig(cfg)) { cfg.tlsMode.should.equal(TlsMode.DISABLED); - cfg.keyProviderUrl.should.equal('http://localhost:3000'); + cfg.keyProviderUrl.should.equal('https://localhost:3000'); } // Test with mTLS explicitly enabled @@ -174,7 +176,7 @@ describe('Configuration', () => { isAdvancedWalletManagerConfig(cfg).should.be.true(); if (isAdvancedWalletManagerConfig(cfg)) { cfg.tlsMode.should.equal(TlsMode.MTLS); - cfg.keyProviderUrl.should.equal('http://localhost:3000'); + cfg.keyProviderUrl.should.equal('https://localhost:3000'); cfg.serverTlsKey!.should.equal(mockTlsKey); cfg.serverTlsCert!.should.equal(mockTlsCert); } @@ -191,14 +193,14 @@ describe('Configuration', () => { isAdvancedWalletManagerConfig(cfg).should.be.true(); if (isAdvancedWalletManagerConfig(cfg)) { cfg.tlsMode.should.equal(TlsMode.MTLS); - cfg.keyProviderUrl.should.equal('http://localhost:3000'); + cfg.keyProviderUrl.should.equal('https://localhost:3000'); cfg.serverTlsKey!.should.equal(mockTlsKey); cfg.serverTlsCert!.should.equal(mockTlsCert); } }); it('should read SIGNING_MODE from environment variables', () => { - process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; process.env.TLS_MODE = 'disabled'; // unset defaults to LOCAL @@ -227,7 +229,7 @@ describe('Configuration', () => { }); it('should read mTLS settings from environment variables', () => { - process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; process.env.SERVER_TLS_KEY = mockTlsKey; process.env.SERVER_TLS_CERT = mockTlsCert; process.env.KEY_PROVIDER_CLIENT_TLS_KEY = mockClientTlsKey; @@ -242,7 +244,7 @@ describe('Configuration', () => { isAdvancedWalletManagerConfig(cfg).should.be.true(); if (isAdvancedWalletManagerConfig(cfg)) { cfg.mtlsAllowedClientFingerprints!.should.deepEqual(['ABC123', 'DEF456']); - cfg.keyProviderUrl.should.equal('http://localhost:3000'); + cfg.keyProviderUrl.should.equal('https://localhost:3000'); cfg.serverTlsKey!.should.equal(mockTlsKey); cfg.serverTlsCert!.should.equal(mockTlsCert); cfg.keyProviderServerCaCertPath!.should.equal( @@ -266,7 +268,7 @@ describe('Configuration', () => { }); it('should succeed when TLS certificates are not set for disabled TLS mode', () => { - process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; process.env.TLS_MODE = 'disabled'; delete process.env.SERVER_TLS_KEY; delete process.env.SERVER_TLS_CERT; @@ -275,12 +277,12 @@ describe('Configuration', () => { isAdvancedWalletManagerConfig(cfg).should.be.true(); if (isAdvancedWalletManagerConfig(cfg)) { cfg.tlsMode.should.equal(TlsMode.DISABLED); - cfg.keyProviderUrl.should.equal('http://localhost:3000'); + cfg.keyProviderUrl.should.equal('https://localhost:3000'); } }); it('should throw error when TLS certificates are not set for MTLS mode', () => { - process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; process.env.TLS_MODE = 'mtls'; delete process.env.SERVER_TLS_KEY; delete process.env.SERVER_TLS_CERT; @@ -288,7 +290,7 @@ describe('Configuration', () => { }); it('should read HTTP_LOGFILE into httpLoggerFile in Advanced wallet manager mode', () => { - process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; process.env.SERVER_TLS_KEY = mockTlsKey; process.env.SERVER_TLS_CERT = mockTlsCert; process.env.KEY_PROVIDER_CLIENT_TLS_KEY = mockClientTlsKey; @@ -306,13 +308,73 @@ describe('Configuration', () => { }); it('should throw error when KEY_PROVIDER_SERVER_CA_CERT_PATH is not set for MTLS mode', () => { - process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; process.env.TLS_MODE = 'mtls'; delete process.env.KEY_PROVIDER_SERVER_CA_CERT_PATH; (() => initConfig()).should.throw( 'KEY_PROVIDER_SERVER_CA_CERT_PATH is required when TLS mode is MTLS', ); }); + + describe('KEY_PROVIDER_URL scheme validation', () => { + it('should throw when KEY_PROVIDER_URL uses http:// in MTLS mode', () => { + process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.TLS_MODE = 'mtls'; + (() => initConfig()).should.throw( + 'KEY_PROVIDER_URL must use https:// when TLS_MODE is mtls, got: http://localhost:3000', + ); + }); + + it('should throw when KEY_PROVIDER_URL uses http:// in disabled mode without override', () => { + process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.TLS_MODE = 'disabled'; + (() => initConfig()).should.throw(/KEY_PROVIDER_URL uses plaintext http:\/\//); + }); + + it('should allow http:// in disabled mode when ALLOW_PLAINTEXT_KEY_PROVIDER=true', () => { + process.env.KEY_PROVIDER_URL = 'http://localhost:3000'; + process.env.TLS_MODE = 'disabled'; + process.env.ALLOW_PLAINTEXT_KEY_PROVIDER = 'true'; + const cfg = initConfig(); + isAdvancedWalletManagerConfig(cfg).should.be.true(); + if (isAdvancedWalletManagerConfig(cfg)) { + cfg.keyProviderUrl.should.equal('http://localhost:3000'); + } + }); + + it('should throw when KEY_PROVIDER_URL has no http(s) scheme', () => { + process.env.KEY_PROVIDER_URL = 'localhost:3000'; + process.env.TLS_MODE = 'disabled'; + (() => initConfig()).should.throw( + 'KEY_PROVIDER_URL must use http:// or https://, got: localhost:3000', + ); + }); + + it('should throw when KEY_PROVIDER_URL is not a valid URL', () => { + process.env.KEY_PROVIDER_URL = 'not a url'; + process.env.TLS_MODE = 'disabled'; + (() => initConfig()).should.throw('KEY_PROVIDER_URL is not a valid URL: not a url'); + }); + + it('should throw when BACKUP_KMS_URL uses http:// without override', () => { + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; + process.env.BACKUP_KMS_URL = 'http://localhost:3001'; + process.env.TLS_MODE = 'disabled'; + (() => initConfig()).should.throw(/BACKUP_KMS_URL uses plaintext http:\/\//); + }); + + it('should allow http:// BACKUP_KMS_URL when ALLOW_PLAINTEXT_KEY_PROVIDER=true', () => { + process.env.KEY_PROVIDER_URL = 'https://localhost:3000'; + process.env.BACKUP_KMS_URL = 'http://localhost:3001'; + process.env.TLS_MODE = 'disabled'; + process.env.ALLOW_PLAINTEXT_KEY_PROVIDER = 'true'; + const cfg = initConfig(); + isAdvancedWalletManagerConfig(cfg).should.be.true(); + if (isAdvancedWalletManagerConfig(cfg)) { + cfg.backupKmsUrl!.should.equal('http://localhost:3001'); + } + }); + }); }); describe('Master Express Mode', () => { diff --git a/src/advancedWalletManager/keyProviderClient/keyProviderClient.ts b/src/advancedWalletManager/keyProviderClient/keyProviderClient.ts index 5ac51884..3484256c 100644 --- a/src/advancedWalletManager/keyProviderClient/keyProviderClient.ts +++ b/src/advancedWalletManager/keyProviderClient/keyProviderClient.ts @@ -40,18 +40,16 @@ export class KeyProviderClient extends BaseHttpClient { const urlObj = new URL(cfg.keyProviderUrl); let agent: https.Agent | undefined; - if (cfg.tlsMode === TlsMode.MTLS) { - urlObj.protocol = 'https:'; - if (cfg.keyProviderServerCaCert || cfg.keyProviderServerCertAllowSelfSigned) { - agent = new https.Agent({ - ca: cfg.keyProviderServerCaCert, - cert: cfg.keyProviderClientTlsCert, - key: cfg.keyProviderClientTlsKey, - rejectUnauthorized: !cfg.keyProviderServerCertAllowSelfSigned, - }); - } - } else { - urlObj.protocol = 'http:'; + if ( + cfg.tlsMode === TlsMode.MTLS && + (cfg.keyProviderServerCaCert || cfg.keyProviderServerCertAllowSelfSigned) + ) { + agent = new https.Agent({ + ca: cfg.keyProviderServerCaCert, + cert: cfg.keyProviderClientTlsCert, + key: cfg.keyProviderClientTlsKey, + rejectUnauthorized: !cfg.keyProviderServerCertAllowSelfSigned, + }); } super(urlObj.toString(), cfg.timeout, agent); diff --git a/src/initConfig.ts b/src/initConfig.ts index 00a84423..13a60942 100644 --- a/src/initConfig.ts +++ b/src/initConfig.ts @@ -136,6 +136,33 @@ function determineTlsMode(): TlsMode { throw new Error(`Invalid TLS_MODE: ${tlsMode}. Must be either "disabled" or "mtls"`); } +function validateKeyProviderUrl(url: string, envVar: string, tlsMode: TlsMode): void { + let protocol: string; + try { + protocol = new URL(url).protocol; + } catch { + throw new Error(`${envVar} is not a valid URL: ${url}`); + } + + if (protocol === 'https:') { + return; + } + if (protocol !== 'http:') { + throw new Error(`${envVar} must use http:// or https://, got: ${url}`); + } + if (tlsMode === TlsMode.MTLS) { + throw new Error(`${envVar} must use https:// when TLS_MODE is mtls, got: ${url}`); + } + if (readEnvVar('ALLOW_PLAINTEXT_KEY_PROVIDER') !== 'true') { + throw new Error( + `${envVar} uses plaintext http://, which exposes private keys in transit. Use https:// or set ALLOW_PLAINTEXT_KEY_PROVIDER=true (development only).`, + ); + } + logger.warn( + `⚠️ ${envVar} uses plaintext http:// (${url}); private keys will be sent unencrypted. Never set ALLOW_PLAINTEXT_KEY_PROVIDER=true in production.`, + ); +} + function advancedWalletManagerEnvConfig(): Partial { const keyProviderUrl = readEnvVar('KEY_PROVIDER_URL'); @@ -238,6 +265,11 @@ function configureAdvancedWalletManagerMode(): AdvancedWalletManagerConfig { const env = advancedWalletManagerEnvConfig(); let config = mergeAkmConfigs(env); + validateKeyProviderUrl(config.keyProviderUrl, 'KEY_PROVIDER_URL', config.tlsMode); + if (config.backupKmsUrl) { + validateKeyProviderUrl(config.backupKmsUrl, 'BACKUP_KMS_URL', config.tlsMode); + } + // Certificate Loading Section logger.info('=== Certificate Loading ===');