From 236ca19b6168b6abcf18171e315d0d5b028db155 Mon Sep 17 00:00:00 2001 From: FedericoAmura Date: Fri, 27 Dec 2024 18:43:29 +0100 Subject: [PATCH 1/2] fix: consider low-s rule application when defining combined ecdsa signature v value --- packages/crypto/src/lib/crypto.ts | 12 ++++++-- .../src/lib/helpers/get-signatures.ts | 2 +- packages/types/src/lib/interfaces.ts | 4 +-- packages/wasm/rust/src/ecdsa.rs | 28 +++++++++++-------- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/packages/crypto/src/lib/crypto.ts b/packages/crypto/src/lib/crypto.ts index 8861a50893..cfaf439bcb 100644 --- a/packages/crypto/src/lib/crypto.ts +++ b/packages/crypto/src/lib/crypto.ts @@ -202,14 +202,20 @@ export const combineEcdsaShares = async ( Buffer.from(share.signatureShare, 'hex') ); - const [r, s, v] = await ecdsaCombine(variant!, presignature, signatureShares); + const [r, s, recId] = await ecdsaCombine( + variant!, + presignature, + signatureShares + ); const publicKey = Buffer.from(anyValidShare.publicKey, 'hex'); const messageHash = Buffer.from(anyValidShare.dataSigned!, 'hex'); - await ecdsaVerify(variant!, messageHash, publicKey, [r, s, v]); + await ecdsaVerify(variant!, messageHash, publicKey, [r, s, recId]); - const signature = splitSignature(Buffer.concat([r, s, Buffer.from([v])])); + const signature = splitSignature( + Buffer.concat([r, s, Buffer.from([recId + 27])]) + ); return { r: signature.r.slice('0x'.length), diff --git a/packages/lit-node-client-nodejs/src/lib/helpers/get-signatures.ts b/packages/lit-node-client-nodejs/src/lib/helpers/get-signatures.ts index b222523ae6..0e8e6ce004 100644 --- a/packages/lit-node-client-nodejs/src/lib/helpers/get-signatures.ts +++ b/packages/lit-node-client-nodejs/src/lib/helpers/get-signatures.ts @@ -259,7 +259,7 @@ export const getSignatures = async (params: { const encodedSig = joinSignature({ r: '0x' + signature.r, s: '0x' + signature.s, - v: signature.recid, + recoveryParam: signature.recid, }); signatures[key] = { diff --git a/packages/types/src/lib/interfaces.ts b/packages/types/src/lib/interfaces.ts index 24f4def9ef..c003e0a083 100644 --- a/packages/types/src/lib/interfaces.ts +++ b/packages/types/src/lib/interfaces.ts @@ -42,7 +42,7 @@ export interface AuthSig { /** * The signature produced by signing the `signMessage` property with the corresponding private key for the `address` property. */ - sig: any; + sig: string; /** * The method used to derive the signature (e.g, `web3.eth.personal.sign`). @@ -598,7 +598,7 @@ export interface SigResponse { r: string; s: string; recid: number; - signature: string; // 0x... + signature: `0x${string}`; publicKey: string; // pkp public key (no 0x prefix) dataSigned: string; } diff --git a/packages/wasm/rust/src/ecdsa.rs b/packages/wasm/rust/src/ecdsa.rs index 255131d271..d89df1b5fc 100644 --- a/packages/wasm/rust/src/ecdsa.rs +++ b/packages/wasm/rust/src/ecdsa.rs @@ -58,22 +58,22 @@ where presignature: Uint8Array, signature_shares: Vec, ) -> JsResult { - let (big_r, s) = Self::combine_inner(presignature, signature_shares)?; - Self::signature_into_js(big_r.to_affine(), s) + let (big_r, s, was_flipped) = Self::combine_inner(presignature, signature_shares)?; + Self::signature_into_js(big_r.to_affine(), s, was_flipped) } pub(crate) fn combine_inner( presignature: Uint8Array, signature_shares: Vec, - ) -> JsResult<(C::ProjectivePoint, C::Scalar)> { + ) -> JsResult<(C::ProjectivePoint, C::Scalar, bool)> { let signature_shares = signature_shares .into_iter() .map(Self::scalar_from_js) .collect::>>()?; let big_r: C::AffinePoint = Self::point_from_js(presignature)?; - let s = Self::sum_scalars(signature_shares)?; - Ok((C::ProjectivePoint::from(big_r), s)) + let (s, was_flipped) = Self::sum_scalars(signature_shares)?; + Ok((C::ProjectivePoint::from(big_r), s, was_flipped)) } pub fn verify( @@ -108,13 +108,14 @@ where Ok(()) } - fn sum_scalars(values: Vec) -> JsResult { + fn sum_scalars(values: Vec) -> JsResult<(C::Scalar, bool)> { if values.is_empty() { return Err(JsError::new("no shares provided")); } let mut acc: C::Scalar = values.into_iter().sum(); + let acc_flipped = acc.is_high().into(); acc.conditional_assign(&(-acc), acc.is_high()); - Ok(acc) + Ok((acc, acc_flipped)) } pub fn derive_key(id: Uint8Array, public_keys: Vec) -> JsResult { @@ -168,10 +169,15 @@ where Ok((r, s, v)) } - fn signature_into_js(big_r: C::AffinePoint, s: C::Scalar) -> JsResult { + fn signature_into_js(big_r: C::AffinePoint, s: C::Scalar, was_flipped: bool) -> JsResult { let r = Self::x_coordinate(&big_r).to_repr(); let s = s.to_repr(); - let v = u8::conditional_select(&0, &1, big_r.y_is_odd()); + let mut v = u8::conditional_select(&0, &1, big_r.y_is_odd()); + + // Flip v if s was normalized (flipped, low-s rule) + if was_flipped { + v = 1 - v; + } Ok(EcdsaSignature { obj: into_js(&(Bytes::new(&r), Bytes::new(&s), v))?, @@ -222,7 +228,7 @@ where public_key: C::ProjectivePoint, ) -> JsResult { let z = Self::scalar_from_hash(message_hash)?; - let (big_r, s) = Self::combine_inner(pre_signature, signature_shares)?; + let (big_r, s, was_flipped) = Self::combine_inner(pre_signature, signature_shares)?; let r = Self::x_coordinate(&big_r.to_affine()); if z.is_zero().into() { @@ -241,7 +247,7 @@ where .is_identity() .into() { - Self::signature_into_js(big_r.to_affine(), s) + Self::signature_into_js(big_r.to_affine(), s, was_flipped) } else { Err(JsError::new("invalid signature")) } From 224fb3ef8aea601fc3137c8d706544fbb4b01fab Mon Sep 17 00:00:00 2001 From: FedericoAmura Date: Fri, 27 Dec 2024 19:30:54 +0100 Subject: [PATCH 2/2] feat: add tests over the recovered public key from pkpSign signature --- ...acityCreditsNFTToAnotherWalletToPkpSign.ts | 18 +++++++++++++++ ...WithUnspecifiedCapacityTokenIdToPkpSign.ts | 20 ++++++++++++++++ ...thSigWithUnspecifiedDelegateesToPkpSign.ts | 22 ++++++++++++++++++ .../tests/testUseEoaSessionSigsToPkpSign.ts | 22 ++++++++++++++++++ .../tests/testUsePkpSessionSigsToPkpSign.ts | 22 ++++++++++++++++++ ...ActionCodeGeneratedSessionSigsToPkpSign.ts | 22 ++++++++++++++++++ ...onIpfsCodeGeneratedSessionSigsToPkpSign.ts | 23 +++++++++++++++++++ 7 files changed, 149 insertions(+) diff --git a/local-tests/tests/testDelegatingCapacityCreditsNFTToAnotherWalletToPkpSign.ts b/local-tests/tests/testDelegatingCapacityCreditsNFTToAnotherWalletToPkpSign.ts index 44ddf4e64e..d5f67581b9 100644 --- a/local-tests/tests/testDelegatingCapacityCreditsNFTToAnotherWalletToPkpSign.ts +++ b/local-tests/tests/testDelegatingCapacityCreditsNFTToAnotherWalletToPkpSign.ts @@ -1,3 +1,5 @@ +import { ethers } from 'ethers'; + import { getEoaSessionSigsWithCapacityDelegations } from 'local-tests/setup/session-sigs/get-eoa-session-sigs'; import { TinnyEnvironment } from 'local-tests/setup/tinny-environment'; @@ -92,5 +94,21 @@ export const testDelegatingCapacityCreditsNFTToAnotherWalletToPkpSign = async ( throw new Error(`Expected "recid" to be parseable as a number`); } + const signature = ethers.utils.joinSignature({ + r: '0x' + res.r, + s: '0x' + res.s, + recoveryParam: res.recid, + }); + const recoveredPubKey = ethers.utils.recoverPublicKey( + alice.loveLetter, + signature + ); + if (recoveredPubKey !== `0x${res.publicKey.toLowerCase()}`) { + throw new Error(`Expected recovered public key to match res.publicKey`); + } + if (recoveredPubKey !== `0x${bob.pkp.publicKey.toLowerCase()}`) { + throw new Error(`Expected recovered public key to match bob.pkp.publicKey`); + } + console.log('✅ res:', res); }; diff --git a/local-tests/tests/testUseCapacityDelegationAuthSigWithUnspecifiedCapacityTokenIdToPkpSign.ts b/local-tests/tests/testUseCapacityDelegationAuthSigWithUnspecifiedCapacityTokenIdToPkpSign.ts index 8daefbec5b..e8ad746c2a 100644 --- a/local-tests/tests/testUseCapacityDelegationAuthSigWithUnspecifiedCapacityTokenIdToPkpSign.ts +++ b/local-tests/tests/testUseCapacityDelegationAuthSigWithUnspecifiedCapacityTokenIdToPkpSign.ts @@ -1,3 +1,5 @@ +import { ethers } from 'ethers'; + import { getEoaSessionSigsWithCapacityDelegations } from 'local-tests/setup/session-sigs/get-eoa-session-sigs'; import { TinnyEnvironment } from 'local-tests/setup/tinny-environment'; @@ -94,5 +96,23 @@ export const testUseCapacityDelegationAuthSigWithUnspecifiedCapacityTokenIdToPkp throw new Error(`Expected "recid" to be parseable as a number`); } + const signature = ethers.utils.joinSignature({ + r: '0x' + res.r, + s: '0x' + res.s, + recoveryParam: res.recid, + }); + const recoveredPubKey = ethers.utils.recoverPublicKey( + alice.loveLetter, + signature + ); + if (recoveredPubKey !== `0x${res.publicKey.toLowerCase()}`) { + throw new Error(`Expected recovered public key to match res.publicKey`); + } + if (recoveredPubKey !== `0x${bob.pkp.publicKey.toLowerCase()}`) { + throw new Error( + `Expected recovered public key to match bob.pkp.publicKey` + ); + } + console.log('✅ res:', res); }; diff --git a/local-tests/tests/testUseCapacityDelegationAuthSigWithUnspecifiedDelegateesToPkpSign.ts b/local-tests/tests/testUseCapacityDelegationAuthSigWithUnspecifiedDelegateesToPkpSign.ts index e479d9c3ab..ba68e4258a 100644 --- a/local-tests/tests/testUseCapacityDelegationAuthSigWithUnspecifiedDelegateesToPkpSign.ts +++ b/local-tests/tests/testUseCapacityDelegationAuthSigWithUnspecifiedDelegateesToPkpSign.ts @@ -1,3 +1,5 @@ +import { ethers } from 'ethers'; + import { getEoaSessionSigsWithCapacityDelegations } from 'local-tests/setup/session-sigs/get-eoa-session-sigs'; import { TinnyEnvironment } from 'local-tests/setup/tinny-environment'; @@ -86,6 +88,26 @@ export const testUseCapacityDelegationAuthSigWithUnspecifiedDelegateesToPkpSign throw new Error(`Expected "signature" to start with 0x`); } + const signature = ethers.utils.joinSignature({ + r: '0x' + runWithSessionSigs.r, + s: '0x' + runWithSessionSigs.s, + recoveryParam: runWithSessionSigs.recid, + }); + const recoveredPubKey = ethers.utils.recoverPublicKey( + alice.loveLetter, + signature + ); + if (recoveredPubKey !== `0x${runWithSessionSigs.publicKey.toLowerCase()}`) { + throw new Error( + `Expected recovered public key to match runWithSessionSigs.publicKey` + ); + } + if (recoveredPubKey !== `0x${bob.pkp.publicKey.toLowerCase()}`) { + throw new Error( + `Expected recovered public key to match bob.pkp.publicKey` + ); + } + // recid must be parseable as a number if (isNaN(runWithSessionSigs.recid)) { throw new Error(`Expected "recid" to be parseable as a number`); diff --git a/local-tests/tests/testUseEoaSessionSigsToPkpSign.ts b/local-tests/tests/testUseEoaSessionSigsToPkpSign.ts index 1b217f3e54..56566bdd9f 100644 --- a/local-tests/tests/testUseEoaSessionSigsToPkpSign.ts +++ b/local-tests/tests/testUseEoaSessionSigsToPkpSign.ts @@ -1,3 +1,5 @@ +import { ethers } from 'ethers'; + import { log } from '@lit-protocol/misc'; import { getEoaSessionSigs } from 'local-tests/setup/session-sigs/get-eoa-session-sigs'; import { TinnyEnvironment } from 'local-tests/setup/tinny-environment'; @@ -58,5 +60,25 @@ export const testUseEoaSessionSigsToPkpSign = async ( throw new Error(`Expected "recid" to be parseable as a number`); } + const signature = ethers.utils.joinSignature({ + r: '0x' + runWithSessionSigs.r, + s: '0x' + runWithSessionSigs.s, + recoveryParam: runWithSessionSigs.recid, + }); + const recoveredPubKey = ethers.utils.recoverPublicKey( + alice.loveLetter, + signature + ); + if (recoveredPubKey !== `0x${runWithSessionSigs.publicKey.toLowerCase()}`) { + throw new Error( + `Expected recovered public key to match runWithSessionSigs.publicKey` + ); + } + if (recoveredPubKey !== `0x${alice.pkp.publicKey.toLowerCase()}`) { + throw new Error( + `Expected recovered public key to match alice.pkp.publicKey` + ); + } + log('✅ testUseEoaSessionSigsToPkpSign'); }; diff --git a/local-tests/tests/testUsePkpSessionSigsToPkpSign.ts b/local-tests/tests/testUsePkpSessionSigsToPkpSign.ts index 27986b20f1..0dc62eed78 100644 --- a/local-tests/tests/testUsePkpSessionSigsToPkpSign.ts +++ b/local-tests/tests/testUsePkpSessionSigsToPkpSign.ts @@ -1,3 +1,5 @@ +import { ethers } from 'ethers'; + import { log } from '@lit-protocol/misc'; import { getPkpSessionSigs } from 'local-tests/setup/session-sigs/get-pkp-session-sigs'; import { TinnyEnvironment } from 'local-tests/setup/tinny-environment'; @@ -58,5 +60,25 @@ export const testUsePkpSessionSigsToPkpSign = async ( throw new Error(`Expected "recid" to be parseable as a number`); } + const signature = ethers.utils.joinSignature({ + r: '0x' + res.r, + s: '0x' + res.s, + recoveryParam: res.recid, + }); + const recoveredPubKey = ethers.utils.recoverPublicKey( + alice.loveLetter, + signature + ); + if (recoveredPubKey !== `0x${res.publicKey.toLowerCase()}`) { + throw new Error(`Expected recovered public key to match res.publicKey`); + } + if ( + recoveredPubKey !== `0x${alice.authMethodOwnedPkp.publicKey.toLowerCase()}` + ) { + throw new Error( + `Expected recovered public key to match alice.authMethodOwnedPkp.publicKey` + ); + } + log('✅ res:', res); }; diff --git a/local-tests/tests/testUseValidLitActionCodeGeneratedSessionSigsToPkpSign.ts b/local-tests/tests/testUseValidLitActionCodeGeneratedSessionSigsToPkpSign.ts index 9ae59420e8..966736aa86 100644 --- a/local-tests/tests/testUseValidLitActionCodeGeneratedSessionSigsToPkpSign.ts +++ b/local-tests/tests/testUseValidLitActionCodeGeneratedSessionSigsToPkpSign.ts @@ -1,3 +1,5 @@ +import { ethers } from 'ethers'; + import { log } from '@lit-protocol/misc'; import { LIT_NETWORK } from '@lit-protocol/constants'; import { getLitActionSessionSigs } from 'local-tests/setup/session-sigs/get-lit-action-session-sigs'; @@ -58,5 +60,25 @@ export const testUseValidLitActionCodeGeneratedSessionSigsToPkpSign = async ( throw new Error(`Expected "recid" to be parseable as a number`); } + const signature = ethers.utils.joinSignature({ + r: '0x' + res.r, + s: '0x' + res.s, + recoveryParam: res.recid, + }); + const recoveredPubKey = ethers.utils.recoverPublicKey( + alice.loveLetter, + signature + ); + if (recoveredPubKey !== `0x${res.publicKey.toLowerCase()}`) { + throw new Error(`Expected recovered public key to match res.publicKey`); + } + if ( + recoveredPubKey !== `0x${alice.authMethodOwnedPkp.publicKey.toLowerCase()}` + ) { + throw new Error( + `Expected recovered public key to match alice.authMethodOwnedPkp.publicKey` + ); + } + log('✅ res:', res); }; diff --git a/local-tests/tests/testUseValidLitActionIpfsCodeGeneratedSessionSigsToPkpSign.ts b/local-tests/tests/testUseValidLitActionIpfsCodeGeneratedSessionSigsToPkpSign.ts index ab59dd5f0d..779bf69300 100644 --- a/local-tests/tests/testUseValidLitActionIpfsCodeGeneratedSessionSigsToPkpSign.ts +++ b/local-tests/tests/testUseValidLitActionIpfsCodeGeneratedSessionSigsToPkpSign.ts @@ -1,3 +1,5 @@ +import { ethers } from 'ethers'; + import { log } from '@lit-protocol/misc'; import { getLitActionSessionSigsUsingIpfsId } from 'local-tests/setup/session-sigs/get-lit-action-session-sigs'; import { TinnyEnvironment } from 'local-tests/setup/tinny-environment'; @@ -66,5 +68,26 @@ export const testUseValidLitActionIpfsCodeGeneratedSessionSigsToPkpSign = throw new Error(`Expected "recid" to be parseable as a number`); } + const signature = ethers.utils.joinSignature({ + r: '0x' + res.r, + s: '0x' + res.s, + recoveryParam: res.recid, + }); + const recoveredPubKey = ethers.utils.recoverPublicKey( + alice.loveLetter, + signature + ); + if (recoveredPubKey !== `0x${res.publicKey.toLowerCase()}`) { + throw new Error(`Expected recovered public key to match res.publicKey`); + } + if ( + recoveredPubKey !== + `0x${alice.authMethodOwnedPkp.publicKey.toLowerCase()}` + ) { + throw new Error( + `Expected recovered public key to match alice.authMethodOwnedPkp.publicKey` + ); + } + log('✅ res:', res); };