Skip to content

Commit b2239f5

Browse files
authored
Merge pull request #8804 from BitGo/WCN-281
feat(sdk-core): add v2 decryption support to password change
2 parents 09d8686 + 792216e commit b2239f5

5 files changed

Lines changed: 193 additions & 16 deletions

File tree

modules/bitgo/test/v2/unit/keychains.ts

Lines changed: 118 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,61 @@ describe('V2 Keychains', function () {
212212
);
213213
});
214214

215+
it('to update the password for a single keychain (async)', async function () {
216+
await keychains
217+
.updateSingleKeychainPasswordAsync({ newPassword: '5678' })
218+
.should.be.rejectedWith('expected old password to be a string');
219+
220+
await keychains
221+
.updateSingleKeychainPasswordAsync({ oldPassword: 1234, newPassword: '5678' })
222+
.should.be.rejectedWith('expected old password to be a string');
223+
224+
await keychains
225+
.updateSingleKeychainPasswordAsync({ oldPassword: '1234' })
226+
.should.be.rejectedWith('expected new password to be a string');
227+
228+
await keychains
229+
.updateSingleKeychainPasswordAsync({ oldPassword: '1234', newPassword: 5678 })
230+
.should.be.rejectedWith('expected new password to be a string');
231+
232+
await keychains
233+
.updateSingleKeychainPasswordAsync({ oldPassword: '1234', newPassword: '5678' })
234+
.should.be.rejectedWith('expected keychain to be an object with an encryptedPrv property');
235+
236+
await keychains
237+
.updateSingleKeychainPasswordAsync({ oldPassword: '1234', newPassword: '5678', keychain: {} })
238+
.should.be.rejectedWith('expected keychain to be an object with an encryptedPrv property');
239+
240+
await keychains
241+
.updateSingleKeychainPasswordAsync({
242+
oldPassword: '1234',
243+
newPassword: '5678',
244+
keychain: { encryptedPrv: 123 },
245+
})
246+
.should.be.rejectedWith('expected keychain to be an object with an encryptedPrv property');
247+
248+
// wrong password — decrypt fails
249+
const keychain = { encryptedPrv: bitgo.encrypt({ input: 'xprv1', password: otherPassword }) };
250+
await keychains
251+
.updateSingleKeychainPasswordAsync({ oldPassword, newPassword, keychain })
252+
.should.be.rejectedWith('failed to update keychain password: incorrect password');
253+
254+
// invalid JSON in encryptedPrv
255+
await keychains
256+
.updateSingleKeychainPasswordAsync({ oldPassword, newPassword, keychain: { encryptedPrv: 'not-valid-json' } })
257+
.should.be.rejectedWith('failed to update keychain password: decrypt: ciphertext is not valid JSON');
258+
259+
// unknown envelope version
260+
const unknownVersionEnvelope = JSON.stringify({ v: 99, ct: 'abc' });
261+
await keychains
262+
.updateSingleKeychainPasswordAsync({
263+
oldPassword,
264+
newPassword,
265+
keychain: { encryptedPrv: unknownVersionEnvelope },
266+
})
267+
.should.be.rejectedWith('failed to update keychain password: decrypt: unknown envelope version 99');
268+
});
269+
215270
it('on any other error', async function () {
216271
nock(bgUrl)
217272
.get('/api/v2/tltc/key')
@@ -225,7 +280,7 @@ describe('V2 Keychains', function () {
225280
],
226281
});
227282

228-
sandbox.stub(keychains, 'updateSingleKeychainPassword').throws('error', 'some random error');
283+
sandbox.stub(keychains, 'updateSingleKeychainPasswordAsync').throws('error', 'some random error');
229284

230285
await keychains.updatePassword({ oldPassword, newPassword }).should.be.rejectedWith('some random error');
231286
});
@@ -313,19 +368,78 @@ describe('V2 Keychains', function () {
313368
validateKeys(keys, newPassword, 3);
314369
});
315370

