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.

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

Fix: apply low-s s flips also to v on ecdsa signature shares combination #750

Merged
merged 2 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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);
};
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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);
};
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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`);
Expand Down
22 changes: 22 additions & 0 deletions local-tests/tests/testUseEoaSessionSigsToPkpSign.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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');
};
22 changes: 22 additions & 0 deletions local-tests/tests/testUsePkpSessionSigsToPkpSign.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
};
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
};
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
};
12 changes: 9 additions & 3 deletions packages/crypto/src/lib/crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ export const getSignatures = async <T>(params: {
const encodedSig = joinSignature({
r: '0x' + signature.r,
s: '0x' + signature.s,
v: signature.recid,
recoveryParam: signature.recid,
Ansonhkg marked this conversation as resolved.
Show resolved Hide resolved
});

signatures[key] = {
Expand Down
4 changes: 2 additions & 2 deletions packages/types/src/lib/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
Expand Down Expand Up @@ -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;
}
Expand Down
28 changes: 17 additions & 11 deletions packages/wasm/rust/src/ecdsa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,22 @@ where
presignature: Uint8Array,
signature_shares: Vec<Uint8Array>,
) -> JsResult<EcdsaSignature> {
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<Uint8Array>,
) -> JsResult<(C::ProjectivePoint, C::Scalar)> {
) -> JsResult<(C::ProjectivePoint, C::Scalar, bool)> {
let signature_shares = signature_shares
.into_iter()
.map(Self::scalar_from_js)
.collect::<JsResult<Vec<_>>>()?;

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(
Expand Down Expand Up @@ -108,13 +108,14 @@ where
Ok(())
}

fn sum_scalars(values: Vec<C::Scalar>) -> JsResult<C::Scalar> {
fn sum_scalars(values: Vec<C::Scalar>) -> 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<Uint8Array>) -> JsResult<Uint8Array> {
Expand Down Expand Up @@ -168,10 +169,15 @@ where
Ok((r, s, v))
}

fn signature_into_js(big_r: C::AffinePoint, s: C::Scalar) -> JsResult<EcdsaSignature> {
fn signature_into_js(big_r: C::AffinePoint, s: C::Scalar, was_flipped: bool) -> JsResult<EcdsaSignature> {
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))?,
Expand Down Expand Up @@ -222,7 +228,7 @@ where
public_key: C::ProjectivePoint,
) -> JsResult<EcdsaSignature> {
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() {
Expand All @@ -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"))
}
Expand Down
Loading