From 73d9c87df176c47bc387b9f69ab49b002e74ae0c Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Fri, 22 Mar 2024 15:14:18 -0500 Subject: [PATCH 1/4] Remove partial precomputation --- Cargo.toml | 2 +- clippy.toml | 2 +- src/range_proof.rs | 29 ++++++++++++++++++++++++++--- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6bb803a..9b1c2e6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,7 @@ description = "A smaller faster implementation of Bulletproofs" [dependencies] blake2 = { version = "0.10", default-features = false } byteorder = { version = "1", default-features = false } -curve25519-dalek = { package = "tari-curve25519-dalek", version = "4.0.3", default-features = false, features = ["alloc", "rand_core", "serde", "zeroize"] } +curve25519-dalek = { version = "4.1", default-features = false, features = ["alloc", "group", "rand_core", "serde", "zeroize"] } digest = { version = "0.10", default-features = false, features = ["alloc"] } itertools = { version = "0.12", default-features = false, features = ["use_alloc"] } merlin = { version = "3", default-features = false } diff --git a/clippy.toml b/clippy.toml index 49cd31c..4330908 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1 @@ -arithmetic-side-effects-allowed = [ "tari_curve25519_dalek::Scalar" ] +arithmetic-side-effects-allowed = [ "curve25519_dalek::Scalar" ] diff --git a/src/range_proof.rs b/src/range_proof.rs index 337619d..736cf3b 100644 --- a/src/range_proof.rs +++ b/src/range_proof.rs @@ -8,7 +8,7 @@ use alloc::{string::ToString, vec, vec::Vec}; use core::{ convert::{TryFrom, TryInto}, - iter::once, + iter::{once, repeat}, marker::PhantomData, ops::{Add, Mul, Shr}, slice::ChunksExact, @@ -330,8 +330,19 @@ where Scalar::random_not_zero(range_proof_transcript.as_mut_rng()) }); } + let padding = 2usize + .checked_mul(statement.generators.bit_length()) + .ok_or(ProofError::SizeOverflow)? + .checked_mul(statement.generators.aggregation_factor()) + .ok_or(ProofError::SizeOverflow)? + .checked_sub(a_li.len()) + .ok_or(ProofError::SizeOverflow)? + .checked_sub(a_ri.len()) + .ok_or(ProofError::SizeOverflow)?; let a = statement.generators.precomp().vartime_mixed_multiscalar_mul( - a_li.iter().interleave(a_ri.iter()), + a_li.iter() + .interleave(a_ri.iter()) + .chain(repeat(&Scalar::ZERO).take(padding)), alpha.iter(), statement.generators.g_bases().iter(), ); @@ -1023,8 +1034,20 @@ where dynamic_points.push(h_base.clone()); // Perform the final check using precomputation + let padding = 2usize + .checked_mul(max_statement.generators.bit_length()) + .ok_or(ProofError::SizeOverflow)? + .checked_mul(max_statement.generators.aggregation_factor()) + .ok_or(ProofError::SizeOverflow)? + .checked_sub(max_mn) + .ok_or(ProofError::SizeOverflow)? + .checked_sub(max_mn) + .ok_or(ProofError::SizeOverflow)?; if precomp.vartime_mixed_multiscalar_mul( - gi_base_scalars.iter().interleave(hi_base_scalars.iter()), + gi_base_scalars + .iter() + .interleave(hi_base_scalars.iter()) + .chain(repeat(&Scalar::ZERO).take(padding)), dynamic_scalars.iter(), dynamic_points.iter(), ) != P::identity() From c0af26954bdf9e15af54c4dee6e78fe1ee4f7ace Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Mon, 25 Mar 2024 18:08:39 -0500 Subject: [PATCH 2/4] Scalar exponentiation --- Cargo.toml | 3 ++- src/range_proof.rs | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9b1c2e6..4f39bc1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ blake2 = { version = "0.10", default-features = false } byteorder = { version = "1", default-features = false } curve25519-dalek = { version = "4.1", default-features = false, features = ["alloc", "group", "rand_core", "serde", "zeroize"] } digest = { version = "0.10", default-features = false, features = ["alloc"] } +ff = { version = "0.13.0", default-features = false } itertools = { version = "0.12", default-features = false, features = ["use_alloc"] } merlin = { version = "3", default-features = false } once_cell = { version = "1", default-features = false, features = ["alloc", "critical-section"] } @@ -27,7 +28,7 @@ rand_chacha = { version = "0.3.1", default-features = false } [features] default = ["rand", "std"] -std = ["blake2/std", "byteorder/std", "digest/std", "itertools/use_std", "merlin/std", "once_cell/std", "rand_core/std", "serde/std", "sha3/std", "zeroize/std"] +std = ["blake2/std", "byteorder/std", "digest/std", "ff/std", "itertools/use_std", "merlin/std", "once_cell/std", "rand_core/std", "serde/std", "sha3/std", "zeroize/std"] rand = ["rand_core/getrandom"] [[bench]] diff --git a/src/range_proof.rs b/src/range_proof.rs index 736cf3b..a281fb4 100644 --- a/src/range_proof.rs +++ b/src/range_proof.rs @@ -18,6 +18,7 @@ use curve25519_dalek::{ scalar::Scalar, traits::{Identity, IsIdentity, MultiscalarMul, VartimePrecomputedMultiscalarMul}, }; +use ff::Field; use itertools::{izip, Itertools}; use merlin::Transcript; use rand_core::CryptoRngCore; @@ -774,7 +775,7 @@ where // Compute 2**n-1 for later use let two = Scalar::from(2u8); - let two_n_minus_one = (0..bit_length.ilog2()).fold(two, |acc, _| acc * acc) - Scalar::ONE; + let two_n_minus_one = two.pow_vartime([bit_length as u64]) - Scalar::ONE; // Weighted coefficients for common generators let mut g_base_scalars = vec![Scalar::ZERO; extension_degree]; @@ -901,7 +902,7 @@ where let e_square = e * e; let challenges_sq: Vec = challenges.iter().map(|c| c * c).collect(); let challenges_sq_inv: Vec = challenges_inv.iter().map(|c| c * c).collect(); - let y_nm = (0..rounds).fold(y, |y_nm, _| y_nm * y_nm); + let y_nm = y.pow_vartime([full_length as u64]); let y_nm_1 = y_nm * y; // Compute the sum of powers of the challenge as a partial sum of a geometric series From 001add607347aa8b8392cef2f2b725b178437bf4 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Tue, 26 Mar 2024 10:28:26 -0500 Subject: [PATCH 3/4] Refactor padding --- src/range_proof.rs | 30 +++++++++++------------------- src/utils/generic.rs | 40 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/src/range_proof.rs b/src/range_proof.rs index a281fb4..15072d9 100644 --- a/src/range_proof.rs +++ b/src/range_proof.rs @@ -37,7 +37,7 @@ use crate::{ traits::{Compressable, Decompressable, FixedBytesRepr, Precomputable}, transcripts::RangeProofTranscript, utils::{ - generic::{nonce, split_at_checked}, + generic::{compute_generator_padding, nonce, split_at_checked}, nullrng::NullRng, }, }; @@ -331,15 +331,11 @@ where Scalar::random_not_zero(range_proof_transcript.as_mut_rng()) }); } - let padding = 2usize - .checked_mul(statement.generators.bit_length()) - .ok_or(ProofError::SizeOverflow)? - .checked_mul(statement.generators.aggregation_factor()) - .ok_or(ProofError::SizeOverflow)? - .checked_sub(a_li.len()) - .ok_or(ProofError::SizeOverflow)? - .checked_sub(a_ri.len()) - .ok_or(ProofError::SizeOverflow)?; + let padding = compute_generator_padding( + statement.generators.bit_length(), + statement.commitments.len(), + statement.generators.aggregation_factor(), + )?; let a = statement.generators.precomp().vartime_mixed_multiscalar_mul( a_li.iter() .interleave(a_ri.iter()) @@ -1035,15 +1031,11 @@ where dynamic_points.push(h_base.clone()); // Perform the final check using precomputation - let padding = 2usize - .checked_mul(max_statement.generators.bit_length()) - .ok_or(ProofError::SizeOverflow)? - .checked_mul(max_statement.generators.aggregation_factor()) - .ok_or(ProofError::SizeOverflow)? - .checked_sub(max_mn) - .ok_or(ProofError::SizeOverflow)? - .checked_sub(max_mn) - .ok_or(ProofError::SizeOverflow)?; + let padding = compute_generator_padding( + max_statement.generators.bit_length(), + max_statement.commitments.len(), + max_statement.generators.aggregation_factor(), + )?; if precomp.vartime_mixed_multiscalar_mul( gi_base_scalars .iter() diff --git a/src/utils/generic.rs b/src/utils/generic.rs index 2a9d257..b02eb65 100644 --- a/src/utils/generic.rs +++ b/src/utils/generic.rs @@ -68,6 +68,28 @@ pub fn split_at_checked(vec: &[T], n: usize) -> Result<(&[T], &[T]), ProofErr } } +/// Compute the padding needed for generator vectors +pub fn compute_generator_padding( + bit_length: usize, + aggregation_factor: usize, + max_aggregation_factor: usize, +) -> Result { + let padded_capacity = 2usize + .checked_mul(bit_length) + .ok_or(ProofError::SizeOverflow)? + .checked_mul(max_aggregation_factor) + .ok_or(ProofError::SizeOverflow)?; + let actual_capacity = 2usize + .checked_mul(bit_length) + .ok_or(ProofError::SizeOverflow)? + .checked_mul(aggregation_factor) + .ok_or(ProofError::SizeOverflow)?; + + padded_capacity + .checked_sub(actual_capacity) + .ok_or(ProofError::SizeOverflow) +} + #[cfg(test)] mod tests { use alloc::{vec, vec::Vec}; @@ -76,10 +98,20 @@ mod tests { use rand_chacha::ChaCha12Rng; use rand_core::SeedableRng; - use crate::{ - protocols::scalar_protocol::ScalarProtocol, - utils::generic::{nonce, split_at_checked, BLAKE2B_PERSONA_LIMIT}, - }; + use crate::{protocols::scalar_protocol::ScalarProtocol, utils::generic::*}; + + #[test] + fn test_padding() { + // No padding + assert_eq!(compute_generator_padding(64, 1, 1).unwrap(), 0); + + // Padding + assert_eq!(compute_generator_padding(64, 1, 2).unwrap(), 128); + + // Invalid + assert!(compute_generator_padding(64, 2, 1).is_err()); + assert!(compute_generator_padding(64, usize::MAX - 1, usize::MAX).is_err()); + } #[test] fn test_split() { From 1be0eb1bb3fc84991055668913afe2208417194f Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Wed, 27 Mar 2024 11:00:16 -0500 Subject: [PATCH 4/4] Update audit report --- docs/quarkslab-audit/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/quarkslab-audit/README.md b/docs/quarkslab-audit/README.md index fa8184e..6369156 100644 --- a/docs/quarkslab-audit/README.md +++ b/docs/quarkslab-audit/README.md @@ -41,6 +41,9 @@ Developers are in the process of testing an updated version of the fork, in orde Further, an open [upstream pull request](https://github.com/dalek-cryptography/curve25519-dalek/pull/546) would add partial precomputation support; if it is accepted, the implementation could change its curve library dependency to upstream. +*Update*: The library has removed support for partial precomputation in favor of a different design. +As a result, the curve library dependency has been changed to upstream. + ## `INFO 2` This issue ran the `clippy` linter against several lints, and identified a number of warnings arising from them.