Skip to content

feat(sdk-core): add v2 decryption support to password change #8804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bdesoky wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(sdk-core): add v2 decryption support to password change #8804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
67 changes: 63 additions & 4 deletions modules/bitgo/test/v2/unit/keychains.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,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 +313,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);
});

it('should return the updated keys with ids', async function () {
nock(bgUrl)
.get('/api/v2/tltc/key')
Expand Down Expand Up @@ -502,7 +561,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
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions modules/express/test/unit/clientRoutes/changeKeychainPassword.ts
View file
Open in desktop
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
View file
Open in desktop
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
61 changes: 55 additions & 6 deletions modules/sdk-core/src/bitgo/keychain/keychains.ts
View file
Open in desktop
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,13 @@ export class Keychains implements IKeychains {
}

/**
* Update the password used to decrypt a single keychain
* Update the password used to decrypt a single keychain.
* Handles v1 (SJCL) envelopes only. For v2 (Argon2id) support use {@link updateSingleKeychainPasswordAsync}.
Comment on lines +149 to +150
Copy link
Copy Markdown
Contributor

@pranavjain97 pranavjain97 May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would add a todo to deprecate this in the future

* @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 +178,53 @@ export class Keychains implements IKeychains {
}
}

/**
* 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 });
// Preserve the original envelope's encryption version so v2-encrypted keys stay v2 after the password change.
let encryptionVersion: EncryptionVersion | undefined;
try {
const parsed = JSON.parse(oldEncryptedPrv);
if (parsed.v === 2) {
encryptionVersion = 2;
}
} catch {
// non-JSON input — default to v1 (no encryptionVersion)
}
const newEncryptedPrv = await this.bitgo.encryptAsync({
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
throw new Error('password used to decrypt keychain private key is incorrect');
Comment on lines +222 to +224
Copy link
Copy Markdown
Contributor

@pranavjain97 pranavjain97 May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cant the input to decrypt be corrupted even for v1?

Copy link
Copy Markdown
Contributor Author

@bdesoky bdesoky May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is existing logic from the original updateSingleKeychainPassword

}
}
Comment on lines +203 to +226
Copy link
Copy Markdown
Contributor

@pranavjain97 pranavjain97 May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be made bit cleaner like this with a helper for detecing the encryption version. Also narrowing down the the try/catch.

const oldEncryptedPrv = params.keychain.encryptedPrv;
let decryptedPrv: string;
try {
  decryptedPrv = await this.bitgo.decryptAsync({ input: oldEncryptedPrv, password: params.oldPassword });
} catch (e) {
  throw new Error('password used to decrypt keychain private key is incorrect');
}
const encryptionVersion = detectEncryptionVersion(oldEncryptedPrv);
const newEncryptedPrv = await this.bitgo.encryptAsync({
  input: decryptedPrv,
  password: params.newPassword,
  encryptionVersion,
});
return _.assign({}, params.keychain, { encryptedPrv: newEncryptedPrv });

Copy link
Copy Markdown
Contributor Author

@bdesoky bdesoky May 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure can add this helper, though i think the callers will still need to handle try/catch since not all of them should throw if the ciphertext is not parsable


/**
* Create a public/private key pair
* @param params - optional params
Expand Down Expand Up @@ -359,17 +408,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