diff --git a/modules/express/src/clientRoutes.ts b/modules/express/src/clientRoutes.ts index 7761fcca75..9e10658849 100755 --- a/modules/express/src/clientRoutes.ts +++ b/modules/express/src/clientRoutes.ts @@ -18,6 +18,7 @@ import { CustomPaillierModulusGetterFunction, CustomRShareGeneratingFunction, CustomSShareGeneratingFunction, + DeriveAddressOptions, EcdsaMPCv2Utils, EcdsaUtils, EddsaMPCv2Utils, @@ -697,6 +698,75 @@ export async function handleV2GenerateWallet(req: ExpressApiRouteRequest<'expres return { ...result, wallet: result.wallet.toJSON() }; } +/** + * Independently verify newly created address(es) against client-supplied trusted keychains. + * + * "Bring your own trusted keys": the caller passes keychains it holds independently, and we + * locally re-derive each created address and compare it to what the service returned. Because the + * keys do not come from the service, a match is an independent correctness guarantee (unlike the + * SDK's built-in check, which verifies using service-fetched keys). Throws on any mismatch. + * + * The caller opted in by supplying the keychains, so if the coin/wallet does not support local + * derivation we surface that as an error rather than silently skipping the requested verification. + */ +/** + * The fields of a created address we read to re-derive it. `wallet.createAddress` is typed `any` + * in the SDK and returns either a single address or `{ addresses: [...] }`, so we model just the + * subset we consume and reuse {@link DeriveAddressOptions} types for the derivation inputs. + */ +type CreatedAddress = { + address?: unknown; + index?: unknown; + chain?: number; + format?: DeriveAddressOptions['format']; + baseAddress?: string; + coinSpecific?: DeriveAddressOptions['coinSpecific']; +}; +type CreateAddressResult = CreatedAddress & { addresses?: CreatedAddress[] }; + +async function verifyCreatedAddressesWithTrustedKeys( + coin: ReturnType, + wallet: Wallet, + result: CreateAddressResult, + trustedKeychains: ({ pub: string } | { commonKeychain: string })[] +): Promise { + const addresses = Array.isArray(result?.addresses) ? result.addresses : [result]; + const walletCoinSpecific = (wallet.coinSpecific?.() ?? {}) as { walletVersion?: number; baseAddress?: string }; + + for (const addr of addresses) { + if (!addr || typeof addr.address !== 'string' || typeof addr.index !== 'number') { + continue; + } + // The caller opted in, so any derivation failure (e.g. an unsupported coin) must surface as a + // 400 rather than the default 500 — hence we map derivation errors and mismatches alike to 400. + try { + const derived = await coin.deriveAddress({ + keychains: trustedKeychains, + index: addr.index, + chain: addr.chain, + format: addr.format, + walletVersion: walletCoinSpecific.walletVersion, + baseAddress: addr.baseAddress ?? walletCoinSpecific.baseAddress, + coinSpecific: addr.coinSpecific, + }); + if (derived.address !== addr.address) { + throw new ApiResponseError( + `address verification failed: service returned ${addr.address} but trusted keychains derive ${derived.address}`, + 400 + ); + } + } catch (e) { + if (e instanceof ApiResponseError) { + throw e; + } + throw new ApiResponseError( + `address verification with trusted keychains failed for ${addr.address}: ${(e as Error).message}`, + 400 + ); + } + } +} + /** * handle new address creation * @param req @@ -705,7 +775,17 @@ export async function handleV2CreateAddress(req: ExpressApiRouteRequest<'express const bitgo = req.bitgo; const coin = bitgo.coin(req.decoded.coin); const wallet = await coin.wallets().get({ id: req.decoded.id }); - return wallet.createAddress(req.decoded); + const result = await wallet.createAddress(req.decoded); + + // Opt-in, per-request: if the caller supplied trusted keychains, independently verify the + // created address(es) before returning. No effect on requests that omit trustedKeychains. + // Token (e.g. SPL) deposit addresses derive differently (associated token account) and are not + // verified here yet — that lands once token derivation (WCN-1054) is available on this path. + const trustedKeychains = req.decoded.trustedKeychains; + if (trustedKeychains && trustedKeychains.length > 0 && !req.decoded.onToken) { + await verifyCreatedAddressesWithTrustedKeys(coin, wallet, result, trustedKeychains); + } + return result; } /** diff --git a/modules/express/src/typedRoutes/api/v2/createAddress.ts b/modules/express/src/typedRoutes/api/v2/createAddress.ts index 7a366fb999..b44251c61d 100644 --- a/modules/express/src/typedRoutes/api/v2/createAddress.ts +++ b/modules/express/src/typedRoutes/api/v2/createAddress.ts @@ -2,6 +2,7 @@ import * as t from 'io-ts'; import { httpRoute, httpRequest, optional } from '@api-ts/io-ts-http'; import { BitgoExpressError } from '../../schemas/error'; import { EIP1559, ForwarderVersion, CreateAddressFormat } from '../../schemas/address'; +import { DeriveAddressKeychainCodec } from './deriveAddress'; /** * Path parameters for creating a wallet address @@ -51,6 +52,15 @@ export const CreateAddressBody = { baseAddress: optional(t.string), /** When false, throws error if address verification is skipped (e.g., during pending chain initialization). Default: true */ allowSkipVerifyAddress: optional(t.boolean), + /** + * Optional independently-held keychains (public key material only). When provided, Express + * locally re-derives each newly created address from these keys and fails the request if it + * does not match the address returned by the service — an independent ("bring your own trusted + * keys") verification folded into the create call. Omit to keep the default behavior. + * Supported for coins with local derivation (BTC/UTXO, ETH MPC + forwarder, SOL incl. SPL tokens); + * requesting it for an unsupported coin returns an error rather than silently skipping. + */ + trustedKeychains: optional(t.array(DeriveAddressKeychainCodec)), } as const; /** Response for creating wallet address(es) */ diff --git a/modules/express/test/unit/typedRoutes/createAddress.ts b/modules/express/test/unit/typedRoutes/createAddress.ts index 5e34dd1669..dad41a13cd 100644 --- a/modules/express/test/unit/typedRoutes/createAddress.ts +++ b/modules/express/test/unit/typedRoutes/createAddress.ts @@ -157,6 +157,98 @@ describe('CreateAddress codec tests', function () { true ); }); + + describe('trustedKeychains verification', function () { + const trustedKeychains = [{ pub: 'xpub-user' }, { pub: 'xpub-backup' }, { pub: 'xpub-bitgo' }]; + + function stubCoin(deriveAddressStub: sinon.SinonStub) { + const mockWallet = { + createAddress: sinon.stub().resolves(mockResponse), + coinSpecific: sinon.stub().returns({}), + }; + const mockCoin = { + wallets: sinon.stub().returns({ get: sinon.stub().resolves(mockWallet) }), + deriveAddress: deriveAddressStub, + }; + sinon.stub(BitGo.prototype, 'coin').returns(mockCoin as any); + return { mockCoin }; + } + + it('returns the address when the derived address matches the trusted keychains', async function () { + const deriveAddressStub = sinon.stub().resolves({ address: mockResponse.address, index: 1 }); + const { mockCoin } = stubCoin(deriveAddressStub); + + const result = await agent + .post(`/api/v2/${coin}/wallet/${walletId}/address`) + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send({ chain: 0, trustedKeychains }); + + assert.strictEqual(result.status, 200); + assert.strictEqual(result.body.address, mockResponse.address); + // deriveAddress was called with the client's trusted keychains (not service keys) + sinon.assert.calledOnce(mockCoin.deriveAddress); + const callArgs = mockCoin.deriveAddress.firstCall.args[0]; + assert.deepStrictEqual(callArgs.keychains, trustedKeychains); + assert.strictEqual(callArgs.index, 1); + }); + + it('returns 400 when the derived address does not match (service returned a different address)', async function () { + const deriveAddressStub = sinon.stub().resolves({ address: '2NDifferentAddressXXXXXXXXXXXXXXXXXXX', index: 1 }); + stubCoin(deriveAddressStub); + + const result = await agent + .post(`/api/v2/${coin}/wallet/${walletId}/address`) + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send({ chain: 0, trustedKeychains }); + + assert.strictEqual(result.status, 400); + result.body.should.have.property('error'); + }); + + it('returns 400 when derivation is not supported for the coin/wallet (caller opted in)', async function () { + const deriveAddressStub = sinon.stub().rejects(new Error('deriveAddress is not supported for this coin')); + stubCoin(deriveAddressStub); + + const result = await agent + .post(`/api/v2/${coin}/wallet/${walletId}/address`) + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send({ chain: 0, trustedKeychains }); + + assert.strictEqual(result.status, 400); + result.body.should.have.property('error'); + }); + + it('does not verify (no deriveAddress call) when trustedKeychains is omitted', async function () { + const deriveAddressStub = sinon.stub().resolves({ address: mockResponse.address, index: 1 }); + const { mockCoin } = stubCoin(deriveAddressStub); + + const result = await agent + .post(`/api/v2/${coin}/wallet/${walletId}/address`) + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send({ chain: 0 }); + + assert.strictEqual(result.status, 200); + sinon.assert.notCalled(mockCoin.deriveAddress); + }); + + it('skips trusted verification for token (onToken) addresses', async function () { + const deriveAddressStub = sinon.stub().resolves({ address: mockResponse.address, index: 1 }); + const { mockCoin } = stubCoin(deriveAddressStub); + + const result = await agent + .post(`/api/v2/${coin}/wallet/${walletId}/address`) + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send({ chain: 0, trustedKeychains, onToken: 'tsol:usdc' }); + + assert.strictEqual(result.status, 200); + sinon.assert.notCalled(mockCoin.deriveAddress); + }); + }); }); describe('CreateAddressParams', function () {