[TIMOB-17891] Android: Support TLS versioning on XHR client
GitHub Issue | n/a |
---|---|
Type | Improvement |
Priority | High |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2015-03-09T22:03:55.000+0000 |
Affected Version/s | n/a |
Fix Version/s | Release 4.0.0 |
Components | Android |
Labels | qe-4.0.0 |
Reporter | Stephen Tramer |
Assignee | Ashraf Abu |
Created | 2014-10-23T19:36:16.000+0000 |
Updated | 2017-03-22T22:43:55.000+0000 |
Description
We need to support different TLS versions in our XHR. A property (
tlsVersion
) will be added to Ti.Network.HTTPClient
for Android. It is currently iOS-only. See https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/network/src/java/ti/modules/titanium/network/TiSocketFactory.java#L25 for where it is ultimately used.
http://developer.android.com/reference/javax/net/ssl/SSLContext.html#getInstance(java.lang.String)
While TLS 1.0 is not vulnerable to POODLE, we may wish to make the default TLS 1.1:
https://securityblog.redhat.com/2014/10/15/poodle-a-ssl3-vulnerability-cve-2014-3566/
I haven't had a customer bug me about this personally. Not sure on the support side where they're at with it.
I think we should have it for parity but I have not run into the same issue for Android as we had when this was a requirement for iOS. Is there a difference in how the two handle this by default? Just wondering why this wasn't a requirement when we implemented iOS. Also, what is our default TLS version currently?
Default is 1.0 for Android, 1.2 for iOS. What was the issue you mention?
I'm currently looking into this. For setting Default TLS 1.1 for Android, based on [^http://developer.android.com/reference/javax/net/ssl/SSLContext.html#getInstance(java.lang.String)], min API level for Android SDK is 16.
I think it would be okay that if you wanted to set the version higher than 1.0 you would need to up your Android SDK min version
Agree, but can we ensure a proper error message is displayed if they try an unsupported TLS version with an older Android SDK.
Okay! I'll look into it more and see the best way I can support this.
Right now, I'm able to change the TLS version based on the Android SDK version being used. Solution 1 (Alan's suggestion): Set default to TLS 1.1. If Android SDK below 16 is used, throw an error. Solution 2: Set default to TLS 1.1. If Android SDK below 16 is used, set default to TLS 1. Which is preferred?
Possibly Option 2 with a WARN statement [~rblalock] or [~bgrantges]?
I'd like our behavior to match iOS as much as possible. Note that iOS does not consider TLS 1.1 by default. This is from a blog post: http://www.appcelerator.com/blog/2012/11/the-titanium-sdk-and-certificate-validation/ "For security reasons, when iOS is unable to connect with the requested TLS version, it treats the attempt as a failure instead of retrying with an older version. Unfortunately, many servers do not support TLS 1.2, and while it would be ideal that these servers updated, we understand the need to fall back to TLS 1.0. In iOS Titanium, we try to connect with 1.2, and if it fails, fall back to TLS 1.0. For speed reasons, if this succeeds, we remember the server’s legacy status for the duration of the application session." Using tlsVersion * tlsVersion should be used for when you know the level of security your server has. * If you are uncertain of a server’s TLS support, leave the tlsVersion property blank. * If you know the server supports only TLS 1.0, you can set this value to TLS_VERSION_1_0 to avoid an unnecessary TLS 1.2 attempt. * If you know your server supports TLS 1.2, set the value to TLS_VERSION_1_2 for added security, as this denies any attempt to downgrade to 1.0. Summary: * All current supported versions of iOS support TLS 1.0, 1.1. and 1.2. * TLS 1.1/1.2 support came in Android 16 (4.1): http://developer.android.com/reference/javax/net/ssl/SSLSocket.html I instead suggest the following: * Add a TLS_VERSION_1_1 constant. I think we need to support 1.1 as an option for reasons like http://threatpost.com/federal-agencies-told-to-support-tls-1-2-by-2015/105906 (where 1.1 is still the ONLY supported protocol. You can't use 1.2, and you can't use 1.0) * Try 1.2 as default, then 1.1, then 1.0 unless one of the constants is set * Write out a INFO for Android if using API < 16 that indicates a less-secure version is being used assuming they have not specified a TLS version (since our current API minimum is 14). If user mandates TLS > 1.0 for Android via a constant such as TLS_VERSION_1_2, and then uses API < 16 , then I think we should fail the build.
Also note that with Titanium SDK release 3.4.2, users will at least be required to build with API level 21 for Lollipop support, even though they will still be able to target earlier versions of the OS (down to 2.3.X)
I think i am fine with the recommendation you provide [~ingo] - Much like Rick, i haven't had anyone asking for this specifically. I would lean towards parity across platform. Bert
[~ingo], [~rblalock], [~bgrantges] I'll do it as recommended by Ingo.
Here's the pull request. When running on Android API < 16 and TLSv1.1 or TLSv1.2 is set, it will give an error: [ERROR] : TiHttpClient: (TiHttpClient-1) [780,3196] Error creating SSLSocketFactory: SSLContext TLSv1.2 implementation not found When running on Android API <16 and TLS is not set, default is used and it will give an info: [INFO] : TiSocketFactory: (TiHttpClient-1) [1101,2636] TLSv1 protocol is being used. It is a less-secure version. All other cases will continue as normal. Test code for app.js as follows:
PR: https://github.com/appcelerator/titanium_mobile/pull/6321
Is also available.
master PR to modify doc: https://github.com/appcelerator/titanium_mobile/pull/6372
Verified the fix on below Test Environment Appc Studio : 4.0.0.201502111039 Ti SDK : 4.0.0.v20150213151526 Mac OSX : 10.10.1 Alloy : 1.5.1 CLI - 3.6.0-dev Code Processor: 1.1.1 Nexus 5 - Android 5.0 Android SDK API 21 setTlsVersion( tlsVersion ) method doesn't handle invalid values. With the below code, the Tls value is set to 400 which is not expected behavior.These should give error message to user. All the other cases are working as expected. Reopening the ticket.
TLS version set using set Method: 400 //getting this on console
If TLS value is not a recognized value, we should alert the user.
Noted. Will do.
PR: https://github.com/appcelerator/titanium_mobile/pull/6674 Error message will appear when setting unrecognized value. Submitting to [~hpham] for review.
Closing ticket as fixed.