-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
SNI: Update SNI related log messages #1240
Conversation
@JDepooter, thanks for your PR! By analyzing the history of the files in this pull request, we identified @mback2k, @nickzman and @bagder to be potential reviewers. |
Git blame puts the "Also disables SNI" comment in schannel at 1394cad, which has a commit message that says in part:
I found a bunch of e-mails from Nov 2012 written by the author but not the one that discusses this issue. I can confirm Windows 7 SP1 SNI is still sent when SCH_CRED_NO_SERVERNAME_CHECK is added to the flags. Test command /cc @okoeroo |
Hi, On the maillist archive I dug up this message: i was amazed about the ill documentation if this side effect really happens. Now you mention that this effect seems to be in line with the MSDN documentation on the Schannel API. I would recommend to follow this and have the message be corrected in libcurl to match the documentation and observation made by @JDepooter and pulled up by @jay. I did not verify the SNI (side-) effects at the time. I focussed on the peer-jacking attack only exclusively based on the paper which only tested libcurl with the OpenSSL backend. :-) |
Thanks for your input @okoeroo. I tested in Windows XP and the SNI is not sent regardless of whether the server name check is enabled. IE which also uses WinSSL shows the same behavior. We could make an <= XP specific message by Curl_verify_windows_version and then if verbose. (Also ref 6842afb) schannel: WinSSL version is too old to connect to some servers (missing SNI, algorithms) Also can we hear from someone with ability to test on Mac versions like @nickzman to know if there is similar inconsistency there. |
a2d8aee
to
d2ecd38
Compare
darwinssl: Turning off host validation does turn off SNI schannel: Turning off host validation does not turn off SNI. However, SNI is not available which SChannel on Windows XP machines, regardless of the host validation setting.
d2ecd38
to
a9e12a0
Compare
@jay I have updated the pull request with a Windows XP specific message which is logged at the beginning of |
- Remove the SNI disabled when host verification disabled message since that is incorrect. - Show a message for legacy versions of Windows <= XP that connections may fail since those versions of WinSSL lack SNI, algorithms, etc. Bug: #1240
Thanks. I've split your schannel changes off since it's sort of a different issue and landed it in 18495ec. The darwinssl change I think we should wait for someone who can test new and old Mac versions to check for inconsistency, like maybe old versions disabled SNI and new versions don't? Also typo s/validatation/validation |
ping. Anyone with Mac experience can give this a review so I can close it out? I found a modified version of darwinssl used by the maintainer of Git OS X Installer where he says:
@mackyle would you mind weighing in on the darwinssl part of this PR, ie that as long as SSLSetPeerDomainName is disabled SNI is disabled as well, that this holds true even in the latest Mac versions? |
Yeah, that's still true; there's no way to disable the peer domain check without also disabling SNI. |
Ok. Thanks guys |
@jay sorry, just saw this. But yeah, if you never call The whole section discussing this can be found in the comments at: https://github.com/mackyle/git-osx-installer/blob/master/patches/curl/curl_darwinssl_macosx.c#L34 There's a table in the comments:
It's only the third entry in the table above that ends up with surprising behavior. Just because you call Other than for literal IPv4/IPv6 names, the TLDR; Always call |
Update debug log messages related to SNI:
schannel: Turning off host validation does not turn off SNI
darwinssl: Turning off host validation does turn off SNI
I recently came across a case which worked on Windows using SChannel but not on a Mac using DarwinSSL. It turns out the request was accessing a machine on AWS which required SNI in the TLS handshake. The request was setup to turn off host validation.
Apparently, turning off host validation on DarwinSSL also turns off SNI, which is why the request failed on the Mac. I have added a debug message when this happens, as I think it will be useful for future debugging.
With SChannel , the debug messages claimed that turning off host validation also turned off SNI, but this is not the case. I have updated that debug message as well. See the attached screenshots for verification. It is possible that this behaviour differs between the various windows versions. I only have access to Windows 10.
Both OpenSSL and GnuTLS have the same behaviour as SChannel, SNI is still used when the verify host option is disabled.