-
-
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
lib/config-win32.h: let SMB/SMBS be enabled with OpenSSL/NSS #1943
Conversation
lib/config-win32.h
Outdated
@@ -709,7 +709,7 @@ Vista | |||
#endif | |||
|
|||
/* Define to use the Windows crypto library. */ | |||
#if !defined(USE_OPENSSL) && !defined(USE_NSS) | |||
#if defined(USE_SCHANNEL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have changed it to use win32 crypto only if schannel but is that really what you meant? Aren't crypto and schannel two separate things, and now that there are changes in ntlm to handle win32 crypto properly can't we just enable USE_WIN32_CRYPTO all the time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going along @dscho's suggestion here, but, yes, as far as I can see the code, this macro is protecting crypto-only and unless I'm missing something NTLM-crypto-only functions (2-4 of them) throughout all lib
sources. If this is correct, it could be enabled based on _WIN32
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're in config-win32.h
, _WIN32
should always be defined, meaning that USE_WIN32_CRYPTO
could just be enabled unconditionally in this header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
USE_WIN32_CRYPTO
requires: wincrypt.h
, CryptAcquireContext()
, CryptCreateHash()
, CryptHashData()
, CryptGetHashParam()
, CryptDestroyHash()
, CryptReleaseContext()
, CryptImportKey()
, CryptImportKey()
, CryptDestroyKey()
.
As per MSDN, these are available starting Windows XP/2003, so the condition required is _WIN32_WINNT >= 0x0501
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per MSDN, these are available starting Windows XP/2003
That's only because mentions of old unsupported OSs are regularly removed from the MSDN documentation. https://msdn.microsoft.com/en-us/library/ms867086.aspx says it's at least available since Windows NT 4 and Windows 95.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI Windows 2000 has all those functions xported in advapi32. Whether they're actually declared or not for Windows 2k I don't know (yes in original mingw), but we've never had an os version check on that symbol before and people do build for Win2k at least according to the survey. I think if you remove the guard it will be fine. If someone is building for Windows 98 (how?) we can wait for feedback if it breaks for them.
edit: run it through CI just to be sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'll commit that and wait for a green flag of course. MSDN tends to be unreliable for minimum versions, exactly like @MarcelRaad says.
The source code is now prepared to handle the case when both Win32 Crypto and OpenSSL/NSS crypto backends are enabled at the same time, making it now possible to enable `USE_WIN32_CRYPTO` whenever the targeted Windows version supports it. Since this matches the minimum Windows version supported by curl (Windows 2000), enable it unconditionally for the Win32 platform. This in turn enables SMB (and SMBS) protocol support whenever Win32 Crypto is available, regardless of what other crypto backends are enabled. Ref: #1840 (comment)
The source code is now prepared to handle the case when both
Win32 Crypto and OpenSSL/NSS crypto backends are enabled
at the same time, making it possible to enable
USE_WIN32_CRYPTO
whenever the targeted Windows version supports it. Since this
matches the minimum Windows version supported by curl
(Windows 2000), enable it unconditionally for the Win32 platform.
This in turn enables SMB (and SMBS) protocol support whenever
Win32 Crypto is available, regardless of what other crypto backends
are enabled.
Ref: #1840 (comment)