From f5d669cd0fc8fc183c53e25e8414655eea4230a0 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sat, 7 Mar 2020 03:10:10 +0000 Subject: [PATCH] Allow for users who have disabled certificate checks in dev. (#5835) * Allow for users who have disabled certificate checks in dev. * Avoid repeated calls * typo * Lock is assumed at this point * Stay safe * rework * spotless * Fix * Handle one more case * Capture the exception * Add test * Comment --- okhttp/src/main/java/okhttp3/Handshake.kt | 15 ++++---- .../internal/connection/ExchangeFinder.kt | 7 ++-- .../internal/connection/RealConnection.kt | 11 ++++-- .../okhttp3/ConnectionCoalescingTest.java | 34 +++++++++++++++++++ 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/okhttp/src/main/java/okhttp3/Handshake.kt b/okhttp/src/main/java/okhttp3/Handshake.kt index 0dd80bdbb..4bc98acc1 100644 --- a/okhttp/src/main/java/okhttp3/Handshake.kt +++ b/okhttp/src/main/java/okhttp3/Handshake.kt @@ -48,8 +48,13 @@ class Handshake internal constructor( peerCertificatesFn: () -> List ) { /** Returns a possibly-empty list of certificates that identify the remote peer. */ - @get:JvmName("peerCertificates") val peerCertificates: List by lazy( - peerCertificatesFn) + @get:JvmName("peerCertificates") val peerCertificates: List by lazy { + try { + peerCertificatesFn() + } catch (spue: SSLPeerUnverifiedException) { + listOf() + } + } @JvmName("-deprecated_tlsVersion") @Deprecated( @@ -121,11 +126,7 @@ class Handshake internal constructor( } override fun toString(): String { - val peerCertificatesString = try { - peerCertificates.map { it.name }.toString() - } catch (_: SSLPeerUnverifiedException) { - "Failed: SSLPeerUnverifiedException" - } + val peerCertificatesString = peerCertificates.map { it.name }.toString() return "Handshake{" + "tlsVersion=$tlsVersion " + "cipherSuite=$cipherSuite " + diff --git a/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt b/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt index bf71ac2d6..1bd32de41 100644 --- a/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt +++ b/okhttp/src/main/java/okhttp3/internal/connection/ExchangeFinder.kt @@ -142,9 +142,10 @@ class ExchangeFinder( synchronized(connectionPool) { if (call.isCanceled()) throw IOException("Canceled") - releasedConnection = call.connection - toClose = if (call.connection != null && - (call.connection!!.noNewExchanges || !call.connection!!.supportsUrl(address.url))) { + val callConnection = call.connection // changes within this overall method + releasedConnection = callConnection + toClose = if (callConnection != null && (callConnection.noNewExchanges || + !callConnection.supportsUrl(address.url))) { call.releaseConnectionNoEvents() } else { null diff --git a/okhttp/src/main/java/okhttp3/internal/connection/RealConnection.kt b/okhttp/src/main/java/okhttp3/internal/connection/RealConnection.kt index 8ceb9ec9a..4dd219bbe 100644 --- a/okhttp/src/main/java/okhttp3/internal/connection/RealConnection.kt +++ b/okhttp/src/main/java/okhttp3/internal/connection/RealConnection.kt @@ -576,9 +576,14 @@ class RealConnection( } // We have a host mismatch. But if the certificate matches, we're still good. - return !noCoalescedConnections && - handshake != null && - OkHostnameVerifier.verify(url.host, handshake!!.peerCertificates[0] as X509Certificate) + return !noCoalescedConnections && handshake != null && certificateSupportHost(url, handshake!!) + } + + private fun certificateSupportHost(url: HttpUrl, handshake: Handshake): Boolean { + val peerCertificates = handshake.peerCertificates + + return peerCertificates.isNotEmpty() && OkHostnameVerifier.verify(url.host, + peerCertificates[0] as X509Certificate) } @Throws(SocketException::class) diff --git a/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java b/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java index e0bdfd523..288ebe766 100644 --- a/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java +++ b/okhttp/src/test/java/okhttp3/ConnectionCoalescingTest.java @@ -19,12 +19,14 @@ import java.io.IOException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Proxy; +import java.security.cert.X509Certificate; import java.util.Collections; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.X509TrustManager; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.testing.PlatformRule; @@ -432,6 +434,38 @@ public final class ConnectionCoalescingTest { assertThat(client.connectionPool().connectionCount()).isEqualTo(2); } + /** + * Won't coalesce if we can't clean certs e.g. a dev setup. + */ + @Test public void redirectWithDevSetup() throws Exception { + X509TrustManager TRUST_MANAGER = new X509TrustManager() { + @Override + public void checkClientTrusted(X509Certificate[] x509Certificates, String s) { + } + + @Override + public void checkServerTrusted(X509Certificate[] x509Certificates, String s) { + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return new X509Certificate[0]; + } + }; + + client = client.newBuilder().sslSocketFactory(client.sslSocketFactory(), TRUST_MANAGER).build(); + + server.enqueue(new MockResponse()); + server.enqueue(new MockResponse()); + + assert200Http2Response(execute(url), server.getHostName()); + + HttpUrl sanUrl = url.newBuilder().host("san.com").build(); + assert200Http2Response(execute(sanUrl), "san.com"); + + assertThat(client.connectionPool().connectionCount()).isEqualTo(2); + } + private Response execute(HttpUrl url) throws IOException { return client.newCall(new Request.Builder().url(url).build()).execute(); }