square / okhttp

Square’s meticulous HTTP client for the JVM, Android, and GraalVM.
https://square.github.io/okhttp/
Apache License 2.0
45.75k stars 9.15k forks source link

Bump requirements to Android 5+, Java 8+ #4481

Closed swankjesse closed 5 years ago

swankjesse commented 5 years ago

For the release OkHttp 3.13 I’d like to change our requirements from this:

To this:

These minimums ensure our TLS stack supports both TLSv1.2 and ALPN. Requiring TLSv1.2 is important because we’re soon going to disable TLSv1.0 and TLSv1.1 by default!

I think we want to ship Java 8 bytecode and use lambas and method references. Downstream Android applications may need to enable desugaring in D8 when building APKs.

We should avoid hard requirements on Java 8 APIs (java.time). We will instead continue to offer them as convenience overloads. Java 8 APIs aren’t available until API 26 (released August 2017).

Tolriq commented 5 years ago

😢 I'm min API 16 with a relatively large user base on those devices as they repurpose their old devices as advanced remotes. (70K+)

90% of my user base do not need or use https isn't there a way to continue to support those? Everything is API 16 on Google side, it's a little too early for dropping.

As I also use Okio and Moshi a lot, I fear that at some point not updating OkHttp will force no more updating Okio and Moshi and this will be a real issue (Specially with the incoming coroutines support)

yschimke commented 5 years ago

Would we consider releasing 3.12.x with critical security fixes, or just abandon really old clients? FWIW I'm for this, I just think it's worth stating what the plan would be.

swankjesse commented 5 years ago

@yschimke Exactly. We’ll do long term fixes on the 3.12 branch, but new features will require newer devices.

Tolriq commented 5 years ago

@swankjesse what about Okio?

Currently OkHttp does not officially support Okio 2.0 but it works, since it's not official I suppose it can break at any point. How will Okio evolution be handled to not be problematic?

swankjesse commented 5 years ago

OkHttp works with Okio 2, but doesn't require it. If you find bugs, we’ll fix ’em.

As for a general policy on what platforms we’ll require, it’s difficult because there’s many competing trade-offs. We want to our libraries to be secure, small, correct, and fast. Usually in that order of preference!

Requiring Android 5 is about TLSv1.2. We might also be able to get it working on Android 4.4 since that release has some opt-in support for TLSv1.2. Needs investigation!

Tolriq commented 5 years ago

My concern are caused because for example Moshi 1.8 did require Okio 2.1 and did not work with 2.0.

From your writing, if couroutines are added in Okio 2.x and not 3.x I can assume that it should work with OkHttp 3.12.x and you'll make necessary fixes if necessary?

Dropping 70K users with a large part paid ones is not an easy thing to do, specially when I maintain an app since more than 7 years and have a close relationship with the users.

Android 4.4 would be nice it's 85% of those 70k and seems more acceptable, even if a solution without https support for 16+ would be nice too.

Or is there a way to have Okio linked for Moshi and other usage but tell OkHttp to not use it at runtime?

swankjesse commented 5 years ago

Moshi 1.8 required a new API, BufferedSource.peek(). We added that API to both Okio’s 1.x branch (1.16) and the 2.x branch (2.1). In that case we went out of our way to keep Okio 1.x working for users that weren’t ready for a Kotlin upgrade.

When OkHttp starts requiring Android 5+, you’ll have a choice:

And if you’re willing to suffer some complexity, you can have it both ways. Ship OkHttp 3.12.x to old devices, and later releases to newer ones.

Tolriq commented 5 years ago

I was maybe not clear, I do understand the implication about OkHttp and how to handle it.

The issue is about keeping OkHttp 3.12 preventing me to update the other things (Okio/Moshi) due to breaking changes that would be incompatible with OkHttp 3.12. If moshi 1.9 fix an issue for me or improve performance and require Okio 2.2 that does not work with OkHttp 3.12 then I will have an issue.

I already do ABI split, adding SDK split would start to be really hard to manage from version numbering point of view and would not solve this other libraries update issue.

If you can stay API 19+ I can plan and announce a 16-18 drop in the coming months, if you keep 21+ then I need to start a complete analysis about my internal Okio / Moshi usages to ensure to issues in the future with my planned refactoring and usages and eventually seek for alternatives.

swankjesse commented 5 years ago

All future releases of Okio 1.x and 2.x will work with OkHttp 3.12.x. We’re pretty strict about compatibility!

Tolriq commented 5 years ago

Ok thanks that was the main blocking point from my first comment :)

Now I'll just hope you'll find a way for 19+ to simplify my future life.

vibin commented 5 years ago

Can we be sure that TLS v1.2 is enabled on all devices on API 21, including Samsung? See https://github.com/square/okhttp/issues/2372#issuecomment-244807676.

yschimke commented 5 years ago

@vibin I think we have the most tolerant code in place here https://github.com/square/okhttp/blob/a74901501e37aa8dc2939a306e5013f9d1aebe1b/okhttp/src/main/java/okhttp3/internal/platform/AndroidPlatform.java#L332

So on API 21 it should try "TLSv1.2", then fallback to "TLS"

ojw28 commented 5 years ago

