Remove UnsafeCell (Attempt 2)

ark-secret-scalar/Cargo.toml

@@ -2,7 +2,7 @@
 name = "ark-secret-scalar"
 description = "Secret scalars for non-constant-time fields and curves"
 authors = ["Jeff Burdges <[email protected]>"]
-version = "0.0.2"
+version = "0.0.3"
 repository = "https://github.com/w3f/ring-vrf/tree/master/ark-secret-scalar"
 edition = "2021"
 license = "MIT/Apache-2.0"
@@ -27,9 +27,7 @@ ark-transcript = { version = "0.0.2", default-features = false, path = "../ark-t
 
 
 [dev-dependencies]
-
-# TODO:  Tests
-# ark-bls12-377 = { version = "0.4", default-features = false, features = [ "curve" ] }
+ark-bls12-377 = { version = "0.4", default-features = false, features = [ "curve" ] }
 
 
 [features]

ark-secret-scalar/src/lib.rs

@@ -3,10 +3,7 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 #![doc = include_str!("../README.md")]
 
-use core::{
-    cell::UnsafeCell,
-    ops::{Add,AddAssign,Mul}
-};
+use core::ops::{Add, AddAssign, Index, IndexMut, Mul};
 
 use ark_ff::{PrimeField}; // Field, Zero
 use ark_ec::{AffineRepr, Group}; // CurveGroup
@@ -19,15 +16,52 @@ use zeroize::Zeroize;
 // TODO:  Remove ark-transcript dependency once https://github.com/arkworks-rs/algebra/pull/643 lands
 use ark_transcript::xof_read_reduced;
 
-
-
 pub struct Rng2Xof<R: RngCore+CryptoRng>(pub R);
 impl<R: RngCore+CryptoRng> XofReader for Rng2Xof<R> {
     fn read(&mut self, dest: &mut [u8]) {
         self.0.fill_bytes(dest);
     }
 }
 
+#[repr(transparent)]
+#[derive(Zeroize, Clone)]
+pub struct SecretScalar<F: PrimeField>(pub F);
+
+impl<F: PrimeField> SecretScalar<F> {
+    /// Initialize and unbiased `SecretScalar` from a `XofReaader`.
+    pub fn from_xof<R: XofReader>(xof: &mut R) -> Self {
+        SecretScalar(xof_read_reduced(&mut *xof))
+    }
+
+    /// Multiply by a scalar.
+    pub fn mul_by_challenge(&self, rhs: &F) -> F {
+        let lhs = SecretSplit::from(self);
+        (lhs[0] * rhs) + (lhs[1] * rhs)
+    }
+}
+
+impl<F: PrimeField> From<&SecretSplit<F>> for SecretScalar<F> {
+    fn from(value: &SecretSplit<F>) -> Self {
+        SecretScalar(value.0[0] + value.0[1])
+    }
+}
+
+impl<F: PrimeField> From<SecretSplit<F>> for SecretScalar<F> {
+    fn from(value: SecretSplit<F>) -> Self {
+        SecretScalar::from(&value)
+    }
+}
+
+impl<C: AffineRepr> Mul<&C> for &SecretScalar<<C as AffineRepr>::ScalarField> {
+    type Output = <C as AffineRepr>::Group;
+
+    /// Arkworks multiplies on the right since ark_ff is a dependency of ark_ec.
+    /// but ark_ec being our dependency requires left multiplication here.
+    fn mul(self, rhs: &C) -> Self::Output {
+        let lhs = SecretSplit::from(self);
+        rhs.mul(lhs[0]) + rhs.mul(lhs[1])
+    }
+}
 
 /// Secret scalar split into the sum of two scalars, which randomly
 /// mutate but retain the same sum.   Incurs 2x penalty in scalar
@@ -48,145 +82,180 @@ impl<R: RngCore+CryptoRng> XofReader for Rng2Xof<R> {
 // TODO:  We split additively right now, but would a multiplicative splitting
 // help against rowhammer attacks on the secret key?
 #[repr(transparent)]
-pub struct SecretScalar<F: PrimeField>(UnsafeCell<[F; 2]>);
+#[derive(Zeroize)]
+pub struct SecretSplit<F: PrimeField>([F; 2]);
+
+impl<F: PrimeField> Index<usize> for SecretSplit<F> {
+    type Output = F;
 
-impl<F: PrimeField> Clone for SecretScalar<F> {
-    fn clone(&self) -> SecretScalar<F> {
-        let n = self.operate(|ss| ss.clone());
-        self.resplit();
-        SecretScalar(UnsafeCell::new(n) )
+    fn index(&self, index: usize) -> &Self::Output {
+        &self.0[index]
     }
 }
 
-impl<F: PrimeField> PartialEq for SecretScalar<F> {
-    fn eq(&self, rhs: &SecretScalar<F>) -> bool {
-        let lhs = unsafe { &*self.0.get() };
-        let rhs = unsafe { &*rhs.0.get() };
-        ( (lhs[0] - rhs[0]) + (lhs[1] - rhs[1]) ).is_zero()
+impl<F: PrimeField> IndexMut<usize> for SecretSplit<F> {
+    fn index_mut(&mut self, index: usize) -> &mut Self::Output {
+        &mut self.0[index]
     }
 }
-impl<F: PrimeField> Eq for SecretScalar<F> {}
 
-impl<F: PrimeField> Zeroize for SecretScalar<F> { 
-    fn zeroize(&mut self) {
-        self.0.get_mut().zeroize()
+impl<F: PrimeField> From<SecretScalar<F>> for SecretSplit<F> {
+    fn from(value: SecretScalar<F>) -> Self {
+        SecretSplit::from(&value)
     }
 }
-impl<F: PrimeField> Drop for SecretScalar<F> {
-    fn drop(&mut self) { self.zeroize() }
-}
 
-impl<F: PrimeField> SecretScalar<F> {
-    /// Do computations with an immutable borrow of the two scalars.
-    ///
-    /// At the module level, we keep this method private, never pass
-    /// these references into user's code, and never accept user's
-    /// closures, so our being `!Sync` ensures memory safety.
-    /// All other method ensure only the sum of the scalars is visible
-    /// outside this module too.
-    fn operate<R,O>(&self, f : O) -> R
-    where O: FnOnce(&[F; 2]) -> R
-    {
-        f(unsafe { &*self.0.get() })
-    }
-
-    /// Internal clone which skips replit.
-    fn risky_clone(&self) -> SecretScalar<F> {
-        let n = self.operate(|ss| ss.clone());
-        SecretScalar(UnsafeCell::new(n) )
-    }
-
-    /// Immutably borrow `&self` but add opposite random values to its two scalars.
-    /// 
-    /// We encapsulate exposed interior mutability of `SecretScalar` here, but
-    /// our other methods should never reveal references into the scalars,
-    /// or even their individual valus.
-    pub fn resplit(&self) {
+/// Randomply splits the secret in two components.
+impl<F: PrimeField> From<&SecretScalar<F>> for SecretSplit<F> {
+    fn from(value: &SecretScalar<F>) -> Self {
         let mut xof = Rng2Xof(getrandom_or_panic());
-        let x = xof_read_reduced(&mut xof);
-        let selfy = unsafe { &mut *self.0.get() };
-        selfy[0] += &x;
-        selfy[1] -= &x;
+        let v1 = xof_read_reduced(&mut xof);
+        let v2 = value.0.sub(v1);
+        SecretSplit([v1, v2])
     }
+}
 
-    pub fn resplit_mut(&mut self) {
+impl<F: PrimeField> From<F> for SecretSplit<F> {
+    fn from(value: F) -> Self {
+        Self::from(&value)
+    }
+}
+
+impl<F: PrimeField> From<&F> for SecretSplit<F> {
+    fn from(value: &F) -> Self {
+        SecretScalar(*value).into()
+    }
+}
+
+/// Randomly resplits the cloned components.
+impl<F: PrimeField> Clone for SecretSplit<F> {
+    fn clone(&self) -> SecretSplit<F> {
+        let mut secret = SecretSplit(self.0.clone());
+        secret.resplit();
+        secret
+    }
+}
+
+impl<F: PrimeField> PartialEq for SecretSplit<F> {
+    fn eq(&self, rhs: &SecretSplit<F>) -> bool {
+        ((self[0] - rhs[0]) + (self[1] - rhs[1])).is_zero()
+    }
+}
+
+impl<F: PrimeField> Eq for SecretSplit<F> {}
+
+impl<F: PrimeField> Drop for SecretSplit<F> {
+    fn drop(&mut self) { self.zeroize() }
+}
+
+impl<F: PrimeField> SecretSplit<F> {
+    /// Randomply resplit the secret in two components.
+    pub fn resplit(&mut self) {
         let mut xof = Rng2Xof(getrandom_or_panic());
         let x = xof_read_reduced(&mut xof);
-        let selfy = self.0.get_mut();
-        selfy[0] += &x;
-        selfy[1] -= &x;
+        self[0] += &x;
+        self[1] -= &x;
     }
 
     /// Initialize and unbiased `SecretScalar` from a `XofReaader`.
     pub fn from_xof<R: XofReader>(xof: &mut R) -> Self {
         let mut xof = || xof_read_reduced(&mut *xof);
-        let mut ss = SecretScalar(UnsafeCell::new([xof(), xof()]) );
-        ss.resplit_mut();
+        let mut ss = SecretSplit([xof(), xof()]);
+        ss.resplit();
         ss
     }
 
     /// Multiply by a scalar.
     pub fn mul_by_challenge(&self, rhs: &F) -> F {
-        let o = self.operate(|ss| (ss[0] * rhs) + (ss[1] * rhs) );
-        self.resplit();
-        o
+        (self[0] * rhs) + (self[1] * rhs)
+    }
+
+    /// Get the secret scalar value by joining the two components.
+    pub fn scalar(&self) -> F {
+        self.0[0] + self.0[1]
     }
 
     /// Arkworks multiplies on the right since ark_ff is a dependency of ark_ec.
     /// but ark_ec being our dependency requires left multiplication here.
     fn mul_action<G: Group<ScalarField=F>>(&self, x: &mut G) {
         let mut y = x.clone();
-        self.operate(|ss| {
-            *x *= ss[0];
-            y *= ss[1];
-            *x += y;
-        });
+        *x *= self[0];
+        y *= self[1];
+        *x += y;
     }
 }
 
-impl<F: PrimeField> AddAssign<&SecretScalar<F>> for SecretScalar<F> {
-    fn add_assign(&mut self, rhs: &SecretScalar<F>) {
-        let lhs = self.0.get_mut();
-        rhs.operate(|rhs| {
-            lhs[0] += rhs[0];
-            lhs[1] += rhs[1];
-        });
+impl<F: PrimeField> AddAssign<&SecretSplit<F>> for SecretSplit<F> {
+    fn add_assign(&mut self, rhs: &SecretSplit<F>) {
+        self[0] += rhs[0];
+        self[1] += rhs[1];
     }
 }
 
-impl<F: PrimeField> Add<&SecretScalar<F>> for &SecretScalar<F> {
-    type Output = SecretScalar<F>;
-    fn add(self, rhs: &SecretScalar<F>) -> SecretScalar<F> {
-        let mut lhs = self.risky_clone();
-        lhs += rhs;
-        lhs.resplit_mut();
-        lhs
-    }
-}
+impl<F: PrimeField> Add<&SecretSplit<F>> for SecretSplit<F> {
+    type Output = SecretSplit<F>;
 
-/*
-impl<G: Group> Mul<&G> for &SecretScalar<<G as Group>::ScalarField> {
-    type Output = G;
-    /// Arkworks multiplies on the right since ark_ff is a dependency of ark_ec.
-    /// but ark_ec being our dependency requires left multiplication here.
-    fn mul(self, rhs: &G) -> G {
-        let mut rhs = rhs.clone();
-        self.mul_action(&mut rhs);
-        rhs
+    fn add(self, rhs: &SecretSplit<F>) -> Self::Output {
+        let mut res = SecretSplit([self[0], self[1]]);
+        res += rhs;
+        res
     }
 }
-*/
 
-impl<C: AffineRepr> Mul<&C> for &SecretScalar<<C as AffineRepr>::ScalarField> {
+impl<C: AffineRepr> Mul<&C> for &SecretSplit<<C as AffineRepr>::ScalarField> {
     type Output = <C as AffineRepr>::Group;
+
     /// Arkworks multiplies on the right since ark_ff is a dependency of ark_ec.
     /// but ark_ec being our dependency requires left multiplication here.
     fn mul(self, rhs: &C) -> Self::Output {
-        let o = self.operate(|lhs| rhs.mul(lhs[0]) + rhs.mul(lhs[1]));
+        let o = rhs.mul(self[0]) + rhs.mul(self[1]);
+
         use ark_ec::CurveGroup;
         debug_assert_eq!(o.into_affine(), { let mut t = rhs.into_group(); self.mul_action(&mut t); t }.into_affine() );
-        self.resplit();
         o
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use ark_bls12_377::Fr;
+    use ark_ff::MontFp;
+    use ark_std::fmt::Debug;
+
+    impl<F: PrimeField + Debug> Debug for SecretSplit<F> {
+        fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+            let selfy = &self.0;
+            f.debug_tuple("SecretScalar")
+                .field(&selfy[0])
+                .field(&selfy[1])
+                .finish()
+        }
+    }
+
+    #[test]
+    fn from_single_scalar_works() {
+        let value: Fr = MontFp!("123456789");
+
+        let mut secret = SecretSplit::from(value);
+        assert_eq!(value, secret.scalar());
+
+        secret.resplit();
+        assert_eq!(value, secret.scalar());
+
+        let secret2 = secret.clone();
+        assert_ne!(secret.0[0], secret2.0[0]);
+        assert_ne!(secret.0[1], secret2.0[1]);
+        assert_eq!(secret, secret2);
+    }
+
+    #[test]
+    fn mul_my_challenge_works() {
+        let value: Fr = MontFp!("123456789");
+        let secret = SecretSplit::from(value);
+
+        let factor = Fr::from(3);
+        let result = secret.mul_by_challenge(&factor);
+        assert_eq!(result, value * factor);
+    }
+}

bandersnatch_vrfs/Cargo.toml

@@ -9,7 +9,7 @@ license = "MIT/Apache-2.0"
 keywords = ["crypto", "cryptography", "vrf", "signature", "privacy"]
 
 [dependencies]
-dleq_vrf = { version = "0.0.2", default-features = false, path = "../dleq_vrf", features = [ "scale" ] }
+dleq_vrf = { default-features = false, path = "../dleq_vrf", features = [ "scale" ] }
 
 rand_core.workspace = true
 zeroize.workspace = true

dleq_vrf/Cargo.toml

@@ -2,7 +2,7 @@
 name = "dleq_vrf"
 description = "VRFs from Chaum-Pedersen DLEQ proofs, usable in Ring VRFs"
 authors = ["Sergey Vasilyev <[email protected]>", "Jeff Burdges <[email protected]>", "Syed Hosseini <[email protected]>"]
-version = "0.0.2"
+version = "0.0.3"
 repository = "https://github.com/w3f/ring-vrf/tree/master/dleq_vrf"
 edition = "2021"
 license = "MIT/Apache-2.0"
@@ -20,8 +20,8 @@ ark-ff.workspace = true
 ark-ec.workspace = true
 ark-serialize.workspace = true
 
-ark-secret-scalar = { version = "0.0.2", default-features = false, path = "../ark-secret-scalar" }
-ark-transcript = { version = "0.0.2", default-features = false, path = "../ark-transcript" }
+ark-secret-scalar = { default-features = false, path = "../ark-secret-scalar" }
+ark-transcript = { default-features = false, path = "../ark-transcript" }
 
 ark-scale = { workspace = true, optional = true }