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

Perform Certificate Validation when Force Encryption enabled on Server #391

Merged
merged 3 commits into from
Mar 10, 2020

Conversation

cheenamalhotra
Copy link
Member

@cheenamalhotra cheenamalhotra commented Jan 21, 2020

Changes default behavior of driver to not validate server certificate if client did not request encryption with "encrypt=true" but encryption was enforced by SQL Server.

This is highly observable when working with Azure DB and client does not request encryption with "encrypt=true" specified in Connection String, in which case the driver does not validate server certificates but enables SSL over TLS.

With this change, the driver shall always validate certificate if encryption is "Required" to happen, irrespective of whether client provides "encrypt=true" in connection string or not.

@Wraith2
Copy link
Contributor

Wraith2 commented Jan 21, 2020

I may be misunderstanding but the first and last sentences don't seem to agree with each other.

From the code it looks like if encryption is forced on by the server side then the code will act as if trusting the server certificate was enabled even if it wasn't set in the connection string. Isn't this going to break a lot of azure users who aren't expecting to need to add the certificate as trusted?

@cheenamalhotra
Copy link
Member Author

I updated description and hopefully it's clear now.

Yes, it will impact users who didn't enable "encrypt" mode from client but were relying on Server side Forced Encryption to validate certificates. If client applications already provide "encrypt=true" in connection string, they will not be impacted, so we are looking at a very less percentage of audience here, as this is mostly expected for Azure/secure SSL connections.

And since we are going for a major version bump, it brings us an opportunity to do things right.

@Wraith2
Copy link
Contributor

Wraith2 commented Jan 21, 2020

Ok. So users with a standard connection string today without setting either Encrypt or TrustServerCertificate and connecting to azure will get an encrypted connection and implicitly trust the server certificate.

This change will make is so that if you don't specify either you get Encrypt=Yes TrustServerCertificate=No behaviour,, but if you specify Encrypt=Yes you get TrustServerCertificate=Yes implicitly. So essentially forcing users to set Encryption or they'll get notified that they need to take some action to configure their connection string, forcing them to read the docs essentially and hoping they use the certificates now they know they can.

@cheenamalhotra
Copy link
Member Author

if you specify Encrypt=Yes you get TrustServerCertificate=Yes implicitly

I would like to correct, with this change, if you specify "Encrypt=yes", you'd still get behavior as "TrustServerCertificate=No" since that's the default value unless users specify "TrustServerCertificate=Yes" in connection string.

Currently without specifying "Encrypt=yes" and connecting to Azure, the driver is working as "TrustServerCertificate=Yes", which is a bug.

@Wraith2
Copy link
Contributor

Wraith2 commented Jan 21, 2020

Ok, got it now 👍

@David-Engel
Copy link
Contributor

Currently without specifying "Encrypt=yes" and connecting to Azure, the driver is working as "TrustServerCertificate=Yes", which is a bug.

I wouldn't say it's a bug. It was by design. If you are running a box install with a certificate that is not part of a client's trust chain (root CAs), like a self-signed certificate, you could simply force encryption on the server side and clients would start connecting encrypted without trying to validate your certificate. With this change, that scenario breaks until clients update their connection string to specifically trust the server certificate or the server uses a certificate that is trusted by the client's trust chain. In the days of cloud computing, it's getting more dangerous to let users run around blindly trusting servers by default. This is a step short of changing Encrypt to true by default. Remember when browsers started blocking websites by default if there was a certificate problem? Same concept.

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.

3 participants