Supporting no-encryption NTLM message signing & GSS MIC calculation

[AvivNaaman]

Hello,
In an attempt to utilize the NTLM SSPI provider for signing a GSS-API negotiation message, I encountered an issue. According to the GSS-API RFC (here), when the underlying scheme supports MIC calculation, it is mandatory to provide the mechListMIC. In my case, since NTLM was required to provide integrity, the mechListMIC had to be included.
I attempted to call the Ntlm::encrypt_message function to sign the mechTypeList value, as per the RFC’s requirements. However, the validation of the mechListMIC failed. Upon examining the implementation of the encrypt_message function, it became evident that it encrypts the input data before signing the digest of the data using the same RC4 state. Consequently, the RC4 state is not the initial state when signing the data. Additionally, there is an extraneous and irrelevant outcome of generating encrypted data for the mechTypeList data.
To address this issue, I devised a proof-of-concept (PoC) solution to sign the MIC message. I proceeded by commenting out the message encryption section from the encrypt_message function. This modification enabled the signing process to proceed successfully.

    fn encrypt_message(
        &mut self,
        _flags: EncryptionFlags,
        message: &mut [SecurityBuffer],
        sequence_number: u32,
    ) -> crate::Result<SecurityStatus> {
        if self.send_sealing_key.is_none() {
            self.complete_auth_token(&mut [])?;
        }
        println!("Starting the encryption process");
        SecurityBuffer::find_buffer_mut(message, SecurityBufferType::Token)?; // check if exists
        let data = SecurityBuffer::find_buffer_mut(message, SecurityBufferType::Data)?;
        let digest = compute_digest(&self.send_signing_key, sequence_number, data.data())?;

        // let encrypted_data = self.send_sealing_key.as_mut().unwrap().process(data.data());
        // if encrypted_data.len() < data.buf_len() {
        //     return Err(Error::new(ErrorKind::BufferTooSmall, "The Data buffer is too small"));
        // }
        // data.write_data(&encrypted_data)?;

        let checksum = self
            .send_sealing_key
            .as_mut()
            .unwrap()
            .process(&digest[0..SIGNATURE_CHECKSUM_SIZE]);

        let signature_buffer = SecurityBuffer::find_buffer_mut(message, SecurityBufferType::Token)?;
        if signature_buffer.buf_len() < SIGNATURE_SIZE {
            return Err(Error::new(ErrorKind::BufferTooSmall, "The Token buffer is too small"));
        }
        let signature = compute_signature(&checksum, sequence_number);
        signature_buffer.write_data(signature.as_slice())?;

        Ok(SecurityStatus::Ok)
    }

Certainly, attempting to utilize the code for actual encryption of data will be futile, as that is not my current requirement. I merely need a signing mechanism as a preliminary step.
Scapy incorporates the Gss_GstMICEx function as part of its NTLM module, which is also utilized in the encryption process (due to the shared logic).
Another noteworthy aspect is the necessity to “back up” the original send key state prior to signing the mechTypeList and restoring it immediately afterward (as specified in MS-SPNG 3.3.5.1).
Do you have any insights regarding message signing within the NTLM SSPI (or, alternatively, in other SSPIs as well)? Specifically, would it be feasible to add such a function to the Ntlm implementation or, at the very least, provide a mechanism for direct access to the send key and its state, suitable for such purposes?
Thank you for your consideration.

[AvivNaaman]

A small patch applied locally can address this issue by implementing the following:

impl Sspi for Ntlm {
    ...
    #[instrument(level = "debug", ret, fields(state = ?self.state), skip(self, _flags))]
    fn encrypt_message(
        &mut self,
        _flags: EncryptionFlags,
        message: &mut [SecurityBuffer],
        sequence_number: u32,
    ) -> crate::Result<SecurityStatus> {
        if self.send_sealing_key.is_none() {
            self.complete_auth_token(&mut [])?;
        }
        SecurityBuffer::find_buffer_mut(message, SecurityBufferType::Token)?; // check if exists

        // Deep copy the data buffer and create a fresh token buffer.
        let mut data_buffer = SecurityBuffer::find_buffer(message, SecurityBufferType::Data)?.data().to_vec();
        let data_copy_buffer = SecurityBuffer::Data(&mut data_buffer);
        let mut token_buffer = vec![0; MESSAGE_INTEGRITY_CHECK_SIZE];
        let token_data_buffer = SecurityBuffer::Token(&mut token_buffer);

        let mut signing_message = vec![data_copy_buffer, token_data_buffer];

        let mut data = SecurityBuffer::find_buffer_mut(message, SecurityBufferType::Data)?;
        let encrypted_data = self.send_sealing_key.as_mut().unwrap().process(data.data());
        if encrypted_data.len() < data.buf_len() {
            return Err(Error::new(ErrorKind::BufferTooSmall, "The Data buffer is too small"));
        }
        data.write_data(&encrypted_data)?;

        self.sign_data(_flags, &mut signing_message, sequence_number)?;
        let mut token = SecurityBuffer::find_buffer_mut(message, SecurityBufferType::Token)?;
        token.write_data(&token_buffer)?;

        Ok(SecurityStatus::Ok)
    }
}
    
