versat / cntlm

Cntlm is an NTLM / NTLM Session Response / NTLMv2 authenticating HTTP proxy intended to help you break free from the chains of Microsoft proprietary world. More info on http://cntlm.sourceforge.net/ website. This version also supports: SSPI (on Windows, NTLM authentication only), Kerberos authentication, IPv6, proxy PAC files.
GNU General Public License v2.0
118 stars 41 forks source link

use userDom instead of target for NTOWFv2 hash function #84

Closed arminfelder closed 1 year ago

arminfelder commented 1 year ago

use userDom instead of target for NTLMv2 hash function, as specified in MS-NLMP p. 59

Fixes #83

fralken commented 1 year ago

Hello @arminfelder, I tested your patch but it doesn't work for me. Proxy authentication fails in this case.

arminfelder commented 1 year ago

@fralken could you generate me a network dump (pcap for wireshark) with a test user, and the corresponding cntlm config and psw?

fralken commented 1 year ago

I'm afraid I cannot provide this, I do not have a test environment.

arminfelder commented 1 year ago

@fralken hmm, ok that makes it more complicated, but it could only be a minor edge case, as the related code is pretty simple. Could you send me your cntlm config without hashes(I am mainly interessted in the domain string, and additional flags), and the target header and payload(should be automatically dissected by wireshark), from a network dump?

fralken commented 1 year ago

The challenge from the proxy has these flags: 0xA2898205. Also, the user domain is different from the proxy domain.

arminfelder commented 1 year ago

@fralken thanks, if the proxy expects a user domain different from the one set in cntlm config, then I it will definatley fail with my initial version. Please try again wit my updated PR

fralken commented 1 year ago

Unfortunately it doesn't work. You are taking the domain in the wrong way. Look at this code snippet: https://github.com/versat/cntlm/blob/83f7c54ac63f4e3bc51aedcbdc6dbe06bf3e2b20/ntlm.c#L354-L389 The data pointed by tbofs is not a string, in fact it is trasformed by printuc() function before printing to console (in debug mode). You are treating it like a string instead. What is your logging when runnning with -q? Mine is something like:

NTLM Challenge:
        Challenge: 01220A25FC62365C (len: 282)
            Flags: 0xA2898205
        NT domain: ****
           Server: ****
           Domain: ****
             FQDN: ****
              TLD: ****
                7: ���
            TBofs: 64
            TBlen: 218
            ttype: 0

That is, starting at tbofs there is all this information (each field in the format: type, length, value)

arminfelder commented 1 year ago

ah yes your are right, the payload is usualy encoded in as 16bit unicode(as it is requested this way), so I will have to decode it as well in my case I get:

NTLM Challenge:
    Challenge: F361F3897F25CB64 (len: 48)
        Flags: 0xA0898205
        TBofs: 0
        TBlen: 0
        ttype: 21582
NTLMv2:
        Nonce: ****
    Timestamp: 133093040380000000
NTLM Response:
     Hostname: '****'
       Domain: '****'
     Username: '***'*
     Response: '*****' (51)
     Response: '*****' (24)

I do not get any information form the target as my Proxy (MacAffee Webgateway) is ignoring the target request flag.

arminfelder commented 1 year ago

I now changed the code o use printuc, unfortunatly I do not have a test environment with a windows server, providing the target infos, I will have to think about how to make such a setup

fralken commented 1 year ago

It is still not correct. The array pointed by tbofs contains a sequence of data in the form: type (2 bytes), length (2 bytes), value (16 bit unicode format). In my case the sequence is: NT Domain (type 2), Server(type 1), Domain (type 4), FQDN (type 3), TLD (type 5) and an unknown type 7. I also tried and extract only the Domain (and also NT Domain), but it doesn't work for me. It works only with the target info in the same format as it is received.

I see that your problem is you don't receive the target info at all, so you cannot compute the hash with it, and so you need to rely on the user domain. Maybe a fix that checks whether tbofs is empty (i.e. tblen == 0) could do the trick.

Can you test your connection with curl? I see from the source code that curl uses the target info as well (from what I understand), It would be interesting to check how it deals with your use case. The command is something like:

curl -v --proxy http://<your proxy> --proxy-ntlm --proxy-user <your user> <your dest url>
fralken commented 1 year ago

Can you try my branch https://github.com/fralken/cntlm/tree/useUserDomForHash if it works for you? My patch is https://github.com/fralken/cntlm/commit/83a6cec518d80241c5963dcc6eaa24d45129ed27

arminfelder commented 1 year ago

@fralken thx your version works fine for me

sonarcloud[bot] commented 1 year ago

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell B 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

arminfelder commented 1 year ago

I merged your code into my branch, I guess we now have something which works for both of us, ot be 100% compliant with the standard, we would need to make the behavior configurable via conf file, as the target includes the serverDomain, but hashing to userDom "SHOULD" be used , but I would say lets leave it here for now

fralken commented 1 year ago

Good news! With this patch at least the function is not aborted when the target info is missing (i.e. tblen == 0), so the authentication message is completed.

Maybe in your case the real problem was this code https://github.com/versat/cntlm/blob/83f7c54ac63f4e3bc51aedcbdc6dbe06bf3e2b20/ntlm.c#L399-L401 that caused un incomplete authentication message.

What do you think?

fralken commented 1 year ago

Just to be sure, can you test again my branch https://github.com/fralken/cntlm/tree/useUserDomForHash ? Now the patch is https://github.com/fralken/cntlm/commit/582e709a77f156d68fda8cbe4407bcb710e54654

fralken commented 1 year ago

Just to be sure, can you test again my branch https://github.com/fralken/cntlm/tree/useUserDomForHash ? Now the patch is fralken@582e709

@arminfelder can you test if this version works for you?

arminfelder commented 1 year ago

@fralken sorry for waiting, fralken@582e709 works fine for me

fralken commented 1 year ago

Thanks, this means that the actual issue was that the message is not completed when the target info is missing from the challenge. This patch is already applied with commit 533f1379af36a5a49cd6bdc074a686937ce9242e, so we can close this PR.