Uh oh!
There was an error while loading. Please reload this page.
Enable TLS 1.1 and TLS 1.2 on Android 4.1-4.4 - #9840
Conversation
gotev
commented
Sep 10, 2016
I've done some tests and the same issue also happens on some Samsung devices with API 21. Solved by applying the solution also for API 21 |
ghost
commented
Sep 10, 2016
By analyzing the blame information on this pull request, we identified @AndrewJack to be a potential reviewer. |
ghost
commented
Sep 10, 2016
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! |
fkoester
commented
Sep 11, 2016
@gotev That's strange, since API 20 it should not be necessary. But maybe Samsung tampered a lot with their Android implementation. So maybe we should then drop the patches API check? Currently it is: if (Build.VERSION.SDK_INT >= 16 && Build.VERSION.SDK_INT < 20) |
ghost
commented
Sep 11, 2016
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
gotev
commented
Sep 11, 2016
@fkoester that's true for stock Android roms. I thought that all the other vendors were aligned, but maybe I was wrong, since the same problem arises on a Samsung rom with API 21. To solve that I simply changed the if statement: if (Build.VERSION.SDK_INT >= 16 && Build.VERSION.SDK_INT < 22) |
facebook-github-bot
commented
Oct 11, 2016
It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @AndrewJack as a potential reviewer. Could you take a look please or cc someone with more context? |
duataud
commented
Oct 21, 2016
Please merge. Stripe api don't acept tls < 1.2 anymore |
alexter42
commented
Oct 21, 2016
Same issue Stripe do not accept tls < 1.2. Does anyone else have the same issue? |
AndrewJack
commented
Oct 22, 2016
cc @bestander or @mkonicek could you review this? |
There was a problem hiding this comment.
Use com.facebook.common.logging.FLog instead.
There was a problem hiding this comment.
Use Build.VERSION_CODES for api levels
Build.VERSION_CODES.JELLY_BEAN == 16Build.VERSION_CODES.KITKAT_WATCH == 20 - maybe use KITKAT (v19) with a <= for readability
There was a problem hiding this comment.
Probably not, I removed it.
bestander
commented
Oct 22, 2016
It looks reasonable to me considering that it is off by default.
|
Credits to Alex Gotev (@gotev) for the nice implementation.
fkoester
commented
Oct 22, 2016
I implemented the suggestions made by @AndrewJack and rebased the PR to latest master. |
bestander
commented
Oct 22, 2016
Andrew, shipit when ready |
lacker
commented
Oct 28, 2016
@facebook-github-bot shipit |
facebook-github-bot
commented
Oct 28, 2016
@lacker has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
I don't understand: Also, this comment #7192 (comment) does not seem to work for me. Maybe it is only for api 16? We want to support 4.4 (api 19) and up. |
This is a proposed patch for issue #7192.
Android 4.1-4.4 has support for TLS 1.1 and 1.2 but it is disabled by default. Because of the known security issues and more and more servers switching to TLS 1.2 only, it would be nice for react-native to enable this support.
I demonstrated a demo application which showcases the problem and can be used to test this patch. All sources and documentation for it can be found here:
https://github.com/bringnow/react-native-tls-test
Credits to Alex Gotev (@gotev) for the nice implementation.