feat: zero out memory for secret key structures

Closes #43

Cargo.toml

@@ -48,8 +48,9 @@ sha3 = "0.8.1"
 rand = "0.6"
 smallvec = "0.6.9"
 cast5 = "0.6.0"
-rsa = "^0.1.3"
+rsa = "^0.1.4"
 nom = "^4.2"
+zeroize = { version = "0.10.1", features = ["zeroize_derive"] }
 
 [dependencies.x25519-dalek]
 version = "0.5"
@@ -66,8 +67,8 @@ version = "0.8.1"
 default-features = false
 
 [dependencies.num-bigint]
-version = "0.4"
-features = ["rand", "i128", "u64_digit", "prime"]
+version = "0.5"
+features = ["rand", "i128", "u64_digit", "prime", "zeroize"]
 package = "num-bigint-dig"
 
 [dependencies.flate2]
@@ -79,7 +80,6 @@ features = ["rust_backend"]
 version = "0.2.0"
 optional = true
 
-
 [dev-dependencies]
 hex-literal = "^0.2"
 serde = { version = "^1.0", features = ["derive"] }

src/crypto/aead.rs

@@ -7,3 +7,9 @@ pub enum AeadAlgorithm {
     Eax = 1,
     Ocb = 2,
 }
+
+impl Default for AeadAlgorithm {
+    fn default() -> Self {
+        AeadAlgorithm::None
+    }
+}

src/crypto/ecdh.rs

@@ -1,6 +1,7 @@
 use block_padding::{Padding, Pkcs7};
 use rand::{CryptoRng, Rng};
 use x25519_dalek::{PublicKey, StaticSecret};
+use zeroize::Zeroize;
 
 use crate::crypto::{aes_kw, ECCCurve, HashAlgorithm, PublicKeyAlgorithm, SymmetricKeyAlgorithm};
 use crate::errors::Result;
@@ -36,7 +37,7 @@ pub fn generate_key<R: Rng + CryptoRng>(rng: &mut R) -> (PublicParams, PlainSecr
             hash,
             alg_sym,
         },
-        PlainSecretParams::ECDH(Mpi::from_raw_slice(&q)),
+        PlainSecretParams::ECDH(Mpi::from_raw(q)),
     )
 }
 
@@ -99,9 +100,11 @@ pub fn decrypt(priv_key: &ECDHSecretKey, mpis: &[Mpi], fingerprint: &[u8]) -> Re
         let private_key = &priv_key.secret[..];
 
         // create scalar and reverse to little endian
-        let private_key_le = private_key.iter().rev().cloned().collect::<Vec<u8>>();
+        let mut private_key_le = private_key.iter().rev().cloned().collect::<Vec<u8>>();
         let mut private_key_arr = [0u8; 32];
         private_key_arr[..].copy_from_slice(&private_key_le);
+        private_key_le.zeroize();
+
         x25519_dalek::StaticSecret::from(private_key_arr)
     };
 

src/crypto/eddsa.rs

@@ -1,5 +1,6 @@
 use ed25519_dalek::Keypair;
 use rand::{CryptoRng, Rng};
+use zeroize::Zeroize;
 
 use crate::crypto::{ECCCurve, HashAlgorithm};
 use crate::errors::Result;
@@ -8,22 +9,23 @@ use crate::types::{EdDSASecretKey, Mpi, PlainSecretParams, PublicParams};
 /// Generate an EdDSA KeyPair.
 pub fn generate_key<R: Rng + CryptoRng>(rng: &mut R) -> (PublicParams, PlainSecretParams) {
     let keypair = Keypair::generate(rng);
-    let bytes = keypair.to_bytes();
+    let mut bytes = keypair.to_bytes();
 
     // public key
     let mut q = Vec::with_capacity(33);
     q.push(0x40);
     q.extend_from_slice(&bytes[32..]);
 
     // secret key
-    let p = &bytes[..32];
+    let p = Mpi::from_raw_slice(&bytes[..32]);
+    bytes.zeroize();
 
     (
         PublicParams::EdDSA {
             curve: ECCCurve::Ed25519,
             q: q.into(),
         },
-        PlainSecretParams::EdDSA(Mpi::from_raw_slice(p)),
+        PlainSecretParams::EdDSA(p),
     )
 }
 

