Skip to content

[MSSQL] Implements Channel Binding Token - #1986

Merged
gabrielg5 merged 7 commits into
fortra:masterfrom
Dfte:mssql_rework
Jul 7, 2025
Merged

gabrielg5 merged 7 commits into
fortra:masterfrom
Dfte:mssql_rework

Conversation

@Dfte

@Dfte Dfte commented Jun 21, 2025 •

Copy link
Copy Markdown
Contributor

Hello,

The following PR contains all the work I have done on the TDS script which:

  • Updates / refactores the code
  • Handles the entire in-memory handshake via pure ssl (no more PyOpenSSL needed as it doesn't support the tls-unique variable)
  • Adds Channel Binding computing

The code was entirely commented so that pretty much all the authentication logic is clear and can be understood (and a blogpost is going to be released ASAP for more information)

As of now, this PR allows mssqlclient.py to support pretty much all hardening configuration including encryption and channel binding except TDS 8 which I will push later.

Please note that I only tested the code on a Microsoft SQL Server (160 3232) (windows server 2022) and made sure that:

  • Authentication (NTLM/Kerberos) works via a domain account and sa if encryption and CBT is disabled ;
  • Authentication (NTLM/Kerberos) works via a domain account and sa if only encryption is enabled ;
  • Authentication (NTLM/Kerberos) works via a domain account and sa if encryption and channel binding is enabled and requried.

For anyone who's willing to play with the PR, here is how you can enable encryption on the sql panel:

image

And how you can enable the Channel Binding Token check:

image

In the upcomming weeks I'll test this PR during assessments to make sure that it does work with older MSSQL version. Until then, let me know if you need anything :)!

@Dfte Dfte changed the title [MSSQL] TDS review, code refactoring, comments, native ssl in memory handshake and channel binding [MSSQL] Implements Channel Binding Token Jun 21, 2025
@Dfte

Dfte commented Jun 22, 2025

Copy link
Copy Markdown
Contributor Author

NTLMRelayX now supports the TDS modification (in-memory handshake review mostly) and doesn't need pyopenssl anymore:
image

@anadrianmanrique

anadrianmanrique commented Jun 22, 2025 •

Copy link
Copy Markdown
Collaborator

@Dfte thanks, this PR looks awesome!
after a quick look, I would recommend to avoid cosmetic changes, as that's something we want to control, and also helps us to keep better track of changes and improve review time responses. It would be great if those specific changes could be reverted, before we start review process.
Thank you!

@Dfte

Dfte commented Jun 23, 2025 •

Copy link
Copy Markdown
Contributor Author

Hey @anadrianmanrique ! Got it! Just to be sure, can I leave the comments ? I believe commenting while make it much more easier to understand for futur maintainers. At least I would have not last 3 days understanding the code if I had that much comments and the TLS handshake in-memory part is quite complicated as well so... Let me know! I'll revert the others things :)

@Dfte

Dfte commented Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

Heyo! So I have removed the unecessary refactoring/code updates only to leave the code I modified myself and the comments I added to make it easier to understand. Let me know if you need anything ;)!

@anadrianmanrique

anadrianmanrique commented Jun 23, 2025 •

Copy link
Copy Markdown
Collaborator

@Dfte thanks! Comments are more than ok! I was referring specifically to code changes

@gabrielg5 gabrielg5 self-assigned this Jun 24, 2025
@anadrianmanrique anadrianmanrique added the medium Medium priority item label Jun 26, 2025
@Dfte

Dfte commented Jun 30, 2025 •

Copy link
Copy Markdown
Contributor Author

I'm actually working on the blogpost that describes my journey implementing this feature. Just finished this schema that explains the new socket logic. Thought it would help reviewing the PR so here it is :)

image

Let me know if you need anything else!!

@gabrielg5

Copy link
Copy Markdown
Collaborator

Hey @Dfte hello!
Will start reviewing the PR, I think will push some changes to your branch (mainly to revert single-quote / double-quotes changes); not to bother you with those cosmetic changes.
That way will be much simpler isolate important changes to check.

(I guess we should define and ty to a codingstandard moving forward to solve this)
Thanks!

