-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
SChannel: Support SCH_USE_STRONG_CRYPTO #6734
Conversation
fixup: Match format preferences for
lib/vtls/schannel.c
Outdated
@@ -335,6 +335,11 @@ set_ssl_ciphers(SCHANNEL_CRED *schannel_cred, char *ciphers) | |||
alg = get_alg_id_by_name(startCur); | |||
if(alg) | |||
algIds[algCount++] = alg; | |||
#ifdef SCH_USE_STRONG_CRYPTO |
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.
Checks revealed that old toolchains don't provide SCH_USE_STRONG_CRYPTO
. Is it better to do this, or should I rather do
#ifndef SCH_USE_STRONG_CRYPTO
#define SCH_USE_STRONG_CRYPTO 0x00400000
#endif
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.
should I rather do
#ifndef SCH_USE_STRONG_CRYPTO #define SCH_USE_STRONG_CRYPTO 0x00400000 #endif
yes
I'm happy with the patch as is, and all test failures appeared to be unrelated. Just tell me if further actions are required. |
It looks fine, it will be added next feature window. Ignore the test failures that's an unrelated issue. |
Thanks. I just landed a slightly modified version, it documents SCH_USE_STRONG_CRYPTO instead of USE_STRONG_CRYPTO but both are accepted. I did this for consistency because the other schannel cipher options use OS symbols, eg CALG_ECDH_EPHEM. |
Feature was discussed on curl-library@cool.haxx.se;
Subject: Adding flags to SChannel cred
.I wasn't sure about where, and to what extent, this should be documented.
Also note that I tested this by compiling curl.exe on the latest preview on windows 10. Downloading https://clienttest.ssllabs.com:8443/ssltest/viewMyClient.html with and without
--ciphers USE_STRONG_CRYPTO
and diffing the results shows that 3des is correctly disabled.