fix: limit the use of S2K KDF with weak hash algorithms

hko-s · hko-s · commit cb26cfd0e817 · 2024-09-23T16:24:35.000+02:00
diff --git a/src/composed/message/decrypt.rs b/src/composed/message/decrypt.rs
@@ -4,7 +4,7 @@ use zeroize::{Zeroize, ZeroizeOnDrop};
 use crate::crypto::sym::SymmetricKeyAlgorithm;
 use crate::errors::Result;
 use crate::packet::SymKeyEncryptedSessionKey;
-use crate::types::{EskType, PkeskBytes, SecretKeyRepr, SecretKeyTrait};
+use crate::types::{EskType, PkeskBytes, SecretKeyRepr, SecretKeyTrait, SkeskVersion};
 
 /// Decrypts session key using secret key.
 pub fn decrypt_session_key<F, L>(
@@ -74,6 +74,18 @@ where
         "SKESK packet encryption algorithm cannot be plaintext"
     );
 
+    // Implementations MUST NOT decrypt a secret using MD5, SHA-1, or RIPEMD-160 as a hash function
+    // in an S2K KDF in a version 6 (or later) packet.
+    //
+    // (See https://www.rfc-editor.org/rfc/rfc9580.html#section-9.5-3)
+    if packet.version() == SkeskVersion::V6 {
+        ensure!(
+            !packet.s2k().known_weak_hash_algo(),
+            "Weak hash algorithm in S2K not allowed for v6 {:?}",
+            packet.s2k()
+        )
+    }
+
     let key = packet
         .s2k()
         .derive_key(&msg_pw(), packet_algorithm.key_size())?;
diff --git a/src/packet/sym_key_encrypted_session_key.rs b/src/packet/sym_key_encrypted_session_key.rs
@@ -215,6 +215,14 @@ impl SymKeyEncryptedSessionKey {
             s2k
         );
 
+        // Implementations MUST NOT generate packets using MD5, SHA-1, or RIPEMD-160 as a hash function in an S2K KDF.
+        // (See https://www.rfc-editor.org/rfc/rfc9580.html#section-9.5-3)
+        ensure!(
+            !s2k.known_weak_hash_algo(),
+            "Weak hash algorithm in S2K not allowed for v6 {:?}",
+            s2k
+        );
+
         let key = s2k.derive_key(&msg_pw(), alg.key_size())?;
 
         let mut private_key = Vec::with_capacity(session_key.len());
@@ -247,6 +255,20 @@ impl SymKeyEncryptedSessionKey {
     where
         F: FnOnce() -> String + Clone,
     {
+        ensure!(
+            s2k.uses_salt(),
+            "Can not use an s2k algorithm without a salt: {:?}",
+            s2k
+        );
+
+        // Implementations MUST NOT generate packets using MD5, SHA-1, or RIPEMD-160 as a hash function in an S2K KDF.
+        // (See https://www.rfc-editor.org/rfc/rfc9580.html#section-9.5-3)
+        ensure!(
+            !s2k.known_weak_hash_algo(),
+            "Weak hash algorithm in S2K not allowed for v6 {:?}",
+            s2k
+        );
+
         // Initial key material is the s2k derived key.
         let ikm = s2k.derive_key(&msg_pw(), sym_algorithm.key_size())?;
         // No salt is used
diff --git a/src/types/params/encrypted_secret.rs b/src/types/params/encrypted_secret.rs
@@ -87,10 +87,19 @@ impl EncryptedSecretParams {
                         StringToKey::Argon2 { .. }
                         | StringToKey::IteratedAndSalted { .. }
                         | StringToKey::Salted { .. } => {
-                            // we'll allow these
+                            // we'll allow these, generally
                         }
                         _ => bail!("Version 6 keys may not use the weak S2k type {:?}", s2k),
                     }
+
+                    // Implementations MUST NOT decrypt a secret using MD5, SHA-1, or RIPEMD-160
+                    // as a hash function in an S2K KDF in a version 6 (or later) packet.
+                    // (See https://www.rfc-editor.org/rfc/rfc9580.html#section-9.5-3)
+                    ensure!(
+                        !s2k.known_weak_hash_algo(),
+                        "Weak hash algorithm in S2K not allowed for v6 {:?}",
+                        s2k
+                    )
                 }
                 _ => bail!("Version 6 keys may only be encrypted with S2k usage AEAD or CFB"),
             }
