diff --git a/.changelog/5897.internal.md b/.changelog/5897.internal.md new file mode 100644 index 00000000000..5601ecc461d --- /dev/null +++ b/.changelog/5897.internal.md @@ -0,0 +1,7 @@ +secret-sharing/src/churp/switch: Verify combined bivariate polynomial + +After all bivariate shares are collected and the switch either +creates a new shareholder or proactivates the share of an existing +one, the new share should be verified to ensure that the verification +matrix of the combined bivariate polynomial satisfies the non-zero +leading term requirements. diff --git a/secret-sharing/src/churp/errors.rs b/secret-sharing/src/churp/errors.rs index 96857c61c5e..6e4d4a9aa07 100644 --- a/secret-sharing/src/churp/errors.rs +++ b/secret-sharing/src/churp/errors.rs @@ -6,6 +6,8 @@ pub enum Error { InvalidKind, #[error("invalid polynomial")] InvalidPolynomial, + #[error("insecure bivariate polynomial")] + InsecureBivariatePolynomial, #[error("invalid switch point")] InvalidSwitchPoint, #[error("invalid state")] @@ -22,6 +24,8 @@ pub enum Error { PolynomialDegreeMismatch, #[error("shareholder encoding failed")] ShareholderEncodingFailed, + #[error("shareholder proactivization already completed")] + ShareholderProactivizationCompleted, #[error("shareholder identity mismatch")] ShareholderIdentityMismatch, #[error("shareholder identity required")] diff --git a/secret-sharing/src/churp/shareholder.rs b/secret-sharing/src/churp/shareholder.rs index 00958c96f8f..7dd1d685136 100644 --- a/secret-sharing/src/churp/shareholder.rs +++ b/secret-sharing/src/churp/shareholder.rs @@ -190,6 +190,30 @@ where return Err(Error::VerificationMatrixZeroHoleMismatch.into()); } + // Verify that the bivariate polynomial `B(x, y)`, from which + // the verification matrix was constructed and the share was derived, + // satisfies the non-zero leading term requirements. Specifically, + // the polynomials `B(x, y)`, `B(x, 0)`, and `B(0, y)` must have + // non-zero leading terms. + if threshold > 0 { + // Skipping the special case where the bivariate polynomial has only + // one coefficient. The verification of this coefficient has already + // been done above, when we checked if the verification matrix + // is zero-hole. + let i = rows - 1; + let j = cols - 1; + + if self.vm.element(i, j).unwrap().is_identity().into() { + return Err(Error::InsecureBivariatePolynomial.into()); + } + if self.vm.element(i, 0).unwrap().is_identity().into() { + return Err(Error::InsecureBivariatePolynomial.into()); + } + if self.vm.element(0, j).unwrap().is_identity().into() { + return Err(Error::InsecureBivariatePolynomial.into()); + } + } + Ok(()) } diff --git a/secret-sharing/src/churp/switch.rs b/secret-sharing/src/churp/switch.rs index bd4c8e90c93..52691631cf4 100644 --- a/secret-sharing/src/churp/switch.rs +++ b/secret-sharing/src/churp/switch.rs @@ -552,10 +552,14 @@ where return Err(Error::NotEnoughBivariateShares.into()); } - // The values cannot be empty since the constructor verifies that - // the number of shareholders is greater than zero. - let p = self.p.take().unwrap(); - let vm = self.vm.take().unwrap(); + let p = self + .p + .take() + .ok_or(Error::ShareholderProactivizationCompleted)?; + let vm = self + .vm + .take() + .ok_or(Error::ShareholderProactivizationCompleted)?; let shareholder = match &self.shareholder { Some(shareholder) => shareholder.proactivize(&p, &vm)?, @@ -565,6 +569,12 @@ where } }; + // Ensure that the combined bivariate polynomial satisfies + // the non-zero leading term requirements. + shareholder + .verifiable_share() + .verify(self.threshold, false, self.full_share)?; + Ok(shareholder) } } @@ -733,7 +743,6 @@ mod tests { #[test] fn test_bivariate_shares() { let threshold = 2; - let zero_hole = true; let me = prepare_shareholder(1); let shareholders = prepare_shareholders(&[1, 2, 3]); @@ -750,78 +759,84 @@ mod tests { // Happy path. for full_share in vec![false, true] { - let mut bs = BivariateShares::::new( - threshold, - zero_hole, - full_share, - me, - shareholders.clone(), - None, - ) - .unwrap(); + for zero_hole in vec![false, true] { + let mut bs = BivariateShares::::new( + threshold, + zero_hole, + full_share, + me, + shareholders.clone(), + None, + ) + .unwrap(); + + let me = 1; + let mut sh = 2; + + // Add invalid share (invalid threshold). + let res = + add_bivariate_shares(threshold + 1, zero_hole, full_share, me, me, &mut bs); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + Error::VerificationMatrixDimensionMismatch.to_string() + ); - let me = 1; - let mut sh = 2; + // Add share. + let res = add_bivariate_shares(threshold, zero_hole, full_share, me, me, &mut bs); + assert!(res.is_ok()); + assert!(!res.unwrap()); - // Add invalid share (invalid threshold). - let res = add_bivariate_shares(threshold + 1, zero_hole, full_share, me, me, &mut bs); - assert!(res.is_err()); - assert_eq!( - res.unwrap_err().to_string(), - Error::VerificationMatrixDimensionMismatch.to_string() - ); + // Add another share twice. + assert!(bs.needs_bivariate_share(&prepare_shareholder(sh))); - // Add share. - let res = add_bivariate_shares(threshold, zero_hole, full_share, me, me, &mut bs); - assert!(res.is_ok()); - assert!(!res.unwrap()); + let res = add_bivariate_shares(threshold, zero_hole, full_share, me, sh, &mut bs); + assert!(res.is_ok()); + assert!(!res.unwrap()); - // Add another share twice. - assert!(bs.needs_bivariate_share(&prepare_shareholder(sh))); + let res = add_bivariate_shares(threshold, zero_hole, full_share, me, sh, &mut bs); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + Error::DuplicateShareholder.to_string() + ); - let res = add_bivariate_shares(threshold, zero_hole, full_share, me, sh, &mut bs); - assert!(res.is_ok()); - assert!(!res.unwrap()); + assert!(!bs.needs_bivariate_share(&prepare_shareholder(sh))); + sh += 1; + + // Try to collect the polynomial and verification matrix. + let res = bs.proactivize_shareholder(); + assert!(res.is_err()); + unsafe { + assert_eq!( + res.unwrap_err_unchecked().to_string(), + Error::NotEnoughBivariateShares.to_string() + ); + } - let res = add_bivariate_shares(threshold, zero_hole, full_share, me, sh, &mut bs); - assert!(res.is_err()); - assert_eq!( - res.unwrap_err().to_string(), - Error::DuplicateShareholder.to_string() - ); + // Add the last share. + let res = add_bivariate_shares(threshold, zero_hole, full_share, me, sh, &mut bs); + assert!(res.is_ok()); + assert!(res.unwrap()); // Enough shares. + sh += 1; - assert!(!bs.needs_bivariate_share(&prepare_shareholder(sh))); - sh += 1; + // Unknown shareholder. + assert!(!bs.needs_bivariate_share(&prepare_shareholder(sh))); - // Try to collect the polynomial and verification matrix. - let res = bs.proactivize_shareholder(); - assert!(res.is_err()); - unsafe { + let res = add_bivariate_shares(threshold, zero_hole, full_share, me, sh, &mut bs); + assert!(res.is_err()); assert_eq!( - res.unwrap_err_unchecked().to_string(), - Error::NotEnoughBivariateShares.to_string() + res.unwrap_err().to_string(), + Error::UnknownShareholder.to_string() ); - } - - // Add the last share. - let res = add_bivariate_shares(threshold, zero_hole, full_share, me, sh, &mut bs); - assert!(res.is_ok()); - assert!(res.unwrap()); // Enough shares. - sh += 1; - // Unknown shareholder. - assert!(!bs.needs_bivariate_share(&prepare_shareholder(sh))); - - let res = add_bivariate_shares(threshold, zero_hole, full_share, me, sh, &mut bs); - assert!(res.is_err()); - assert_eq!( - res.unwrap_err().to_string(), - Error::UnknownShareholder.to_string() - ); - - // Try to collect the polynomial and verification matrix again. - let res = bs.proactivize_shareholder(); - assert!(res.is_ok()); + // Try to collect the polynomial and verification matrix again. + let res = bs.proactivize_shareholder(); + match zero_hole { + true => assert!(res.is_err()), // The combined polynomial has zero secret (not allowed). + false => assert!(res.is_ok()), // The combined polynomial has non-zero secret. + } + } } } }