apache / cordova-plugin-file-transfer

Apache Cordova File Transfer Plugin
https://cordova.apache.org/
Apache License 2.0
596 stars 888 forks source link

CB-12809 android #179

Closed amovsesy closed 7 years ago

amovsesy commented 7 years ago

Fixing a security issue which is banned by google play that can be found https://support.google.com/faqs/answer/6346016

Adding a check for the certificate that comes in when connecting to the server

Platforms affected

Android

What does this PR do?

Adds a check for the connection that gets created

What testing has been done on this change?

Ran the tests

Checklist

amovsesy commented 7 years ago

@stevengill Can you please take a look at this

cordova-qa commented 7 years ago

Cordova CI Build has completed successfully.

Commit - Link Dashboard - Link

318 tests run, 15 skipped, 0 failed.

jcesarmobile commented 7 years ago

Those methods are there to ignore the certificates if you pass trustAllHosts param set to true (default is false)

amovsesy commented 7 years ago

@jcesarmobile, I understand, but this is violating Google's play ToS and it clearly states that any new updates or apps using an unsafe implementation of TrustManager will be blocked. https://support.google.com/faqs/answer/6346016. Given that, any apps using this code would be in violation and could be blocked from the google store.

jcesarmobile commented 7 years ago

Yeah, so the solution should be to deprecate trustAllHosts, documenting it and then remove those methods, not to implement them with a safe implementation because that will make trustAllHosts to stop working

filmaj commented 7 years ago

+1 to @jcesarmobile's proposed solution.

kerrishotts commented 7 years ago

+1 to deprecation as well.