Skip to content

[MSSQL] Fix regression: Connection establishment #2005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rtpt-romankarwacik
Copy link
Contributor

This commit broke mssqlclient.py for me: a60a1f1

With the commit a connection to a Microsoft SQL Server 2012 is not possible anymore, during connection establishment (TLS client hello) the server does not respond. The only difference between the packets in the old impacket version I tested ( abfaea2 ) and the commit above is that a tlsv1 client hello is now used, wheres before it was a tlsv1.2 client hello.

This commit reintroduces the explicit setting of the ciphers, which seems to fix this. I am not sure if this is platform dependent which TLS record layer is used, I am testing on Debian 12 with Python 3.11.2 and OpenSSL 3.0.16-1~deb12u1.

@rtpt-romankarwacik rtpt-romankarwacik changed the title Fix regression: MSSQL connection establishment [MSSQL] Fix regression: Connection establishment Jul 16, 2025
@gabrielg5 gabrielg5 self-assigned this Jul 16, 2025
@Dfte
Copy link
Contributor

Dfte commented Jul 16, 2025

Damn sorry for the trouble and thanks for the patch :)!!

@gabrielg5 gabrielg5 added bug Unexpected problem or unintended behavior high High priority item enhancement Implemented features can be improved or revised and removed bug Unexpected problem or unintended behavior enhancement Implemented features can be improved or revised labels Jul 17, 2025
@gabrielg5
Copy link
Collaborator

gabrielg5 commented Jul 17, 2025

Hey guys,

Testing this I found that @rtpt-romankarwacik changes are not enough (at least in my environment - win10, py3.12, sql2012 -)
I'm getting [-] [SSL: UNSUPPORTED_PROTOCOL] unsupported protocol (_ssl.c:1000) (I have the TLS minimum_version in the context as TLS 1.2)
So we need to change the minimum_version of the context.

This reminds me a PR I've been working on last week #2003 (perhaps not the best PR name :) )
We are redefining there the create_default_context function of the ssl module to create an "insecure" context we can use to connect to old systems.

I guess we can leverage that function to create ssl contexts that allow connection to both new and old TLS versions

EDIT: If setting the minimum_version (context.minimum_version = ssl.TLSVersion.MINIMUM_SUPPORTED) connection can be established ok in my env

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected problem or unintended behavior high High priority item
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants