openpgp: Add S2K::Implicit.

  - For historical reasons, if the S2K usage octet is not a known S2K
    mechanism, the octet denotes a symmetric algorithm used to
    encrypt the key material with.  In this case, the symmetric key is
    the MD5 sum over the password.  See section 5.5.3. Secret-Key
    Packet Formats of RFC4880.While this is obviously not a great
    choice, it is no worse than `S2K::Simple { hash: MD5 }`, since
    it is equivalent to that.

  - Model this by adding a new S2K variant.

  - Notably, this fixes handling of packets with unknown S2K
    mechanisms.  Under the model of RFC4880, which we implement, any
    unknown S2K mechanism is an implicit S2K, where the usage octet
    denotes an unsupported symmetric algorithm.  Using this will fail,
    but we now can parse and serialize it correctly, and with them the
    secret key packets they come in.

  - Fixes #1095.

5 files changed, 97 insertions, 21 deletions

diff --git a/openpgp/NEWS b/openpgp/NEWS
index 7b0d62f9..95d629e4 100644
--- a/openpgp/NEWS
+++ b/openpgp/NEWS
@@ -5,6 +5,7 @@
 
 * Changes in 1.20.0
 ** New functionality
+   - S2K::Implicit
    - Signature::verify_signature
 * Changes in 1.19.0
 ** Notable fixes
diff --git a/openpgp/src/crypto/s2k.rs b/openpgp/src/crypto/s2k.rs
index 9b7c24d6..fc4bdc90 100644
--- a/openpgp/src/crypto/s2k.rs
+++ b/openpgp/src/crypto/s2k.rs
@@ -78,6 +78,18 @@ pub enum S2K {
         hash: HashAlgorithm
     },
 
+    /// Simply hashes the password using MD5
+    ///
+    /// This mechanism uses neither iteration to increase the time it
+    /// takes to derive the key from the password nor does it salt the
+    /// password, as well as using a very weak and fast hash
+    /// algorithm.  This makes dictionary attacks more feasible.
+    ///
+    /// This mechanism has been deprecated in RFC 2440. Do not use
+    /// this variant.
+    #[deprecated(note = "Use `S2K::Iterated`.")]
+    Implicit,
+
     /// Private S2K algorithm.
     Private {
         /// Tag identifying the private algorithm.
@@ -224,6 +236,7 @@ impl S2K {
                                 hash.update(&data[0..tail]);
                             }
                         }
+                        S2K::Implicit |
                         S2K::Unknown { .. } | &S2K::Private { .. } =>
                             unreachable!(),
                     }
@@ -234,6 +247,9 @@ impl S2K {
 
                 Ok(ret.into())
             }),
+            S2K::Implicit => S2K::Simple {
+                hash: HashAlgorithm::MD5,
+            }.derive_key(password, key_size),
             S2K::Unknown { tag, .. } | S2K::Private { tag, .. } =>
                 Err(Error::MalformedPacket(
                         format!("Unknown S2K type {:#x}", tag)).into()),
@@ -248,6 +264,7 @@ impl S2K {
             Simple { .. }
             | Salted { .. }
             | Iterated { .. }
+            | Implicit
             => true,
             S2K::Private { .. }
             | S2K::Unknown { .. }
@@ -353,6 +370,7 @@ impl fmt::Display for S2K {
                     salt[4], salt[5], salt[6], salt[7],
                     hash_bytes))
             }
+            S2K::Implicit => f.write_str("Implicit S2K"),
             S2K::Private { tag, parameters } =>
                 if let Some(p) = parameters.as_ref() {
                     write!(f, "Private/Experimental S2K {}:{:?}", tag, p)
diff --git a/openpgp/src/parse.rs b/openpgp/src/parse.rs
index 6bcbf869..9ebea413 100644
--- a/openpgp/src/parse.rs
+++ b/openpgp/src/parse.rs
@@ -2358,14 +2358,19 @@ impl Key4<key::UnspecifiedParts, key::UnspecifiedRole>
                             Some(mpi::SecretKeyChecksum::Sum16)));
                     sec.into()
                 }
