openpgp: Explicitly implement Eq and Hash if PartialEq is.

  - If we don't derive PartialEq, then deriving hash is unsafe.

  - See #92.

6 files changed, 66 insertions, 8 deletions

diff --git a/openpgp/src/crypto/mem.rs b/openpgp/src/crypto/mem.rs
index a67cf993..4dfe5457 100644
--- a/openpgp/src/crypto/mem.rs
+++ b/openpgp/src/crypto/mem.rs
@@ -2,6 +2,7 @@
 
 use std::cmp::{min, Ordering};
 use std::fmt;
+use std::hash::{Hash, Hasher};
 use std::ops::{Deref, DerefMut};
 use std::pin::Pin;
 
@@ -11,7 +12,7 @@ use memsec;
 ///
 /// The memory is guaranteed not to be copied around, and is cleared
 /// when the object is dropped.
-#[derive(Clone, Eq, Hash)]
+#[derive(Clone)]
 pub struct Protected(Pin<Box<[u8]>>);
 
 impl PartialEq for Protected {
@@ -20,6 +21,14 @@ impl PartialEq for Protected {
     }
 }
 
+impl Eq for Protected {}
+
+impl Hash for Protected {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.0.hash(state);
+    }
+}
+
 impl Protected {
     /// Converts to a buffer for modification.
     pub unsafe fn into_vec(self) -> Vec<u8> {
diff --git a/openpgp/src/crypto/mpis.rs b/openpgp/src/crypto/mpis.rs
index 37f75213..4460cc58 100644
--- a/openpgp/src/crypto/mpis.rs
+++ b/openpgp/src/crypto/mpis.rs
@@ -20,7 +20,7 @@ use crate::Error;
 use crate::Result;
 
 /// Holds a single MPI.
-#[derive(Clone, Hash)]
+#[derive(Clone)]
 pub struct MPI {
     /// Integer value as big-endian.
     value: Box<[u8]>,
@@ -216,10 +216,16 @@ impl PartialEq for MPI {
 
 impl Eq for MPI {}
 
+impl std::hash::Hash for MPI {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.value.hash(state);
+    }
+}
+
 /// Holds a single MPI containing secrets.
 ///
 /// The memory will be cleared when the object is dropped.
-#[derive(Clone, Hash)]
+#[derive(Clone)]
 pub struct ProtectedMPI {
     /// Integer value as big-endian.
     value: Protected,
@@ -245,6 +251,12 @@ impl From<MPI> for ProtectedMPI {
     }
 }
 
+impl std::hash::Hash for ProtectedMPI {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.value.hash(state);
+    }
+}
+
 impl ProtectedMPI {
     /// Returns the length of the MPI in bits.
     pub fn bits(&self) -> usize {
@@ -481,6 +493,8 @@ impl Arbitrary for PublicKey {
 ///
 /// Provides a typed and structured way of storing multiple MPIs in
 /// packets.
+// Deriving Hash here is okay: PartialEq is manually implemented to
+// ensure that secrets are compared in constant-time.
 #[derive(Clone, Hash)]
 pub enum SecretKeyMaterial {
     /// RSA secret key.
diff --git a/openpgp/src/packet/key/mod.rs b/openpgp/src/packet/key/mod.rs
index a4e0272d..4d2ae31a 100644
--- a/openpgp/src/packet/key/mod.rs
+++ b/openpgp/src/packet/key/mod.rs
@@ -1171,7 +1171,7 @@ impl SecretKeyMaterial {
 /// demand.  See [`crypto::mem::Encrypted`] for details.
 ///
 ///  [`crypto::mem::Encrypted`]: ../../crypto/mem/struct.Encrypted.html
-#[derive(Eq, Hash, Clone, Debug)]
+#[derive(Clone, Debug)]
 pub struct Unencrypted {
     /// MPIs of the secret key.
     mpis: mem::Encrypted,
@@ -1183,6 +1183,14 @@ impl PartialEq for Unencrypted {
     }
 }
 
+impl Eq for Unencrypted {}
+
+impl std::hash::Hash for Unencrypted {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.map(|k| std::hash::Hash::hash(k, state));
+    }
+}
+
 impl From<mpis::SecretKeyMaterial> for Unencrypted {
     fn from(mpis: mpis::SecretKeyMaterial) -> Self {
         use crate::serialize::Serialize;
diff --git a/openpgp/src/packet/one_pass_sig.rs b/openpgp/src/packet/one_pass_sig.rs
index b2430a82..68f69f77 100644
--- a/openpgp/src/packet/one_pass_sig.rs
+++ b/openpgp/src/packet/one_pass_sig.rs
@@ -5,6 +5,7 @@
 //!   [Section 5.4 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.4
 
 use std::fmt;
+use std::hash::{Hash, Hasher};
 use quickcheck::{Arbitrary, Gen};
 
 use crate::Error;
@@ -23,7 +24,7 @@ use crate::serialize::SerializeInto;
 /// See [Section 5.4 of RFC 4880] for details.
 ///
 ///   [Section 5.4 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.4
-#[derive(Eq, Hash, Clone)]
+#[derive(Clone)]
 pub struct OnePassSig3 {
     /// CTB packet header fields.
     pub(crate) common: packet::Common,
@@ -65,6 +66,18 @@ impl PartialEq for OnePassSig3 {
     }
 }
 
+impl Eq for OnePassSig3 {}
+
+impl Hash for OnePassSig3 {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.typ.hash(state);
+        self.hash_algo.hash(state);
+        self.pk_algo.hash(state);
+        self.issuer.hash(state);
+        self.last.hash(state);
+    }
+}
+
 impl OnePassSig3 {
     /// Returns a new `Signature` packet.
     pub fn new(typ: SignatureType) ->  Self {
diff --git a/openpgp/src/packet/tag.rs b/openpgp/src/packet/tag.rs
index fb6bc96a..17ce2293 100644
--- a/openpgp/src/packet/tag.rs
+++ b/openpgp/src/packet/tag.rs
@@ -1,5 +1,6 @@
 use std::fmt;
 use std::cmp::Ordering;
+use std::hash::{Hash, Hasher};
 
 use quickcheck::{Arbitrary, Gen};
 
@@ -8,7 +9,7 @@ use quickcheck::{Arbitrary, Gen};
 ///   [Section 4.3 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-4.3
 ///
 /// The values correspond to the serialized format.
-#[derive(Clone, Copy, Debug, Hash)]
+#[derive(Clone, Copy, Debug)]
 pub enum Tag {
     /// Reserved Packet tag.
     Reserved,
@@ -78,6 +79,13 @@ impl Ord for Tag
     }
 }
 
+impl Hash for Tag {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        let t: u8 = (*self).into();
+        t.hash(state);
+    }
+}
+
 impl From<u8> for Tag {
     fn from(u: u8) -> Self {
         use crate::packet::Tag::*;
diff --git a/openpgp/src/packet/user_attribute.rs b/openpgp/src/packet/user_attribute.rs
index eca5ff02..cfbe608f 100644
--- a/openpgp/src/packet/user_attribute.rs
+++ b/openpgp/src/packet/user_attribute.rs
@@ -6,6 +6,7 @@
 
 use std::fmt;
 use std::cmp::Ordering;
+use std::hash::{Hash, Hasher};
 
 use quickcheck::{Arbitrary, Gen};
 use rand::Rng;
@@ -27,7 +28,7 @@ use crate::serialize::SerializeInto;
 /// See [Section 5.12 of RFC 4880] for details.
 ///
 ///   [Section 5.12 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.12
-#[derive(Hash, Clone)]
+#[derive(Clone)]
 pub struct UserAttribute {
     /// CTB packet header fields.
     pub(crate) common: packet::Common,
@@ -59,7 +60,12 @@ impl PartialEq for UserAttribute {
     }
 }
 
-impl Eq for UserAttribute {
+impl Eq for UserAttribute {}
+
+impl Hash for UserAttribute {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.value.hash(state);
+    }
 }
 
 impl PartialOrd for UserAttribute {