Skip to content

Commit

Permalink
[❄] Make signing actually (maybe) secure
Browse files Browse the repository at this point in the history
We weren't following any of the FROST papers in how to compute the
binding coefficient. To follow [1] all we have to do is hash the
parties involved.

[1]: https://eprint.iacr.org/2023/899
  • Loading branch information
LLFourn committed Aug 26, 2024
1 parent c478a4e commit fca38b5
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
21 changes: 15 additions & 6 deletions schnorr_fun/src/frost/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,15 @@
//!
//! The original scheme was introduced in *[FROST: Flexible Round-Optimized Schnorr Threshold
//! Signatures][FROST]*. A more satisfying security proof was provided in *[Security of Multi- and Threshold
//! Signatures]*.
//! Signatures]*. This implementation follows most closely *[Practical Schnorr Threshold Signatures Without the Algebraic Group Model]*.
//!
//! > ⚠ At this stage this implementation is for API exploration purposes only. The way it is
//! > currently implemented is not proven secure.
//! > ⚠ CAUTION ⚠: We *think* that this follows the scheme in the "Practical" paper which is proven secure but
//! > we haven't put a lot of effort into verifying this yet.
//!
//! [FROST]: <https://eprint.iacr.org/2020/852.pdf>
//! [secp256k1-zkp]: <https://github.com/ElementsProject/secp256k1-zkp/pull/138>
//! [Security of Multi- and Threshold Signatures]: <https://eprint.iacr.org/2021/1375.pdf>
//! [Practical Schnorr Threshold Signatures Without the Algebraic Group Model]: https://eprint.iacr.org/2023/899
//! [`musig`]: crate::musig
//! [`Scalar`]: crate::fun::Scalar

Expand Down Expand Up @@ -263,7 +264,7 @@ impl<H: Hash32, NG> Frost<H, NG> {
agg_binonce: binonce::Nonce<Zero>,
message: Message,
) -> PartySignSession {
let binding_coeff = self.binding_coefficient(public_key, agg_binonce, message);
let binding_coeff = self.binding_coefficient(public_key, agg_binonce, message, &parties);
let (final_nonce, binonce_needs_negation) = agg_binonce.bind(binding_coeff);
let challenge = self.schnorr.challenge(&final_nonce, &public_key, message);

Expand Down Expand Up @@ -297,7 +298,12 @@ impl<H: Hash32, NG> Frost<H, NG> {

let agg_binonce = binonce::Nonce::aggregate(nonces.values().cloned());

let binding_coeff = self.binding_coefficient(shared_key.public_key(), agg_binonce, message);
let binding_coeff = self.binding_coefficient(
shared_key.public_key(),
agg_binonce,
message,
&nonces.keys().cloned().collect(),
);
let (final_nonce, binonce_needs_negation) = agg_binonce.bind(binding_coeff);

let challenge = self
Expand All @@ -323,12 +329,15 @@ impl<H: Hash32, NG> Frost<H, NG> {
public_key: Point<EvenY>,
agg_binonce: Nonce<Zero>,
message: Message,
parties: &BTreeSet<PartyIndex>,
) -> Scalar<Public> {
Scalar::from_hash(
self.binding_hash
.clone()
.add(agg_binonce)
.add(public_key)
.add((parties.len() as u32).to_be_bytes())
.add(parties)
.add(agg_binonce)
.add(message),
)
.public()
Expand Down
9 changes: 9 additions & 0 deletions secp256kfun/src/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,15 @@ impl<K: HashInto, V: HashInto> HashInto for alloc::collections::BTreeMap<K, V> {
}
}

#[cfg(feature = "alloc")]
impl<T: HashInto + Ord> HashInto for alloc::collections::BTreeSet<T> {
fn hash_into(self, hash: &mut impl digest::Update) {
for item in self {
item.hash_into(hash)
}
}
}

/// Extension trait for [`digest::Update`] to make adding things to the hash convenient.
pub trait HashAdd {
/// Converts something that implements [`HashInto`] to bytes and then incorporate the result into the digest (`self`).
Expand Down

0 comments on commit fca38b5

Please sign in to comment.