Skip to content
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

Modifies EncryptionLevel to Off instead of NotSupported #708

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

boersmamarcel
Copy link

Resolves #707

@boersmamarcel
Copy link
Author

@pangjunrong could you help out with this merge? If there are additional steps I need to take then please let me know. Or could you refer me to somebody else to help get this merge approved?

@pangjunrong
Copy link
Contributor

pangjunrong commented Nov 24, 2024

So I spent some time understanding the underlying mechanisms for the different encryption options from [MS-SSTDS]: Tabular Data Stream Protocol Version 4.2 (specifically 2.2.6.4) and came across several possible issues with using EncryptionLevel::Off, such as tiberius #320.

I think the most comprehensive way forward would be to account for all possible DB setups — in your case, the expected connection might not be encrypted but EncryptionLevel::Off is necessary because a TLS handshake is still expected during PRELOGIN. For someone else, their setup might not support TLS at all and having EncryptionLevel::Off might lead to a timeout as the client attempts a secured authentication handshake prior to the unencrypted connection.

May I propose that when encrypt=false, we do EncryptionLevel::Off and otherwise we default to EncryptionLevel::NotSupported?

For the merge to be approved, you will need @wangxiaoying

@wangxiaoying
Copy link
Contributor

Thanks @boersmamarcel for the PR!

I agree with @pangjunrong that we might want to have both EncryptionLevel::Off and EncryptionLevel::NotSupported supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ConnectorX MsSQL encryptionlevel TimedOut error
3 participants