openpgp: Refactor memory protection.

  - Create a new module for memory protection.  Move common code from
    `Password` and `SessionKey` to a new type `Protected`.

4 files changed, 123 insertions, 71 deletions

diff --git a/openpgp/src/crypto/aead.rs b/openpgp/src/crypto/aead.rs
index 167a4f31..b6ca09d3 100644
--- a/openpgp/src/crypto/aead.rs
+++ b/openpgp/src/crypto/aead.rs
@@ -15,7 +15,7 @@ use conversions::{
 use Error;
 use Result;
 use crypto::SessionKey;
-use super::secure_cmp;
+use crypto::mem::secure_cmp;
 
 impl AEADAlgorithm {
     /// Returns the digest size of the AEAD algorithm.
diff --git a/openpgp/src/crypto/mem.rs b/openpgp/src/crypto/mem.rs
new file mode 100644
index 00000000..77469209
--- /dev/null
+++ b/openpgp/src/crypto/mem.rs
@@ -0,0 +1,98 @@
+//! Memory protection.
+
+use std::cmp::{min, Ordering};
+use std::fmt;
+use std::ops::{Deref, DerefMut};
+
+use memsec;
+
+/// Holds a session key.
+///
+/// The session key is cleared when dropped.
+#[derive(Clone, Eq)]
+pub struct Protected(Box<[u8]>);
+
+impl PartialEq for Protected {
+    fn eq(&self, other: &Self) -> bool {
+        secure_cmp(&self.0, &other.0) == Ordering::Equal
+    }
+}
+
+impl Protected {
+    /// Converts to a buffer for modification.
+    pub unsafe fn into_vec(mut self) -> Vec<u8> {
+        std::mem::replace(&mut self.0, vec![].into()).into()
+    }
+}
+
+impl Deref for Protected {
+    type Target = [u8];
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl AsRef<[u8]> for Protected {
+    fn as_ref(&self) -> &[u8] {
+        &self.0
+    }
+}
+
+impl DerefMut for Protected {
+    fn deref_mut(&mut self) -> &mut [u8] {
+        &mut self.0
+    }
+}
+
+impl From<Vec<u8>> for Protected {
+    fn from(v: Vec<u8>) -> Self {
+        Protected(v.into_boxed_slice())
+    }
+}
+
+impl From<Box<[u8]>> for Protected {
+    fn from(v: Box<[u8]>) -> Self {
+        Protected(v)
+    }
+}
+
+impl From<&[u8]> for Protected {
+    fn from(v: &[u8]) -> Self {
+        Vec::from(v).into()
+    }
+}
+
+impl Drop for Protected {
+    fn drop(&mut self) {
+        unsafe {
+            memsec::memzero(self.0.as_mut_ptr(), self.0.len());
+        }
+    }
+}
+
+impl fmt::Debug for Protected {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if cfg!(debug_assertions) {
+            write!(f, "{:?}", self.0)
+        } else {
+            f.write_str("[<Redacted>]")
+        }
+    }
+}
+
+/// Time-constant comparison.
+pub fn secure_cmp(a: &[u8], b: &[u8]) -> Ordering {
+    let ord1 = a.len().cmp(&b.len());
+    let ord2 = unsafe {
+        memsec::memcmp(a.as_ptr(), b.as_ptr(), min(a.len(), b.len()))
+    };
+    let ord2 = match ord2 {
+        0 => Ordering::Equal,
+        a if a < 0 => Ordering::Less,
+        a if a > 0 => Ordering::Greater,
+        _ => unreachable!(),
+    };
+
+    if ord1 == Ordering::Equal { ord2 } else { ord1 }
+}
diff --git a/openpgp/src/crypto/mod.rs b/openpgp/src/crypto/mod.rs
index 63f32393..cc1bdc18 100644
--- a/openpgp/src/crypto/mod.rs
+++ b/openpgp/src/crypto/mod.rs
@@ -3,9 +3,7 @@
 use std::io::Read;
 use std::ops::{Deref, DerefMut};
 use std::fmt;
-use std::cmp::{min, Ordering};
 
-use memsec;
 use nettle::{self, Random, Yarrow};
 
 use constants::HashAlgorithm;
@@ -17,6 +15,7 @@ pub(crate) mod ecdh;
 mod hash;
 mod keygrip;
 pub use self::keygrip::Keygrip;
+pub(crate) mod mem;
 pub mod mpis;
 pub mod s2k;
 pub mod sexp;
@@ -33,26 +32,20 @@ pub use self::hash::Hash;
 /// Holds a session key.
 ///
 /// The session key is cleared when dropped.
-#[derive(Clone, Eq)]
-pub struct SessionKey(Box<[u8]>);
-
-impl PartialEq for SessionKey {
-    fn eq(&self, other: &Self) -> bool {
-        secure_cmp(&self.0, &other.0) == Ordering::Equal
-    }
-}
+#[derive(Clone, PartialEq, Eq)]
+pub struct SessionKey(mem::Protected);
 
 impl SessionKey {
     /// Creates a new session key.
     pub fn new(rng: &mut Yarrow, size: usize) -> Self {
-        let mut sk = vec![0; size];
+        let mut sk: mem::Protected = vec![0; size].into();
         rng.random(&mut sk);
-        sk.into()
+        Self(sk)
     }
 
     /// Converts to a buffer for modification.
-    pub unsafe fn into_vec(mut self) -> Vec<u8> {
-        std::mem::replace(&mut self.0, vec![].into()).into()
+    pub unsafe fn into_vec(self) -> Vec<u8> {
+        self.0.into_vec()
     }
 }
 
@@ -76,15 +69,21 @@ impl DerefMut for SessionKey {
     }
 }
 
+impl From<mem::Protected> for SessionKey {
+    fn from(v: mem::Protected) -> Self {
+        SessionKey(v)
+    }
+}
+
 impl From<Vec<u8>> for SessionKey {
     fn from(v: Vec<u8>) -> Self {
-        SessionKey(v.into_boxed_slice())
+        SessionKey(v.into())
     }
 }
 
 impl From<Box<[u8]>> for SessionKey {
     fn from(v: Box<[u8]>) -> Self {
-        SessionKey(v)
+        SessionKey(v.into())
     }
 }
 
@@ -94,35 +93,17 @@ impl From<&[u8]> for SessionKey {
     }
 }
 
-impl Drop for SessionKey {
-    fn drop(&mut self) {
-        unsafe {
-            memsec::memzero(self.0.as_mut_ptr(), self.0.len());
-        }
-    }
-}
-
 impl fmt::Debug for SessionKey {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        if cfg!(debug_assertions) {
-            write!(f, "SessionKey ({:?})", self.0)
-        } else {
-            f.write_str("SessionKey ( <Redacted> )")
-        }
+        write!(f, "SessionKey ({:?})", self.0)
     }
 }
 
 /// Holds a password.
 ///
 /// The password is cleared when dropped.
