Skip to content

Commit 2bbadb5

Browse files
xolokixoloki
committed
clean up schnorr::ID to use standard variable names for a schnorr proof
delete sensitive values in the signer after dkg and signing clear sensitive data from coordinators add helper functions to zero out schnorr::ID and PolyCommitment; zero out dkg_public_shares and keep after reset in dkg_ended add helper functions to check if a schnorr::ID and PolyCommitment has been zeroed out; add test to see if sensitive data has been cleared add missing tests to frost coordinator fix comment on PolyCommitment::is_zero use values fn to iterate over map values
1 parent a03bfd8 commit 2bbadb5

7 files changed

Lines changed: 145 additions & 30 deletions

File tree

‎src/common.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ impl PolyCommitment {
3737
pub fn verify(&self) -> bool {
3838
self.id.verify(&self.poly[0])
3939
}
40+
41+
/// Zero out the schnorr::ID
42+
pub fn zero(&mut self) {
43+
self.id.zero();
44+
}
45+
46+
/// Check if schnorr proof is zeroed out
47+
pub fn is_zero(&self) -> bool {
48+
self.id.is_zero()
49+
}
4050
}
4151

4252
impl Display for PolyCommitment {

‎src/schnorr.rs‎

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use num_traits::Zero;
12
use rand_core::{CryptoRng, RngCore};
23
use serde::{Deserialize, Serialize};
34
use sha2::{Digest, Sha256};
@@ -12,46 +13,56 @@ use crate::{
1213

1314
#[allow(non_snake_case)]
1415
#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
15-
/// ID type which encapsulates the ID and a schnorr proof of ownership of the polynomial
16+
/// Encapsulatetion of the ID and a zero knowledge proof of ownership of a private key bound to ID
1617
pub struct ID {
17-
/// The ID
18+
/// ID
1819
pub id: Scalar,
19-
/// The public schnorr response
20-
pub kG: Point,
21-
/// The aggregate of the schnorr committed values
22-
pub kca: Scalar,
20+
/// Commitment to the proof random value
21+
pub R: Point,
22+
/// Sigma protocol response
23+
pub s: Scalar,
2324
}
2425

2526
#[allow(non_snake_case)]
2627
impl ID {
27-
/// Construct a new schnorr ID which binds the passed `Scalar` `id` and `Scalar` `a`, with a zero-knowledge proof of ownership of `a`
28-
pub fn new<RNG: RngCore + CryptoRng>(id: &Scalar, a: &Scalar, rng: &mut RNG) -> Self {
29-
let k = Scalar::random(rng);
30-
let c = Self::challenge(id, &(&k * &G), &(a * &G));
31-
32-
Self {
33-
id: *id,
34-
kG: &k * G,
35-
kca: &k + c * a,
36-
}
28+
/// Construct a new schnorr ID that proves ownership of private key `x` bound to `id`
29+
pub fn new<RNG: RngCore + CryptoRng>(id: &Scalar, x: &Scalar, rng: &mut RNG) -> Self {
30+
let r = Scalar::random(rng);
31+
let R = r * G;
32+
let X = x * G;
33+
let c = Self::challenge(id, &R, &X);
34+
let s = r + c * x;
35+
36+
Self { id: *id, R, s }
3737
}
3838

3939
/// Compute the schnorr challenge
40-
pub fn challenge(id: &Scalar, K: &Point, A: &Point) -> Scalar {
40+
pub fn challenge(id: &Scalar, R: &Point, X: &Point) -> Scalar {
4141
let mut hasher = Sha256::new();
4242
let tag = "WSTS/polynomial-constant";
4343

4444
hasher.update(tag.as_bytes());
4545
hasher.update(id.to_bytes());
46-
hasher.update(K.compress().as_bytes());
47-
hasher.update(A.compress().as_bytes());
46+
hasher.update(R.compress().as_bytes());
47+
hasher.update(X.compress().as_bytes());
4848

4949
hash_to_scalar(&mut hasher)
5050
}
5151

52-
/// Verify the proof
53-
pub fn verify(&self, A: &Point) -> bool {
54-
let c = Self::challenge(&self.id, &self.kG, A);
55-
&self.kca * &G == &self.kG + c * A
52+
/// Verify the proof against the public key `X`
53+
pub fn verify(&self, X: &Point) -> bool {
54+
let c = Self::challenge(&self.id, &self.R, X);
55+
&self.s * &G == &self.R + c * X
56+
}
57+
58+
/// Zero out the schnorr proof
59+
pub fn zero(&mut self) {
60+
self.R = Point::new();
61+
self.s = Scalar::zero();
62+
}
63+
64+
/// Check if schnorr proof is zeroed out
65+
pub fn is_zero(&self) -> bool {
66+
self.R == Point::new() && self.s == Scalar::zero()
5667
}
5768
}

‎src/state_machine/coordinator/fire.rs‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -737,7 +737,9 @@ impl<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
737737
// Cache the polynomials used in DKG for the aggregator
738738
for signer_id in self.dkg_private_shares.keys() {
739739
for (party_id, comm) in &self.dkg_public_shares[signer_id].comms {
740-
self.party_polynomials.insert(*party_id, comm.clone());
740+
let mut comm = comm.clone();
741+
comm.zero();
742+
self.party_polynomials.insert(*party_id, comm);
741743
}
742744
}
743745

@@ -749,6 +751,10 @@ impl<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
749751
.fold(Point::default(), |s, (_, comm)| s + comm.poly[0]);
750752

751753
info!("Aggregate public key: {key}");
754+
755+
self.dkg_public_shares.clear();
756+
self.dkg_private_shares.clear();
757+
752758
self.aggregate_public_key = Some(key);
753759
self.move_to(State::Idle)
754760
}
@@ -1087,6 +1093,7 @@ impl<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
10871093

10881094
self.move_to(State::Idle)?;
10891095
}
1096+
10901097
Ok(())
10911098
}
10921099

@@ -1403,7 +1410,8 @@ pub mod test {
14031410
bad_signature_share_request, check_signature_shares, coordinator_state_machine,
14041411
empty_private_shares, empty_public_shares, equal_after_save_load,
14051412
feedback_messages, feedback_mutated_messages, gen_nonces, invalid_nonce,
1406-
new_coordinator, run_dkg_sign, setup, setup_with_timeouts, start_dkg_round,
1413+
new_coordinator, run_dkg_sign, sensitive_data, setup, setup_with_timeouts,
1414+
start_dkg_round,
14071415
},
14081416
Config, Coordinator as CoordinatorTrait, State,
14091417
},
@@ -3144,6 +3152,16 @@ pub mod test {
31443152
gen_nonces::<FireCoordinator<v2::Aggregator>, v2::Signer>(5, 1);
31453153
}
31463154

3155+
#[test]
3156+
fn sensitive_data_v1() {
3157+
sensitive_data::<FireCoordinator<v1::Aggregator>, v1::Signer>(5, 1);
3158+
}
3159+
3160+
#[test]
3161+
fn sensitive_data_v2() {
3162+
sensitive_data::<FireCoordinator<v2::Aggregator>, v2::Signer>(5, 1);
3163+
}
3164+
31473165
#[test]
31483166
fn bad_signature_share_request_v1() {
31493167
bad_signature_share_request::<FireCoordinator<v1::Aggregator>, v1::Signer>(5, 2);

‎src/state_machine/coordinator/frost.rs‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,9 @@ impl<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
414414
return Err(Error::BadStateChange(format!("Should not have transitioned to DkgEndGather since we were missing DkgPublicShares from signer {signer_id}")));
415415
};
416416
for (party_id, comm) in &dkg_public_shares.comms {
417-
self.party_polynomials.insert(*party_id, comm.clone());
417+
let mut comm = comm.clone();
418+
comm.zero();
419+
self.party_polynomials.insert(*party_id, comm);
418420
}
419421
}
420422