@Dfte

Dfte commented Jul 1, 2025

Copy link
Copy Markdown
Contributor Author

All right! Let me know if I can help, next time I'll push things I'll make sure not to change cosmetics ;)!
But yeah if you ever need help doing code review/refactor I'll gladly help

@gabrielg5

Copy link
Copy Markdown
Collaborator

Checking the new functions...

I think we may be missing some data in the send/receive functions (writing while understanding that, so perhaps not fully clear right now)

tls_send

  • Data is written in the TLS context
  • 4096 encrypted bytes are read and finally sent to the server

If encrypted data length is > 4k we may be missing to send some bytes
Analogous thing is happening in the tls_recv function

@Dfte

Dfte commented Jul 2, 2025 •

Copy link
Copy Markdown
Contributor Author

About the tls_send function, it's not entirely true. The code actually loops and tries to read 4096 bits of encrypted data that is sent right after via the socket.sendall function. If we have a SSLWantReadError or WriteError then that means the TLS context gave us all the encrypted data which is why I break out of the while True condition.

That's pretty much the same with tls_recv, the only difference is that the loop is handled directly witthin the recvTDS function. Inside the TDS structure there is the "Lenght" field that tells us how many bits are sent, so the recvTDS function loops until it get all the data then calls socketRcv that dispatches the data depending of if there is a TLS context or not.

So far I didn't have troubles using these functions but I'll try to add data into a dabtase and retrieve the content to make sure everything works as expected :)!

So I have tried uploading PsExec using the upload command and it works correctly:

image

Even thought there is an error at the end, the base64 file is uploaded:

image

And it corresponds the legitimate tool once decoded:

image

@gabrielg5

gabrielg5 commented Jul 2, 2025 •

Copy link
Copy Markdown
Collaborator

Hey, yes, you are right! tls_send is handling the scenario in its loop.

I still doubt with the tls_recv (theoretical doubts :D) as we are receiving 4k encrypted data but reading bufsize from the tlssocket; and there's no loop in here.

Will try to check this function in practice to validate this.

Also will start doing some tests with different server versions. Will let you know how those go
Thanks!

@Dfte

Dfte commented Jul 2, 2025

Copy link
Copy Markdown
Contributor Author

Actually I believe it does in the recvTDS:

while status != TDS_STATUS_EOM:
            if remaining is not None:
                tmpPacket = TDSPacket(remaining)
            else:
                tmpPacket = TDSPacket(self.socketRecv(packetSize))

            packetLen = tmpPacket["Length"] - 8
            while packetLen > len(tmpPacket["Data"]):
                data = self.socketRecv(packetSize)
                tmpPacket["Data"] += data

            remaining = None
            if packetLen <  len(tmpPacket["Data"]):
                remaining = tmpPacket["Data"][packetLen:]
                tmpPacket["Data"] = tmpPacket["Data"][:packetLen]

            status = tmpPacket["Status"]
            packet["Data"] += tmpPacket["Data"]
            packet["Length"] += tmpPacket["Length"] - 8

TDS_STATUS_EOM indicates it's the last packet being sent by the MSSQL server ^^

@gabrielg5

Copy link
Copy Markdown
Collaborator

I think there is something wrong with that.

Let me share my tests real quick...
(I added some debug lines to the tds.py to get some more info there - I can push those to the branch, at least while we are testing it and we can remove them before merge -)

  • I'm working with a SQLServer 2022 enforcing encryption
  • I open a sql shell authenticating with a sysadmin account
    python mssqlclient.py test@10.10.10.10 -debug
  • Try to download a 25KB file from the SQLServer host to my kali box
    SQL (test dbo@master)> download C:\a\test.txt /tmp/test.txt
  • This is the trace I receive
