chore: remove derived Debug and Serialize from private key types (#…

…696)

* signature crate refactor

* changelog & version

* remove unnecessary comment

* address comments

* fixed length

* phrases

signature/CHANGELOG.md

@@ -3,6 +3,14 @@
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.2.0]((https://github.com/EspressoSystems/jellyfish/compare/jf-signatures-v0.1.1...jf-signatures-v0.2.0)) (2024-10-29)
+
+### Changed
+
+- [#696](https://github.com/EspressoSystems/jellyfish/pull/696): removed derived `Debug` and `Serialize` for private keys.
+  - Manually implemented `Debug` for private signing keys to not print the actual value.
+  - Remove derived (de)serialization. Implemented `to/from_bytes()` and the conversions to/from `TaggedBase64`.
+
 ## [0.1.1]((https://github.com/EspressoSystems/jellyfish/compare/0.4.5...jf-signatures-v0.1.1)) (2024-07-25)
 
 ### Changed

signature/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "jf-signature"
-version = "0.1.1"
+version = "0.2.0"
 description = "Implementation of signature schemes, including BLS and Schnorr."
 authors = { workspace = true }
 edition = { workspace = true }

signature/src/bls_over_bls12381.rs

@@ -89,66 +89,66 @@ use ark_std::{
 };
 use blst::{min_sig::*, BLST_ERROR};
 use derivative::Derivative;
-use tagged_base64::tagged;
+use tagged_base64::{tagged, TaggedBase64, Tb64Error};
 use zeroize::{Zeroize, Zeroizing};
 
-#[tagged(tag::BLS_SIGNING_KEY)]
 #[derive(Clone, Derivative, Zeroize)]
 #[zeroize(drop)]
-#[derivative(Debug)]
-/// A BLS Secret Key (Signing Key).
-pub struct BLSSignKey(#[derivative(Debug = "ignore")] SecretKey);
-
-impl Deref for BLSSignKey {
-    type Target = SecretKey;
-    fn deref(&self) -> &Self::Target {
-        &self.0
+/// A BLS Secret Key (Signing Key). We intentionally omit the implementation of
+/// `serde::Serializable` so that it won't be unnoticably serialized or printed
+/// out through outer structs. However, users could manually serialize it into
+/// bytes by calling `to_bytes()` or `to_tagged_base64()` and exercise with
+/// self-cautions.
+pub struct BLSSignKey(SecretKey);
+
+// This content-hiding `Debug` implementation makes sure that the auto-derived
+// `Debug` for user's outer struct won't undesirably print out this secret key.
+impl core::fmt::Debug for BLSSignKey {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        f.debug_struct("<BLSSignKey>").finish()
     }
 }
 
-impl CanonicalSerialize for BLSSignKey {
-    /// Secret key can only be serialized in compressed mode.
-    fn serialize_with_mode<W: Write>(
-        &self,
-        writer: W,
-        _compress: Compress,
-    ) -> Result<(), SerializationError> {
-        // TODO (tessico): should we fail if compress is `Compress::No`?
-        CanonicalSerialize::serialize_compressed(&self.to_bytes()[..], writer)
+impl BLSSignKey {
+    /// Explicit calls to serialize a `SignKey` into bytes.
+    pub fn to_bytes(&self) -> [u8; 32] {
+        self.0.to_bytes()
     }
 
-    fn serialized_size(&self, _compress: Compress) -> usize {
-        BLS_SIG_SK_SIZE
+    /// Deserialize `SignKey` from bytes.
+    pub fn from_bytes(bytes: &[u8; 32]) -> Result<BLSSignKey, BLST_ERROR> {
+        SecretKey::from_bytes(bytes).map(|sk| BLSSignKey(sk))
+    }
+
+    /// Explicit calls to serialize a `SignKey` into `TaggedBase64`.
+    pub fn to_tagged_base64(&self) -> Result<TaggedBase64, Tb64Error> {
+        TaggedBase64::new(tag::BLS_SIGNING_KEY, &self.to_bytes())
     }
 }
 
-impl CanonicalDeserialize for BLSSignKey {
-    fn deserialize_with_mode<R: Read>(
-        mut reader: R,
-        compress: Compress,
-        validate: Validate,
-    ) -> Result<Self, SerializationError> {
-        let len = <usize as ark_serialize::CanonicalDeserialize>::deserialize_with_mode(
-            &mut reader,
-            compress,
-            validate,
-        )?;
-        if len != BLS_SIG_SK_SIZE {
-            return Err(SerializationError::InvalidData);
-        }
+impl<'a> TryFrom<&'a TaggedBase64> for BLSSignKey {
+    type Error = Tb64Error;
 
-        let mut sk_bytes = [0u8; BLS_SIG_SK_SIZE];
-        reader.read_exact(&mut sk_bytes)?;
-        SecretKey::deserialize(&sk_bytes)
-            .map(Self)
-            .map_err(|_| SerializationError::InvalidData)
+    fn try_from(tb: &'a TaggedBase64) -> Result<Self, Self::Error> {
+        if tb.tag() != tag::BLS_SIGNING_KEY {
+            return Err(Tb64Error::InvalidTag);
+        }
+        SecretKey::from_bytes(tb.as_ref())
+            .map(|sk| BLSSignKey(sk))
+            .map_err(|err| Tb64Error::InvalidData)
     }
 }
 
-impl Valid for BLSSignKey {
-    fn check(&self) -> Result<(), ark_serialize::SerializationError> {
-        // TODO no `validate()` method in `blst` on `SecretKey`
-        Ok(())
+impl TryFrom<TaggedBase64> for BLSSignKey {
+    type Error = Tb64Error;
+
+    fn try_from(tb: TaggedBase64) -> Result<Self, Self::Error> {
+        if tb.tag() != tag::BLS_SIGNING_KEY {
+            return Err(Tb64Error::InvalidTag);
+        }
+        SecretKey::from_bytes(tb.as_ref())
+            .map(|sk| BLSSignKey(sk))
+            .map_err(|err| Tb64Error::InvalidData)
     }
 }
 
@@ -374,7 +374,7 @@ impl SignatureScheme for BLSSignatureScheme {
         msg: M,
         _prng: &mut R,
     ) -> Result<Self::Signature, SignatureError> {
-        Ok(BLSSignature(sk.sign(
+        Ok(BLSSignature(sk.0.sign(
             msg.as_ref(),
             Self::CS_ID.as_bytes(),
             &[],
@@ -448,7 +448,14 @@ mod test {
         let msg = "The quick brown fox jumps over the lazy dog";
         let sig = BLSSignatureScheme::sign(&(), &sk, msg, &mut rng).unwrap();
 
-        test_canonical_serde_helper(sk);
+        let bytes = sk.to_bytes();
+        let de = BLSSignKey::from_bytes(&bytes).unwrap();
+        assert_eq!(sk, de);
+
+        let tagged_blob = sk.to_tagged_base64().unwrap();
+        let de: BLSSignKey = tagged_blob.try_into().unwrap();
+        assert_eq!(sk, de);
+
         test_canonical_serde_helper(pk);
         test_canonical_serde_helper(sig);
     }

signature/src/bls_over_bn254.rs

@@ -57,6 +57,7 @@ use ark_std::{
     hash::{Hash, Hasher},
     rand::{CryptoRng, RngCore},
     string::ToString,
+    vec,
     vec::Vec,
     One, UniformRand,
 };
@@ -67,7 +68,7 @@ use sha3::Keccak256;
 
 use crate::SignatureError::{ParameterError, VerificationError};
 
-use tagged_base64::tagged;
+use tagged_base64::{tagged, TaggedBase64, Tb64Error};
 use zeroize::Zeroize;
 
 /// BLS signature scheme.
@@ -224,24 +225,66 @@ impl AggregateableSignatureSchemes for BLSOverBN254CurveSignatureScheme {
 // =====================================================
 // Signing key
 // =====================================================
-#[tagged(tag::BLS_SIGNING_KEY)]
-#[derive(
-    Clone,
-    Hash,
-    Default,
-    Zeroize,
-    Eq,
-    PartialEq,
-    CanonicalSerialize,
-    CanonicalDeserialize,
-    Derivative,
-    Ord,
-    PartialOrd,
-)]
+#[derive(Clone, Hash, Default, Zeroize, Eq, PartialEq)]
 #[zeroize(drop)]
-#[derivative(Debug)]
-/// Signing key for BLS signature.
-pub struct SignKey(#[derivative(Debug = "ignore")] pub(crate) ScalarField);
+/// Signing key for BLS signature. We intentionally omit the implementation of
+/// `serde::Serializable` so that it won't be unnoticably serialized or printed
+/// out through outer structs. However, users could manually serialize it into
+/// bytes by calling `to_bytes()` or `to_tagged_base64()` and exercise with
+/// self-cautions.
+pub struct SignKey(pub(crate) ScalarField);
+
+// This content-hiding `Debug` implementation makes sure that the auto-derived
+// `Debug` for user's outer struct won't undesirably print out this secret key.
+impl core::fmt::Debug for SignKey {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        f.debug_struct("<BLSSignKey>").finish()
+    }
+}
+
+impl SignKey {
+    /// Signature Key generation function
+    pub fn generate<R: CryptoRng + RngCore>(prng: &mut R) -> SignKey {
+        SignKey(ScalarField::rand(prng))
+    }
+
+    /// Explicit calls to serialize a `SignKey` into bytes.
+    pub fn to_bytes(&self) -> Vec<u8> {
+        self.0.into_bigint().to_bytes_le()
+    }
+
+    /// Deserialize `SignKey` from bytes.
+    pub fn from_bytes(bytes: &[u8]) -> Self {
+        Self(ScalarField::from_le_bytes_mod_order(bytes))
+    }
+
+    /// Explicit calls to serialize a `SignKey` into `TaggedBase64`.
+    pub fn to_tagged_base64(&self) -> Result<TaggedBase64, Tb64Error> {
+        TaggedBase64::new(tag::BLS_SIGNING_KEY, &self.to_bytes())
+    }
+}
+
+impl<'a> TryFrom<&'a TaggedBase64> for SignKey {
+    type Error = Tb64Error;
+
+    fn try_from(tb: &'a TaggedBase64) -> Result<Self, Self::Error> {
+        if tb.tag() != tag::BLS_SIGNING_KEY {
+            return Err(Tb64Error::InvalidTag);
+        }
+        Ok(SignKey::from_bytes(tb.as_ref()))
+    }
+}
+
+impl TryFrom<TaggedBase64> for SignKey {
+    type Error = Tb64Error;
+
+    fn try_from(tb: TaggedBase64) -> Result<Self, Self::Error> {
+        if tb.tag() != tag::BLS_SIGNING_KEY {
+            return Err(Tb64Error::InvalidTag);
+        }
+        Ok(SignKey::from_bytes(tb.as_ref()))
+    }
+}
 
 // =====================================================
 // Verification key
@@ -279,7 +322,7 @@ impl VerKey {
 // =====================================================
 
 /// Signature secret key pair used to sign messages
-#[derive(CanonicalSerialize, CanonicalDeserialize, Clone, Zeroize)]
+#[derive(Clone, Debug, Zeroize, Eq, PartialEq)]
 #[zeroize(drop)]
 pub struct KeyPair {
     sk: SignKey,
@@ -402,6 +445,43 @@ impl KeyPair {
         let sigma = hash_value * self.sk.0;
         Signature { sigma }
     }
+
+    /// Explicit calls to serialize a `KeyPair` into bytes.
+    pub fn to_bytes(&self) -> Vec<u8> {
+        self.sk.to_bytes()
+    }
+
+    /// Deserialize `KeyPair` from bytes.
+    pub fn from_bytes(bytes: &[u8]) -> Self {
+        Self::generate_with_sign_key(ScalarField::from_le_bytes_mod_order(bytes))
+    }
+
+    /// Explicit calls to serialize a `KeyPair` into `TaggedBase64`.
+    pub fn to_tagged_base64(&self) -> Result<TaggedBase64, Tb64Error> {
+        TaggedBase64::new(tag::BLS_KEY_PAIR, &self.to_bytes())
+    }
+}
+
+impl<'a> TryFrom<&'a TaggedBase64> for KeyPair {
+    type Error = Tb64Error;
+
+    fn try_from(tb: &'a TaggedBase64) -> Result<Self, Self::Error> {
+        if tb.tag() != tag::BLS_KEY_PAIR {
+            return Err(Tb64Error::InvalidTag);
+        }
+        Ok(KeyPair::from_bytes(tb.as_ref()))
+    }
+}
+
+impl TryFrom<TaggedBase64> for KeyPair {
+    type Error = Tb64Error;
+
+    fn try_from(tb: TaggedBase64) -> Result<Self, Self::Error> {
+        if tb.tag() != tag::BLS_KEY_PAIR {
+            return Err(Tb64Error::InvalidTag);
+        }
+        Ok(KeyPair::from_bytes(tb.as_ref()))
+    }
 }
 
 impl From<SignKey> for KeyPair {
@@ -411,13 +491,6 @@ impl From<SignKey> for KeyPair {
     }
 }
 
-impl SignKey {
-    /// Signature Key generation function
-    pub fn generate<R: CryptoRng + RngCore>(prng: &mut R) -> SignKey {
-        SignKey(ScalarField::rand(prng))
-    }
-}
-
 impl From<&SignKey> for VerKey {
     fn from(sk: &SignKey) -> Self {
         VerKey(G2Projective::generator() * sk.0)
@@ -532,17 +605,22 @@ mod tests {
         let msg = vec![87u8];
         let sig = keypair.sign(&msg, CS_ID_BLS_BN254);
 
-        let mut ser_bytes: Vec<u8> = Vec::new();
-        keypair.serialize_compressed(&mut ser_bytes).unwrap();
-        let de: KeyPair = KeyPair::deserialize_compressed(&ser_bytes[..]).unwrap();
-        assert_eq!(de.ver_key_ref(), keypair.ver_key_ref());
-        assert_eq!(de.ver_key_ref(), &VerKey::from(&de.sk));
+        let mut ser_bytes: Vec<u8> = keypair.to_bytes();
+        let de = KeyPair::from_bytes(&ser_bytes);
+        assert_eq!(de, keypair);
 
-        let mut ser_bytes: Vec<u8> = Vec::new();
-        sk.serialize_compressed(&mut ser_bytes).unwrap();
-        let de: SignKey = SignKey::deserialize_compressed(&ser_bytes[..]).unwrap();
+        let tagged_blob = keypair.to_tagged_base64().unwrap();
+        let de: KeyPair = tagged_blob.try_into().unwrap();
+        assert_eq!(de, keypair);
+
+        let mut ser_bytes: Vec<u8> = sk.to_bytes();
+        let de = SignKey::from_bytes(&ser_bytes);
         assert_eq!(VerKey::from(&de), VerKey::from(&sk));
 
+        let tagged_blob = sk.to_tagged_base64().unwrap();
+        let de: SignKey = tagged_blob.try_into().unwrap();
+        assert_eq!(de, sk);
+
         let mut ser_bytes: Vec<u8> = Vec::new();
         vk.serialize_compressed(&mut ser_bytes).unwrap();
         let de: VerKey = VerKey::deserialize_compressed(&ser_bytes[..]).unwrap();

signature/src/constants.rs

@@ -7,6 +7,8 @@
 
 /// Tags
 pub mod tag {
+    /// Tag for BLS key pair
+    pub const BLS_KEY_PAIR: &str = "BLS_KEY_PAIR";
     /// Tag for BLS verification key
     pub const BLS_VER_KEY: &str = "BLS_VER_KEY";
     /// Tag for BLS signing key

signature/src/lib.rs

@@ -74,15 +74,7 @@ pub trait SignatureScheme: Clone + Send + Sync + 'static {
     const CS_ID: &'static str;
 
     /// Signing key.
-    type SigningKey: Debug
-        + Clone
-        + Send
-        + Sync
-        + Zeroize
-        + for<'a> Deserialize<'a>
-        + Serialize
-        + PartialEq
-        + Eq;
+    type SigningKey: Debug + Clone + Send + Sync + Zeroize + PartialEq + Eq;
 
     /// Verification key
     type VerificationKey: Debug