Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 118 additions & 4 deletions modules/bitgo/test/v2/unit/keychains.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,61 @@ describe('V2 Keychains', function () {
);
});

it('to update the password for a single keychain (async)', async function () {
await keychains
.updateSingleKeychainPasswordAsync({ newPassword: '5678' })
.should.be.rejectedWith('expected old password to be a string');

await keychains
.updateSingleKeychainPasswordAsync({ oldPassword: 1234, newPassword: '5678' })
.should.be.rejectedWith('expected old password to be a string');

await keychains
.updateSingleKeychainPasswordAsync({ oldPassword: '1234' })
.should.be.rejectedWith('expected new password to be a string');

await keychains
.updateSingleKeychainPasswordAsync({ oldPassword: '1234', newPassword: 5678 })
.should.be.rejectedWith('expected new password to be a string');

await keychains
.updateSingleKeychainPasswordAsync({ oldPassword: '1234', newPassword: '5678' })
.should.be.rejectedWith('expected keychain to be an object with an encryptedPrv property');

await keychains
.updateSingleKeychainPasswordAsync({ oldPassword: '1234', newPassword: '5678', keychain: {} })
.should.be.rejectedWith('expected keychain to be an object with an encryptedPrv property');

await keychains
.updateSingleKeychainPasswordAsync({
oldPassword: '1234',
newPassword: '5678',
keychain: { encryptedPrv: 123 },
})
.should.be.rejectedWith('expected keychain to be an object with an encryptedPrv property');

// wrong password — decrypt fails
const keychain = { encryptedPrv: bitgo.encrypt({ input: 'xprv1', password: otherPassword }) };
await keychains
.updateSingleKeychainPasswordAsync({ oldPassword, newPassword, keychain })
.should.be.rejectedWith('failed to update keychain password: incorrect password');

// invalid JSON in encryptedPrv
await keychains
.updateSingleKeychainPasswordAsync({ oldPassword, newPassword, keychain: { encryptedPrv: 'not-valid-json' } })
.should.be.rejectedWith('failed to update keychain password: decrypt: ciphertext is not valid JSON');

// unknown envelope version
const unknownVersionEnvelope = JSON.stringify({ v: 99, ct: 'abc' });
await keychains
.updateSingleKeychainPasswordAsync({
oldPassword,
newPassword,
keychain: { encryptedPrv: unknownVersionEnvelope },
})
.should.be.rejectedWith('failed to update keychain password: decrypt: unknown envelope version 99');
});

