From ab92e6892d986c3caf9a9a2e43df75f85395413f Mon Sep 17 00:00:00 2001 From: Pranav Jain Date: Wed, 9 Sep 2026 13:54:18 -0400 Subject: [PATCH] perf(sdk-core): pin high-entropy encryption to v1 TICKET: WCN-2616 --- modules/key-card/src/generateQrData.ts | 27 ++--- modules/key-card/src/types.ts | 1 + .../key-card/src/upgradeWalletEncryption.ts | 1 - modules/key-card/test/unit/generateQrData.ts | 18 ++-- modules/sdk-core/src/api/types.ts | 7 ++ .../sdk-core/src/bitgo/internal/keycard.ts | 8 +- modules/sdk-core/src/bitgo/wallet/iWallet.ts | 2 + modules/sdk-core/src/bitgo/wallet/wallet.ts | 18 +++- modules/sdk-core/src/bitgo/wallet/wallets.ts | 51 ++++++---- .../bitgo/wallet/walletsEncryptionVersion.ts | 98 +++++++++++-------- 10 files changed, 133 insertions(+), 98 deletions(-) diff --git a/modules/key-card/src/generateQrData.ts b/modules/key-card/src/generateQrData.ts index 4258126357..a0ca96952e 100644 --- a/modules/key-card/src/generateQrData.ts +++ b/modules/key-card/src/generateQrData.ts @@ -1,5 +1,5 @@ import { BaseCoin } from '@bitgo/statics'; -import { Keychain, KeychainsTriplet } from '@bitgo/sdk-core'; +import { HIGH_ENTROPY_ENCRYPTION_VERSION, Keychain, KeychainsTriplet } from '@bitgo/sdk-core'; import { encrypt } from '@bitgo/sdk-api'; import * as assert from 'assert'; import { @@ -136,10 +136,12 @@ function generateUserMasterPublicKeyQRData(publicKey: string): MasterPublicKeyQr async function generatePasscodeQrData( passphrase: string, passcodeEncryptionCode: string, - encryptionVersion?: 1 | 2, entityNoun: KeycardEntity = 'wallet' ): Promise { - const encryptedPasscode = await encrypt(passcodeEncryptionCode, passphrase, { encryptionVersion }); + // Box D uses the fixed version for its generated encryption key. + const encryptedPasscode = await encrypt(passcodeEncryptionCode, passphrase, { + encryptionVersion: HIGH_ENTROPY_ENCRYPTION_VERSION, + }); const titleNoun = entityNoun === 'safe' ? 'Safe' : 'Wallet'; return { title: `D: Encrypted ${titleNoun} Password`, @@ -199,11 +201,7 @@ export async function generateQrData(params: GenerateQrDataParams): Promise { let encryptedWalletPasscode: string | undefined; if (passphrase && passcodeEncryptionCode) { + // Box D uses its fixed version; the backup key follows encryptionVersion. encryptedWalletPasscode = await encrypt({ input: passphrase, password: passcodeEncryptionCode, - encryptionVersion, + encryptionVersion: HIGH_ENTROPY_ENCRYPTION_VERSION, }); } @@ -373,7 +374,8 @@ function renderKeycardPdf(options: DrawKeycardLayoutOptions, keyData: any): any /** * Draw a keycard into a new pdf document object. - * Defaults to v2 (Argon2id) encryption for Box D; pass `encryptionVersion: 1` for legacy v1. + * + * `encryptionVersion` applies to the backup key. Box D uses its fixed version. * @param options */ export async function drawKeycard(options: DrawKeycardOptions): Promise { diff --git a/modules/sdk-core/src/bitgo/wallet/iWallet.ts b/modules/sdk-core/src/bitgo/wallet/iWallet.ts index 9397bc0b24..9741ea9c10 100644 --- a/modules/sdk-core/src/bitgo/wallet/iWallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/iWallet.ts @@ -832,6 +832,7 @@ export interface ShareWalletOptions { */ skipKeychain?: boolean; disableEmail?: boolean; + /** @deprecated Ignored because the shared key is high entropy. */ encryptionVersion?: EncryptionVersion; /** * Pre-decrypted wallet keychain. When supplied, shareWallet skips its internal @@ -856,6 +857,7 @@ export interface BulkWalletShareOptions { path: string; permissions: string[]; }>; + /** @deprecated Ignored because the shared key is high entropy. */ encryptionVersion?: EncryptionVersion; } diff --git a/modules/sdk-core/src/bitgo/wallet/wallet.ts b/modules/sdk-core/src/bitgo/wallet/wallet.ts index ba8e7f971e..02f0ab5e31 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallet.ts @@ -8,7 +8,7 @@ import BigNumber from 'bignumber.js'; import * as t from 'io-ts'; import { BigIntFromString } from 'io-ts-types'; import * as _ from 'lodash'; -import { EncryptionVersion, IRequestTracer } from '../../api'; +import { EncryptionVersion, HIGH_ENTROPY_ENCRYPTION_VERSION, IRequestTracer } from '../../api'; import * as common from '../../common'; import { AddressBook, IAddressBook } from '../address-book'; import { @@ -2043,7 +2043,7 @@ export class Wallet implements IWallet { * @param pub - The wallet's public key * @param userPubkey - The recipient user's public key * @param path - The key path - * @param encryptionVersion - Optional encryption version (defaults to v2) + * @param encryptionVersion - Deprecated and ignored because the shared key is high entropy. * @returns The encrypted keychain for the recipient with all required fields */ async encryptPrvForUser( @@ -2051,11 +2051,18 @@ export class Wallet implements IWallet { pub: string, userPubkey: string, path: string, + /** @deprecated Ignored because the shared key is high entropy. */ encryptionVersion?: EncryptionVersion ): Promise { + void encryptionVersion; const eckey = makeRandomKey(); const secret = getSharedSecret(eckey, Buffer.from(userPubkey, 'hex')).toString('hex'); - const newEncryptedPrv = await this.bitgo.encrypt({ password: secret, input: decryptedPrv, encryptionVersion }); + // Use the fixed version for encryption to the recipient. + const newEncryptedPrv = await this.bitgo.encrypt({ + password: secret, + input: decryptedPrv, + encryptionVersion: HIGH_ENTROPY_ENCRYPTION_VERSION, + }); const keychain: BulkWalletShareKeychain = { pub, @@ -2093,12 +2100,15 @@ export class Wallet implements IWallet { * @param walletPassphrase - The passphrase to decrypt the keychain * @param pubkey - The recipient's public key * @param path - The key path + * @param encryptionVersion - Deprecated and ignored because the shared key is high entropy. + * @param decryptedKeychain - Pre-decrypted keychain for bulk sharing * @returns The encrypted keychain for the recipient */ async prepareSharedKeychain( walletPassphrase: string | undefined, pubkey: string, path: string, + /** @deprecated Ignored because the shared key is high entropy. */ encryptionVersion?: EncryptionVersion, decryptedKeychain?: DecryptedKeychainData ): Promise { @@ -3453,7 +3463,7 @@ export class Wallet implements IWallet { /** * Creates and downloads PDF keycard for wallet (requires response from wallets.generateWallet). - * Defaults to v2 encryption for Box D; pass `encryptionVersion: 1` for legacy v1. + * `encryptionVersion` applies to the backup key. Box D uses its fixed version. * * Note: this is example code and is not the version used on bitgo.com * diff --git a/modules/sdk-core/src/bitgo/wallet/wallets.ts b/modules/sdk-core/src/bitgo/wallet/wallets.ts index 88d7a4e19b..7303acbaf0 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallets.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallets.ts @@ -7,7 +7,7 @@ import { bip32 } from '@bitgo/utxo-lib'; import * as _ from 'lodash'; import { CoinFeature } from '@bitgo/statics'; -import { EncryptionVersion, IEncryptionSession, sanitizeLegacyPath } from '../../api'; +import { EncryptionVersion, HIGH_ENTROPY_ENCRYPTION_VERSION, IEncryptionSession, sanitizeLegacyPath } from '../../api'; import * as common from '../../common'; import { IBaseCoin, KeychainsTriplet, SupplementGenerateWalletOptions } from '../baseCoin'; import { BitGoBase } from '../bitgoBase'; @@ -173,6 +173,19 @@ export class Wallets implements IWallets { }; } + /** + * Encrypt the wallet passphrase for recovery and Box D. + * + * Box D uses a fixed version independent of the caller's keychain encryption version. + */ + private async encryptPassphraseForRecovery(passphrase: string, passcodeEncryptionCode: string): Promise { + return await this.bitgo.encrypt({ + input: passphrase, + password: passcodeEncryptionCode, + encryptionVersion: HIGH_ENTROPY_ENCRYPTION_VERSION, + }); + } + private async generateLightningWallet(params: GenerateLightningWalletOptions): Promise { const reqId = new RequestTracer(); this.bitgo.setRequestTracer(reqId); @@ -341,11 +354,10 @@ export class Wallets implements IWallets { ); const walletData = await this.generateLightningWallet(options); - walletData.encryptedWalletPassphrase = await this.bitgo.encrypt({ - input: options.passphrase, - password: options.passcodeEncryptionCode, - encryptionVersion: options.encryptionVersion, - }); + walletData.encryptedWalletPassphrase = await this.encryptPassphraseForRecovery( + options.passphrase, + options.passcodeEncryptionCode + ); return walletData; } @@ -362,11 +374,10 @@ export class Wallets implements IWallets { const walletData = await this.generateGoAccountWallet(options); if (options.passphrase !== undefined && options.passcodeEncryptionCode !== undefined) { - walletData.encryptedWalletPassphrase = await this.bitgo.encrypt({ - input: options.passphrase, - password: options.passcodeEncryptionCode, - encryptionVersion: options.encryptionVersion, - }); + walletData.encryptedWalletPassphrase = await this.encryptPassphraseForRecovery( + options.passphrase, + options.passcodeEncryptionCode + ); } return walletData; } @@ -472,11 +483,10 @@ export class Wallets implements IWallets { encryptionVersion: params.encryptionVersion, }); if (params.passcodeEncryptionCode) { - walletData.encryptedWalletPassphrase = await this.bitgo.encrypt({ - input: passphrase, - password: params.passcodeEncryptionCode, - encryptionVersion: params.encryptionVersion, - }); + walletData.encryptedWalletPassphrase = await this.encryptPassphraseForRecovery( + passphrase, + params.passcodeEncryptionCode + ); } return walletData; } @@ -745,11 +755,10 @@ export class Wallets implements IWallets { } if (canEncrypt && params.passcodeEncryptionCode) { - result.encryptedWalletPassphrase = await this.bitgo.encrypt({ - input: passphrase, - password: params.passcodeEncryptionCode, - encryptionVersion: params.encryptionVersion, - }); + result.encryptedWalletPassphrase = await this.encryptPassphraseForRecovery( + passphrase, + params.passcodeEncryptionCode + ); } return result; diff --git a/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts b/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts index ab908b00a6..cf08425c7b 100644 --- a/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts +++ b/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts @@ -3,6 +3,8 @@ import * as sinon from 'sinon'; import 'should'; import { Wallets } from '../../../../src/bitgo/wallet/wallets'; import { Wallet } from '../../../../src/bitgo/wallet/wallet'; +import { makeRandomKey } from '../../../../src/bitgo/bitcoin'; +import { HIGH_ENTROPY_ENCRYPTION_VERSION } from '../../../../src/api'; describe('Wallets - encryptionVersion threading', function () { let wallets: Wallets; @@ -213,8 +215,10 @@ describe('Wallets - encryptionVersion threading', function () { describe('Wallet.shareWallet / createBulkWalletShare', function () { let wallet: Wallet; + let recipientPubKey: string; beforeEach(function () { + recipientPubKey = makeRandomKey().publicKey.toString('hex'); const mockWalletData = { id: 'wallet-id', keys: ['key-1', 'key-2', 'key-3'], @@ -229,72 +233,84 @@ describe('Wallets - encryptionVersion threading', function () { wallet = new Wallet(mockBitGo, mockBaseCoin, mockWalletData); }); - it('shareWallet passes encryptionVersion to prepareSharedKeychain', async function () { - const prepareStub = sinon.stub(wallet, 'prepareSharedKeychain').resolves({}); - mockBitGo.getSharingKey = sinon.stub().resolves({ userId: 'user-id', pubkey: 'recvPub', path: 'm/0' }); - mockBitGo.post.returns({ send: sinon.stub().returns({ result: sinon.stub().resolves({}) }) }); + it('encryptPrvForUser encrypts to the ECDH secret at the high-entropy version', async function () { + await wallet.encryptPrvForUser('prv', 'pub', recipientPubKey, 'm/0', 2); - await wallet.shareWallet({ - email: 'test@test.com', - permissions: 'spend', - walletPassphrase: 'passphrase', - encryptionVersion: 2, + assert.ok(mockBitGo.encrypt.calledOnce, 'encrypt should have been called'); + assert.strictEqual(mockBitGo.encrypt.firstCall.args[0].encryptionVersion, HIGH_ENTROPY_ENCRYPTION_VERSION); + }); + + it('preserves the encryptionVersion position before a pre-decrypted keychain', async function () { + await wallet.prepareSharedKeychain(undefined, recipientPubKey, 'm/0', 2, { + prv: 'prv', + pub: 'pub', }); - assert.ok(prepareStub.calledOnce, 'prepareSharedKeychain should be called'); - assert.strictEqual(prepareStub.firstCall.args[3], 2, 'encryptionVersion should be forwarded'); + assert.strictEqual(mockBitGo.encrypt.firstCall.args[0].input, 'prv'); + assert.strictEqual(mockBitGo.encrypt.firstCall.args[0].encryptionVersion, HIGH_ENTROPY_ENCRYPTION_VERSION); }); - it('shareWallet passes encryptionVersion: undefined when not set', async function () { - const prepareStub = sinon.stub(wallet, 'prepareSharedKeychain').resolves({}); - mockBitGo.getSharingKey = sinon.stub().resolves({ userId: 'user-id', pubkey: 'recvPub', path: 'm/0' }); + it('shareWallet ignores a caller-supplied encryptionVersion for the shared keychain', async function () { + sinon.stub(wallet, 'getDecryptedKeychainForSharing').resolves({ prv: 'prv', pub: 'pub' }); + mockBitGo.getSharingKey = sinon.stub().resolves({ userId: 'user-id', pubkey: recipientPubKey, path: 'm/0' }); mockBitGo.post.returns({ send: sinon.stub().returns({ result: sinon.stub().resolves({}) }) }); await wallet.shareWallet({ email: 'test@test.com', permissions: 'spend', walletPassphrase: 'passphrase', + encryptionVersion: 2, }); - assert.ok(prepareStub.calledOnce); - assert.strictEqual(prepareStub.firstCall.args[3], undefined); + assert.ok(mockBitGo.encrypt.calledOnce, 'encrypt should have been called'); + assert.strictEqual( + mockBitGo.encrypt.firstCall.args[0].encryptionVersion, + HIGH_ENTROPY_ENCRYPTION_VERSION, + 'the ECDH-keyed share must not be encrypted at the caller-requested version' + ); }); - it('createBulkWalletShare passes encryptionVersion to encryptPrvForUser', async function () { - const encryptPrvStub = sinon - .stub(wallet, 'encryptPrvForUser') - .resolves({ encryptedPrv: 'enc', pub: 'pub', fromPubKey: 'fpk', toPubKey: 'tpk', path: 'm/0' }); - sinon - .stub(wallet as any, 'getDecryptedKeychainForSharing') - .resolves({ prv: 'prv', pub: 'pub', encryptedPrv: 'encPrv' }); - sinon.stub(wallet as any, 'createBulkKeyShares').resolves({ shares: [] }); + it('createBulkWalletShare ignores a caller-supplied encryptionVersion for every share', async function () { + sinon.stub(wallet, 'getDecryptedKeychainForSharing').resolves({ prv: 'prv', pub: 'pub' }); + sinon.stub(wallet, 'createBulkKeyShares').resolves({ shares: [] }); await wallet.createBulkWalletShare({ walletPassphrase: 'passphrase', - keyShareOptions: [{ userId: 'user-1', pubKey: 'pubKey', path: 'm/0', permissions: ['spend'] }], + keyShareOptions: [ + { userId: 'user-1', pubKey: recipientPubKey, path: 'm/0', permissions: ['spend'] }, + { userId: 'user-2', pubKey: recipientPubKey, path: 'm/1', permissions: ['spend'] }, + ], encryptionVersion: 2, }); - assert.ok(encryptPrvStub.calledOnce); - assert.strictEqual(encryptPrvStub.firstCall.args[4], 2, 'encryptionVersion should be forwarded'); + assert.strictEqual(mockBitGo.encrypt.callCount, 2, 'each recipient gets its own encryption'); + for (const call of mockBitGo.encrypt.getCalls()) { + assert.strictEqual(call.args[0].encryptionVersion, HIGH_ENTROPY_ENCRYPTION_VERSION); + } }); + }); - it('createBulkWalletShare passes encryptionVersion: undefined when not set', async function () { - const encryptPrvStub = sinon - .stub(wallet, 'encryptPrvForUser') - .resolves({ encryptedPrv: 'enc', pub: 'pub', fromPubKey: 'fpk', toPubKey: 'tpk', path: 'm/0' }); - sinon - .stub(wallet as any, 'getDecryptedKeychainForSharing') - .resolves({ prv: 'prv', pub: 'pub', encryptedPrv: 'encPrv' }); - sinon.stub(wallet as any, 'createBulkKeyShares').resolves({ shares: [] }); - - await wallet.createBulkWalletShare({ - walletPassphrase: 'passphrase', - keyShareOptions: [{ userId: 'user-1', pubKey: 'pubKey', path: 'm/0', permissions: ['spend'] }], + describe('Wallets.generateWallet', function () { + it('keeps human-passphrase encryption at the requested version and pins recovery encryption', async function () { + mockKeychains.createBackup = sinon.stub().resolves({ id: 'backup-key-id', pub: 'backup-pub' }); + mockKeychains.createBitGo = sinon.stub().resolves({ id: 'bitgo-key-id', pub: 'bitgo-pub' }); + mockBitGo.post.returns({ send: sinon.stub().returns({ result: sinon.stub().resolves({ id: 'wallet-id' }) }) }); + mockBaseCoin.getDefaultMultisigType = sinon.stub().returns('onchain'); + mockBaseCoin.isEVM = sinon.stub().returns(false); + mockBaseCoin.isValidMofNSetup = sinon.stub().returns(true); + mockBaseCoin.supplementGenerateWallet = sinon.stub().callsFake(async (params: unknown) => params); + mockBaseCoin.signMessage = sinon.stub().resolves(Buffer.from('aabbcc', 'hex')); + + await wallets.generateWallet({ + label: 'Test Wallet', + passphrase: 'wallet-passphrase', + passcodeEncryptionCode: 'recovery-code', + encryptionVersion: 2, }); - assert.ok(encryptPrvStub.calledOnce); - assert.strictEqual(encryptPrvStub.firstCall.args[4], undefined); + assert.strictEqual(mockBitGo.encrypt.callCount, 2); + assert.strictEqual(mockBitGo.encrypt.firstCall.args[0].encryptionVersion, 2); + assert.strictEqual(mockBitGo.encrypt.secondCall.args[0].encryptionVersion, HIGH_ENTROPY_ENCRYPTION_VERSION); }); }); });