Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CB-12809 android #179

Closed
wants to merge 3 commits into from
Closed

Conversation

amovsesy
Copy link

@amovsesy amovsesy commented May 12, 2017

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

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

…und https://support.google.com/faqs/answer/6346016

Adding a check for the certificate that comes in when connecting to the server
…lay 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
@amovsesy
Copy link
Author

@stevengill Can you please take a look at this

@amovsesy amovsesy changed the title Fixing a security issue which is banned by google play that can be fo… CB-12809 android May 12, 2017
@cordova-qa
Copy link

Cordova CI Build has completed successfully.

Commit - Link
Dashboard - Link

318 tests run, 15 skipped, 0 failed.

@jcesarmobile
Copy link
Member

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

@amovsesy
Copy link
Author

@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
Copy link
Member

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
Copy link
Contributor

filmaj commented May 13, 2017

+1 to @jcesarmobile's proposed solution.

@kerrishotts
Copy link

+1 to deprecation as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants