From 5e60ea971845fd547c5679ab4ee75a41fa557753 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Mon, 18 Aug 2025 06:39:39 -0400 Subject: [PATCH] Don't offset nonces yet negate to achieve an even Y coordinate Replaces an iterative loop with an immediate result, if action is necessary. --- Cargo.lock | 1 + crypto/frost/src/algorithm.rs | 2 ++ networks/bitcoin/Cargo.toml | 2 ++ networks/bitcoin/src/crypto.rs | 36 +++++++++++----------------- networks/bitcoin/src/tests/crypto.rs | 7 +++--- networks/bitcoin/src/wallet/mod.rs | 17 +++++++------ networks/bitcoin/tests/wallet.rs | 2 +- 7 files changed, 33 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 92273d03..c10e41fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1075,6 +1075,7 @@ dependencies = [ "serde_json", "simple-request", "std-shims", + "subtle", "thiserror 1.0.64", "tokio", "zeroize", diff --git a/crypto/frost/src/algorithm.rs b/crypto/frost/src/algorithm.rs index b595e03b..7e4d6167 100644 --- a/crypto/frost/src/algorithm.rs +++ b/crypto/frost/src/algorithm.rs @@ -135,6 +135,8 @@ pub trait Hram: Send + Sync + Clone { } /// Schnorr signature algorithm ((R, s) where s = r + cx). +/// +/// `verify`, `verify_share` must be called after `sign_share` is called. #[derive(Clone)] pub struct Schnorr> { transcript: T, diff --git a/networks/bitcoin/Cargo.toml b/networks/bitcoin/Cargo.toml index 7ab21eae..338237b2 100644 --- a/networks/bitcoin/Cargo.toml +++ b/networks/bitcoin/Cargo.toml @@ -20,6 +20,7 @@ std-shims = { version = "0.1.1", path = "../../common/std-shims", default-featur thiserror = { version = "1", default-features = false, optional = true } +subtle = { version = "2", default-features = false } zeroize = { version = "^1.5", default-features = false } rand_core = { version = "0.6", default-features = false } @@ -46,6 +47,7 @@ std = [ "thiserror", + "subtle/std", "zeroize/std", "rand_core/std", diff --git a/networks/bitcoin/src/crypto.rs b/networks/bitcoin/src/crypto.rs index 11510909..12aa2c1e 100644 --- a/networks/bitcoin/src/crypto.rs +++ b/networks/bitcoin/src/crypto.rs @@ -1,3 +1,5 @@ +use subtle::{Choice, ConstantTimeEq, ConditionallySelectable}; + use k256::{ elliptic_curve::sec1::{Tag, ToEncodedPoint}, ProjectivePoint, @@ -17,17 +19,9 @@ pub fn x_only(key: &ProjectivePoint) -> XOnlyPublicKey { XOnlyPublicKey::from_slice(&x(key)).expect("x_only was passed a point which was infinity or odd") } -/// Make a point even by adding the generator until it is even. -/// -/// Returns the even point and the amount of additions required. -#[cfg(any(feature = "std", feature = "hazmat"))] -pub fn make_even(mut key: ProjectivePoint) -> (ProjectivePoint, u64) { - let mut c = 0; - while key.to_encoded_point(true).tag() == Tag::CompressedOddY { - key += ProjectivePoint::GENERATOR; - c += 1; - } - (key, c) +/// Return if a point must be negated to have an even Y coordinate and be eligible for use. +pub(crate) fn needs_negation(key: &ProjectivePoint) -> Choice { + u8::from(key.to_encoded_point(true).tag()).ct_eq(&u8::from(Tag::CompressedOddY)) } #[cfg(feature = "std")] @@ -60,25 +54,27 @@ mod frost_crypto { #[allow(non_snake_case)] impl HramTrait for Hram { fn hram(R: &ProjectivePoint, A: &ProjectivePoint, m: &[u8]) -> Scalar { - // Convert the nonce to be even - let (R, _) = make_even(*R); - const TAG_HASH: Sha256 = Sha256::const_hash(b"BIP0340/challenge"); let mut data = Sha256::engine(); data.input(TAG_HASH.as_ref()); data.input(TAG_HASH.as_ref()); - data.input(&x(&R)); + data.input(&x(R)); data.input(&x(A)); data.input(m); - Scalar::reduce(U256::from_be_slice(Sha256::from_engine(data).as_ref())) + let c = Scalar::reduce(U256::from_be_slice(Sha256::from_engine(data).as_ref())); + // If the nonce was odd, sign `r - cx` instead of `r + cx`, allowing us to negate `s` at the + // end to sign as `-r + cx` + <_>::conditional_select(&c, &-c, needs_negation(R)) } } /// BIP-340 Schnorr signature algorithm. /// - /// This must be used with a ThresholdKeys whose group key is even. If it is odd, this will panic. + /// This must be used with a ThresholdKeys whose group key is even. If it is odd, this may panic. + /// + /// `verify`, `verify_share` must be called after `sign_share` is called. #[derive(Clone)] pub struct Schnorr(FrostSchnorr); impl Schnorr { @@ -141,11 +137,7 @@ mod frost_crypto { sum: Scalar, ) -> Option { self.0.verify(group_key, nonces, sum).map(|mut sig| { - // Make the R of the final signature even - let offset; - (sig.R, offset) = make_even(sig.R); - // s = r + cx. Since we added to the r, add to s - sig.s += Scalar::from(offset); + sig.s = <_>::conditional_select(&sum, &-sum, needs_negation(&sig.R)); // Convert to a Bitcoin signature by dropping the byte for the point's sign bit sig.serialize()[1 ..].try_into().unwrap() }) diff --git a/networks/bitcoin/src/tests/crypto.rs b/networks/bitcoin/src/tests/crypto.rs index 57a0eb3e..cd2ce298 100644 --- a/networks/bitcoin/src/tests/crypto.rs +++ b/networks/bitcoin/src/tests/crypto.rs @@ -2,7 +2,6 @@ use rand_core::OsRng; use secp256k1::{Secp256k1 as BContext, Message, schnorr::Signature}; -use k256::Scalar; use frost::{ curve::Secp256k1, Participant, @@ -11,7 +10,8 @@ use frost::{ use crate::{ bitcoin::hashes::{Hash as HashTrait, sha256::Hash}, - crypto::{x_only, make_even, Schnorr}, + crypto::{x_only, Schnorr}, + wallet::tweak_keys, }; #[test] @@ -20,8 +20,7 @@ fn test_algorithm() { const MESSAGE: &[u8] = b"Hello, World!"; for keys in keys.values_mut() { - let (_, offset) = make_even(keys.group_key()); - *keys = keys.offset(Scalar::from(offset)); + *keys = tweak_keys(keys.clone()); } let algo = Schnorr::new(); diff --git a/networks/bitcoin/src/wallet/mod.rs b/networks/bitcoin/src/wallet/mod.rs index 1a078958..7e985db0 100644 --- a/networks/bitcoin/src/wallet/mod.rs +++ b/networks/bitcoin/src/wallet/mod.rs @@ -26,7 +26,7 @@ use bitcoin::{hashes::Hash, consensus::encode::Decodable, TapTweakHash}; use crate::crypto::x_only; #[cfg(feature = "std")] -use crate::crypto::make_even; +use crate::crypto::needs_negation; #[cfg(feature = "std")] mod send; @@ -43,7 +43,7 @@ pub use send::*; /// existence of the unspendable script path may not provable, without an understanding of the /// algorithm used here. #[cfg(feature = "std")] -pub fn tweak_keys(keys: &ThresholdKeys) -> ThresholdKeys { +pub fn tweak_keys(keys: ThresholdKeys) -> ThresholdKeys { // Adds the unspendable script path per // https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#cite_note-23 let keys = { @@ -64,11 +64,14 @@ pub fn tweak_keys(keys: &ThresholdKeys) -> ThresholdKeys { ))) }; - // This doesn't risk re-introducing a script path as you'd have to find a preimage for the tweak - // hash with whatever increment, or manipulate the key so that the tweak hash and increment - // equals the desired offset, yet manipulating the key would change the tweak hash - let (_, offset) = make_even(keys.group_key()); - keys.offset(Scalar::from(offset)) + let needs_negation = needs_negation(&keys.group_key()); + keys + .scale(<_ as subtle::ConditionallySelectable>::conditional_select( + &Scalar::ONE, + &-Scalar::ONE, + needs_negation, + )) + .expect("scaling keys by 1 or -1 yet interpreted as 0?") } /// Return the Taproot address payload for a public key. diff --git a/networks/bitcoin/tests/wallet.rs b/networks/bitcoin/tests/wallet.rs index 45371414..83344048 100644 --- a/networks/bitcoin/tests/wallet.rs +++ b/networks/bitcoin/tests/wallet.rs @@ -80,7 +80,7 @@ async fn send_and_get_output(rpc: &Rpc, scanner: &Scanner, key: ProjectivePoint) fn keys() -> (HashMap>, ProjectivePoint) { let mut keys = key_gen(&mut OsRng); for keys in keys.values_mut() { - *keys = tweak_keys(keys); + *keys = tweak_keys(keys.clone()); } let key = keys.values().next().unwrap().group_key(); (keys, key)