From ca3e8af859e057177554ffe9b82ad925b0dea703 Mon Sep 17 00:00:00 2001 From: Peter Nose Date: Fri, 10 May 2024 23:38:54 +0200 Subject: [PATCH 1/2] go/keymanager/churp: Limit threshold Limiting the threshold ensures that the dimensions of bivariate polynomials (t, 2t) never exceed the range of uint8. --- go/keymanager/churp/requests.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/go/keymanager/churp/requests.go b/go/keymanager/churp/requests.go index 4c189c46861..5516710bfee 100644 --- a/go/keymanager/churp/requests.go +++ b/go/keymanager/churp/requests.go @@ -10,6 +10,12 @@ import ( "github.com/oasisprotocol/oasis-core/go/common/crypto/signature" ) +// maxThreshold is the maximum threshold. +// +// Limiting the threshold ensures that the dimensions of bivariate polynomials +// (t, 2t) never exceed the range of uint8. +const maxThreshold = 127 + var ( // ApplicationRequestSignatureContext is the signature context used to sign // application requests with runtime signing key (RAK). @@ -50,6 +56,9 @@ func (c *CreateRequest) ValidateBasic() error { if c.SuiteID > 0 { return fmt.Errorf("unsupported suite, ID %d", c.SuiteID) } + if c.Threshold > maxThreshold { + return fmt.Errorf("threshold too large: got %d, max %d", c.Threshold, maxThreshold) + } if c.Policy.Policy.ID != c.ID { return fmt.Errorf("policy ID mismatch: got %d, expected %d", c.Policy.Policy.ID, c.ID) } From ddf51345a9a468dd6ec72b5292df9c8904cd81c9 Mon Sep 17 00:00:00 2001 From: Peter Nose Date: Sat, 11 May 2024 10:37:49 +0200 Subject: [PATCH 2/2] secret-sharing/src/churp: Fix threshold overflow --- .changelog/5690.trivial.md | 0 keymanager/src/churp/handler.rs | 2 +- secret-sharing/src/churp/dealer.rs | 17 +++++++++-------- secret-sharing/src/churp/errors.rs | 2 ++ secret-sharing/src/churp/handoff.rs | 2 +- 5 files changed, 13 insertions(+), 10 deletions(-) create mode 100644 .changelog/5690.trivial.md diff --git a/.changelog/5690.trivial.md b/.changelog/5690.trivial.md new file mode 100644 index 00000000000..e69de29bb2d diff --git a/keymanager/src/churp/handler.rs b/keymanager/src/churp/handler.rs index a9795ae73dd..fa89ec05179 100644 --- a/keymanager/src/churp/handler.rs +++ b/keymanager/src/churp/handler.rs @@ -1038,7 +1038,7 @@ impl Churp { // will detect the polynomial change because the checksum // of the verification matrix in the submitted application // will also change. - let dealer = Dealer::new(threshold, dealing_phase, &mut OsRng); + let dealer = Dealer::new(threshold, dealing_phase, &mut OsRng)?; // Encrypt and store the polynomial in case of a restart. let polynomial = dealer.bivariate_polynomial(); diff --git a/secret-sharing/src/churp/dealer.rs b/secret-sharing/src/churp/dealer.rs index bb2879e96cf..0f03e89cec3 100644 --- a/secret-sharing/src/churp/dealer.rs +++ b/secret-sharing/src/churp/dealer.rs @@ -1,7 +1,6 @@ //! CHURP dealer. use anyhow::Result; - use rand_core::RngCore; use crate::{ @@ -12,7 +11,7 @@ use crate::{ }, }; -use super::{HandoffKind, ShareholderId}; +use super::{Error, HandoffKind, ShareholderId}; /// Dealer is responsible for generating a secret bivariate polynomial, /// computing a verification matrix, and deriving secret shares for other @@ -35,14 +34,16 @@ where S: Suite, { /// Creates a new dealer. - pub fn new(threshold: u8, dealing_phase: bool, rng: &mut impl RngCore) -> Self { + pub fn new(threshold: u8, dealing_phase: bool, rng: &mut impl RngCore) -> Result { let dx = threshold; - let dy = 2 * threshold; + let dy = threshold.checked_mul(2).ok_or(Error::ThresholdTooLarge)?; - match dealing_phase { + let dealer = match dealing_phase { true => Dealer::random(dx, dy, rng), false => Dealer::zero_hole(dx, dy, rng), - } + }; + + Ok(dealer) } /// Creates a new dealer with a random bivariate polynomial. @@ -113,7 +114,7 @@ mod tests { let threshold = 2; for dealing_phase in vec![true, false] { - let dealer = Dealer::new(threshold, dealing_phase, &mut rng); + let dealer = Dealer::new(threshold, dealing_phase, &mut rng).unwrap(); assert_eq!(dealer.verification_matrix().is_zero_hole(), !dealing_phase); assert_eq!(dealer.bivariate_polynomial().deg_x, 2); assert_eq!(dealer.bivariate_polynomial().deg_y, 4); @@ -123,7 +124,7 @@ mod tests { let threshold = 0; for dealing_phase in vec![true, false] { - let dealer = Dealer::new(threshold, dealing_phase, &mut rng); + let dealer = Dealer::new(threshold, dealing_phase, &mut rng).unwrap(); assert_eq!(dealer.verification_matrix().is_zero_hole(), !dealing_phase); assert_eq!(dealer.bivariate_polynomial().deg_x, 0); assert_eq!(dealer.bivariate_polynomial().deg_y, 0); diff --git a/secret-sharing/src/churp/errors.rs b/secret-sharing/src/churp/errors.rs index 4ce7279b15c..f75b0b21bf6 100644 --- a/secret-sharing/src/churp/errors.rs +++ b/secret-sharing/src/churp/errors.rs @@ -22,6 +22,8 @@ pub enum Error { PolynomialDegreeMismatch, #[error("shareholder encoding failed")] ShareholderEncodingFailed, + #[error("threshold too large")] + ThresholdTooLarge, #[error("too many switch points")] TooManySwitchPoints, #[error("unknown shareholder")] diff --git a/secret-sharing/src/churp/handoff.rs b/secret-sharing/src/churp/handoff.rs index 06811951fed..e61b7dacad1 100644 --- a/secret-sharing/src/churp/handoff.rs +++ b/secret-sharing/src/churp/handoff.rs @@ -383,7 +383,7 @@ mod tests { ) -> HashMap { let mut dealers = HashMap::new(); for sh in committee.iter() { - let d = Dealer::new(threshold, dealing_phase, rng); + let d = Dealer::new(threshold, dealing_phase, rng).unwrap(); dealers.insert(sh.clone(), d); } dealers