[ed25519] improve signing key life cycle (#51)

* make sk non-copy

* lint

* simplify zeroization

curve25519/solana-ed25519/src/backend/serial/u64/field.rs

@@ -22,7 +22,7 @@ use subtle::Choice;
 use subtle::ConditionallySelectable;
 
 #[cfg(feature = "zeroize")]
-use zeroize::Zeroize;
+use zeroize::DefaultIsZeroes;
 
 /// A `FieldElement51` represents an element of the field
 /// \\( \mathbb Z / (2\^{255} - 19)\\).
@@ -49,11 +49,7 @@ impl Debug for FieldElement51 {
 }
 
 #[cfg(feature = "zeroize")]
-impl Zeroize for FieldElement51 {
+impl DefaultIsZeroes for FieldElement51 {}
-    fn zeroize(&mut self) {
-        self.0.zeroize();
-    }
-}
 
 impl<'a> AddAssign<&'a FieldElement51> for FieldElement51 {
     fn add_assign(&mut self, _rhs: &'a FieldElement51) {

curve25519/solana-ed25519/src/backend/serial/u64/scalar.rs

@@ -16,27 +16,23 @@ use core::ops::{Index, IndexMut};
 use subtle::{Choice, ConditionallySelectable};
 
 #[cfg(feature = "zeroize")]
-use zeroize::Zeroize;
+use zeroize::{DefaultIsZeroes, Zeroize};
 
 use crate::constants;
 
 /// The `Scalar52` struct represents an element in
 /// \\(\mathbb Z / \ell \mathbb Z\\) as 5 \\(52\\)-bit limbs.
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Default)]
 pub struct Scalar52(pub [u64; 5]);
 
 impl Debug for Scalar52 {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        write!(f, "Scalar52: {:?}", &self.0[..])
+        f.write_str("Scalar52{..}")
     }
 }
 
 #[cfg(feature = "zeroize")]