diff --git a/src/types/params/plain_secret.rs b/src/types/params/plain_secret.rs
@@ -353,6 +353,23 @@ impl PlainSecretParams {
     ) -> Result<EncryptedSecretParams> {
         let version = pub_key.version();
 
+        // forbid weak hash algo in s2k
+
+        match &s2k_params {
+            S2kParams::Cfb { s2k, .. }
+            | S2kParams::Aead { s2k, .. }
+            | S2kParams::MalleableCfb { s2k, .. } => {
+                // Implementations MUST NOT generate packets using MD5, SHA-1, or RIPEMD-160 as a hash function in an S2K KDF.
+                // (See https://www.rfc-editor.org/rfc/rfc9580.html#section-9.5-3)
+                ensure!(
+                    !s2k.known_weak_hash_algo(),
+                    "Weak hash algorithm in S2K not allowed for v6 {:?}",
+                    s2k
+                )
+            }
+            _ => {}
+        }
+
         match &s2k_params {
             S2kParams::Unprotected => bail!("cannot encrypt to unprotected"),
             S2kParams::Cfb { sym_alg, s2k, iv } => {
diff --git a/src/types/s2k.rs b/src/types/s2k.rs
@@ -241,8 +241,31 @@ impl StringToKey {
         ]
     }
 
+    /// RFC 9580 limits the use of S2K KDF results that are based on MD5, SHA-1, or RIPEMD-160.
+    /// This function returns true for StringToKey configurations that use one of these hash algorithms.
+    pub(crate) fn known_weak_hash_algo(&self) -> bool {
+        match self {
+            Self::Simple { hash_alg }
+            | Self::Salted { hash_alg, .. }
+            | Self::IteratedAndSalted { hash_alg, .. } => {
+                hash_alg == &HashAlgorithm::MD5
+                    || hash_alg == &HashAlgorithm::SHA1
+                    || hash_alg == &HashAlgorithm::RIPEMD160
+            }
+
+            _ => false,
+        }
+    }
+
     /// String-To-Key methods are used to convert a given password string into a key.
     /// Ref: <https://www.rfc-editor.org/rfc/rfc9580.html#name-string-to-key-s2k-specifier>
+    ///
+    /// Note that RFC 9580 specifies that:
+    ///
+    /// - Implementations MUST NOT generate packets using MD5, SHA-1, or RIPEMD-160 as a hash
+    ///   function in an S2K KDF.
+    /// - Implementations MUST NOT decrypt a secret using MD5, SHA-1, or RIPEMD-160 as a hash
+    ///   function in an S2K KDF in a version 6 (or later) packet.
     pub fn derive_key(&self, passphrase: &str, key_size: usize) -> Result<Vec<u8>> {
         let key = match self {
             Self::Simple { hash_alg, .. }

Original file line number	Diff line number	Diff line change
`@@ -87,10 +87,19 @@ impl EncryptedSecretParams {`
`87`	`87`	`StringToKey::Argon2 { .. }`
`88`	`88`	`\| StringToKey::IteratedAndSalted { .. }`
`89`	`89`	`\| StringToKey::Salted { .. } => {`
`90`		`- // we'll allow these`
	`90`	`+ // we'll allow these, generally`
`91`	`91`	`}`
`92`	`92`	`_ => bail!("Version 6 keys may not use the weak S2k type {:?}", s2k),`
`93`	`93`	`}`
	`94`	`+`
	`95`	`+ // Implementations MUST NOT decrypt a secret using MD5, SHA-1, or RIPEMD-160`
	`96`	`+ // as a hash function in an S2K KDF in a version 6 (or later) packet.`
	`97`	`+ // (See https://www.rfc-editor.org/rfc/rfc9580.html#section-9.5-3)`
	`98`	`+ ensure!(`
	`99`	`+ !s2k.known_weak_hash_algo(),`
	`100`	`+ "Weak hash algorithm in S2K not allowed for v6 {:?}",`
	`101`	`+ s2k`
	`102`	`+ )`
`94`	`103`	`}`
`95`	`104`	`_ => bail!("Version 6 keys may only be encrypted with S2k usage AEAD or CFB"),`
`96`	`105`	`}`