openpgp: Fix comparing streamed containers.

  - Compute a digest over the streamed data and use it to implement
    equality.

  - Fixes #93.

9 files changed, 140 insertions, 146 deletions

diff --git a/openpgp/src/lib.rs b/openpgp/src/lib.rs
index 3bca389f..f503f77f 100644
--- a/openpgp/src/lib.rs
+++ b/openpgp/src/lib.rs
@@ -285,19 +285,8 @@ pub enum Error {
 /// than a `CompressedData` packet.
 ///
 ///   [Section 5 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5
-///
-/// # A note on partial equality
-///
-/// Container packets, like the [`CompressedData`] and the [`SEIP`]
-/// packet, can be streamed.  If a packet is streamed, we no longer
-/// have access to the content, and therefore cannot compare it to
-/// other packets.  Consequently, a streamed packet is not considered
-/// equal to any other packet.
-///
-///   [`CompressedData`]: packet/struct.CompressedData.html
-///   [`SEIP`]: packet/enum.SEIP.html
 #[derive(Debug)]
-#[derive(PartialEq, Hash, Clone)]
+#[derive(PartialEq, Eq, Hash, Clone)]
 pub enum Packet {
     /// Unknown packet.
     Unknown(packet::Unknown),
diff --git a/openpgp/src/packet/aed.rs b/openpgp/src/packet/aed.rs
index c5d69391..1dfe579b 100644
--- a/openpgp/src/packet/aed.rs
+++ b/openpgp/src/packet/aed.rs
@@ -17,14 +17,7 @@ use crate::Result;
 /// of RFC 4880bis] for details.
 ///
 /// [Section 5.16 of RFC 4880bis]: https://tools.ietf.org/html/draft-ietf-openpgp-rfc4880bis-05#section-5.16
-///
-/// # A note on partial equality
-///
-/// Container packets, like this one, can be streamed.  If a packet is
-/// streamed, we no longer have access to the content, and therefore
-/// cannot compare it to other packets.  Consequently, a streamed
-/// packet is not considered equal to any other packet.
-#[derive(PartialEq, Hash, Clone, Debug)]
+#[derive(PartialEq, Eq, Hash, Clone, Debug)]
 pub struct AED1 {
     /// CTB packet header fields.
     pub(crate) common: packet::Common,
diff --git a/openpgp/src/packet/compressed_data.rs b/openpgp/src/packet/compressed_data.rs
index 684b0358..70691ccb 100644
--- a/openpgp/src/packet/compressed_data.rs
+++ b/openpgp/src/packet/compressed_data.rs
@@ -15,14 +15,7 @@ use crate::types::CompressionAlgorithm;
 /// of a `CompressedData` packet.
 ///
 /// [Section 5.6 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.6
-///
-/// # A note on partial equality
-///
-/// Container packets, like this one, can be streamed.  If a packet is
-/// streamed, we no longer have access to the content, and therefore
-/// cannot compare it to other packets.  Consequently, a streamed
-/// packet is not considered equal to any other packet.
-#[derive(PartialEq, Hash, Clone)]
+#[derive(PartialEq, Eq, Hash, Clone)]
 pub struct CompressedData {
     /// CTB packet header fields.
     pub(crate) common: packet::Common,
@@ -40,6 +33,7 @@ impl fmt::Debug for CompressedData {
             .field("children", &self.container.children_ref())
             .field("body (bytes)",
                    &self.container.body().len())
+            .field("body_digest", &self.container.body_digest())
             .finish()
     }
 }
diff --git a/openpgp/src/packet/container.rs b/openpgp/src/packet/container.rs
index b5df17eb..3f77b098 100644
--- a/openpgp/src/packet/container.rs
+++ b/openpgp/src/packet/container.rs
@@ -4,26 +4,22 @@
 //! structure.
 
 use std::fmt;
+use std::hash::{Hash, Hasher};
 use std::slice;
 use std::vec;
 
 use crate::{
     Packet,
+    crypto::hash,
     packet::Iter,
+    types::HashAlgorithm,
 };
 
 /// Holds zero or more OpenPGP packets.
 ///
 /// This is used by OpenPGP container packets, like the compressed
 /// data packet, to store the containing packets.
-///
-/// # A note on partial equality
-///
-/// Container packets, like this one, can be streamed.  If a packet is
-/// streamed, we no longer have access to the content, and therefore
-/// cannot compare it to other packets.  Consequently, a streamed
-/// packet is not considered equal to any other packet.
-#[derive(Hash, Clone)]
+#[derive(Clone)]
 pub(crate) struct Container {
     /// Used by container packets (such as the encryption and
     /// compression packets) to reference their immediate children.
@@ -87,22 +83,25 @@ pub(crate) struct Container {
     /// content.
     body: Vec<u8>,
 
-    /// Remembers whether or not this packet has been streamed.
-    ///
-    /// If it has, then we lost (parts of) the content, and cannot
-    /// compare it to other packets.
-    was_streamed: bool,
+    /// We compute a digest over the body to implement comparison.
+    body_digest: Vec<u8>,
 }
 
+const CONTAINER_BODY_HASH: HashAlgorithm = HashAlgorithm::SHA256;
+
 impl PartialEq for Container {
     fn eq(&self, other: &Container) -> bool {
-        if self.was_streamed || other.was_streamed {
-            // If either was streamed, consider them not equal.
-            false
-        } else {
-            self.packets == other.packets
-                && self.body == other.body
-        }
+        self.packets == other.packets
+            && self.body_digest == other.body_digest
+    }
+}
+
+impl Eq for Container {}
+
+impl Hash for Container {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.packets.hash(state);
+        self.body_digest.hash(state);
     }
 }
 
@@ -111,7 +110,7 @@ impl Default for Container {
         Self {
             packets: Vec::with_capacity(0),
             body: Vec::with_capacity(0),
-            was_streamed: false,
+            body_digest: Self::empty_body_digest(),
         }
     }
 }
@@ -121,7 +120,7 @@ impl From<Vec<Packet>> for Container {
         Self {
             packets,
             body: Vec::with_capacity(0),
-            was_streamed: false,
+            body_digest: Self::empty_body_digest(),
         }
     }
 }
@@ -139,6 +138,7 @@ impl fmt::Debug for Container {
         f.debug_struct("Container")
             .field("packets", &self.packets)
             .field("body", &prefix_fmt)
+            .field("body_digest", &self.body_digest())
             .finish()
     }
 }