it('on any other error', async function () {
nock(bgUrl)
.get('/api/v2/tltc/key')
Expand All @@ -225,7 +280,7 @@ describe('V2 Keychains', function () {
],
});

sandbox.stub(keychains, 'updateSingleKeychainPassword').throws('error', 'some random error');
sandbox.stub(keychains, 'updateSingleKeychainPasswordAsync').throws('error', 'some random error');

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

it('single keychain password update', () => {
it('single keychain password update', async () => {
const prv = 'xprvtest';
const keychain = {
xpub: 'xpub123',
encryptedPrv: bitgo.encrypt({ input: prv, password: oldPassword }),
};

const newKeychain = keychains.updateSingleKeychainPassword({ keychain, oldPassword, newPassword });
const newKeychain = await keychains.updateSingleKeychainPassword({ keychain, oldPassword, newPassword });

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

it('single keychain password update preserves v2 (Argon2id) envelope', async () => {
const prv = 'xprvtest-v2';
const encryptedPrv = await bitgo.encryptAsync({ input: prv, password: oldPassword, encryptionVersion: 2 });
const envelope = JSON.parse(encryptedPrv);
envelope.v.should.equal(2, 'pre-condition: keychain must be v2-encrypted');

const keychain = { xpub: 'xpub123', encryptedPrv };
const newKeychain = await keychains.updateSingleKeychainPasswordAsync({ keychain, oldPassword, newPassword });

const newEnvelope = JSON.parse(newKeychain.encryptedPrv);
newEnvelope.v.should.equal(2, 're-encrypted keychain must still be v2');

const decryptedPrv = await bitgo.decryptAsync({ input: newKeychain.encryptedPrv, password: newPassword });
decryptedPrv.should.equal(prv, 'new password must decrypt to original prv');

await bitgo.decryptAsync({ input: newKeychain.encryptedPrv, password: oldPassword }).should.be.rejected();
});

it('updatePassword handles a mix of v1 and v2 keychains', async function () {
const v1Prv = 'xprv-v1';
const v2Prv = 'xprv-v2';

nock(bgUrl)
.get('/api/v2/tltc/key')
.query(true)
.reply(200, {
keys: [
{
pub: 'xpub-v1',
encryptedPrv: bitgo.encrypt({ input: v1Prv, password: oldPassword }),
},
{
pub: 'xpub-v2',
encryptedPrv: await bitgo.encryptAsync({ input: v2Prv, password: oldPassword, encryptionVersion: 2 }),
},
{
pub: 'xpub-other',
encryptedPrv: bitgo.encrypt({ input: 'xprv-other', password: 'different-password' }),
},
],
});

const updatedKeys = await keychains.updatePassword({ oldPassword, newPassword });

assert.strictEqual(Object.keys(updatedKeys).length, 2, 'only the two matching keychains should be updated');

const updatedV1 = updatedKeys['xpub-v1'];
const updatedV2 = updatedKeys['xpub-v2'];
assert.ok(updatedV1, 'v1 keychain must be in the result');
assert.ok(updatedV2, 'v2 keychain must be in the result');

bitgo.decrypt({ input: updatedV1, password: newPassword }).should.equal(v1Prv);

const updatedV2Envelope = JSON.parse(updatedV2);
updatedV2Envelope.v.should.equal(2, 'v2 keychain must remain v2 after password change');
const decryptedV2 = await bitgo.decryptAsync({ input: updatedV2, password: newPassword });
decryptedV2.should.equal(v2Prv);
});

Comment thread
pranavjain97 marked this conversation as resolved.
it('should return the updated keys with ids', async function () {
nock(bgUrl)
.get('/api/v2/tltc/key')
Expand Down Expand Up @@ -502,7 +616,7 @@ describe('V2 Keychains', function () {
},
});

sandbox.stub(BitGo.prototype, 'decrypt').returns(decryptResult);
sandbox.stub(bitgo, 'decryptAsync').resolves(decryptResult);
});

afterEach(function () {
Expand Down
2 changes: 1 addition & 1 deletion modules/express/src/clientRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1247,7 +1247,7 @@ export async function handleKeychainChangePassword(
);
}

const updatedKeychain = coin.keychains().updateSingleKeychainPassword({
const updatedKeychain = await coin.keychains().updateSingleKeychainPasswordAsync({
keychain,
oldPassword,
newPassword,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('Change Wallet Password', function () {
const newPassword = 'newPasswordString';

const keychainBaseCoinStub = {
keychains: () => ({ updateSingleKeychainPassword: () => Promise.resolve({ result: 'stubbed' }) }),
keychains: () => ({ updateSingleKeychainPasswordAsync: () => Promise.resolve({ result: 'stubbed' }) }),
};

it('should change wallet password', async function () {
Expand All @@ -27,7 +27,7 @@ describe('Change Wallet Password', function () {
const coinStub = {
keychains: () => ({
get: () => Promise.resolve(keychainStub),
updateSingleKeychainPassword: () => ({ result: 'stubbed' }),
updateSingleKeychainPasswordAsync: () => Promise.resolve({ result: 'stubbed' }),
}),
url: () => 'url',
};
Expand Down Expand Up @@ -82,7 +82,7 @@ describe('Change Wallet Password', function () {
const coinStub = {
keychains: () => ({
get: () => Promise.resolve(keychainStub),
updateSingleKeychainPassword: () => ({ result: 'stubbed' }),
updateSingleKeychainPasswordAsync: () => Promise.resolve({ result: 'stubbed' }),
}),
url: () => 'url',
};
Expand Down Expand Up @@ -136,7 +136,7 @@ describe('Change Wallet Password', function () {
const coinStub = {
keychains: () => ({
get: () => Promise.resolve(keychainStub),
updateSingleKeychainPassword: () => ({ result: 'stubbed' }),
updateSingleKeychainPasswordAsync: () => Promise.resolve({ result: 'stubbed' }),
}),
url: () => 'url',
};
Expand Down Expand Up @@ -189,7 +189,7 @@ describe('Change Wallet Password', function () {
const coinStub = {
keychains: () => ({
get: () => Promise.resolve(keychainStub),
updateSingleKeychainPassword: () => ({ result: 'stubbed' }),
updateSingleKeychainPasswordAsync: () => Promise.resolve({ result: 'stubbed' }),
}),
url: () => 'url',
};
Expand Down
1 change: 1 addition & 0 deletions modules/sdk-core/src/bitgo/keychain/iKeychains.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ export interface IKeychains {
list(params?: ListKeychainOptions): Promise<ListKeychainsResult>;
updatePassword(params: UpdatePasswordOptions): Promise<ChangedKeychains>;
updateSingleKeychainPassword(params?: UpdateSingleKeychainPasswordOptions): Keychain;
updateSingleKeychainPasswordAsync(params?: UpdateSingleKeychainPasswordOptions): Promise<Keychain>;
create(params?: { seed?: Buffer; isRootKey?: boolean }): KeyPair;
add(params?: AddKeychainOptions): Promise<Keychain>;
createBitGo(params?: CreateBitGoOptions): Promise<Keychain>;
Expand Down
74 changes: 68 additions & 6 deletions modules/sdk-core/src/bitgo/keychain/keychains.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
UpdateSingleKeychainPasswordOptions,
} from './iKeychains';
import { BitGoKeyFromOvcShares, BitGoToOvcJSON, OvcToBitGoJSON } from './ovcJsonCodec';
import { EncryptionVersion } from '../../api';

export class Keychains implements IKeychains {
private readonly bitgo: BitGoBase;
Expand Down Expand Up @@ -113,7 +114,7 @@ export class Keychains implements IKeychains {
continue;
}
try {
const updatedKeychain = this.updateSingleKeychainPassword({
const updatedKeychain = await this.updateSingleKeychainPasswordAsync({
keychain: key,
oldPassword: params.oldPassword,
newPassword: params.newPassword,
Expand Down Expand Up @@ -145,12 +146,14 @@ export class Keychains implements IKeychains {
}

/**
* Update the password used to decrypt a single keychain
* TODO: Deprecate this function in favor of updateSingleKeychainPasswordAsync once v2 encryption is default
* Update the password used to decrypt a single keychain.
* Handles v1 (SJCL) envelopes only. For v2 (Argon2id) support use {@link updateSingleKeychainPasswordAsync}.
Comment thread
bdesoky marked this conversation as resolved.
* @param params
* @param params.keychain - The keychain whose password should be updated
* @param params.oldPassword - The old password used for encrypting the key
* @param params.newPassword - The new password to be used for encrypting the key
* @returns {object}
* @returns {Keychain}
*/
updateSingleKeychainPassword(params: UpdateSingleKeychainPasswordOptions = {}): Keychain {
if (!_.isString(params.oldPassword)) {
Expand All @@ -176,6 +179,65 @@ export class Keychains implements IKeychains {
}
}

/**
* Helper function to determine the encryption version of a ciphertext by parsing it as JSON and checking the "v" field.
* Return undefined if the ciphertext is not a valid JSON or does not contain a supported "v" field.
*/
private getEncryptionVersion(ciphertext: string): EncryptionVersion | undefined {
try {
const envelope = JSON.parse(ciphertext);
switch (envelope.v) {
case 1:
return 1;
case 2:
return 2;
default:
return undefined;
}
} catch (_) {
return undefined;
}
}

/**
* Update the password used to decrypt a single keychain, with support for v2 (Argon2id) envelopes.
* Automatically detects and preserves the envelope version — a v2-encrypted key stays v2 after the password change.
* @param params
* @param params.keychain - The keychain whose password should be updated
* @param params.oldPassword - The old password used for encrypting the key
* @param params.newPassword - The new password to be used for encrypting the key
* @returns {Promise<Keychain>}
*/
async updateSingleKeychainPasswordAsync(params: UpdateSingleKeychainPasswordOptions = {}): Promise<Keychain> {
if (!_.isString(params.oldPassword)) {
throw new Error('expected old password to be a string');
}

if (!_.isString(params.newPassword)) {
throw new Error('expected new password to be a string');
}

if (!_.isObject(params.keychain) || !_.isString(params.keychain.encryptedPrv)) {
throw new Error('expected keychain to be an object with an encryptedPrv property');
}

const oldEncryptedPrv = params.keychain.encryptedPrv;
try {
const decryptedPrv = await this.bitgo.decryptAsync({ input: oldEncryptedPrv, password: params.oldPassword });
const encryptionVersion = this.getEncryptionVersion(oldEncryptedPrv);
const newEncryptedPrv = await this.bitgo.encryptAsync({
Comment thread
pranavjain97 marked this conversation as resolved.
input: decryptedPrv,
password: params.newPassword,
encryptionVersion,
});
return _.assign({}, params.keychain, { encryptedPrv: newEncryptedPrv });
} catch (e) {
// catching an error here means that the password was incorrect or, less likely, the input to decrypt is corrupted
const errorDetail = e instanceof Error ? e.message : String(e);
throw new Error(`failed to update keychain password: ${errorDetail}`);
}
}
Comment thread
bdesoky marked this conversation as resolved.

/**
* Create a public/private key pair
* @param params - optional params
Expand Down Expand Up @@ -359,17 +421,17 @@ export class Keychains implements IKeychains {
throw new Error('failed to get recovery info');
}

const decryptedWalletPassphrase = this.bitgo.decrypt({
const decryptedWalletPassphrase = await this.bitgo.decryptAsync({
input: params.encryptedMaterial.encryptedWalletPassphrase,
password: recoveryInfo.passcodeEncryptionCode,
});

const decryptedUserKey = this.bitgo.decrypt({
const decryptedUserKey = await this.bitgo.decryptAsync({
input: params.encryptedMaterial.encryptedUserKey,
password: decryptedWalletPassphrase,
});

const decryptedBackupKey = this.bitgo.decrypt({
const decryptedBackupKey = await this.bitgo.decryptAsync({
input: params.encryptedMaterial.encryptedBackupKey,
password: decryptedWalletPassphrase,
});
Expand Down
Loading