From cd1f66e5f9a2fbba367009840f35802d7e749b1f Mon Sep 17 00:00:00 2001 From: lhear <121179341+lhear@users.noreply.github.com> Date: Thu, 14 May 2026 16:02:22 +0800 Subject: [PATCH] fix: derive independent target_key to prevent target host substitution The target host was previously encrypted with a shared master key, which was vulnerable to ciphertext substitution attacks. This change updates the KDF to derive a dedicated 'target_key' alongside the connection keys. By binding the target host's ciphertext to the session-specific connection nonce, we ensure that the target cannot be swapped between different sessions. --- src/client/handshake.rs | 20 ++++++++++++-------- src/crypto/handshake.rs | 28 +++++++++++++++++----------- src/crypto/mod.rs | 2 +- src/server/handlers.rs | 17 ++++++++++------- 4 files changed, 40 insertions(+), 27 deletions(-) diff --git a/src/client/handshake.rs b/src/client/handshake.rs index d1c41ba..c1c13bd 100644 --- a/src/client/handshake.rs +++ b/src/client/handshake.rs @@ -34,13 +34,15 @@ pub async fn try_pq_connect( info!(session_id = %session_id, target = %target_host, "session resumption: attempting to reuse session"); let conn_nonce: [u8; 16] = rand::rng().random(); - let (upload_key, download_key) = crypto::derive_connection_keys(master, &conn_nonce); + let (upload_key, download_key, target_key) = + crypto::derive_connection_keys(master, &conn_nonce); let upload_cipher = Arc::new(AesFrameCipher::new(upload_key)); let download_cipher = Arc::new(AesFrameCipher::new(download_key)); - let cookie_master_key = crypto::derive_cookie_master_key(master); - let enc_target = crypto::encrypt_bytes(&cookie_master_key, target_host.as_bytes())?; - let enc_conn_nonce = crypto::encrypt_bytes(&cookie_master_key, &conn_nonce)?; + let enc_target = crypto::encrypt_bytes(&target_key, target_host.as_bytes())?; + + let cookie_nonce_key = crypto::derive_cookie_nonce_key(master); + let enc_conn_nonce = crypto::encrypt_bytes(&cookie_nonce_key, &conn_nonce)?; let cookie_val = format!( "{}:{}:{}", @@ -269,13 +271,15 @@ pub async fn full_handshake( } let conn_nonce: [u8; 16] = rand::rng().random(); - let (upload_key, download_key) = crypto::derive_connection_keys(&*master, &conn_nonce); + let (upload_key, download_key, target_key) = + crypto::derive_connection_keys(&*master, &conn_nonce); let upload_cipher = Arc::new(AesFrameCipher::new(upload_key)); let download_cipher = Arc::new(AesFrameCipher::new(download_key)); - let cookie_master_key = crypto::derive_cookie_master_key(&*master); - let enc_target = crypto::encrypt_bytes(&cookie_master_key, target_host.as_bytes())?; - let enc_conn_nonce = crypto::encrypt_bytes(&cookie_master_key, &conn_nonce)?; + let enc_target = crypto::encrypt_bytes(&target_key, target_host.as_bytes())?; + + let cookie_nonce_key = crypto::derive_cookie_nonce_key(&*master); + let enc_conn_nonce = crypto::encrypt_bytes(&cookie_nonce_key, &conn_nonce)?; let cookie_val = format!( "{}:{}:{}", diff --git a/src/crypto/handshake.rs b/src/crypto/handshake.rs index 0e13571..0b48158 100644 --- a/src/crypto/handshake.rs +++ b/src/crypto/handshake.rs @@ -23,28 +23,33 @@ pub fn derive_initial_master(mlkem_ss: &[u8], x25519_ss: &[u8]) -> Zeroizing<[u8 master } -pub fn derive_cookie_master_key(master: &[u8; 32]) -> AesKey { +pub fn derive_cookie_nonce_key(master: &[u8; 32]) -> AesKey { let hkdf = Hkdf::::new(None, master); let mut key = [0u8; 32]; - hkdf.expand(b"cookie_master_key", &mut key) + hkdf.expand(b"cookie_nonce_key", &mut key) .expect("32 bytes is valid for HKDF"); key.into() } -pub fn derive_connection_keys(master: &[u8; 32], conn_nonce: &[u8; 16]) -> (AesKey, AesKey) { +pub fn derive_connection_keys( + master: &[u8; 32], + conn_nonce: &[u8; 16], +) -> (AesKey, AesKey, AesKey) { let hkdf = Hkdf::::new(None, master); let mut info = Vec::with_capacity(16 + 15); info.extend_from_slice(conn_nonce); info.extend_from_slice(b"connection_keys"); - let mut buf = [0u8; 64]; + let mut buf = [0u8; 96]; hkdf.expand(&info, &mut buf) - .expect("64 bytes is valid for HKDF"); + .expect("96 bytes is valid for HKDF"); let mut upload_key = [0u8; 32]; let mut download_key = [0u8; 32]; + let mut target_key = [0u8; 32]; upload_key.copy_from_slice(&buf[..32]); - download_key.copy_from_slice(&buf[32..]); - (upload_key.into(), download_key.into()) + download_key.copy_from_slice(&buf[32..64]); + target_key.copy_from_slice(&buf[64..]); + (upload_key.into(), download_key.into(), target_key.into()) } #[cfg(test)] @@ -72,10 +77,11 @@ mod tests { fn connection_keys_deterministic() { let master = [0xBBu8; 32]; let nonce = [0xCCu8; 16]; - let (up1, dn1) = derive_connection_keys(&master, &nonce); - let (up2, dn2) = derive_connection_keys(&master, &nonce); + let (up1, dn1, tg1) = derive_connection_keys(&master, &nonce); + let (up2, dn2, tg2) = derive_connection_keys(&master, &nonce); assert_eq!(up1, up2); assert_eq!(dn1, dn2); + assert_eq!(tg1, tg2); } #[test] @@ -83,8 +89,8 @@ mod tests { let master = [0xBBu8; 32]; let n1 = [0xCCu8; 16]; let n2 = [0xDDu8; 16]; - let (up1, _) = derive_connection_keys(&master, &n1); - let (up2, _) = derive_connection_keys(&master, &n2); + let (up1, _, _) = derive_connection_keys(&master, &n1); + let (up2, _, _) = derive_connection_keys(&master, &n2); assert_ne!(up1, up2); } } diff --git a/src/crypto/mod.rs b/src/crypto/mod.rs index 51c0275..ccf1fc9 100644 --- a/src/crypto/mod.rs +++ b/src/crypto/mod.rs @@ -4,7 +4,7 @@ mod keys; pub use cipher::{AesFrameCipher, decrypt_bytes, encrypt_bytes}; pub use handshake::{ - derive_connection_keys, derive_cookie_master_key, derive_handshake_key, derive_initial_master, + derive_connection_keys, derive_cookie_nonce_key, derive_handshake_key, derive_initial_master, }; pub use keys::{ b64_to_private_key, b64_to_public_key, bytes_to_encapsulation_key, diffie_hellman, diff --git a/src/server/handlers.rs b/src/server/handlers.rs index 4dd334f..cbdfdb8 100644 --- a/src/server/handlers.rs +++ b/src/server/handlers.rs @@ -348,20 +348,16 @@ async fn handle_pq_download( return Err(ServerError::precondition_required("master key expired")); } - let cookie_master_key = crypto::derive_cookie_master_key(&*master); + let cookie_nonce_key = crypto::derive_cookie_nonce_key(&*master); let enc_target = base64::engine::general_purpose::URL_SAFE_NO_PAD .decode(enc_target_b64) .map_err(|_| ServerError::bad_request("invalid cookie encoding"))?; - let target_bytes = crypto::decrypt_bytes(&cookie_master_key, &enc_target) - .map_err(|_| ServerError::bad_request("failed to decrypt target"))?; - let target = String::from_utf8(target_bytes) - .map_err(|_| ServerError::bad_request("invalid target utf8"))?; let enc_nonce = base64::engine::general_purpose::URL_SAFE_NO_PAD .decode(enc_nonce_b64) .map_err(|_| ServerError::bad_request("invalid cookie encoding"))?; - let conn_nonce_bytes = crypto::decrypt_bytes(&cookie_master_key, &enc_nonce) + let conn_nonce_bytes = crypto::decrypt_bytes(&cookie_nonce_key, &enc_nonce) .map_err(|_| ServerError::bad_request("failed to decrypt conn_nonce"))?; let conn_nonce: [u8; 16] = conn_nonce_bytes .try_into() @@ -378,7 +374,14 @@ async fn handle_pq_download( drop(entry); - let (upload_key, download_key) = crypto::derive_connection_keys(&*master, &conn_nonce); + let (upload_key, download_key, target_key) = + crypto::derive_connection_keys(&*master, &conn_nonce); + + let target_bytes = crypto::decrypt_bytes(&target_key, &enc_target) + .map_err(|_| ServerError::bad_request("failed to decrypt target"))?; + let target = String::from_utf8(target_bytes) + .map_err(|_| ServerError::bad_request("invalid target utf8"))?; + let upload_cipher = Arc::new(AesFrameCipher::new(upload_key)); let download_cipher = Arc::new(AesFrameCipher::new(download_key));