Skip to content

Commit

Permalink
fix: limit the use of S2K KDF with weak hash algorithms
Browse files Browse the repository at this point in the history
  • Loading branch information
hko-s committed Sep 23, 2024
1 parent bc79460 commit cb26cfd
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 2 deletions.
14 changes: 13 additions & 1 deletion src/composed/message/decrypt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down Expand Up @@ -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())?;
Expand Down
22 changes: 22 additions & 0 deletions src/packet/sym_key_encrypted_session_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion src/types/params/encrypted_secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
Expand Down
17 changes: 17 additions & 0 deletions src/types/params/plain_secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 } => {
Expand Down
23 changes: 23 additions & 0 deletions src/types/s2k.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, .. }
Expand Down

0 comments on commit cb26cfd

Please sign in to comment.