Okhttp: Crash on Android 10 with underscore in subdomain

Created on 3 Mar 2020  路  10Comments  路  Source: square/okhttp

Crash on OkHttp 3.12.9 and 3.12.10 when a URL subdomain contains underscores. Reproducible on API 29 (Android 10) emulator but not API 28 (Android 9).

On OkHttp 3.12.8 it errors but does not crash. Similarly OkHttp 3.12.10 errors but does not crash on Android 9.

Reproduce by making a GET request to an HTTPS URL with underscore in subdomain.

I saw old issue #242 closed saying underscores aren't allowed in hostnames, which may be technically true, but they are sometimes seen in the wild. Examples:

Logcats:

OkHttp 3.12.10 on Android 10:

--------- beginning of crash
E/AndroidRuntime: FATAL EXCEPTION: OkHttp Dispatcher
Process: com.talklittle.testokhttpunderscoresubdomain, PID: 10321
java.lang.IllegalArgumentException: Invalid input to toASCII: example_underscore.s3.amazonaws.com
    at java.net.IDN.toASCII(IDN.java:115)
    at javax.net.ssl.SNIHostName.<init>(SNIHostName.java:99)
    at com.android.org.conscrypt.Platform.getSSLParameters(Platform.java:153)
    at com.android.org.conscrypt.ConscryptFileDescriptorSocket.getSSLParameters(ConscryptFileDescriptorSocket.java:1117)
    at okhttp3.internal.platform.Android10Platform.configureTlsExtensions(Android10Platform.java:40)
    at okhttp3.internal.connection.RealConnection.connectTls(RealConnection.java:315)
    at okhttp3.internal.connection.RealConnection.establishProtocol(RealConnection.java:284)
    at okhttp3.internal.connection.RealConnection.connect(RealConnection.java:169)
    at okhttp3.internal.connection.StreamAllocation.findConnection(StreamAllocation.java:258)
    at okhttp3.internal.connection.StreamAllocation.findHealthyConnection(StreamAllocation.java:135)
    at okhttp3.internal.connection.StreamAllocation.newStream(StreamAllocation.java:114)
    at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:42)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
    at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:93)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
    at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
    at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:127)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
    at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:257)
    at okhttp3.RealCall$AsyncCall.execute(RealCall.java:201)
    at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
    at java.lang.Thread.run(Thread.java:919)
 Caused by: The input does not conform to the STD 3 ASCII rules. line:  0. preContext:  . postContext: e_underscore

    at android.icu.impl.IDNA2003.convertToASCII(IDNA2003.java:219)
    at android.icu.impl.IDNA2003.convertIDNToASCII(IDNA2003.java:277)
    at android.icu.text.IDNA.convertIDNToASCII(IDNA.java:654)
    at java.net.IDN.toASCII(IDN.java:108)
    at javax.net.ssl.SNIHostName.<init>(SNIHostName.java:99)聽
    at com.android.org.conscrypt.Platform.getSSLParameters(Platform.java:153)聽
    at com.android.org.conscrypt.ConscryptFileDescriptorSocket.getSSLParameters(ConscryptFileDescriptorSocket.java:1117)聽
    at okhttp3.internal.platform.Android10Platform.configureTlsExtensions(Android10Platform.java:40)聽
    at okhttp3.internal.connection.RealConnection.connectTls(RealConnection.java:315)聽
    at okhttp3.internal.connection.RealConnection.establishProtocol(RealConnection.java:284)聽
    at okhttp3.internal.connection.RealConnection.connect(RealConnection.java:169)聽
    at okhttp3.internal.connection.StreamAllocation.findConnection(StreamAllocation.java:258)聽
    at okhttp3.internal.connection.StreamAllocation.findHealthyConnection(StreamAllocation.java:135)聽
    at okhttp3.internal.connection.StreamAllocation.newStream(StreamAllocation.java:114)聽
    at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:42)聽
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)聽
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)聽
    at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:93)聽
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)聽
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)聽
    at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)聽
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)聽
    at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:127)聽
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)聽
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)聽
    at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:257)聽
    at okhttp3.RealCall$AsyncCall.execute(RealCall.java:201)聽
    at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)聽
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)聽
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)聽
    at java.lang.Thread.run(Thread.java:919)

OkHttp 3.12.8 on Android 10, error but no crash:

javax.net.ssl.SSLHandshakeException: java.lang.IllegalArgumentException: Invalid input to toASCII: example_underscore.s3.amazonaws.com
    at com.android.org.conscrypt.ConscryptFileDescriptorSocket.startHandshake(ConscryptFileDescriptorSocket.java:231)
    at okhttp3.internal.connection.RealConnection.connectTls(RealConnection.java:320)
    at okhttp3.internal.connection.RealConnection.establishProtocol(RealConnection.java:284)
    at okhttp3.internal.connection.RealConnection.connect(RealConnection.java:169)
    at okhttp3.internal.connection.StreamAllocation.findConnection(StreamAllocation.java:258)
    at okhttp3.internal.connection.StreamAllocation.findHealthyConnection(StreamAllocation.java:135)
    at okhttp3.internal.connection.StreamAllocation.newStream(StreamAllocation.java:114)
    at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:42)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
    at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:93)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
    at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
    at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:127)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
    at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:257)
    at okhttp3.RealCall$AsyncCall.execute(RealCall.java:201)
    at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
    at java.lang.Thread.run(Thread.java:919)
Caused by: java.security.cert.CertificateException: java.lang.IllegalArgumentException: Invalid input to toASCII: example_underscore.s3.amazonaws.com
    at com.android.org.conscrypt.ConscryptFileDescriptorSocket.verifyCertificateChain(ConscryptFileDescriptorSocket.java:415)
    at com.android.org.conscrypt.NativeCrypto.SSL_do_handshake(Native Method)
    at com.android.org.conscrypt.NativeSsl.doHandshake(NativeSsl.java:387)
    at com.android.org.conscrypt.ConscryptFileDescriptorSocket.startHandshake(ConscryptFileDescriptorSocket.java:226)
        ... 23 more
Caused by: java.lang.IllegalArgumentException: Invalid input to toASCII: example_underscore.s3.amazonaws.com
    at java.net.IDN.toASCII(IDN.java:115)
    at javax.net.ssl.SNIHostName.<init>(SNIHostName.java:99)
    at com.android.org.conscrypt.Platform.getSSLParameters(Platform.java:153)
    at com.android.org.conscrypt.ConscryptFileDescriptorSocket.getSSLParameters(ConscryptFileDescriptorSocket.java:1117)
    at com.android.org.conscrypt.TrustManagerImpl.getTrustedChainForServer(TrustManagerImpl.java:352)
    at android.security.net.config.NetworkSecurityTrustManager.checkServerTrusted(NetworkSecurityTrustManager.java:94)
    at android.security.net.config.RootTrustManager.checkServerTrusted(RootTrustManager.java:89)
    at com.android.org.conscrypt.Platform.checkServerTrusted(Platform.java:224)
    at com.android.org.conscrypt.ConscryptFileDescriptorSocket.verifyCertificateChain(ConscryptFileDescriptorSocket.java:407)
        ... 26 more
Caused by: The input does not conform to the STD 3 ASCII rules. line:  0. preContext:  . postContext: e_underscore
    at android.icu.impl.IDNA2003.convertToASCII(IDNA2003.java:219)
    at android.icu.impl.IDNA2003.convertIDNToASCII(IDNA2003.java:277)
    at android.icu.text.IDNA.convertIDNToASCII(IDNA.java:654)
    at java.net.IDN.toASCII(IDN.java:108)
        ... 34 more
android bug
Source
picture talklittle

Most helpful comment

I'll raise an AOSP bug for this, but it might be a struggle to get it changed in time for Android 11 (that said, the AOSP bug will probably come back to me anyway, so maybe ;)

Background: on Android IDN.toASCII() is implemented using ICU's IDNA2003 classes which are (a) deprecated[*] and (b) adhere very strictly to the rules so getting changes made upstream will be hard. E.g. in the case of the root domain name . (which different IDN implementations treat differently anyway) it ended up being easier to special-case it in the IDN class.

Special casing underscores would be uglier and probably only work for ASCII domain names, e.g. something like: If IDNA rejects the name and it contains underscores, strip them and try again. If the second attempt succeeds and the returned string is identical, then we know the original string is already ASCII. I'm open to better ideas! It might be easier to push that logic into SNIHostname.

Personally I think accepting underscores is fine, that fight has been lost and we should be compatible with browsers, and at a cursory glance I don't see any CTS tests for IDN saying they should be rejected, so it should be doable.

Meantime, what Jesse said: okhttp should probably catch and rethrow.

[*] I did agitate to start moving towards IDN 2008 in Android 11, but it didn't get any traction.

picture prbprbprb on 3 Mar 2020
👍2

All 10 comments

I've reported upstream. I'm not sure this is something we really want to fix in OkHttp

https://github.com/square/okhttp/blob/okhttp_3.12.x/okhttp/src/main/java/okhttp3/internal/platform/Android10Platform.java#L34-L47

  @SuppressLint("NewApi")
  @IgnoreJRERequirement
  @Override public void configureTlsExtensions(
      SSLSocket sslSocket, String hostname, List<Protocol> protocols) {
    enableSessionTickets(sslSocket);

// Fails here
    SSLParameters sslParameters = sslSocket.getSSLParameters();

    // Enable ALPN.
    String[] protocolsArray = Platform.alpnProtocolNames(protocols).toArray(new String[0]);
    sslParameters.setApplicationProtocols(protocolsArray);

    sslSocket.setSSLParameters(sslParameters);
  }