[+] [tls_send] #1 len(data): 196 - len(encrypted): 225
[+] [tls_send] #2 len(data): 196 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 106 - len(decrypted): 77
[+] [tls_send] #1 len(data): 116 - len(encrypted): 145
[+] [tls_send] #2 len(data): 116 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 183 - len(decrypted): 154
[+] File exists, downloading...
[+] [tls_send] #1 len(data): 160 - len(encrypted): 189
[+] [tls_send] #2 len(data): 160 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] except ssl.SSLWantReadError
[+] [tls_recv] except ssl.SSLWantReadError
[+] [tls_recv] except ssl.SSLWantReadError
[+] [tls_recv] bufsize: 16192 - len(encrypted): 3933 - len(decrypted): 16192
[+] [tls_recv] except ssl.SSLWantReadError
[-] Unhandled Exception: ('unpack requires a buffer of 1 bytes', "When unpacking field 'Type | <B | b''[:1]'")
[+] [tls_send] #1 len(data): 128 - len(encrypted): 157
[+] [tls_send] #2 len(data): 128 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] except ssl.SSLWantReadError
[+] [tls_recv] bufsize: 16192 - len(encrypted): 735 - len(decrypted): 8898
[-] Unknown Token 75
SQL (-@master)> 

I guess we are not handling ok the ssl.SSLWantReadError exception in the tls_recv function.

Side note, same scenario but with upload command, works fine

SQL (test  dbo@master)> upload /tmp/test.txt C:\a\test.txt
[+] [tls_send] #1 len(data): 150 - len(encrypted): 179
[+] [tls_send] #2 len(data): 150 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 353 - len(decrypted): 324
[+] [tls_send] #1 len(data): 96 - len(encrypted): 125
[+] [tls_send] #2 len(data): 96 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 227 - len(decrypted): 198
[+] [tls_send] #1 len(data): 150 - len(encrypted): 179
[+] [tls_send] #2 len(data): 150 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 353 - len(decrypted): 324
[+] Data length (b64-encoded): 32.55 KB with MD5: 77fd81db56175e7393c8f80a5062e3b9
[+] Uploading...
[+] [tls_send] #1 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #2 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #3 len(data): 10346 - len(encrypted): 2183
[+] [tls_send] #4 len(data): 10346 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 91 - len(decrypted): 62
[+] [tls_send] #1 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #2 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #3 len(data): 10346 - len(encrypted): 2183
[+] [tls_send] #4 len(data): 10346 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 91 - len(decrypted): 62
[+] [tls_send] #1 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #2 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #3 len(data): 10346 - len(encrypted): 2183
[+] [tls_send] #4 len(data): 10346 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 91 - len(decrypted): 62
[+] [tls_send] #1 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #2 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #3 len(data): 10346 - len(encrypted): 2183
[+] [tls_send] #4 len(data): 10346 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 91 - len(decrypted): 62
[+] [tls_send] #1 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #2 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #3 len(data): 10346 - len(encrypted): 2183
[+] [tls_send] #4 len(data): 10346 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 91 - len(decrypted): 62
[+] [tls_send] #1 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #2 len(data): 10346 - len(encrypted): 4096
[+] [tls_send] #3 len(data): 10346 - len(encrypted): 2183
[+] [tls_send] #4 len(data): 10346 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 91 - len(decrypted): 62
[+] [tls_send] #1 len(data): 5338 - len(encrypted): 4096
[+] [tls_send] #2 len(data): 5338 - len(encrypted): 1271
[+] [tls_send] #3 len(data): 5338 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 91 - len(decrypted): 62
[+] [tls_send] #1 len(data): 86 - len(encrypted): 115
[+] [tls_send] #2 len(data): 86 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 197 - len(decrypted): 168
[+] Uploaded
[+] [tls_send] #1 len(data): 154 - len(encrypted): 183
[+] [tls_send] #2 len(data): 154 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 280 - len(decrypted): 251
[+] certutil -decode "C:\a\test.txt.b64" "C:\a\test.txt"
[+] [tls_send] #1 len(data): 96 - len(encrypted): 125
[+] [tls_send] #2 len(data): 96 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 91 - len(decrypted): 62
[+] del "C:\a\test.txt.b64"
[+] [tls_send] #1 len(data): 126 - len(encrypted): 155
[+] [tls_send] #2 len(data): 126 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 318 - len(decrypted): 289
[+] certutil -hashfile "C:\a\test.txt" MD5
[+] MD5 hashes match
[+] [tls_send] #1 len(data): 128 - len(encrypted): 157
[+] [tls_send] #2 len(data): 128 - len(encrypted): 0
[+] [tls_send] not encrypted
[+] [tls_recv] bufsize: 16192 - len(encrypted): 99 - len(decrypted): 70
SQL (test  dbo@master)> 

