Adding "danger" feature to expose underlying elements (#27)

* Adding internal feature to expose underlying elements

* Applying @daxpedda's comments and suggestions

.github/workflows/main.yml

@@ -18,6 +18,7 @@ jobs:
           - p256,ristretto255_u64
         frontend_feature:
           - serde
+          - danger
         toolchain:
           - stable
           - 1.51.0
@@ -64,6 +65,7 @@ jobs:
         frontend_feature:
           -
           - --features serde
+          - --features danger
     steps:
       - uses: actions/checkout@v2
       - uses: hecrj/setup-rust-action@v1

Cargo.toml

@@ -13,6 +13,7 @@ resolver = "2"
 
 [features]
 default = ["ristretto255_u64", "serde"]
+danger = []
 ristretto255_u64 = ["curve25519-dalek/u64_backend"]
 ristretto255_u32 = ["curve25519-dalek/u32_backend"]
 ristretto255_fiat_u64 = ["curve25519-dalek/fiat_u64_backend"]
@@ -42,12 +43,13 @@ zeroize = { version = "1", default-features = false }
 generic-array = { version = "0.14", features = ["more_lengths"] }
 hex = "0.4"
 json = "0.12"
+proptest = "1"
 rand = "0.8"
-sha2 = "0.9"
 regex = "1"
-voprf = { path = "", default-features = false, features = ["std"] }
+sha2 = "0.9"
+voprf = { path = "", default-features = false, features = ["std", "danger"] }
 
 [package.metadata.docs.rs]
-features = ["p256", "std"]
+features = ["danger", "p256", "std"]
 targets = []
 rustdoc-args = ["--cfg", "docsrs"]

src/lib.rs

@@ -401,6 +401,11 @@
 //! - The `serde` feature, enabled by default, provides convenience functions for serializing and deserializing with
 //! [serde](https://serde.rs/).
 //!
+//! - The `danger` feature, disabled by default, exposes functions for setting and getting
+//!   internal values not available in the default API. These functions are intended for use in
+//!   by higher-level cryptographic protocols that need access to these raw values and are able to
+//!   perform the necessary validations on them (such as being valid group elements).
+//!
 //! - The backend features are re-exported from
 //! [curve25519-dalek](https://doc.dalek.rs/curve25519_dalek/index.html#backends-and-features) and allow for selecting
 //! the corresponding backend for the curve arithmetic used. The `ristretto255_u64` feature is included as the default.
@@ -408,7 +413,7 @@
 //!
 //! - The `ristretto255_simd` feature is re-exported from
 //! [curve25519-dalek](https://doc.dalek.rs/curve25519_dalek/index.html#backends-and-features) and enables parallel formulas,
-//! using either AVX2 or AVX512-IFMA. This will automatically enable the `ristretto255_u64` and requires Rust nightly.
+//! using either AVX2 or AVX512-IFMA. This will automatically enable the `ristretto255_u64` feature and requires Rust nightly.
 
 #![deny(unsafe_code)]
 #![warn(clippy::cargo, missing_docs)]

src/serialization.rs

@@ -142,7 +142,7 @@ impl<G: Group, H: BlockInput + Digest> Proof<G, H> {
     /// Deserialization from bytes
     pub fn deserialize(input: &[u8]) -> Result<Self, InternalError> {
         let scalar_len = <G as Group>::ScalarLen::USIZE;
-        if input.len() < scalar_len + scalar_len {
+        if input.len() != scalar_len + scalar_len {
             return Err(InternalError::SizeError);
         }
         Ok(Proof {
@@ -161,6 +161,10 @@ impl<G: Group, H: BlockInput + Digest> BlindedElement<G, H> {
 
     /// Deserialization from bytes
     pub fn deserialize(input: &[u8]) -> Result<Self, InternalError> {
+        let elem_len = <G as Group>::ElemLen::USIZE;
+        if input.len() != elem_len {
+            return Err(InternalError::SizeError);
+        }
         Ok(Self {
             value: G::from_element_slice(input)?,
             hash: PhantomData,
@@ -176,6 +180,10 @@ impl<G: Group, H: BlockInput + Digest> EvaluationElement<G, H> {
 
     /// Deserialization from bytes
     pub fn deserialize(input: &[u8]) -> Result<Self, InternalError> {
+        let elem_len = <G as Group>::ElemLen::USIZE;
+        if input.len() != elem_len {
+            return Err(InternalError::SizeError);
+        }
         Ok(Self {
             value: G::from_element_slice(input)?,
             hash: PhantomData,
@@ -218,7 +226,10 @@ pub(crate) fn serialize<L: ArrayLength<u8>>(input: &[u8]) -> Result<Vec<u8>, Int
 #[cfg(test)]
 mod unit_tests {
     use super::*;
+    use curve25519_dalek::ristretto::RistrettoPoint;
     use generic_array::typenum::{U1, U2};
+    use proptest::{collection::vec, prelude::*};
+    use sha2::Sha512;
 
     // Test the error condition for I2OSP
     #[test]
@@ -233,4 +244,42 @@ mod unit_tests {
         assert!(i2osp::<U2>(256 * 256).is_err());
         assert!(i2osp::<U2>(256 * 256 + 1).is_err());
     }
+
+    proptest! {
+        #[test]
+        fn test_nocrash_nonverifiable_client(bytes in vec(any::<u8>(), 0..200)) {
+            NonVerifiableClient::<RistrettoPoint, Sha512>::deserialize(&bytes[..]).map_or(true, |_| true);
+        }
+
+        #[test]
+        fn test_nocrash_verifiable_client(bytes in vec(any::<u8>(), 0..200)) {
+            VerifiableClient::<RistrettoPoint, Sha512>::deserialize(&bytes[..]).map_or(true, |_| true);
+        }
+
+        #[test]
+        fn test_nocrash_nonverifiable_server(bytes in vec(any::<u8>(), 0..200)) {
+            NonVerifiableServer::<RistrettoPoint, Sha512>::deserialize(&bytes[..]).map_or(true, |_| true);
+        }
+
+        #[test]
+        fn test_nocrash_verifiable_server(bytes in vec(any::<u8>(), 0..200)) {
+            VerifiableServer::<RistrettoPoint, Sha512>::deserialize(&bytes[..]).map_or(true, |_| true);
+        }
+
+        #[test]
+        fn test_nocrash_blinded_element(bytes in vec(any::<u8>(), 0..200)) {
+            BlindedElement::<RistrettoPoint, Sha512>::deserialize(&bytes[..]).map_or(true, |_| true);
+        }
+
+        #[test]
+        fn test_nocrash_evaluation_element(bytes in vec(any::<u8>(), 0..200)) {
+            EvaluationElement::<RistrettoPoint, Sha512>::deserialize(&bytes[..]).map_or(true, |_| true);
+        }
+
+        #[test]
+        fn test_nocrash_proof(bytes in vec(any::<u8>(), 0..200)) {
+            Proof::<RistrettoPoint, Sha512>::deserialize(&bytes[..]).map_or(true, |_| true);
+        }
+
+    }
 }

src/voprf.rs

@@ -192,20 +192,11 @@ impl<G: Group, H: BlockInput + Digest> NonVerifiableClient<G, H> {
         }
     }
 
-    #[cfg(test)]
+    #[cfg(feature = "danger")]
-    /// Only used for test functions
+    /// Exposes the blind group element
     pub fn get_blind(&self) -> <G as Group>::Scalar {
         self.blind
     }
-
-    #[cfg(test)]
-    /// Only used for testing zeroize
-    pub fn as_ptrs(&self) -> Vec<Vec<u8>> {
-        vec![
-            self.data.clone(),
-            <G as Group>::scalar_as_bytes(self.blind).to_vec(),
-        ]
-    }
 }
 
 impl<G: Group, H: BlockInput + Digest> VerifiableClient<G, H> {
@@ -330,16 +321,6 @@ impl<G: Group, H: BlockInput + Digest> VerifiableClient<G, H> {
     pub fn get_blind(&self) -> <G as Group>::Scalar {
         self.blind
     }
-
-    #[cfg(test)]
-    /// Only used for testing zeroize
-    pub fn as_ptrs(&self) -> Vec<Vec<u8>> {
-        vec![
-            self.data.clone(),
-            <G as Group>::scalar_as_bytes(self.blind).to_vec(),
-            self.blinded_element.to_arr().to_vec(),
-        ]
-    }
 }
 
 impl<G: Group, H: BlockInput + Digest> NonVerifiableServer<G, H> {
@@ -405,12 +386,6 @@ impl<G: Group, H: BlockInput + Digest> NonVerifiableServer<G, H> {
             },
         })
     }
-
-    #[cfg(test)]
-    /// Only used for testing zeroize
-    pub fn as_ptrs(&self) -> Vec<Vec<u8>> {
-        vec![<G as Group>::scalar_as_bytes(self.sk).to_vec()]
-    }
 }
 
 impl<G: Group, H: BlockInput + Digest> VerifiableServer<G, H> {
@@ -523,15 +498,6 @@ impl<G: Group, H: BlockInput + Digest> VerifiableServer<G, H> {
     pub fn get_public_key(&self) -> G {
         self.pk
     }
-
-    #[cfg(test)]
-    /// Only used for testing zeroize
-    pub fn as_ptrs(&self) -> Vec<Vec<u8>> {
-        vec![
-            <G as Group>::scalar_as_bytes(self.sk).to_vec(),
-            self.pk.to_arr().to_vec(),
-        ]
-    }
 }
 
 /////////////////////////
@@ -598,10 +564,24 @@ impl<G: Group, H: BlockInput + Digest> BlindedElement<G, H> {
         }
     }
 
-    #[cfg(test)]
+    #[cfg(feature = "danger")]
-    /// Only used for testing zeroize
+    /// Creates a [BlindedElement] from a raw group element.
-    pub fn as_ptrs(&self) -> Vec<Vec<u8>> {
+    ///
-        vec![self.value.to_arr().to_vec()]
+    /// # Caution
+    ///
+    /// This should be used with caution, since
+    /// it does not perform any checks on the validity of the value itself!
+    pub fn from_value_unchecked(value: G) -> Self {
+        Self {
+            value,
+            hash: PhantomData,
+        }
+    }
+
+    #[cfg(feature = "danger")]
+    /// Exposes the internal value
+    pub fn value(&self) -> G {
+        self.value
     }
 }
 
@@ -614,21 +594,24 @@ impl<G: Group, H: BlockInput + Digest> EvaluationElement<G, H> {
         }
     }
 
-    #[cfg(test)]
+    #[cfg(feature = "danger")]
-    /// Only used for testing zeroize
+    /// Creates an [EvaluationElement] from a raw group element.
-    pub fn as_ptrs(&self) -> Vec<Vec<u8>> {
+    ///
-        vec![self.value.to_arr().to_vec()]
+    /// # Caution
+    ///
+    /// This should be used with caution, since
+    /// it does not perform any checks on the validity of the value itself!
+    pub fn from_value_unchecked(value: G) -> Self {
+        Self {
+            value,
+            hash: PhantomData,
+        }
     }
-}
 
-impl<G: Group, H: BlockInput + Digest> Proof<G, H> {
+    #[cfg(feature = "danger")]
-    #[cfg(test)]
+    /// Exposes the internal value
-    /// Only used for testing zeroize
+    pub fn value(&self) -> G {
-    pub fn as_ptrs(&self) -> Vec<Vec<u8>> {
+        self.value
-        vec![
-            <G as Group>::scalar_as_bytes(self.c_scalar).to_vec(),
-            <G as Group>::scalar_as_bytes(self.s_scalar).to_vec(),
-        ]
     }
 }
 
@@ -1053,15 +1036,11 @@ mod tests {
 
         let mut state = client_blind_result.state;
         Zeroize::zeroize(&mut state);
-        for bytes in state.as_ptrs() {
+        assert!(state.serialize().iter().all(|&x| x == 0));
-            assert!(bytes.iter().all(|&x| x == 0));
-        }
 
         let mut message = client_blind_result.message;
         Zeroize::zeroize(&mut message);
-        for bytes in message.as_ptrs() {
+        assert!(message.serialize().iter().all(|&x| x == 0));
-            assert!(bytes.iter().all(|&x| x == 0));
-        }
     }
 
     fn zeroize_verifiable_client<G: Group, H: BlockInput + Digest>() {
@@ -1072,15 +1051,11 @@ mod tests {
 
         let mut state = client_blind_result.state;
         Zeroize::zeroize(&mut state);
-        for bytes in state.as_ptrs() {
+        assert!(state.serialize().iter().all(|&x| x == 0));
-            assert!(bytes.iter().all(|&x| x == 0));
-        }
 
         let mut message = client_blind_result.message;
         Zeroize::zeroize(&mut message);
-        for bytes in message.as_ptrs() {
+        assert!(message.serialize().iter().all(|&x| x == 0));
-            assert!(bytes.iter().all(|&x| x == 0));
-        }
     }
 
     fn zeroize_base_server<G: Group, H: BlockInput + Digest>() {
@@ -1096,15 +1071,11 @@ mod tests {
 
         let mut state = server;
         Zeroize::zeroize(&mut state);
-        for bytes in state.as_ptrs() {
+        assert!(state.serialize().iter().all(|&x| x == 0));
-            assert!(bytes.iter().all(|&x| x == 0));
-        }
 
         let mut message = server_result.message;
         Zeroize::zeroize(&mut message);
-        for bytes in message.as_ptrs() {
+        assert!(message.serialize().iter().all(|&x| x == 0));
-            assert!(bytes.iter().all(|&x| x == 0));
-        }
     }
 
     fn zeroize_verifiable_server<G: Group, H: BlockInput + Digest>() {
@@ -1120,21 +1091,15 @@ mod tests {
 
         let mut state = server;
         Zeroize::zeroize(&mut state);
-        for bytes in state.as_ptrs() {
+        assert!(state.serialize().iter().all(|&x| x == 0));
-            assert!(bytes.iter().all(|&x| x == 0));
-        }
 
         let mut message = server_result.message;
         Zeroize::zeroize(&mut message);
-        for bytes in message.as_ptrs() {
+        assert!(message.serialize().iter().all(|&x| x == 0));
-            assert!(bytes.iter().all(|&x| x == 0));
-        }
 
         let mut proof = server_result.proof;
         Zeroize::zeroize(&mut proof);
-        for bytes in proof.as_ptrs() {
+        assert!(proof.serialize().iter().all(|&x| x == 0));
-            assert!(bytes.iter().all(|&x| x == 0));
-        }
     }
 
     #[test]