@@ -428,6 +430,9 @@ impl<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
428430
%key,
429431
"Aggregate public key"
430432
);
433+
self.dkg_public_shares.clear();
434+
self.dkg_private_shares.clear();
435+
431436
self.aggregate_public_key = Some(key);
432437
self.move_to(State::Idle)
433438
}
@@ -982,8 +987,9 @@ pub mod test {
982987
frost::Coordinator as FrostCoordinator,
983988
test::{
984989
bad_signature_share_request, check_signature_shares, coordinator_state_machine,
985-
empty_private_shares, empty_public_shares, equal_after_save_load, invalid_nonce,
986-
new_coordinator, run_dkg_sign, setup, start_dkg_round,
990+
empty_private_shares, empty_public_shares, equal_after_save_load, gen_nonces,
991+
invalid_nonce, new_coordinator, run_dkg_sign, sensitive_data, setup,
992+
start_dkg_round,
987993
},
988994
Config, Coordinator as CoordinatorTrait, State,
989995
},
@@ -1401,6 +1407,26 @@ pub mod test {
14011407
);
14021408
}
14031409

1410+
#[test]
1411+
fn gen_nonces_v1() {
1412+
gen_nonces::<FrostCoordinator<v1::Aggregator>, v1::Signer>(5, 1);
1413+
}
1414+
1415+
#[test]
1416+
fn gen_nonces_v2() {
1417+
gen_nonces::<FrostCoordinator<v2::Aggregator>, v2::Signer>(5, 1);
1418+
}
1419+
1420+
#[test]
1421+
fn sensitive_data_v1() {
1422+
sensitive_data::<FrostCoordinator<v1::Aggregator>, v1::Signer>(5, 1);
1423+
}
1424+
1425+
#[test]
1426+
fn sensitive_data_v2() {
1427+
sensitive_data::<FrostCoordinator<v2::Aggregator>, v2::Signer>(5, 1);
1428+
}
1429+
14041430
#[test]
14051431
fn bad_signature_share_request_v1() {
14061432
bad_signature_share_request::<FrostCoordinator<v1::Aggregator>, v1::Signer>(5, 2);

‎src/state_machine/coordinator/mod.rs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,6 +1092,40 @@ pub mod test {
10921092
}
10931093
}
10941094