@gabrielg5

Copy link
Copy Markdown
Collaborator

@Dfte, just pushed the debug logging I used to check that... will revert once we check it
Thanks!

@Dfte

Dfte commented Jul 3, 2025

Copy link
Copy Markdown
Contributor Author

Alright I'll take a look at that later this afternoon :)! Thanks for the feedback!

@Dfte

Dfte commented Jul 3, 2025

Copy link
Copy Markdown
Contributor Author

Hey @gabrielg5 ,

I have been trying to repoduce the error and realized it might not be related with my PR. Here I enabled TLS only (no CBT required):

  • TLS conf:

image

  • CBT conf:

image

And I get an error as well:

image

So there is definitely an issue here but may be not from my PR but rather the original code. I'll try to debug that right now :)!

@gabrielg5

gabrielg5 commented Jul 3, 2025 •

Copy link
Copy Markdown
Collaborator

yep, performed same test in master and download is failing as well...

SQL (test  dbo@master)> download C:\a\test.txt /tmp/test.txt
[+] File exists, downloading...
[-] Unhandled Exception: 
SQL (-@master)> 

@Dfte

Dfte commented Jul 3, 2025 •

Copy link
Copy Markdown
Contributor Author

But I was actually able to upload xD, what happened ahah but the error comes from mssqlshell. Debugging it right now !

@gabrielg5

Copy link
Copy Markdown
Collaborator

mm perhaps some of the SQL functions we are calling in those actions?
I noticed that SQLServer 2014 is failing because sys.dm_os_file_exists does not exist there

@Dfte

Dfte commented Jul 3, 2025 •

Copy link
Copy Markdown
Contributor Author

Beside the print_exception, here is the fullstack
image

So yeah it's related to SSL but not to my PR ahah. We'll try to patch that ^^

And as you mentionned earlier, that error implies that we are not reading enough data from the TLS context ahah

Error is located in mssqlshell.py

image

The function does exist and work correctly:

image

@Dfte

Dfte commented Jul 3, 2025 •

Copy link
Copy Markdown
Contributor Author

Got some news! For some reason the error is located in that part of the code (the one from my PR):

 while status != TDS_STATUS_EOM:
            print("LAST PACKET FOUND")
            if remaining is not None:
                tmpPacket = TDSPacket(remaining)
            else:
                print(packetLen)
                tmpPacket = TDSPacket(self.socketRecv(packetSize))

            packetLen = tmpPacket["Length"] - 8
            while packetLen > len(tmpPacket["Data"]):
                data = self.socketRecv(packetSize)
                tmpPacket["Data"] += data

            remaining = None
            if packetLen <  len(tmpPacket["Data"]):
                remaining = tmpPacket["Data"][packetLen:]
                tmpPacket["Data"] = tmpPacket["Data"][:packetLen]

            status = tmpPacket["Status"]
            packet["Data"] += tmpPacket["Data"]
            packet["Length"] += tmpPacket["Length"] - 8

Removing it does allow you downloading the entire file:

mssqlclient.py WHITEFLAG.LOCAL/Administrateur:'Defte@WF'@adcs.whiteflag.local -windows-auth -debug -c "download C:\\Windows\\temp\\PsExec.exe.b64 /tmp/PsExec.exe"

image

The reason the download fails is probably because, for some reason, the server sends a TDS status = 0 which triggers the while loop. Thing is, it's not the last packet. Thus the error "SSL.WantReadError". Now I need to find out why tf the status sent by the MSSQL server is 0 xD

EDIT: instead of removing the entire while loop, you can edit line 828:

status = packet["Status"]
status = 1

Okay I got a better idea of what's going on, basically the file is so huge MSSQL splits it into multiples TCP packets. Thus we do miss a loop reading all the pacekts.

