From 18e00a19c09e28e399dce6498989cb1ed0c1599a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Fri, 25 Sep 2026 10:05:02 -0400 Subject: [PATCH] fix(ntlm): preserve RC4 state across mechListMIC Restore only the sealing handle used for MIC generation or verification so CredSSP wrapping between MIC exchanges stays in sync. Cover late verification and invalid signatures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/ntlm/mod.rs | 93 ++++------------------------- src/ntlm/test.rs | 148 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 83 deletions(-) diff --git a/src/ntlm/mod.rs b/src/ntlm/mod.rs index 52743251..7d14e9cc 100644 --- a/src/ntlm/mod.rs +++ b/src/ntlm/mod.rs @@ -99,8 +99,6 @@ pub struct Ntlm { // If the NTLM is used as server, then our_seq_number is the server sequence number and remote seq_number is the client sequence number. our_seq_number: u32, remote_seq_number: u32, - // This flag is needed to correctly reset cipher state after MIC token generation/verification. - is_client: bool, session_key: Option<[u8; SESSION_KEY_SIZE]>, } @@ -162,7 +160,6 @@ impl Ntlm { our_seq_number: 0, remote_seq_number: 0, - is_client: true, } } @@ -194,7 +191,6 @@ impl Ntlm { our_seq_number: 0, remote_seq_number: 0, - is_client: true, } } @@ -226,7 +222,6 @@ impl Ntlm { our_seq_number: 0, remote_seq_number: 0, - is_client: true, } } @@ -252,47 +247,6 @@ impl Ntlm { }); } - /// Resets the cipher state. - /// - /// According to the specification, we need to reset ciphers before and after MIC token generation/verification. - /// [3.2.5.1 NTLM RC4 Key State for MechListMIC and First Signed Message](https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-spng/f38ae8e3-847d-4829-b933-5ac1911a00ba): - /// > When NTLM is negotiated, the SPNG server MUST set OriginalHandle to ServerHandle before generating the mechListMIC, - /// > then set ServerHandle to OriginalHandle after generating the mechListMIC. This results in the RC4 key state - /// > being the same for the mechListMIC and for the first message signed by the application. - /// > - /// > Because the RC4 key state is the same for the mechListMIC and for the first message signed by the application, - /// > the SPNEGO Extension server MUST set OriginalHandle to ClientHandle before validating the mechListMIC and then - /// > set ClientHandle to OriginalHandle after validating the mechListMIC. - fn reset_cipher_state(&mut self) -> crate::Result<()> { - use crate::ntlm::messages::computations::generate_signing_key; - use crate::ntlm::messages::{CLIENT_SEAL_MAGIC, CLIENT_SIGN_MAGIC, SERVER_SEAL_MAGIC, SERVER_SIGN_MAGIC}; - - let session_key = self.session_key.as_ref().ok_or_else(|| { - Error::new( - ErrorKind::OutOfSequence, - "the session key is not established, cannot reset cipher state", - ) - })?; - - if self.is_client { - self.send_signing_key = generate_signing_key(session_key.as_ref(), CLIENT_SIGN_MAGIC); - self.recv_signing_key = generate_signing_key(session_key.as_ref(), SERVER_SIGN_MAGIC); - self.send_sealing_key = Some(Rc4::new( - generate_signing_key(session_key.as_ref(), CLIENT_SEAL_MAGIC).as_ref(), - )); - self.recv_sealing_key = Some(Rc4::new( - generate_signing_key(session_key.as_ref(), SERVER_SEAL_MAGIC).as_ref(), - )); - } else { - self.send_signing_key = generate_signing_key(session_key, SERVER_SIGN_MAGIC); - self.recv_signing_key = generate_signing_key(session_key, CLIENT_SIGN_MAGIC); - self.send_sealing_key = Some(Rc4::new(generate_signing_key(session_key, SERVER_SEAL_MAGIC).as_ref())); - self.recv_sealing_key = Some(Rc4::new(generate_signing_key(session_key, CLIENT_SEAL_MAGIC).as_ref())); - } - - Ok(()) - } - /// Returns the next sequence number for outgoing messages and increments the internal counter. fn our_seq_num(&mut self) -> u32 { let seq_num = self.our_seq_number; @@ -370,8 +324,6 @@ impl Ntlm { &mut self, builder: FilledAcceptSecurityContext<'_, ::CredentialsHandle>, ) -> crate::Result { - self.is_client = false; - let input = builder .input .ok_or_else(|| Error::new(ErrorKind::InvalidToken, "Input buffers must be specified"))?; @@ -417,8 +369,6 @@ impl Ntlm { &mut self, builder: &mut FilledInitializeSecurityContext<'_, '_, ::CredentialsHandle>, ) -> crate::Result { - self.is_client = true; - trace!(?builder); let status = match self.state { @@ -803,49 +753,24 @@ impl SspiEx for Ntlm { } fn verify_mic_token(&mut self, signature: &[u8], data: &[u8], _: crate::private::Sealed) -> crate::Result<()> { - // We reset the cipher state before and after MIC token verification. - // - // [3.2.5.1 NTLM RC4 Key State for MechListMIC and First Signed Message](https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-spng/f38ae8e3-847d-4829-b933-5ac1911a00ba): - // > When NTLM is negotiated, the SPNG server MUST set OriginalHandle to ServerHandle before generating the mechListMIC, - // > then set ServerHandle to OriginalHandle after generating the mechListMIC. This results in the RC4 key state - // > being the same for the mechListMIC and for the first message signed by the application. - // > - // > Because the RC4 key state is the same for the mechListMIC and for the first message signed by the application, - // > the SPNEGO Extension server MUST set OriginalHandle to ClientHandle before validating the mechListMIC and then - // > set ClientHandle to OriginalHandle after validating the mechListMIC. - if self.recv_sealing_key.is_none() { self.complete_auth_token(&mut [])?; - } else { - self.reset_cipher_state()?; } let seq_number = self.remote_seq_num(); let digest = compute_digest(self.recv_signing_key.as_ref(), seq_number, data)?; - self.check_signature(seq_number, &digest, signature)?; - - self.reset_cipher_state()?; - Ok(()) + // MS-SPNG 3.2.5.1 / 3.3.5.1: the MIC must not advance the application's RC4 handle. + let original_recv_sealing_key = self.recv_sealing_key.clone(); + let result = self.check_signature(seq_number, &digest, signature); + self.recv_sealing_key = original_recv_sealing_key; + result } fn generate_mic_token(&mut self, data: &[u8], _: crate::private::Sealed) -> crate::Result> { - // We reset the cipher state before and after MIC token generation. - // - // [3.2.5.1 NTLM RC4 Key State for MechListMIC and First Signed Message](https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-spng/f38ae8e3-847d-4829-b933-5ac1911a00ba): - // > When NTLM is negotiated, the SPNG server MUST set OriginalHandle to ServerHandle before generating the mechListMIC, - // > then set ServerHandle to OriginalHandle after generating the mechListMIC. This results in the RC4 key state - // > being the same for the mechListMIC and for the first message signed by the application. - // > - // > Because the RC4 key state is the same for the mechListMIC and for the first message signed by the application, - // > the SPNEGO Extension server MUST set OriginalHandle to ClientHandle before validating the mechListMIC and then - // > set ClientHandle to OriginalHandle after validating the mechListMIC. - if self.send_sealing_key.is_none() { self.complete_auth_token(&mut [])?; - } else { - self.reset_cipher_state()?; } let seq_number = self.our_seq_num(); @@ -854,9 +779,11 @@ impl SspiEx for Ntlm { let mut mic_token = vec![0; SIGNATURE_SIZE]; let mut message = [SecurityBufferRef::token_buf(&mut mic_token)]; - self.compute_checksum(&mut message, seq_number, &digest)?; - - self.reset_cipher_state()?; + // MS-SPNG 3.2.5.1 / 3.3.5.1: restore only the handle used to sign the MIC. + let original_send_sealing_key = self.send_sealing_key.clone(); + let result = self.compute_checksum(&mut message, seq_number, &digest); + self.send_sealing_key = original_send_sealing_key; + result?; Ok(mic_token) } diff --git a/src/ntlm/test.rs b/src/ntlm/test.rs index e54eef10..2acdf952 100644 --- a/src/ntlm/test.rs +++ b/src/ntlm/test.rs @@ -1,5 +1,7 @@ use crate::crypto::{HASH_SIZE, Rc4}; +use crate::ntlm::messages::computations::generate_signing_key; use crate::ntlm::messages::test::TEST_CREDENTIALS; +use crate::ntlm::messages::{CLIENT_SEAL_MAGIC, CLIENT_SIGN_MAGIC, SERVER_SEAL_MAGIC, SERVER_SIGN_MAGIC}; use crate::ntlm::{ AuthenticateMessage, CHALLENGE_SIZE, ChallengeMessage, Mic, NegotiateFlags, NegotiateMessage, Ntlm, NtlmState, SIGNATURE_SIZE, @@ -331,6 +333,152 @@ fn verify_signature_fails_on_invalid_signature() { ); } +fn mic_test_pair() -> (Ntlm, Ntlm) { + let mut client = Ntlm::new(); + let mut server = Ntlm::new(); + client.flags = NegotiateFlags::NTLM_SSP_NEGOTIATE_KEY_EXCH; + server.flags = NegotiateFlags::NTLM_SSP_NEGOTIATE_KEY_EXCH; + client.session_key = Some(SEALING_KEY); + server.session_key = Some(SEALING_KEY); + + client.send_signing_key = generate_signing_key(&SEALING_KEY, CLIENT_SIGN_MAGIC); + server.recv_signing_key = client.send_signing_key.clone(); + server.send_signing_key = generate_signing_key(&SEALING_KEY, SERVER_SIGN_MAGIC); + client.recv_signing_key = server.send_signing_key.clone(); + + client.send_sealing_key = Some(Rc4::new(generate_signing_key(&SEALING_KEY, CLIENT_SEAL_MAGIC).as_ref())); + server.recv_sealing_key = client.send_sealing_key.clone(); + server.send_sealing_key = Some(Rc4::new(generate_signing_key(&SEALING_KEY, SERVER_SEAL_MAGIC).as_ref())); + client.recv_sealing_key = server.send_sealing_key.clone(); + + (client, server) +} + +fn assert_wrapped_message(sender: &mut Ntlm, receiver: &mut Ntlm, plaintext: &[u8]) { + let mut token = [0; SIGNATURE_SIZE]; + let mut data = plaintext.to_vec(); + { + let mut buffers = [ + SecurityBufferRef::token_buf(&mut token), + SecurityBufferRef::data_buf(&mut data), + ]; + sender.encrypt_message(EncryptionFlags::empty(), &mut buffers).unwrap(); + assert_ne!(buffers[1].data(), plaintext); + } + + let mut buffers = [ + SecurityBufferRef::data_buf(&mut data), + SecurityBufferRef::token_buf(&mut token), + ]; + receiver.decrypt_message(&mut buffers).unwrap(); + assert_eq!(buffers[0].data(), plaintext); +} + +#[test] +fn generate_mic_token_preserves_both_sealing_handles() { + let (client, server) = mic_test_pair(); + for mut context in [client, server] { + let _ = context + .send_sealing_key + .as_mut() + .unwrap() + .process(b"earlier outgoing data"); + let _ = context + .recv_sealing_key + .as_mut() + .unwrap() + .process(b"earlier incoming data"); + let mut send_before = context.send_sealing_key.clone().unwrap(); + let mut recv_before = context.recv_sealing_key.clone().unwrap(); + + let mic = context.generate_mic_token(b"mech types", private::Sealed).unwrap(); + + assert_eq!(mic.len(), SIGNATURE_SIZE); + assert_eq!( + context.send_sealing_key.as_mut().unwrap().process(TEST_DATA), + send_before.process(TEST_DATA) + ); + assert_eq!( + context.recv_sealing_key.as_mut().unwrap().process(TEST_DATA), + recv_before.process(TEST_DATA) + ); + } +} + +#[test] +fn verify_mic_token_after_pub_key_auth_preserves_send_state() { + let (mut client, mut server) = mic_test_pair(); + let mech_types = b"mech types"; + let server_mic = server.generate_mic_token(mech_types, private::Sealed).unwrap(); + + assert_wrapped_message(&mut client, &mut server, b"client pubKeyAuth"); + let mut reply_token = [0; SIGNATURE_SIZE]; + let mut reply_data = b"server pubKeyAuth".to_vec(); + server + .encrypt_message( + EncryptionFlags::empty(), + &mut [ + SecurityBufferRef::token_buf(&mut reply_token), + SecurityBufferRef::data_buf(&mut reply_data), + ], + ) + .unwrap(); + + client + .verify_mic_token(&server_mic, mech_types, private::Sealed) + .unwrap(); + client + .decrypt_message(&mut [ + SecurityBufferRef::data_buf(&mut reply_data), + SecurityBufferRef::token_buf(&mut reply_token), + ]) + .unwrap(); + assert_eq!(reply_data, b"server pubKeyAuth"); + + assert_wrapped_message(&mut client, &mut server, b"following client payload"); +} + +#[test] +fn verify_mic_token_preserves_advanced_receive_state() { + let (mut client, mut server) = mic_test_pair(); + assert_wrapped_message(&mut server, &mut client, b"earlier server payload"); + + let mech_types = b"mech types"; + let server_mic = server.generate_mic_token(mech_types, private::Sealed).unwrap(); + client + .verify_mic_token(&server_mic, mech_types, private::Sealed) + .unwrap(); + + assert_wrapped_message(&mut server, &mut client, b"following server payload"); +} + +#[test] +fn verify_mic_token_restores_receive_state_on_invalid_signature() { + let (mut client, mut server) = mic_test_pair(); + assert_wrapped_message(&mut server, &mut client, b"earlier server payload"); + + let mut mic = server.generate_mic_token(b"mech types", private::Sealed).unwrap(); + mic[4] ^= 0xff; + let mut recv_before = client.recv_sealing_key.clone().unwrap(); + let mut send_before = client.send_sealing_key.clone().unwrap(); + + assert_eq!( + client + .verify_mic_token(&mic, b"mech types", private::Sealed) + .unwrap_err() + .error_type, + ErrorKind::MessageAltered + ); + assert_eq!( + client.recv_sealing_key.as_mut().unwrap().process(TEST_DATA), + recv_before.process(TEST_DATA) + ); + assert_eq!( + client.send_sealing_key.as_mut().unwrap().process(TEST_DATA), + send_before.process(TEST_DATA) + ); +} + #[test] fn initialize_security_context_wrong_state_negotiate() { let mut context = Ntlm::new();