fix(parser): there should be no edata except after ESKs

And the Edata should unwrap into exactly one Message. All other cases should yield errors.

Closes #298

src/composed/message/decrypt.rs

@@ -1,13 +1,8 @@
-use std::boxed::Box;
-use std::io::Cursor;
-
-use crate::composed::message::types::{Edata, Message};
-use crate::composed::shared::Deserializable;
 use crate::crypto::sym::SymmetricKeyAlgorithm;
 use crate::crypto::{checksum, ecdh, rsa};
 use crate::errors::Result;
 use crate::packet::SymKeyEncryptedSessionKey;
-use crate::types::{KeyTrait, Mpi, SecretKeyRepr, SecretKeyTrait, Tag};
+use crate::types::{KeyTrait, Mpi, SecretKeyRepr, SecretKeyTrait};
 
 /// Decrypts session key using secret key.
 pub fn decrypt_session_key<F>(
@@ -112,62 +107,3 @@ where
 
     Ok((decrypted_key[1..].to_vec(), session_key_algorithm))
 }
-
-pub struct MessageDecrypter<'a> {
-    key: Vec<u8>,
-    alg: SymmetricKeyAlgorithm,
-    edata: &'a [Edata],
-    // position in the edata slice
-    pos: usize,
-    // the current msgs that are already decrypted
-    current_msgs: Option<Box<dyn Iterator<Item = Result<Message>>>>,
-}
-
-impl<'a> MessageDecrypter<'a> {
-    pub fn new(session_key: Vec<u8>, alg: SymmetricKeyAlgorithm, edata: &'a [Edata]) -> Self {
-        MessageDecrypter {
-            key: session_key,
-            alg,
-            edata,
-            pos: 0,
-            current_msgs: None,
-        }
-    }
-}
-
-impl<'a> Iterator for MessageDecrypter<'a> {
-    type Item = Result<Message>;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        if self.pos >= self.edata.len() && self.current_msgs.is_none() {
-            return None;
-        }
-
-        if self.current_msgs.is_none() {
-            // need to decrypt another packet
-            let packet = &self.edata[self.pos];
-            self.pos += 1;
-
-            let mut res = packet.data()[..].to_vec();
-            let protected = packet.tag() == Tag::SymEncryptedProtectedData;
-
-            debug!("decrypting protected = {:?}", protected);
-
-            let decrypted_packet: &[u8] = if protected {
-                err_opt!(self.alg.decrypt_protected(&self.key, &mut res))
-            } else {
-                err_opt!(self.alg.decrypt(&self.key, &mut res))
-            };
-
-            self.current_msgs = Some(Message::from_bytes_many(Cursor::new(
-                decrypted_packet.to_vec(),
-            )));
-        };
-
-        let mut msgs = self.current_msgs.take().expect("just checked");
-        let next = msgs.next();
-        self.current_msgs = Some(msgs);
-
-        next
-    }
-}

src/composed/message/parser.rs

@@ -4,9 +4,10 @@ use std::iter::Peekable;
 
 use crate::composed::message::Message;
 use crate::composed::Deserializable;
-use crate::errors::Result;
+use crate::errors::{Error, Result};
 use crate::packet::Packet;
 use crate::types::Tag;
