Do not enable TLSv1 if it is not a supported protocol

[newyankeecodeshop]

Motivation

Our application runs in an Android 4.2 environment where TLSv1 has been disabled for security compliance. The recent code change in #14245 enables TLSv1 in all environments, without first checking to see if that version of the protocol is supported. This causes an exception:

FATAL EXCEPTION: OkHttp Dispatcher java.lang.IllegalArgumentException: protocol TLSv1 is not supported at org.apache.harmony.xnet.provider.jsse.NativeCrypto.checkEnabledProtocols(NativeCrypto.java:569) at org.apache.harmony.xnet.provider.jsse.OpenSSLSocketImpl.setEnabledProtocols(OpenSSLSocketImpl.java:782) at com.facebook.react.modules.network.TLSSocketFactory.enableTLSOnSocket(TLSSocketFactory.java:74) at com.facebook.react.modules.network.TLSSocketFactory.createSocket(TLSSocketFactory.java:49) <... remaining stack omitted for brevity...>

Test Plan

I rebuilt the app with a patched version of React Native 0.50, and it runs successfully.

Related PRs

#14245

Release Notes

[ANDROID] [BUGFIX] [Network] - Enable TLSv1 only if supported by the OS

[facebook-github-bot]

@newyankeecodeshop I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[newyankeecodeshop]

@shergin When you get a chance can you take a look at this? This is needed because of the change in #14245 . thanks!

[urbane]

@newyankeecodeshop I also have this issue, however I think you might want to consider using an intersection rather than manually specifying just one protocol to ensure it works with various configurations.

List<String> allowedProtocols = new ArrayList<>(Arrays.asList(new String[] {"TLSv1", "TLSv1.1", "TLSv1.2"}));
allowedProtocols.retainAll(new HashSet<>(Arrays.asList(((SSLSocket)socket).getSupportedProtocols())));
((SSLSocket)socket).setEnabledProtocols(allowedProtocols.toArray(new String[allowedProtocols.size()]));

[newyankeecodeshop]

@urbane Fair enough. Is it common that "TSLv1.1" and/or "TLSv1.2" is not supported also?

[urbane]

@newyankeecodeshop I agree that it probably isn't a common concern at this time, but the problem is that it would break if the device didn't support either. Given some of the vendors I've worked with, I wouldn't be terribly surprised if they randomly decided to enable or disable support.

An intersection is more expensive, but I'm not aware of another solution that would work in every environment.

[facebook-github-bot]

@newyankeecodeshop I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[karthikpala]

Hello @newyankeecodeshop Thanks very much for this PR. Are there any updates when this is going to be merged (or) is there any workaround we can use. Please let me know. Thanks in advance :)

[karthikpala]

Hey @newyankeecodeshop , Is this issue resolved for you?