-
-
Notifications
You must be signed in to change notification settings - Fork 652
fix: skip SNI for IP addresses in TLS connection #3835
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3835 +/- ##
==========================================
- Coverage 89.83% 89.78% -0.06%
==========================================
Files 86 86
Lines 13596 13607 +11
Branches 1606 1607 +1
==========================================
+ Hits 12214 12217 +3
- Misses 1382 1390 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
It should be possible to choose the servername. I connect with an IP address, but the certificate is generated with a domain (the FQDN) in the alt_names that isn't on any public DNS. By connecting with an IP address as host and a servername for the SSL configuration, this warning disappears. |
I think it makes a lot of sense to allow specifically setting the servername as you do in #3845. I considered adding it as part of this as well but I would classify it as a separate feature. If the SSL certificate does not have a FQDN/SAN, I will still need the change as proposed here. |
7300a89 to
780ab4d
Compare
|
Thanks, @mex! It's already available in |
|
Perfect, thanks! |
When connecting using TLS there is an optional field,
servername, for setting the Server Name Indication (SNI) but this field only makes sense to set if we are using a hostname to connect and not an IP. Since v12 of Node.js, it's been deprecated to provide an IP address as the SNI so it will be ignored at some point anyway.This PR prevents setting
servernamein the TLS config if the MySQL confighostfield is an IP which in turn quiets this deprecation warning:The logic for setting SNI to the host was originally added in #1554.
Fixes #2588