We would catch a specific error due to a specific bug/strict handling in Conscrypt

yschimke picture yschimke on 3 Mar 2020

Prior to the fix for Android R blacklisting methods, we didn't read the SSL Parameters, we only set the value.

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

      setHostname.invokeOptionalWithoutCheckedException(sslSocket, hostname);
yschimke picture yschimke on 3 Mar 2020

If it turns out to be fixed in Android 11 (v30), then we could bump the version check to >= 30, and keep using the Gray listed methods on Android 10 (v29). Since this is a regression in that sense.

Or catch this specific exception on all 3 branches, with 3 different patches IIUC.

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

  public static @Nullable Platform buildIfSupported() {
    try {
      if (getSdkInt() >= 29) {
        Class<?> sslParametersClass =
            Class.forName("com.android.org.conscrypt.SSLParametersImpl");

        return new Android10Platform(sslParametersClass);
      }
    } catch (ClassNotFoundException ignored) {
    }

    return null; // Not an Android 10+ runtime.
  }
yschimke picture yschimke on 3 Mar 2020

Underscores in domains work in Firefox and Chrome; they should work in Android! We should ask AOSP to fix.

But if Conscrypt won't handshake these URLs anyway, we should catch the IllegalArgumentException and rethrow whichever SSLException subtype we were throwing previously. Throwing unchecked exceptions deep inside of OkHttp is bad news.

swankjesse picture swankjesse on 3 Mar 2020
👍2

I'll put up 3 PRs but it might take a couple of days. Free for someone else to pick it up in the meantime :)

yschimke picture yschimke on 3 Mar 2020

I'll raise an AOSP bug for this, but it might be a struggle to get it changed in time for Android 11 (that said, the AOSP bug will probably come back to me anyway, so maybe ;)

Background: on Android IDN.toASCII() is implemented using ICU's IDNA2003 classes which are (a) deprecated[*] and (b) adhere very strictly to the rules so getting changes made upstream will be hard. E.g. in the case of the root domain name . (which different IDN implementations treat differently anyway) it ended up being easier to special-case it in the IDN class.

Special casing underscores would be uglier and probably only work for ASCII domain names, e.g. something like: If IDNA rejects the name and it contains underscores, strip them and try again. If the second attempt succeeds and the returned string is identical, then we know the original string is already ASCII. I'm open to better ideas! It might be easier to push that logic into SNIHostname.

Personally I think accepting underscores is fine, that fight has been lost and we should be compatible with browsers, and at a cursory glance I don't see any CTS tests for IDN saying they should be rejected, so it should be doable.

Meantime, what Jesse said: okhttp should probably catch and rethrow.

[*] I did agitate to start moving towards IDN 2008 in Android 11, but it didn't get any traction.

prbprbprb picture prbprbprb on 3 Mar 2020
👍2

Android platform engineer here :)

Underscores in domains work in Firefox and Chrome; they should work in Android! We should ask AOSP to fix.

My understanding is that underscores aren't RFC compliant. I'm guessing you'd considered this in your statement, and think that AOSP should favor consistency with common browsers over standards compliance? Personally, I'm more hesitant to allow this than you seem to be; I'd at least not want to rush it, since the fact that Android doesn't support them is not new behaviour; is my understanding correct?

15characterlimi picture 15characterlimi on 3 Mar 2020

@15characterlimi As you know OkHttp strongly favours browser consistency.

But the focus of the 3 fixes is just to swallow an I|llegalArgumentException that crashes apps, and turn into a IOException that users already deal with. There is no additional work to try to make it work on Android 10 via graylisted methods (wouldn't work in Android 11), it's literally to stop crashing the app if someone posts content with _ in a hostname. It's basically a DOS against Apps in the Google Play store unless we fix it.

The Android or Conscrypt fixes are out of scope of this fix, debate with @swankjesse. For now, we've just reported upstream, your call how to deal with those.

yschimke picture yschimke on 3 Mar 2020

I can also swallow the exception completely, so we just drop back to HTTP/1.1. But otherwise probably still work?

yschimke picture yschimke on 3 Mar 2020

Fixed in 4.5.0-RC1.

swankjesse picture swankjesse on 19 Mar 2020
Was this page helpful?
0 / 5 - 0 ratings

Related issues

sargunv picture sargunv  路  3Comments
SandroMachado picture SandroMachado  路  3Comments
vanshg picture vanshg  路  3Comments
yschimke picture yschimke  路  3Comments
rfc2822 picture rfc2822  路  3Comments