-impl Zeroize for Scalar52 {
+impl DefaultIsZeroes for Scalar52 {}
-    fn zeroize(&mut self) {
-        self.0.zeroize();
-    }
-}
 
 impl Index<usize> for Scalar52 {
     type Output = u64;
@@ -81,6 +77,9 @@ impl Scalar52 {
         s[3] = ((words[2] >> 28) | (words[3] << 36)) & mask;
         s[4] =  (words[3] >> 16)                     & top_mask;
 
+        #[cfg(feature = "zeroize")]
+        words.zeroize();
+
         s
     }
 
@@ -112,7 +111,16 @@ impl Scalar52 {
         lo = Scalar52::montgomery_mul(&lo, &constants::R);  // (lo * R) / R = lo
         hi = Scalar52::montgomery_mul(&hi, &constants::RR); // (hi * R^2) / R = hi * R
 
-        Scalar52::add(&hi, &lo)
+        let reduced = Scalar52::add(&hi, &lo);
+
+        #[cfg(feature = "zeroize")]
+        {
+            words.zeroize();
+            lo.zeroize();
+            hi.zeroize();
+        }
+
+        reduced
     }
 
     /// Pack the limbs of this `Scalar52` into 32 bytes
@@ -299,29 +307,67 @@ impl Scalar52 {
 
     /// Compute `a * b` (mod l)
     #[inline(never)]
+    #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
     pub fn mul(a: &Scalar52, b: &Scalar52) -> Scalar52 {
-        let ab = Scalar52::montgomery_reduce(&Scalar52::mul_internal(a, b));
+        let mut ab_limbs = Scalar52::mul_internal(a, b);
-        Scalar52::montgomery_reduce(&Scalar52::mul_internal(&ab, &constants::RR))
+        let mut ab = Scalar52::montgomery_reduce(&ab_limbs);
+        let mut rr_limbs = Scalar52::mul_internal(&ab, &constants::RR);
+        let product = Scalar52::montgomery_reduce(&rr_limbs);
+
+        #[cfg(feature = "zeroize")]
+        {
+            ab_limbs.zeroize();
+            ab.zeroize();
+            rr_limbs.zeroize();
+        }
+
+        product
     }
 
     /// Compute `a^2` (mod l)
     #[inline(never)]
+    #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
     #[allow(dead_code)] // XXX we don't expose square() via the Scalar API
     pub fn square(&self) -> Scalar52 {
-        let aa = Scalar52::montgomery_reduce(&Scalar52::square_internal(self));
+        let mut aa_limbs = Scalar52::square_internal(self);
-        Scalar52::montgomery_reduce(&Scalar52::mul_internal(&aa, &constants::RR))
+        let mut aa = Scalar52::montgomery_reduce(&aa_limbs);
+        let mut rr_limbs = Scalar52::mul_internal(&aa, &constants::RR);
+        let square = Scalar52::montgomery_reduce(&rr_limbs);
+
+        #[cfg(feature = "zeroize")]
+        {
+            aa_limbs.zeroize();
+            aa.zeroize();
+            rr_limbs.zeroize();
+        }
+
+        square
     }
 
     /// Compute `(a * b) / R` (mod l), where R is the Montgomery modulus 2^260
     #[inline(never)]
+    #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
     pub fn montgomery_mul(a: &Scalar52, b: &Scalar52) -> Scalar52 {
-        Scalar52::montgomery_reduce(&Scalar52::mul_internal(a, b))
+        let mut limbs = Scalar52::mul_internal(a, b);
+        let product = Scalar52::montgomery_reduce(&limbs);
+
+        #[cfg(feature = "zeroize")]
+        limbs.zeroize();
+
+        product
     }
 
     /// Compute `(a^2) / R` (mod l) in Montgomery form, where R is the Montgomery modulus 2^260
     #[inline(never)]
+    #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
     pub fn montgomery_square(&self) -> Scalar52 {
-        Scalar52::montgomery_reduce(&Scalar52::square_internal(self))
+        let mut limbs = Scalar52::square_internal(self);
+        let square = Scalar52::montgomery_reduce(&limbs);
+
+        #[cfg(feature = "zeroize")]
+        limbs.zeroize();
+
+        square
     }
 
     /// Puts a Scalar52 in to Montgomery form, i.e. computes `a*R (mod l)`
@@ -338,7 +384,12 @@ impl Scalar52 {
         for i in 0..5 {
             limbs[i] = self[i] as u128;
         }
-        Scalar52::montgomery_reduce(&limbs)
+        let scalar = Scalar52::montgomery_reduce(&limbs);
+
+        #[cfg(feature = "zeroize")]
+        limbs.zeroize();
+
+        scalar
     }
 }
 

curve25519/solana-ed25519/src/ed_sigs.rs

@@ -2,6 +2,8 @@
 
 use crate::scalar::Scalar;
 use sha2::{Digest, Sha512};
+#[cfg(feature = "zeroize")]
+use zeroize::Zeroize;
 
 #[cfg(test)]
 mod tests;
@@ -20,8 +22,17 @@ pub use signing_key::SigningKey;
 pub use verification_key::{VerificationKey, VerificationKeyBytes};
 
 pub(crate) fn scalar_from_sha512(hash: Sha512) -> Scalar {
-    let output = hash.finalize();
+    #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+    let mut output = hash.finalize();
     let mut bytes = [0u8; 64];
     bytes.copy_from_slice(output.as_slice());
-    Scalar::from_bytes_mod_order_wide(&bytes)
+    let scalar = Scalar::from_bytes_mod_order_wide(&bytes);
+
+    #[cfg(feature = "zeroize")]
+    {
+        output.zeroize();
+        bytes.zeroize();
+    }
+
+    scalar
 }

curve25519/solana-ed25519/src/ed_sigs/signing_key.rs

@@ -36,7 +36,7 @@ use pkcs8::{
     DecodePrivateKey, DecodePublicKey, Document, EncodePrivateKey, EncodePublicKey,
     ObjectIdentifier, PrivateKeyInfo, spki::AlgorithmIdentifierRef,
 };
-#[cfg(all(feature = "pem", feature = "pkcs8"))]
+#[cfg(all(feature = "pkcs8", feature = "zeroize"))]
 use zeroize::Zeroizing;
 
 #[cfg(all(feature = "pem", feature = "pkcs8"))]
@@ -59,7 +59,7 @@ pub type SecretKey = [u8; SECRET_KEY_LENGTH];
 /// An Ed25519 signing key.
 ///
 /// This is also called a secret key by other implementations.
-#[derive(Copy, Clone)]
+#[derive(Clone)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(feature = "serde", serde(from = "SerdeHelper"))]
 #[cfg_attr(feature = "serde", serde(into = "SerdeHelper"))]
@@ -80,6 +80,13 @@ impl Zeroize for SigningKey {
     }
 }
 