src/crypto/hash.rs

@@ -33,6 +33,8 @@ pub enum HashAlgorithm {
     Private10 = 110,
 }
 
+impl zeroize::DefaultIsZeroes for HashAlgorithm {}
+
 impl Default for HashAlgorithm {
     fn default() -> Self {
         HashAlgorithm::SHA2_256

src/crypto/rsa.rs

@@ -15,9 +15,7 @@ pub fn decrypt(priv_key: &RSAPrivateKey, mpis: &[Mpi], _fingerprint: &[u8]) -> R
     ensure_eq!(mpis.len(), 1, "invalid input");
 
     let mpi = &mpis[0];
-    info!("RSA m^e mod n: {:?}", mpi);
     let m = priv_key.decrypt(PaddingScheme::PKCS1v15, mpi.as_bytes())?;
-    info!("m: {}", hex::encode(&m));
 
     Ok(m)
 }
@@ -29,8 +27,6 @@ pub fn encrypt<R: CryptoRng + Rng>(
     e: &[u8],
     plaintext: &[u8],
 ) -> Result<Vec<Vec<u8>>> {
-    info!("RSA encrypt");
-
     let key = RSAPublicKey::new(BigUint::from_bytes_be(n), BigUint::from_bytes_be(e))?;
     let data = key.encrypt(rng, PaddingScheme::PKCS1v15, plaintext)?;
 
@@ -72,8 +68,6 @@ pub fn verify(n: &[u8], e: &[u8], hash: HashAlgorithm, hashed: &[u8], sig: &[u8]
     let key = RSAPublicKey::new(BigUint::from_bytes_be(n), BigUint::from_bytes_be(e))?;
     let rsa_hash: Option<rsa::hash::Hashes> = hash.try_into().ok();
 
-    info!("n: {}", hex::encode(n));
-    info!("e: {}", hex::encode(e));
     key.verify(PaddingScheme::PKCS1v15, rsa_hash.as_ref(), &hashed[..], sig)
         .map_err(Into::into)
 }

src/crypto/sym.rs

@@ -13,10 +13,6 @@ use crate::errors::Result;
 
 macro_rules! decrypt {
     ($mode:ident, $key:expr, $iv:expr, $prefix:expr, $data:expr, $bs:expr, $resync:expr) => {{
-        info!("key {}", hex::encode($key));
-        info!("iv {}", hex::encode($iv));
-        info!("prefix {}", hex::encode(&$prefix));
-
         let mut mode = Cfb::<$mode>::new_var($key, $iv)?;
         mode.decrypt($prefix);
 
@@ -32,7 +28,6 @@ macro_rules! decrypt {
             "cfb decrypt, quick check part 2"
         );
 
-        info!("decypting: {}", hex::encode(&$data));
         if $resync {
             unimplemented!("CFB resync is not here");
         // info!("resync {}", hex::encode(&$prefix[2..$bs + 2]));
@@ -46,14 +41,9 @@ macro_rules! decrypt {
 
 macro_rules! encrypt {
     ($mode:ident, $key:expr, $iv:expr, $prefix:expr, $data:expr, $bs:expr, $resync:expr) => {{
-        info!("key {}", hex::encode($key));
-        info!("iv {}", hex::encode($iv));
-        info!("prefix {}", hex::encode(&$prefix));
-
         let mut mode = Cfb::<$mode>::new_var($key, $iv)?;
         mode.encrypt($prefix);
 
-        info!("encrypting: {}", hex::encode(&$data));
         if $resync {
             unimplemented!("CFB resync is not here");
         // info!("resync {}", hex::encode(&$prefix[2..$bs + 2]));
@@ -103,6 +93,8 @@ pub enum SymmetricKeyAlgorithm {
     Private10 = 110,
 }
 
+impl zeroize::DefaultIsZeroes for SymmetricKeyAlgorithm {}
+
 impl Default for SymmetricKeyAlgorithm {
     fn default() -> Self {
         SymmetricKeyAlgorithm::AES128
@@ -164,24 +156,13 @@ impl SymmetricKeyAlgorithm {
     /// Uses an IV of all zeroes, as specified in the openpgp cfb mode.
     /// Does not do resynchronization.
     pub fn decrypt_protected<'a>(self, key: &[u8], ciphertext: &'a mut [u8]) -> Result<&'a [u8]> {
-        info!("{}", hex::encode(&ciphertext));
         info!("protected decrypt");
         let iv_vec = vec![0u8; self.block_size()];
-        let cv_len = ciphertext.len();
         let (prefix, res) = self.decrypt_with_iv(key, &iv_vec, ciphertext, false)?;
-        info!("{}", hex::encode(&res));
+
         // MDC is 1 byte packet tag, 1 byte length prefix and 20 bytes SHA1 hash.
         let mdc_len = 22;
         let (data, mdc) = res.split_at(res.len() - mdc_len);
-        info!(
-            "decrypted {}b from {}b ({}|{})",
-            res.len(),
-            cv_len,
-            data.len(),
-            mdc.len()
-        );
-
-        info!("mdc: {}", hex::encode(mdc));
 
         ensure_eq!(mdc[0], 0xD3, "invalid MDC tag");
         ensure_eq!(mdc[1], 0x14, "invalid MDC length");
@@ -301,11 +282,6 @@ impl SymmetricKeyAlgorithm {
             }
         }
 
-        info!(
-            "{}\n{}",
-            hex::encode(&encrypted_prefix),
-            hex::encode(&encrypted_data)
-        );
         Ok((encrypted_prefix, encrypted_data))
     }
 
@@ -386,7 +362,6 @@ impl SymmetricKeyAlgorithm {
     }
 
     pub fn encrypt_protected<'a>(self, key: &[u8], plaintext: &'a [u8]) -> Result<Vec<u8>> {
-        info!("{}", hex::encode(&plaintext));
         info!("protected encrypt");
 
         // MDC is 1 byte packet tag, 1 byte length prefix and 20 bytes SHA1 hash.
@@ -416,15 +391,9 @@ impl SymmetricKeyAlgorithm {
         let checksum = &Sha1::digest(&ciphertext[..(prefix_len + plaintext_len + 2)])[..20];
         ciphertext[(prefix_len + plaintext_len + 2)..].copy_from_slice(checksum);
 
-        info!(
-            "mdc: {}",
-            hex::encode(&ciphertext[(prefix_len + plaintext_len)..])
-        );
         // IV is all zeroes
         let iv_vec = vec![0u8; self.block_size()];
 
-        info!("encrypting: {}", hex::encode(&ciphertext));
-
         self.encrypt_with_iv(key, &iv_vec, &mut ciphertext, false)?;
 
         Ok(ciphertext)

src/packet/secret_key_macro.rs

@@ -7,6 +7,21 @@ macro_rules! impl_secret_key {
             pub(crate) secret_params: $crate::types::SecretParams,
         }
 
+        impl zeroize::Zeroize for $name {
+            fn zeroize(&mut self) {
+                // details are not zeroed as they are public knowledge.
+
+                self.secret_params.zeroize();
+            }
+        }
+
+        impl Drop for $name {
+            fn drop(&mut self) {
+                use zeroize::Zeroize;
+                self.zeroize();
+            }
+        }
+
         impl $name {
             /// Parses a `SecretKey` packet from the given slice.
             pub fn from_slice(

src/types/mpi.rs

@@ -3,10 +3,11 @@ use std::{fmt, io};
 use byteorder::{BigEndian, WriteBytesExt};
 use nom::{self, be_u16, Err, InputIter, InputTake};
 use num_bigint::BigUint;
+use zeroize::Zeroize;
 
 use crate::errors;
 use crate::ser::Serialize;
-use crate::util::{bit_size, strip_leading_zeros};
+use crate::util::{bit_size, strip_leading_zeros, strip_leading_zeros_vec};
 
 /// Number of bits we accept when reading or writing MPIs.
 /// The value is the same as gnupgs.
@@ -55,7 +56,7 @@ pub fn mpi<'a>(input: &'a [u8]) -> nom::IResult<&'a [u8], MpiRef<'a>> {
 
 /// Represents an owned MPI value.
 /// The inner value is ready to be serialized, without the need to strip leading zeros.
-#[derive(Clone, PartialEq, Eq)]
+#[derive(Default, Clone, PartialEq, Eq, Zeroize)]
 pub struct Mpi(Vec<u8>);
 
 /// Represents a borrowed MPI value.
@@ -70,6 +71,11 @@ impl AsRef<[u8]> for Mpi {
 }
 
 impl Mpi {
+    pub fn from_raw(mut v: Vec<u8>) -> Self {
+        strip_leading_zeros_vec(&mut v);
+        Mpi(v)
+    }
+
     pub fn from_slice(slice: &[u8]) -> Self {
         Mpi(slice.to_vec())
     }

src/types/params/plain_secret.rs

@@ -4,14 +4,16 @@ use std::{fmt, io};
 use byteorder::{BigEndian, ByteOrder};
 use rand::{CryptoRng, Rng};
 use rsa::RSAPrivateKey;
+use zeroize::Zeroize;
 
 use crate::crypto::{checksum, ECCCurve, PublicKeyAlgorithm, SymmetricKeyAlgorithm};
 use crate::errors::Result;
 use crate::ser::Serialize;
 use crate::types::*;
 use crate::util::TeeWriter;
 
-#[derive(Clone, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq, Zeroize)]
+#[zeroize(drop)]
 pub enum PlainSecretParams {
     RSA { d: Mpi, p: Mpi, q: Mpi, u: Mpi },
     DSA(Mpi),

src/types/params/secret.rs

@@ -2,6 +2,7 @@ use std::io;
 
 use nom::{be_u8, rest_len};
 use num_traits::FromPrimitive;
+use zeroize::Zeroize;
 
 use crate::crypto::public_key::PublicKeyAlgorithm;
 use crate::crypto::sym::SymmetricKeyAlgorithm;
@@ -17,6 +18,15 @@ pub enum SecretParams {
     Encrypted(EncryptedSecretParams),
 }
 
+impl Zeroize for SecretParams {
+    fn zeroize(&mut self) {
+        match self {
+            SecretParams::Plain(p) => p.zeroize(),
+            SecretParams::Encrypted(_) => { /* encrypted params do not need zeroing */ }
+        }
+    }
+}
+
 impl SecretParams {
     pub fn is_encrypted(&self) -> bool {
         match self {

src/types/secret_key_repr.rs

@@ -2,6 +2,7 @@ use std::fmt;
 
 use num_bigint::BigUint;
 use rsa::RSAPrivateKey;
+use zeroize::Zeroize;
 
 use crate::crypto::hash::HashAlgorithm;
 use crate::crypto::sym::SymmetricKeyAlgorithm;
@@ -17,7 +18,8 @@ pub enum SecretKeyRepr {
 }
 
 /// Secret key for ECDH with Curve25519, the only combination we currently support.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Zeroize)]
+#[zeroize(drop)]
 pub struct ECDHSecretKey {
     /// The secret point.
     pub secret: [u8; 32],
@@ -27,15 +29,17 @@ pub struct ECDHSecretKey {
 }
 
 /// Secret key for EdDSA with Curve25519, the only combination we currently support.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Zeroize)]
+#[zeroize(drop)]
 pub struct EdDSASecretKey {
     /// The secret point.
     pub secret: [u8; 32],
     pub oid: Vec<u8>,
 }
 
 /// Secret key for DSA.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Zeroize)]
+#[zeroize(drop)]
 pub struct DSASecretKey {
     x: BigUint,
 }

src/util.rs

@@ -79,6 +79,15 @@ pub fn strip_leading_zeros(bytes: &[u8]) -> &[u8] {
     }
 }
 
+#[inline]
+pub fn strip_leading_zeros_vec(bytes: &mut Vec<u8>) {
+    if let Some(offset) = bytes.iter_mut().position(|b| b != &0) {
+        for i in 0..offset {
+            bytes.remove(i);
+        }
+    }
+}
+
 /// Convert a slice into an array.
 pub fn clone_into_array<A, T>(slice: &[T]) -> A
 where