From b1913a9ca730efa9e34c204a2a6ccb3ffd2de5da Mon Sep 17 00:00:00 2001 From: Pratyush Mishra Date: Wed, 18 Mar 2020 22:29:16 -0700 Subject: [PATCH] Change default `to_bits` and `to_bytes` functions to the safe versions --- .../src/nizk/gm17/constraints.rs | 7 -- .../src/nizk/groth16/constraints.rs | 7 -- .../src/prf/blake2s/constraints.rs | 8 -- .../src/signature/schnorr/constraints.rs | 8 -- r1cs-std/src/bits/boolean.rs | 9 +-- r1cs-std/src/bits/mod.rs | 78 ++++++------------- r1cs-std/src/bits/uint32.rs | 7 -- r1cs-std/src/bits/uint64.rs | 7 -- r1cs-std/src/fields/fp.rs | 57 +++++++------- r1cs-std/src/fields/fp12.rs | 12 +-- r1cs-std/src/fields/fp2.rs | 12 +-- r1cs-std/src/fields/fp6_3over2.rs | 21 +++-- r1cs-std/src/fields/mod.rs | 4 +- .../curves/short_weierstrass/bls12/mod.rs | 19 +++-- .../groups/curves/short_weierstrass/mod.rs | 16 ++-- .../src/groups/curves/twisted_edwards/mod.rs | 16 ++-- r1cs-std/src/groups/mod.rs | 6 +- 17 files changed, 118 insertions(+), 176 deletions(-) diff --git a/crypto-primitives/src/nizk/gm17/constraints.rs b/crypto-primitives/src/nizk/gm17/constraints.rs index 529df85..69d8db9 100644 --- a/crypto-primitives/src/nizk/gm17/constraints.rs +++ b/crypto-primitives/src/nizk/gm17/constraints.rs @@ -388,13 +388,6 @@ where } Ok(bytes) } - - fn to_bytes_strict>( - &self, - cs: CS, - ) -> Result, SynthesisError> { - self.to_bytes(cs) - } } #[cfg(test)] diff --git a/crypto-primitives/src/nizk/groth16/constraints.rs b/crypto-primitives/src/nizk/groth16/constraints.rs index 85aecdf..b4ff914 100644 --- a/crypto-primitives/src/nizk/groth16/constraints.rs +++ b/crypto-primitives/src/nizk/groth16/constraints.rs @@ -335,13 +335,6 @@ where } Ok(bytes) } - - fn to_bytes_strict>( - &self, - cs: CS, - ) -> Result, SynthesisError> { - self.to_bytes(cs) - } } #[cfg(test)] diff --git a/crypto-primitives/src/prf/blake2s/constraints.rs b/crypto-primitives/src/prf/blake2s/constraints.rs index e3d96a9..2d779f0 100644 --- a/crypto-primitives/src/prf/blake2s/constraints.rs +++ b/crypto-primitives/src/prf/blake2s/constraints.rs @@ -447,14 +447,6 @@ impl ToBytesGadget for Blake2sOutputGadget ) -> Result, SynthesisError> { Ok(self.0.clone()) } - - #[inline] - fn to_bytes_strict>( - &self, - cs: CS, - ) -> Result, SynthesisError> { - self.to_bytes(cs) - } } impl AllocGadget<[u8; 32], ConstraintF> for Blake2sOutputGadget { diff --git a/crypto-primitives/src/signature/schnorr/constraints.rs b/crypto-primitives/src/signature/schnorr/constraints.rs index b6b8bce..558fcd1 100644 --- a/crypto-primitives/src/signature/schnorr/constraints.rs +++ b/crypto-primitives/src/signature/schnorr/constraints.rs @@ -208,12 +208,4 @@ where ) -> Result, SynthesisError> { self.pub_key.to_bytes(&mut cs.ns(|| "PubKey To Bytes")) } - - fn to_bytes_strict>( - &self, - mut cs: CS, - ) -> Result, SynthesisError> { - self.pub_key - .to_bytes_strict(&mut cs.ns(|| "PubKey To Bytes")) - } } diff --git a/r1cs-std/src/bits/boolean.rs b/r1cs-std/src/bits/boolean.rs index a799d56..22d2c1c 100644 --- a/r1cs-std/src/bits/boolean.rs +++ b/r1cs-std/src/bits/boolean.rs @@ -802,6 +802,7 @@ impl ConditionalEqGadget for Boolean { } impl ToBytesGadget for Boolean { + /// Outputs `1u8` if `self` is true, and `0u8` otherwise. fn to_bytes>( &self, _cs: CS, @@ -813,14 +814,6 @@ impl ToBytesGadget for Boolean { let byte = UInt8 { bits, value }; Ok(vec![byte]) } - - /// Additionally checks if the produced list of booleans is 'valid'. - fn to_bytes_strict>( - &self, - cs: CS, - ) -> Result, SynthesisError> { - self.to_bytes(cs) - } } impl CondSelectGadget for Boolean { diff --git a/r1cs-std/src/bits/mod.rs b/r1cs-std/src/bits/mod.rs index 7f513ce..3829d8b 100644 --- a/r1cs-std/src/bits/mod.rs +++ b/r1cs-std/src/bits/mod.rs @@ -11,16 +11,24 @@ pub mod uint64; pub mod uint8; pub trait ToBitsGadget { + /// Outputs the canonical bit-wise representation of `self`. + /// + /// This is the correct default for 99% of use cases. fn to_bits>( &self, cs: CS, ) -> Result, SynthesisError>; - /// Additionally checks if the produced list of booleans is 'valid'. - fn to_bits_strict>( + /// Outputs a possibly non-unique bit-wise representation of `self`. + /// + /// If you're not absolutely certain that your usecase can get away with a + /// non-canonical representation, please use `self.to_bits(cs)` instead. + fn to_non_unique_bits>( &self, cs: CS, - ) -> Result, SynthesisError>; + ) -> Result, SynthesisError> { + self.to_bits(cs) + } } impl ToBitsGadget for Boolean { @@ -30,13 +38,6 @@ impl ToBitsGadget for Boolean { ) -> Result, SynthesisError> { Ok(vec![self.clone()]) } - - fn to_bits_strict>( - &self, - _: CS, - ) -> Result, SynthesisError> { - Ok(vec![self.clone()]) - } } impl ToBitsGadget for [Boolean] { @@ -46,14 +47,8 @@ impl ToBitsGadget for [Boolean] { ) -> Result, SynthesisError> { Ok(self.to_vec()) } - - fn to_bits_strict>( - &self, - _cs: CS, - ) -> Result, SynthesisError> { - Ok(self.to_vec()) - } } + impl ToBitsGadget for Vec { fn to_bits>( &self, @@ -61,13 +56,6 @@ impl ToBitsGadget for Vec { ) -> Result, SynthesisError> { Ok(self.clone()) } - - fn to_bits_strict>( - &self, - _cs: CS, - ) -> Result, SynthesisError> { - Ok(self.clone()) - } } impl ToBitsGadget for [UInt8] { @@ -81,26 +69,27 @@ impl ToBitsGadget for [UInt8] { } Ok(result) } - - fn to_bits_strict>( - &self, - cs: CS, - ) -> Result, SynthesisError> { - self.to_bits(cs) - } } pub trait ToBytesGadget { + /// Outputs a canonical byte-wise representation of `self`. + /// + /// This is the correct default for 99% of use cases. fn to_bytes>( &self, cs: CS, ) -> Result, SynthesisError>; - /// Additionally checks if the produced list of booleans is 'valid'. - fn to_bytes_strict>( + /// Outputs a possibly non-unique byte decomposition of `self`. + /// + /// If you're not absolutely certain that your usecase can get away with a + /// non-canonical representation, please use `self.to_bytes(cs)` instead. + fn to_non_unique_bytes>( &self, cs: CS, - ) -> Result, SynthesisError>; + ) -> Result, SynthesisError> { + self.to_bytes(cs) + } } impl ToBytesGadget for [UInt8] { @@ -110,13 +99,6 @@ impl ToBytesGadget for [UInt8] { ) -> Result, SynthesisError> { Ok(self.to_vec()) } - - fn to_bytes_strict>( - &self, - cs: CS, - ) -> Result, SynthesisError> { - self.to_bytes(cs) - } } impl<'a, ConstraintF: Field, T: 'a + ToBytesGadget> ToBytesGadget @@ -128,13 +110,6 @@ impl<'a, ConstraintF: Field, T: 'a + ToBytesGadget> ToBytesGadget Result, SynthesisError> { (*self).to_bytes(cs) } - - fn to_bytes_strict>( - &self, - cs: CS, - ) -> Result, SynthesisError> { - self.to_bytes(cs) - } } impl<'a, ConstraintF: Field> ToBytesGadget for &'a [UInt8] { @@ -144,11 +119,4 @@ impl<'a, ConstraintF: Field> ToBytesGadget for &'a [UInt8] { ) -> Result, SynthesisError> { Ok(self.to_vec()) } - - fn to_bytes_strict>( - &self, - cs: CS, - ) -> Result, SynthesisError> { - self.to_bytes(cs) - } } diff --git a/r1cs-std/src/bits/uint32.rs b/r1cs-std/src/bits/uint32.rs index ec3a447..846d5a2 100644 --- a/r1cs-std/src/bits/uint32.rs +++ b/r1cs-std/src/bits/uint32.rs @@ -306,13 +306,6 @@ impl ToBytesGadget for UInt32 { Ok(bytes) } - - fn to_bytes_strict>( - &self, - cs: CS, - ) -> Result, SynthesisError> { - self.to_bytes(cs) - } } impl PartialEq for UInt32 { diff --git a/r1cs-std/src/bits/uint64.rs b/r1cs-std/src/bits/uint64.rs index ddbc519..50a168d 100644 --- a/r1cs-std/src/bits/uint64.rs +++ b/r1cs-std/src/bits/uint64.rs @@ -310,13 +310,6 @@ impl ToBytesGadget for UInt64 { Ok(bytes) } - - fn to_bytes_strict>( - &self, - cs: CS, - ) -> Result, SynthesisError> { - self.to_bytes(cs) - } } impl PartialEq for UInt64 { diff --git a/r1cs-std/src/fields/fp.rs b/r1cs-std/src/fields/fp.rs index 100eff5..999bc71 100644 --- a/r1cs-std/src/fields/fp.rs +++ b/r1cs-std/src/fields/fp.rs @@ -326,9 +326,18 @@ impl NEqGadget for FpGadget { } impl ToBitsGadget for FpGadget { - /// Outputs the binary representation of the value in `self` in *big-endian* + /// Outputs the unique bit-wise decomposition of `self` in *big-endian* /// form. fn to_bits>(&self, mut cs: CS) -> Result, SynthesisError> { + let bits = self.to_non_unique_bits(&mut cs)?; + Boolean::enforce_in_field::<_, _, F>(&mut cs, &bits)?; + Ok(bits) + } + + fn to_non_unique_bits>( + &self, + mut cs: CS, + ) -> Result, SynthesisError> { let num_bits = F::Params::MODULUS_BITS; use algebra::BitIterator; let bit_values = match self.value { @@ -375,20 +384,29 @@ impl ToBitsGadget for FpGadget { Ok(bits.into_iter().map(Boolean::from).collect()) } - - fn to_bits_strict>( - &self, - mut cs: CS, - ) -> Result, SynthesisError> { - let bits = self.to_bits(&mut cs)?; - Boolean::enforce_in_field::<_, _, F>(&mut cs, &bits)?; - - Ok(bits) - } } impl ToBytesGadget for FpGadget { + /// Outputs the unique byte decomposition of `self` in *little-endian* + /// form. fn to_bytes>(&self, mut cs: CS) -> Result, SynthesisError> { + let bytes = self.to_non_unique_bytes(&mut cs)?; + Boolean::enforce_in_field::<_, _, F>( + &mut cs, + &bytes.iter() + .flat_map(|byte_gadget| byte_gadget.into_bits_le()) + // This reverse maps the bits into big-endian form, as required by `enforce_in_field`. + .rev() + .collect::>(), + )?; + + Ok(bytes) + } + + fn to_non_unique_bytes>( + &self, + mut cs: CS, + ) -> Result, SynthesisError> { let byte_values = match self.value { Some(value) => to_bytes![&value.into_repr()]? .into_iter() @@ -425,23 +443,6 @@ impl ToBytesGadget for FpGadget { Ok(bytes) } - - fn to_bytes_strict>( - &self, - mut cs: CS, - ) -> Result, SynthesisError> { - let bytes = self.to_bytes(&mut cs)?; - Boolean::enforce_in_field::<_, _, F>( - &mut cs, - &bytes.iter() - .flat_map(|byte_gadget| byte_gadget.into_bits_le()) - // This reverse maps the bits into big-endian form, as required by `enforce_in_field`. - .rev() - .collect::>(), - )?; - - Ok(bytes) - } } impl CondSelectGadget for FpGadget { diff --git a/r1cs-std/src/fields/fp12.rs b/r1cs-std/src/fields/fp12.rs index 9588a6c..a02d1ac 100644 --- a/r1cs-std/src/fields/fp12.rs +++ b/r1cs-std/src/fields/fp12.rs @@ -731,12 +731,12 @@ where Ok(c0) } - fn to_bits_strict>( + fn to_non_unique_bits>( &self, mut cs: CS, ) -> Result, SynthesisError> { - let mut c0 = self.c0.to_bits_strict(cs.ns(|| "c0"))?; - let mut c1 = self.c1.to_bits_strict(cs.ns(|| "c1"))?; + let mut c0 = self.c0.to_non_unique_bits(cs.ns(|| "c0"))?; + let mut c1 = self.c1.to_non_unique_bits(cs.ns(|| "c1"))?; c0.append(&mut c1); Ok(c0) } @@ -757,12 +757,12 @@ where Ok(c0) } - fn to_bytes_strict>( + fn to_non_unique_bytes>( &self, mut cs: CS, ) -> Result, SynthesisError> { - let mut c0 = self.c0.to_bytes_strict(cs.ns(|| "c0"))?; - let mut c1 = self.c1.to_bytes_strict(cs.ns(|| "c1"))?; + let mut c0 = self.c0.to_non_unique_bytes(cs.ns(|| "c0"))?; + let mut c1 = self.c1.to_non_unique_bytes(cs.ns(|| "c1"))?; c0.append(&mut c1); Ok(c0) } diff --git a/r1cs-std/src/fields/fp2.rs b/r1cs-std/src/fields/fp2.rs index 6e7f606..1ba763b 100644 --- a/r1cs-std/src/fields/fp2.rs +++ b/r1cs-std/src/fields/fp2.rs @@ -527,12 +527,12 @@ impl, ConstraintF: PrimeField> ToBitsGadget>( + fn to_non_unique_bits>( &self, mut cs: CS, ) -> Result, SynthesisError> { - let mut c0 = self.c0.to_bits_strict(cs.ns(|| "c0"))?; - let mut c1 = self.c1.to_bits_strict(cs.ns(|| "c1"))?; + let mut c0 = self.c0.to_non_unique_bits(cs.ns(|| "c0"))?; + let mut c1 = self.c1.to_non_unique_bits(cs.ns(|| "c1"))?; c0.append(&mut c1); Ok(c0) } @@ -551,12 +551,12 @@ impl, ConstraintF: PrimeField> ToBytesGadget< Ok(c0) } - fn to_bytes_strict>( + fn to_non_unique_bytes>( &self, mut cs: CS, ) -> Result, SynthesisError> { - let mut c0 = self.c0.to_bytes_strict(cs.ns(|| "c0"))?; - let mut c1 = self.c1.to_bytes_strict(cs.ns(|| "c1"))?; + let mut c0 = self.c0.to_non_unique_bytes(cs.ns(|| "c0"))?; + let mut c1 = self.c1.to_non_unique_bytes(cs.ns(|| "c1"))?; c0.append(&mut c1); Ok(c0) } diff --git a/r1cs-std/src/fields/fp6_3over2.rs b/r1cs-std/src/fields/fp6_3over2.rs index 6b8e2a7..f7661f4 100644 --- a/r1cs-std/src/fields/fp6_3over2.rs +++ b/r1cs-std/src/fields/fp6_3over2.rs @@ -800,13 +800,13 @@ where Ok(c0) } - fn to_bits_strict>( + fn to_non_unique_bits>( &self, mut cs: CS, ) -> Result, SynthesisError> { - let mut c0 = self.c0.to_bits_strict(cs.ns(|| "c0"))?; - let mut c1 = self.c1.to_bits_strict(cs.ns(|| "c1"))?; - let mut c2 = self.c2.to_bits_strict(cs.ns(|| "c2"))?; + let mut c0 = self.c0.to_non_unique_bits(cs.ns(|| "c0"))?; + let mut c1 = self.c1.to_non_unique_bits(cs.ns(|| "c1"))?; + let mut c2 = self.c2.to_non_unique_bits(cs.ns(|| "c2"))?; c0.append(&mut c1); c0.append(&mut c2); @@ -834,11 +834,18 @@ where Ok(c0) } - fn to_bytes_strict>( + fn to_non_unique_bytes>( &self, - cs: CS, + mut cs: CS, ) -> Result, SynthesisError> { - self.to_bytes(cs) + let mut c0 = self.c0.to_non_unique_bytes(cs.ns(|| "c0"))?; + let mut c1 = self.c1.to_non_unique_bytes(cs.ns(|| "c1"))?; + let mut c2 = self.c2.to_non_unique_bytes(cs.ns(|| "c2"))?; + + c0.append(&mut c1); + c0.append(&mut c2); + + Ok(c0) } } diff --git a/r1cs-std/src/fields/mod.rs b/r1cs-std/src/fields/mod.rs index 1af32ad..3dad571 100644 --- a/r1cs-std/src/fields/mod.rs +++ b/r1cs-std/src/fields/mod.rs @@ -452,7 +452,9 @@ pub(crate) mod tests { let n = F::alloc(&mut cs.ns(|| "alloc new var"), || Ok(negone)).unwrap(); let _ = n.to_bytes(&mut cs.ns(|| "ToBytes")).unwrap(); - let _ = n.to_bytes_strict(&mut cs.ns(|| "ToBytes Strict")).unwrap(); + let _ = n + .to_non_unique_bytes(&mut cs.ns(|| "ToBytes Strict")) + .unwrap(); let ab_false = a .conditionally_add_constant( diff --git a/r1cs-std/src/groups/curves/short_weierstrass/bls12/mod.rs b/r1cs-std/src/groups/curves/short_weierstrass/bls12/mod.rs index c4c5505..534793b 100644 --- a/r1cs-std/src/groups/curves/short_weierstrass/bls12/mod.rs +++ b/r1cs-std/src/groups/curves/short_weierstrass/bls12/mod.rs @@ -52,11 +52,12 @@ impl ToBytesGadget for G1PreparedGadget