1095+
pub fn sensitive_data<Coordinator: CoordinatorTrait, SignerType: SignerTrait>(
1096+
num_signers: u32,
1097+
keys_per_signer: u32,
1098+
) {
1099+
let (coordinators, signers) =
1100+
run_dkg::<Coordinator, SignerType>(num_signers, keys_per_signer);
1101+
1102+
// check the coordinators to see if they deleted public and private shares
1103+
for coordinator in &coordinators {
1104+
let state = coordinator.save();
1105+
1106+
assert!(state.dkg_public_shares.is_empty());
1107+
assert!(state.dkg_private_shares.is_empty());
1108+
1109+
for (_party_id, comm) in &state.party_polynomials {
1110+
assert!(comm.is_zero());
1111+
}
1112+
}
1113+
1114+
// check the signers to see if they deleted private shares and zeroed out poly commitments
1115+
for signer in &signers {
1116+
assert!(signer.dkg_private_shares.is_empty());
1117+
1118+
for (_party_id, comm) in &signer.commitments {
1119+
assert!(comm.is_zero());
1120+
}
1121+
1122+
for dkg_public_shares in signer.dkg_public_shares.values() {
1123+
for (_id, comm) in &dkg_public_shares.comms {
1124+
assert!(comm.is_zero());
1125+
}
1126+
}
1127+
}
1128+
}
10951129
pub fn bad_signature_share_request<Coordinator: CoordinatorTrait, SignerType: SignerTrait>(
10961130
num_signers: u32,
10971131
keys_per_signer: u32,

‎src/state_machine/signer/mod.rs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ impl<SignerType: SignerTrait> Signer<SignerType> {
381381
self.invalid_private_shares.clear();
382382
self.public_nonces.clear();
383383
self.signer.reset_polys(rng);
384+
self.signer.gen_nonces(rng);
384385
self.dkg_public_shares.clear();
385386
self.dkg_private_shares.clear();
386387
self.dkg_private_begin_msg = None;
@@ -452,6 +453,7 @@ impl<SignerType: SignerTrait> Signer<SignerType> {
452453
/// DKG is done so compute secrets
453454
pub fn dkg_ended<R: RngCore + CryptoRng>(&mut self, rng: &mut R) -> Result<Message, Error> {
454455
if !self.can_dkg_end() {
456+
self.reset(self.dkg_id, rng);
455457
return Ok(Message::DkgEnd(DkgEnd {
456458
dkg_id: self.dkg_id,
457459
signer_id: self.signer_id,
@@ -467,6 +469,7 @@ impl<SignerType: SignerTrait> Signer<SignerType> {
467469

468470
let Some(dkg_end_begin) = &self.dkg_end_begin_msg else {
469471
// no cached DkgEndBegin message
472+
self.reset(self.dkg_id, rng);
470473
return Ok(Message::DkgEnd(DkgEnd {
471474
dkg_id: self.dkg_id,
472475
signer_id: self.signer_id,
@@ -490,6 +493,7 @@ impl<SignerType: SignerTrait> Signer<SignerType> {
490493
}
491494

492495
if num_dkg_keys < self.dkg_threshold {
496+
self.reset(self.dkg_id, rng);
493497
return Ok(Message::DkgEnd(DkgEnd {
494498
dkg_id: self.dkg_id,
495499
signer_id: self.signer_id,
@@ -532,6 +536,7 @@ impl<SignerType: SignerTrait> Signer<SignerType> {
532536
}
533537

534538
if !missing_public_shares.is_empty() {
539+
self.reset(self.dkg_id, rng);
535540
return Ok(Message::DkgEnd(DkgEnd {
536541
dkg_id: self.dkg_id,
537542
signer_id: self.signer_id,
@@ -540,6 +545,7 @@ impl<SignerType: SignerTrait> Signer<SignerType> {
540545
}
541546

542547
if !bad_public_shares.is_empty() {
548+
self.reset(self.dkg_id, rng);
543549
return Ok(Message::DkgEnd(DkgEnd {
544550
dkg_id: self.dkg_id,
545551
signer_id: self.signer_id,
@@ -548,6 +554,7 @@ impl<SignerType: SignerTrait> Signer<SignerType> {
548554
}
549555

550556
if !missing_private_shares.is_empty() {
557+
self.reset(self.dkg_id, rng);
551558
return Ok(Message::DkgEnd(DkgEnd {
552559
dkg_id: self.dkg_id,
553560
signer_id: self.signer_id,
@@ -614,6 +621,14 @@ impl<SignerType: SignerTrait> Signer<SignerType> {
614621
"sending DkgEnd"
615622
);
616623

624+
for (_, dps) in self.dkg_public_shares.iter_mut() {
625+
for (_, comm) in dps.comms.iter_mut() {
626+
comm.zero();
627+
}
628+
}
629+
let dkg_public_shares = self.dkg_public_shares.clone();
630+
self.reset(self.dkg_id, rng);
631+
self.dkg_public_shares = dkg_public_shares;
617632
let dkg_end = Message::DkgEnd(dkg_end);
618633
Ok(dkg_end)
619634
}
@@ -765,7 +780,8 @@ impl<SignerType: SignerTrait> Signer<SignerType> {
765780
SignatureType::Frost => self.signer.sign(msg, &signer_ids, &key_ids, &nonces),
766781
};
767782

768-
self.signer.gen_nonces(rng);
783+
// delete sensitive values to prevent protocol attacks
784+
self.reset(self.dkg_id, rng);
769785

770786
let response = SignatureShareResponse {
771787
dkg_id: sign_request.dkg_id,

‎src/traits.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ pub mod test_helpers {
362362
if party_id == bad_party_id {
363363
// alter the schnorr proof so it will fail verification
364364
let mut bad_comm = comm.clone();
365-
bad_comm.id.kca += Scalar::from(1);
365+
bad_comm.id.s += Scalar::from(1);
366366
(party_id, bad_comm)
367367
} else {
368368
(party_id, comm)

0 commit comments

Comments
 (0)