Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
Hi Ken, On 04/10/13 16:16, Ken Thomases wrote: Hi Jacek, On Apr 10, 2013, at 4:24 AM, Jacek Caban wrote: On 3/28/13 8:31 PM, Ken Thomases wrote: Mac OS X 10.8 introduced support for TLS 1.1 and 1.2. Can someone with Mac OS X 10.8 test the attached patch for me, please. All I need is to verify that it compiles and when running dlls/secur32/tests/secur32_test.exe.so schannel, TLS 1.1 and TLS 1.2 are listed as supported protocol. Thanks for doing this. I don't have a 10.8 build system handy, myself, or I would have done it or would test what you've done. However, Apple's guidance on using weak linking says that you must explicitly compare against NULL. They don't quite say that testing the symbol as a standalone boolean expression won't work, but they do say that using negation (the ! operator) isn't sufficient. https://developer.apple.com/library/mac/documentation/developertools/conceptual/cross_development/Using/using.html#//apple_ref/doc/uid/20002000-SW4 I'm not sure what the rationale is, but I think there's a peculiarity of what the compiler thinks it knows versus what the linker actually knows. Basically, the compiler figures there's a symbol that will be resolved (or cause a link error), so the check can be optimized away somehow. I think. Oh, such restriction seem to indicate a broken design of this weak linking... Following this is fine, through, but perhaps we should use nil to be sure that NULL override in winnt.h won't confuse the heuristic? Other than that, the patch looks good to me. Thanks for the review. Cheers, Jacek
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
On 3/28/13 8:31 PM, Ken Thomases wrote:
Mac OS X 10.8 introduced support for TLS 1.1 and 1.2.
Can someone with Mac OS X 10.8 test the attached patch for me, please.
All I need is to verify that it compiles and when running
dlls/secur32/tests/secur32_test.exe.so schannel, TLS 1.1 and TLS 1.2 are
listed as supported protocol.
Thanks,
Jacek
commit 78f9768f8d6759af1df99c4b67b8fd6a93369da4
Author: Jacek Caban ja...@codeweavers.com
Date: Tue Apr 9 12:35:33 2013 +0200
secur32: Added support for TLS 1.1 and TLS 1.2 on Mac.
diff --git a/dlls/secur32/schannel_macosx.c b/dlls/secur32/schannel_macosx.c
index 5ec06cf..27bb667 100644
--- a/dlls/secur32/schannel_macosx.c
+++ b/dlls/secur32/schannel_macosx.c
@@ -1007,7 +1007,25 @@ BOOL schan_imp_init(void)
supported_protocols = SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT |
SP_PROT_TLS1_0_CLIENT;
#if MAC_OS_X_VERSION_MAX_ALLOWED = 1080
-/* FIXME: Test max allowed version for TLS 1.1 and TLS 1.2 */
+if(SSLGetProtocolVersionMax) {
+SSLProtocol max_protocol;
+SSLContextRef ctx;
+OSStatus status;
+
+status = SSLNewContext(FALSE, ctx);
+if(status == noErr) {
+status = SSLGetProtocolVersionMax(ctx, max_protocol);
+if(status == noErr) {
+if(max_protocol = kTLSProtocol11)
+supported_protocols |= SP_PROT_TLS1_1_CLIENT;
+if(max_protocol = kTLSProtocol12)
+supported_protocols |= SP_PROT_TLS1_2_CLIENT;
+}
+SSLDisposeContext(ctx);
+}else {
+WARN(SSLNewContext failed\n);
+}
+}
#endif
return TRUE;
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
Hi Jacek, On Apr 10, 2013, at 4:24 AM, Jacek Caban wrote: On 3/28/13 8:31 PM, Ken Thomases wrote: Mac OS X 10.8 introduced support for TLS 1.1 and 1.2. Can someone with Mac OS X 10.8 test the attached patch for me, please. All I need is to verify that it compiles and when running dlls/secur32/tests/secur32_test.exe.so schannel, TLS 1.1 and TLS 1.2 are listed as supported protocol. Thanks for doing this. I don't have a 10.8 build system handy, myself, or I would have done it or would test what you've done. However, Apple's guidance on using weak linking says that you must explicitly compare against NULL. They don't quite say that testing the symbol as a standalone boolean expression won't work, but they do say that using negation (the ! operator) isn't sufficient. https://developer.apple.com/library/mac/documentation/developertools/conceptual/cross_development/Using/using.html#//apple_ref/doc/uid/20002000-SW4 I'm not sure what the rationale is, but I think there's a peculiarity of what the compiler thinks it knows versus what the linker actually knows. Basically, the compiler figures there's a symbol that will be resolved (or cause a link error), so the check can be optimized away somehow. I think. Other than that, the patch looks good to me. -Ken
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
Hi Juan, On 03/29/13 18:19, Juan Lang wrote: Hi Jacek, thanks for the detailed reply. On Fri, Mar 29, 2013 at 3:02 AM, Jacek Caban ja...@codeweavers.com mailto:ja...@codeweavers.com wrote: Each protocol has two kinds of enable/disable flags: enabled and disabled by default. Those have different default values for each protocol and there are registry setting allowing to change each of them. Only enabled protocols are used at all. This patch limits enabled protocols to those that we can really support. If an application asks schannel to use default set of protocols (which I'd expect them to do unless they have a good reason), schannel will use all enabled protocols that are not disabled by default. An alternative to default set of protocols is listing each allowed separately. This means that if protocol is enabled and disabled by default it won't be used unless application explicitly asks for it. SSL2 is such a protocol by default. Do you think we should do this differently? Yes, my suggestion here is to explicitly disable SSL2 support altogether. GnuTLS doesn't support it, and having behavior that differs between Linux and Mac can be kind of maddening. I can imagine something like, embedded browser doesn't work for game X, with lots of works for me reports and the occasional fails here too, only to discover that it works on Mac but not Linux. The additional cost of a difference in behavior doesn't seem worth it, especially when the protocol itself really should have died long ago. Most of the argument could be used against enabling TLS 1.1 and TLS 1.2, because it's not present on older Macs (nor enabled by default on Windows), so we'll have different behaviour. That's sadly something we have to live with. Anyway, I'm all for trying to avoid using SSL2, but I'd like to be a bit less extreme. How about this patch: http://source.winehq.org/patches/data/95298 With this patch, SSL2 will be completely disabled in default Wine config, but it will be still possible to enable by registries, if someone really needs it. Jacek
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
Hi Jacek, On Sat, Mar 30, 2013 at 8:36 AM, Jacek Caban ja...@codeweavers.com wrote: Most of the argument could be used against enabling TLS 1.1 and TLS 1.2, because it's not present on older Macs (nor enabled by default on Windows), so we'll have different behaviour. That's sadly something we have to live with. Anyway, I'm all for trying to avoid using SSL2, but I'd like to be a bit less extreme. How about this patch: http://source.winehq.org/patches/data/95298 With this patch, SSL2 will be completely disabled in default Wine config, but it will be still possible to enable by registries, if someone really needs it. Thanks, this approach looks good to me. --Juan
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
Hi Ken,
On 03/28/13 20:31, Ken Thomases wrote:
On Mar 28, 2013, at 6:05 AM, Jacek Caban wrote:
--- a/dlls/secur32/schannel_macosx.c
+++ b/dlls/secur32/schannel_macosx.c
@@ -630,6 +630,11 @@ static OSStatus schan_push_adapter(SSLConnectionRef
transport, const void *buff,
return ret;
}
+DWORD schan_imp_enabled_protocols(void)
+{
+/* NOTE: No support for TLS 1.1 and TLS 1.2 */
+return SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT |
SP_PROT_TLS1_0_CLIENT;
+}
Mac OS X 10.8 introduced support for TLS 1.1 and 1.2. You can test at build
time with:
#if MAC_OS_X_VERSION_MAX_ALLOWED = 1080
...
#else
...
#endif
If we want to support building on 10.8 for deployment to earlier versions,
we'd do something like:
#if MAC_OS_X_VERSION_MAX_ALLOWED = 1080
SSLProtocol maxProtocol;
if (SSLGetProtocolVersionMax != NULL
SSLGetProtocolVersionMax(context, maxProtocol) == noErr)
{
... compare maxProtocol against kTLSProtocol11 and
kTLSProtocol12 ...
}
...
#else
...
#endif
Thanks for the pointers, I've been meaning to explore it as follow-up.
My problem is that I'm still on 10.6 with Xcode 3.2. Would you mind
taking care of the patch?
The idea is that SSLGetProtocolVersionMax() would be weak linked, so we'd
check if it was actually available before calling it. Of course, the other
complication is that that function requires a context parameter, but we can
create one just for the query if we're interested in the framework
capabilities (as opposed to what's been configured for a particular context).
Yes, in this case we're only interested in framework capabilities. We
should determine protocols used for given context ourselves, based on
caller's requested protocol and confuration, and pass that to framework.
Setting up framework is not implemented yet, I have patches for that
that I want to test a bit more before sending.
Thanks,
Jacek
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
Hi Juan,
On 03/28/13 21:55, Juan Lang wrote:
On Thu, Mar 28, 2013 at 12:31 PM, Ken Thomases k...@codeweavers.com
mailto:k...@codeweavers.com wrote:
On Mar 28, 2013, at 6:05 AM, Jacek Caban wrote:
--- a/dlls/secur32/schannel_macosx.c
+++ b/dlls/secur32/schannel_macosx.c
@@ -630,6 +630,11 @@ static OSStatus
schan_push_adapter(SSLConnectionRef transport, const void *buff,
return ret;
}
+DWORD schan_imp_enabled_protocols(void)
+{
+/* NOTE: No support for TLS 1.1 and TLS 1.2 */
+return SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT |
SP_PROT_TLS1_0_CLIENT;
Do we really want to continue supporting SSL2? It's got a number of
vulnerabilities, and is disabled pretty much everywhere by now:
http://en.wikipedia.org/wiki/Transport_Layer_Security#SSL_2.0
I implemented it the way it's done in Windows. It's a bit
under-documented and contains usual MSDN mistakes, so let me explain
what I found when testing Windows (most of it is implemented by
http://source.winehq.org/git/wine.git/commitdiff/0f2e0365ea1f5c6baba4cfd9c0ff69defe66d7ea).
Each protocol has two kinds of enable/disable flags: enabled and
disabled by default. Those have different default values for each
protocol and there are registry setting allowing to change each of them.
Only enabled protocols are used at all. This patch limits enabled
protocols to those that we can really support. If an application asks
schannel to use default set of protocols (which I'd expect them to do
unless they have a good reason), schannel will use all enabled
protocols that are not disabled by default. An alternative to default
set of protocols is listing each allowed separately.
This means that if protocol is enabled and disabled by default it
won't be used unless application explicitly asks for it. SSL2 is such a
protocol by default. Do you think we should do this differently?
Thanks,
Jacek
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
Hi Jacek, thanks for the detailed reply. On Fri, Mar 29, 2013 at 3:02 AM, Jacek Caban ja...@codeweavers.com wrote: Each protocol has two kinds of enable/disable flags: enabled and disabled by default. Those have different default values for each protocol and there are registry setting allowing to change each of them. Only enabled protocols are used at all. This patch limits enabled protocols to those that we can really support. If an application asks schannel to use default set of protocols (which I'd expect them to do unless they have a good reason), schannel will use all enabled protocols that are not disabled by default. An alternative to default set of protocols is listing each allowed separately. This means that if protocol is enabled and disabled by default it won't be used unless application explicitly asks for it. SSL2 is such a protocol by default. Do you think we should do this differently? Yes, my suggestion here is to explicitly disable SSL2 support altogether. GnuTLS doesn't support it, and having behavior that differs between Linux and Mac can be kind of maddening. I can imagine something like, embedded browser doesn't work for game X, with lots of works for me reports and the occasional fails here too, only to discover that it works on Mac but not Linux. The additional cost of a difference in behavior doesn't seem worth it, especially when the protocol itself really should have died long ago. --Juan
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
On Mar 28, 2013, at 6:05 AM, Jacek Caban wrote:
--- a/dlls/secur32/schannel_macosx.c
+++ b/dlls/secur32/schannel_macosx.c
@@ -630,6 +630,11 @@ static OSStatus schan_push_adapter(SSLConnectionRef
transport, const void *buff,
return ret;
}
+DWORD schan_imp_enabled_protocols(void)
+{
+/* NOTE: No support for TLS 1.1 and TLS 1.2 */
+return SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT | SP_PROT_TLS1_0_CLIENT;
+}
Mac OS X 10.8 introduced support for TLS 1.1 and 1.2. You can test at build
time with:
#if MAC_OS_X_VERSION_MAX_ALLOWED = 1080
...
#else
...
#endif
If we want to support building on 10.8 for deployment to earlier versions, we'd
do something like:
#if MAC_OS_X_VERSION_MAX_ALLOWED = 1080
SSLProtocol maxProtocol;
if (SSLGetProtocolVersionMax != NULL
SSLGetProtocolVersionMax(context, maxProtocol) == noErr)
{
... compare maxProtocol against kTLSProtocol11 and
kTLSProtocol12 ...
}
...
#else
...
#endif
The idea is that SSLGetProtocolVersionMax() would be weak linked, so we'd check
if it was actually available before calling it. Of course, the other
complication is that that function requires a context parameter, but we can
create one just for the query if we're interested in the framework capabilities
(as opposed to what's been configured for a particular context).
-Ken
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
On Thu, Mar 28, 2013 at 12:31 PM, Ken Thomases k...@codeweavers.com wrote:
On Mar 28, 2013, at 6:05 AM, Jacek Caban wrote:
--- a/dlls/secur32/schannel_macosx.c
+++ b/dlls/secur32/schannel_macosx.c
@@ -630,6 +630,11 @@ static OSStatus schan_push_adapter(SSLConnectionRef
transport, const void *buff,
return ret;
}
+DWORD schan_imp_enabled_protocols(void)
+{
+/* NOTE: No support for TLS 1.1 and TLS 1.2 */
+return SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT |
SP_PROT_TLS1_0_CLIENT;
Do we really want to continue supporting SSL2? It's got a number of
vulnerabilities, and is disabled pretty much everywhere by now:
http://en.wikipedia.org/wiki/Transport_Layer_Security#SSL_2.0
--Juan
