openpgp: Make key::Encrypted::ciphertext fail with unknown S2K.justus/fallible-encrypted-ciphertext

3 files changed, 53 insertions, 10 deletions

diff --git a/openpgp/src/packet/key.rs b/openpgp/src/packet/key.rs
index 8776a7e5..1f23d1f6 100644
--- a/openpgp/src/packet/key.rs
+++ b/openpgp/src/packet/key.rs
@@ -1370,7 +1370,7 @@ impl Unencrypted {
             self.map(|mpis| mpis.serialize_chksumd(&mut encryptor))?;
         }
 
-        Ok(Encrypted { s2k, algo, ciphertext: esk.into_boxed_slice() })
+        Ok(Encrypted::new(s2k, algo, esk.into_boxed_slice()))
     }
 }
 
@@ -1386,7 +1386,15 @@ pub struct Encrypted {
     /// Symmetric algorithm used to encrypt the secret key material.
     algo: SymmetricAlgorithm,
     /// Encrypted MPIs prefixed with the IV.
-    ciphertext: Box<[u8]>,
+    ///
+    /// If we recognized the S2K object during parsing, we can
+    /// successfully parse the data into S2K, 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.
+    ciphertext: std::result::Result<Box<[u8]>,  // IV + ciphertext.
+                                    Box<[u8]>>, // S2K body + IV + ciphertext.
 }
 
 impl Encrypted {
@@ -1394,6 +1402,15 @@ impl Encrypted {
     pub fn new(s2k: S2K, algo: SymmetricAlgorithm, ciphertext: Box<[u8]>)
         -> Self
     {
+        Self::new_raw(s2k, algo, Ok(ciphertext))
+    }
+
+    /// Creates a new encrypted key object.
+    pub(crate) fn new_raw(s2k: S2K, algo: SymmetricAlgorithm,
+                          ciphertext: std::result::Result<Box<[u8]>,
+                                                          Box<[u8]>>)
+        -> Self
+    {
         Encrypted { s2k, algo, ciphertext }
     }
 
@@ -1409,8 +1426,26 @@ impl Encrypted {
     }
 
     /// Returns the encrypted secret key material.
+    ///
+    /// If the [`S2K`] mechanism is not supported by Sequoia, this
+    /// function will fail.
+    ///
+    ///   [`S2K`]: ../../crypto/enum.S2K.html
     pub fn ciphertext(&self) -> Result<&[u8]> {
-        Ok(&self.ciphertext)
+        self.ciphertext
+            .as_ref()
+            .map(|ciphertext| &ciphertext[..])
+            .map_err(|_| Error::MalformedPacket(
+                format!("Unknown S2K: {:?}", self.s2k)).into())
+    }
+
+    /// Returns the encrypted secret key material, possibly including
+    /// the body of the S2K object.
+    pub(crate) fn raw_ciphertext(&self) -> &[u8] {
+        match self.ciphertext.as_ref() {
+            Ok(ciphertext) => &ciphertext[..],
+            Err(s2k_ciphertext) => &s2k_ciphertext[..],
+        }
     }
 
     /// Decrypts the secret key material using `password`.
@@ -1425,7 +1460,7 @@ impl Encrypted {
         use crate::crypto::symmetric::Decryptor;
 
         let key = self.s2k.derive_key(password, self.algo.key_size()?)?;
-        let cur = Cursor::new(&self.ciphertext);
+        let cur = Cursor::new(self.ciphertext()?);
         let mut dec = Decryptor::new(self.algo, &key, cur)?;
 
         // Consume the first block.
diff --git a/openpgp/src/parse.rs b/openpgp/src/parse.rs
index 6db6a4d7..2962dbbb 100644
--- a/openpgp/src/parse.rs
+++ b/openpgp/src/parse.rs
@@ -2128,11 +2128,19 @@ impl Key4<key::UnspecifiedParts, key::UnspecifiedRole>
                 254 => {
                     let sk: SymmetricAlgorithm = php_try!(php.parse_u8("sym_algo")).into();
                     let s2k = php_try!(S2K::parse(&mut php));
+                    let s2k_supported = s2k.is_supported();
                     let cipher =
-                        php_try!(php.parse_bytes_eof("encrypted_mpis"));
-
-                    crate::packet::key::Encrypted::new(
-                        s2k, sk, cipher.into_boxed_slice()).into()
+                        php_try!(php.parse_bytes_eof("encrypted_mpis"))
+                        .into_boxed_slice();
+
+                    crate::packet::key::Encrypted::new_raw(
+                        s2k, sk,
+                        if s2k_supported {
+                            Ok(cipher)
+                        } else {
+                            Err(cipher)
+                        },
+                    ).into()
                 }
                 // Encrypted, S2K & mod 65536 checksum: unsupported
                 255 => {
diff --git a/openpgp/src/serialize.rs b/openpgp/src/serialize.rs
index e57fdacc..838ef7e0 100644
--- a/openpgp/src/serialize.rs
+++ b/openpgp/src/serialize.rs
@@ -1819,7 +1819,7 @@ impl<P, R> Key4<P, R>
                     write_byte(o, 254)?;
                     write_byte(o, e.algo().into())?;
                     e.s2k().serialize(o)?;
-                    o.write_all(e.ciphertext()?)?;
+                    o.write_all(e.raw_ciphertext())?;
                 },
             }
         }
@@ -1841,7 +1841,7 @@ impl<P, R> Key4<P, R>
                         + 2, // Two octet checksum.
                     SecretKeyMaterial::Encrypted(ref e) =>
                         1 + e.s2k().serialized_len()
-                        + e.ciphertext().unwrap().len(),
+                        + e.raw_ciphertext().len(),
                 }
             } else {
                 0