crypto: zeroize private key in BLS12377 (#850)

Author: joyqvq

crypto/src/bls12377/mod.rs

@@ -27,6 +27,7 @@ use serde_with::serde_as;
 use signature::{Signer, Verifier};
 
 use fastcrypto::traits::{Authenticator, KeyPair, SigningKey, VerifyingKey};
+use zeroize::Zeroize;
 
 mod ark_serialize;
 
@@ -400,7 +401,7 @@ impl KeyPair for BLS12377KeyPair {
     }
 
     fn private(self) -> Self::PrivKey {
-        self.secret
+        BLS12377PrivateKey::from_bytes(self.secret.as_ref()).unwrap()
     }
 
     fn generate<R: rand::CryptoRng + rand::RngCore>(rng: &mut R) -> Self {
@@ -592,3 +593,32 @@ impl From<&BLS12377PublicKey> for BLS12377PublicKeyBytes {
         BLS12377PublicKeyBytes::from_bytes(pk.as_ref()).unwrap()
     }
 }
+
+impl zeroize::Zeroize for BLS12377PrivateKey {
+    fn zeroize(&mut self) {
+        // PrivateKey.zeroize here is not necessary here because the underlying implicitly zeroizes.
+        self.bytes.take().zeroize();
+    }
+}
+
+impl zeroize::ZeroizeOnDrop for BLS12377PrivateKey {}
+
+impl Drop for BLS12377PrivateKey {
+    fn drop(&mut self) {
+        self.zeroize();
+    }
+}
+
+impl zeroize::Zeroize for BLS12377KeyPair {
+    fn zeroize(&mut self) {
+        self.secret.zeroize()
+    }
+}
+
+impl zeroize::ZeroizeOnDrop for BLS12377KeyPair {}
+
+impl Drop for BLS12377KeyPair {
+    fn drop(&mut self) {
+        self.zeroize();
+    }
+}

crypto/src/tests/bls12377_tests.rs

@@ -9,9 +9,9 @@ use fastcrypto::traits::{
 };
 use fastcrypto::{Hash, SignatureService};
 
+use crate::bls12377::CELO_BLS_PRIVATE_KEY_LENGTH;
 use rand::{rngs::StdRng, SeedableRng as _};
 use signature::{Signer, Verifier};
-
 pub fn keys() -> Vec<BLS12377KeyPair> {
     let mut rng = StdRng::from_seed([0; 32]);
     (0..4)
@@ -370,3 +370,52 @@ async fn signature_service() {
     // Verify the signature we received.
     assert!(pk.verify(digest.as_ref(), &signature).is_ok());
 }
+
+#[test]
+fn test_sk_zeroization_on_drop() {
+    let ptr: *const u8;
+    let bytes_ptr: *const u8;
+
+    let mut sk_bytes = Vec::new();
+
+    {
+        let mut rng = StdRng::from_seed([9; 32]);
+        let kp = BLS12377KeyPair::generate(&mut rng);
+        let sk = kp.private();
+        sk_bytes.extend_from_slice(sk.as_ref());
+
+        ptr = std::ptr::addr_of!(sk.privkey) as *const u8;
+        bytes_ptr = &sk.as_ref()[0] as *const u8;
+
+        // Assert the exact location in SecretKey is stored as private key bytes.
+        unsafe {
+            for (i, &byte) in sk_bytes
+                .iter()
+                .enumerate()
+                .take(CELO_BLS_PRIVATE_KEY_LENGTH)
+            {
+                assert_eq!(*ptr.add(i + 41), byte);
+            }
+        }
+        let sk_memory: &[u8] =
+            unsafe { ::std::slice::from_raw_parts(bytes_ptr, CELO_BLS_PRIVATE_KEY_LENGTH) };
+        // Assert that this is equal to sk_bytes before deletion.
+        assert_eq!(sk_memory, &sk_bytes[..]);
+    }
+
+    // Assert the exact position in SecretKey is no longer private key bytes.
+    unsafe {
+        for (i, &byte) in sk_bytes
+            .iter()
+            .enumerate()
+            .take(CELO_BLS_PRIVATE_KEY_LENGTH)
+        {
+            assert_ne!(*ptr.add(i + 41), byte);
+        }
+    }
+
+    // Check that self.bytes is reset to OnceCell default.
+    let sk_memory: &[u8] =
+        unsafe { ::std::slice::from_raw_parts(bytes_ptr, CELO_BLS_PRIVATE_KEY_LENGTH) };
+    assert_ne!(sk_memory, &sk_bytes[..]);
+}