+#[cfg(feature = "zeroize")]
+impl Drop for SigningKey {
+    fn drop(&mut self) {
+        self.zeroize();
+    }
+}
+
 impl core::fmt::Debug for SigningKey {
     fn fmt(&self, fmt: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         fmt.debug_struct("SigningKey")
@@ -100,12 +107,6 @@ impl<'a> From<&'a SigningKey> for VerificationKeyBytes {
     }
 }
 
-impl AsRef<[u8]> for SigningKey {
-    fn as_ref(&self) -> &[u8] {
-        &self.seed[..]
-    }
-}
-
 impl From<SigningKey> for SecretKey {
     fn from(sk: SigningKey) -> SecretKey {
         sk.seed
@@ -128,8 +129,12 @@ impl TryFrom<&[u8]> for SigningKey {
 impl From<SecretKey> for SigningKey {
     #[allow(non_snake_case)]
     fn from(seed: [u8; 32]) -> SigningKey {
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut seed = seed;
+
         // Expand the seed to a 64-byte array with SHA512.
-        let h = Sha512::digest(&seed[..]);
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut h = Sha512::digest(&seed[..]);
 
         // Convert the low half to a scalar with Ed25519 "clamping"
         let s = {
@@ -138,20 +143,22 @@ impl From<SecretKey> for SigningKey {
             scalar_bytes[0] &= 248;
             scalar_bytes[31] &= 127;
             scalar_bytes[31] |= 64;
-            Scalar::from_bytes_mod_order(scalar_bytes)
+            let s = Scalar::from_bytes_mod_order(scalar_bytes);
+
+            #[cfg(feature = "zeroize")]
+            scalar_bytes.zeroize();
+
+            s
         };
 
         // Extract and cache the high half.
-        let prefix = {
+        let mut prefix = [0u8; 32];
-            let mut prefix = [0u8; 32];
+        prefix[..].copy_from_slice(&h[32..64]);
-            prefix[..].copy_from_slice(&h[32..64]);
-            prefix
-        };
 
         // Compute the public key as A = [s]B.
         let A = EdwardsPoint::mul_base(&s);
 
-        SigningKey {
+        let signing_key = SigningKey {
             seed,
             s,
             prefix,
@@ -159,7 +166,16 @@ impl From<SecretKey> for SigningKey {
                 minus_A: -A,
                 A_bytes: VerificationKeyBytes(A.compress().to_bytes()),
             },
+        };
+
+        #[cfg(feature = "zeroize")]
+        {
+            h.zeroize();
+            prefix.zeroize();
+            seed.zeroize();
         }
+
+        signing_key
     }
 }
 
@@ -260,12 +276,16 @@ impl EncodePrivateKey for SigningKey {
         // In RFC 8410, the octet string containing the private key is encapsulated by
         // another octet string. Just add octet string bytes to the key when building
         // the document.
+        #[cfg(feature = "zeroize")]
+        let mut final_key = Zeroizing::new([0u8; 34]);
+        #[cfg(not(feature = "zeroize"))]
         let mut final_key = [0u8; 34];
+
         final_key[..2].copy_from_slice(&[0x04, 0x20]);
         final_key[2..].copy_from_slice(&self.seed);
         SecretDocument::try_from(PrivateKeyInfo {
             algorithm: ALGORITHM_ID,
-            private_key: &final_key,
+            private_key: &final_key[..],
             public_key: Some(self.vk.A_bytes.0.as_slice()),
         })
     }
@@ -296,6 +316,20 @@ impl DecodePrivateKey for SigningKey {
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 struct SerdeHelper([u8; 32]);
 
+#[cfg(feature = "zeroize")]
+impl Zeroize for SerdeHelper {
+    fn zeroize(&mut self) {
+        self.0.zeroize();
+    }
+}
+
+#[cfg(feature = "zeroize")]
+impl Drop for SerdeHelper {
+    fn drop(&mut self) {
+        self.zeroize();
+    }
+}
+
 impl From<SerdeHelper> for SigningKey {
     fn from(helper: SerdeHelper) -> SigningKey {
         helper.0.into()
@@ -319,12 +353,24 @@ impl SigningKey {
     /// Convert this [`SigningKey`] into a `SecretKey`
     #[inline]
     pub fn to_bytes(&self) -> SecretKey {
-        (*self).into()
+        self.to_secret_key_bytes()
     }
 
     /// Convert this [`SigningKey`] into a `SecretKey` reference
     #[inline]
     pub fn as_bytes(&self) -> &SecretKey {
+        self.as_secret_key_bytes()
+    }
+
+    /// Copy this [`SigningKey`]'s RFC8032 seed bytes.
+    #[inline]
+    pub fn to_secret_key_bytes(&self) -> SecretKey {
+        self.seed
+    }
+
+    /// Borrow this [`SigningKey`]'s RFC8032 seed bytes.
+    #[inline]
+    pub fn as_secret_key_bytes(&self) -> &SecretKey {
         &self.seed
     }
 
@@ -333,7 +379,12 @@ impl SigningKey {
     pub fn new<R: RngCore + CryptoRng>(mut rng: R) -> SigningKey {
         let mut bytes = [0u8; 32];
         rng.fill_bytes(&mut bytes[..]);
-        bytes.into()
+        let signing_key = bytes.into();
+
+        #[cfg(feature = "zeroize")]
+        bytes.zeroize();
+
+        signing_key
     }
 
     /// Get the [`VerificationKey`] for this [`SigningKey`].
@@ -344,18 +395,29 @@ impl SigningKey {
     /// Create a signature on `msg` using this key.
     #[allow(non_snake_case)]
     pub fn sign(&self, msg: &[u8]) -> Signature {
-        let r = scalar_from_sha512(Sha512::default().chain(&self.prefix[..]).chain(msg));
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut r = scalar_from_sha512(Sha512::default().chain(&self.prefix[..]).chain(msg));
 
         let R_bytes = EdwardsPoint::mul_base(&r).compress().to_bytes();
 
-        let k = scalar_from_sha512(
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut k = scalar_from_sha512(
             Sha512::default()
                 .chain(&R_bytes[..])
                 .chain(&self.vk.A_bytes.0[..])
                 .chain(msg),
         );
 
-        let s_bytes = (r + k * self.s).to_bytes();
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut s = r + k * self.s;
+        let s_bytes = s.to_bytes();
+
+        #[cfg(feature = "zeroize")]
+        {
+            r.zeroize();
+            k.zeroize();
+            s.zeroize();
+        }
 
         Signature::from_components(R_bytes, s_bytes)
     }
@@ -375,10 +437,14 @@ impl SigningKey {
         // In RFC 8410, the octet string containing the private key is encapsulated by
         // another octet string. Just add octet string bytes to the key when building
         // the document.
+        #[cfg(feature = "zeroize")]
+        let mut final_key = Zeroizing::new([0u8; 34]);
+        #[cfg(not(feature = "zeroize"))]
         let mut final_key = [0u8; 34];
+
         final_key[..2].copy_from_slice(&[0x04, 0x20]);
         final_key[2..].copy_from_slice(&self.seed);
-        SecretDocument::try_from(PrivateKeyInfo::new(ALGORITHM_ID, &final_key))
+        SecretDocument::try_from(PrivateKeyInfo::new(ALGORITHM_ID, &final_key[..]))
     }
 
     /// Serialize [`SigningKey`] as a PEM-encoded PKCS#8 string. Note that this

curve25519/solana-ed25519/src/ed_sigs/tests/decoding.rs

@@ -44,12 +44,18 @@ fn decode_der_to_signing_key() {
     // Test against a v1 DER key.
     let sk1 = SigningKey::from_pkcs8_der(PKCS8_V1_DER).unwrap();
     let sk_bytes_string_1 = "D4EE72DBF913584AD5B6D8F1F769F8AD3AFE7C28CBF1D4FBE097A88F44755842";
-    assert_eq!(hex::decode(sk_bytes_string_1).unwrap(), sk1.as_ref());
+    assert_eq!(
+        hex::decode(sk_bytes_string_1).unwrap(),
+        sk1.as_secret_key_bytes()
+    );
 
     // Test against a v2 DER key.
     let sk2 = SigningKey::from_pkcs8_der(PKCS8_V2_DER).unwrap();
     let sk_bytes_string_2 = "D4EE72DBF913584AD5B6D8F1F769F8AD3AFE7C28CBF1D4FBE097A88F44755842";
-    assert_eq!(hex::decode(sk_bytes_string_2).unwrap(), sk2.as_ref());
+    assert_eq!(
+        hex::decode(sk_bytes_string_2).unwrap(),
+        sk2.as_secret_key_bytes()
+    );
 
     // Test against a v2 DER key with a mismatched public key.
     assert!(SigningKey::from_pkcs8_der(PKCS8_V2_DER_BAD).is_err());
@@ -67,12 +73,18 @@ fn decode_doc_to_signing_key() {
     // Test against a v1 PEM key.
     let sk1 = SigningKey::from_pkcs8_pem(PKCS8_V1_PEM).unwrap();
     let sk_bytes_string_1 = "D4EE72DBF913584AD5B6D8F1F769F8AD3AFE7C28CBF1D4FBE097A88F44755842";
-    assert_eq!(hex::decode(sk_bytes_string_1).unwrap(), sk1.as_ref());
+    assert_eq!(
+        hex::decode(sk_bytes_string_1).unwrap(),
+        sk1.as_secret_key_bytes()
+    );
 
     // Test against a valid v2 PEM key.
     let sk2 = SigningKey::from_pkcs8_pem(PKCS8_V2_PEM).unwrap();
     let sk_bytes_string_2 = "D4EE72DBF913584AD5B6D8F1F769F8AD3AFE7C28CBF1D4FBE097A88F44755842";
-    assert_eq!(hex::decode(sk_bytes_string_2).unwrap(), sk2.as_ref());
+    assert_eq!(
+        hex::decode(sk_bytes_string_2).unwrap(),
+        sk2.as_secret_key_bytes()
+    );
 
     // Test against a v2 DER key with a mismatched public key.
     assert!(SigningKey::from_pkcs8_pem(PKCS8_V2_PEM_BAD).is_err());

curve25519/solana-ed25519/src/ed_sigs/tests/unit_tests.rs

@@ -11,7 +11,7 @@ fn parsing() {
     let pkb = VerificationKeyBytes::from(&sk);
     let sig = sk.sign(b"test");
 
-    let sk_array: [u8; 32] = sk.into();
+    let sk_array: [u8; 32] = sk.to_secret_key_bytes();
     let pk_array: [u8; 32] = pk.into();
     let pkb_array: [u8; 32] = pkb.into();
     let sig_array: [u8; 64] = sig.into();
@@ -26,7 +26,7 @@ fn parsing() {
     assert_eq!(pkb, pkb2);
     assert_eq!(sig, sig2);
 
-    let sk3: SigningKey = bincode::deserialize(sk.as_ref()).unwrap();
+    let sk3: SigningKey = bincode::deserialize(sk.as_secret_key_bytes()).unwrap();
     let pk3: VerificationKey = bincode::deserialize(pk.as_ref()).unwrap();
     let pkb3: VerificationKeyBytes = bincode::deserialize(pkb.as_ref()).unwrap();
 

curve25519/solana-ed25519/src/field.rs

@@ -33,6 +33,9 @@ use subtle::ConstantTimeEq;
 use crate::backend;
 use crate::constants;
 
+#[cfg(feature = "zeroize")]
+use zeroize::Zeroize;
+
 #[cfg(feature = "digest")]
 use digest::{
     Digest, FixedOutput, HashMarker,
@@ -87,18 +90,25 @@ impl FieldElement {
 
         // Interpret both sides as field elements
         let mut fe_f = Self::from_bytes(&fl);
-        let fe_g = Self::from_bytes(&gl);
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut fe_g = Self::from_bytes(&gl);
 
         // The full field elem is now fe_f + 2²⁵⁵ fl_top_bit + 2²⁵⁶ fe_g + 2⁵¹¹ gl_top_bit
 
         // Add the masked off bits back to fe_f. fl_top_bit, if set, is 2^255 ≡ 19 (mod q).
         // gl_top_bit, if set, is 2^511 ≡ 722 (mod q)
-        let top_bits_sum = {
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut top_bits_sum = {
             // This only need to be a u16 because the max value is 741
             let addend: u16 = fl_top_bit * 19 + gl_top_bit * 722;
             let mut addend_bytes = [0u8; 32];
             addend_bytes[..2].copy_from_slice(&addend.to_le_bytes());
-            Self::from_bytes(&addend_bytes)
+            let top_bits_sum = Self::from_bytes(&addend_bytes);
+
+            #[cfg(feature = "zeroize")]
+            addend_bytes.zeroize();
+
+            top_bits_sum
         };
         fe_f += &top_bits_sum;
 
@@ -109,6 +119,14 @@ impl FieldElement {
         ]);
         fe_f += &(&THIRTY_EIGHT * &fe_g);
 
+        #[cfg(feature = "zeroize")]
+        {
+            fl.zeroize();
+            gl.zeroize();
+            fe_g.zeroize();
+            top_bits_sum.zeroize();
+        }
+
         fe_f
     }
 
@@ -184,6 +202,9 @@ impl FieldElement {
         let mut scratch = vec![FieldElement::ONE; n];
 
         Self::internal_invert_batch(inputs, &mut scratch);
+
+        #[cfg(feature = "zeroize")]
+        scratch.zeroize();
     }
 
     /// Given a slice of pub(crate)lic `FieldElements`, replace each with its inverse. `scratch` can
@@ -208,7 +229,12 @@ impl FieldElement {
         for (input, scratch) in inputs.iter().zip(scratch.iter_mut()) {
             *scratch = acc;
             // acc <- acc * input, but skipping zeros (constant-time)
-            acc.conditional_assign(&(&acc * input), !input.is_zero());
+            #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+            let mut product = &acc * input;
+            acc.conditional_assign(&product, !input.is_zero());
+
+            #[cfg(feature = "zeroize")]
+            product.zeroize();
         }
 
         // acc is nonzero because we skipped zeros in inputs
@@ -220,12 +246,27 @@ impl FieldElement {
         // Pass through the vector backwards to compute the inverses
         // in place
         for (input, scratch) in inputs.iter_mut().rev().zip(scratch.iter_mut().rev()) {
-            let tmp = &acc * input;
+            #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+            let mut tmp = &acc * input;
+            #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+            let mut inverse = &acc * scratch;
             // input <- acc * scratch, then acc <- tmp
             // Again, we skip zeros in a constant-time way
             let nz = !input.is_zero();
-            input.conditional_assign(&(&acc * scratch), nz);
+            input.conditional_assign(&inverse, nz);
             acc.conditional_assign(&tmp, nz);
+
+            #[cfg(feature = "zeroize")]
+            {
+                tmp.zeroize();
+                inverse.zeroize();
+            }
+        }
+
+        #[cfg(feature = "zeroize")]
+        {
+            acc.zeroize();
+            scratch.zeroize();
         }
     }
 

curve25519/solana-ed25519/src/scalar.rs

@@ -216,7 +216,14 @@ impl Scalar {
     /// Construct a `Scalar` by reducing a 512-bit little-endian integer
     /// modulo the group order \\( \ell \\).
     pub fn from_bytes_mod_order_wide(input: &[u8; 64]) -> Scalar {
-        UnpackedScalar::from_bytes_wide(input).pack()
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut unpacked = UnpackedScalar::from_bytes_wide(input);
+        let scalar = unpacked.pack();
+
+        #[cfg(feature = "zeroize")]
+        unpacked.zeroize();
+
+        scalar
     }
 
     /// Attempt to construct a `Scalar` from a canonical byte representation.
@@ -257,7 +264,7 @@ impl Scalar {
 
 impl Debug for Scalar {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        write!(f, "Scalar{{\n\tbytes: {:?},\n}}", &self.bytes)
+        f.write_str("Scalar{..}")
     }
 }
 
@@ -344,9 +351,22 @@ impl Neg for &Scalar {
     type Output = Scalar;
     #[allow(non_snake_case)]
     fn neg(self) -> Scalar {
-        let self_R = UnpackedScalar::mul_internal(&self.unpack(), &constants::R);
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
-        let self_mod_l = UnpackedScalar::montgomery_reduce(&self_R);
+        let mut self_unpacked = self.unpack();
-        UnpackedScalar::sub(&UnpackedScalar::ZERO, &self_mod_l).pack()
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut self_R = UnpackedScalar::mul_internal(&self_unpacked, &constants::R);
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut self_mod_l = UnpackedScalar::montgomery_reduce(&self_R);
+        let negated = UnpackedScalar::sub(&UnpackedScalar::ZERO, &self_mod_l).pack();
+
+        #[cfg(feature = "zeroize")]
+        {
+            self_unpacked.zeroize();
+            self_R.zeroize();
+            self_mod_l.zeroize();
+        }
+
+        negated
     }
 }
 
@@ -419,8 +439,13 @@ impl<'de> Deserialize<'de> for Scalar {
                         .next_element()?
                         .ok_or_else(|| serde::de::Error::invalid_length(i, &"expected 32 bytes"))?;
                 }
-                Option::from(Scalar::from_canonical_bytes(bytes))
+                let scalar = Option::from(Scalar::from_canonical_bytes(bytes))
-                    .ok_or_else(|| serde::de::Error::custom("scalar was not canonically encoded"))
+                    .ok_or_else(|| serde::de::Error::custom("scalar was not canonically encoded"));
+
+                #[cfg(feature = "zeroize")]
+                bytes.zeroize();
+
+                scalar
             }
         }
 
@@ -604,9 +629,15 @@ impl Scalar {
     /// # }
     #[cfg(feature = "rand_core")]
     pub fn random<R: CryptoRng + RngCore + ?Sized>(rng: &mut R) -> Self {
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
         let mut scalar_bytes = [0u8; 64];
         rng.fill_bytes(&mut scalar_bytes);
-        Scalar::from_bytes_mod_order_wide(&scalar_bytes)
+        let scalar = Scalar::from_bytes_mod_order_wide(&scalar_bytes);
+
+        #[cfg(feature = "zeroize")]
+        scalar_bytes.zeroize();
+
+        scalar
     }
 
     #[cfg(feature = "digest")]
@@ -681,9 +712,20 @@ impl Scalar {
     where
         D: Digest<OutputSize = U64>,
     {
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut digest = hash.finalize();
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
         let mut output = [0u8; 64];
-        output.copy_from_slice(hash.finalize().as_slice());
+        output.copy_from_slice(digest.as_slice());
-        Scalar::from_bytes_mod_order_wide(&output)
+        let scalar = Scalar::from_bytes_mod_order_wide(&output);
+
+        #[cfg(feature = "zeroize")]
+        {
+            digest.zeroize();
+            output.zeroize();
+        }
+
+        scalar
     }
 
     /// Convert this `Scalar` to its underlying sequence of bytes.
@@ -754,7 +796,19 @@ impl Scalar {
     /// assert!(should_be_one == Scalar::ONE);
     /// ```
     pub fn invert(&self) -> Scalar {
-        self.unpack().invert().pack()
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut unpacked = self.unpack();
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut inverse = unpacked.invert();
+        let scalar = inverse.pack();
+
+        #[cfg(feature = "zeroize")]
+        {
+            unpacked.zeroize();
+            inverse.zeroize();
+        }
+
+        scalar
     }
 
     /// Given a slice of nonzero (possibly secret) `Scalar`s,
@@ -866,7 +920,7 @@ impl Scalar {
         }
 
         #[cfg(feature = "zeroize")]
-        Zeroize::zeroize(&mut scratch.iter_mut());
+        scratch.zeroize();
 
         ret
     }
@@ -1183,10 +1237,22 @@ impl Scalar {
     /// Reduce this `Scalar` modulo \\(\ell\\).
     #[allow(non_snake_case)]
     fn reduce(&self) -> Scalar {
-        let x = self.unpack();
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
-        let xR = UnpackedScalar::mul_internal(&x, &constants::R);
+        let mut x = self.unpack();
-        let x_mod_l = UnpackedScalar::montgomery_reduce(&xR);
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
-        x_mod_l.pack()
+        let mut xR = UnpackedScalar::mul_internal(&x, &constants::R);
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut x_mod_l = UnpackedScalar::montgomery_reduce(&xR);
+        let scalar = x_mod_l.pack();
+
+        #[cfg(feature = "zeroize")]
+        {
+            x.zeroize();
+            xR.zeroize();
+            x_mod_l.zeroize();
+        }
+
+        scalar
     }
 
     /// Check whether this `Scalar` is the canonical representative mod \\(\ell\\). This is not
@@ -1206,19 +1272,20 @@ impl UnpackedScalar {
 
     /// Inverts an UnpackedScalar in Montgomery form.
     #[rustfmt::skip] // keep alignment of addition chain and squarings
+    #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
     #[allow(clippy::just_underscores_and_digits)]
     pub fn montgomery_invert(&self) -> UnpackedScalar {
         // Uses the addition chain from
         // https://briansmith.org/ecc-inversion-addition-chains-01#curve25519_scalar_inversion
-        let    _1 = *self;
+        let mut    _1 = *self;
-        let   _10 = _1.montgomery_square();
+        let mut   _10 = _1.montgomery_square();
-        let  _100 = _10.montgomery_square();
+        let mut  _100 = _10.montgomery_square();
-        let   _11 = UnpackedScalar::montgomery_mul(&_10,     &_1);
+        let mut   _11 = UnpackedScalar::montgomery_mul(&_10,     &_1);
-        let  _101 = UnpackedScalar::montgomery_mul(&_10,    &_11);
+        let mut  _101 = UnpackedScalar::montgomery_mul(&_10,    &_11);
-        let  _111 = UnpackedScalar::montgomery_mul(&_10,   &_101);
+        let mut  _111 = UnpackedScalar::montgomery_mul(&_10,   &_101);
-        let _1001 = UnpackedScalar::montgomery_mul(&_10,   &_111);
+        let mut _1001 = UnpackedScalar::montgomery_mul(&_10,   &_111);
-        let _1011 = UnpackedScalar::montgomery_mul(&_10,  &_1001);
+        let mut _1011 = UnpackedScalar::montgomery_mul(&_10,  &_1001);
-        let _1111 = UnpackedScalar::montgomery_mul(&_100, &_1011);
+        let mut _1111 = UnpackedScalar::montgomery_mul(&_100, &_1011);
 
         // _10000
         let mut y = UnpackedScalar::montgomery_mul(&_1111, &_1);
@@ -1259,12 +1326,40 @@ impl UnpackedScalar {
         square_multiply(&mut y,       3, &_101);
         square_multiply(&mut y,   1 + 2, &_11);
 
-        y
+        let inverse = y;
+
+        #[cfg(feature = "zeroize")]
+        {
+            _1.zeroize();
+            _10.zeroize();
+            _100.zeroize();
+            _11.zeroize();
+            _101.zeroize();
+            _111.zeroize();
+            _1001.zeroize();
+            _1011.zeroize();
+            _1111.zeroize();
+            y.zeroize();
+        }
+
+        inverse
     }
 
     /// Inverts an UnpackedScalar not in Montgomery form.
     pub fn invert(&self) -> UnpackedScalar {
-        self.as_montgomery().montgomery_invert().from_montgomery()
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut montgomery = self.as_montgomery();
+        #[cfg_attr(not(feature = "zeroize"), allow(unused_mut))]
+        let mut inverse = montgomery.montgomery_invert();
+        let result = inverse.from_montgomery();
+
+        #[cfg(feature = "zeroize")]
+        {
+            montgomery.zeroize();
+            inverse.zeroize();
+        }
+
+        result
     }
 }
 
@@ -1277,7 +1372,12 @@ impl Field for Scalar {
         // NOTE: this is duplicated due to different `rng` bounds
         let mut scalar_bytes = [0u8; 64];
         rng.fill_bytes(&mut scalar_bytes);
-        Self::from_bytes_mod_order_wide(&scalar_bytes)
+        let scalar = Self::from_bytes_mod_order_wide(&scalar_bytes);
+
+        #[cfg(feature = "zeroize")]
+        scalar_bytes.zeroize();
+
+        scalar
     }
 
     fn square(&self) -> Self {