impl Ntlm {
    ...
    fn sign_data(
        &mut self,
        _flags: EncryptionFlags,
        messages: &mut [SecurityBuffer],
        sequence_number: u32,
    ) -> crate::Result<SecurityStatus> {
        let data = SecurityBuffer::find_buffer_mut(messages, SecurityBufferType::Data)?;
        let digest = compute_digest(&self.send_signing_key, sequence_number, data.data())?;

        let checksum = self
            .send_sealing_key
            .as_mut()
            .unwrap()
            .process(&digest[0..SIGNATURE_CHECKSUM_SIZE]);

        let signature_buffer = SecurityBuffer::find_buffer_mut(messages, SecurityBufferType::Token)?;
        if signature_buffer.buf_len() < SIGNATURE_SIZE {
            return Err(Error::new(ErrorKind::BufferTooSmall, "The Token buffer is too small"));
        }
        let signature = compute_signature(&checksum, sequence_number);
        signature_buffer.write_data(signature.as_slice())?;

        Ok(SecurityStatus::Ok)
    }

    pub fn sign_and_revert_state(&mut self, messages: &mut [SecurityBuffer], sequence_number: u32) -> crate::Result<()> {
        let original_key_state = self.send_sealing_key.clone().ok_or(Error::new(ErrorKind::OutOfSequence, "send_sealing_key is None"))?;
        self.sign_data(EncryptionFlags::empty(), messages, sequence_number)?;
        self.send_sealing_key = Some(original_key_state);
        Ok(())
    }
}

Although it is not yet a perfect patch, I believe I can propose a PR and incorporate it. However, I would appreciate your insights on the optimal design for this feature.

[CBenoit]

Thank you for opening this issue. @TheBestTvarynka what do you think about this?

[AvivNaaman]

@CBenoit @TheBestTvarynka -- Any updates regarding this suggestion?

[TheBestTvarynka]

@AvivNaaman sorry for the big delay 😔
Currently, I'm on vacation.
I'm going to answer in a day or two. Thank you 😊

[TheBestTvarynka]

Hi, @AvivNaaman. I finally got a chance to look at your problem.
I agree that we must patch the code to support message signing (MIC generation). Our implementation is RDP auth oriented (CredSSP and related stuff). We need to improve it.

About your proposed patch: I have a suspicion that it will break a regular RDP auth in RDP. I think it's better to add the make_signature method to the Sspi trait and implement it for Ntlm (and maybe other protocols). The original SSPI has the MakeSignature function (https://learn.microsoft.com/en-us/windows/win32/api/sspi/nf-sspi-makesignature), so I think it's ok to extend our trait this way.

How urgent this is for you? I can implement it by myself when finish current tasks. Or I can guide you 😊

[AvivNaaman]

Hi @TheBestTvarynka,

Thanks for getting back to me.

This isn’t super urgent right now, since I’m already working on a local fix for the issue.

I’m pretty sure I can handle this on my own, but I wanted to mention something important about the key state after signing, as described in MS-SPNG: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-spng/b87587b3-9d72-4027-8131-b76b5368115f -- the signing key must be reset after its usage for generating the mechListMic.

I think it would be a good idea to add a function to reset the key state or maybe let us clone keys or instances. What do you think?

[TheBestTvarynka]

I’m already working on a local fix for the issue

👍

I think it would be a good idea to add a function to reset the key state or maybe let us clone keys or instances. What do you think?

I think you can do it in the make_signature function implementation. Cloning sounds better than resetting the original key state. Because resetting the original keys can affect the encrypt/decrypt_message functions.

[AvivNaaman]

@TheBestTvarynka As you can see, I pushed a basic commit that implements the make_signature and verify_signature for NTLM. The new functions use the same core logic as the encryption and decryption functions, and tests are passing!

What should we do regarding other providers (such as Kerberos)? Should we leave the implementations as todo!()'s, return an error code, or implement those functions properly?

I will add some documentation and tests down the road; I have also seen a reference to some QoS flag (which may be used for NTLM signing, but didn't dig into it just yet).

[TheBestTvarynka]

As you can see, I pushed a basic commit

Great! I am going to review it soon.

What should we do regarding other providers (such as Kerberos)?

I suggest to do the following:

1. Write blank implementation for make/verify_signature methods that simply return an error. Example:

   sspi-rs/src/lib.rs

   Lines 785 to 790 in a8272f7

   	fn query_context_stream_sizes(&mut self) -> Result<StreamSizes> {
   	Err(Error::new(
   	ErrorKind::UnsupportedFunction,
   	"query_context_stream_sizes is not supported",
   	))
   	}

2. Create an issue about implementing these methods for the Kerberos protocol. The thing is, we currently use the Kerberos encrypt/decrypt_message methods to generate GSS MIC tokens, which are signatures (at least ideologically)... We need to investigate how to refactor it better.

[AvivNaaman]

Thanks! I'll make the changes to the PR soon (including the CR).

Regarding the API of Kerberos for signing, I encountered the following in MSDN, under SSPI/Kerberos:
"(!) Note: Do not use the MakeSignature or VerifySignature functions when GSS_Wrap and GSS_Unwrap are called for" here.
The GSS_(Un)Wrap function(s) are basically encryption/decryption functions, and they're the correct functions to be used for kerberos. Maybe in such case, it will be best to redirect users to take advantage of the encryption/decryption functions instead of using the *_signature functions.

[TheBestTvarynka]

"(!) Note: Do not use the MakeSignature or VerifySignature functions when GSS_Wrap and GSS_Unwrap are called for" here.

🤔 got it. thank you for the clarification. I haven't seen It before 😅