diff --git a/crates/crypto/src/tbls.rs b/crates/crypto/src/tbls.rs index f09b1208..6aaaa500 100644 --- a/crates/crypto/src/tbls.rs +++ b/crates/crypto/src/tbls.rs @@ -846,6 +846,92 @@ mod tests { assert!(!shares.contains_key(&0), "Should not contain key 0"); } + /// All size-`k` combinations of `items`, order-independent. + fn combinations(items: &[T], k: usize) -> Vec> { + if k == 0 { + return vec![vec![]]; + } + // No combinations exist once `k` exceeds the remaining items. + let Some(max_start) = items.len().checked_sub(k) else { + return vec![]; + }; + let mut out = Vec::new(); + for i in 0..=max_start { + let rest = &items[i.saturating_add(1)..]; + for mut tail in combinations(rest, k.saturating_sub(1)) { + let mut combo = vec![items[i]]; + combo.append(&mut tail); + out.push(combo); + } + } + out + } + + /// Lagrange interpolation of the secret must recover the original from + /// *every* threshold-sized subset of shares, not just the first `threshold` + /// or the full set — exercising `lagrange_interpolate_secret` across many + /// distinct (and non-contiguous) index sets. + #[test] + fn recover_secret_from_every_threshold_subset() { + use rand::rngs::OsRng; + + for (total, threshold) in [(4u64, 2u64), (5, 3), (6, 4), (7, 4)] { + let secret = generate_secret_key(OsRng).unwrap(); + let shares = threshold_split(&secret, total, threshold).unwrap(); + let indices: Vec = { + let mut ks: Vec = shares.keys().copied().collect(); + ks.sort_unstable(); + ks + }; + + for subset in combinations(&indices, usize::try_from(threshold).unwrap()) { + let picked: HashMap = + subset.iter().map(|idx| (*idx, shares[idx])).collect(); + let recovered = recover_secret(&picked).unwrap(); + assert_eq!( + secret, recovered, + "recovery from subset {subset:?} of (t={threshold}, n={total}) must match", + ); + } + } + } + + /// Lagrange interpolation of signatures (the Pippenger MSM path) must + /// reconstruct the group signature from *every* threshold-sized subset of + /// partial signatures, and it must equal the signature produced directly by + /// the master secret. + #[test] + fn threshold_aggregate_from_every_threshold_subset() { + use rand::rngs::OsRng; + + let data = b"hello obol!"; + for (total, threshold) in [(4u64, 2u64), (5, 3), (6, 4), (7, 4)] { + let secret = generate_secret_key(OsRng).unwrap(); + let direct_sig = sign(&secret, data).unwrap(); + + let shares = threshold_split(&secret, total, threshold).unwrap(); + let partials: HashMap = shares + .iter() + .map(|(idx, key)| (*idx, sign(key, data).unwrap())) + .collect(); + let indices: Vec = { + let mut ks: Vec = partials.keys().copied().collect(); + ks.sort_unstable(); + ks + }; + + for subset in combinations(&indices, usize::try_from(threshold).unwrap()) { + let picked: HashMap = + subset.iter().map(|idx| (*idx, partials[idx])).collect(); + let aggregated = threshold_aggregate(&picked).unwrap(); + assert_eq!( + direct_sig, aggregated, + "aggregate from subset {subset:?} of (t={threshold}, n={total}) must match direct sign", + ); + } + } + } + /// Compressed `x = 4`: on the curve, outside the order-`r` subgroup G1. /// `0x80` is the compression flag, sign bit clear. const OFF_SUBGROUP_G1_POINT: PublicKey = { diff --git a/crates/crypto/src/tbls/math.rs b/crates/crypto/src/tbls/math.rs index e64172ce..a32c689a 100644 --- a/crates/crypto/src/tbls/math.rs +++ b/crates/crypto/src/tbls/math.rs @@ -5,11 +5,17 @@ use std::collections::HashSet; -use blst::min_pk::{ - PublicKey as BlstPublicKey, SecretKey as BlstSecretKey, Signature as BlstSignature, +use blst::{ + MultiPoint, + min_pk::{PublicKey as BlstPublicKey, SecretKey as BlstSecretKey, Signature as BlstSignature}, }; +use zeroize::Zeroize; -use crate::types::{Error, Index}; +use crate::types::{Error, Index, SCALAR_LENGTH}; + +/// Bit width of BLS12-381 scalars as consumed by blst multi-scalar +/// multiplication. +const SCALAR_BITS: usize = 255; /// Aggregate public keys pub(super) fn aggregate_public_keys(pks: &[BlstPublicKey]) -> Result { @@ -42,26 +48,33 @@ pub(super) fn evaluate_polynomial( poly: &[BlstSecretKey], x: Index, ) -> Result { - if poly.is_empty() { - return Err(Error::PolynomialIsEmpty); - } - - // Start with the constant term - let mut result = poly[0].clone(); + // The fr-domain copy of the secret coefficients is wiped on drop; the + // evaluation result is wiped explicitly once converted back to a key. + let poly_fr = SecretFrVec::from_secrets(poly); + let mut acc = evaluate_polynomial_fr(&poly_fr.0, x)?; + let result = secret_from_fr(&acc); + wipe_fr(std::slice::from_mut(&mut acc)); + result +} - // Horner-free evaluation: `x_power` holds x^i entering iteration i. - let x_scalar = scalar_from_u64(x); - let mut x_power = x_scalar.clone(); +/// Evaluate polynomial at point x using Horner's method in the fr domain: +/// poly(x) = a_0 + a_1*x + a_2*x^2 + ... + a_n*x^n +fn evaluate_polynomial_fr(poly: &[blst::blst_fr], x: Index) -> Result { + let Some(highest) = poly.last() else { + return Err(Error::PolynomialIsEmpty); + }; - for coeff in poly.iter().skip(1) { - // result += coeff * x_power - let term = scalar_mult_secret(coeff, &x_power)?; - result = scalar_add_secret(&result, &term)?; + let x_fr = fr_from_scalar(&scalar_from_u64(x)); + let mut acc = *highest; - x_power = scalar_mult_scalars(&x_power, &x_scalar)?; + unsafe { + for coeff in poly.iter().rev().skip(1) { + blst::blst_fr_mul(&mut acc, &acc, &x_fr); + blst::blst_fr_add(&mut acc, &acc, coeff); + } } - Ok(result) + Ok(acc) } /// Lagrange interpolation of secret keys at x=0 @@ -74,17 +87,28 @@ pub(super) fn lagrange_interpolate_secret( return Err(Error::IndicesSharesMismatch); } - // Compute Lagrange coefficients and interpolate let coeffs = compute_lagrange_coefficients(indices)?; - let mut result = BlstSecretKey::default(); + // The fr-domain copies of the shares and the accumulator hold secret + // material; both are wiped before returning. + let shares_fr = SecretFrVec::from_secrets(shares); + let mut acc = blst::blst_fr::default(); - for i in 0..shares.len() { - let term = scalar_mult_secret(&shares[i], &coeffs[i])?; - result = scalar_add_secret(&result, &term)?; + unsafe { + for (share, coeff) in shares_fr.0.iter().zip(&coeffs) { + // `term = share_i·λ_i` is recoverable secret material and `blst_fr` + // is `Copy` with no zeroizing `Drop`, so wipe it each iteration. + let mut term = blst::blst_fr::default(); + blst::blst_fr_mul(&mut term, share, coeff); + blst::blst_fr_add(&mut acc, &acc, &term); + wipe_fr(std::slice::from_mut(&mut term)); + } } - Ok(result) + let result = secret_from_fr(&acc); + wipe_fr(std::slice::from_mut(&mut acc)); + + result } /// Lagrange interpolation of signatures at x=0 @@ -100,191 +124,139 @@ pub(super) fn lagrange_interpolate_signature( // Compute Lagrange coefficients let coeffs = compute_lagrange_coefficients(indices)?; - // Multiply each signature by its Lagrange coefficient and aggregate - let first_sig_scaled = signature_mult(&signatures[0], &coeffs[0])?; - let mut result_p2 = blst::blst_p2::default(); - - unsafe { - // Convert first scaled signature to projective - let first_affine: &blst::blst_p2_affine = (&first_sig_scaled).into(); - blst::blst_p2_from_affine(&mut result_p2, first_affine); - - for i in 1..signatures.len() { - let sig_scaled = signature_mult(&signatures[i], &coeffs[i])?; - let sig_affine: &blst::blst_p2_affine = (&sig_scaled).into(); - blst::blst_p2_add_or_double_affine(&mut result_p2, &result_p2, sig_affine); - } - - // Convert back to affine - let mut result_affine = blst::blst_p2_affine::default(); - blst::blst_p2_to_affine(&mut result_affine, &result_p2); - Ok(BlstSignature::from(result_affine)) + let mut scalar_bytes = Vec::with_capacity(SCALAR_LENGTH.saturating_mul(coeffs.len())); + for coeff in &coeffs { + scalar_bytes.extend_from_slice(&scalar_from_fr(coeff).b); } + + // Multi-scalar multiplication (Pippenger) of all signatures by their + // Lagrange coefficients in one pass, with a single final affine + // conversion (each affine conversion costs a field inversion). `blst`'s + // `MultiPoint for [Signature]` transmutes to the affine slice and runs the + // same MSM, so this matches the hand-rolled version without the manual + // affine extraction and conversion. + Ok(signatures.mult(&scalar_bytes, SCALAR_BITS).to_signature()) } -/// Compute Lagrange coefficients for interpolation at x=0 +/// Compute Lagrange coefficients for interpolation at x=0, in the fr domain: /// λ_i = ∏_{j≠i} (0 - x_j) / (x_i - x_j) = ∏_{j≠i} x_j / (x_j - x_i) -fn compute_lagrange_coefficients(indices: &[Index]) -> Result, Error> { +fn compute_lagrange_coefficients(indices: &[Index]) -> Result, Error> { // Check if indices are unique if indices.len() != indices.iter().collect::>().len() { return Err(Error::IndicesNotUnique); } - let mut coeffs = Vec::with_capacity(indices.len()); + let indices_fr: Vec = indices + .iter() + .map(|&x| fr_from_scalar(&scalar_from_u64(x))) + .collect(); + let one = fr_from_scalar(&scalar_from_u64(1)); - for (i, &x_i) in indices.iter().enumerate() { - let mut numerator = scalar_from_u64(1); - let mut denominator = scalar_from_u64(1); + let mut coeffs = Vec::with_capacity(indices.len()); - for (j, &x_j) in indices.iter().enumerate() { - if i == j { - continue; + unsafe { + for (i, x_i) in indices_fr.iter().enumerate() { + let mut numerator = one; + let mut denominator = one; + + for (j, x_j) in indices_fr.iter().enumerate() { + if i == j { + continue; + } + + // numerator *= x_j + blst::blst_fr_mul(&mut numerator, &numerator, x_j); + + // denominator *= (x_j - x_i), computed modulo the field order. + let mut diff = blst::blst_fr::default(); + blst::blst_fr_sub(&mut diff, x_j, x_i); + blst::blst_fr_mul(&mut denominator, &denominator, &diff); } - // numerator *= x_j - let x_j_scalar = scalar_from_u64(x_j); - numerator = scalar_mult_scalars(&numerator, &x_j_scalar)?; + // `blst_fr_eucl_inverse` below is variable-time, which is fine + // here: it only ever operates on public share indices. + // Unreachable with unique indices, but guard division regardless. + if scalar_from_fr(&denominator) == blst::blst_scalar::default() { + return Err(Error::DivisionByZero); + } - // denominator *= (x_j - x_i) - let diff = if x_j > x_i { - scalar_from_u64(x_j.abs_diff(x_i)) - } else { - // For negative differences, we need to work in the scalar field - // x_j - x_i (mod r) where r is the curve order - scalar_negate(&scalar_from_u64(x_i.abs_diff(x_j)))? - }; + // coeff = numerator / denominator + let mut inverse = blst::blst_fr::default(); + blst::blst_fr_eucl_inverse(&mut inverse, &denominator); - denominator = scalar_mult_scalars(&denominator, &diff)?; + let mut coeff = blst::blst_fr::default(); + blst::blst_fr_mul(&mut coeff, &numerator, &inverse); + coeffs.push(coeff); } - - // Compute numerator / denominator = numerator * denominator^{-1} - let coeff = scalar_div(&numerator, &denominator)?; - coeffs.push(coeff); } Ok(coeffs) } -/// Convert u64 to blst scalar -fn scalar_from_u64(val: u64) -> blst::blst_scalar { - let mut scalar = blst::blst_scalar::default(); - let limbs: [u64; 4] = [val, 0, 0, 0]; - unsafe { - blst::blst_scalar_from_uint64(&mut scalar, limbs.as_ptr()); - } - scalar +/// Converts a scalar to the fr (Montgomery) domain. +fn fr_from_scalar(scalar: &blst::blst_scalar) -> blst::blst_fr { + let mut fr = blst::blst_fr::default(); + unsafe { blst::blst_fr_from_scalar(&mut fr, scalar) }; + fr } -/// Multiply secret key by scalar -fn scalar_mult_secret( - sk: &BlstSecretKey, - scalar: &blst::blst_scalar, -) -> Result { - let sk_scalar = sk.into(); - let result_scalar = scalar_mult_scalars(sk_scalar, scalar)?; - let sk: &BlstSecretKey = (&result_scalar) - .try_into() - .map_err(|_| Error::FailedToConvertSkToBlstScalar)?; - Ok(sk.clone()) +/// Converts an fr (Montgomery) value back to a scalar. +fn scalar_from_fr(fr: &blst::blst_fr) -> blst::blst_scalar { + let mut scalar = blst::blst_scalar::default(); + unsafe { blst::blst_scalar_from_fr(&mut scalar, fr) }; + scalar } -/// Add two secret keys -fn scalar_add_secret(sk1: &BlstSecretKey, sk2: &BlstSecretKey) -> Result { - let result = scalar_add(sk1.into(), sk2.into())?; - let sk: &BlstSecretKey = (&result) - .try_into() - .map_err(|_| Error::FailedToConvertScalarToSecretKey)?; - Ok(sk.clone()) +/// Converts an fr value to a secret key, validating it (nonzero, below the +/// group order) exactly like the previous scalar-domain conversion did. +fn secret_from_fr(fr: &blst::blst_fr) -> Result { + let mut scalar = scalar_from_fr(fr); + let result = <&BlstSecretKey>::try_from(&scalar) + .cloned() + .map_err(|_| Error::FailedToConvertScalarToSecretKey); + scalar.zeroize(); + result } -/// Multiply signature by scalar -fn signature_mult(sig: &BlstSignature, scalar: &blst::blst_scalar) -> Result { - let mut sig_proj = blst::blst_p2::default(); - let mut result_p2 = blst::blst_p2::default(); - let mut result_affine = blst::blst_p2_affine::default(); - - unsafe { - // Convert affine to projective - let sig_affine: &blst::blst_p2_affine = sig.into(); - blst::blst_p2_from_affine(&mut sig_proj, sig_affine); - // Multiply - blst::blst_p2_mult(&mut result_p2, &sig_proj, scalar.b.as_ptr(), 255); - // Convert back to affine - blst::blst_p2_to_affine(&mut result_affine, &result_p2); +/// Best-effort volatile wipe of fr values holding secret material. +fn wipe_fr(values: &mut [blst::blst_fr]) { + for value in values.iter_mut() { + // SAFETY: `value` is a valid, aligned, exclusive reference. + unsafe { std::ptr::write_volatile(value, blst::blst_fr::default()) }; } - - Ok(BlstSignature::from(result_affine)) } -/// Add two scalars -fn scalar_add(a: &blst::blst_scalar, b: &blst::blst_scalar) -> Result { - let mut result = blst::blst_scalar::default(); - unsafe { - if blst::blst_sk_add_n_check(&mut result, a, b) { - Ok(result) - } else { - Err(Error::FailedToAddScalars) - } +/// Fr-domain copies of secret keys, wiped on drop. +struct SecretFrVec(Vec); + +impl SecretFrVec { + fn from_secrets(secrets: &[BlstSecretKey]) -> Self { + Self( + secrets + .iter() + .map(|sk| { + let scalar: &blst::blst_scalar = sk.into(); + fr_from_scalar(scalar) + }) + .collect(), + ) } } -/// Multiply two scalars -fn scalar_mult_scalars( - a: &blst::blst_scalar, - b: &blst::blst_scalar, -) -> Result { - let mut result = blst::blst_scalar::default(); - unsafe { - if blst::blst_sk_mul_n_check(&mut result, a, b) { - Ok(result) - } else { - Err(Error::FailedToMultiplyScalars) - } - } -} - -/// Negate a scalar -fn scalar_negate(a: &blst::blst_scalar) -> Result { - // To negate in the field, we compute (r - a) where r is the curve order - // But blst doesn't expose this directly, so we use: -a ≡ r - a - // We can compute this as: 0 - a - let zero = scalar_from_u64(0); - let mut result_scalar = blst::blst_scalar::default(); - - unsafe { - // Convert scalars to fr for arithmetic - let mut a_fr = blst::blst_fr::default(); - let mut zero_fr = blst::blst_fr::default(); - - blst::blst_fr_from_scalar(&mut a_fr, a); - blst::blst_fr_from_scalar(&mut zero_fr, &zero); - - let mut result_fr = blst::blst_fr::default(); - blst::blst_fr_sub(&mut result_fr, &zero_fr, &a_fr); - - blst::blst_scalar_from_fr(&mut result_scalar, &result_fr); +impl Drop for SecretFrVec { + fn drop(&mut self) { + wipe_fr(&mut self.0); } - - Ok(result_scalar) } -/// Divide two scalars (multiply by inverse) -fn scalar_div( - numerator: &blst::blst_scalar, - denominator: &blst::blst_scalar, -) -> Result { - let zero = blst::blst_scalar::default(); - if *denominator == zero { - return Err(Error::DivisionByZero); - } - - let mut inv_scalar = blst::blst_scalar::default(); - +/// Convert u64 to blst scalar +fn scalar_from_u64(val: u64) -> blst::blst_scalar { + let mut scalar = blst::blst_scalar::default(); + let limbs: [u64; 4] = [val, 0, 0, 0]; unsafe { - blst::blst_sk_inverse(&mut inv_scalar, denominator); + blst::blst_scalar_from_uint64(&mut scalar, limbs.as_ptr()); } - - scalar_mult_scalars(numerator, &inv_scalar) + scalar } #[cfg(test)] @@ -389,11 +361,11 @@ mod tests { } // 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. + // `compute_lagrange_coefficients` subtracts modulo the field order, so + // x_j < x_i wraps rather than underflowing. #[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")] + #[test_case(&[5, 4, 2] ; "descending, driving the negative-difference branch")] fn lagrange_interpolate_secret_recovers_constant_term(indices: &[Index]) { let recovered = lagrange_interpolate_secret(indices, &shares_at(indices)).unwrap(); @@ -516,45 +488,42 @@ mod tests { 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) - )); + /// The BLS12-381 scalar-field order minus 3, big-endian. Written from the + /// published curve order, not read off this implementation. + const R_MINUS_3: [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, 0xfe, + ]; + + /// The big-endian bytes of an fr value. + fn fr_be_bytes(fr: &blst::blst_fr) -> [u8; 32] { + let mut bytes = scalar_from_fr(fr).b; + // `blst_scalar` is little-endian; the expectations are big-endian. + bytes.reverse(); + bytes } + // The coefficients themselves, by hand, so the two arithmetic steps the + // fr domain took over from the old `scalar_negate`/`scalar_div` helpers + // are pinned directly: λ₁ = (2/1)·(3/2) = 3, λ₂ = (1/−1)·(3/1) = −3, + // λ₃ = (1/−2)·(2/−1) = 1. The negative denominators exercise the + // subtraction modulo r, and the halves exercise the modular inverse. #[test] - fn scalar_div_multiplies_by_modular_inverse() { - let quotient = scalar_div(&scalar_from_u64(42), &scalar_from_u64(6)).unwrap(); + fn compute_lagrange_coefficients_matches_hand_computed_values() { + let coeffs = compute_lagrange_coefficients(&[1, 2, 3]).unwrap(); - assert_eq!( - quotient.b, - scalar_from_u64(7).b, - "42 / 6 = 7 in the scalar field" - ); + assert_eq!(coeffs.len(), 3); + assert_eq!(fr_be_bytes(&coeffs[0]), be_bytes(3), "λ₁ = 3"); + assert_eq!(fr_be_bytes(&coeffs[1]), R_MINUS_3, "λ₂ = −3 = r − 3"); + assert_eq!(fr_be_bytes(&coeffs[2]), be_bytes(1), "λ₃ = 1"); } - // 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" - ); + fn compute_lagrange_coefficients_rejects_duplicate_indices() { + assert!(matches!( + compute_lagrange_coefficients(&[1, 2, 2]), + Err(Error::IndicesNotUnique) + )); } }