congress-android
congress-android copied to clipboard
konklone
Upgrade to OkHttp 3.12.2
Along with this package upgrade, it also requires a minimum Android SDK Version of 21 to get updated TLS security.
My changes for #762 are included as well.
This generally looks great. However, the minimum API version is a significant change, which impacts device compatibility. Current stats suggest this will cut out about 10% of active Android devices across the ecosystem: https://developer.android.com/about/dashboards I could look at the stats specific to this app inside my own publisher dashboard, but if anything I'd expect it to be more severe.
Could that part be removed from this PR, and the rest accepted? What's the rationale to move to 21 now?
That's the minimum API level for OkHttp. There's still some Heartbeat related SSL/TLS issues with lower Android API versions.
On Sun, Mar 17, 2019 at 5:03 PM Eric Mill [email protected] wrote:
This generally looks great. However, the minimum API version is a significant change, which impacts device compatibility. Current stats suggest this will cut out about 10% of active Android devices across the ecosystem: https://developer.android.com/about/dashboards I could look at the stats specific to this app inside my own publisher dashboard, but if anything I'd expect it to be more severe.
Could that part be removed from this PR, and the rest accepted? What's the rationale to move to 21 now?
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/konklone/congress-android/pull/763#issuecomment-473719530, or mute the thread https://github.com/notifications/unsubscribe-auth/AACYUhtSdfPmnWiN7xvf0APKgFqSQ2YLks5vXruUgaJpZM4b4Z5B .
Help me understand the impact here - is it required in order to enable TLS v1.2, or is it about mitigating weaknesses with arbitrary servers? The app has a defined set of services it can connect to, so it doesn't have to worry about connecting to known-weak services.
AWS, Microsoft, and Google cloud services run at TLS v1.2 by default. All major browsers will stop supporting anything before TLS v1.2 within the next year. If ProPublica or congressional servers providing the images (Picasso uses OkHttp) upgrade to servers using TLS v1.2 at a minimum, the app will stop functioning on devices without TLS v1.2. Depending on how the current versions of OkHttp in the application work, the app could stop working on newer devices as well.
On the downside, going to a Lollipop minimum will drop some supported devices on the new version of the app. That would exclude phones and device produced before November 2014 and some after. Although, with the SSL concerns around that time, most carriers and manufacturers upgraded even earlier devices.
On Mon, Mar 18, 2019 at 8:54 AM Eric Mill [email protected] wrote:
Help me understand the impact here - is it required in order to enable TLS v1.2, or is it about mitigating weaknesses with arbitrary servers? The app has a defined set of services it can connect to, so it doesn't have to worry about connecting to known-weak services.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/konklone/congress-android/pull/763#issuecomment-473919208, or mute the thread https://github.com/notifications/unsubscribe-auth/AACYUhON_PZDQuKhR4sVZv5JgLUD5fozks5vX5qigaJpZM4b4Z5B .
Reading the OkHttp blog post on this, it seems like what we want for now is the 3.12 stable branch for which they'll backport fixes for a while. I'm all about improving our TLS security, but I don't see any reason to drop support for these devices until servers do. The app will still be preferring TLS v1.2 over 1.0 by moving to a recent 3.x version.
Would you mind updating the PR to use 3.12, and to not bump the API level? The work you did in the PR to update the network classes and their use of the OkHTTP API is extremely helpful, and I'd love to merge that in to the app and move us to 3.x.
OK, changes made to use in OkHttp 3.12.2 and minimum API lowered back to 16.
@smpeters @konklone Bump, I believe this can be merged now. @smpeters Great work :)
I'm waiting for this to be merged so I can start a light refactoring of the code, I think everything is good right now
Yep, I deleted that repo. I'm going a different direction with my repo. Sitting for a year on a patch to merge is more than I really care to do.