Remove unsafe cell from SecretKey

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 <jeff@web3.foundation>"]
-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,Mul};
 
 use ark_ff::{PrimeField}; // Field, Zero
 use ark_ec::{AffineRepr, Group}; // CurveGroup
@@ -19,16 +16,13 @@ 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);
     }
 }
 
-
 /// Secret scalar split into the sum of two scalars, which randomly
 /// mutate but retain the same sum.   Incurs 2x penalty in scalar
 /// multiplications, but provides side channel defenses.
@@ -48,145 +42,162 @@ 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 SecretScalar<F: PrimeField>([F; 2]);
+
+impl<F: PrimeField> From<F> for SecretScalar<F> {
+    fn from(value: F) -> Self {
+        Self::from(&value)
+    }
+}
+
+impl<F: PrimeField + Clone> From<&F> for SecretScalar<F> {
+    fn from(value: &F) -> Self {
+        let mut xof = Rng2Xof(getrandom_or_panic());
+        let v1 = xof_read_reduced(&mut xof);
+        let v2 = value.sub(v1);
+        SecretScalar([v1, v2])
+    }
+}
 
 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) )
+        let mut secret = SecretScalar(self.0.clone());
+        secret.resplit();
+        secret
     }
 }
 
 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()
+        let lhs = &self.0;
+        let rhs = &rhs.0;
+        ((lhs[0] - rhs[0]) + (lhs[1] - rhs[1])).is_zero()
     }
 }
+
 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> 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.
+    /// Internal clone which skips resplit.
     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) {
-        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;
+        SecretScalar(self.0.clone())
     }
 
-    pub fn resplit_mut(&mut self) {
+    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();
+        let selfy = &mut self.0;
         selfy[0] += &x;
         selfy[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 = SecretScalar([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
+        let lhs = &self.clone().0;
+        (lhs[0] * rhs) + (lhs[1] * rhs)
+    }
+
+    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;
-        });
+        let selfy = &self.0;
+        *x *= selfy[0];
+        y *= selfy[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];
-        });
+        let lhs = &mut self.0;
+        let rhs = &rhs.clone().0;
+        lhs[0] += rhs[0];
+        lhs[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<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
-    }
-}
-*/
-
 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 o = self.operate(|lhs| rhs.mul(lhs[0]) + rhs.mul(lhs[1]));
+        let lhs = &self.clone().0;
+        let o = rhs.mul(lhs[0]) + rhs.mul(lhs[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 SecretScalar<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 = SecretScalar::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 = SecretScalar::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 <swasilyev@gmail.com>", "Jeff Burdges <jeff@web3.foundation>", "Syed Hosseini <syed@riseup.net>"]
-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 }
 

dleq_vrf/src/keys.rs

@@ -11,12 +11,11 @@ use ark_std::{vec::Vec, io::{Read, Write}};
 
 use ark_ec::{AffineRepr}; // Group, CurveGroup
 use ark_serialize::{CanonicalSerialize,CanonicalDeserialize,SerializationError};
+use ark_transcript::xof_read_reduced;
 
 #[cfg(feature = "getrandom")]
 use ark_secret_scalar::{rand_core, RngCore};
 
-use ark_secret_scalar::SecretScalar;
-
 use crate::{
     ThinVrf,
     transcript::digest::{Update,XofReader},
@@ -86,7 +85,7 @@ pub struct SecretKey<K: AffineRepr> {
     pub(crate) thin: ThinVrf<K>,
 
     /// Secret key represented as a scalar.
-    pub(crate) key: SecretScalar<<K as AffineRepr>::ScalarField>,
+    pub(crate) key: K::ScalarField,
 
     /// Seed for deriving the nonces used in Schnorr proofs.
     ///
@@ -146,8 +145,9 @@ impl<K: AffineRepr> ThinVrf<K> {
     {
         let mut nonce_seed: [u8; 32] = [0u8; 32];
         xof.read(&mut nonce_seed);
-        let mut key = SecretScalar::from_xof(&mut xof);
-        let public = self.make_public(&mut key);
+
+        let key = xof_read_reduced(&mut xof);
+        let public = self.make_public(&key);
         SecretKey { thin: self, key, nonce_seed, public, 
             #[cfg(debug_assertions)]
             test_vector_fake_rng: false,
@@ -156,7 +156,7 @@ impl<K: AffineRepr> ThinVrf<K> {
 
     /// Generate a `SecretKey` from a 32 byte seed.
     pub fn secretkey_from_seed(self, seed: &[u8; 32]) -> SecretKey<K> {
-        use crate::transcript::digest::{ExtendableOutput};
+        use crate::transcript::digest::ExtendableOutput;
         let mut xof = crate::transcript::Shake128::default();
         xof.update(b"VrfSecretSeed");
         xof.update(seed.as_ref());

dleq_vrf/src/pedersen.rs

@@ -9,6 +9,7 @@
 
 use ark_ff::PrimeField;
 use ark_ec::{AffineRepr, CurveGroup};
+use ark_secret_scalar::SecretScalar;
 use ark_serialize::{CanonicalSerialize,CanonicalDeserialize};
 use ark_std::{borrow::BorrowMut, vec::Vec};
 
@@ -322,8 +323,9 @@ where K: AffineRepr, H: AffineRepr<ScalarField = K::ScalarField>,
         for i in 0..B {
             blindings.push( k.blindings[i] + c * secret_blindings.0[i] );
         }
+        let secret = SecretScalar::from(&secret.key);
         let s = Scalars {
-            keying: k.keying + secret.key.mul_by_challenge(&c),
+            keying: k.keying + secret.mul_by_challenge(&c),
             blindings: blindings.into_inner().unwrap(),
         };
         k.zeroize();