-
Notifications
You must be signed in to change notification settings - Fork 294
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
Conversation
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? |
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. |
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. |
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. |
Ok, got it now 👍 |
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. |
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.