316-
it('single keychain password update', () => {
371+
it('single keychain password update', async () => {
317372
const prv = 'xprvtest';
318373
const keychain = {
319374
xpub: 'xpub123',
320375
encryptedPrv: bitgo.encrypt({ input: prv, password: oldPassword }),
321376
};
322377

323-
const newKeychain = keychains.updateSingleKeychainPassword({ keychain, oldPassword, newPassword });
378+
const newKeychain = await keychains.updateSingleKeychainPassword({ keychain, oldPassword, newPassword });
324379

325380
const decryptedPrv = bitgo.decrypt({ input: newKeychain.encryptedPrv, password: newPassword });
326381
decryptedPrv.should.equal(prv);
327382
});
328383

384+
it('single keychain password update preserves v2 (Argon2id) envelope', async () => {
385+
const prv = 'xprvtest-v2';
386+
const encryptedPrv = await bitgo.encryptAsync({ input: prv, password: oldPassword, encryptionVersion: 2 });
387+
const envelope = JSON.parse(encryptedPrv);
388+
envelope.v.should.equal(2, 'pre-condition: keychain must be v2-encrypted');
389+
390+
const keychain = { xpub: 'xpub123', encryptedPrv };
391+
const newKeychain = await keychains.updateSingleKeychainPasswordAsync({ keychain, oldPassword, newPassword });
392+
393+
const newEnvelope = JSON.parse(newKeychain.encryptedPrv);
394+
newEnvelope.v.should.equal(2, 're-encrypted keychain must still be v2');
395+
396+
const decryptedPrv = await bitgo.decryptAsync({ input: newKeychain.encryptedPrv, password: newPassword });
397+
decryptedPrv.should.equal(prv, 'new password must decrypt to original prv');
398+
399+
await bitgo.decryptAsync({ input: newKeychain.encryptedPrv, password: oldPassword }).should.be.rejected();
400+
});
401+
402+
it('updatePassword handles a mix of v1 and v2 keychains', async function () {
403+
const v1Prv = 'xprv-v1';
404+
const v2Prv = 'xprv-v2';
405+
406+
nock(bgUrl)
407+
.get('/api/v2/tltc/key')
408+
.query(true)
409+
.reply(200, {
410+
keys: [
411+
{
412+
pub: 'xpub-v1',
413+
encryptedPrv: bitgo.encrypt({ input: v1Prv, password: oldPassword }),
414+
},
415+
{
416+
pub: 'xpub-v2',
417+
encryptedPrv: await bitgo.encryptAsync({ input: v2Prv, password: oldPassword, encryptionVersion: 2 }),
418+
},
419+
{
420+
pub: 'xpub-other',
421+
encryptedPrv: bitgo.encrypt({ input: 'xprv-other', password: 'different-password' }),
422+
},
423+
],
424+
});
425+
426+
const updatedKeys = await keychains.updatePassword({ oldPassword, newPassword });
427+
428+
assert.strictEqual(Object.keys(updatedKeys).length, 2, 'only the two matching keychains should be updated');
429+
430+
const updatedV1 = updatedKeys['xpub-v1'];
431+
const updatedV2 = updatedKeys['xpub-v2'];
432+
assert.ok(updatedV1, 'v1 keychain must be in the result');
433+
assert.ok(updatedV2, 'v2 keychain must be in the result');
434+
435+
bitgo.decrypt({ input: updatedV1, password: newPassword }).should.equal(v1Prv);
436+
437+
const updatedV2Envelope = JSON.parse(updatedV2);
438+
updatedV2Envelope.v.should.equal(2, 'v2 keychain must remain v2 after password change');
439+
const decryptedV2 = await bitgo.decryptAsync({ input: updatedV2, password: newPassword });
440+
decryptedV2.should.equal(v2Prv);
441+
});
442+
329443
it('should return the updated keys with ids', async function () {
330444
nock(bgUrl)
331445
.get('/api/v2/tltc/key')
@@ -502,7 +616,7 @@ describe('V2 Keychains', function () {
502616
},
503617
});
504618

505-
sandbox.stub(BitGo.prototype, 'decrypt').returns(decryptResult);
619+
sandbox.stub(bitgo, 'decryptAsync').resolves(decryptResult);
506620
});
507621

508622
afterEach(function () {

modules/express/src/clientRoutes.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1248,7 +1248,7 @@ export async function handleKeychainChangePassword(
12481248
);
12491249
}
12501250

1251-
const updatedKeychain = coin.keychains().updateSingleKeychainPassword({
1251+
const updatedKeychain = await coin.keychains().updateSingleKeychainPasswordAsync({
12521252
keychain,
12531253
oldPassword,
12541254
newPassword,

modules/express/test/unit/clientRoutes/changeKeychainPassword.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ describe('Change Wallet Password', function () {
1616
const newPassword = 'newPasswordString';
1717

1818
const keychainBaseCoinStub = {
19-
keychains: () => ({ updateSingleKeychainPassword: () => Promise.resolve({ result: 'stubbed' }) }),
19+
keychains: () => ({ updateSingleKeychainPasswordAsync: () => Promise.resolve({ result: 'stubbed' }) }),
2020
};
2121

2222
it('should change wallet password', async function () {
@@ -27,7 +27,7 @@ describe('Change Wallet Password', function () {
2727
const coinStub = {
2828
keychains: () => ({
2929
get: () => Promise.resolve(keychainStub),
30-
updateSingleKeychainPassword: () => ({ result: 'stubbed' }),
30+
updateSingleKeychainPasswordAsync: () => Promise.resolve({ result: 'stubbed' }),
3131
}),
3232
url: () => 'url',
3333
};
@@ -82,7 +82,7 @@ describe('Change Wallet Password', function () {
8282
const coinStub = {
8383
keychains: () => ({
8484
get: () => Promise.resolve(keychainStub),
85-
updateSingleKeychainPassword: () => ({ result: 'stubbed' }),
85+
updateSingleKeychainPasswordAsync: () => Promise.resolve({ result: 'stubbed' }),
8686
}),
8787
url: () => 'url',
8888
};
@@ -136,7 +136,7 @@ describe('Change Wallet Password', function () {
136136
const coinStub = {
137137
keychains: () => ({
138138
get: () => Promise.resolve(keychainStub),
139-
updateSingleKeychainPassword: () => ({ result: 'stubbed' }),
139+
updateSingleKeychainPasswordAsync: () => Promise.resolve({ result: 'stubbed' }),
140140
}),
141141
url: () => 'url',
142142
};
@@ -189,7 +189,7 @@ describe('Change Wallet Password', function () {
189189
const coinStub = {
190190
keychains: () => ({
191191
get: () => Promise.resolve(keychainStub),
192-
updateSingleKeychainPassword: () => ({ result: 'stubbed' }),
192+
updateSingleKeychainPasswordAsync: () => Promise.resolve({ result: 'stubbed' }),
193193
}),
194194
url: () => 'url',
195195
};

modules/sdk-core/src/bitgo/keychain/iKeychains.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ export interface IKeychains {
240240
list(params?: ListKeychainOptions): Promise<ListKeychainsResult>;
241241
updatePassword(params: UpdatePasswordOptions): Promise<ChangedKeychains>;
242242
updateSingleKeychainPassword(params?: UpdateSingleKeychainPasswordOptions): Keychain;
243+
updateSingleKeychainPasswordAsync(params?: UpdateSingleKeychainPasswordOptions): Promise<Keychain>;
243244
create(params?: { seed?: Buffer; isRootKey?: boolean }): KeyPair;
244245
add(params?: AddKeychainOptions): Promise<Keychain>;
245246
createBitGo(params?: CreateBitGoOptions): Promise<Keychain>;

modules/sdk-core/src/bitgo/keychain/keychains.ts

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
UpdateSingleKeychainPasswordOptions,
2525
} from './iKeychains';
2626
import { BitGoKeyFromOvcShares, BitGoToOvcJSON, OvcToBitGoJSON } from './ovcJsonCodec';
27+
import { EncryptionVersion } from '../../api';
2728

2829
export class Keychains implements IKeychains {
2930
private readonly bitgo: BitGoBase;
@@ -113,7 +114,7 @@ export class Keychains implements IKeychains {
113114
continue;
114115
}
115116
try {
116-
const updatedKeychain = this.updateSingleKeychainPassword({
117+
const updatedKeychain = await this.updateSingleKeychainPasswordAsync({
117118
keychain: key,
118119
oldPassword: params.oldPassword,
119120
newPassword: params.newPassword,
@@ -145,12 +146,14 @@ export class Keychains implements IKeychains {
145146
}
146147

147148
/**
148-
* Update the password used to decrypt a single keychain
149+
* TODO: Deprecate this function in favor of updateSingleKeychainPasswordAsync once v2 encryption is default
150+
* Update the password used to decrypt a single keychain.
151+
* Handles v1 (SJCL) envelopes only. For v2 (Argon2id) support use {@link updateSingleKeychainPasswordAsync}.
149152
* @param params
150153
* @param params.keychain - The keychain whose password should be updated
151154
* @param params.oldPassword - The old password used for encrypting the key
152155
* @param params.newPassword - The new password to be used for encrypting the key
153-
* @returns {object}
156+
* @returns {Keychain}
154157
*/
155158
updateSingleKeychainPassword(params: UpdateSingleKeychainPasswordOptions = {}): Keychain {
156159
if (!_.isString(params.oldPassword)) {
@@ -176,6 +179,65 @@ export class Keychains implements IKeychains {
176179
}
177180
}
178181

182+
/**
183+
* Helper function to determine the encryption version of a ciphertext by parsing it as JSON and checking the "v" field.
184+
* Return undefined if the ciphertext is not a valid JSON or does not contain a supported "v" field.
185+
*/
186+
private getEncryptionVersion(ciphertext: string): EncryptionVersion | undefined {
187+
try {
188+
const envelope = JSON.parse(ciphertext);
189+
switch (envelope.v) {
190+
case 1:
191+
return 1;
192+
case 2:
193+
return 2;
194+
default:
195+
return undefined;
196+
}
197+
} catch (_) {
198+
return undefined;
199+
}
200+
}
201+
202+
/**
203+
* Update the password used to decrypt a single keychain, with support for v2 (Argon2id) envelopes.
204+
* Automatically detects and preserves the envelope version — a v2-encrypted key stays v2 after the password change.
205+
* @param params
206+
* @param params.keychain - The keychain whose password should be updated
207+
* @param params.oldPassword - The old password used for encrypting the key
208+
* @param params.newPassword - The new password to be used for encrypting the key
209+
* @returns {Promise<Keychain>}
210+
*/
211+
async updateSingleKeychainPasswordAsync(params: UpdateSingleKeychainPasswordOptions = {}): Promise<Keychain> {
212+
if (!_.isString(params.oldPassword)) {
213+
throw new Error('expected old password to be a string');
214+
}
215+
216+
if (!_.isString(params.newPassword)) {
217+
throw new Error('expected new password to be a string');
218+
}
219+
220+
if (!_.isObject(params.keychain) || !_.isString(params.keychain.encryptedPrv)) {
221+
throw new Error('expected keychain to be an object with an encryptedPrv property');
222+
}
223+
224+
const oldEncryptedPrv = params.keychain.encryptedPrv;
225+
try {
226+
const decryptedPrv = await this.bitgo.decryptAsync({ input: oldEncryptedPrv, password: params.oldPassword });
227+
const encryptionVersion = this.getEncryptionVersion(oldEncryptedPrv);
228+
const newEncryptedPrv = await this.bitgo.encryptAsync({
229+
input: decryptedPrv,
230+
password: params.newPassword,
231+
encryptionVersion,
232+
});
233+
return _.assign({}, params.keychain, { encryptedPrv: newEncryptedPrv });
234+
} catch (e) {
235+
// catching an error here means that the password was incorrect or, less likely, the input to decrypt is corrupted
236+
const errorDetail = e instanceof Error ? e.message : String(e);
237+
throw new Error(`failed to update keychain password: ${errorDetail}`);
238+
}
239+
}
240+
179241
/**
180242
* Create a public/private key pair
181243
* @param params - optional params
@@ -359,17 +421,17 @@ export class Keychains implements IKeychains {
359421
throw new Error('failed to get recovery info');
360422
}
361423

362-
const decryptedWalletPassphrase = this.bitgo.decrypt({
424+
const decryptedWalletPassphrase = await this.bitgo.decryptAsync({
363425
input: params.encryptedMaterial.encryptedWalletPassphrase,
364426
password: recoveryInfo.passcodeEncryptionCode,
365427
});
366428

367-
const decryptedUserKey = this.bitgo.decrypt({
429+
const decryptedUserKey = await this.bitgo.decryptAsync({
368430
input: params.encryptedMaterial.encryptedUserKey,
369431
password: decryptedWalletPassphrase,
370432
});
371433

372-
const decryptedBackupKey = this.bitgo.decrypt({
434+
const decryptedBackupKey = await this.bitgo.decryptAsync({
373435
input: params.encryptedMaterial.encryptedBackupKey,
374436
password: decryptedWalletPassphrase,
375437
});

0 commit comments

Comments
 (0)