Add identify verification option for TLS #1554
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Right now the hostname presented on a TLS certificate is never validated. This means that man-in-the-middle attacks are trivial when using for example public CAs to sign certificates. The issue is that anyone can have a public CA sign a certificate for a hostname they own, which is key to the security model there. But with the current functionality in the `mysql2` driver, the hostname is never verified. This means an attacker could do a man in the middle attack and only has to present a signed certificate but doesn't have to match the hostname. It does not only apply to public CAs in system roots, but also for example Amazon RDS where a single root chain is also used. The changes here add an option to explicitly verify the identity on the certificate. This option is available in other MySQL drivers as well, such as `libmysqlclient` and everything based on that (often drivers for Ruby, Python etc.). Also other languages like Go, Java, .Net etc. provide an option to have strict identity verification. Ideally of course, this validation would be the default and always enforced, but that also would likely be a significant breaking change, so right now in the code here the setting is opt-in for having a secure connection to MySQL. Additionally, it adds SNI on the connection which is a best practice as well.
|
@sidorares Any feedback on this change? |
@sidorares Any update on this one? |
@sidorares Any updates on this one? |
sidorares
approved these changes
Nov 10, 2022
|
@dbussink sorry for the wait, ( and thanks for your persistence ) Your change looks good to me and I plan to have it as part of v3.0.0 ( and probably enable by default in 3.1.0 ) |
|
Hello , do you have an expected delivery date for the correction of this vulnerability? I understand you'll fix it in 3.1.0? Thanks |
|
@dbouchierarcad I'll do one more 3.0.0 rc with this change, should be part of final v3.0 |
|
in terms of ETA - the only blocker is release-please configuration, I can manually publish |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Right now the hostname presented on a TLS certificate is never validated. This means that man-in-the-middle attacks are trivial when using for example public CAs to sign certificates.
The issue is that anyone can have a public CA sign a certificate for a hostname they own, which is key to the security model there. But with the current functionality in the
mysql2driver, the hostname is never verified.This means an attacker could do a man in the middle attack and only has to present a signed certificate but doesn't have to match the hostname.
It does not only apply to public CAs in system roots, but also for example Amazon RDS where a single root chain is also used.
The changes here add an option to explicitly verify the identity on the certificate. This option is available in other MySQL drivers as well, such as
libmysqlclientand everything based on that (often drivers for Ruby, Python etc.). Also other languages like Go, Java, .Net etc. provide an option to have strict identity verification.Ideally of course, this validation would be the default and always enforced, but that also would likely be a significant breaking change, so right now in the code here the setting is opt-in for having a secure connection to MySQL.
Additionally, it adds SNI on the connection which is a best practice as well.