Dropping support for API level 19 seems quite aggressive, given 7.6% of devices are still using it (according to https://developer.android.com/about/dashboards/). If the goal is to disable something by default, is it in any way feasible to do that based on Build.SDK_INT >= 20 and still keep support back to API level 19?

calvarez-ov commented 5 years ago

About Java 8+, note that some java 8 apis are only available on Android starting from Android 7/Nougat.

swankjesse commented 5 years ago

@calvarez-ov we're using a Maven plugin called Animal Sniffer to warn us if we inadvertently use an API we shouldn't.

svenjacobs commented 5 years ago

Requiring Android 5 is about TLSv1.2. We might also be able to get it working on Android 4.4 since that release has some opt-in support for TLSv1.2. Needs investigation!

Finding a solution that supports Android 4.4 (API 19) would be greatly appreciated since - as mentioned before - 7.6% of users are still on this version. That are 152 million devices out of a total of 2 billion Android devices!

Regarding our app we still have a significant number of customers on Android 4.4.

noamtamim commented 5 years ago

@swankjesse Unless I'm missing something, with Google's SSL provider, old Android devices can support TLS 1.2. https://medium.com/tech-quizlet/working-with-tls-1-2-on-android-4-4-and-lower-f4f5205629a

This affects the system's HTTPURLConnection, but as long as the system's SSLSocketFactory is used, I think it should apply to OkHttp too.

noamtamim commented 5 years ago

@swankjesse please respond to my comment above. Does Google's solution only work for HTTPURLConnection?

yschimke commented 5 years ago

@noamtamim particularly with Google's SSL provider, and 3.12.1 we should try to use TLSv1.2 if supported

https://github.com/square/okhttp/blob/a74901501e37aa8dc2939a306e5013f9d1aebe1b/okhttp/src/main/java/okhttp3/internal/platform/AndroidPlatform.java#L332

noamtamim commented 5 years ago

@yschimke if Google's SSL provider solves the TLSv1.2 issue, why should the version requirement of okhttp be raised?

swankjesse commented 5 years ago

I’ve written a full explanation of why we’re making this change and what the next steps are for developers. It’ll go live with the release.

noamtamim commented 5 years ago

@swankjesse I understand your point; I'm just saying that when I run my app on an Android 4.2.2 device (Samsung Galaxy Ace 3) and install the SSL provider before trying to access the network, EventListener.secureConnectEnd() reports that the TLS version as TLSv1.2.

Lambdas etc are also supported on these old devices (by desugaring) - so unless you're using new Android APIs that did not exist in 4.2, I don't see a good reason not to support that version.

Android 2.3 is indeed a very old release (not supported by Google Play anymore), but sadly 4.2 is still alive.

I really hope that you delay this decision, preferably until Google Play stops supporting those versions.

swankjesse commented 5 years ago

The good reason is that the Google SSL provider isn't universally available. Sufficiently motivated developers will need UI to instruct their users to install or update it. I don't want to get tangled up in that.

oskouei commented 5 years ago

Instead of install Google SSL provider by users, we can use Conscrypt library SSL provider inside Okhttp itself.

import org.conscrypt.OpenSSLProvider;
…

if (Build.VERSION.SDK_INT < 21 && Security.getProvider("Okhttp") == null){
    Security.insertProviderAt(new OpenSSLProvider("Okhttp"), 1);
}
hqzxzwb commented 4 years ago

@swankjesse Is there any chance to allow using new OkHttp along with Conscrypt on KitKat devices or is there a reason for not doing so? For now OkHttp will crash under this case due to a fail fast piece of code in Platform.kt. Then I tried with a bit code changed, and it runs well on the KitKat emulator.

The code causing crash:

val isSupported: Boolean = when {
      !isAndroid -> false
      else -> {
        // Fail Fast
        check(
            Build.VERSION.SDK_INT >= 21) { "Expected Android API level 21+ but was ${Build.VERSION.SDK_INT}" }

        true
      }
    }

I changed the check call to a comparison, plus a minor change in the ConscryptPlatform preventing a NoClassDefFoundError, then the code seems to run well with Conscrypt. But I did not test it thoroughly.

swankjesse commented 4 years ago

@hqzxzwb please build your own OkHttp .jar and try it!

mikehardy commented 4 years ago

I'm sure everyone that needs to has probably gotten this working but in the general case for Android API16 and higher (a very generous minimum, at this point) TLS1.2 is not so hard, here is exactly how in an app with tons of users verifying it works: https://github.com/ankidroid/Anki-Android/pull/5658/commits/47feecb9ac2e215bc56f387d043ccf9b271c032d#diff-36861f300e525ff4bc27048485a2ae3dR211

But I'm mainly commenting to see if anyone has investigated all the fancy new de-sugaring in Android Studio 4.0 to see if that could allow me (with my thousands upon thousands of Android <5 users) to move forward on this dependency to the modern branches. I might try it myself but someone may have done so already

yschimke commented 4 years ago

@mikehardy OkHttp 3.12.12+ will already do most of what your Tls12SocketFactory does

https://github.com/square/okhttp/blob/okhttp_3.12.x/okhttp/src/main/java/okhttp3/internal/platform/AndroidPlatform.java#L445

Not sure about the rest of the code or desugaring.

mikehardy commented 4 years ago

How did I miss that? Thanks for the pointer @yschimke I will test that and likely be able to delete unnecessary code, a maintainer's favorite thing...