diff --git a/crates/crypto/src/tbls.rs b/crates/crypto/src/tbls.rs index 700299cb..f09b1208 100644 --- a/crates/crypto/src/tbls.rs +++ b/crates/crypto/src/tbls.rs @@ -325,6 +325,9 @@ pub fn verify_aggregate( #[cfg(test)] mod tests { + use rand::{SeedableRng, rngs::StdRng}; + use test_case::test_case; + use super::*; use crate::types::PUBLIC_KEY_LENGTH; @@ -926,4 +929,243 @@ mod tests { Err(Error::InvalidPublicKey(BlsError::BadEncoding)) )); } + + /// An RNG that yields one byte value forever, so these assertions do not + /// depend on a particular `rand` version's stream. + struct ConstantRng(u8); + + impl RngCore for ConstantRng { + fn next_u32(&mut self) -> u32 { + u32::from_le_bytes([self.0; 4]) + } + + fn next_u64(&mut self) -> u64 { + u64::from_le_bytes([self.0; 8]) + } + + fn fill_bytes(&mut self, dest: &mut [u8]) { + dest.fill(self.0); + } + + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), rand_core::Error> { + self.fill_bytes(dest); + Ok(()) + } + } + + impl CryptoRng for ConstantRng {} + + /// Below the scalar-field order (which begins `0x73`) and non-zero, so it + /// is accepted on the first draw. + const VALID_DRAW: PrivateKey = [0x01; 32]; + + /// Above the scalar-field order, so it is always rejected. + const INVALID_DRAW: u8 = 0xff; + + #[test] + fn generate_insecure_secret_is_deterministic() { + let first = super::generate_insecure_secret(StdRng::from_seed([1u8; 32])).unwrap(); + let again = super::generate_insecure_secret(StdRng::from_seed([1u8; 32])).unwrap(); + let other = super::generate_insecure_secret(StdRng::from_seed([2u8; 32])).unwrap(); + + assert_eq!(first, again, "the same seed must give the same secret"); + assert_ne!(first, other, "different seeds must give different secrets"); + } + + // Unlike `generate_secret_key`, this performs no key derivation: it hands + // back the draw verbatim. That is why it is documented as insecure. + #[test] + fn generate_insecure_secret_returns_raw_rng_draw() { + // Asserted, so this cannot quietly become a test of the retry loop. + assert!( + BlstSecretKey::from_bytes(&VALID_DRAW).is_ok(), + "the fixture draw must already be a valid scalar" + ); + + let secret = super::generate_insecure_secret(ConstantRng(VALID_DRAW[0])).unwrap(); + + assert_eq!( + secret, VALID_DRAW, + "the returned secret must be the RNG draw itself" + ); + } + + // The retry loop is bounded at 100 attempts, so an RNG that can never + // produce a valid scalar must terminate rather than spin. + #[test] + fn generate_insecure_secret_exhausts_retry_budget() { + let Err(error) = super::generate_insecure_secret(ConstantRng(INVALID_DRAW)) else { + panic!("an RNG that never yields a valid scalar must exhaust the retry budget") + }; + + assert!( + matches!(error, Error::InvalidSecretKey(BlsError::KeyGeneration)), + "expected InvalidSecretKey(KeyGeneration)" + ); + } + + // `generate_secret_key` runs EIP-2333 `key_gen` over the RNG output, so + // the returned key is *not* the draw. + #[test] + fn generate_secret_key_derives_from_rng_draw() { + let derived = generate_secret_key(ConstantRng(VALID_DRAW[0])).unwrap(); + + // `VALID_DRAW` is itself a usable scalar, so returning it verbatim + // would have looked correct. + assert_ne!( + derived, VALID_DRAW, + "the key must be derived from the IKM, not equal to it" + ); + assert_eq!( + derived, + generate_secret_key(ConstantRng(VALID_DRAW[0])).unwrap(), + "derivation must be deterministic for fixed input key material" + ); + } + + // Three empty inputs, three different errors, all raised by this layer + // before it delegates. `tbls::math` reports `IndicesSharesMismatch` for the + // empty case — a different guard, a different variant. + #[test] + fn empty_inputs_reject_with_distinct_errors() { + assert!(matches!( + recover_secret(&HashMap::new()), + Err(Error::SharesAreEmpty) + )); + assert!(matches!( + threshold_aggregate(&HashMap::new()), + Err(Error::EmptySignatureArray) + )); + assert!(matches!( + verify_aggregate(&[], IDENTITY_SIGNATURE, b"data"), + Err(Error::EmptyPublicKeyArray) + )); + } + + // Aggregating fewer than `threshold` partials returns `Ok` with a + // well-formed 96-byte signature; only verification reveals it signs + // nothing. (`math.rs` pins the secret-side twin.) + #[test] + fn threshold_aggregate_below_threshold_does_not_verify() { + const MSG: &[u8] = b"sub-threshold aggregate"; + + let secret = generate_secret_key(rand::rngs::OsRng).unwrap(); + let public_key = secret_to_public_key(&secret).unwrap(); + let shares = threshold_split(&secret, 5, 3).unwrap(); + + // Fixed explicitly: `HashMap` order is not stable, so "the first two" + // would be flaky. + let mut partials = HashMap::new(); + for idx in [1u64, 2] { + let share = shares.get(&idx).expect("shares are 1-indexed over 1..=5"); + partials.insert(idx, sign(share, MSG).unwrap()); + } + + let aggregated = threshold_aggregate(&partials) + .expect("sub-threshold aggregation succeeds — that is the hazard"); + + assert_eq!( + aggregated.len(), + SIGNATURE_LENGTH, + "the result is indistinguishable from a real signature by shape" + ); + assert!( + matches!( + verify(&public_key, MSG, &aggregated), + Err(Error::VerificationFailed(_)) + ), + "only verification reveals that 2 of 3 was not enough" + ); + } + + // A *key* error, never a verification failure: the two lead a caller to + // opposite conclusions — "my input is broken" vs "this signer is lying". + #[test_case([0u8; PUBLIC_KEY_LENGTH] ; "all zero")] + #[test_case([0xff; PUBLIC_KEY_LENGTH] ; "all ones")] + fn verify_rejects_malformed_public_key(public_key: PublicKey) { + let result = verify(&public_key, b"data", &IDENTITY_SIGNATURE); + + assert!( + matches!(result, Err(Error::InvalidPublicKey(BlsError::BadEncoding))), + "expected InvalidPublicKey(BadEncoding)" + ); + } + + // Four functions parse signatures independently; only `aggregate` had + // coverage. Each remaining call site maps its own error, so a missing or + // mis-mapped `?` is otherwise invisible. + #[test] + fn all_entry_points_reject_malformed_signature() { + const MSG: &[u8] = b"malformed signature"; + const BAD: Signature = [0u8; SIGNATURE_LENGTH]; + + let secret = generate_secret_key(rand::rngs::OsRng).unwrap(); + let public_key = secret_to_public_key(&secret).unwrap(); + + let threshold = threshold_aggregate(&HashMap::from([(1u64, BAD)])); + assert!( + matches!(threshold, Err(Error::InvalidSignature(_))), + "threshold_aggregate: expected InvalidSignature" + ); + + let verified = verify(&public_key, MSG, &BAD); + assert!( + matches!(verified, Err(Error::InvalidSignature(_))), + "verify: expected InvalidSignature" + ); + + let verified_aggregate = verify_aggregate(&[public_key], BAD, MSG); + assert!( + matches!(verified_aggregate, Err(Error::InvalidSignature(_))), + "verify_aggregate: expected InvalidSignature" + ); + } + + // `aggregate(&[])` hands back the identity signature, which must fail + // rather than verify against anything. + #[test] + fn verify_rejects_identity_signature() { + let secret = generate_secret_key(rand::rngs::OsRng).unwrap(); + let public_key = secret_to_public_key(&secret).unwrap(); + + let result = verify(&public_key, b"data", &IDENTITY_SIGNATURE); + + assert!( + matches!(result, Err(Error::VerificationFailed(_))), + "the identity signature must not verify" + ); + } + + // Through `verify`, an off-subgroup key is indistinguishable from a bad + // signature: blst 0.3.17's `aggregate_verify` collapses every per-key + // failure into one flag and returns a flat `BLST_VERIFY_FAIL`. + // `verify_aggregate` calls `key_validate` first and reports + // `InvalidPublicKey`. Asserted together so the contrast cannot drift. + #[test] + fn verify_reports_off_subgroup_key_as_verify_failure() { + const MSG: &[u8] = b"off subgroup key"; + + // Genuine, so the outcome is attributable to the key alone. + let secret = generate_secret_key(rand::rngs::OsRng).unwrap(); + let signature = sign(&secret, MSG).unwrap(); + + let result = verify(&OFF_SUBGROUP_G1_POINT, MSG, &signature); + assert!( + matches!( + result, + Err(Error::VerificationFailed(BlsError::VerifyFailed)) + ), + "expected VerificationFailed(VerifyFailed)" + ); + + // The same 48 bytes, reported far more precisely one function over. + let aggregate_result = verify_aggregate(&[OFF_SUBGROUP_G1_POINT], signature, MSG); + assert!( + matches!( + aggregate_result, + Err(Error::InvalidPublicKey(BlsError::PointNotInGroup)) + ), + "verify_aggregate must name the real cause" + ); + } } diff --git a/crates/crypto/src/tbls/math.rs b/crates/crypto/src/tbls/math.rs index 036f0a36..e64172ce 100644 --- a/crates/crypto/src/tbls/math.rs +++ b/crates/crypto/src/tbls/math.rs @@ -289,7 +289,9 @@ fn scalar_div( #[cfg(test)] mod tests { - use super::*; + use test_case::test_case; + + use super::{super::ETH2_DST, *}; #[test] fn scalar_from_u64_upper_limbs_are_zero() { @@ -312,4 +314,247 @@ mod tests { ); } } + + /// The BLS12-381 scalar-field order minus 19, big-endian. Written from the + /// published curve order, not read off this implementation. + const R_MINUS_19: [u8; 32] = [ + 0x73, 0xed, 0xa7, 0x53, 0x29, 0x9d, 0x7d, 0x48, 0x33, 0x39, 0xd8, 0x08, 0x09, 0xa1, 0xd8, + 0x05, 0x53, 0xbd, 0xa4, 0x02, 0xff, 0xfe, 0x5b, 0xfe, 0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, + 0xff, 0xee, + ]; + + /// A secret key holding the non-zero field element `v`. + fn sk(v: u64) -> BlstSecretKey { + let scalar = scalar_from_u64(v); + let sk: &BlstSecretKey = (&scalar) + .try_into() + .expect("a small non-zero value is a valid BLS scalar"); + sk.clone() + } + + /// The big-endian encoding of a small field element. + fn be_bytes(v: u64) -> [u8; 32] { + let mut out = [0u8; 32]; + out[24..].copy_from_slice(&v.to_be_bytes()); + out + } + + /// f(x) = 7 + 11x + 13x², so f(0) = 7 is the secret. + fn poly_7_11_13() -> Vec { + vec![sk(7), sk(11), sk(13)] + } + + fn shares_at(indices: &[Index]) -> Vec { + indices + .iter() + .map(|&i| evaluate_polynomial(&poly_7_11_13(), i).expect("the polynomial is not empty")) + .collect() + } + + #[test] + fn evaluate_polynomial_rejects_empty_polynomial() { + assert!(matches!( + evaluate_polynomial(&[], 1), + Err(Error::PolynomialIsEmpty) + )); + } + + // Evaluated by hand, so the expectations do not come from the code under + // test: 7+11+13 = 31, 7+22+52 = 81, 7+55+325 = 387. + #[test_case(1, 31 ; "x = 1 sums the coefficients")] + #[test_case(2, 81 ; "x = 2")] + #[test_case(5, 387 ; "x = 5, where the squared term dominates")] + fn evaluate_polynomial_matches_hand_computed_values(x: Index, expected: u64) { + let value = evaluate_polynomial(&poly_7_11_13(), x).unwrap(); + + assert_eq!( + value.to_bytes(), + be_bytes(expected), + "f({x}) should be {expected}" + ); + } + + // The `skip(1)` loop never runs, so x must not reach the result. + #[test] + fn evaluate_polynomial_degree_zero_ignores_x() { + let poly = vec![sk(7)]; + + for x in [1u64, 2, 1_000] { + assert_eq!( + evaluate_polynomial(&poly, x).unwrap().to_bytes(), + be_bytes(7), + "a constant polynomial must evaluate to 7 at x = {x}" + ); + } + } + + // The descending set is the row that matters: + // `compute_lagrange_coefficients` negates in the scalar field when + // x_j < x_i instead of subtracting in the integers. + #[test_case(&[1, 2, 3] ; "contiguous ascending")] + #[test_case(&[2, 4, 5] ; "non-contiguous")] + #[test_case(&[5, 4, 2] ; "descending, driving the scalar_negate branch")] + fn lagrange_interpolate_secret_recovers_constant_term(indices: &[Index]) { + let recovered = lagrange_interpolate_secret(indices, &shares_at(indices)).unwrap(); + + assert_eq!( + recovered.to_bytes(), + be_bytes(7), + "f(0) = 7 must be recovered from {indices:?}" + ); + } + + // The negative property: too few shares do not fail, they silently + // interpolate a different field element. The line through f(1) and f(2) + // gives 2·f(1) − f(2) = 62 − 81 = −19, i.e. exactly r − 19. + #[test] + fn lagrange_interpolate_secret_below_threshold_yields_wrong_scalar() { + let recovered = lagrange_interpolate_secret(&[1, 2], &shares_at(&[1, 2])) + .expect("sub-threshold interpolation succeeds — that is the hazard"); + + assert_ne!( + recovered.to_bytes(), + be_bytes(7), + "two shares must not recover a 3-of-n secret" + ); + assert_eq!( + recovered.to_bytes(), + R_MINUS_19, + "sub-threshold recovery yields 2·f(1) − f(2) = −19 mod r" + ); + } + + #[test] + fn lagrange_interpolate_secret_rejects_duplicate_indices() { + let indices = [1, 2, 2]; + + assert!(matches!( + lagrange_interpolate_secret(&indices, &shares_at(&indices)), + Err(Error::IndicesNotUnique) + )); + } + + // Not `SharesAreEmpty`: that comes from `tbls::recover_secret`, which + // guards the empty map before this layer. Different guards, different + // errors. + #[test_case(&[], &[] ; "empty")] + #[test_case(&[1, 2, 3], &[1, 2] ; "more indices than shares")] + fn lagrange_interpolate_secret_rejects_length_mismatch( + indices: &[Index], + share_points: &[Index], + ) { + let result = lagrange_interpolate_secret(indices, &shares_at(share_points)); + + assert!( + matches!(result, Err(Error::IndicesSharesMismatch)), + "expected IndicesSharesMismatch" + ); + } + + // BLS signing is deterministic, so the interpolated signature must be + // byte-identical to the one the recovered secret produces. Descending + // indices again, for the negated-denominator branch. + #[test] + fn lagrange_interpolate_signature_recovers_group_signature() { + const MSG: &[u8] = b"lagrange interpolate signature"; + + let indices: [Index; 3] = [5, 4, 2]; + let partials: Vec = shares_at(&indices) + .iter() + .map(|share| share.sign(MSG, ETH2_DST, &[])) + .collect(); + + let interpolated = lagrange_interpolate_signature(&indices, &partials).unwrap(); + + assert_eq!( + interpolated.to_bytes(), + sk(7).sign(MSG, ETH2_DST, &[]).to_bytes(), + "the interpolated signature must equal the group signature" + ); + } + + // Same shape of guard as the secret path, but a *different* variant: + // `EmptySignatureArray`, not `IndicesSharesMismatch`. + #[test_case(&[], &[] ; "empty")] + #[test_case(&[1, 2, 3], &[1, 2] ; "more indices than signatures")] + fn lagrange_interpolate_signature_rejects_length_mismatch( + indices: &[Index], + sig_points: &[Index], + ) { + let partials: Vec = shares_at(sig_points) + .iter() + .map(|share| share.sign(b"mismatch", ETH2_DST, &[])) + .collect(); + + let result = lagrange_interpolate_signature(indices, &partials); + + assert!( + matches!(result, Err(Error::EmptySignatureArray)), + "expected EmptySignatureArray" + ); + } + + // pk(7) + pk(11) = pk(18). A fold that dropped or double-counted a term + // would still return a well-formed point. + #[test] + fn aggregate_public_keys_is_additively_homomorphic() { + let agg = aggregate_public_keys(&[sk(7).sk_to_pk(), sk(11).sk_to_pk()]).unwrap(); + + assert_eq!( + agg.to_bytes(), + sk(18).sk_to_pk().to_bytes(), + "pk(7) + pk(11) must equal pk(18)" + ); + } + + // n = 1 is the `skip(1)` boundary: the loop body never runs, so an + // off-by-one in the accumulator shows up only here. + #[test] + fn aggregate_public_keys_of_single_key_is_that_key() { + let agg = aggregate_public_keys(&[sk(7).sk_to_pk()]).unwrap(); + + assert_eq!(agg.to_bytes(), sk(7).sk_to_pk().to_bytes()); + } + + #[test] + fn scalar_div_rejects_zero_denominator() { + assert!(matches!( + scalar_div(&scalar_from_u64(42), &scalar_from_u64(0)), + Err(Error::DivisionByZero) + )); + } + + #[test] + fn scalar_div_multiplies_by_modular_inverse() { + let quotient = scalar_div(&scalar_from_u64(42), &scalar_from_u64(6)).unwrap(); + + assert_eq!( + quotient.b, + scalar_from_u64(7).b, + "42 / 6 = 7 in the scalar field" + ); + } + + // Negating zero, −19 == r − 19, and involution. + #[test] + fn scalar_negate_computes_additive_inverse() { + assert_eq!( + scalar_negate(&scalar_from_u64(0)).unwrap().b, + scalar_from_u64(0).b, + "the additive inverse of zero is zero" + ); + + let negative_19 = scalar_negate(&scalar_from_u64(19)).unwrap(); + + // `blst_scalar` is little-endian; `R_MINUS_19` is written big-endian. + let mut expected = R_MINUS_19; + expected.reverse(); + assert_eq!(negative_19.b, expected, "−19 must be r − 19"); + + assert_eq!( + scalar_negate(&negative_19).unwrap().b, + scalar_from_u64(19).b, + "negation must be an involution" + ); + } } diff --git a/crates/crypto/src/types.rs b/crates/crypto/src/types.rs index d0ec02ad..ba449f88 100644 --- a/crates/crypto/src/types.rs +++ b/crates/crypto/src/types.rs @@ -331,4 +331,114 @@ mod tests { let eth2_sig = sig_to_eth2(sig); assert_eq!(sig[..], eth2_sig[..]); } + + // Exhaustive over blst 0.3.17's eight variants. `BLST_SUCCESS` is the row + // that earns its place: it reaches `Unknown` through the catch-all. The + // table cannot notice a ninth variant a future blst adds — only dropping + // the `_` arm in production would, and this pass is test-only. + #[test_case(BLST_ERROR::BLST_SUCCESS, BlsError::Unknown ; "success falls through the catch-all")] + #[test_case(BLST_ERROR::BLST_BAD_ENCODING, BlsError::BadEncoding ; "bad encoding")] + #[test_case(BLST_ERROR::BLST_POINT_NOT_ON_CURVE, BlsError::PointNotOnCurve ; "point not on curve")] + #[test_case(BLST_ERROR::BLST_POINT_NOT_IN_GROUP, BlsError::PointNotInGroup ; "point not in group")] + #[test_case(BLST_ERROR::BLST_AGGR_TYPE_MISMATCH, BlsError::AggregateMismatch ; "aggregate type mismatch")] + #[test_case(BLST_ERROR::BLST_VERIFY_FAIL, BlsError::VerifyFailed ; "verify fail")] + #[test_case(BLST_ERROR::BLST_PK_IS_INFINITY, BlsError::InvalidPublicKey ; "public key is infinity")] + #[test_case(BLST_ERROR::BLST_BAD_SCALAR, BlsError::InvalidScalar ; "bad scalar")] + fn bls_error_from_blst_error(blst_error: BLST_ERROR, expected: BlsError) { + assert_eq!(BlsError::from(blst_error), expected); + } + + // These five wrap a `BlsError` and interpolate it with `{0}`. Dropping the + // `{0}` compiles and silently discards why the operation failed. Each row + // carries a different inner error, so no fixed string satisfies the table. + // The static-text variants are deliberately not covered. + #[test_case( + Error::InvalidSecretKey(BlsError::KeyGeneration), + "Failed to deserialize secret key", + "Key generation failed" + ; "invalid secret key" + )] + #[test_case( + Error::InvalidPublicKey(BlsError::PointNotInGroup), + "Failed to deserialize public key", + "Point not in group" + ; "invalid public key" + )] + #[test_case( + Error::InvalidSignature(BlsError::BadEncoding), + "Failed to deserialize signature", + "Bad encoding" + ; "invalid signature" + )] + #[test_case( + Error::VerificationFailed(BlsError::VerifyFailed), + "Signature verification failed", + "Verification failed" + ; "verification failed" + )] + #[test_case( + Error::AggregationFailed(BlsError::AggregateMismatch), + "Signature aggregation failed", + "Aggregate mismatch" + ; "aggregation failed" + )] + fn error_display_carries_wrapped_bls_error(error: Error, context: &str, cause: &str) { + let rendered = error.to_string(); + + assert!( + rendered.starts_with(context), + "expected the message to start with {context:?}" + ); + assert!( + rendered.contains(cause), + "expected the message to carry the wrapped cause {cause:?}" + ); + } + + // Two fields of the same type: swapping them compiles and produces a + // plausible message that says the opposite of the truth. + #[test] + fn invalid_threshold_display_does_not_swap_fields() { + let rendered = Error::InvalidThreshold { + threshold: 3, + total: 5, + } + .to_string(); + + assert!( + rendered.contains("threshold=3") && rendered.contains("total=5"), + "expected threshold=3 and total=5" + ); + } + + // Unreachable on 64-bit targets, so nothing else constructs it. + #[test] + fn threshold_overflow_display_carries_threshold() { + let rendered = Error::ThresholdOverflow { + threshold: 4_294_967_296, + } + .to_string(); + + assert!( + rendered.contains("4294967296"), + "expected the offending threshold in the message" + ); + } + + // The `*_from_bytes_invalid` tables pin the struct *fields*; this pins the + // rendered message. Both fields are `usize`, so a swapped format string + // leaves those tables green. + #[test] + fn conv_error_display_does_not_swap_expected_and_got() { + let rendered = ConvError::InvalidLength { + expected: PUBLIC_KEY_LENGTH, + got: PRIVATE_KEY_LENGTH, + } + .to_string(); + + assert!( + rendered.contains("expected 48") && rendered.contains("got 32"), + "expected 'expected 48' and 'got 32'" + ); + } } diff --git a/crates/frost/src/curve.rs b/crates/frost/src/curve.rs index 6a767a8b..cbb9414e 100644 --- a/crates/frost/src/curve.rs +++ b/crates/frost/src/curve.rs @@ -325,7 +325,7 @@ impl From for G1Projective { } #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; #[test] @@ -412,4 +412,214 @@ mod tests { assert_eq!(G1Projective::from(affine), generator); } + + /// The BLS12-381 scalar-field order `r`, little-endian as `from_bytes` + /// takes it. Written from the published curve parameter. + const R_LE: [u8; 32] = [ + 0x01, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xfe, 0x5b, 0xfe, 0xff, 0x02, 0xa4, 0xbd, + 0x53, 0x05, 0xd8, 0xa1, 0x09, 0x08, 0xd8, 0x39, 0x33, 0x48, 0x7d, 0x9d, 0x29, 0x53, 0xa7, + 0xed, 0x73, + ]; + + /// `r - 1`, the largest representable scalar. + const R_MINUS_1_LE: [u8; 32] = [ + 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xfe, 0x5b, 0xfe, 0xff, 0x02, 0xa4, 0xbd, + 0x53, 0x05, 0xd8, 0xa1, 0x09, 0x08, 0xd8, 0x39, 0x33, 0x48, 0x7d, 0x9d, 0x29, 0x53, 0xa7, + 0xed, 0x73, + ]; + + /// `(2^512 - 1) mod r`, computed independently of this crate. + const MAX_WIDE_REDUCED_LE: [u8; 32] = [ + 0x6c, 0x9c, 0xf2, 0xf3, 0x90, 0xe9, 0x99, 0xc9, 0x23, 0x5c, 0x92, 0x87, 0xcb, 0xed, 0x6c, + 0x2b, 0x8f, 0x39, 0x54, 0x72, 0x96, 0x14, 0xd3, 0x05, 0x11, 0xff, 0x59, 0x9f, 0xd9, 0xd9, + 0x48, 0x07, + ]; + + /// Compressed `x = 4`: a well-formed encoding of a point on the curve but + /// outside the order-`r` subgroup. `0x80` is the compression flag with the + /// sign bit clear. Shared with `frost_core`'s tests. + pub(crate) const OFF_SUBGROUP_G1_POINT: [u8; 48] = { + let mut bytes = [0u8; 48]; + bytes[0] = 0x80; + bytes[47] = 4; + bytes + }; + + /// Widen little-endian bytes to the 64 `from_bytes_wide` expects. + fn widen(bytes: &[u8; 32]) -> [u8; 64] { + let mut wide = [0u8; 64]; + wide[..32].copy_from_slice(bytes); + wide + } + + // Computed by hand rather than read off the operators. The existing tests + // only assert relationships (`scalar * inverse == ONE`), which any + // consistently wrong pair of operations also satisfies. + #[test] + fn scalar_arithmetic_matches_hand_computed_values() { + assert_eq!( + Scalar::from(7u64) + Scalar::from(11u64), + Scalar::from(18u64) + ); + assert_eq!(Scalar::from(11u64) - Scalar::from(7u64), Scalar::from(4u64)); + assert_eq!( + Scalar::from(7u64) * Scalar::from(11u64), + Scalar::from(77u64) + ); + } + + // Wrap-around at the two ends of the field, where a missing reduction + // produces something that is not a field element at all. + #[test] + fn scalar_arithmetic_wraps_at_field_edges() { + let max = Scalar::from_bytes(&R_MINUS_1_LE).expect("r - 1 is in range"); + + assert_eq!( + max + Scalar::from(1u64), + Scalar::ZERO, + "(r - 1) + 1 must wrap to zero" + ); + assert_eq!( + Scalar::ZERO - Scalar::from(1u64), + max, + "0 - 1 must wrap to r - 1" + ); + } + + // The two deserializers disagree on `r` by design: `from_bytes` is + // range-checked and rejects it, `from_bytes_wide` reduces it. Each + // assertion alone reads like an accident. + #[test] + fn scalar_from_bytes_rejects_field_order_but_wide_reduces_it() { + assert_eq!( + Scalar::from_bytes(&R_LE), + None, + "r is not a representable scalar" + ); + assert_eq!( + Scalar::from_bytes_wide(&widen(&R_LE)), + Scalar::ZERO, + "r must reduce to zero" + ); + } + + // r + 5 ≡ 5. A reduction that subtracted the modulus the wrong number of + // times still lands on *a* field element, so the value is stated. + #[test] + fn scalar_from_bytes_wide_reduces_modulo_field_order() { + let mut r_plus_5 = widen(&R_LE); + r_plus_5[0] = 0x06; + + assert_eq!(Scalar::from_bytes_wide(&r_plus_5), Scalar::from(5u64)); + } + + // The largest possible input. The round trip proves the result is a + // canonical in-range scalar and not merely some 32 bytes. + #[test] + fn scalar_from_bytes_wide_reduces_largest_input() { + let reduced = Scalar::from_bytes_wide(&[0xff; 64]); + + assert_eq!(reduced.to_bytes(), MAX_WIDE_REDUCED_LE, "(2^512 - 1) mod r"); + assert_eq!( + Scalar::from_bytes(&MAX_WIDE_REDUCED_LE), + Some(reduced), + "the reduced value must pass the range check" + ); + } + + /// Compressed `2G`, `3G` and `5G`, computed outside this crate by modular + /// arithmetic over `y^2 = x^3 + 4 (mod p)`. Comparing `G + G` against + /// `G * 2` would only show that `Add` and `Mul` agree with each other. + const TWO_G: &str = "a572cbea904d67468808c8eb50a9450c9721db309128012543902d0ac358a62ae28f75bb8f1c7c42c39a8c5529bf0f4e"; + const THREE_G: &str = "89ece308f9d1f0131765212deca99697b112d61f9be9a5f1f3780a51335b3ff981747a0b2ca2179b96d2c0c9024e5224"; + const FIVE_G: &str = "b0e7791fb972fe014159aa33a98622da3cdc98ff707965e536d8636b5fcc5ac7a91a8c46e59a00dca575af0f18fb13dc"; + + fn compressed(point: G1Projective) -> String { + hex::encode(G1Affine::from(point).to_compressed()) + } + + // `blst_p1_add_or_double` branches on whether its operands are equal, so + // doubling is a distinct code path from ordinary addition. + #[test] + fn g1_arithmetic_matches_independently_computed_points() { + let g = G1Projective::generator(); + + assert_eq!(compressed(g + g), TWO_G, "doubling"); + assert_eq!( + compressed(g * Scalar::from(3u64)), + THREE_G, + "scalar multiplication" + ); + assert_eq!( + compressed(g + g + g + g + g), + FIVE_G, + "repeated addition of unequal operands" + ); + assert_eq!( + compressed(g * Scalar::from(5u64) - g * Scalar::from(2u64)), + THREE_G, + "subtraction" + ); + } + + // `G - G == identity` also covers `Sub`, which negates and re-adds rather + // than calling blst directly. + #[test] + fn g1_group_laws_hold_for_identity() { + let g = G1Projective::generator(); + let identity = G1Projective::identity(); + + assert_eq!(g + identity, g, "the identity is neutral for addition"); + assert!( + (g - g).is_identity(), + "a point minus itself is the identity" + ); + assert!( + (g * Scalar::ZERO).is_identity(), + "multiplying by zero gives the identity" + ); + assert_eq!(g * Scalar::ONE, g, "multiplying by one is a no-op"); + } + + // The point is on the curve, so uncompression succeeds and only + // `blst_p1_affine_in_g1` stands between it and acceptance. Both + // constructors are checked because the projective one delegates. + #[test] + fn g1_from_compressed_rejects_off_subgroup_point() { + assert!( + G1Affine::from_compressed(&OFF_SUBGROUP_G1_POINT).is_none(), + "an off-subgroup point must not deserialize" + ); + assert_eq!( + G1Projective::from_compressed(&OFF_SUBGROUP_G1_POINT), + None, + "the projective constructor must reject it too" + ); + } + + // Both rejected before any curve arithmetic happens. + #[test] + fn g1_from_compressed_rejects_malformed_encodings() { + // Compression flag clear: not a compressed point at all. + assert!(G1Affine::from_compressed(&[0x00; 48]).is_none()); + + // Compression flag set, but x is larger than the base field modulus. + let mut x_out_of_range = [0xffu8; 48]; + x_out_of_range[0] = 0x9f; + assert!(G1Affine::from_compressed(&x_out_of_range).is_none()); + } + + // The identity *is* a G1 element, so the affine constructor accepts it and + // only the projective one rejects it. That asymmetry matters because + // `from_commitments` goes through the projective constructor. + #[test] + fn g1_affine_accepts_identity_that_projective_rejects() { + let identity = G1Affine::from(G1Projective::identity()).to_compressed(); + + assert!( + G1Affine::from_compressed(&identity).is_some_and(|affine| affine.is_identity()), + "the affine constructor accepts the identity" + ); + assert_eq!(G1Projective::from_compressed(&identity), None); + } } diff --git a/crates/frost/src/frost_core.rs b/crates/frost/src/frost_core.rs index 4dce936f..8c23d5d6 100644 --- a/crates/frost/src/frost_core.rs +++ b/crates/frost/src/frost_core.rs @@ -619,4 +619,66 @@ mod tests { Err(FrostCoreError::EmptyPolynomial) )); } + + /// Three distinct, valid G1 commitments: `[G, 2G, 3G]`. + // `clippy.toml` exempts `#[test]` bodies from this lint but not the helpers + // they call. + #[expect( + clippy::arithmetic_side_effects, + reason = "BLS12-381 group/scalar arithmetic has no integer overflow semantics" + )] + fn valid_commitments() -> [[u8; 48]; 3] { + let g = G1Projective::generator(); + + [ + G1Affine::from(g).to_compressed(), + G1Affine::from(g * Scalar::from(2u64)).to_compressed(), + G1Affine::from(g * Scalar::from(3u64)).to_compressed(), + ] + } + + // `deserialize_commitment_rejects_invalid_point` cannot see *position*: + // the `collect::>()` short-circuits, so a check that only looked + // at the first coefficient would still pass it. The positive control is + // here because "rejects everything" would satisfy the sweep on its own. + #[test] + fn from_commitments_rejects_off_subgroup_point_at_any_position() { + let valid = valid_commitments(); + + assert!( + VerifiableSecretSharingCommitment::from_commitments(&valid).is_some(), + "the unmodified vector must be accepted" + ); + + for position in 0..valid.len() { + let mut corrupted = valid; + corrupted[position] = crate::curve::tests::OFF_SUBGROUP_G1_POINT; + + assert!( + VerifiableSecretSharingCommitment::from_commitments(&corrupted).is_none(), + "an off-subgroup point at position {position} must be rejected" + ); + } + } + + // A commitment to the identity is a commitment to a zero coefficient — for + // the constant term, a group public key of infinity. Only + // `G1Projective::from_compressed` rejects it, so this pins that + // `from_commitments` goes through the projective constructor. + #[test] + fn from_commitments_rejects_identity() { + let identity = G1Affine::from(G1Projective::identity()).to_compressed(); + + assert!( + VerifiableSecretSharingCommitment::from_commitments(&[identity]).is_none(), + "a lone identity commitment must be rejected" + ); + + let mut with_identity = valid_commitments(); + with_identity[1] = identity; + assert!( + VerifiableSecretSharingCommitment::from_commitments(&with_identity).is_none(), + "the identity must be rejected mid-vector too" + ); + } } diff --git a/crates/frost/src/kryptology.rs b/crates/frost/src/kryptology.rs index f2dd7041..c8049fd4 100644 --- a/crates/frost/src/kryptology.rs +++ b/crates/frost/src/kryptology.rs @@ -748,6 +748,10 @@ mod tests { use super::*; + /// A DKG context byte that is not zero. At `ctx = 0` a context dropped on + /// the floor is indistinguishable from one that was used. + const NON_ZERO_CTX: u8 = 0x2a; + #[test] fn shamir_share_debug_redacts_value() { let share = ShamirShare { @@ -978,12 +982,17 @@ mod tests { } /// 2-of-3 DKG then BLS threshold signing (Ethereum 2.0 compatible). + /// + /// Runs at a non-zero context; `bls_round_trip_3_of_3` keeps the `ctx = 0` + /// case. It is the positive control for the `ctx` tests below: without it, + /// "a different ctx is rejected" would also pass against an implementation + /// where any non-zero `ctx` breaks the protocol outright. #[test] fn bls_round_trip_2_of_3() { let mut rng = StdRng::seed_from_u64(123); let threshold = 2u16; let max_signers = 3u16; - let ctx = 0u8; + let ctx = NON_ZERO_CTX; let mut bcasts: BTreeMap = BTreeMap::new(); let mut all_shares: BTreeMap> = BTreeMap::new(); @@ -1334,4 +1343,88 @@ mod tests { Err(KryptologyError::DuplicateIdentifier(1)) )); } + + // `ctx` does not enter the secret polynomial — the Feldman commitments are + // byte-identical across contexts — but it does enter the Schnorr + // challenge, so `ci` and `wi` both change. Both RNGs are seeded + // identically, so the context byte is the only difference. + #[test] + fn ctx_binds_schnorr_proof_but_not_polynomial() { + let (threshold, max_signers) = (2u16, 3u16); + + let mut rng_zero = StdRng::seed_from_u64(1789); + let (zero_ctx, ..) = + round1(1, threshold, max_signers, 0, &mut rng_zero).expect("round1 at ctx 0"); + + let mut rng_non_zero = StdRng::seed_from_u64(1789); + let (non_zero_ctx, ..) = round1(1, threshold, max_signers, NON_ZERO_CTX, &mut rng_non_zero) + .expect("round1 at a non-zero ctx"); + + assert_eq!( + zero_ctx.commitments, non_zero_ctx.commitments, + "ctx must not reach the secret polynomial" + ); + assert_ne!( + zero_ctx.ci, non_zero_ctx.ci, + "ctx must reach the Schnorr challenge" + ); + assert_ne!( + zero_ctx.wi, non_zero_ctx.wi, + "a different challenge must give a different response" + ); + } + + // The context byte is what stops a proof from one DKG session being + // replayed into another. With a single sender the culprit is unambiguous. + #[test] + fn round2_rejects_broadcast_from_different_ctx() { + let mut rng = StdRng::seed_from_u64(31337); + let (threshold, max_signers) = (2u16, 2u16); + + let (_bcast1, _shares1, secret1) = + round1(1, threshold, max_signers, NON_ZERO_CTX, &mut rng).expect("round1 at our ctx"); + let (foreign_bcast, foreign_shares, _secret2) = + round1(2, threshold, max_signers, 0, &mut rng).expect("round1 at a foreign ctx"); + + let result = round2( + secret1, + &[(2, foreign_bcast)].into(), + &[(2, foreign_shares[&1].clone())].into(), + ); + + assert!( + matches!(result, Err(KryptologyError::InvalidProof { culprit: 2 })), + "expected InvalidProof from participant 2" + ); + } + + // The interoperability surface with Go's kryptology: a reordered preimage + // still produces a self-consistent Rust DKG, so every other test here would + // pass while charon rejected every proof we send. + #[test] + fn challenge_preimage_is_id_ctx_commitment_nonce() { + let commitment_0 = G1Projective::generator() * Scalar::from(11u64); + let nonce_point = G1Projective::generator() * Scalar::from(13u64); + + let mut expected_preimage = vec![7u8, NON_ZERO_CTX]; + expected_preimage.extend_from_slice(&G1Affine::from(commitment_0).to_compressed()); + expected_preimage.extend_from_slice(&G1Affine::from(nonce_point).to_compressed()); + assert_eq!(expected_preimage.len(), 98); + + assert_eq!( + kryptology_challenge(7, NON_ZERO_CTX, &commitment_0, &nonce_point), + kryptology_hash_to_scalar(&expected_preimage) + ); + + // Neither the two leading bytes nor the two points are + // interchangeable, which the assertion above cannot show on its own. + assert_ne!( + kryptology_challenge(7, NON_ZERO_CTX, &commitment_0, &nonce_point), + kryptology_challenge(NON_ZERO_CTX, 7, &commitment_0, &nonce_point) + ); + assert_ne!( + kryptology_challenge(7, NON_ZERO_CTX, &commitment_0, &nonce_point), + kryptology_challenge(7, NON_ZERO_CTX, &nonce_point, &commitment_0) + ); + } } diff --git a/crates/k1util/src/k1util.rs b/crates/k1util/src/k1util.rs index 208e49d7..c20a6259 100644 --- a/crates/k1util/src/k1util.rs +++ b/crates/k1util/src/k1util.rs @@ -490,4 +490,215 @@ mod tests { assert_eq!(result.unwrap(), key, "Key should match"); } } + + fn key_1() -> SecretKey { + SecretKey::from_slice(&hex::decode(PRIV_KEY_1).expect("PRIV_KEY_1 is valid hex")) + .expect("PRIV_KEY_1 is a valid secp256k1 scalar") + } + + /// A second, unrelated key. `0x1111..11` is a valid secp256k1 scalar. + fn key_2() -> SecretKey { + SecretKey::from_slice(&[0x11u8; SCALAR_LEN]).expect("0x11..11 is a valid secp256k1 scalar") + } + + fn digest_1() -> Vec { + hex::decode(DIGEST_1).expect("DIGEST_1 is valid hex") + } + + fn sig_1() -> [u8; SIGNATURE_LEN] { + let bytes = hex::decode(SIG_1).expect("SIG_1 is valid hex"); + let mut sig = [0u8; SIGNATURE_LEN]; + sig.copy_from_slice(&bytes); + sig + } + + /// Ways a *well-formed* input can still fail to verify: every length, + /// scalar and recovery byte stays valid, so no error path is taken. + #[derive(Debug, Clone, Copy)] + enum Corruption { + WrongKey, + WrongHash, + CorruptedR, + } + + fn corrupted_input(corruption: Corruption) -> (PublicKey, Vec, [u8; SIGNATURE_LEN]) { + match corruption { + Corruption::WrongKey => (key_2().public_key(), digest_1(), sig_1()), + Corruption::WrongHash => { + let mut hash = digest_1(); + hash[0] ^= 0xff; + (key_1().public_key(), hash, sig_1()) + } + Corruption::CorruptedR => { + let mut sig = sig_1(); + // 0xe0 ^ 0xff = 0x1f, so R stays a valid non-zero scalar. + sig[0] ^= 0xff; + (key_1().public_key(), digest_1(), sig) + } + } + } + + // Every other assertion here checks a *successful* verification, so a + // `verify_64` hard-wired to `Ok(true)` would pass the whole suite. A failed + // verification is `Ok(false)`, not an error. + #[test_case(Corruption::WrongKey ; "wrong public key")] + #[test_case(Corruption::WrongHash ; "wrong hash")] + #[test_case(Corruption::CorruptedR ; "corrupted r")] + fn verify_64_returns_false_for_wrong_signature(corruption: Corruption) { + let (pubkey, hash, sig) = corrupted_input(corruption); + + let verified = verify_64(&pubkey, &hash, &sig[..SIGNATURE_LEN_WITHOUT_V]) + .expect("a well-formed input must not produce an error"); + + assert!(!verified, "{corruption:?} must verify as false"); + } + + // `verify_65` recovers and compares rather than verifying prehashed, so it + // needs its own `Ok(false)` catcher. `CorruptedR` is absent: it changes + // which key is recovered, and recovery is allowed to fail outright. + #[test_case(Corruption::WrongKey ; "wrong public key")] + #[test_case(Corruption::WrongHash ; "wrong hash")] + fn verify_65_returns_false_for_wrong_signature(corruption: Corruption) { + let (pubkey, hash, sig) = corrupted_input(corruption); + + let verified = + verify_65(&pubkey, &hash, &sig).expect("a well-formed input must not produce an error"); + + assert!(!verified, "{corruption:?} must verify as false"); + } + + // Both the expected and the actual length must be reported: swapping the + // two fields would still satisfy an `is_err()` check. + #[test_case(SIGNATURE_LEN_WITHOUT_V - 1 ; "one byte short")] + #[test_case(SIGNATURE_LEN ; "the 65-byte format offered to the 64-byte verifier")] + fn verify_64_rejects_wrong_length_signature(len: usize) { + let err = verify_64(&key_1().public_key(), &digest_1(), &vec![0u8; len]) + .expect_err("a wrong-length signature must be rejected"); + + assert!( + matches!( + err, + K1UtilError::InvalidSignatureLength { expected, actual } + if expected == SIGNATURE_LEN_WITHOUT_V && actual == len + ), + "expected InvalidSignatureLength {{ expected: {SIGNATURE_LEN_WITHOUT_V}, actual: {len} }}" + ); + } + + #[test_case(K1_HASH_LEN - 1 ; "one byte short")] + #[test_case(K1_HASH_LEN + 1 ; "one byte long")] + fn verify_64_rejects_wrong_length_hash(len: usize) { + let sig = sig_1(); + + let err = verify_64( + &key_1().public_key(), + &vec![0u8; len], + &sig[..SIGNATURE_LEN_WITHOUT_V], + ) + .expect_err("a wrong-length hash must be rejected"); + + assert!( + matches!(err, K1UtilError::InvalidHashLength { actual } if actual == len), + "expected InvalidHashLength {{ actual: {len} }}" + ); + } + + // 64 is the interesting row: confusing [R || S] with [R || S || V] is the + // realistic mistake. + #[test_case(SIGNATURE_LEN_WITHOUT_V ; "the 64-byte format offered to the recoverer")] + #[test_case(SIGNATURE_LEN + 1 ; "one byte long")] + fn recover_rejects_wrong_length_signature(len: usize) { + let err = recover(&digest_1(), &vec![0u8; len]) + .expect_err("a wrong-length signature must be rejected"); + + assert!( + matches!( + err, + K1UtilError::InvalidSignatureLength { expected, actual } + if expected == SIGNATURE_LEN && actual == len + ), + "expected InvalidSignatureLength {{ expected: {SIGNATURE_LEN}, actual: {len} }}" + ); + } + + #[test_case(K1_HASH_LEN - 1 ; "one byte short")] + #[test_case(K1_HASH_LEN + 1 ; "one byte long")] + fn recover_rejects_wrong_length_hash(len: usize) { + let err = + recover(&vec![0u8; len], &sig_1()).expect_err("a wrong-length hash must be rejected"); + + assert!( + matches!(err, K1UtilError::InvalidHashLength { actual } if actual == len), + "expected InvalidHashLength {{ actual: {len} }}" + ); + } + + #[test_case(K1_HASH_LEN - 1 ; "one byte short")] + #[test_case(K1_HASH_LEN + 1 ; "one byte long")] + fn sign_rejects_wrong_length_hash(len: usize) { + let err = + sign(&key_1(), &vec![0u8; len]).expect_err("a wrong-length hash must be rejected"); + + assert!( + matches!(err, K1UtilError::InvalidHashLength { actual } if actual == len), + "expected InvalidHashLength {{ actual: {len} }}" + ); + } + + // `recover_recovery_id_domain` only asserts that 27 and 28 are *accepted*. + // This pins what as: 27 is the Bitcoin-compact spelling of 0, 28 of 1. A + // mapping that folded them together would still pass the domain test. + #[test] + fn recover_maps_compact_recovery_ids_to_ethereum_ids() { + let digest = digest_1(); + let mut sig = sig_1(); + + let mut recovered_with = |recovery_byte: u8| { + sig[K1_REC_IDX] = recovery_byte; + recover(&digest, &sig) + .unwrap_or_else(|e| panic!("recovery byte {recovery_byte} must recover: {e}")) + }; + + let from_0 = recovered_with(0); + let from_1 = recovered_with(1); + let from_27 = recovered_with(27); + let from_28 = recovered_with(28); + + // Without this the two equalities below would hold for any mapping. + assert_ne!( + from_0, from_1, + "recovery ids 0 and 1 must yield different keys" + ); + assert_eq!(from_27, from_0, "27 is the Bitcoin-compact spelling of 0"); + assert_eq!(from_28, from_1, "28 is the Bitcoin-compact spelling of 1"); + } + + #[test] + fn public_key_from_libp2p_round_trips_secp256k1_key() { + let sec1 = key_1().public_key().to_sec1_bytes(); + let libp2p_key = libp2p::identity::secp256k1::PublicKey::try_from_bytes(&sec1) + .expect("a compressed sec1 point is a valid libp2p secp256k1 key"); + + let converted = public_key_from_libp2p(Libp2pPublicKey::from(libp2p_key)) + .expect("a secp256k1 libp2p key must convert"); + + assert_eq!( + converted.to_sec1_bytes().to_vec(), + hex::decode(PUB_KEY_1).unwrap(), + "conversion must preserve the key" + ); + } + + #[test] + fn public_key_from_libp2p_rejects_non_secp256k1_key() { + let ed25519 = libp2p::identity::ed25519::Keypair::generate().public(); + + let err = public_key_from_libp2p(Libp2pPublicKey::from(ed25519)) + .expect_err("a non-secp256k1 key must be rejected"); + + assert!( + matches!(err, K1UtilError::FailedToParseLibp2pPublicKey(_)), + "expected FailedToParseLibp2pPublicKey" + ); + } }