diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index 833aa8ac68..34c9613d56 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -102,6 +102,7 @@ const { getExternalChainCode, isChainCode, scriptTypeForChain, outputScripts } = type Unspent = bitgo.Unspent; type RootWalletKeys = bitgo.RootWalletKeys; + export interface VerifyAddressOptions extends BaseVerifyAddressOptions { chain: number; index: number; @@ -280,9 +281,9 @@ export interface RecoverFromWrongChainOptions { } export interface VerifyKeySignaturesOptions { - userKeychain?: Keychain; - keychainToVerify?: Keychain; - keySignature?: string; + userKeychain: { pub?: string }; + keychainToVerify: { pub?: string }; + keySignature: string; } export interface VerifyUserPublicKeyOptions { @@ -677,7 +678,7 @@ export abstract class AbstractUtxoCoin extends BaseCoin { * @param {VerifyKeySignaturesOptions} params * @return {{backup: boolean, bitgo: boolean}} */ - protected verifyKeySignature(params: VerifyKeySignaturesOptions): boolean { + public verifyKeySignature(params: VerifyKeySignaturesOptions): boolean { // first, let's verify the integrity of the user key, whose public key is used for subsequent verifications const { userKeychain, keychainToVerify, keySignature } = params; if (!userKeychain) { @@ -695,10 +696,16 @@ export abstract class AbstractUtxoCoin extends BaseCoin { // verify the signature against the user public key assert(userKeychain.pub); const publicKey = bip32.fromBase58(userKeychain.pub).publicKey; + // Due to interface of `bitcoinMessage`, we need to convert the public key to an address. + // Note that this address has no relationship to on-chain transactions. We are + // only interested in the address as a representation of the public key. const signingAddress = utxolib.address.toBase58Check( utxolib.crypto.hash160(publicKey), utxolib.networks.bitcoin.pubKeyHash, - this.network + // we do not pass `this.network` here because it would fail for zcash + // the bitcoinMessage library decodes the address and throws away the first byte + // because zcash has a two-byte prefix, verify() decodes zcash addresses to an invalid pubkey hash + utxolib.networks.bitcoin ); // BG-5703: use BTC mainnet prefix for all key signature operations @@ -740,7 +747,13 @@ export abstract class AbstractUtxoCoin extends BaseCoin { if (!keySignature) { throw new Error(`missing required custom change ${KeyIndices[keyIndex].toLowerCase()} keychain signature`); } - if (!this.verifyKeySignature({ userKeychain, keychainToVerify, keySignature })) { + if ( + !this.verifyKeySignature({ + userKeychain: userKeychain as { pub: string }, + keychainToVerify: keychainToVerify as { pub: string }, + keySignature, + }) + ) { debug('failed to verify custom change %s key signature!', KeyIndices[keyIndex].toLowerCase()); return false; } @@ -810,8 +823,16 @@ export abstract class AbstractUtxoCoin extends BaseCoin { // let's verify these keychains const keySignatures = parsedTransaction.keySignatures; if (!_.isEmpty(keySignatures)) { - const verify = (key, pub) => - this.verifyKeySignature({ userKeychain: keychains.user, keychainToVerify: key, keySignature: pub }); + const verify = (key, pub) => { + if (!keychains.user || !keychains.user.pub) { + throw new Error('missing user keychain'); + } + return this.verifyKeySignature({ + userKeychain: keychains.user as { pub: string }, + keychainToVerify: key, + keySignature: pub, + }); + }; const isBackupKeySignatureValid = verify(keychains.backup, keySignatures.backupPub); const isBitgoKeySignatureValid = verify(keychains.bitgo, keySignatures.bitgoPub); if (!isBackupKeySignatureValid || !isBitgoKeySignatureValid) { diff --git a/modules/bitgo/test/v2/unit/coins/utxo/keySignatures.ts b/modules/bitgo/test/v2/unit/coins/utxo/keySignatures.ts new file mode 100644 index 0000000000..4c03cc2dd6 --- /dev/null +++ b/modules/bitgo/test/v2/unit/coins/utxo/keySignatures.ts @@ -0,0 +1,31 @@ +import * as assert from 'assert'; +import { AbstractUtxoCoin } from '@bitgo/abstract-utxo'; +import { Keychain } from '@bitgo/sdk-core'; + +import { utxoCoins } from './util'; + +function describeWithCoin(coin: AbstractUtxoCoin) { + describe(`verifyKeySignatures for ${coin.getChain()}`, function () { + it('should verify key signature of ZEC', async () => { + const userKeychain = await coin.keychains().create(); + const backupKeychain = await coin.keychains().create(); + const bitgoKeychain = await coin.keychains().create(); + + const signatures = await coin.createKeySignatures( + userKeychain.prv, + { pub: backupKeychain.pub as string }, + { pub: bitgoKeychain.pub as string } + ); + + assert.ok( + await coin.verifyKeySignature({ + userKeychain: userKeychain as unknown as Keychain, + keychainToVerify: backupKeychain as unknown as Keychain, + keySignature: signatures.backup, + }) + ); + }); + }); +} + +utxoCoins.forEach((coin) => describeWithCoin(coin)); diff --git a/modules/sdk-core/src/bitgo/baseCoin/baseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/baseCoin.ts index 420f4473a0..466023d1df 100644 --- a/modules/sdk-core/src/bitgo/baseCoin/baseCoin.ts +++ b/modules/sdk-core/src/bitgo/baseCoin/baseCoin.ts @@ -227,6 +227,28 @@ export abstract class BaseCoin implements IBaseCoin { return signMessage(message, bip32.fromBase58(key.prv), utxolib.networks.bitcoin); } + /** + * Create signatures for the backup and bitgo keys using the user key. + * We can verify the signatures when fetching the keys from wallet-platform later. + * Currently only `AbstractUtxoCoin` implements and uses the complementary `verifyKeySignature` method. + * @param prv - the user private key + * @param backupKeychain - contains the backup public key + * @param bitgoKeychain - contains the bitgo public key + */ + public async createKeySignatures( + prv: string, + backupKeychain: { pub: string }, + bitgoKeychain: { pub: string } + ): Promise<{ + backup: string; + bitgo: string; + }> { + return { + backup: (await this.signMessage({ prv }, backupKeychain.pub)).toString('hex'), + bitgo: (await this.signMessage({ prv }, bitgoKeychain.pub)).toString('hex'), + }; + } + /** * Decompose a raw transaction into useful information. * @param options - coin-specific diff --git a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts index 0ed4f37c38..23b3a713fa 100644 --- a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts +++ b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts @@ -438,6 +438,14 @@ export interface IBaseCoin { baseUnitsToBigUnits(baseUnits: string | number): string; bigUnitsToBaseUnits(bigUnits: string | number): string; signMessage(key: { prv: string }, message: string): Promise; + createKeySignatures( + prv: string, + backupKeychain: { pub: string }, + bitgoKeychain: { pub: string } + ): Promise<{ + backup: string; + bitgo: string; + }>; explainTransaction(options: Record): Promise | undefined>; verifyTransaction(params: VerifyTransactionOptions): Promise; verifyAddress(params: VerifyAddressOptions): Promise;