diff --git a/android-test/src/androidTest/java/okhttp/android/test/EchTest.kt b/android-test/src/androidTest/java/okhttp/android/test/EchTest.kt index 4b0a727f6de0..ae60bb358d9b 100644 --- a/android-test/src/androidTest/java/okhttp/android/test/EchTest.kt +++ b/android-test/src/androidTest/java/okhttp/android/test/EchTest.kt @@ -22,9 +22,7 @@ import app.cash.burst.Burst import assertk.assertThat import assertk.assertions.contains import assertk.assertions.doesNotContain -import assertk.assertions.isEqualTo import assertk.assertions.isFalse -import assertk.assertions.isTrue import okhttp3.HttpUrl.Companion.toHttpUrl import okhttp3.OkHttpClient import okhttp3.Request @@ -72,40 +70,60 @@ class EchTest( fun tlsEchDevUsesEch() { val body = client.get("https://tls-ech.dev/") + // Only the heading identifies the server we reached; every page links to all of the others. + assertThat(body).contains("

tls-ech.dev

") assertThat(body).contains("You are using ECH") assertThat(body).doesNotContain("not using ECH") } + /** Port 444, because port 443 is the plain tls-ech.dev server. */ @Test - fun staleEchConfigIsNotRetried() { - val rejection = client.echRejectionFrom("https://stale.tls-ech.dev/") + fun staleEchConfigIsRetried() { + val body = client.get("https://stale.tls-ech.dev:444/") - // TODO retry with these, then assert "You are using ECH" like tlsEchDevUsesEch. - assertThat(rejection.hasRetryConfigList()).isTrue() - assertThat(rejection.publicHostname).isEqualTo("public.tls-ech.dev") + assertThat(body).contains("

stale.tls-ech.dev

") + assertThat(body).contains("You are using ECH") + assertThat(body).doesNotContain("not using ECH") } + /** Port 445, because port 443 is the plain tls-ech.dev server. */ @Test - fun wrongPublicNameIsNotRetried() { - val rejection = client.echRejectionFrom("https://wrong.tls-ech.dev/") + fun differentPublicHostnameIsVerifiedBeforeRetry() { + // The outer certificate authenticates public.tls-ech.dev, + // so the retry config may be used if it matches. + // https://www.rfc-editor.org/rfc/rfc9849.html#section-6.1.6 + val verifiedHostnames = mutableListOf() + val hostnameVerifier = client.hostnameVerifier + val client = + client + .newBuilder() + .hostnameVerifier { hostname, session -> + verifiedHostnames += hostname + hostnameVerifier.verify(hostname, session) + } + .build() + + val body = client.get("https://wrong.tls-ech.dev:445/") - // TODO retry with these, then assert "You are using ECH" like tlsEchDevUsesEch. - assertThat(rejection.hasRetryConfigList()).isTrue() - assertThat(rejection.publicHostname).isEqualTo("public.tls-ech.dev") + assertThat(body).contains("

wrong.tls-ech.dev

") + assertThat(body).contains("You are using ECH") + assertThat(verifiedHostnames).contains("public.tls-ech.dev") } /** * TLS 1.2 cannot carry ECH. + * + * Port 446, because port 443 is the plain tls-ech.dev server. */ @Test fun tls12OffersNothingToRetryWith() { - assertThat(client.echRejectionFrom("https://tls12.tls-ech.dev/").hasRetryConfigList()).isFalse() + val rejection = client.echRejectionFrom("https://tls12.tls-ech.dev:446/") + + assertThat(rejection.hasRetryConfigList()).isFalse() } /** * Makes the call at [url] and returns the ECH rejection it fails with. - * - * TODO handle EchConfigMismatchException.retry_configs. */ private fun OkHttpClient.echRejectionFrom(url: String): EchConfigMismatchException { val body = diff --git a/okhttp-testing-support/src/main/kotlin/okhttp3/FakeDns.kt b/okhttp-testing-support/src/main/kotlin/okhttp3/FakeDns.kt index 78fff4e85481..0f7075ac9935 100644 --- a/okhttp-testing-support/src/main/kotlin/okhttp3/FakeDns.kt +++ b/okhttp-testing-support/src/main/kotlin/okhttp3/FakeDns.kt @@ -275,7 +275,7 @@ class FakeDns( } is ResourceRecord.IpAddress -> { - val ipAddressRecord = Dns.Record.IpAddress(request.hostname, resourceRecord.address) + val ipAddressRecord = Dns.Record.IpAddress(resourceRecord.name, resourceRecord.address) when (resourceRecord.address) { is Inet4Address -> ipv4Records += ipAddressRecord is Inet6Address -> ipv6Records += ipAddressRecord diff --git a/okhttp/src/androidMain/kotlin/okhttp3/internal/platform/Android10Platform.kt b/okhttp/src/androidMain/kotlin/okhttp3/internal/platform/Android10Platform.kt index e89e0260f59c..f7e7f7eb4e2b 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/internal/platform/Android10Platform.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/internal/platform/Android10Platform.kt @@ -17,17 +17,20 @@ package okhttp3.internal.platform import android.annotation.SuppressLint import android.content.Context +import android.net.ssl.EchConfigMismatchException import android.os.Build import android.os.StrictMode import android.security.NetworkSecurityPolicy import android.util.CloseGuard import android.util.Log import javax.net.ssl.SSLContext +import javax.net.ssl.SSLException import javax.net.ssl.SSLSocket import javax.net.ssl.SSLSocketFactory import javax.net.ssl.X509TrustManager import okhttp3.Protocol import okhttp3.internal.SuppressSignatureCheck +import okhttp3.internal.dns.EchRetryConfig import okhttp3.internal.platform.AndroidPlatform.Companion.Tag import okhttp3.internal.platform.android.Android10SocketAdapter import okhttp3.internal.platform.android.Android17SocketAdapter @@ -39,6 +42,7 @@ import okhttp3.internal.platform.android.DeferredSocketAdapter import okhttp3.internal.tls.CertificateChainCleaner import okhttp3.internal.tls.TrustRootIndex import okio.ByteString +import okio.ByteString.Companion.toByteString /** Android 10+ (API 29+). */ @SuppressSignatureCheck @@ -86,6 +90,24 @@ class Android10Platform : ?.configureTlsExtensions(sslSocket, hostname, protocols, echConfigList) } + @SuppressLint("NewApi") + internal override fun getEchRetryConfig(exception: SSLException): EchRetryConfig? { + if (Build.VERSION.SDK_INT < 37 || exception !is EchConfigMismatchException) return null + + // From https://cs.android.com/android/platform/superproject/+/android-latest-release:external/conscrypt/platform/src/main/java/org/conscrypt/Platform.java;bpv=0 + // we can get neither, publicHostname only, or both. Conscrypt only hands us an EchConfigList + // if it is non-empty and self-consistent; BoringSSL does the real validation (version checks + // and such) when we hand the list back to it. + return EchRetryConfig( + publicHostname = exception.publicHostname ?: return null, + // An absent retry config list is how a server securely disables ECH. + configList = + exception.retryConfigList + ?.toBytes() + ?.toByteString(), + ) + } + override fun getSelectedProtocol(sslSocket: SSLSocket): String? = // No TLS extensions if the socket class is custom. socketAdapters.find { it.matchesSocket(sslSocket) }?.getSelectedProtocol(sslSocket) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt index 5047053680d5..30f20ad174c1 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt @@ -25,6 +25,7 @@ import java.net.Socket as JavaNetSocket import java.net.UnknownServiceException import java.security.cert.X509Certificate import java.util.concurrent.TimeUnit +import javax.net.ssl.SSLException import javax.net.ssl.SSLPeerUnverifiedException import javax.net.ssl.SSLSocket import okhttp3.CertificatePinner @@ -38,6 +39,7 @@ import okhttp3.internal.closeQuietly import okhttp3.internal.concurrent.TaskRunner import okhttp3.internal.concurrent.withLock import okhttp3.internal.connection.RoutePlanner.ConnectResult +import okhttp3.internal.dns.EchRetryConfig import okhttp3.internal.http.ExchangeCodec import okhttp3.internal.http1.Http1ExchangeCodec import okhttp3.internal.platform.Platform @@ -73,6 +75,7 @@ class ConnectPlan internal constructor( private val tunnelRequest: Request?, internal val connectionSpecIndex: Int, internal val isTlsFallback: Boolean, + private val echRetryConfig: EchRetryConfig? = null, ) : RoutePlanner.Plan, ExchangeCodec.Carrier { /** True if this connect was canceled; typically because it lost a race. */ @@ -98,10 +101,12 @@ class ConnectPlan internal constructor( get() = protocol != null private fun copy( + route: Route = this.route, attempt: Int = this.attempt, tunnelRequest: Request? = this.tunnelRequest, connectionSpecIndex: Int = this.connectionSpecIndex, isTlsFallback: Boolean = this.isTlsFallback, + echRetryConfig: EchRetryConfig? = this.echRetryConfig, ): ConnectPlan = ConnectPlan( taskRunner = taskRunner, @@ -120,6 +125,7 @@ class ConnectPlan internal constructor( tunnelRequest = tunnelRequest, connectionSpecIndex = connectionSpecIndex, isTlsFallback = isTlsFallback, + echRetryConfig = echRetryConfig, ) override fun connectTcp(): ConnectResult { @@ -200,11 +206,13 @@ class ConnectPlan internal constructor( val tlsEquipPlan = planWithCurrentOrInitialConnectionSpec(connectionSpecs, sslSocket) val connectionSpec = connectionSpecs[tlsEquipPlan.connectionSpecIndex] - // Figure out the next connection spec in case we need a retry. - retryTlsConnection = tlsEquipPlan.nextConnectionSpec(connectionSpecs, sslSocket) - connectionSpec.apply(sslSocket, isFallback = tlsEquipPlan.isTlsFallback) - connectTls(sslSocket, connectionSpec) + try { + connectTls(sslSocket, connectionSpec) + } catch (e: SSLException) { + retryTlsConnection = tlsEquipPlan.nextConnectionSpec(connectionSpecs, sslSocket, e) + throw e + } call.eventListener.secureConnectEnd(call, handshake) } else { javaNetSocket = rawSocket @@ -239,10 +247,6 @@ class ConnectPlan internal constructor( call.eventListener.connectFailed(call, route.socketAddress, route.proxy, null, e) connectionPool.connectionListener.connectFailed(route, call, e) - if (!retryOnConnectionFailure || !retryTlsHandshake(e)) { - retryTlsConnection = null - } - return ConnectResult( plan = this, nextPlan = retryTlsConnection, @@ -479,7 +483,7 @@ class ConnectPlan internal constructor( sslSocket: SSLSocket, ): ConnectPlan { if (connectionSpecIndex != -1) return this - return nextConnectionSpec(connectionSpecs, sslSocket) + return nextCompatibleConnectionSpec(connectionSpecs, sslSocket) ?: throw UnknownServiceException( "Unable to find acceptable protocols." + " isFallback=$isTlsFallback," + @@ -489,12 +493,66 @@ class ConnectPlan internal constructor( } /** - * Returns a copy of this connection with the next connection spec to try, or null if no other - * compatible connection specs are available. + * Returns a copy of this connection that recovers from [sslException], or null if the failure + * should not be retried. */ internal fun nextConnectionSpec( connectionSpecs: List, sslSocket: SSLSocket, + sslException: SSLException, + ): ConnectPlan? { + if (!retryOnConnectionFailure) return null + + val offeredEchRetryConfig = Platform.get().getEchRetryConfig(sslException) + if (offeredEchRetryConfig != null) { + // TODO should we emit an event that we considered ech retry? + + // https://www.rfc-editor.org/rfc/rfc9849.html#section-6.1.6 + val retryable = + when (offeredEchRetryConfig.configList) { + // The server securely disabled ECH. Retry unless we already disabled ECH. + null -> echRetryConfig == null || echRetryConfig.configList != null + + // A retry config in response to a retry config signals a misconfigured server. + else -> echRetryConfig == null + } + if (!retryable) return null + + // Validate the publicHostname against the session certificate + // The session is protected by the outer client hello (e.g. cloudflare-ech.com) + // not the origin server + val hostnameVerifier = route.address.hostnameVerifier!! + if (!hostnameVerifier.verify(offeredEchRetryConfig.publicHostname, sslSocket.session)) { + return null + } + + return copy( + route = + Route( + address = route.address, + proxy = route.proxy, + socketAddress = route.socketAddress, + echConfigList = offeredEchRetryConfig.configList, + ), + // echRetryConfig.configList is possibly null to retry with ECH disabled + echRetryConfig = offeredEchRetryConfig, + ) + } + + // If this was already in response to an ech retry, we are done for this + // connection + if (echRetryConfig != null || !retryTlsHandshake(sslException)) return null + + return nextCompatibleConnectionSpec(connectionSpecs, sslSocket) + } + + /** + * Returns a copy of this connection with the next compatible connection spec, or null if none + * are available. + */ + private fun nextCompatibleConnectionSpec( + connectionSpecs: List, + sslSocket: SSLSocket, ): ConnectPlan? { for (i in connectionSpecIndex + 1 until connectionSpecs.size) { if (connectionSpecs[i].isCompatible(sslSocket)) { @@ -561,6 +619,7 @@ class ConnectPlan internal constructor( tunnelRequest = tunnelRequest, connectionSpecIndex = connectionSpecIndex, isTlsFallback = isTlsFallback, + echRetryConfig = echRetryConfig, ) fun closeQuietly() { diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RouteSelector.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RouteSelector.kt index 0bba12759fb4..e144932f78d6 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RouteSelector.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RouteSelector.kt @@ -181,10 +181,14 @@ class RouteSelector internal constructor( val routes = dnsLookup(proxy, socketHost, socketPort) + // If DNS advertises ECH for any route, don't permit a retry without ECH. + val echRoutes = routes.filter { it.echConfigList != null } + val routesToTry = echRoutes.ifEmpty { routes } + // Try each address for best behavior in mixed IPv4/IPv6 environments. return when { - fastFallback -> reorderForHappyEyeballs(routes) - else -> routes + fastFallback -> reorderForHappyEyeballs(routesToTry) + else -> routesToTry } } diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/EchRetryConfig.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/EchRetryConfig.kt new file mode 100644 index 000000000000..3e5ed626ba0f --- /dev/null +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/EchRetryConfig.kt @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2026 OkHttp Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package okhttp3.internal.dns + +import okio.ByteString + +/** + * ECH retry config. Sent by a server when the ECH configuration we offered has fallen out of sync + * with the one it accepts: its TTL expired, or the server rotated to a new configuration. (For + * example, Cloudflare publishes one configuration at a time and rotates it hourly, honoring the + * previous one for a further 4 hours. A configuration cached past that grace period earns a retry + * config.) + * + * If a new [configList] is present, the server securely replaced our ECH configuration, and it + * must only be used when [publicHostname] can be validated against the certificate from the + * SSLSession (the outer client hello). Authenticating the public name is what makes this safe: + * https://www.rfc-editor.org/rfc/rfc9849.html#section-6.1.7 + * + * A null [configList] means the server offered no usable retry configuration, which securely + * disables ECH. Retry without ECH. + * + * https://www.rfc-editor.org/rfc/rfc9849.html#section-6.1.6 + */ +internal data class EchRetryConfig( + /** The client-facing server's name from `ECHConfig.contents.public_name`. */ + val publicHostname: String, + /** updated ECH configList or null to retry without ECH */ + val configList: ByteString?, +) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/platform/Platform.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/platform/Platform.kt index 3633ab55801d..8f9eb1a45406 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/platform/Platform.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/platform/Platform.kt @@ -26,6 +26,7 @@ import java.util.logging.Logger import javax.net.ssl.ExtendedSSLSession import javax.net.ssl.SNIHostName import javax.net.ssl.SSLContext +import javax.net.ssl.SSLException import javax.net.ssl.SSLSocket import javax.net.ssl.SSLSocketFactory import javax.net.ssl.TrustManager @@ -34,6 +35,7 @@ import javax.net.ssl.X509TrustManager import okhttp3.Dns import okhttp3.OkHttpClient import okhttp3.Protocol +import okhttp3.internal.dns.EchRetryConfig import okhttp3.internal.publicsuffix.PublicSuffixDatabase import okhttp3.internal.readFieldOrNull import okhttp3.internal.tls.BasicCertificateChainCleaner @@ -122,6 +124,9 @@ open class Platform { ) { } + /** Returns the ECH retry configuration carried by [exception]. */ + internal open fun getEchRetryConfig(exception: SSLException): EchRetryConfig? = null + /** Called after the TLS handshake to release resources allocated by [configureTlsExtensions]. */ open fun afterHandshake(sslSocket: SSLSocket) { } diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/InterceptorOverridesTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/InterceptorOverridesTest.kt index b664b7f3547c..9b4b4399bb4c 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/InterceptorOverridesTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/InterceptorOverridesTest.kt @@ -39,7 +39,6 @@ import java.util.Locale.getDefault import java.util.concurrent.TimeUnit import javax.net.SocketFactory import javax.net.ssl.HostnameVerifier -import javax.net.ssl.SSLException import javax.net.ssl.SSLSocket import javax.net.ssl.SSLSocketFactory import javax.net.ssl.X509TrustManager @@ -242,23 +241,15 @@ class InterceptorOverridesTest { OverrideParam.RetryOnConnectionFailure -> { enableTls() - var first = true + + server.enqueue(MockResponse.Builder().failHandshake().build()) client = client .newBuilder() .connectionSpecs(listOf(ConnectionSpec.RESTRICTED_TLS, ConnectionSpec.MODERN_TLS)) - .eventListener( - object : EventListener() { - override fun secureConnectEnd( - call: Call, - handshake: Handshake?, - ) { - if (first) { - first = false - throw SSLException("") - } - } - }, + .sslSocketFactory( + FallbackTestClientSocketFactory(handshakeCertificates.sslSocketFactory()), + handshakeCertificates.trustManager, ).build() overrideBadImplementation( diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RetryConnectionTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RetryConnectionTest.kt index 8d75d19e9d6d..83e2cf6df7f7 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RetryConnectionTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RetryConnectionTest.kt @@ -17,20 +17,31 @@ package okhttp3.internal.connection import assertk.assertThat import assertk.assertions.containsExactlyInAnyOrder +import assertk.assertions.isEmpty +import assertk.assertions.isEqualTo import assertk.assertions.isFalse +import assertk.assertions.isNotEqualTo import assertk.assertions.isNotNull import assertk.assertions.isNull import assertk.assertions.isTrue import java.io.IOException import java.security.cert.CertificateException +import javax.net.ssl.SSLException import javax.net.ssl.SSLHandshakeException import javax.net.ssl.SSLSocket import okhttp3.ConnectionSpec +import okhttp3.FakeDns import okhttp3.OkHttpClientTestRule +import okhttp3.Route import okhttp3.TestValueFactory import okhttp3.TlsVersion +import okhttp3.internal.dns.EchRetryConfig +import okhttp3.internal.dns.ResourceRecord +import okhttp3.internal.platform.Platform import okhttp3.testing.PlatformRule import okhttp3.tls.internal.TlsUtil.localhost +import okio.ByteString +import okio.ByteString.Companion.encodeUtf8 import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension @@ -39,12 +50,35 @@ class RetryConnectionTest { private val factory = TestValueFactory() private val handshakeCertificates = localhost() private val retryableException = SSLHandshakeException("Simulated handshake exception") + private val echRetryException = SSLHandshakeException("ECH Mismatch with updated config") + private val echDisabledException = SSLHandshakeException("ECH Mismatch without config") + private val echRetryConfig = + EchRetryConfig( + configList = "retry config".encodeUtf8(), + publicHostname = "public.tls-ech.dev", + ) + private val echDisabledConfig = + EchRetryConfig( + configList = null, + publicHostname = "public.tls-ech.dev", + ) @RegisterExtension val clientTestRule = OkHttpClientTestRule() @RegisterExtension - val platform = PlatformRule() + val platform = + PlatformRule( + platform = + object : Platform() { + override fun getEchRetryConfig(exception: SSLException): EchRetryConfig? = + when { + exception === echRetryException -> echRetryConfig + exception === echDisabledException -> echDisabledConfig + else -> null + } + }, + ) private var client = clientTestRule.newClient() @@ -69,6 +103,158 @@ class RetryConnectionTest { assertThat(retryTlsHandshake(retryableException)).isTrue() } + @Test fun echRetryConfigIsUsedOnceWithoutTlsFallback() { + val address = newEchAddress() + val routePlanner = factory.newRoutePlanner(client, address) + val route = factory.newRoute(address) + val connectionSpecs = listOf(ConnectionSpec.MODERN_TLS, ConnectionSpec.COMPATIBLE_TLS) + val socket = createEchSocket() + val attempt0 = + routePlanner + .planConnectToRoute(route) + .planWithCurrentOrInitialConnectionSpec(connectionSpecs, socket) + + val attempt1 = attempt0.nextConnectionSpec(connectionSpecs, socket, echRetryException) + + assertThat(attempt1).isNotNull() + assertThat(attempt1!!.route.echConfigList).isEqualTo(echRetryConfig.configList) + assertThat(attempt1.isTlsFallback).isFalse() + assertThat(verifiedHostnames).isEqualTo(listOf(echRetryConfig.publicHostname)) + + verifiedHostnames.clear() + val attempt2 = attempt1.nextConnectionSpec(connectionSpecs, socket, retryableException) + assertThat(attempt2).isNull() + // An ordinary handshake failure doesn't verify a public hostname. + assertThat(verifiedHostnames).isEmpty() + socket.close() + } + + /** https://www.rfc-editor.org/rfc/rfc9849.html#section-6.1.6 */ + @Test fun echRetryUsesOnlyAddressesFromOriginalDnsResults() { + val dns = FakeDns() + val hostname = "stale.tls-ech.dev" + val originalAddresses = dns.allocate(2) + val newAddress = dns.allocate(1).single() + factory.dns = dns + factory.uriHost = hostname + dns[hostname] = + listOf( + ResourceRecord.Https( + name = hostname, + timeToLive = 5, + echConfigList = "stale config".encodeUtf8(), + ), + *originalAddresses + .map { + ResourceRecord.IpAddress( + name = hostname, + timeToLive = 5, + address = it, + ) + }.toTypedArray(), + ) + val address = newEchAddress() + val routePlanner = factory.newRoutePlanner(client, address) + val connectionSpecs = listOf(ConnectionSpec.MODERN_TLS) + val socket = createEchSocket() + val attempt0 = + routePlanner + .planConnect() + .planWithCurrentOrInitialConnectionSpec(connectionSpecs, socket) + + // A new DNS result must not influence a retry of the previous ECH configuration. + dns[hostname] = listOf(newAddress) + val attempt1 = attempt0.nextConnectionSpec(connectionSpecs, socket, echRetryException) + + assertThat(attempt1).isNotNull() + assertThat(attempt1!!.route.socketAddress.address).isEqualTo(originalAddresses[0]) + assertThat(attempt1.route.socketAddress.address).isNotEqualTo(newAddress) + assertThat(verifiedHostnames).isEqualTo(listOf(echRetryConfig.publicHostname)) + dns.assertRequests(hostname) + socket.close() + } + + @Test fun untrustedEchRetryConfigIsNotRetried() { + val address = newEchAddress(verified = false) + val routePlanner = factory.newRoutePlanner(client, address) + val route = factory.newRoute(address) + val connectionSpecs = listOf(ConnectionSpec.MODERN_TLS, ConnectionSpec.COMPATIBLE_TLS) + val socket = createEchSocket() + val attempt0 = + routePlanner + .planConnectToRoute(route) + .planWithCurrentOrInitialConnectionSpec(connectionSpecs, socket) + + // not retried because validation failed + val attempt1 = attempt0.nextConnectionSpec(connectionSpecs, socket, echRetryException) + + assertThat(attempt1).isNull() + assertThat(verifiedHostnames).isEqualTo(listOf(echRetryConfig.publicHostname)) + socket.close() + } + + /** + * A server that offers no retry config has securely disabled ECH, so we retry without it. + * + * https://www.rfc-editor.org/rfc/rfc9849.html#section-6.1.6 + */ + @Test fun missingEchRetryConfigIsRetriedWithout() { + val address = newEchAddress() + val routePlanner = factory.newRoutePlanner(client, address) + val route = factory.newRoute(address).withEchConfigList("stale config".encodeUtf8()) + val connectionSpecs = listOf(ConnectionSpec.MODERN_TLS, ConnectionSpec.COMPATIBLE_TLS) + val socket = createEchSocket() + val attempt0 = + routePlanner + .planConnectToRoute(route) + .planWithCurrentOrInitialConnectionSpec(connectionSpecs, socket) + + val attempt1 = attempt0.nextConnectionSpec(connectionSpecs, socket, echDisabledException) + + assertThat(attempt1).isNotNull() + assertThat(attempt1!!.route.echConfigList).isNull() + assertThat(attempt1.isTlsFallback).isFalse() + assertThat(verifiedHostnames).isEqualTo(listOf(echDisabledConfig.publicHostname)) + + // Having disabled ECH once, we don't do it again. + verifiedHostnames.clear() + val attempt2 = attempt1.nextConnectionSpec(connectionSpecs, socket, echDisabledException) + assertThat(attempt2).isNull() + assertThat(verifiedHostnames).isEmpty() + socket.close() + } + + /** + * A retry config in response to a retry config signals a misconfigured server, but the server may + * still securely disable ECH. + * + * https://www.rfc-editor.org/rfc/rfc9849.html#section-6.1.6 + */ + @Test fun echRetryConfigIsRetriedOnceOnly() { + val address = factory.newHttpsAddress(hostnameVerifier = { _, _ -> true }) + val routePlanner = factory.newRoutePlanner(client, address) + val route = factory.newRoute(address).withEchConfigList("stale config".encodeUtf8()) + val connectionSpecs = listOf(ConnectionSpec.MODERN_TLS, ConnectionSpec.COMPATIBLE_TLS) + val socket = createEchSocket() + val attempt0 = + routePlanner + .planConnectToRoute(route) + .planWithCurrentOrInitialConnectionSpec(connectionSpecs, socket) + + val attempt1 = attempt0.nextConnectionSpec(connectionSpecs, socket, echRetryException) + assertThat(attempt1).isNotNull() + assertThat(attempt1!!.route.echConfigList).isEqualTo(echRetryConfig.configList) + + // A second retry config is not honored. + assertThat(attempt1.nextConnectionSpec(connectionSpecs, socket, echRetryException)).isNull() + + // But securely disabling ECH is. + val attempt2 = attempt1.nextConnectionSpec(connectionSpecs, socket, echDisabledException) + assertThat(attempt2).isNotNull() + assertThat(attempt2!!.route.echConfigList).isNull() + socket.close() + } + @Test fun someFallbacksSupported() { val sslV3 = ConnectionSpec @@ -94,7 +280,7 @@ class RetryConnectionTest { assertThat(attempt0.isTlsFallback).isFalse() connectionSpecs[attempt0.connectionSpecIndex].apply(socket, attempt0.isTlsFallback) assertEnabledProtocols(socket, TlsVersion.TLS_1_2) - val attempt1 = attempt0.nextConnectionSpec(connectionSpecs, socket) + val attempt1 = attempt0.nextConnectionSpec(connectionSpecs, socket, retryableException) assertThat(attempt1).isNotNull() assertThat(attempt1!!.isTlsFallback).isTrue() socket.close() @@ -110,18 +296,44 @@ class RetryConnectionTest { assertEnabledProtocols(socket, TlsVersion.TLS_1_2, TlsVersion.TLS_1_1, TlsVersion.TLS_1_0) } - val attempt2 = attempt1.nextConnectionSpec(connectionSpecs, socket) + val attempt2 = attempt1.nextConnectionSpec(connectionSpecs, socket, retryableException) assertThat(attempt2).isNull() socket.close() // sslV3 is not used because SSLv3 is not enabled on the socket. } + /** Records each hostname the [newEchAddress] verifier was asked to verify. */ + private val verifiedHostnames = mutableListOf() + + private fun newEchAddress(verified: Boolean = true) = + factory.newHttpsAddress( + hostnameVerifier = { hostname, _ -> + verifiedHostnames += hostname + verified + }, + ) + + /** + * ECH is only carried by TLS 1.3, so every ECH attempt needs it enabled. + * + * https://www.rfc-editor.org/rfc/rfc9849.html#section-1 + */ + private fun createEchSocket(): SSLSocket = createSocketWithEnabledProtocols(TlsVersion.TLS_1_3, TlsVersion.TLS_1_2) + private fun createSocketWithEnabledProtocols(vararg tlsVersions: TlsVersion): SSLSocket = (handshakeCertificates.sslSocketFactory().createSocket() as SSLSocket).apply { enabledProtocols = javaNames(*tlsVersions) } + private fun Route.withEchConfigList(echConfigList: ByteString): Route = + Route( + address = address, + proxy = proxy, + socketAddress = socketAddress, + echConfigList = echConfigList, + ) + private fun assertEnabledProtocols( socket: SSLSocket, vararg required: TlsVersion, diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RouteSelectorTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RouteSelectorTest.kt index 10ef278e2319..38c4e52b30c5 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RouteSelectorTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RouteSelectorTest.kt @@ -18,6 +18,7 @@ package okhttp3.internal.connection import app.cash.burst.Burst import assertk.assertThat import assertk.assertions.containsExactly +import assertk.assertions.hasSize import assertk.assertions.isEqualTo import assertk.assertions.isFalse import assertk.assertions.isSameInstanceAs @@ -133,6 +134,43 @@ class RouteSelectorTest( dns.assertRequests(uriHost) } + /** https://www.rfc-editor.org/rfc/rfc9848.html#section-5.1 */ + @Test fun echAddressesDoNotFallBackToNonEch() { + assumeTrue(entryPoint == EntryPoint.NewCall) + + val echAddress = dns.allocate(1).single() + val nonEchAddress = dns.allocate(1).single() + dns[uriHost] = + listOf( + ResourceRecord.Https( + name = uriHost, + timeToLive = 5, + targetName = "ech.$uriHost", + echConfigList = echConfigList, + ), + ResourceRecord.IpAddress( + name = "ech.$uriHost", + timeToLive = 5, + address = echAddress, + ), + ResourceRecord.IpAddress( + name = "non-ech.$uriHost", + timeToLive = 5, + address = nonEchAddress, + ), + ) + val address = factory.newAddress() + val routeSelector = newRouteSelector(address) + + val selection = routeSelector.next() + + assertThat(selection.routes).hasSize(1) + assertRoute(selection.next(), address, Proxy.NO_PROXY, echAddress, uriPort, echConfigList) + assertThat(selection.hasNext()).isFalse() + assertThat(routeSelector.hasNext()).isFalse() + dns.assertRequests(uriHost) + } + @Test fun singleRouteReturnsFailedRoute() { val address = factory.newAddress() var routeSelector = newRouteSelector(address)