-                // Encrypted & MD5 for key derivation: unsupported
-                1..=253 => {
-                    return php.fail("unsupported secret key encryption");
-                }
-                // Encrypted, S2K & SHA-1 checksum
-                254 | 255 => {
-                    let sk: SymmetricAlgorithm = php_try!(php.parse_u8("sym_algo")).into();
-                    let s2k = php_try!(S2K::parse_v4(&mut php));
+                // Encrypted, whether we support the S2K method or not.
+                _ => {
+                    let sk: SymmetricAlgorithm = match s2k_usage {
+                        254 | 255 =>
+                            php_try!(php.parse_u8("sym_algo")).into(),
+                        _ => s2k_usage.into(),
+                    };
+                    let s2k = match s2k_usage {
+                        254 | 255 => php_try!(S2K::parse_v4(&mut php)),
+                        _ => {
+                            #[allow(deprecated)] S2K::Implicit
+                        },
+                    };
                     let s2k_supported = s2k.is_supported();
                     let cipher =
                         php_try!(php.parse_bytes_eof("encrypted_mpis"))
@@ -2373,10 +2378,10 @@ impl Key4<key::UnspecifiedParts, key::UnspecifiedRole>
 
                     crate::packet::key::Encrypted::new_raw(
                         s2k, sk,
-                        if s2k_usage == 254 {
-                            Some(mpi::SecretKeyChecksum::SHA1)
-                        } else {
-                            Some(mpi::SecretKeyChecksum::Sum16)
+                        match s2k_usage {
+                            254 => Some(mpi::SecretKeyChecksum::SHA1),
+                            255 => Some(mpi::SecretKeyChecksum::Sum16),
+                            _ => Some(mpi::SecretKeyChecksum::Sum16),
                         },
                         if s2k_supported {
                             Ok(cipher)
@@ -6757,4 +6762,30 @@ yCsBO81bKqlfklugX5yRX5qTopuXX6KbWpFZXKJXUlGSetb4dXm+gYFBCRcA
         Ok(())
     }
 
+    /// Tests for issue 1095, parsing a secret key packet with an
+    /// unknown S2K mechanism.
+    #[test]
+    fn key_unknown_s2k() -> Result<()> {
+        let mut ppr = PacketParser::from_bytes(
+            crate::tests::key("hardware-backed-secret.pgp"))?;
+        let mut i = 0;
+        while let PacketParserResult::Some(pp) = ppr {
+            if i == 0 {
+                assert!(matches!(&pp.packet, Packet::SecretKey(_)));
+            }
+            if i == 3 {
+                assert!(matches!(&pp.packet, Packet::SecretSubkey(_)));
+            }
+
+            // Make sure it roundtrips.
+            let p = &pp.packet;
+            let v = p.to_vec()?;
+            let q = Packet::from_bytes(&v)?;
+            assert_eq!(p, &q);
+
+            ppr = pp.recurse()?.1;
+            i += 1;
+        }
+        Ok(())
+    }
 }
diff --git a/openpgp/src/serialize.rs b/openpgp/src/serialize.rs
index 6dcc1845..89ca0edf 100644
--- a/openpgp/src/serialize.rs
+++ b/openpgp/src/serialize.rs
@@ -1319,6 +1319,7 @@ impl Marshal for S2K {
                 w.write_all(&salt[..])?;
                 w.write_all(&[S2K::encode_count(hash_bytes)?])?;
             }
+            S2K::Implicit => (),
             S2K::Private { tag, parameters }
             | S2K::Unknown { tag, parameters} => {
                 w.write_all(&[*tag])?;
@@ -1339,6 +1340,7 @@ impl MarshalInto for S2K {
             &S2K::Simple{ .. } => 2,
             &S2K::Salted{ .. } => 2 + 8,
             &S2K::Iterated{ .. } => 2 + 8 + 1,
+            S2K::Implicit => 0,
             S2K::Private { parameters, .. }
             | S2K::Unknown { parameters, .. } =>
                 1 + parameters.as_ref().map(|p| p.len()).unwrap_or(0),
@@ -2001,14 +2003,21 @@ impl<P, R> Marshal for Key4<P, R>
                     mpis.serialize_with_checksum(o, SecretKeyChecksum::Sum16)
                 })?,
                 SecretKeyMaterial::Encrypted(ref e) => {
-                    // S2K usage.
-                    write_byte(o, match e.checksum() {
-                        Some(SecretKeyChecksum::SHA1) => 254,
-                        Some(SecretKeyChecksum::Sum16) => 255,
-                        None => return Err(Error::InvalidOperation(
-                            "In Key4 packets, encrypted secret keys must be \
-                             checksummed".into()).into()),
-                    })?;
+                    // S2K usage
+                    #[allow(deprecated)]
+                    if let S2K::Implicit = e.s2k() {
+                        // When the legacy implicit S2K mechanism is
+                        // in use, the symmetric algorithm octet below
+                        // takes the place of the S2K usage octet.
+                    } else {
+                        write_byte(o, match e.checksum() {
+                            Some(SecretKeyChecksum::SHA1) => 254,
+                            Some(SecretKeyChecksum::Sum16) => 255,
+                            None => return Err(Error::InvalidOperation(
+                                "In Key4 packets, encrypted secret keys must be \
+                                 checksummed".into()).into()),
+                        })?;
+                    }
                     write_byte(o, e.algo().into())?;
                     e.s2k().serialize(o)?;
                     o.write_all(e.raw_ciphertext())?;
@@ -2024,6 +2033,7 @@ impl<P, R> NetLength for Key4<P, R>
     where P: key::KeyParts,
           R: key::KeyRole,
 {
+    #[allow(deprecated)]
     fn net_len(&self) -> usize {
         let have_secret_key = P::significant_secrets() && self.has_secret();
 
@@ -2037,7 +2047,9 @@ impl<P, R> NetLength for Key4<P, R>
                         u.map(|mpis| mpis.serialized_len())
                         + 2, // Two octet checksum.
                     SecretKeyMaterial::Encrypted(ref e) =>
-                        1 + e.s2k().serialized_len()
+                        matches!(e.s2k(), S2K::Implicit)
+                        .then_some(0).unwrap_or(1)
+                        + e.s2k().serialized_len()
                         + e.raw_ciphertext().len(),
                 }
             } else {
diff --git a/openpgp/tests/data/keys/hardware-backed-secret.pgp b/openpgp/tests/data/keys/hardware-backed-secret.pgp
new file mode 100644
index 00000000..b211205a
--- /dev/null
+++ b/openpgp/tests/data/keys/hardware-backed-secret.pgp
@@ -0,0 +1,14 @@
+-----BEGIN PGP PRIVATE KEY BLOCK-----
+Comment: from https://gitlab.com/dkg/openpgp-hardware-secrets/-/merge_requests/2
+
+xTQEZgWtcxYJKwYBBAHaRw8BAQdAlLK6UPQsVHR2ETk1SwVIG3tBmpiEtikYYlCy
+1TIiqzb8zR08aGFyZHdhcmUtc2VjcmV0QGV4YW1wbGUub3JnPsKNBBAWCAA1AhkB
+BQJmBa1zAhsDCAsJCAcKDQwLBRUKCQgLAhYCFiEEXlP8Tur0WZR+f0I33/i9Uh4O
+HEkACgkQ3/i9Uh4OHEnryAD8CzH2ajJvASp46ApfI4pLPY57rjBX++d/2FQPRyqG
+HJUA/RLsNNgxiFYmK5cjtQe2/DgzWQ7R6PxPC6oa3XM7xPcCxzkEZgWtcxIKKwYB
+BAGXVQEFAQEHQE1YXOKeaklwG01Yab4xopP9wbu1E+pCrP1xQpiFZW5KAwEIB/zC
+eAQYFggAIAUCZgWtcwIbDBYhBF5T/E7q9FmUfn9CN9/4vVIeDhxJAAoJEN/4vVIe
+DhxJVTgA/1WaFrKdP3AgL0Ffdooc5XXbjQsj0uHo6FZSHRI4pchMAQCyJnKQ3RvW
+/0gm41JCqImyg2fxWG4hY0N5Q7Rc6PyzDQ==
+=3w/O
+-----END PGP PRIVATE KEY BLOCK-----