@Dfte

Dfte commented Jul 3, 2025

Copy link
Copy Markdown
Contributor Author

Note that upload still work with my PR. However there is still that error:

mssqlclient.py WHITEFLAG.LOCAL/Administrateur:'Defte@WF'@adcs.whiteflag.local -windows-auth -debug -c "upload /opt/tools/exe/magnet_ram.exe C:\\Windows\\temp\\magnet_ram.exe"

image

image

This is probably due to the fact that the account running the MSSQL database is underprivileged (virtual MSSQL account).

@gabrielg5

Copy link
Copy Markdown
Collaborator

Just wondering, could it be related with the fact that package has to be decrypted all at once? I mean, receiving all the encrypted packages and once all have been received, decrypt
We worked in a similar issue a couple weeks ago (#1959), but in LDAP

@Dfte

Dfte commented Jul 5, 2025

Copy link
Copy Markdown
Contributor Author

I have no clue yet :/... This is beyond my PR but I'll try to take a look ASAP, not before monday tho :/

@Dfte

Dfte commented Jul 7, 2025 •

Copy link
Copy Markdown
Contributor Author

Here is a small update, if mssqlshell.py wasn't able to find the file after upload, it's because there are language hardcoded values that weren't found. Looking at this code:

print("EXEC xp_fileexist '" + remote_path + ".b64")
result = self.sql_query("EXEC xp_fileexist '" + remote_path + ".b64'")
print(remote_path, result)
if result[0].get('File Exists') != 1:
    print("[-] Error uploading file. Check permissions in the configured remote path")
    return
print("[+] Uploaded")

We can see that it expects a key to be named "File exists" but looking at my MSSQL server configured in french I get the following SQL response:

image

Means modifying "File exists" with "Ce fichier existe déjà", upload works:

image

Yet there is still the reading issue. Working on it :)!

@Dfte

Dfte commented Jul 7, 2025

Copy link
Copy Markdown
Contributor Author

Disabling TLS, I realized that I was able to download files corectly. So yeah this is related to the TLS context somehow...

@Dfte

Dfte commented Jul 7, 2025

Copy link
Copy Markdown
Contributor Author

GOT ITTTTTT!! You were right @gabrielg5 , the TLS _recv function wasn't reading enough data, here is the patch:

def tls_recv(self, bufsize):
        while True:
            try:
                # Try to read decrypted data first
                decrypted = self.tlsSocket.read(bufsize)
                if decrypted:
                    return decrypted
            except ssl.SSLWantReadError:
                pass  # Means we need more encrypted bytes

            # Read more encrypted bytes from the socket
            encrypted = self.socket.recv(bufsize)
            if not encrypted:
                # Remote closed the connection
                return b""

            self.in_bio.write(encrypted)

Now having both TLS and CBT required:

image

image

We can download files:

image

And also upload:

image

Nice catch! Do you mind reverting the changes and testing the patched function ? I'm sorry I don't have much time right now.... :/

@gabrielg5

Copy link
Copy Markdown
Collaborator

hey @Dfte, great!!

Been doing some tests with different SQLServer versions (2016, 2017, 2019, 2022) and all worked fine

Will update the codebase to test this patch for big messages

Will let you know how it goes

@gabrielg5

Copy link
Copy Markdown
Collaborator

It's working great!

I also found that

I noticed that SQLServer 2014 is failing because sys.dm_os_file_exists does not exist there

is also failing in SQLServer 2016


Will create this issue and the one you mentioned with the "File Exists" column so they can be worked on in separate tickets.

Will merge this one.

Thank you!!

@Dfte

Dfte commented Jul 7, 2025

Copy link
Copy Markdown
Contributor Author

Glad to hear that it works!! Once this PR is merged, I'll push two others:

  • One that implements "decrypting" the version of the MSSQL server ;
  • One that fixes "File exist" error when the sql database is not configured in english.

And once all of that is done, I'll integrate TDS 8 as well and we should have all MSSQL functions and securities handled!! :)

@Neustradamus

Copy link
Copy Markdown

TLS Channel Binding is linked to:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

medium Medium priority item

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants