fix(kerberos): enforce the sequence number of MIC token only for acce… (#772)

This PR fixes [#748](#748)
by enforcing sequence number verification only on the acceptor side. The
initiator verifies the MIC checksum without enforcing the sequence
number.

---------

Co-authored-by: Marc-André Moreau <mamoreau@devolutions.net>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: 3 people

Cargo.lock

@@ -129,7 +129,7 @@ winscard = { workspace = true, optional = true }
 rsa = { version = "=0.10.0-rc.18", default-features = false }
 tracing = { version = "0.1", default-features = true }
 serde = "1"
-picky-krb = "0.12"
+picky-krb = "0.13"
 picky-asn1 = { version = "0.10", features = ["time_conversion"] }
 byteorder = "1.5"
 crypto-bigint = { version = "0.7", default-features = false, features = ["alloc"] }

Cargo.toml

@@ -24,7 +24,7 @@ num-traits = { version = "0.2", default-features = true }
 uuid = { version = "1.23", default-features = false, features = ["std"] }
 picky-asn1 = "0.10"
 picky-asn1-der = "0.5"
-picky-krb = "0.12"
+picky-krb = "0.13"
 picky-asn1-x509 = { version = "0.15", features = ["pkcs7"] }
 sha1 = { version = "0.11", default-features = false }
 sha2 = "0.11"

crates/dpapi/Cargo.toml

@@ -13,7 +13,7 @@ description = "A minimal Rust implementation of the KDC (Key Distribution Center
 picky-asn1 = { version = "0.10", features = ["time_conversion"] }
 picky-asn1-der = "0.5"
 picky-asn1-x509 = "0.15"
-picky-krb = "0.12"
+picky-krb = "0.13"
 tracing = { version = "0.1", default-features = false }
 time = { version = "0.3", default-features = false }
 

crates/kdc/Cargo.toml

@@ -767,8 +767,18 @@ impl SspiEx for Kerberos {
     }
 
     fn verify_mic_token(&mut self, token: &[u8], data: &[u8], _: crate::private::Sealed) -> Result<()> {
+        // The sequence number verification is only enforced for the acceptor side.
+        // RFC 1964 §1.2.1 / RFC 4121 make sequence-number checking optional and tied to
+        // the `SEQUENCE_DETECT`/`REPLAY_DETECT` flags negotiated at context establishment.
+        // This client does not negotiate those flags.
+        // Additionally, Windows acceptor signs the SPNEGO mechListMIC with sequence number 0
+        // regardless of the AP-REP sequence number, so enforcing this on the initiator side
+        // breaks interop with the Windows implementation.
+        let enforce_sequence_number = !self.is_client();
+
         utils::validate_mic_token(
             self.is_client(),
+            enforce_sequence_number,
             self.remote_seq_number.into(),
             token,
             &self.encryption_params,

src/kerberos/mod.rs

@@ -3,8 +3,8 @@ use picky_krb::crypto::CipherSuite;
 use picky_krb::crypto::aes::{AesSize, checksum_sha_aes};
 use picky_krb::gss_api::MicToken;
 
-use crate::kerberos::{EncryptionParams, KerberosConfig, KerberosState, test_data};
-use crate::{EncryptionFlags, Kerberos, SecurityBufferFlags, SecurityBufferRef, Sspi, SspiEx};
+use crate::kerberos::{EncryptionParams, KerberosConfig, KerberosState, test_data, utils};
+use crate::{EncryptionFlags, ErrorKind, Kerberos, SecurityBufferFlags, SecurityBufferRef, Sspi, SspiEx};
 
 #[test]
 fn stream_buffer_decryption() {
@@ -326,3 +326,35 @@ fn aes128_subkey_mic_uses_subkey_size_with_aes256_ticket_enctype() {
 
     assert_mic_uses_aes_size(CipherSuite::Aes256CtsHmacSha196, AesSize::Aes128, &key);
 }
+
+#[test]
+fn initiator_accepts_mic_token_with_invalid_seq_number() {
+    let mut client = test_data::fake_client();
+    client.remote_seq_number = 10;
+
+    let data = b"SPNEGO mechTypes";
+    let sub_session_key = client.encryption_params.sub_session_key.clone().unwrap();
+    let mic_token = utils::generate_mic_token(false, 0, data.to_vec(), &sub_session_key, &AesSize::Aes256).unwrap();
+
+    client
+        .verify_mic_token(&mic_token, data, crate::private::Sealed)
+        .expect("MIC verification should succeed with seq number 0");
+}
+
+#[test]
+fn acceptor_rejects_mic_token_with_invalid_seq_number() {
+    let mut server = test_data::fake_server();
+    server.remote_seq_number = 10;
+
+    let data = b"SPNEGO mechTypes";
+    let sub_session_key = server.encryption_params.sub_session_key.clone().unwrap();
+    let mic_token = utils::generate_mic_token(true, 0, data.to_vec(), &sub_session_key, &AesSize::Aes256).unwrap();
+
+    assert_eq!(
+        server
+            .verify_mic_token(&mic_token, data, crate::private::Sealed)
+            .unwrap_err()
+            .error_type,
+        ErrorKind::InvalidToken
+    );
+}

src/kerberos/tests.rs

@@ -23,6 +23,7 @@ pub(super) fn serialize_message<T: ?Sized + Serialize>(v: &T) -> Result<Vec<u8>>
 
 pub(crate) fn validate_mic_token(
     is_client: bool,
+    enforce_seq_number: bool,
     expected_seq_number: u64,
     raw_token: &[u8],
     params: &EncryptionParams,
@@ -33,7 +34,7 @@ pub(crate) fn validate_mic_token(
     let token = MicToken::decode(raw_token)?;
     let token_flags = token.flags;
 
-    if token.seq_num != expected_seq_number {
+    if enforce_seq_number && token.seq_num != expected_seq_number {
         return Err(Error::new(
             ErrorKind::InvalidToken,
             format!(

src/kerberos/utils.rs

@@ -313,9 +313,10 @@ impl Pku2u {
         crate::kerberos::utils::generate_mic_token(self.is_client(), sequence_number, data.to_vec(), key, &aes_size)
     }
 
-    fn verify_mic(&self, sequence_number: u64, token: &[u8], data: &[u8]) -> Result<()> {
+    fn verify_mic(&self, sequence_number: u64, token: &[u8], data: &[u8], enforce_sequence_number: bool) -> Result<()> {
         crate::kerberos::utils::validate_mic_token(
             self.is_client(),
+            enforce_sequence_number,
             sequence_number,
             token,
             &self.encryption_params,
@@ -972,7 +973,7 @@ impl Sspi for Pku2u {
             acc.extend_from_slice(buffer.data());
             acc
         });
-        self.verify_mic(sequence_number.into(), &token, &data)?;
+        self.verify_mic(sequence_number.into(), &token, &data, true)?;
 
         Ok(0)
     }
@@ -1498,7 +1499,7 @@ impl SspiEx for Pku2u {
     }
 
     fn verify_mic_token(&mut self, token: &[u8], data: &[u8], _: crate::private::Sealed) -> Result<()> {
-        self.verify_mic(self.remote_gss_seq_number.into(), token, data)?;
+        self.verify_mic(self.remote_gss_seq_number.into(), token, data, true)?;
         self.remote_gss_seq_number = self.remote_gss_seq_number.wrapping_add(1);
         Ok(())
     }
@@ -1734,8 +1735,7 @@ xFnLp2UBrhxA9GYrpJ5i0onRmexQnTVSl5DDq07s+3dbr9YAKjrg9IDZYqLbdwP1
         );
     }
 
-    #[test]
-    fn stream_buffer_decryption() {
+    fn minimal_pku2u_pair() -> (Pku2u, Pku2u) {
         let session_key = vec![
             137, 60, 120, 245, 164, 179, 76, 200, 242, 96, 57, 174, 111, 209, 90, 76, 58, 117, 55, 138, 81, 75, 110,
             235, 80, 228, 14, 238, 76, 128, 139, 81,
@@ -1746,13 +1746,12 @@ xFnLp2UBrhxA9GYrpJ5i0onRmexQnTVSl5DDq07s+3dbr9YAKjrg9IDZYqLbdwP1
         ];
 
         let mut rng = StdRng::try_from_rng(&mut SysRng).unwrap();
-
         let (p2p_certificate, private_key) = test_p2p_identity();
 
         let mut negoex_random = [0; RANDOM_ARRAY_SIZE];
         rng.fill_bytes(&mut negoex_random);
 
-        let mut pku2u_server = Pku2u {
+        let server = Pku2u {
             mode: Pku2uMode::Server,
             config: Pku2uConfig {
                 p2p_certificate: p2p_certificate.clone(),
@@ -1793,14 +1792,14 @@ xFnLp2UBrhxA9GYrpJ5i0onRmexQnTVSl5DDq07s+3dbr9YAKjrg9IDZYqLbdwP1
         let mut negoex_random = [0; RANDOM_ARRAY_SIZE];
         rng.try_fill_bytes(&mut negoex_random).unwrap();
 
-        let mut pku2u_client = Pku2u {
+        let client = Pku2u {
             mode: Pku2uMode::Client,
             config: Pku2uConfig {
                 p2p_certificate: p2p_certificate.clone(),
                 private_key: private_key.into(),
                 client_hostname: "hostname".into(),
                 additional_credentials: Vec::new(),
-                trusted_client_certificates: vec![p2p_certificate.clone()],
+                trusted_client_certificates: vec![p2p_certificate],
                 trusted_server_certificates: Vec::new(),
             },
             state: Pku2uState::Final,
@@ -1831,6 +1830,51 @@ xFnLp2UBrhxA9GYrpJ5i0onRmexQnTVSl5DDq07s+3dbr9YAKjrg9IDZYqLbdwP1
             ticket_encryption_key: None,
         };
 
+        (client, server)
+    }
+
+    #[test]
+    fn verify_signature_rejects_invalid_sequence_number() {
+        let (mut client, mut server) = minimal_pku2u_pair();
+
+        let mut signature = vec![0; 64];
+        let mut signed_data = b"signed Pku2u message".to_vec();
+        let mut message = [
+            SecurityBufferRef::token_buf(&mut signature),
+            SecurityBufferRef::data_buf(&mut signed_data),
+        ];
+
+        server.make_signature(0, &mut message, 42).unwrap();
+
+        assert_eq!(
+            client.verify_signature(&mut message, 7).unwrap_err().error_type,
+            ErrorKind::InvalidToken
+        );
+    }
+
+    #[test]
+    fn verify_mic_token_rejects_invalid_sequence_number() {
+        let (mut client, mut server) = minimal_pku2u_pair();
+        server.gss_seq_number = 7;
+
+        let mic = server
+            .generate_mic_token(b"SPNEGO mech list", crate::private::Sealed)
+            .unwrap();
+
+        assert_eq!(
+            client
+                .verify_mic_token(&mic, b"SPNEGO mech list", crate::private::Sealed)
+                .unwrap_err()
+                .error_type,
+            ErrorKind::InvalidToken
+        );
+        assert_eq!(client.remote_gss_seq_number, 0);
+    }
+
+    #[test]
+    fn stream_buffer_decryption() {
+        let (mut pku2u_client, mut pku2u_server) = minimal_pku2u_pair();
+
         assert_eq!(PKG_NAME, "pku2u");
         assert_eq!(PACKAGE_INFO.rpc_id, 31);
         assert_eq!(PACKAGE_INFO.max_token_len, 12_000);