{ self.0.to_bytes(&mut cs.ns(|| "g_alpha to bytes")) } - fn to_bytes_strict>( + fn to_non_unique_bytes>( &self, - cs: CS, + mut cs: CS, ) -> Result, SynthesisError> { - self.to_bytes(cs) + self.0 + .to_non_unique_bytes(&mut cs.ns(|| "g_alpha to bytes")) } } @@ -86,11 +87,17 @@ impl ToBytesGadget for G2PreparedGadget

{ Ok(bytes) } - fn to_bytes_strict>( + fn to_non_unique_bytes>( &self, - cs: CS, + mut cs: CS, ) -> Result, SynthesisError> { - self.to_bytes(cs) + let mut bytes = Vec::new(); + for (i, coeffs) in self.ell_coeffs.iter().enumerate() { + let mut cs = cs.ns(|| format!("Iteration {}", i)); + bytes.extend_from_slice(&coeffs.0.to_non_unique_bytes(&mut cs.ns(|| "c0"))?); + bytes.extend_from_slice(&coeffs.1.to_non_unique_bytes(&mut cs.ns(|| "c1"))?); + } + Ok(bytes) } } diff --git a/r1cs-std/src/groups/curves/short_weierstrass/mod.rs b/r1cs-std/src/groups/curves/short_weierstrass/mod.rs index 680216f..3d7d8b6 100644 --- a/r1cs-std/src/groups/curves/short_weierstrass/mod.rs +++ b/r1cs-std/src/groups/curves/short_weierstrass/mod.rs @@ -605,16 +605,16 @@ where Ok(x_bits) } - fn to_bits_strict>( + fn to_non_unique_bits>( &self, mut cs: CS, ) -> Result, SynthesisError> { let mut x_bits = self .x - .to_bits_strict(&mut cs.ns(|| "X Coordinate To Bits"))?; + .to_non_unique_bits(&mut cs.ns(|| "X Coordinate To Bits"))?; let y_bits = self .y - .to_bits_strict(&mut cs.ns(|| "Y Coordinate To Bits"))?; + .to_non_unique_bits(&mut cs.ns(|| "Y Coordinate To Bits"))?; x_bits.extend_from_slice(&y_bits); x_bits.push(self.infinity); @@ -640,17 +640,19 @@ where Ok(x_bytes) } - fn to_bytes_strict>( + fn to_non_unique_bytes>( &self, mut cs: CS, ) -> Result, SynthesisError> { let mut x_bytes = self .x - .to_bytes_strict(&mut cs.ns(|| "X Coordinate To Bytes"))?; + .to_non_unique_bytes(&mut cs.ns(|| "X Coordinate To Bytes"))?; let y_bytes = self .y - .to_bytes_strict(&mut cs.ns(|| "Y Coordinate To Bytes"))?; - let inf_bytes = self.infinity.to_bytes(&mut cs.ns(|| "Infinity to Bytes"))?; + .to_non_unique_bytes(&mut cs.ns(|| "Y Coordinate To Bytes"))?; + let inf_bytes = self + .infinity + .to_non_unique_bytes(&mut cs.ns(|| "Infinity to Bytes"))?; x_bytes.extend_from_slice(&y_bytes); x_bytes.extend_from_slice(&inf_bytes); diff --git a/r1cs-std/src/groups/curves/twisted_edwards/mod.rs b/r1cs-std/src/groups/curves/twisted_edwards/mod.rs index 70b2f34..fddfcb3 100644 --- a/r1cs-std/src/groups/curves/twisted_edwards/mod.rs +++ b/r1cs-std/src/groups/curves/twisted_edwards/mod.rs @@ -1359,12 +1359,16 @@ where Ok(x_bits) } - fn to_bits_strict>( + fn to_non_unique_bits>( &self, mut cs: CS, ) -> Result, SynthesisError> { - let mut x_bits = self.x.to_bits_strict(cs.ns(|| "X Coordinate To Bits"))?; - let y_bits = self.y.to_bits_strict(cs.ns(|| "Y Coordinate To Bits"))?; + let mut x_bits = self + .x + .to_non_unique_bits(cs.ns(|| "X Coordinate To Bits"))?; + let y_bits = self + .y + .to_non_unique_bits(cs.ns(|| "Y Coordinate To Bits"))?; x_bits.extend_from_slice(&y_bits); Ok(x_bits) @@ -1387,12 +1391,12 @@ where Ok(x_bytes) } - fn to_bytes_strict>( + fn to_non_unique_bytes>( &self, mut cs: CS, ) -> Result, SynthesisError> { - let mut x_bytes = self.x.to_bytes_strict(cs.ns(|| "x"))?; - let y_bytes = self.y.to_bytes_strict(cs.ns(|| "y"))?; + let mut x_bytes = self.x.to_non_unique_bytes(cs.ns(|| "x"))?; + let y_bytes = self.y.to_non_unique_bytes(cs.ns(|| "y"))?; x_bytes.extend_from_slice(&y_bytes); Ok(x_bytes) diff --git a/r1cs-std/src/groups/mod.rs b/r1cs-std/src/groups/mod.rs index 6350b56..fd1ffbf 100644 --- a/r1cs-std/src/groups/mod.rs +++ b/r1cs-std/src/groups/mod.rs @@ -206,11 +206,13 @@ mod test { assert_eq!(b2, b_b); let _ = a.to_bytes(&mut cs.ns(|| "ToBytes")).unwrap(); - let _ = a.to_bytes_strict(&mut cs.ns(|| "ToBytes Strict")).unwrap(); + let _ = a + .to_non_unique_bytes(&mut cs.ns(|| "ToBytes Strict")) + .unwrap(); let _ = b.to_bytes(&mut cs.ns(|| "b ToBytes")).unwrap(); let _ = b - .to_bytes_strict(&mut cs.ns(|| "b ToBytes Strict")) + .to_non_unique_bytes(&mut cs.ns(|| "b ToBytes Strict")) .unwrap(); if !cs.is_satisfied() { println!("{:?}", cs.which_is_unsatisfied().unwrap());