diff options
author | Justus Winter <justus@sequoia-pgp.org> | 2020-08-12 16:19:50 +0200 |
---|---|---|
committer | Justus Winter <justus@sequoia-pgp.org> | 2020-08-20 17:43:21 +0200 |
commit | 27c1b9c65700d3781512858e1c86db1f5737ab60 (patch) | |
tree | ed0764624508aa2cba8fc1dec0b0c8436eb8b7a5 | |
parent | 95df35ef7a29f92aba2769e11026b998ad8590bf (diff) |
openpgp: Make key::Encrypted::ciphertext fail with unknown S2K.
-rw-r--r-- | openpgp/src/packet/key.rs | 70 | ||||
-rw-r--r-- | openpgp/src/parse.rs | 16 | ||||
-rw-r--r-- | openpgp/src/serialize.rs | 4 |
3 files changed, 79 insertions, 11 deletions
diff --git a/openpgp/src/packet/key.rs b/openpgp/src/packet/key.rs index 2cd1becc..2efe7d54 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())) } } @@ -1379,14 +1379,45 @@ impl Unencrypted { /// This data structure is used by the [`SecretKeyMaterial`] enum. /// /// [`SecretKeyMaterial`]: enum.SecretKeyMaterial.html -#[derive(PartialEq, Eq, Hash, Clone, Debug)] +#[derive(Clone, Debug)] pub struct Encrypted { /// Key derivation mechanism to use. s2k: S2K, /// 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. +} + +// Because the S2K and ciphertext cannot be cleanly separated at parse +// time, we need to carefully compare and hash encrypted key packets. + +impl PartialEq for Encrypted { + fn eq(&self, other: &Encrypted) -> bool { + self.algo == other.algo + // Treat S2K and ciphertext as opaque blob. + && self.s2k == other.s2k + && self.raw_ciphertext() == other.raw_ciphertext() + } +} + +impl Eq for Encrypted {} + +impl std::hash::Hash for Encrypted { + fn hash<H: std::hash::Hasher>(&self, state: &mut H) { + self.algo.hash(state); + // Treat S2K and ciphertext as opaque blob. + self.s2k.hash(state); + self.raw_ciphertext().hash(state); + } } impl Encrypted { @@ -1394,6 +1425,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 +1449,28 @@ impl Encrypted { } /// Returns the encrypted secret key material. + /// + /// 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 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 +1485,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 7373f242..790eade8 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 fe1d36cc..a9fa5385 100644 --- a/openpgp/src/serialize.rs +++ b/openpgp/src/serialize.rs @@ -1826,7 +1826,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())?; }, } } @@ -1848,7 +1848,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 |