+use crate::Edata;
 
 pub struct MessageParser<I: Sized + Iterator<Item = Result<Packet>>> {
     source: Peekable<I>,
@@ -40,7 +41,6 @@ fn next<I: Iterator<Item = Result<Packet>>>(packets: &mut Peekable<I>) -> Option
                 return match packet.try_into() {
                     Ok(p) => {
                         let mut esk = vec![p];
-                        let mut edata = Vec::new();
 
                         // while ESK take em
                         while let Some(res) = packets.next_if(|res| {
@@ -55,44 +55,27 @@ fn next<I: Iterator<Item = Result<Packet>>>(packets: &mut Peekable<I>) -> Option
                             }
                         }
 
-                        // while edata take em (FIXME: the message grammar only allows one "Encrypted Data" packet)
-                        while let Some(res) = packets.next_if(|res| {
-                            res.as_ref().is_ok_and(|p| {
-                                p.tag() == Tag::SymEncryptedData
-                                    || p.tag() == Tag::SymEncryptedProtectedData
-                            })
-                        }) {
-                            match res {
-                                Ok(packet) => edata.push(packet.try_into().expect("peeked")),
-                                Err(e) => return Some(Err(e)),
+                        // we expect exactly one edata after the ESKs
+                        let edata = match packets.next() {
+                            Some(Ok(p))
+                                if p.tag() == Tag::SymEncryptedData
+                                    || p.tag() == Tag::SymEncryptedProtectedData =>
+                            {
+                                Edata::try_from(p).expect("peeked")
                             }
-                        }
-
-                        Some(Ok(Message::Encrypted { esk, edata }))
-                    }
-                    Err(err) => Some(Err(err)),
-                };
-            }
-            //    Encrypted Data :- Symmetrically Encrypted Data Packet |
-            //          Symmetrically Encrypted Integrity Protected Data Packet
-            Tag::SymEncryptedData | Tag::SymEncryptedProtectedData => {
-                return match packet.try_into() {
-                    Ok(p) => {
-                        let esk = Vec::new();
-                        let mut edata = vec![p];
-
-                        // while edata take em (FIXME: the message grammar only allows one "Encrypted Data" packet)
-                        while let Some(res) = packets.next_if(|res| {
-                            res.as_ref().is_ok_and(|p| {
-                                p.tag() == Tag::SymEncryptedData
-                                    || p.tag() == Tag::SymEncryptedProtectedData
-                            })
-                        }) {
-                            match res {
-                                Ok(packet) => edata.push(packet.try_into().expect("peeked")),
-                                Err(e) => return Some(Err(e)),
+                            Some(Ok(p)) => {
+                                return Some(Err(Error::Message(format!(
+                                    "Expected encrypted data packet, but found {:?}",
+                                    p
+                                ))));
                             }
-                        }
+                            None => {
+                                return Some(Err(Error::Message(
+                                    "Missing encrypted data packet".to_string(),
+                                )))
+                            }
+                            Some(Err(e)) => return Some(Err(e)),
+                        };
 
                         Some(Ok(Message::Encrypted { esk, edata }))
                     }

src/composed/message/types.rs

@@ -2,6 +2,7 @@ use std::boxed::Box;
 use std::collections::BTreeMap;
 use std::convert::TryFrom;
 use std::io;
+use std::io::Cursor;
 
 use bstr::BStr;
 use chrono::{self, SubsecRound};
@@ -44,7 +45,7 @@ pub enum Message {
     },
     Encrypted {
         esk: Vec<Esk>,
-        edata: Vec<Edata>,
+        edata: Edata,
     },
 }
 
@@ -162,6 +163,21 @@ impl Edata {
             Edata::SymEncryptedProtectedData(_) => Tag::SymEncryptedProtectedData,
         }
     }
+
+    pub fn decrypt(&self, key: Vec<u8>, alg: SymmetricKeyAlgorithm) -> Result<Message> {
+        let mut res = self.data()[..].to_vec();
+        let protected = self.tag() == Tag::SymEncryptedProtectedData;
+
+        debug!("decrypting protected = {:?}", protected);
+
+        let decrypted_packet: &[u8] = if protected {
+            alg.decrypt_protected(&key, &mut res)
+        } else {
+            alg.decrypt(&key, &mut res)
+        }?;
+
+        Message::from_bytes(Cursor::new(decrypted_packet.to_vec()))
+    }
 }
 
 impl Serialize for Message {
@@ -190,9 +206,7 @@ impl Serialize for Message {
                 for e in esk {
                     e.to_writer(writer)?;
                 }
-                for e in edata {
-                    e.to_writer(writer)?;
-                }
+                edata.to_writer(writer)?;
 
                 Ok(())
             }
@@ -307,9 +321,12 @@ impl Message {
     ) -> Result<Self> {
         let data = self.to_bytes()?;
 
-        let edata = vec![Edata::SymEncryptedProtectedData(
-            SymEncryptedProtectedData::encrypt_with_rng(rng, alg, &session_key, &data)?,
-        )];
+        let edata = Edata::SymEncryptedProtectedData(SymEncryptedProtectedData::encrypt_with_rng(
+            rng,
+            alg,
+            &session_key,
+            &data,
+        )?);
 
         Ok(Message::Encrypted { esk, edata })
     }
@@ -453,11 +470,7 @@ impl Message {
 
     /// Decrypt the message using the given key.
     /// Returns a message decrypter, and a list of [KeyId]s that are valid recipients of this message.
-    pub fn decrypt<'a, G>(
-        &'a self,
-        key_pw: G,
-        keys: &[&SignedSecretKey],
-    ) -> Result<(MessageDecrypter<'a>, Vec<KeyId>)>
+    pub fn decrypt<G>(&self, key_pw: G, keys: &[&SignedSecretKey]) -> Result<(Message, Vec<KeyId>)>
     where
         G: FnOnce() -> String + Clone,
     {
@@ -564,17 +577,16 @@ impl Message {
                     "session key algorithm cannot be plaintext"
                 );
 
-                Ok((
-                    MessageDecrypter::new(session_key, session_key_algorithm, edata),
-                    ids,
-                ))
+                let msg = edata.decrypt(session_key, session_key_algorithm)?;
+
+                Ok((msg, ids))
             }
         }
     }
 
     /// Decrypt the message using the given key.
     /// Returns a message decrypter, and a list of [KeyId]s that are valid recipients of this message.
-    pub fn decrypt_with_password<F>(&self, msg_pw: F) -> Result<MessageDecrypter<'_>>
+    pub fn decrypt_with_password<F>(&self, msg_pw: F) -> Result<Message>
     where
         F: FnOnce() -> String + Clone,
     {
@@ -602,11 +614,7 @@ impl Message {
                     "session key algorithm cannot be plaintext"
                 );
 
-                Ok(MessageDecrypter::new(
-                    session_key,
-                    session_key_algorithm,
-                    edata,
-                ))
+                edata.decrypt(session_key, session_key_algorithm)
             }
         }
     }