-#[derive(Clone, Eq)]
-pub struct Password(Box<[u8]>);
-
-impl PartialEq for Password {
-    fn eq(&self, other: &Self) -> bool {
-        secure_cmp(&self.0, &other.0) == Ordering::Equal
-    }
-}
+#[derive(Clone, PartialEq, Eq)]
+pub struct Password(mem::Protected);
 
 impl AsRef<[u8]> for Password {
     fn as_ref(&self) -> &[u8] {
@@ -140,13 +121,13 @@ impl Deref for Password {
 
 impl From<Vec<u8>> for Password {
     fn from(v: Vec<u8>) -> Self {
-        Password(v.into_boxed_slice())
+        Password(v.into())
     }
 }
 
 impl From<Box<[u8]>> for Password {
     fn from(v: Box<[u8]>) -> Self {
-        Password(v)
+        Password(v.into())
     }
 }
 
@@ -168,21 +149,9 @@ impl From<&[u8]> for Password {
     }
 }
 
-impl Drop for Password {
-    fn drop(&mut self) {
-        unsafe {
-            memsec::memzero(self.0.as_mut_ptr(), self.0.len());
-        }
-    }
-}
-
 impl fmt::Debug for Password {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        if cfg!(debug_assertions) {
-            write!(f, "Password ({:?})", self.0)
-        } else {
-            f.write_str("Password ( <Redacted> )")
-        }
+        write!(f, "Password ({:?})", self.0)
     }
 }
 
@@ -240,19 +209,3 @@ fn hash_file_test() {
                    &::conversions::to_hex(&digest[..], false));
     }
 }
-
-/// Time-constant comparison.
-fn secure_cmp(a: &[u8], b: &[u8]) -> Ordering {
-    let ord1 = a.len().cmp(&b.len());
-    let ord2 = unsafe {
-        memsec::memcmp(a.as_ptr(), b.as_ptr(), min(a.len(), b.len()))
-    };
-    let ord2 = match ord2 {
-        0 => Ordering::Equal,
-        a if a < 0 => Ordering::Less,
-        a if a > 0 => Ordering::Greater,
-        _ => unreachable!(),
-    };
-
-    if ord1 == Ordering::Equal { ord2 } else { ord1 }
-}
diff --git a/openpgp/src/crypto/mpis.rs b/openpgp/src/crypto/mpis.rs
index c563ccb3..d93223ca 100644
--- a/openpgp/src/crypto/mpis.rs
+++ b/openpgp/src/crypto/mpis.rs
@@ -14,6 +14,7 @@ use constants::{
     SymmetricAlgorithm,
 };
 use crypto::Hash;
+use crypto::mem::secure_cmp;
 use serialize::Serialize;
 
 use nettle;
@@ -443,7 +444,7 @@ impl fmt::Debug for SecretKey {
 
 fn secure_mpi_cmp(a: &MPI, b: &MPI) -> Ordering {
     let ord1 = a.bits.cmp(&b.bits);
-    let ord2 = super::secure_cmp(&a.value, &b.value);
+    let ord2 = secure_cmp(&a.value, &b.value);
 
     if ord1 == Ordering::Equal { ord2 } else { ord1 }
 }
@@ -499,7 +500,7 @@ impl PartialOrd for SecretKey {
             }
             (&SecretKey::Unknown{ mpis: ref mpis1, rest: ref rest1 }
             ,&SecretKey::Unknown{ mpis: ref mpis2, rest: ref rest2 }) => {
-                let o1 = super::secure_cmp(rest1, rest2);
+                let o1 = secure_cmp(rest1, rest2);
                 let on = mpis1.iter().zip(mpis2.iter()).map(|(a,b)| {
                     secure_mpi_cmp(a, b)
                 }).collect::<Vec<_>>();