@@ -191,20 +191,43 @@ impl Container {
     /// descendants.
     pub fn set_body(&mut self, data: Vec<u8>) -> Vec<u8> {
         self.packets.clear();
+        let mut h = Self::make_body_hash();
+        h.update(&data);
+        self.set_body_hash(h);
         std::mem::replace(&mut self.body, data)
     }
 
-    /// Returns whether or not this packet has been streamed.
-    ///
-    /// If it has, then we lost (parts of) the content, and cannot
-    /// compare it to other packets.
-    pub fn was_streamed(&self) -> bool {
-        self.was_streamed
+    /// Returns the hash for the empty body.
+    fn empty_body_digest() -> Vec<u8> {
+        lazy_static!{
+            static ref DIGEST: Vec<u8> = {
+                let mut h = Container::make_body_hash();
+                let mut d = vec![0; h.digest_size()];
+                h.digest(&mut d);
+                d
+            };
+        }
+
+        DIGEST.clone()
     }
 
-    /// Sets whether or not this packet has been streamed.
-    pub(crate) fn set_streamed(&mut self, value: bool) {
-        self.was_streamed = value;
+    /// Creates a hash context for hashing the body.
+    pub(crate) // For parse.rs
+    fn make_body_hash() -> hash::Context {
+        CONTAINER_BODY_HASH.context()
+            .expect("CONTAINER_BODY_HASH must be implemented")
+    }
+
+    /// Hashes content that has been streamed.
+    pub(crate) // For parse.rs
+    fn set_body_hash(&mut self, mut h: hash::Context) {
+        self.body_digest.resize(h.digest_size(), 0);
+        h.digest(&mut self.body_digest);
+    }
+
+    pub(crate)
+    fn body_digest(&self) -> String {
+        crate::fmt::hex::encode(&self.body_digest)
     }
 
     pub(crate) // For parse.rs
@@ -262,14 +285,6 @@ macro_rules! the_common_container_forwards {
         pub fn set_body(&mut self, data: Vec<u8>) -> Vec<u8> {
             self.container.set_body(data)
         }
-
-        /// Returns whether or not this packet has been streamed.
-        ///
-        /// If it has, then we lost (parts of) the content, and cannot
-        /// compare it to other packets.
-        pub fn was_streamed(&self) -> bool {
-            self.container.was_streamed()
-        }
     };
 }
 
@@ -363,12 +378,4 @@ impl Packet {
     pub(crate) fn body(&self) -> Option<&[u8]> {
         self.container_ref().map(|c| c.body())
     }
-
-    /// Returns whether or not this packet has been streamed.
-    ///
-    /// If it has, then we lost (parts of) the content, and cannot
-    /// compare it to other packets.
-    pub fn was_streamed(&self) -> bool {
-        self.container_ref().map(|c| c.was_streamed()).unwrap_or(false)
-    }
 }
diff --git a/openpgp/src/packet/literal.rs b/openpgp/src/packet/literal.rs
index 45222753..729cbc6f 100644
--- a/openpgp/src/packet/literal.rs
+++ b/openpgp/src/packet/literal.rs
@@ -21,14 +21,7 @@ use crate::Result;
 /// See [Section 5.9 of RFC 4880] for details.
 ///
 ///   [Section 5.9 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.9
-///
-/// # A note on partial equality
-///
-/// Container packets, like this one, can be streamed.  If a packet is
-/// streamed, we no longer have access to the content, and therefore
-/// cannot compare it to other packets.  Consequently, a streamed
-/// packet is not considered equal to any other packet.
-#[derive(PartialEq, Hash, Clone)]
+#[derive(PartialEq, Eq, Hash, Clone)]
 pub struct Literal {
     /// CTB packet header fields.
     pub(crate) common: packet::Common,
@@ -60,24 +53,20 @@ impl fmt::Debug for Literal {
         };
 
         let threshold = 36;
-        let body_str = if self.was_streamed() {
-            "(streamed)".into()
-        } else {
-            let body = self.body();
-            let prefix = &body[..cmp::min(threshold, body.len())];
-            let mut prefix_fmt = String::from_utf8_lossy(prefix).into_owned();
-            if body.len() > threshold {
-                prefix_fmt.push_str("...");
-            }
-            prefix_fmt.push_str(&format!(" ({} bytes)", body.len())[..]);
-            prefix_fmt
-        };
+        let body = self.body();
+        let prefix = &body[..cmp::min(threshold, body.len())];
+        let mut prefix_fmt = String::from_utf8_lossy(prefix).into_owned();
+        if body.len() > threshold {
+            prefix_fmt.push_str("...");
+        }
+        prefix_fmt.push_str(&format!(" ({} bytes)", body.len())[..]);
 
         f.debug_struct("Literal")
             .field("format", &self.format)
             .field("filename", &filename)
             .field("date", &self.date)
-            .field("body", &body_str)
+            .field("body", &prefix_fmt)
+            .field("body_digest", &self.container.body_digest())
             .finish()
     }
 }
diff --git a/openpgp/src/packet/mod.rs b/openpgp/src/packet/mod.rs
index fc55a376..86f028de 100644
--- a/openpgp/src/packet/mod.rs
+++ b/openpgp/src/packet/mod.rs
@@ -608,7 +608,7 @@ impl<P: key::KeyParts, R: key::KeyRole> DerefMut for Key<P, R> {
 /// 4880] for details.
 ///
 /// [Section 5.13 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.13
-#[derive(PartialEq, Hash, Clone, Debug)]
+#[derive(PartialEq, Eq, Hash, Clone, Debug)]
 pub enum SEIP {
     /// SEIP packet version 1.
     V1(self::seip::SEIP1),
@@ -655,7 +655,7 @@ impl DerefMut for SEIP {
 /// of RFC 4880bis] for details.
 ///
 /// [Section 5.16 of RFC 4880bis]: https://tools.ietf.org/html/draft-ietf-openpgp-rfc4880bis-05#section-5.16
-#[derive(PartialEq, Hash, Clone, Debug)]
+#[derive(PartialEq, Eq, Hash, Clone, Debug)]
 pub enum AED {
     /// AED packet version 1.
     V1(self::aed::AED1),
diff --git a/openpgp/src/packet/seip.rs b/openpgp/src/packet/seip.rs
index a9ebada2..5a1872e7 100644
--- a/openpgp/src/packet/seip.rs
+++ b/openpgp/src/packet/seip.rs
@@ -15,14 +15,7 @@ use crate::Packet;
 /// 4880] for details.
 ///
 /// [Section 5.13 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.13
-///
-/// # A note on partial equality
-///
-/// Container packets, like this one, can be streamed.  If a packet is
-/// streamed, we no longer have access to the content, and therefore
-/// cannot compare it to other packets.  Consequently, a streamed
-/// packet is not considered equal to any other packet.
-#[derive(PartialEq, Hash, Clone, Debug)]
+#[derive(PartialEq, Eq, Hash, Clone, Debug)]
 pub struct SEIP1 {
     /// CTB packet header fields.
     pub(crate) common: packet::Common,
diff --git a/openpgp/src/packet/unknown.rs b/openpgp/src/packet/unknown.rs
index f528f0b2..6f482c12 100644
--- a/openpgp/src/packet/unknown.rs
+++ b/openpgp/src/packet/unknown.rs
@@ -37,6 +37,8 @@ impl PartialEq for Unknown {
     }
 }
 
+impl Eq for Unknown { }
+
 impl Hash for Unknown {
     fn hash<H: Hasher>(&self, state: &mut H) {
         self.common.hash(state);
diff --git a/openpgp/src/parse/parse.rs b/openpgp/src/parse/parse.rs
index 415eed3a..a7b45ce2 100644
--- a/openpgp/src/parse/parse.rs
+++ b/openpgp/src/parse/parse.rs
@@ -20,7 +20,10 @@ use crate::{
     },
     crypto::s2k::S2K,
     Error,
-    packet::Header,
+    packet::{
+        Container,
+        Header,
+    },
     packet::signature::Signature4,
     packet::prelude::*,
     Packet,
@@ -327,6 +330,7 @@ impl<'a> PacketHeaderParser<'a> {
             decrypted: true,
             finished: false,
             map: self.map,
+            body_hash: None,
             state: self.state,
         })
     }
@@ -2677,6 +2681,10 @@ pub struct PacketParser<'a> {
     /// A map of this packet.
     map: Option<map::Map>,
 
+    /// We compute a hashsum over the body to implement comparison on
+    /// containers that have been streamed..
+    body_hash: Option<crate::crypto::hash::Context>,
+
     state: PacketParserState,
 }
 
@@ -3603,11 +3611,7 @@ impl <'a> PacketParser<'a> {
     /// # return Ok(());
     /// # }
     pub fn buffer_unread_content(&mut self) -> Result<&[u8]> {
-        // If the packet has not yet been streamed, then the following
-        // read operation should not be considered streaming.
-        let content_was_read = self.content_was_read;
         let mut rest = self.steal_eof()?;
-        self.set_content_was_read(content_was_read);
 
         match &mut self.packet {
             Packet::Literal(p) => {
@@ -3693,10 +3697,6 @@ impl <'a> PacketParser<'a> {
 
         let recursion_depth = self.recursion_depth();
 
-        // If there is no unread content left at this point, we want
-        // to preserve the content_was_read flag.
-        let content_was_read = self.content_was_read;
-
         let unread_content = if self.state.settings.buffer_unread_content {
             t!("({:?} at depth {}): buffering {} bytes of unread content",
                self.packet.tag(), recursion_depth,
@@ -3708,12 +3708,7 @@ impl <'a> PacketParser<'a> {
                self.packet.tag(), recursion_depth,
                self.data_eof().unwrap().len());
 
-            let dropped = self.drop_eof()?;
-            if ! dropped {
-                // Nothing was dropped, restore.
-                self.set_content_was_read(content_was_read);
-            }
-            dropped
+            self.drop_eof()?
         };
 
         if unread_content {
@@ -3730,25 +3725,28 @@ impl <'a> PacketParser<'a> {
             }
         }
 
+        if let Some(c) = self.packet.container_mut() {
+            let h = self.body_hash.take()
+                .unwrap_or_else(Container::make_body_hash);
+            c.set_body_hash(h);
+        }
+
         self.finished = true;
 
         Ok(&mut self.packet)
     }
 
-    /// Sets the content_was_read flag if `cond` is true.
-    fn mark_content_was_read(&mut self, cond: bool) {
-        if cond {
-            self.packet.container_mut().map(|c| c.set_streamed(true));
+    /// Hashes content that has been streamed.
+    fn hash_read_content(&mut self, b: &[u8]) {
+        if b.len() > 0 {
+            if self.body_hash.is_none() {
+                self.body_hash = Some(Container::make_body_hash());
+            }
+            self.body_hash.as_mut().map(|c| c.update(b));
             self.content_was_read = true;
         }
     }
 
-    /// Sets the content_was_read flag to `value`.
-    fn set_content_was_read(&mut self, value: bool) {
-        self.packet.container_mut().map(|c| c.set_streamed(value));
-        self.content_was_read = value;
-    }
-
     /// Returns a reference to the current packet's header.
     pub fn header(&self) -> &Header {
         &self.header
@@ -3773,8 +3771,9 @@ impl <'a> PacketParser<'a> {
 /// `BufferedReader` interfaces.
 impl<'a> io::Read for PacketParser<'a> {
     fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
-        self.mark_content_was_read(true);
-        return buffered_reader_generic_read_impl(self, buf);
+        let v = buffered_reader_generic_read_impl(self, buf)?;
+        self.hash_read_content(&buf[..v]);
+        Ok(v)
     }
 }
 
@@ -3808,43 +3807,71 @@ impl<'a> BufferedReader<Cookie> for PacketParser<'a> {
     }
 
     fn consume(&mut self, amount: usize) -> &[u8] {
-        self.mark_content_was_read(amount > 0);
+        // This is awkward.  Juggle mutable references around.
+        if let Some(mut body_hash) = self.body_hash.take() {
+            let data = self.data_hard(amount)
+                .expect("It is an error to consume more than data returns");
+            let read_something = data.len() > 0;
+            body_hash.update(data);
+            self.body_hash = Some(body_hash);
+            self.content_was_read |= read_something;
+        }
+
         self.reader.consume(amount)
     }
 
     fn data_consume(&mut self, amount: usize) -> io::Result<&[u8]> {
-        self.mark_content_was_read(amount > 0);
+        // This is awkward.  Juggle mutable references around.
+        if let Some(mut body_hash) = self.body_hash.take() {
+            let data = self.data(amount)?;
+            let read_something = data.len() > 0;
+            body_hash.update(data);
+            self.body_hash = Some(body_hash);
+            self.content_was_read |= read_something;
+        }
+
         self.reader.data_consume(amount)
     }
 
     fn data_consume_hard(&mut self, amount: usize) -> io::Result<&[u8]> {
-        self.mark_content_was_read(amount > 0);
+        // This is awkward.  Juggle mutable references around.
+        if let Some(mut body_hash) = self.body_hash.take() {
+            let data = self.data_hard(amount)?;
+            let read_something = data.len() > 0;
+            body_hash.update(data);
+            self.body_hash = Some(body_hash);
+            self.content_was_read |= read_something;
+        }
+
         self.reader.data_consume_hard(amount)
     }
 
     fn read_be_u16(&mut self) -> io::Result<u16> {
-        self.mark_content_was_read(true);
-        self.reader.read_be_u16()
+        let v = self.reader.read_be_u16()?;
+        self.hash_read_content(&v.to_be_bytes());
+        Ok(v)
     }
 
     fn read_be_u32(&mut self) -> io::Result<u32> {
-        self.mark_content_was_read(true);
-        self.reader.read_be_u32()
+        let v = self.reader.read_be_u32()?;
+        self.hash_read_content(&v.to_be_bytes());
+        Ok(v)
     }
 
     fn steal(&mut self, amount: usize) -> io::Result<Vec<u8>> {
-        self.mark_content_was_read(amount > 0);
-        self.reader.steal(amount)
+        let v = self.reader.steal(amount)?;
+        self.hash_read_content(&v);
+        Ok(v)
     }
 
     fn steal_eof(&mut self) -> io::Result<Vec<u8>> {
-        self.mark_content_was_read(true);
-        self.reader.steal_eof()
+        let v = self.reader.steal_eof()?;
+        self.hash_read_content(&v);
+        Ok(v)
     }
 
     fn drop_eof(&mut self) -> io::Result<bool> {
-        self.mark_content_was_read(true);
-        self.reader.drop_eof()
+        Ok(! self.steal_eof()?.is_empty())
     }
 
     fn get_mut(&mut self) -> Option<&mut dyn BufferedReader<Cookie>> {