@@ -768,13 +776,7 @@ mod tests {
 
         let parsed = Message::from_armor_single(Cursor::new(&armored)).unwrap().0;
 
-        let decrypted = parsed
-            .decrypt(|| "test".into(), &[&skey])
-            .unwrap()
-            .0
-            .next()
-            .unwrap()
-            .unwrap();
+        let decrypted = parsed.decrypt(|| "test".into(), &[&skey]).unwrap().0;
 
         assert_eq!(compressed_msg, decrypted);
     }
@@ -802,13 +804,7 @@ mod tests {
 
             let parsed = Message::from_armor_single(Cursor::new(&armored)).unwrap().0;
 
-            let decrypted = parsed
-                .decrypt(|| "".into(), &[&skey])
-                .unwrap()
-                .0
-                .next()
-                .unwrap()
-                .unwrap();
+            let decrypted = parsed.decrypt(|| "".into(), &[&skey]).unwrap().0;
 
             assert_eq!(compressed_msg, decrypted);
         }
@@ -836,12 +832,7 @@ mod tests {
 
         let parsed = Message::from_armor_single(Cursor::new(&armored)).unwrap().0;
 
-        let decrypted = parsed
-            .decrypt_with_password(|| "secret".into())
-            .unwrap()
-            .next()
-            .unwrap()
-            .unwrap();
+        let decrypted = parsed.decrypt_with_password(|| "secret".into()).unwrap();
 
         assert_eq!(compressed_msg, decrypted);
     }

src/crypto/ecdh.rs

@@ -368,11 +368,10 @@ mod tests {
             let (message, _headers) = Message::from_armor_single(fs::File::open(msg_file).unwrap())
                 .expect("failed to parse message");
 
-            let (mut decrypter, _ids) = message
+            let (msg, _ids) = message
                 .decrypt(String::default, &[&decrypt_key])
                 .expect("failed to init decryption");
 
-            let msg = decrypter.next().unwrap().unwrap();
             let data = msg.get_literal().unwrap().data();
 
             assert_eq!(data, "hello\n".as_bytes());

tests/message_test.rs

@@ -77,16 +77,11 @@ fn test_parse_msg(entry: &str, base_path: &str, is_normalized: bool) {
 
     match &message {
         Message::Encrypted { .. } => {
-            let (mut decrypter, ids) = message
+            let (decrypted, ids) = message
                 .decrypt(|| details.passphrase.clone(), &[&decrypt_key])
                 .expect("failed to init decryption");
             assert_eq!(ids.len(), 1);
 
-            let decrypted = decrypter
-                .next()
-                .expect("no message")
-                .expect("message decryption failed");
-
             if let Some(verify_key) = verify_key {
                 decrypted
                     .verify(&verify_key.primary_key)
@@ -258,10 +253,7 @@ fn msg_large_indeterminate_len() {
     let decrypted = message
         .decrypt(|| "moon".to_string(), &[&decrypt_key])
         .expect("failed to decrypt message")
-        .0
-        .next()
-        .expect("no mesage")
-        .expect("message decryption failed");
+        .0;
 
     let raw = match decrypted {
         Message::Literal(data) => data,