Skip to content
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

Jump to bottom

feat: remove GG18 signing during ECDSA TSS recovery #4883

Closed
wants to merge 4 commits into from
Closed

feat: remove GG18 signing during ECDSA TSS recovery #4883

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ab29ff9
feat(abstract-cosmos): remove GG18 recovery tx signing
zahin-mohammad Sep 3, 2024
b550b28
feat(abstract-eth): remove GG18 recovery tx signing
zahin-mohammad Sep 3, 2024
0f71cb9
chore(root): remove openSSLBytes from tests
zahin-mohammad Sep 3, 2024
af3ab2d
WIP
zahin-mohammad Sep 10, 2024
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
81 changes: 46 additions & 35 deletions modules/abstract-cosmos/src/cosmosCoin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
VerifyAddressOptions,
VerifyTransactionOptions,
} from '@bitgo/sdk-core';
import { EcdsaPaillierProof, EcdsaRangeProof, EcdsaTypes } from '@bitgo/sdk-lib-mpc';
import { EcdsaPaillierProof, EcdsaRangeProof, EcdsaTypes, DklsTypes, DklsUtils } from '@bitgo/sdk-lib-mpc';
import { BaseCoin as StaticsBaseCoin, CoinFamily } from '@bitgo/statics';
import { bip32 } from '@bitgo/utxo-lib';
import { Coin } from '@cosmjs/stargate';
Expand Down Expand Up @@ -136,7 +136,7 @@ export class CosmosCoin extends BaseCoin {
* @returns {CosmosLikeCoinRecoveryOutput} the serialized transaction hex string and index
* of the address being swept
*/
async recover(params: RecoveryOptions, openSSLBytes: Uint8Array): Promise<CosmosLikeCoinRecoveryOutput> {
async recover(params: RecoveryOptions): Promise<CosmosLikeCoinRecoveryOutput> {
// Step 1: Check if params contains the required parameters
if (!params.bitgoKey) {
throw new Error('missing bitgoKey');
Expand All @@ -157,9 +157,6 @@ export class CosmosCoin extends BaseCoin {
if (!params.walletPassphrase) {
throw new Error('missing wallet passphrase');
}
if (!openSSLBytes) {
throw new Error('missing openSSLBytes');
}

// Step 2: Fetch the bitgo key from params
const userKey = params.userKey.replace(/\s/g, '');
Expand Down Expand Up @@ -215,47 +212,61 @@ export class CosmosCoin extends BaseCoin {
const signableHex = unsignedTransaction.signablePayload.toString('hex');

const isGG18SigningMaterial = ECDSAUtils.isGG18SigningMaterial(userKey, params.walletPassphrase);
let signature: ECDSA.Signature;

if (isGG18SigningMaterial) {
// GG18
const [userKeyCombined, backupKeyCombined] = ((): [
ECDSAMethodTypes.KeyCombined | undefined,
ECDSAMethodTypes.KeyCombined | undefined
] => {
const [userKeyCombined, backupKeyCombined] = this.getKeyCombinedFromTssKeyShares(
userKey,
backupKey,
params.walletPassphrase
);
return [userKeyCombined, backupKeyCombined];
})();

if (!userKeyCombined || !backupKeyCombined) {
throw new Error('Missing combined key shares for user or backup');
}
/**
* MPCv2 Signing Params
*/
let userKeyShare: Buffer;
let backupKeyShare: Buffer;
let commonKeyChain: string;

// Step 7: Sign the tx
signature = await this.signRecoveryTSS(userKeyCombined, backupKeyCombined, signableHex, openSSLBytes);
} else {
// DKLS
const { userKeyShare, backupKeyShare, commonKeyChain } = await ECDSAUtils.getMpcV2RecoveryKeyShares(
// Step 7: Prepare the key shares for signing
if (isGG18SigningMaterial) {
// Retrofit the GG18 keys to DKLS
const [userKeyCombined, backupKeyCombined] = this.getKeyCombinedFromTssKeyShares(
userKey,
backupKey,
params.walletPassphrase
); // baseAddress is not extracted
);

if (!userKeyShare || !backupKeyShare || !commonKeyChain) {
throw new Error('Missing combined key shares for user or backup or common');
const aKeyCombine = {
xShare: userKeyCombined.xShare,
};
const bKeyCombine = {
xShare: backupKeyCombined.xShare,
};
const retrofitDataA: DklsTypes.RetrofitData = {
xShare: aKeyCombine.xShare,
};
const retrofitDataB: DklsTypes.RetrofitData = {
xShare: bKeyCombine.xShare,
};
const [user, backup] = await DklsUtils.generate2of2KeyShares(retrofitDataA, retrofitDataB);

userKeyShare = user.getKeyShare();
backupKeyShare = backup.getKeyShare();
if (DklsTypes.getCommonKeychain(userKeyShare) !== DklsTypes.getCommonKeychain(backupKeyShare)) {
throw new Error('Common keychain mismatch! Ensure the correct user and backup keys where provided!');
}
commonKeyChain = DklsTypes.getCommonKeychain(userKeyShare);
} else {
// DKLS
const mpcv2KeyShares = await ECDSAUtils.getMpcV2RecoveryKeyShares(userKey, backupKey, params.walletPassphrase); // baseAddress is not extracted

// Step 7: Sign the tx
const message = unsignedTransaction.signablePayload;
const messageHash = (utils.getHashFunction() || createHash('sha256')).update(message).digest();
userKeyShare = mpcv2KeyShares.userKeyShare;
backupKeyShare = mpcv2KeyShares.backupKeyShare;
commonKeyChain = mpcv2KeyShares.commonKeyChain;

signature = await ECDSAUtils.signRecoveryMpcV2(messageHash, userKeyShare, backupKeyShare, commonKeyChain);
if (!userKeyShare || !backupKeyShare || !commonKeyChain) {
throw new Error('Missing combined key shares for user or backup or common');
}
}

// Step 8: Sign the tx
const message = unsignedTransaction.signablePayload;
const messageHash = (utils.getHashFunction() || createHash('sha256')).update(message).digest();
const signature = await ECDSAUtils.signRecoveryMpcV2(messageHash, userKeyShare, backupKeyShare, commonKeyChain);

const signableBuffer = Buffer.from(signableHex, 'hex');
MPC.verify(signableBuffer, signature, this.getHashFunction());
const cosmosKeyPair = this.getKeyPair(publicKey);
Expand Down
185 changes: 53 additions & 132 deletions modules/abstract-eth/src/abstractEthLikeNewCoins.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,13 @@ import {
BitGoBase,
BuildNftTransferDataOptions,
common,
ECDSA,
Ecdsa,
ECDSAMethodTypes,
EthereumLibraryUnavailableError,
FeeEstimateOptions,
FullySignedTransaction,
getIsUnsignedSweep,
HalfSignedTransaction,
hexToBigInt,
InvalidAddressError,
InvalidAddressVerificationObjectPropertyError,
IWallet,
Expand All @@ -36,7 +34,7 @@ import {
Wallet,
ECDSAUtils,
} from '@bitgo/sdk-core';
import { EcdsaPaillierProof, EcdsaRangeProof, EcdsaTypes } from '@bitgo/sdk-lib-mpc';
import { DklsTypes, DklsUtils } from '@bitgo/sdk-lib-mpc';
import {
BaseCoin as StaticsBaseCoin,
CoinMap,
Expand Down Expand Up @@ -202,15 +200,19 @@ interface UnformattedTxInfo {
recipient: Recipient;
}

/**
* @deprecated: this type is no longer used and will be removed in future versions
*/
export type RecoverOptionsWithBytes = {
isTss: true;
openSSLBytes: Uint8Array;
};

export type NonTSSRecoverOptions = {
isTss?: false | undefined;
isTss?: boolean;
};

export type TSSRecoverOptions = RecoverOptionsWithBytes | NonTSSRecoverOptions;
export type TSSRecoverOptions = NonTSSRecoverOptions;

export type RecoverOptions = {
userKey: string;
Expand Down Expand Up @@ -1051,108 +1053,6 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
}
}

/**
* Method to sign tss recovery transaction
* @param {ECDSA.KeyCombined} userKeyCombined
* @param {ECDSA.KeyCombined} backupKeyCombined
* @param {string} txHex
* @param {Object} options
* @param {EcdsaTypes.SerializedNtilde} options.rangeProofChallenge
* @returns {Promise<ECDSAMethodTypes.Signature>}
*/
private async signRecoveryTSS(
userKeyCombined: ECDSA.KeyCombined,
backupKeyCombined: ECDSA.KeyCombined,
txHex: string,
openSSLBytes: Uint8Array,
{
rangeProofChallenge,
}: {
rangeProofChallenge?: EcdsaTypes.SerializedNtilde;
} = {}
): Promise<ECDSAMethodTypes.Signature> {
if (!userKeyCombined || !backupKeyCombined) {
throw new Error('Missing key combined shares for user or backup');
}

const MPC = new Ecdsa();
const signerOneIndex = userKeyCombined.xShare.i;
const signerTwoIndex = backupKeyCombined.xShare.i;

rangeProofChallenge =
rangeProofChallenge ?? EcdsaTypes.serializeNtildeWithProofs(await EcdsaRangeProof.generateNtilde(openSSLBytes));

const userToBackupPaillierChallenge = await EcdsaPaillierProof.generateP(
hexToBigInt(userKeyCombined.yShares[signerTwoIndex].n)
);
const backupToUserPaillierChallenge = await EcdsaPaillierProof.generateP(
hexToBigInt(backupKeyCombined.yShares[signerOneIndex].n)
);

const userXShare = MPC.appendChallenge(
userKeyCombined.xShare,
rangeProofChallenge,
EcdsaTypes.serializePaillierChallenge({ p: userToBackupPaillierChallenge })
);
const userYShare = MPC.appendChallenge(
userKeyCombined.yShares[signerTwoIndex],
rangeProofChallenge,
EcdsaTypes.serializePaillierChallenge({ p: backupToUserPaillierChallenge })
);
const backupXShare = MPC.appendChallenge(
backupKeyCombined.xShare,
rangeProofChallenge,
EcdsaTypes.serializePaillierChallenge({ p: backupToUserPaillierChallenge })
);
const backupYShare = MPC.appendChallenge(
backupKeyCombined.yShares[signerOneIndex],
rangeProofChallenge,
EcdsaTypes.serializePaillierChallenge({ p: userToBackupPaillierChallenge })
);

const signShares: ECDSA.SignShareRT = await MPC.signShare(userXShare, userYShare);

const signConvertS21 = await MPC.signConvertStep1({
xShare: backupXShare,
yShare: backupYShare, // YShare corresponding to the other participant signerOne
kShare: signShares.kShare,
});
const signConvertS12 = await MPC.signConvertStep2({
aShare: signConvertS21.aShare,
wShare: signShares.wShare,
});
const signConvertS21_2 = await MPC.signConvertStep3({
muShare: signConvertS12.muShare,
bShare: signConvertS21.bShare,
});

const [signCombineOne, signCombineTwo] = [
MPC.signCombine({
gShare: signConvertS12.gShare,
signIndex: {
i: signConvertS12.muShare.i,
j: signConvertS12.muShare.j,
},
}),
MPC.signCombine({
gShare: signConvertS21_2.gShare,
signIndex: {
i: signConvertS21_2.signIndex.i,
j: signConvertS21_2.signIndex.j,
},
}),
];

const MESSAGE = Buffer.from(txHex, 'hex');

const [signA, signB] = [
MPC.sign(MESSAGE, signCombineOne.oShare, signCombineTwo.dShare, Keccak('keccak256')),
MPC.sign(MESSAGE, signCombineTwo.oShare, signCombineOne.dShare, Keccak('keccak256')),
];

return MPC.constructSignature([signA, signB]);
}

/**
* Helper which combines key shares of user and backup
* @param {string} userPublicOrPrivateKeyShare
Expand Down Expand Up @@ -1284,7 +1184,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
*/
async recover(params: RecoverOptions): Promise<RecoveryInfo | OfflineVaultTxInfo> {
if (params.isTss === true) {
return this.recoverTSS(params, params.openSSLBytes);
return this.recoverTSS(params);
}
return this.recoverEthLike(params);
}
Expand Down Expand Up @@ -1939,10 +1839,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
* Recovers a tx with TSS key shares
* same expected arguments as recover method, but with TSS key shares
*/
protected async recoverTSS(
params: RecoverOptions,
openSSLBytes: Uint8Array
): Promise<RecoveryInfo | OfflineVaultTxInfo> {
protected async recoverTSS(params: RecoverOptions): Promise<RecoveryInfo | OfflineVaultTxInfo> {
this.validateRecoveryParams(params);
// Clean up whitespace from entered values
const userPublicOrPrivateKeyShare = params.userKey.replace(/\s/g, '');
Expand Down Expand Up @@ -1979,43 +1876,67 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
userPublicOrPrivateKeyShare,
params.walletPassphrase
);
let signature: ECDSAMethodTypes.Signature;
let unsignedTx: EthLikeTxLib.Transaction | EthLikeTxLib.FeeMarketEIP1559Transaction;
/**
* MPCv2 Signing Params
*/
let userKeyShare: Buffer;
let backupKeyShare: Buffer;
let commonKeyChain: string;

// Prepare the key shares for signing
if (isGG18SigningMaterial) {
// Retrofit the GG18 keys to DKLS
const [userKeyCombined, backupKeyCombined] = this.getKeyCombinedFromTssKeyShares(
userPublicOrPrivateKeyShare,
backupPrivateOrPublicKeyShare,
params.walletPassphrase
);
const backupKeyPair = new KeyPairLib({ pub: backupKeyCombined.xShare.y });
const baseAddress = backupKeyPair.getAddress();

unsignedTx = (await this.buildTssRecoveryTxn(baseAddress, gasPrice, gasLimit, params)).tx;
const aKeyCombine = {
xShare: userKeyCombined.xShare,
};
const bKeyCombine = {
xShare: backupKeyCombined.xShare,
};
const retrofitDataA: DklsTypes.RetrofitData = {
xShare: aKeyCombine.xShare,
};
const retrofitDataB: DklsTypes.RetrofitData = {
xShare: bKeyCombine.xShare,
};
const [user, backup] = await DklsUtils.generate2of2KeyShares(retrofitDataA, retrofitDataB);

signature = await this.signRecoveryTSS(
userKeyCombined,
backupKeyCombined,
unsignedTx.getMessageToSign(false).toString('hex'),
openSSLBytes
);
userKeyShare = user.getKeyShare();
backupKeyShare = backup.getKeyShare();
if (DklsTypes.getCommonKeychain(userKeyShare) !== DklsTypes.getCommonKeychain(backupKeyShare)) {
throw new Error('Common keychain mismatch! Ensure the correct user and backup keys where provided!');
}
commonKeyChain = DklsTypes.getCommonKeychain(userKeyShare);
} else {
const { userKeyShare, backupKeyShare, commonKeyChain } = await ECDSAUtils.getMpcV2RecoveryKeyShares(
// DKLS
const mpcv2KeyShares = await ECDSAUtils.getMpcV2RecoveryKeyShares(
userPublicOrPrivateKeyShare,
backupPrivateOrPublicKeyShare,
params.walletPassphrase
);

const MPC = new Ecdsa();
const derivedCommonKeyChain = MPC.deriveUnhardened(commonKeyChain, 'm');
const backupKeyPair = new KeyPairLib({ pub: derivedCommonKeyChain.slice(0, 66) });
const baseAddress = backupKeyPair.getAddress();
userKeyShare = mpcv2KeyShares.userKeyShare;
backupKeyShare = mpcv2KeyShares.backupKeyShare;
commonKeyChain = mpcv2KeyShares.commonKeyChain;

unsignedTx = (await this.buildTssRecoveryTxn(baseAddress, gasPrice, gasLimit, params)).tx;
if (!userKeyShare || !backupKeyShare || !commonKeyChain) {
throw new Error('Missing combined key shares for user or backup or common');
}
}

const messageHash = unsignedTx.getMessageToSign(true);
const MPC = new Ecdsa();
const derivedCommonKeyChain = MPC.deriveUnhardened(commonKeyChain, 'm/0');
const keyPair = new KeyPairLib({ pub: derivedCommonKeyChain.slice(0, 66) });
const baseAddress = keyPair.getAddress();

signature = await ECDSAUtils.signRecoveryMpcV2(messageHash, userKeyShare, backupKeyShare, commonKeyChain);
}
const unsignedTx = (await this.buildTssRecoveryTxn(baseAddress, gasPrice, gasLimit, params)).tx;
const messageHash = unsignedTx.getMessageToSign(true);
const signature = await ECDSAUtils.signRecoveryMpcV2(messageHash, userKeyShare, backupKeyShare, commonKeyChain);

const ethCommmon = AbstractEthLikeNewCoins.getEthLikeCommon(params.eip1559, params.replayProtectionOptions);
const signedTx = this.getSignedTxFromSignature(ethCommmon, unsignedTx, signature);
Expand Down
2 changes: 0 additions & 2 deletions modules/bitgo/test/v2/fixtures/tss/recoveryFixtures.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
export const ethLikeGG18Keycard = {
senderAddress: '0xf3243334a491b6558f3f2c791afecc6a5eb955fa',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hardcoding these addresses hid a bug in the actual recover code.

destinationAddress: '0xac05da78464520aa7c9d4c19bd7a440b111b3054',
userKey:
'{"iv":"bn1gTo9+ycrgiqtm0fkHlg==","v":1,"iter":10000,"ks":256,"ts":64,"mode"\n' +
Expand Down Expand Up @@ -159,7 +158,6 @@ export const ethLikeGG18Keycard = {
walletPassphrase: 'Ghghjkg!455544llll',
};
export const ethLikeDKLSKeycard = {
senderAddress: '0x88c2ab227908d39f6afdb85203dca3e937bb77af',
destinationAddress: '0xac05da78464520aa7c9d4c19bd7a440b111b3054',
userKey:
'{"iv":"82EY6GQ62/d9EzrkcIcqfA==","v":1,"iter":10000,"ks":256,"ts":64,"mode"\n' +
Expand Down
Loading
Loading