diff --git a/modules/abstract-lightning/src/codecs/api/payment.ts b/modules/abstract-lightning/src/codecs/api/payment.ts index 11ab6ccc520..04b6c246fa2 100644 --- a/modules/abstract-lightning/src/codecs/api/payment.ts +++ b/modules/abstract-lightning/src/codecs/api/payment.ts @@ -123,6 +123,11 @@ export const SubmitPaymentParams = t.intersection([ t.partial({ sequenceId: optionalString, comment: optionalString, + /** + * Upgrade the stored encryption of your user auth key to the latest format after signing with your + * passphrase. Defaults to the `upgradeEncryptionOnSign` setting. + */ + upgradeEncryption: t.boolean, }), ]); diff --git a/modules/abstract-lightning/src/codecs/api/wallet.ts b/modules/abstract-lightning/src/codecs/api/wallet.ts index 69b813d959d..87aa20b1eb1 100644 --- a/modules/abstract-lightning/src/codecs/api/wallet.ts +++ b/modules/abstract-lightning/src/codecs/api/wallet.ts @@ -1,9 +1,16 @@ +import type { IRequestTracer } from '@bitgo/sdk-core'; import * as t from 'io-ts'; import { NonEmptyString } from 'io-ts-types'; import { getCodecPair } from '../shared'; // codec for lightning wallet and keychain related apis +export type UpgradeUserAuthKeyEncryptionParams = { + /** The wallet passphrase that decrypts the user auth key. */ + passphrase: string; + reqId?: IRequestTracer; +}; + export const KeyPurpose = t.union([t.literal('userAuth'), t.literal('nodeAuth')], 'KeyPurpose'); export type KeyPurpose = t.TypeOf; diff --git a/modules/abstract-lightning/src/codecs/api/withdraw.ts b/modules/abstract-lightning/src/codecs/api/withdraw.ts index 0b1da5be5d8..bd6ca868ab6 100644 --- a/modules/abstract-lightning/src/codecs/api/withdraw.ts +++ b/modules/abstract-lightning/src/codecs/api/withdraw.ts @@ -16,6 +16,11 @@ export const LightningOnchainWithdrawParams = t.intersection([ t.partial({ sequenceId: optionalString, comment: optionalString, + /** + * Upgrade the stored encryption of your user auth key to the latest format after signing with your + * passphrase. Defaults to the `upgradeEncryptionOnSign` setting. + */ + upgradeEncryption: t.boolean, }), ]); export type LightningOnchainWithdrawParams = t.TypeOf; diff --git a/modules/abstract-lightning/src/wallet/lightning.ts b/modules/abstract-lightning/src/wallet/lightning.ts index 8198b37533c..313b3df9590 100644 --- a/modules/abstract-lightning/src/wallet/lightning.ts +++ b/modules/abstract-lightning/src/wallet/lightning.ts @@ -8,6 +8,9 @@ import { commonTssMethods, TxRequestState, decodeOrElse, + KeyEncryptionUpgradeFailureReason, + KeyEncryptionUpgradeResult, + shouldUpgradeEncryption, } from '@bitgo/sdk-core'; import * as t from 'io-ts'; import { @@ -35,6 +38,7 @@ import { ListPaymentsResponse, LndCreateWithdrawResponse, WatchOnly, + UpgradeUserAuthKeyEncryptionParams, } from '../codecs'; import { LightningPaymentIntent, LightningPaymentRequest } from '@bitgo/public-types'; @@ -161,6 +165,15 @@ export interface ILightningWallet { */ payInvoice(params: SubmitPaymentParams): Promise; + /** + * Upgrade the stored encryption of the user auth key to the latest format, using the wallet + * passphrase. Use it after a payment or withdrawal to run the upgrade and show the outcome. It + * applies regardless of the `upgradeEncryptionOnSign` setting and never throws. + * @param params.passphrase - the wallet passphrase that decrypts the user auth key + * @param params.reqId - optional request tracer + */ + upgradeUserAuthKeyEncryption(params: UpgradeUserAuthKeyEncryptionParams): Promise; + /** * On chain withdrawal * @param {LightningOnchainWithdrawParams} params - Withdraw parameters @@ -254,6 +267,18 @@ export class LightningWallet implements ILightningWallet { }); } + async upgradeUserAuthKeyEncryption(params: UpgradeUserAuthKeyEncryptionParams): Promise { + let userAuthKey: LightningAuthKeychain; + try { + ({ userAuthKey } = await getLightningAuthKeychains(this.wallet)); + } catch (e) { + return { upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }; + } + return this.wallet.baseCoin + .keychains() + .upgradeEncryption({ keychain: userAuthKey, passphrase: params.passphrase, reqId: params.reqId }); + } + async payInvoice(params: SubmitPaymentParams): Promise { const reqId = new RequestTracer(); this.wallet.bitgo.setRequestTracer(reqId); @@ -267,6 +292,9 @@ export class LightningWallet implements ILightningWallet { t.exact(LightningPaymentRequest).encode(params), await this.wallet.bitgo.decrypt({ password: params.passphrase, input: userAuthKeyEncryptedPrv }) ); + if (shouldUpgradeEncryption(this.wallet.bitgo, params.upgradeEncryption)) { + void this.wallet.baseCoin.keychains().upgradeEncryption({ keychain: userAuthKey, passphrase: params.passphrase }); + } const paymentIntent: { intent: LightningPaymentIntent } = { intent: { @@ -392,6 +420,9 @@ export class LightningWallet implements ILightningWallet { transactionRequestCreate.transactions[0].unsignedTx.serializedTxHex, await this.wallet.bitgo.decrypt({ password: params.passphrase, input: userAuthKeyEncryptedPrv }) ); + if (shouldUpgradeEncryption(this.wallet.bitgo, params.upgradeEncryption)) { + void this.wallet.baseCoin.keychains().upgradeEncryption({ keychain: userAuthKey, passphrase: params.passphrase }); + } const transactionRequestWithSignature = (await this.wallet.bitgo .put( diff --git a/modules/bitgo/test/v2/unit/keychains.ts b/modules/bitgo/test/v2/unit/keychains.ts index caf1269742e..c04691426f8 100644 --- a/modules/bitgo/test/v2/unit/keychains.ts +++ b/modules/bitgo/test/v2/unit/keychains.ts @@ -9,7 +9,17 @@ import nock = require('nock'); import should = require('should'); import * as sinon from 'sinon'; -import { common, decodeOrElse, ECDSAUtils, EDDSAUtils, Keychain, Keychains, OvcShare } from '@bitgo/sdk-core'; +import { + common, + decodeOrElse, + ECDSAUtils, + EDDSAUtils, + KeyEncryptionUpgradeFailureReason, + KeyEncryptionUpgradeSkipReason, + Keychain, + Keychains, + OvcShare, +} from '@bitgo/sdk-core'; import { TestBitGo } from '@bitgo/sdk-test'; import { BitGo } from '../../../src/bitgo'; import { SinonStub } from 'sinon'; @@ -1467,4 +1477,226 @@ describe('V2 Keychains', function () { await keychains.reencryptAsV2(encrypted, 'wrongPass').should.be.rejected(); }); }); + + describe('upgradeEncryption', function () { + const prv = 'xprvUpgradeTestKey'; + const passphrase = 'upgradePass'; + const keyId = 'upgradeKeyId'; + let sandbox: sinon.SinonSandbox; + let encryptedV1: string; + type UpgradeBody = { + encryptedPrv: string; + existingEncryptedPrv: string; + encryptionUpgrade: boolean; + pub?: string; + }; + + before(async function () { + encryptedV1 = await bitgo.encrypt({ input: prv, password: passphrase, encryptionVersion: 1 }); + }); + + beforeEach(function () { + sandbox = sinon.createSandbox(); + }); + + afterEach(function () { + sandbox.restore(); + nock.cleanAll(); + // Every upgrade a test starts must have settled, so none is shared with the next test. + keychains.upgradesInFlight.size.should.equal(0); + }); + + function v1Keychain(overrides: Partial = {}): Keychain { + return { id: keyId, pub: 'xpubUpgrade', encryptedPrv: encryptedV1, type: 'independent', ...overrides }; + } + + function mockPut(status = 200, times = 1) { + const bodies: UpgradeBody[] = []; + const scope = nock(bgUrl) + .put(`/api/v2/tltc/key/${keyId}`) + .times(times) + .reply((uri, body) => { + bodies.push(body as UpgradeBody); + return [status, status === 200 ? { id: keyId } : { error: 'failed' }]; + }); + return { scope, bodies }; + } + + it('upgrades a v1 key by sending the v2 encryption of the same prv with existingEncryptedPrv', async function () { + const keychain = v1Keychain(); + const { scope, bodies } = mockPut(); + + const result = await keychains.upgradeEncryption({ keychain, passphrase }); + + result.should.deepEqual({ upgraded: true }); + scope.isDone().should.be.true(); + bodies.should.have.length(1); + const body = bodies[0]; + body.encryptionUpgrade.should.equal(true); + body.existingEncryptedPrv.should.equal(encryptedV1); + body.should.not.have.property('pub'); + JSON.parse(body.encryptedPrv).v.should.equal(2); + (await bitgo.decrypt({ input: body.encryptedPrv, password: passphrase })).should.equal(prv); + should(keychain.encryptedPrv).equal(encryptedV1); + }); + + it('upgrades when called directly, even with upgradeEncryptionOnSign off', async function () { + sandbox.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(false); + const { scope, bodies } = mockPut(); + + const result = await keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }); + + result.should.deepEqual({ upgraded: true }); + scope.isDone().should.be.true(); + bodies.should.have.length(1); + }); + + it('skips a key that is already v2', async function () { + const encryptedV2 = await bitgo.encrypt({ input: prv, password: passphrase, encryptionVersion: 2 }); + const { scope } = mockPut(); + + const result = await keychains.upgradeEncryption({ + keychain: v1Keychain({ encryptedPrv: encryptedV2 }), + passphrase, + }); + + result.should.deepEqual({ upgraded: false, skipped: KeyEncryptionUpgradeSkipReason.AlreadyUpgraded }); + scope.isDone().should.be.false(); + }); + + it('reports an error without writing when encryptedPrv cannot be parsed', async function () { + const { scope } = mockPut(); + + const result = await keychains.upgradeEncryption({ + keychain: v1Keychain({ encryptedPrv: 'not json' }), + passphrase, + }); + + result.should.deepEqual({ upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }); + scope.isDone().should.be.false(); + }); + + it('reports an error without writing when the keychain has no id', async function () { + const { scope } = mockPut(); + + const result = await keychains.upgradeEncryption({ + keychain: v1Keychain({ id: undefined as unknown as string }), + passphrase, + }); + + result.should.deepEqual({ upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }); + scope.isDone().should.be.false(); + }); + + it('reports an error without writing when the passphrase is wrong', async function () { + const { scope } = mockPut(); + + const result = await keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase: 'wrongPass' }); + + result.should.deepEqual({ upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }); + scope.isDone().should.be.false(); + }); + + it('reports keyChanged when the key changed on the server (409)', async function () { + const { scope } = mockPut(409); + + const result = await keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }); + + scope.isDone().should.be.true(); + result.should.deepEqual({ upgraded: false, skipped: KeyEncryptionUpgradeSkipReason.KeyChanged }); + }); + + it('reports an error when the server fails with 500', async function () { + const { scope } = mockPut(500); + + const result = await keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }); + + scope.isDone().should.be.true(); + result.should.deepEqual({ upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }); + }); + + it('reports unavailable when the server rejects the upgrade with 503', async function () { + const { scope } = mockPut(503); + + const result = await keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }); + + scope.isDone().should.be.true(); + result.should.deepEqual({ upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Unavailable }); + }); + + it('reports an error when the request fails at the network level', async function () { + const scope = nock(bgUrl).put(`/api/v2/tltc/key/${keyId}`).replyWithError('socket hang up'); + + const result = await keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }); + + scope.isDone().should.be.true(); + result.should.deepEqual({ upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }); + }); + + it('reports verificationFailed without writing when the new encryption does not decrypt back to the same prv', async function () { + const decrypt = sandbox.stub(bitgo, 'decrypt').callThrough(); + decrypt.onSecondCall().resolves('other'); + const { scope } = mockPut(); + + const result = await keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }); + + decrypt.callCount.should.equal(2); + result.should.deepEqual({ upgraded: false, failed: KeyEncryptionUpgradeFailureReason.VerificationFailed }); + scope.isDone().should.be.false(); + }); + + it('sends one write for concurrent upgrades of the same key and gives both callers its outcome', async function () { + const { bodies } = mockPut(200, 2); + + const results = await Promise.all([ + keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }), + keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }), + ]); + + bodies.should.have.length(1); + results.should.deepEqual([{ upgraded: true }, { upgraded: true }]); + }); + + it('starts a new upgrade of the same key after the previous one settled', async function () { + const { bodies } = mockPut(200, 2); + + const first = await keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }); + const second = await keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }); + + first.should.deepEqual({ upgraded: true }); + second.should.deepEqual({ upgraded: true }); + bodies.should.have.length(2); + }); + + it('starts a new upgrade of the same key after the previous one was skipped', async function () { + const { bodies } = mockPut(409, 2); + + const first = await keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }); + const second = await keychains.upgradeEncryption({ keychain: v1Keychain(), passphrase }); + + first.should.deepEqual({ upgraded: false, skipped: KeyEncryptionUpgradeSkipReason.KeyChanged }); + second.should.deepEqual({ upgraded: false, skipped: KeyEncryptionUpgradeSkipReason.KeyChanged }); + bodies.should.have.length(2); + }); + + it('includes pub for an OFC multi-user key', async function () { + const ofcKeychains = bitgo.coin('ofc').keychains(); + const bodies: UpgradeBody[] = []; + const scope = nock(bgUrl) + .put(`/api/v2/ofc/key/${keyId}`) + .reply((uri, body) => { + bodies.push(body as UpgradeBody); + return [200, { id: keyId }]; + }); + + await ofcKeychains.upgradeEncryption({ + keychain: v1Keychain({ coinSpecific: { ofc: { features: ['multi-user-key'] } } }), + passphrase, + }); + + scope.isDone().should.be.true(); + should(bodies[0].pub).equal('xpubUpgrade'); + bodies[0].encryptionUpgrade.should.equal(true); + }); + }); }); diff --git a/modules/bitgo/test/v2/unit/lightning/lightningWallets.ts b/modules/bitgo/test/v2/unit/lightning/lightningWallets.ts index df4a0a8cc6e..a9b5cbcea52 100644 --- a/modules/bitgo/test/v2/unit/lightning/lightningWallets.ts +++ b/modules/bitgo/test/v2/unit/lightning/lightningWallets.ts @@ -1,7 +1,16 @@ import * as assert from 'assert'; import { TestBitGo } from '@bitgo/sdk-test'; import nock = require('nock'); -import { BaseCoin, PendingApprovalData, State, Type } from '@bitgo/sdk-core'; +import * as sinon from 'sinon'; +import { + BaseCoin, + KeyEncryptionUpgradeFailureReason, + Keychains, + PendingApprovalData, + RequestTracer, + State, + Type, +} from '@bitgo/sdk-core'; import { CreateInvoiceBody, getLightningWallet, @@ -68,6 +77,37 @@ describe('Lightning wallets', function () { nock.pendingMocks().length.should.equal(0); }); + afterEach(function () { + sinon.restore(); + }); + + type UpgradeBody = { encryptedPrv: string; existingEncryptedPrv: string; encryptionUpgrade: boolean }; + type UpgradeNock = { scope: nock.Scope; bodies: UpgradeBody[] }; + + /** Nock the fire-and-forget v1 to v2 upgrade PUT of the user auth key and capture its body. */ + function nockUserAuthKeyUpgrade(): UpgradeNock { + const bodies: UpgradeBody[] = []; + const scope = nock(bgUrl) + .put('/api/v2/' + coinName + '/key/def') + .reply((uri, body) => { + bodies.push(body as UpgradeBody); + return [200, userAuthKey]; + }); + return { scope, bodies }; + } + + async function assertUserAuthKeyUpgraded(upgrade: sinon.SinonSpy, upgradeNock: UpgradeNock, passphrase: string) { + assert.strictEqual(upgrade.callCount, 1); + assert.strictEqual(upgrade.firstCall.args[0].keychain.id, userAuthKey.id); + assert.strictEqual(upgrade.firstCall.args[0].passphrase, passphrase); + await upgrade.firstCall.returnValue; + assert.ok(upgradeNock.scope.isDone()); + const body = upgradeNock.bodies[0]; + assert.strictEqual(body.encryptionUpgrade, true); + assert.strictEqual(body.existingEncryptedPrv, userAuthKey.encryptedPrv); + assert.strictEqual(JSON.parse(body.encryptedPrv).v, 2); + } + describe('Generate lightning wallet', function () { it('should validate parameters', async function () { await wallets @@ -358,7 +398,8 @@ describe('Lightning wallets', function () { createInvoiceNock.done(); }); - it('should pay invoice', async function () { + /** Nock every request a successful payInvoice makes, except the auth key upgrade. */ + function nockPayInvoice() { const params: SubmitPaymentParams = { invoice: 'lnbc1...', amountMsat: 1000n, @@ -403,7 +444,7 @@ describe('Lightning wallets', function () { }; const createTxRequestNock = nock(bgUrl) - .post(`/api/v2/wallet/${wallet.wallet.id()}/txrequests`) + .post(`/api/v2/wallet/${wallet.wallet.id()}/txrequests`, (body) => !('upgradeEncryption' in body)) .reply(200, txRequestResponse); const createTransferNock = nock(bgUrl) @@ -421,6 +462,19 @@ describe('Lightning wallets', function () { .get('/api/v2/' + coinName + '/key/ghi') .reply(200, nodeAuthKey); + return { + params, + finalPaymentResponse, + scopes: [createTxRequestNock, createTransferNock, sendTxRequestNock, userAuthKeyNock, nodeAuthKeyNock], + }; + } + + it('should pay invoice', async function () { + sinon.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(true); + const { params, finalPaymentResponse, scopes } = nockPayInvoice(); + const upgrade = sinon.spy(Keychains.prototype, 'upgradeEncryption'); + const upgradeNock = nockUserAuthKeyUpgrade(); + const response = await wallet.payInvoice(params); assert.strictEqual(response.txRequestId, 'txReq123'); assert.strictEqual(response.txRequestState, 'delivered'); @@ -446,11 +500,40 @@ describe('Lightning wallets', function () { finalPaymentResponse.transactions[0].unsignedTx.coinSpecific.paymentPreimage ); - createTxRequestNock.done(); - sendTxRequestNock.done(); - createTransferNock.done(); - userAuthKeyNock.done(); - nodeAuthKeyNock.done(); + scopes.forEach((scope) => scope.done()); + await assertUserAuthKeyUpgraded(upgrade, upgradeNock, params.passphrase); + }); + + it('should not upgrade the user auth key with the default setting', async function () { + const { params, scopes } = nockPayInvoice(); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await wallet.payInvoice(params); + + scopes.forEach((scope) => scope.done()); + assert.strictEqual(upgrade.callCount, 0); + }); + + it('should upgrade the user auth key when the call opts in', async function () { + const { params, scopes } = nockPayInvoice(); + const upgrade = sinon.spy(Keychains.prototype, 'upgradeEncryption'); + const upgradeNock = nockUserAuthKeyUpgrade(); + + await wallet.payInvoice({ ...params, upgradeEncryption: true }); + + scopes.forEach((scope) => scope.done()); + await assertUserAuthKeyUpgraded(upgrade, upgradeNock, params.passphrase); + }); + + it('should not upgrade the user auth key when the setting is on and the call opts out', async function () { + sinon.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(true); + const { params, scopes } = nockPayInvoice(); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await wallet.payInvoice({ ...params, upgradeEncryption: false }); + + scopes.forEach((scope) => scope.done()); + assert.strictEqual(upgrade.callCount, 0); }); it('should handle pending approval when paying invoice', async function () { @@ -604,6 +687,62 @@ describe('Lightning wallets', function () { }); }); + describe('upgradeUserAuthKeyEncryption', function () { + const passphrase = 'password123'; + let wallet: LightningWallet; + beforeEach(function () { + wallet = getLightningWallet( + new Wallet(bitgo, basecoin, { + id: 'walletId', + coin: 'tlnbtc', + subType: 'lightningCustody', + coinSpecific: { keys: ['def', 'ghi'] }, + }) + ) as LightningWallet; + }); + + afterEach(function () { + nock.cleanAll(); + }); + + it('upgrades the user auth key and returns the result', async function () { + const userAuthKeyNock = nock(bgUrl) + .get('/api/v2/' + coinName + '/key/def') + .reply(200, userAuthKey); + const nodeAuthKeyNock = nock(bgUrl) + .get('/api/v2/' + coinName + '/key/ghi') + .reply(200, nodeAuthKey); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + const reqId = new RequestTracer(); + + const result = await wallet.upgradeUserAuthKeyEncryption({ passphrase, reqId }); + + assert.deepStrictEqual(result, { upgraded: true }); + sinon.assert.calledOnce(upgrade); + assert.strictEqual(upgrade.firstCall.args[0]?.passphrase, passphrase); + assert.strictEqual(upgrade.firstCall.args[0]?.reqId, reqId); + userAuthKeyNock.done(); + nodeAuthKeyNock.done(); + }); + + it('reports an error when the auth keychains cannot be fetched', async function () { + const userAuthKeyNock = nock(bgUrl) + .get('/api/v2/' + coinName + '/key/def') + .reply(500, { error: 'internal error' }); + nock(bgUrl) + .get('/api/v2/' + coinName + '/key/ghi') + .reply(200, nodeAuthKey); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption'); + + const result = await wallet.upgradeUserAuthKeyEncryption({ passphrase }); + + assert.deepStrictEqual(result, { upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }); + sinon.assert.notCalled(upgrade); + userAuthKeyNock.done(); + }); + }); + describe('Get lightning key(s)', function () { const walletData = { id: 'fakeid', @@ -822,7 +961,9 @@ describe('Lightning wallets', function () { }) ) as LightningWallet; }); - it('should withdraw on chain', async function () { + + /** Nock every request a successful withdrawOnchain makes, except the auth key upgrade. */ + function nockWithdrawOnchain() { const params: LightningOnchainWithdrawParams = { recipients: [ { @@ -949,7 +1090,7 @@ describe('Lightning wallets', function () { }; const createTxRequestNock = nock(bgUrl) - .post(`/api/v2/wallet/${wallet.wallet.id()}/txrequests`) + .post(`/api/v2/wallet/${wallet.wallet.id()}/txrequests`, (body) => !('upgradeEncryption' in body)) .reply(200, txRequestResponse); const storeSignatureNock = nock(bgUrl) @@ -971,6 +1112,25 @@ describe('Lightning wallets', function () { .get('/api/v2/' + coinName + '/key/ghi') .reply(200, nodeAuthKey); + return { + params, + scopes: [ + userAuthKeyNock, + nodeAuthKeyNock, + createTxRequestNock, + storeSignatureNock, + createTransferNock, + sendTxRequestNock, + ], + }; + } + + it('should withdraw on chain', async function () { + sinon.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(true); + const { params, scopes } = nockWithdrawOnchain(); + const upgrade = sinon.spy(Keychains.prototype, 'upgradeEncryption'); + const upgradeNock = nockUserAuthKeyUpgrade(); + const response = await wallet.withdrawOnchain(params); assert.strictEqual(response.txRequestId, 'txReq123'); assert.strictEqual(response.txRequestState, 'delivered'); @@ -978,12 +1138,40 @@ describe('Lightning wallets', function () { assert.strictEqual(response.withdrawStatus?.txid, 'tx123'); assert.strictEqual((response.withdrawStatus as any).signature, undefined); - userAuthKeyNock.done(); - nodeAuthKeyNock.done(); - createTxRequestNock.done(); - storeSignatureNock.done(); - createTransferNock.done(); - sendTxRequestNock.done(); + scopes.forEach((scope) => scope.done()); + await assertUserAuthKeyUpgraded(upgrade, upgradeNock, params.passphrase); + }); + + it('should not upgrade the user auth key with the default setting', async function () { + const { params, scopes } = nockWithdrawOnchain(); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await wallet.withdrawOnchain(params); + + scopes.forEach((scope) => scope.done()); + assert.strictEqual(upgrade.callCount, 0); + }); + + it('should upgrade the user auth key when the call opts in', async function () { + const { params, scopes } = nockWithdrawOnchain(); + const upgrade = sinon.spy(Keychains.prototype, 'upgradeEncryption'); + const upgradeNock = nockUserAuthKeyUpgrade(); + + await wallet.withdrawOnchain({ ...params, upgradeEncryption: true }); + + scopes.forEach((scope) => scope.done()); + await assertUserAuthKeyUpgraded(upgrade, upgradeNock, params.passphrase); + }); + + it('should not upgrade the user auth key when the setting is on and the call opts out', async function () { + sinon.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(true); + const { params, scopes } = nockWithdrawOnchain(); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await wallet.withdrawOnchain({ ...params, upgradeEncryption: false }); + + scopes.forEach((scope) => scope.done()); + assert.strictEqual(upgrade.callCount, 0); }); it('should handle pending approval when withdrawing onchain', async function () { diff --git a/modules/bitgo/test/v2/unit/pendingApproval.ts b/modules/bitgo/test/v2/unit/pendingApproval.ts index 77a3b6fd119..985ee9002aa 100644 --- a/modules/bitgo/test/v2/unit/pendingApproval.ts +++ b/modules/bitgo/test/v2/unit/pendingApproval.ts @@ -14,6 +14,7 @@ import { ECDSAUtils, EddsaUtils, Environments, + Keychain, Keychains, PendingApproval, PendingApprovalData, @@ -386,6 +387,106 @@ describe('Pending Approvals:', () => { sandbox.verify(); }); + + describe('v1 to v2 user key upgrade on sign', function () { + const walletPassphrase = 'test'; + const userKeychain: Keychain = { id: 'userKeyId', type: 'tss', commonKeychain: 'ck', encryptedPrv: '{"ct":"x"}' }; + const txRequest = { + txRequestId: 'test', + apiVersion: 'lite', + unsignedTxs: [{ signableHex: 'randomhex', serializedTxHex: 'randomhex2', derivationPath: 'm/0' }], + } as unknown as TxRequest; + let upgradeOnSignSetting: sinon.SinonStub; + + beforeEach(() => { + upgradeOnSignSetting = sandbox.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(true); + }); + + it('triggers the upgrade for the user keychain after getPrv, without waiting for it', async () => { + pendingApprovalData['txRequestId'] = 'requestTxIdTest'; + const pendingApproval = new PendingApproval(bitgo, coin, pendingApprovalData, wallet); + sandbox.stub(Keychains.prototype, 'getKeysForSigning').resolves([userKeychain]); + const getPrv = sandbox.stub(Wallet.prototype, 'getPrv').resolves('decryptedPrv'); + const upgrade = sandbox.stub(Keychains.prototype, 'upgradeEncryption').returns(new Promise(() => undefined)); + sandbox.stub(TssUtils.prototype, 'recreateTxRequest').resolves(txRequest); + + const recreated = await pendingApproval.recreateAndSignTSSTransaction( + { walletPassphrase }, + new RequestTracer() + ); + + recreated.should.deepEqual({ txHex: 'randomhex2' }); + sinon.assert.calledOnce(getPrv); + sinon.assert.calledOnce(upgrade); + sinon.assert.calledWithExactly(upgrade, { keychain: userKeychain, passphrase: walletPassphrase }); + sinon.assert.callOrder(getPrv, upgrade); + }); + + it('does not upgrade when getPrv fails', async () => { + pendingApprovalData['txRequestId'] = 'requestTxIdTest'; + const pendingApproval = new PendingApproval(bitgo, coin, pendingApprovalData, wallet); + sandbox.stub(Keychains.prototype, 'getKeysForSigning').resolves([userKeychain]); + sandbox.stub(Wallet.prototype, 'getPrv').rejects(new Error('error decrypting wallet private key')); + const upgrade = sandbox.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await pendingApproval + .recreateAndSignTSSTransaction({ walletPassphrase }, new RequestTracer()) + .should.be.rejectedWith('error decrypting wallet private key'); + sinon.assert.notCalled(upgrade); + }); + + const setupGatingTest = () => { + pendingApprovalData['txRequestId'] = 'requestTxIdTest'; + const pendingApproval = new PendingApproval(bitgo, coin, pendingApprovalData, wallet); + sandbox.stub(Keychains.prototype, 'getKeysForSigning').resolves([userKeychain]); + sandbox.stub(Wallet.prototype, 'getPrv').resolves('decryptedPrv'); + const upgrade = sandbox.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + sandbox.stub(TssUtils.prototype, 'recreateTxRequest').resolves(txRequest); + return { pendingApproval, upgrade }; + }; + + it('does not upgrade with the default setting', async () => { + upgradeOnSignSetting.restore(); + const { pendingApproval, upgrade } = setupGatingTest(); + + await pendingApproval.recreateAndSignTSSTransaction({ walletPassphrase }, new RequestTracer()); + + sinon.assert.notCalled(upgrade); + }); + + it('does not upgrade when upgradeEncryptionOnSign is off', async () => { + upgradeOnSignSetting.returns(false); + const { pendingApproval, upgrade } = setupGatingTest(); + + await pendingApproval.recreateAndSignTSSTransaction({ walletPassphrase }, new RequestTracer()); + + sinon.assert.notCalled(upgrade); + }); + + it('upgrades when upgradeEncryptionOnSign is off and the call passes upgradeEncryption: true', async () => { + upgradeOnSignSetting.returns(false); + const { pendingApproval, upgrade } = setupGatingTest(); + + await pendingApproval.recreateAndSignTSSTransaction( + { walletPassphrase, upgradeEncryption: true }, + new RequestTracer() + ); + + sinon.assert.calledOnce(upgrade); + sinon.assert.calledWithExactly(upgrade, { keychain: userKeychain, passphrase: walletPassphrase }); + }); + + it('does not upgrade when the call passes upgradeEncryption: false', async () => { + const { pendingApproval, upgrade } = setupGatingTest(); + + await pendingApproval.recreateAndSignTSSTransaction( + { walletPassphrase, upgradeEncryption: false }, + new RequestTracer() + ); + + sinon.assert.notCalled(upgrade); + }); + }); }); describe('recreateAndSignTransaction', function () { diff --git a/modules/bitgo/test/v2/unit/wallet.ts b/modules/bitgo/test/v2/unit/wallet.ts index 60b563514af..0de3c720ff6 100644 --- a/modules/bitgo/test/v2/unit/wallet.ts +++ b/modules/bitgo/test/v2/unit/wallet.ts @@ -16,6 +16,8 @@ import { ECDSAUtils, EDDSAUtils, GetUserPrvOptions, + Keychain, + KeyEncryptionUpgradeFailureReason, Keychains, KeyType, ManageUnspentsOptions, @@ -38,6 +40,7 @@ import { TypedMessage, Wallet, WalletSignMessageOptions, + WalletSignTransactionOptions, WalletSignTypedDataOptions, } from '@bitgo/sdk-core'; @@ -413,6 +416,254 @@ describe('V2 Wallet:', function () { }); }); + describe('V1 to v2 user key upgrade on signing', () => { + const walletPassphrase = 'upgradePassphrase'; + const signed = { txHex: 'signedTxHex' }; + let userKeychain: Keychain; + let upgradeOnSignSetting: sinon.SinonStub; + + before(async function () { + userKeychain = { + id: wallet.keyIds()[0], + pub: 'xpubUser', + encryptedPrv: await bitgo.encrypt({ input: 'xprv1', password: walletPassphrase, encryptionVersion: 1 }), + type: 'independent', + }; + }); + + beforeEach(function () { + upgradeOnSignSetting = sinon.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(true); + }); + + afterEach(function () { + nock.cleanAll(); + }); + + function signParams(overrides: Partial = {}): WalletSignTransactionOptions { + return { + txPrebuild: { txHex: 'unsignedTxHex' }, + keychain: userKeychain, + walletPassphrase, + pubs: ['pub0', 'pub1', 'pub2'], + ...overrides, + }; + } + + it('triggers the upgrade with the user keychain and passphrase without changing the signing result', async function () { + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + const coinSign = sinon.stub(basecoin, 'signTransaction').resolves(signed); + + const result = await wallet.signTransaction(signParams()); + + result.should.deepEqual(signed); + sinon.assert.calledOnce(coinSign); + coinSign.firstCall.args[0].prv.should.equal('xprv1'); + sinon.assert.calledOnce(upgrade); + sinon.assert.calledWithExactly(upgrade, { keychain: userKeychain, passphrase: walletPassphrase }); + }); + + it('does not wait for the upgrade to settle', async function () { + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').returns(new Promise(() => undefined)); + sinon.stub(basecoin, 'signTransaction').resolves(signed); + + (await wallet.signTransaction(signParams())).should.deepEqual(signed); + sinon.assert.calledOnce(upgrade); + }); + + it('sends the upgrade PUT and still signs when the PUT fails', async function () { + const upgrade = sinon.spy(Keychains.prototype, 'upgradeEncryption'); + sinon.stub(basecoin, 'signTransaction').resolves(signed); + let putBody: { encryptionUpgrade?: boolean; existingEncryptedPrv?: string } | undefined; + const putNock = nock(bgUrl) + .put(`/api/v2/tbtc/key/${userKeychain.id}`) + .reply((uri, body) => { + putBody = body as typeof putBody; + return [500, { error: 'internal error' }]; + }); + + (await wallet.signTransaction(signParams())).should.deepEqual(signed); + await upgrade.firstCall.returnValue; + + putNock.isDone().should.be.true(); + should(putBody?.encryptionUpgrade).equal(true); + should(putBody?.existingEncryptedPrv).equal(userKeychain.encryptedPrv); + }); + + it('does not upgrade when the caller supplies prv', async function () { + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + sinon.stub(basecoin, 'signTransaction').resolves(signed); + + await wallet.signTransaction(signParams({ prv: 'xprv1' })); + + sinon.assert.notCalled(upgrade); + }); + + it('does not upgrade when only getUserPrv is called', async function () { + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + (await wallet.getUserPrv({ keychain: userKeychain, walletPassphrase })).should.equal('xprv1'); + + sinon.assert.notCalled(upgrade); + }); + + it('does not upgrade with the default setting', async function () { + upgradeOnSignSetting.restore(); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + sinon.stub(basecoin, 'signTransaction').resolves(signed); + + (await wallet.signTransaction(signParams())).should.deepEqual(signed); + + sinon.assert.notCalled(upgrade); + }); + + it('upgrades when BitGo is constructed with upgradeEncryptionOnSign: true', async function () { + upgradeOnSignSetting.restore(); + const optedInBitgo = TestBitGo.decorate(BitGo, { env: 'test', upgradeEncryptionOnSign: true }); + const optedInCoin = optedInBitgo.coin('tbtc'); + const optedInWallet = new Wallet(optedInBitgo, optedInCoin, walletData); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + sinon.stub(optedInCoin, 'signTransaction').resolves(signed); + + (await optedInWallet.signTransaction(signParams())).should.deepEqual(signed); + + sinon.assert.calledOnce(upgrade); + sinon.assert.calledWithExactly(upgrade, { keychain: userKeychain, passphrase: walletPassphrase }); + }); + + it('does not upgrade when upgradeEncryptionOnSign is off', async function () { + upgradeOnSignSetting.returns(false); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + sinon.stub(basecoin, 'signTransaction').resolves(signed); + + (await wallet.signTransaction(signParams())).should.deepEqual(signed); + + sinon.assert.notCalled(upgrade); + }); + + it('upgrades when upgradeEncryptionOnSign is off and the call passes upgradeEncryption: true', async function () { + upgradeOnSignSetting.returns(false); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + sinon.stub(basecoin, 'signTransaction').resolves(signed); + + await wallet.signTransaction(signParams({ upgradeEncryption: true })); + + sinon.assert.calledOnce(upgrade); + sinon.assert.calledWithExactly(upgrade, { keychain: userKeychain, passphrase: walletPassphrase }); + }); + + it('does not upgrade when the call passes upgradeEncryption: false', async function () { + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + sinon.stub(basecoin, 'signTransaction').resolves(signed); + + await wallet.signTransaction(signParams({ upgradeEncryption: false })); + + sinon.assert.notCalled(upgrade); + }); + + it('reads upgradeEncryption from the call even if presignTransaction drops it', async function () { + upgradeOnSignSetting.returns(false); + sinon + .stub(wallet.baseCoin, 'presignTransaction') + .callsFake(async (p) => _.omit(p, 'upgradeEncryption') as typeof p); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + sinon.stub(basecoin, 'signTransaction').resolves(signed); + + await wallet.signTransaction(signParams({ upgradeEncryption: true })); + + sinon.assert.calledOnce(upgrade); + }); + + it('does not send upgradeEncryption to BitGo from sendMany', async function () { + upgradeOnSignSetting.returns(false); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + const bodies: Record[] = []; + sinon.stub(Keychains.prototype, 'getKeysForSigning').resolves([userKeychain]); + sinon.stub(basecoin, 'getExtraPrebuildParams').resolves({}); + sinon.stub(basecoin, 'getLatestBlockHeight').resolves(undefined); + sinon.stub(basecoin, 'postProcessPrebuild').resolves({ txHex: 'unsignedTxHex' }); + sinon.stub(basecoin, 'verifyTransaction').resolves(true); + sinon.stub(basecoin, 'signTransaction').resolves(signed); + const buildNock = nock(bgUrl) + .post(`/api/v2/tbtc/wallet/${wallet.id()}/tx/build`) + .reply((uri, body) => { + bodies.push(body as Record); + return [200, { txHex: 'unsignedTxHex' }]; + }); + const sendNock = nock(bgUrl) + .post(`/api/v2/tbtc/wallet/${wallet.id()}/tx/send`) + .reply((uri, body) => { + bodies.push(body as Record); + return [200, { status: 'signed' }]; + }); + + await wallet.sendMany({ + recipients: [{ address: 'tb1qrecipient', amount: '1000' }], + walletPassphrase, + upgradeEncryption: true, + }); + + buildNock.isDone().should.be.true(); + sendNock.isDone().should.be.true(); + sinon.assert.calledOnce(upgrade); + bodies.should.have.length(2); + bodies.forEach((b) => b.should.not.have.property('upgradeEncryption')); + }); + }); + + describe('upgradeUserKeyEncryption', function () { + const walletPassphrase = 'upgradePassphrase'; + let userKeychain: Keychain; + + before(async function () { + userKeychain = { + id: wallet.keyIds()[0], + pub: 'xpubUser', + encryptedPrv: await bitgo.encrypt({ input: 'xprv1', password: walletPassphrase, encryptionVersion: 1 }), + type: 'independent', + }; + }); + + afterEach(function () { + sinon.restore(); + }); + + it('upgrades the user keychain and returns the result', async function () { + sinon.stub(Keychains.prototype, 'get').resolves(userKeychain); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + const result = await wallet.upgradeUserKeyEncryption({ walletPassphrase }); + + result.should.deepEqual({ upgraded: true }); + sinon.assert.calledOnce(upgrade); + sinon.assert.calledWithExactly(upgrade, { + keychain: userKeychain, + passphrase: walletPassphrase, + reqId: undefined, + }); + }); + + it('upgrades even when upgradeEncryptionOnSign is off', async function () { + const upgradeOnSignEnabled = sinon.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(false); + sinon.stub(Keychains.prototype, 'get').resolves(userKeychain); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await wallet.upgradeUserKeyEncryption({ walletPassphrase }); + + sinon.assert.calledOnce(upgrade); + sinon.assert.notCalled(upgradeOnSignEnabled); + }); + + it('reports an error and does not throw when the user keychain cannot be fetched', async function () { + sinon.stub(Keychains.prototype, 'get').rejects(new Error('network')); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption'); + + const result = await wallet.upgradeUserKeyEncryption({ walletPassphrase }); + + result.should.deepEqual({ upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }); + sinon.assert.notCalled(upgrade); + }); + }); + describe('TETH Wallet Transactions', function () { let ethWallet; @@ -4641,6 +4892,53 @@ describe('V2 Wallet:', function () { }) .should.be.rejectedWith('txRequestId required to sign transactions with TSS'); }); + + it('triggers the v1 to v2 user key upgrade when signing decrypts the user key with the passphrase', async function () { + sandbox.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(true); + const walletPassphrase = 'upgradePassphrase'; + const userKeychain: Keychain = { + id: tssSolWallet.keyIds()[0], + commonKeychain: 'test', + type: 'tss', + encryptedPrv: await bitgo.encrypt({ input: 'secretKey', password: walletPassphrase, encryptionVersion: 1 }), + }; + const signTxRequest = sandbox.stub(TssUtils.prototype, 'signTxRequest').resolves(txRequest); + const upgrade = sandbox.stub(Keychains.prototype, 'upgradeEncryption').returns(new Promise(() => undefined)); + + const signedTransaction = await tssSolWallet.signTransaction({ + reqId, + txPrebuild: { txRequestId: 'id', txHex: 'ababcdcd' }, + keychain: userKeychain, + walletPassphrase, + }); + + signedTransaction.should.deepEqual(txRequest); + signTxRequest.firstCall.args[0].prv.should.equal('secretKey'); + sinon.assert.calledOnce(upgrade); + sinon.assert.calledWithExactly(upgrade, { keychain: userKeychain, passphrase: walletPassphrase }); + }); + + it('does not upgrade the v1 user key when upgradeEncryptionOnSign is off', async function () { + sandbox.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(false); + const walletPassphrase = 'upgradePassphrase'; + const userKeychain: Keychain = { + id: tssSolWallet.keyIds()[0], + commonKeychain: 'test', + type: 'tss', + encryptedPrv: await bitgo.encrypt({ input: 'secretKey', password: walletPassphrase, encryptionVersion: 1 }), + }; + sandbox.stub(TssUtils.prototype, 'signTxRequest').resolves(txRequest); + const upgrade = sandbox.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await tssSolWallet.signTransaction({ + reqId, + txPrebuild: { txRequestId: 'id', txHex: 'ababcdcd' }, + keychain: userKeychain, + walletPassphrase, + }); + + sinon.assert.notCalled(upgrade); + }); }); describe('getUserKeyAndSignTssTransaction', function () { @@ -4956,6 +5254,138 @@ describe('V2 Wallet:', function () { }); }); + describe('v1 to v2 user key upgrade on sign', function () { + const walletPassphrase = 'upgradePassphrase'; + let userKeychain: Keychain; + let upgradeOnSignSetting: sinon.SinonStub; + + beforeEach(async function () { + upgradeOnSignSetting = sinon.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(true); + userKeychain = { + id: 'userKeyId', + commonKeychain: 'test', + pub: '', + type: 'tss', + encryptedPrv: await bitgo.encrypt({ input: 'secretKey', password: walletPassphrase, encryptionVersion: 1 }), + }; + (Keychains.prototype.getKeysForSigning as sinon.SinonStub).resolves([userKeychain]); + nock(bgUrl) + .get( + `/api/v2/wallet/${tssEthWallet.id()}/txrequests?txRequestIds=${ + txRequestForMessageSigning.txRequestId + }&latest=true` + ) + .reply(200, { txRequests: [txRequestForMessageSigning] }); + }); + + it('triggers the upgrade when signing decrypts the user key with the passphrase', async function () { + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').returns(new Promise(() => undefined)); + + const signed = await tssEthWallet.signMessage({ + reqId, + message: { + messageRaw, + txRequestId: txRequestForMessageSigning.txRequestId, + messageStandardType: MessageStandardType.EIP191, + }, + walletPassphrase, + }); + + assert.ok(signed.state === 'delivered'); + signed.signature.should.equal(txHash); + signTxRequestForMessage.firstCall.args[0].prv.should.equal('secretKey'); + sinon.assert.calledOnce(upgrade); + sinon.assert.calledWithExactly(upgrade, { keychain: userKeychain, passphrase: walletPassphrase }); + }); + + it('does not upgrade when the caller supplies prv', async function () { + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await tssEthWallet.signMessage({ + reqId, + message: { + messageRaw, + txRequestId: txRequestForMessageSigning.txRequestId, + messageStandardType: MessageStandardType.EIP191, + }, + prv: 'secretKey', + walletPassphrase, + }); + + sinon.assert.notCalled(upgrade); + }); + + it('does not upgrade with the default setting', async function () { + upgradeOnSignSetting.restore(); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await tssEthWallet.signMessage({ + reqId, + message: { + messageRaw, + txRequestId: txRequestForMessageSigning.txRequestId, + messageStandardType: MessageStandardType.EIP191, + }, + walletPassphrase, + }); + + sinon.assert.notCalled(upgrade); + }); + + it('does not upgrade when upgradeEncryptionOnSign is off', async function () { + upgradeOnSignSetting.returns(false); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await tssEthWallet.signMessage({ + reqId, + message: { + messageRaw, + txRequestId: txRequestForMessageSigning.txRequestId, + messageStandardType: MessageStandardType.EIP191, + }, + walletPassphrase, + }); + + sinon.assert.notCalled(upgrade); + }); + + it('upgrades when upgradeEncryptionOnSign is off and the call passes upgradeEncryption: true', async function () { + upgradeOnSignSetting.returns(false); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await tssEthWallet.signMessage({ + reqId, + message: { + messageRaw, + txRequestId: txRequestForMessageSigning.txRequestId, + messageStandardType: MessageStandardType.EIP191, + }, + walletPassphrase, + upgradeEncryption: true, + }); + + sinon.assert.calledOnce(upgrade); + sinon.assert.calledWithExactly(upgrade, { keychain: userKeychain, passphrase: walletPassphrase }); + }); + + it('does not upgrade when the call passes upgradeEncryption: false', async function () { + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await tssEthWallet.signMessage({ + reqId, + message: { + messageRaw, + txRequestId: txRequestForMessageSigning.txRequestId, + messageStandardType: MessageStandardType.EIP191, + }, + walletPassphrase, + upgradeEncryption: false, + }); + + sinon.assert.notCalled(upgrade); + }); + }); + describe('SOL message signing (UTF-8 string path)', function () { const solSignerAddress = 'J8TEPFqNJjFTp2NGHuWgijyuF45Lrzo2feZ61kS7yY2q'; // the message class the signing stack (BGM_SOL_MESSAGE) accepts: well-formed SIWS @@ -5501,6 +5931,114 @@ describe('V2 Wallet:', function () { }); }); }); + + describe('v1 to v2 user key upgrade on sign', function () { + const walletPassphrase = 'upgradePassphrase'; + let userKeychain: Keychain; + let upgradeOnSignSetting: sinon.SinonStub; + + beforeEach(async function () { + upgradeOnSignSetting = sinon.stub(bitgo, 'isUpgradeEncryptionOnSignEnabled').returns(true); + userKeychain = { + id: 'userKeyId', + commonKeychain: 'test', + pub: '', + type: 'tss', + encryptedPrv: await bitgo.encrypt({ input: 'secretKey', password: walletPassphrase, encryptionVersion: 1 }), + }; + (Keychains.prototype.getKeysForSigning as sinon.SinonStub).resolves([userKeychain]); + nock(bgUrl) + .get( + `/api/v2/wallet/${tssEthWallet.id()}/txrequests?txRequestIds=${ + txRequestForTypedDataSigning.txRequestId + }&latest=true` + ) + .reply(200, { txRequests: [txRequestForTypedDataSigning] }); + }); + + it('triggers the upgrade when signing decrypts the user key with the passphrase', async function () { + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').returns(new Promise(() => undefined)); + + const signed = await tssEthWallet.signTypedData({ + reqId, + typedData: { ...typedDataBase, txRequestId: txRequestForTypedDataSigning.txRequestId }, + walletPassphrase, + }); + + assert.ok(signed.state === 'delivered'); + signed.signature.should.equal(txHash); + signTxRequestForMessage.firstCall.args[0].prv.should.equal('secretKey'); + sinon.assert.calledOnce(upgrade); + sinon.assert.calledWithExactly(upgrade, { keychain: userKeychain, passphrase: walletPassphrase }); + }); + + it('does not upgrade when the caller supplies prv', async function () { + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await tssEthWallet.signTypedData({ + reqId, + typedData: { ...typedDataBase, txRequestId: txRequestForTypedDataSigning.txRequestId }, + prv: 'secretKey', + walletPassphrase, + }); + + sinon.assert.notCalled(upgrade); + }); + + it('does not upgrade with the default setting', async function () { + upgradeOnSignSetting.restore(); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await tssEthWallet.signTypedData({ + reqId, + typedData: { ...typedDataBase, txRequestId: txRequestForTypedDataSigning.txRequestId }, + walletPassphrase, + }); + + sinon.assert.notCalled(upgrade); + }); + + it('does not upgrade when upgradeEncryptionOnSign is off', async function () { + upgradeOnSignSetting.returns(false); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await tssEthWallet.signTypedData({ + reqId, + typedData: { ...typedDataBase, txRequestId: txRequestForTypedDataSigning.txRequestId }, + walletPassphrase, + }); + + sinon.assert.notCalled(upgrade); + }); + + it('upgrades when upgradeEncryptionOnSign is off and the call passes upgradeEncryption: true', async function () { + upgradeOnSignSetting.returns(false); + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await tssEthWallet.signTypedData({ + reqId, + typedData: { ...typedDataBase, txRequestId: txRequestForTypedDataSigning.txRequestId }, + walletPassphrase, + upgradeEncryption: true, + }); + + sinon.assert.calledOnce(upgrade); + sinon.assert.calledWithExactly(upgrade, { keychain: userKeychain, passphrase: walletPassphrase }); + }); + + it('does not upgrade when the call passes upgradeEncryption: false', async function () { + const upgrade = sinon.stub(Keychains.prototype, 'upgradeEncryption').resolves({ upgraded: true }); + + await tssEthWallet.signTypedData({ + reqId, + typedData: { ...typedDataBase, txRequestId: txRequestForTypedDataSigning.txRequestId }, + walletPassphrase, + upgradeEncryption: false, + }); + + sinon.assert.notCalled(upgrade); + }); + }); }); describe('Message Signing with pending approval (policy flow)', function () { diff --git a/modules/express/test/unit/typedRoutes/lightningPayment.ts b/modules/express/test/unit/typedRoutes/lightningPayment.ts index 9582e1733a7..2039387c1ca 100644 --- a/modules/express/test/unit/typedRoutes/lightningPayment.ts +++ b/modules/express/test/unit/typedRoutes/lightningPayment.ts @@ -333,6 +333,7 @@ describe('Lightning Payment API Tests', function () { const mockBitgo: any = { setRequestTracer: sinon.stub(), + isUpgradeEncryptionOnSignEnabled: () => false, decrypt: sinon .stub() .resolves( @@ -504,6 +505,7 @@ describe('Lightning Payment API Tests', function () { // Mock bitgo instance with required methods const mockBitgo: any = { setRequestTracer: sinon.stub(), + isUpgradeEncryptionOnSignEnabled: () => false, decrypt: sinon .stub() .resolves( @@ -635,6 +637,7 @@ describe('Lightning Payment API Tests', function () { // Mock bitgo instance with required methods const mockBitgo: any = { setRequestTracer: sinon.stub(), + isUpgradeEncryptionOnSignEnabled: () => false, decrypt: sinon .stub() .resolves( @@ -792,6 +795,7 @@ describe('Lightning Payment API Tests', function () { // Mock bitgo instance with required methods const mockBitgo: any = { setRequestTracer: sinon.stub(), + isUpgradeEncryptionOnSignEnabled: () => false, decrypt: sinon .stub() .resolves( @@ -871,6 +875,7 @@ describe('Lightning Payment API Tests', function () { // Mock bitgo instance with required methods const mockBitgo: any = { setRequestTracer: sinon.stub(), + isUpgradeEncryptionOnSignEnabled: () => false, decrypt: sinon .stub() .resolves( @@ -951,6 +956,7 @@ describe('Lightning Payment API Tests', function () { // Mock bitgo instance with required methods const mockBitgo: any = { setRequestTracer: sinon.stub(), + isUpgradeEncryptionOnSignEnabled: () => false, decrypt: sinon .stub() .resolves( diff --git a/modules/sdk-api/src/bitgoAPI.ts b/modules/sdk-api/src/bitgoAPI.ts index 16bbb40b7d8..4959ff1abc5 100644 --- a/modules/sdk-api/src/bitgoAPI.ts +++ b/modules/sdk-api/src/bitgoAPI.ts @@ -265,6 +265,7 @@ export class BitGoAPI implements BitGoBase { protected readonly _env: EnvironmentName; protected readonly _authVersion: Exclude = 2; protected _hmacVerification = true; + protected readonly _upgradeEncryptionOnSign: boolean; protected readonly _proxy?: string; protected _user?: User; protected _extensionKey?: ECPairInterface; @@ -444,6 +445,7 @@ export class BitGoAPI implements BitGoBase { // turned off by setting to false. can also be overridden individually in the // functions that use it. this._validate = params.validate === undefined ? true : params.validate; + this._upgradeEncryptionOnSign = params.upgradeEncryptionOnSign ?? false; if (!params.hmacVerification && params.hmacVerification !== undefined) { if ((env == 'prod' || env == 'adminProd') && common.Environments[env].hmacVerificationEnforced) { @@ -557,6 +559,14 @@ export class BitGoAPI implements BitGoBase { return this._env; } + /** + * Whether signing with your wallet passphrase upgrades the stored encryption of your user key + * to the latest format when the call does not set `upgradeEncryption`. + */ + isUpgradeEncryptionOnSignEnabled(): boolean { + return this._upgradeEncryptionOnSign; + } + /** * Return the current auth version used for requests to the BitGo server */ diff --git a/modules/sdk-api/src/types.ts b/modules/sdk-api/src/types.ts index 8e51d6d8d1f..6d290799180 100644 --- a/modules/sdk-api/src/types.ts +++ b/modules/sdk-api/src/types.ts @@ -74,6 +74,12 @@ export interface BitGoAPIOptions { userAgent?: string; validate?: boolean; cookiesPropagationEnabled?: boolean; + /** + * Upgrade the stored encryption of your user key to the latest format when you sign with your + * wallet passphrase. Defaults to false. Individual signing calls can override it with + * `upgradeEncryption`. + */ + upgradeEncryptionOnSign?: boolean; getAdditionalHeadersCb?: AdditionalHeadersCallback; requestIdPrefix?: string; evm?: { diff --git a/modules/sdk-api/test/unit/bitgoAPI.ts b/modules/sdk-api/test/unit/bitgoAPI.ts index b6da41d55ba..a962f21789d 100644 --- a/modules/sdk-api/test/unit/bitgoAPI.ts +++ b/modules/sdk-api/test/unit/bitgoAPI.ts @@ -43,6 +43,31 @@ describe('Constructor', function () { }); }); + describe('upgradeEncryptionOnSign argument', function () { + it('is off by default', function () { + const bitgo = new BitGoAPI({ env: 'custom', customRootURI: 'https://app.example.local' }); + bitgo.isUpgradeEncryptionOnSignEnabled().should.equal(false); + }); + + it('is on when set to true', function () { + const bitgo = new BitGoAPI({ + env: 'custom', + customRootURI: 'https://app.example.local', + upgradeEncryptionOnSign: true, + }); + bitgo.isUpgradeEncryptionOnSignEnabled().should.equal(true); + }); + + it('is off when set to false', function () { + const bitgo = new BitGoAPI({ + env: 'custom', + customRootURI: 'https://app.example.local', + upgradeEncryptionOnSign: false, + }); + bitgo.isUpgradeEncryptionOnSignEnabled().should.equal(false); + }); + }); + describe('requestIdPrefix argument', function () { afterEach(function () { nock.cleanAll(); diff --git a/modules/sdk-core/src/bitgo/bitgoBase.ts b/modules/sdk-core/src/bitgo/bitgoBase.ts index 698e4846fd4..4c2543d7280 100644 --- a/modules/sdk-core/src/bitgo/bitgoBase.ts +++ b/modules/sdk-core/src/bitgo/bitgoBase.ts @@ -51,6 +51,7 @@ export interface BitGoBase { getECDHKeychain(ecdhKeychainPub?: string, options?: { allowMissing?: boolean }): Promise; getEcdhKeypairPrivate(password: string, entId?: string): Promise; getEnv(): EnvironmentName; + isUpgradeEncryptionOnSignEnabled(): boolean; getSharingKey({ email }: GetSharingKeyOptions): Promise; getSigningKeyForUser(enterpriseId: string, userId?: string): Promise; microservicesUrl(path: string): string; diff --git a/modules/sdk-core/src/bitgo/keychain/iKeychains.ts b/modules/sdk-core/src/bitgo/keychain/iKeychains.ts index 6b022c54a92..5c8f26491a9 100644 --- a/modules/sdk-core/src/bitgo/keychain/iKeychains.ts +++ b/modules/sdk-core/src/bitgo/keychain/iKeychains.ts @@ -169,6 +169,51 @@ export interface UpdateSingleKeychainPasswordOptions { encryptionSession?: IEncryptionSession; } +/** + * Parameters for Keychains.upgradeEncryption + * @property {Keychain} keychain - the keychain as read from BitGo; its `encryptedPrv` is the envelope to upgrade. + * Only `id`, `encryptedPrv`, `pub` and `coinSpecific` are read. + * @property {string} passphrase - the passphrase that decrypts `keychain.encryptedPrv` + * @property {IRequestTracer} reqId - optional reqId + */ +export interface UpgradeKeychainEncryptionOptions { + keychain: Pick; + passphrase: string; + reqId?: IRequestTracer; +} + +/** Why an upgrade of a key's stored encryption was not attempted or not needed. */ +export enum KeyEncryptionUpgradeSkipReason { + /** The key already uses the latest format. */ + AlreadyUpgraded = 'alreadyUpgraded', + /** The key was changed elsewhere since it was read, so it was left as is. */ + KeyChanged = 'keyChanged', + /** There is no stored private key to upgrade. */ + NoPrivateKey = 'noPrivateKey', +} + +/** Why an upgrade of a key's stored encryption failed. The key is left unchanged and can be upgraded later. */ +export enum KeyEncryptionUpgradeFailureReason { + /** The re-encrypted key did not check out, so it was not saved. */ + VerificationFailed = 'verificationFailed', + /** BitGo is not accepting upgrades right now. Try again later. */ + Unavailable = 'unavailable', + /** Any other failure, such as a wrong passphrase or a network error. */ + Error = 'error', +} + +/** + * Outcome of upgrading the stored encryption of a key. + * - `{ upgraded: true }`: the key now uses the latest format. + * - `{ upgraded: false, skipped }`: no upgrade was attempted or needed; `skipped` says why. + * - `{ upgraded: false, failed }`: the key was left unchanged and can be upgraded on a later attempt; + * `failed` says why. + */ +export type KeyEncryptionUpgradeResult = + | { upgraded: true } + | { upgraded: false; skipped: KeyEncryptionUpgradeSkipReason } + | { upgraded: false; failed: KeyEncryptionUpgradeFailureReason }; + /** * Parameters for the rotateKeychain method for ofc multi-user-key * @property {string} id - the public id of the keychain @@ -333,6 +378,11 @@ export interface IKeychains { updateSingleKeychainPassword(params?: UpdateSingleKeychainPasswordOptions): Promise; getEncryptionVersion(ciphertext: string): EncryptionVersion; reencryptAsV2(encryptedPrv: string, passphrase: string): Promise; + /** + * Upgrades the stored encryption of this key to the latest format, using the same passphrase. + * Never throws; returns what happened. + */ + upgradeEncryption(params: UpgradeKeychainEncryptionOptions): Promise; create(params?: { seed?: Buffer; isRootKey?: boolean }): KeyPair; add(params?: AddKeychainOptions): Promise; createBitGo(params?: CreateBitGoOptions): Promise; diff --git a/modules/sdk-core/src/bitgo/keychain/index.ts b/modules/sdk-core/src/bitgo/keychain/index.ts index 360a4d52b45..36bc4b44b01 100644 --- a/modules/sdk-core/src/bitgo/keychain/index.ts +++ b/modules/sdk-core/src/bitgo/keychain/index.ts @@ -2,3 +2,4 @@ export * from './iKeychains'; export * from './keychains'; export * from './ovcJsonCodec'; export * from './decryptKeychain'; +export * from './upgradeEncryption'; diff --git a/modules/sdk-core/src/bitgo/keychain/keychains.ts b/modules/sdk-core/src/bitgo/keychain/keychains.ts index ec226f0a059..51aa1818734 100644 --- a/modules/sdk-core/src/bitgo/keychain/keychains.ts +++ b/modules/sdk-core/src/bitgo/keychain/keychains.ts @@ -4,7 +4,7 @@ import * as _ from 'lodash'; import * as common from '../../common'; import { IBaseCoin, KeychainsTriplet, KeyPair } from '../baseCoin'; import { BitGoBase } from '../bitgoBase'; -import { IncorrectPasswordError, SafeMpcCeremonyUnsupportedError } from '../errors'; +import { ApiResponseError, IncorrectPasswordError, SafeMpcCeremonyUnsupportedError } from '../errors'; import { decodeEd25519StrKeyPublicKey, encodeDerivableEd25519Pub, @@ -29,18 +29,32 @@ import { RotateKeychainOptions, UpdatePasswordOptions, UpdateSingleKeychainPasswordOptions, + KeyEncryptionUpgradeFailureReason, + KeyEncryptionUpgradeResult, + KeyEncryptionUpgradeSkipReason, + UpgradeKeychainEncryptionOptions, } from './iKeychains'; import { BitGoKeyFromOvcShares, BitGoToOvcJSON, OvcToBitGoJSON } from './ovcJsonCodec'; import { EncryptionVersion, IEncryptionSession } from '../../api'; +const debug = require('debug')('bitgo:v2:keychains'); + /** Server-side transaction budget for one bulk key update: each write is sub-KB, so hundreds of * envelopes fit in one Mongo transaction. Only batches past this size are chunked, at the cost * of all-or-nothing across chunks. */ const BULK_KEY_UPDATE_BATCH_SIZE = 500; +/** An upgradeEncryption request whose keychain id and v1 envelope have already been validated. */ +type ValidatedKeyEncryptionUpgrade = UpgradeKeychainEncryptionOptions & { keyId: string; oldEncryptedPrv: string }; + export class Keychains implements IKeychains { private readonly bitgo: BitGoBase; private readonly baseCoin: IBaseCoin; + /** + * Pending upgradeEncryption outcomes on this instance, by keychain id. A call for a key that is + * already being upgraded shares the pending outcome instead of starting a second write. + */ + private readonly upgradesInFlight = new Map>(); constructor(bitgo: BitGoBase, baseCoin: IBaseCoin) { this.bitgo = bitgo; @@ -396,6 +410,95 @@ export class Keychains implements IKeychains { return this.bitgo.encrypt({ input: prv, password: passphrase, encryptionVersion: 2 }); } + /** + * Upgrades the stored encryption of this key to the latest format, using the same passphrase. + * Never throws; returns what happened, so it can be shown to the user or ignored. + * Concurrent calls for the same key share one upgrade and all receive its outcome. + * + * @param params.keychain - the keychain as read from BitGo + * @param params.passphrase - the passphrase that decrypts the key + * @param params.reqId - optional request tracer + * @returns the outcome of the upgrade + */ + async upgradeEncryption(params: UpgradeKeychainEncryptionOptions): Promise { + const { keychain, passphrase } = params; + const keyId = keychain?.id; + const oldEncryptedPrv = keychain?.encryptedPrv; + if (!_.isString(keyId) || !_.isString(passphrase)) { + return { upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }; + } + if (!_.isString(oldEncryptedPrv)) { + return { upgraded: false, skipped: KeyEncryptionUpgradeSkipReason.NoPrivateKey }; + } + let encryptionVersion: EncryptionVersion; + try { + encryptionVersion = this.getEncryptionVersion(oldEncryptedPrv); + } catch (e) { + return { upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }; + } + if (encryptionVersion !== 1) { + return { upgraded: false, skipped: KeyEncryptionUpgradeSkipReason.AlreadyUpgraded }; + } + const inFlight = this.upgradesInFlight.get(keyId); + if (inFlight) { + return inFlight; + } + // Registered before any await so concurrent calls for the same key share this outcome. + const upgrade = this.runEncryptionUpgrade({ ...params, keyId, oldEncryptedPrv }); + this.upgradesInFlight.set(keyId, upgrade); + const release = () => { + if (this.upgradesInFlight.get(keyId) === upgrade) { + this.upgradesInFlight.delete(keyId); + } + }; + upgrade.then(release, release); + return upgrade; + } + + /** + * Re-encrypts a validated v1 envelope as v2, checks it decrypts back to the same key, and writes it + * only if the stored envelope is still the one that was read. Never rejects. + */ + private async runEncryptionUpgrade(params: ValidatedKeyEncryptionUpgrade): Promise { + const { keychain, passphrase, reqId, keyId, oldEncryptedPrv } = params; + try { + const prv = await this.bitgo.decrypt({ input: oldEncryptedPrv, password: passphrase }); + const newEncryptedPrv = await this.bitgo.encrypt({ input: prv, password: passphrase, encryptionVersion: 2 }); + // Decrypt the new envelope again and compare before anything is written. + const roundTrip = await this.bitgo.decrypt({ input: newEncryptedPrv, password: passphrase }); + if (roundTrip !== prv) { + debug('upgradeEncryption: v2 round-trip mismatch for key %s, not writing', keyId); + return { upgraded: false, failed: KeyEncryptionUpgradeFailureReason.VerificationFailed }; + } + if (reqId) { + this.bitgo.setRequestTracer(reqId); + } + // The server replaces the stored envelope only if it still equals the one read here (409 otherwise), + // so a concurrent password change is never rolled back. + await this.bitgo + .put(this.baseCoin.url('/key/' + encodeURIComponent(keyId))) + .send({ + encryptedPrv: newEncryptedPrv, + existingEncryptedPrv: oldEncryptedPrv, + encryptionUpgrade: true, + ...(Keychains.isMultiUserKey(keychain) && keychain.pub ? { pub: keychain.pub } : {}), + }) + .result(); + debug('upgradeEncryption: upgraded key %s to v2', keyId); + return { upgraded: true }; + } catch (e) { + debug('upgradeEncryption: failed for key %s: %s', keyId, e instanceof Error ? e.message : String(e)); + const status = (e as Partial>)?.status; + if (status === 409) { + return { upgraded: false, skipped: KeyEncryptionUpgradeSkipReason.KeyChanged }; + } + if (status === 503) { + return { upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Unavailable }; + } + return { upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }; + } + } + /** * Create a public/private key pair * @param params - optional params @@ -842,7 +945,7 @@ export class Keychains implements IKeychains { * Static helper method to determine if a keychain is a ofc multi-user-key * @param keychain */ - static isMultiUserKey(keychain: Keychain): boolean { + static isMultiUserKey(keychain: Pick): boolean { return (keychain.coinSpecific?.ofc?.['features'] ?? []).includes('multi-user-key'); } } diff --git a/modules/sdk-core/src/bitgo/keychain/upgradeEncryption.ts b/modules/sdk-core/src/bitgo/keychain/upgradeEncryption.ts new file mode 100644 index 00000000000..2e325c3eb82 --- /dev/null +++ b/modules/sdk-core/src/bitgo/keychain/upgradeEncryption.ts @@ -0,0 +1,15 @@ +import { BitGoBase } from '../bitgoBase'; + +/** + * Whether a signing call should upgrade the stored encryption of your user key to the latest format. + * A per-call `upgradeEncryption` wins; otherwise the `upgradeEncryptionOnSign` constructor setting applies. + * + * @param bitgo - the BitGo instance the signing call runs on + * @param upgradeEncryption - the signing call's `upgradeEncryption` option, if any + */ +export function shouldUpgradeEncryption( + bitgo: Pick, + upgradeEncryption?: boolean +): boolean { + return upgradeEncryption ?? bitgo.isUpgradeEncryptionOnSignEnabled(); +} diff --git a/modules/sdk-core/src/bitgo/pendingApproval/iPendingApproval.ts b/modules/sdk-core/src/bitgo/pendingApproval/iPendingApproval.ts index 6376164a4e1..61fda7fdc2e 100644 --- a/modules/sdk-core/src/bitgo/pendingApproval/iPendingApproval.ts +++ b/modules/sdk-core/src/bitgo/pendingApproval/iPendingApproval.ts @@ -35,6 +35,11 @@ export interface ApproveOptions { xprv?: string; previewPendingTxs?: boolean; pendingApprovalId?: string; + /** + * Upgrade the stored encryption of your user key to the latest format after signing with your + * wallet passphrase. Defaults to the `upgradeEncryptionOnSign` setting. + */ + upgradeEncryption?: boolean; } export interface PendingApprovalInfo { diff --git a/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts b/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts index 6eb61f3eb3d..bb4db3daf7b 100644 --- a/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts +++ b/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts @@ -18,6 +18,7 @@ import { } from '../pendingApproval'; import { RequestTracer, RequestType } from '../utils'; import { IWallet } from '../wallet'; +import { shouldUpgradeEncryption } from '../keychain'; import { isSafeChildPublicOnlyKeychain } from '../wallet/safeKeychain'; import { BuildParams, ERC7984_INTENT_TX_TYPES } from '../wallet/BuildParams'; import { IRequestTracer } from '../../api'; @@ -244,7 +245,7 @@ export class PendingApproval implements IPendingApproval { * @param {RequestTracer} reqId id tracer. */ async recreateAndSignTSSTransaction(params: ApproveOptions, reqId: IRequestTracer): Promise<{ txHex: string }> { - const { walletPassphrase } = params; + const { walletPassphrase, upgradeEncryption } = params; const txRequestId = this._pendingApproval.txRequestId; if (!this.wallet) { @@ -263,12 +264,20 @@ export class PendingApproval implements IPendingApproval { await this.wallet.baseCoin.keychains().getKeysForSigning({ wallet: this.wallet, reqId }) )[0]; - const decryptedPrv = isSafeChildPublicOnlyKeychain(this.wallet.safeId(), childUserKeychain) + const isSafeChild = isSafeChildPublicOnlyKeychain(this.wallet.safeId(), childUserKeychain); + const decryptedPrv = isSafeChild ? await this.wallet.getUserPrv({ keychain: childUserKeychain, walletPassphrase, }) : await this.wallet.getPrv({ walletPassphrase }); + if (!isSafeChild && childUserKeychain?.encryptedPrv && shouldUpgradeEncryption(this.bitgo, upgradeEncryption)) { + // getPrv decrypted the first keychain with an encryptedPrv, which is this same user keychain. + // Fire-and-forget; never awaited and never affects signing. + void this.wallet.baseCoin + .keychains() + .upgradeEncryption({ keychain: childUserKeychain, passphrase: walletPassphrase }); + } const txRequest = await this.tssUtils!.recreateTxRequest(txRequestId, decryptedPrv, reqId); if (txRequest.apiVersion === 'lite') { if (!txRequest.unsignedTxs || txRequest.unsignedTxs.length === 0) { diff --git a/modules/sdk-core/src/bitgo/wallet/iWallet.ts b/modules/sdk-core/src/bitgo/wallet/iWallet.ts index 107962047e9..ae32b28eb94 100644 --- a/modules/sdk-core/src/bitgo/wallet/iWallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/iWallet.ts @@ -15,7 +15,7 @@ import { UnifiedRecipientPreference, } from '../baseCoin'; import { BitGoBase } from '../bitgoBase'; -import { Keychain, KeychainWithEncryptedPrv } from '../keychain'; +import { KeyEncryptionUpgradeResult, Keychain, KeychainWithEncryptedPrv } from '../keychain'; import { IPendingApproval, PendingApprovalData } from '../pendingApproval'; import { IDefiVault } from '../defi'; import { IGoStakingWallet, IStakingWallet } from '../staking'; @@ -407,6 +407,12 @@ export interface CustomSigningFunction { }): Promise; } +export interface UpgradeUserKeyEncryptionOptions { + /** The wallet passphrase that decrypts the user key. */ + walletPassphrase: string; + reqId?: IRequestTracer; +} + export interface WalletSignBaseOptions { reqId?: IRequestTracer; prv?: string; @@ -416,6 +422,11 @@ export interface WalletSignBaseOptions { isLastSignature?: boolean; customSigningFunction?: CustomSigningFunction; bulk?: boolean; + /** + * Upgrade the stored encryption of your user key to the latest format after signing with your + * wallet passphrase. Defaults to the `upgradeEncryptionOnSign` setting. + */ + upgradeEncryption?: boolean; } export interface WalletSignTransactionOptions extends WalletSignBaseOptions { @@ -1391,6 +1402,7 @@ export interface IWallet { toAddressBook(): IAddressBook; downloadKeycard(params?: DownloadKeycardOptions): Promise; upgradeEncryption(params: UpgradeEncryptionOptions): Promise; + upgradeUserKeyEncryption(params: UpgradeUserKeyEncryptionOptions): Promise; buildAccountConsolidations(params?: BuildConsolidationTransactionOptions): Promise; sendAccountConsolidation(params?: PrebuildAndSignTransactionOptions): Promise; sendAccountConsolidations(params?: BuildConsolidationTransactionOptions): Promise; diff --git a/modules/sdk-core/src/bitgo/wallet/wallet.ts b/modules/sdk-core/src/bitgo/wallet/wallet.ts index 3a371108016..fab56d16ca8 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallet.ts @@ -34,7 +34,15 @@ import { import { SubmitTransactionResponse } from '../inscriptionBuilder'; import { drawKeycard } from '../internal'; import * as internal from '../internal/internal'; -import { decryptKeychainPrivateKey, Keychain, KeychainWithEncryptedPrv, KeyIndices } from '../keychain'; +import { + decryptKeychainPrivateKey, + Keychain, + KeychainWithEncryptedPrv, + KeyEncryptionUpgradeFailureReason, + KeyEncryptionUpgradeResult, + KeyIndices, + shouldUpgradeEncryption, +} from '../keychain'; import { CreateLightningInvoiceParams, LightningInvoiceResponse } from '../../lightning'; import { getLightningAuthKey } from '../lightning/lightningWalletUtil'; import { IPendingApproval, PendingApproval, PendingApprovals } from '../pendingApproval'; @@ -137,6 +145,7 @@ import { UpdateWalletOptions, UpgradeEncryptionOptions, UpgradeEncryptionResult, + UpgradeUserKeyEncryptionOptions, VerifyKeyOptions, VerifyKeyResult, WalletCoinSpecific, @@ -2460,9 +2469,11 @@ export class Wallet implements IWallet { } if (this.multisigType() === 'tss') { + const prv = await this.getUserPrv(presign as GetUserPrvOptions); + this.startUserKeyEncryptionUpgrade(presign as GetUserPrvOptions, params.upgradeEncryption); return this.signTransactionTss({ ...presign, - prv: await this.getUserPrv(presign as GetUserPrvOptions), + prv, apiVersion, }); } @@ -2502,6 +2513,9 @@ export class Wallet implements IWallet { if (this.baseCoin.getFamily() === 'ofc') { const userKeySigningRequired = this.toTradingAccount().userKeySigningRequired; const prv = userKeySigningRequired ? await this.getUserPrv(presign as GetUserPrvOptions) : undefined; + if (userKeySigningRequired) { + this.startUserKeyEncryptionUpgrade(presign as GetUserPrvOptions, params.upgradeEncryption); + } return this.baseCoin.signTransaction({ ...signTransactionParams, prv, @@ -2509,13 +2523,62 @@ export class Wallet implements IWallet { }); } + const prv = await this.getUserPrv(presign as GetUserPrvOptions); + this.startUserKeyEncryptionUpgrade(presign as GetUserPrvOptions, params.upgradeEncryption); return this.baseCoin.signTransaction({ ...signTransactionParams, - prv: await this.getUserPrv(presign as GetUserPrvOptions), + prv, wallet: this, }); } + /** + * Upgrade the stored encryption of this wallet's user key to the latest format, using the wallet + * passphrase. Use it after a signing call to run the upgrade and show the outcome, for example in + * a UI. It applies regardless of the `upgradeEncryptionOnSign` setting, never throws, and never + * affects signing. + * + * @param params.walletPassphrase - the wallet passphrase that decrypts the user key + * @param params.reqId - optional request tracer + */ + async upgradeUserKeyEncryption(params: UpgradeUserKeyEncryptionOptions): Promise { + const { walletPassphrase, reqId } = params; + let keychain: Keychain; + try { + keychain = await this.baseCoin.keychains().get({ id: this.keyIds()[KeyIndices.USER], reqId }); + } catch (e) { + return { upgraded: false, failed: KeyEncryptionUpgradeFailureReason.Error }; + } + return this.baseCoin.keychains().upgradeEncryption({ keychain, passphrase: walletPassphrase, reqId }); + } + + /** + * Fire-and-forget upgrade of the user keychain's v1 `encryptedPrv` to v2, for signing paths + * that just resolved the prv from `walletPassphrase` via `getUserPrv(params)`. + * + * Does nothing when the upgrade is not enabled for this call (per-call `upgradeEncryption`, else + * the `upgradeEncryptionOnSign` setting), when the caller supplied `prv`, when there is no + * passphrase, when the keychain has no `encryptedPrv`, or for safe child keychains (their prv + * comes from the root key). The upgrade is never awaited and can never affect the signing result. + * + * @param params - the same options that were passed to `getUserPrv` + * @param upgradeEncryption - the signing call's `upgradeEncryption` option + */ + private startUserKeyEncryptionUpgrade(params: GetUserPrvOptions, upgradeEncryption?: boolean): void { + const keychain = params.keychain || params.key; + const walletPassphrase = params.walletPassphrase; + if ( + params.prv || + !keychain?.encryptedPrv || + !_.isString(walletPassphrase) || + isSafeChildPublicOnlyKeychain(this.safeId(), keychain) || + !shouldUpgradeEncryption(this.bitgo, upgradeEncryption) + ) { + return; + } + void this.baseCoin.keychains().upgradeEncryption({ keychain, passphrase: walletPassphrase }); + } + /** * Sign a typed structured data using TSS * @param params @@ -2542,11 +2605,13 @@ export class Wallet implements IWallet { keychain: keychains[0], }; assert(keychains[0].commonKeychain, 'Unable to find commonKeychain in keychains'); + const prv = await this.getUserPrv(userPrvOptions); + this.startUserKeyEncryptionUpgrade(userPrvOptions, params.upgradeEncryption); const presign = { ...params, walletData: this._wallet, tssUtils: this.tssUtils, - prv: await this.getUserPrv(userPrvOptions), + prv, keychain: keychains[0], backupKeychain: keychains.length > 1 ? keychains[1] : null, bitgoKeychain: keychains.length > 2 ? keychains[2] : null, @@ -2586,11 +2651,13 @@ export class Wallet implements IWallet { keychain: keychains[0], }; assert(keychains[0].commonKeychain, 'Unable to find commonKeychain in keychains'); + const prv = await this.getUserPrv(userPrvOptions); + this.startUserKeyEncryptionUpgrade(userPrvOptions, params.upgradeEncryption); const presign = { ...params, walletData: this._wallet, tssUtils: this.tssUtils, - prv: await this.getUserPrv(userPrvOptions), + prv, keychain: keychains[0], backupKeychain: keychains.length > 1 ? keychains[1] : null, bitgoKeychain: keychains.length > 2 ? keychains[2] : null, diff --git a/modules/sdk-core/test/unit/bitgo/keychain/upgradeEncryption.ts b/modules/sdk-core/test/unit/bitgo/keychain/upgradeEncryption.ts new file mode 100644 index 00000000000..637b62322cb --- /dev/null +++ b/modules/sdk-core/test/unit/bitgo/keychain/upgradeEncryption.ts @@ -0,0 +1,21 @@ +import 'should'; +import { shouldUpgradeEncryption } from '../../../../src'; + +describe('shouldUpgradeEncryption', function () { + const enabled = { isUpgradeEncryptionOnSignEnabled: () => true }; + const disabled = { isUpgradeEncryptionOnSignEnabled: () => false }; + + it('follows the constructor setting when no per-call value is given', function () { + shouldUpgradeEncryption(enabled).should.equal(true); + shouldUpgradeEncryption(disabled).should.equal(false); + shouldUpgradeEncryption(enabled, undefined).should.equal(true); + shouldUpgradeEncryption(disabled, undefined).should.equal(false); + }); + + it('lets a per-call value override the constructor setting', function () { + shouldUpgradeEncryption(enabled, false).should.equal(false); + shouldUpgradeEncryption(disabled, true).should.equal(true); + shouldUpgradeEncryption(enabled, true).should.equal(true); + shouldUpgradeEncryption(disabled, false).should.equal(false); + }); +}); diff --git a/modules/sdk-core/test/unit/bitgo/wallet/ofcWalletSignTransaction.ts b/modules/sdk-core/test/unit/bitgo/wallet/ofcWalletSignTransaction.ts index 6ba3a9b3b7a..3a377f7f812 100644 --- a/modules/sdk-core/test/unit/bitgo/wallet/ofcWalletSignTransaction.ts +++ b/modules/sdk-core/test/unit/bitgo/wallet/ofcWalletSignTransaction.ts @@ -59,6 +59,7 @@ describe('Wallet - OFC', function () { url: sinon.stub().returns('https://test.bitgo.com/'), post: sinon.stub(), setRequestTracer: sinon.stub(), + isUpgradeEncryptionOnSignEnabled: () => false, }; ofcToken = new OfcToken(mockBitGo, TEST_TOKEN_CONFIG); mockBitGo.coin = sinon.stub().returns(ofcToken); diff --git a/modules/sdk-core/test/unit/bitgo/wallet/safeGetUserPrv.ts b/modules/sdk-core/test/unit/bitgo/wallet/safeGetUserPrv.ts index 9ec0e1ae1a0..edda4854b6a 100644 --- a/modules/sdk-core/test/unit/bitgo/wallet/safeGetUserPrv.ts +++ b/modules/sdk-core/test/unit/bitgo/wallet/safeGetUserPrv.ts @@ -611,7 +611,7 @@ describe('WCN-1200 recreateAndSignTSSTransaction safe path', function () { transactions: [], }); const pendingApproval = new PendingApproval( - {} as any, + { isUpgradeEncryptionOnSignEnabled: () => false } as any, wallet.baseCoin, { id: 'pa0',