openpgp: Make SKESK5::aead_iv fail with unknown S2K.

3 files changed, 117 insertions, 21 deletions

diff --git a/openpgp/src/packet/skesk.rs b/openpgp/src/packet/skesk.rs
index 7b7e132c..3f535def 100644
--- a/openpgp/src/packet/skesk.rs
+++ b/openpgp/src/packet/skesk.rs
@@ -308,20 +308,69 @@ impl Arbitrary for SKESK4 {
 /// [Section 5.3 of RFC 4880]: https://tools.ietf.org/html/draft-ietf-openpgp-rfc4880bis-05#section-5.3
 ///
 /// This feature is [experimental](../../index.html#experimental-features).
-// IMPORTANT: If you add fields to this struct, you need to explicitly
-// IMPORTANT: implement PartialEq, Eq, and Hash.
-#[derive(PartialEq, Eq, Hash, Clone, Debug)]
+#[derive(Clone, Debug)]
 pub struct SKESK5 {
     /// Common fields.
     pub(crate) skesk4: SKESK4,
     /// AEAD algorithm.
     aead_algo: AEADAlgorithm,
     /// Initialization vector for the AEAD algorithm.
-    aead_iv: Box<[u8]>,
+    ///
+    /// If we recognized the S2K object during parsing, we can
+    /// successfully parse the data into S2K, AEAED IV, and
+    /// ciphertext.  However, if we do not recognize the S2K type, we
+    /// do not know how large its parameters are, so we cannot cleanly
+    /// parse it, and have to accept that the S2K's body bleeds into
+    /// the rest of the data.  In this case, the raw data is put into
+    /// the `esk` field, and `aead_iv` is set to `None`.
+    aead_iv: Option<Box<[u8]>>,
     /// Digest for the AEAD algorithm.
     aead_digest: Box<[u8]>,
 }
 
+// Because the S2K, IV, and ESK cannot be cleanly separated at parse
+// time, we need to carefully compare and hash SKESK5 packets.
+
+impl PartialEq for SKESK5 {
+    fn eq(&self, other: &SKESK5) -> bool {
+        self.skesk4.version == other.skesk4.version
+            && self.skesk4.sym_algo == other.skesk4.sym_algo
+            && self.aead_digest == other.aead_digest
+            // Treat S2K, IV, and ESK as opaque blob.
+            && self.skesk4.s2k == other.skesk4.s2k
+            && {
+                // XXX: This would be nicer without the allocations.
+                let mut a = Vec::new();
+                let mut b = Vec::new();
+                if let Ok(iv) = self.aead_iv() {
+                    a.extend_from_slice(iv);
+                }
+                if let Ok(iv) = other.aead_iv() {
+                    b.extend_from_slice(iv);
+                }
+                a.extend_from_slice(self.skesk4.raw_esk());
+                b.extend_from_slice(other.skesk4.raw_esk());
+                a == b
+            }
+    }
+}
+
+impl Eq for SKESK5 {}
+
+impl std::hash::Hash for SKESK5 {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.skesk4.version.hash(state);
+        self.skesk4.sym_algo.hash(state);
+        self.aead_digest.hash(state);
+        // Treat S2K, IV, and ESK as opaque blob.
+        self.skesk4.s2k.hash(state);
+        if let Some(iv) = self.aead_iv.as_ref() {
+            iv.hash(state);
+        }
+        self.skesk4.raw_esk().hash(state);
+    }
+}
+
 impl Deref for SKESK5 {
     type Target = SKESK4;
 
@@ -344,13 +393,31 @@ impl SKESK5 {
     pub fn new(esk_algo: SymmetricAlgorithm, esk_aead: AEADAlgorithm,
                s2k: S2K, iv: Box<[u8]>, esk: Box<[u8]>, digest: Box<[u8]>)
                -> Result<Self> {
+        Self::new_raw(esk_algo, esk_aead, s2k, Ok((iv, esk)), digest)
+    }
+
+    /// Creates a new SKESK version 5 packet.
+    ///
+    /// The given symmetric algorithm is the one used to encrypt the
+    /// session key.
+    pub(crate) fn new_raw(esk_algo: SymmetricAlgorithm, esk_aead: AEADAlgorithm,
+                          s2k: S2K,
+                          iv_esk: std::result::Result<(Box<[u8]>, Box<[u8]>),
+                                                      Box<[u8]>>,
+                          digest: Box<[u8]>)
+                          -> Result<Self> {
+        let (iv, esk) = match iv_esk {
+            Ok((iv, esk)) => (Some(iv), Ok(Some(esk))),
+            Err(raw) => (None, Err(raw)),
+        };
+
         Ok(SKESK5{
             skesk4: SKESK4{
                 common: Default::default(),
                 version: 5,
                 sym_algo: esk_algo,
                 s2k,
-                esk: Ok(Some(esk)),
+                esk,
             },
             aead_algo: esk_aead,
             aead_iv: iv,
@@ -424,7 +491,7 @@ impl SKESK5 {
         if let Some(ref esk) = self.esk()? {
             // Use the derived key to decrypt the ESK.
             let mut cipher = self.aead_algo.context(
-                self.symmetric_algo(), &key, &self.aead_iv)?;
+                self.symmetric_algo(), &key, &self.aead_iv()?)?;
 
             let ad = [0xc3, 5 /* Version.  */, self.symmetric_algo().into(),
                       self.aead_algo.into()];
@@ -456,13 +523,23 @@ impl SKESK5 {
     }
 
     /// Gets the AEAD initialization vector.
+    ///
+    /// If the [`S2K`] mechanism is not supported by Sequoia, this
+    /// function will fail.  Note that the information is not lost,
+    /// but stored in the packet.  If the packet is serialized again,
+    /// it is written out.
+    ///
+    ///   [`S2K`]: ../../crypto/enum.S2K.html
     pub fn aead_iv(&self) -> Result<&[u8]> {
-        Ok(&self.aead_iv)
+        self.aead_iv.as_ref()
+            .map(|iv| &iv[..])
+            .ok_or_else(|| Error::MalformedPacket(
+                format!("Unknown S2K: {:?}", self.s2k)).into())
     }
 
     /// Sets the AEAD initialization vector.
-    pub fn set_aead_iv(&mut self, iv: Box<[u8]>) -> Box<[u8]> {
-        ::std::mem::replace(&mut self.aead_iv, iv)
+    pub fn set_aead_iv(&mut self, iv: Box<[u8]>) -> Option<Box<[u8]>> {
+        ::std::mem::replace(&mut self.aead_iv, Some(iv))
     }
 
     /// Gets the AEAD digest.
diff --git a/openpgp/src/parse.rs b/openpgp/src/parse.rs
index f3b1f6f5..9146e043 100644
--- a/openpgp/src/parse.rs
+++ b/openpgp/src/parse.rs
@@ -2570,27 +2570,46 @@ impl SKESK {
                 let aead_algo: AEADAlgorithm =
                     php_try!(php.parse_u8("aead_algo")).into();
                 let s2k = php_try!(S2K::parse(&mut php));
+                let s2k_supported = s2k.is_supported();
                 let iv_size = php_try!(aead_algo.iv_size());
                 let digest_size = php_try!(aead_algo.digest_size());
-                let aead_iv = php_try!(php.parse_bytes("aead_iv", iv_size));
 
-                // The rest of the packet is the ESK, and the AEAD
-                // digest.  We don't know the size of the former, but
-                // we know the size of the latter.
+                // The rest of the packet is (potentially) the S2K
+                // parameters, the AEAD IV, the ESK, and the AEAD
+                // digest.  We don't know the size of the S2K
+                // parameters if the S2K method is not supported, and
+                // we don't know the size of the ESK.
                 let mut esk = php_try!(php.reader.steal_eof()
                                        .map_err(|e| anyhow::Error::from(e)));
+                let aead_iv = if s2k_supported {
+                    // We know the S2K method, so the parameters have
+                    // been parsed into the S2K object.  So, `esk`
+                    // starts with iv_size bytes of IV.
+                    let mut iv = esk;
+                    esk = iv.split_off(iv_size);
+                    iv
+                } else {
+                    Vec::with_capacity(0) // A dummy value.
+                };
+
                 let l = esk.len();
                 let aead_digest = esk.split_off(l.saturating_sub(digest_size));
                 // Now fix the map.
+                if s2k_supported {
+                    php.field("aead_iv", iv_size);
+                }
                 php.field("esk", esk.len());
                 php.field("aead_digest", aead_digest.len());
 
-                SKESK::V5(php_try!(SKESK5::new(
+                SKESK::V5(php_try!(SKESK5::new_raw(
                     sym_algo,
                     aead_algo,
                     s2k,
-                    aead_iv.into_boxed_slice(),
-                    esk.into(),
+                    if s2k_supported {
+                        Ok((aead_iv.into(), esk.into()))
+                    } else {
+                        Err(esk.into())
+                    },
                     aead_digest.into_boxed_slice(),
                 )))
             },
diff --git a/openpgp/src/serialize.rs b/openpgp/src/serialize.rs
index 99e689bc..c2c8b84f 100644
--- a/openpgp/src/serialize.rs
+++ b/openpgp/src/serialize.rs
@@ -2343,10 +2343,10 @@ impl Marshal for SKESK5 {
         write_byte(o, self.symmetric_algo().into())?;
         write_byte(o, self.aead_algo().into())?;
         self.s2k().serialize(o)?;
-        o.write_all(self.aead_iv()?)?;
-        if let Some(ref esk) = self.esk()? {
-            o.write_all(&esk[..])?;
+        if let Ok(iv) = self.aead_iv() {
+            o.write_all(iv)?;
         }
+        o.write_all(self.raw_esk())?;
         o.write_all(self.aead_digest())?;
 
         Ok(())
@@ -2359,8 +2359,8 @@ impl NetLength for SKESK5 {
             + 1 // Cipher algo.
             + 1 // AEAD algo.
             + self.s2k().serialized_len()
-            + self.aead_iv().unwrap().len()
-            + self.esk().unwrap().map(|esk| esk.len()).unwrap_or(0)
+            + self.aead_iv().map(|iv| iv.len()).unwrap_or(0)
+            + self.raw_esk().len()
             + self.aead_digest().len()
     }
 }