diff --git a/okhttp-testing-support/src/main/kotlin/okhttp3/sockets/FakeNetworkEchRejectedException.kt b/okhttp-testing-support/src/main/kotlin/okhttp3/sockets/FakeNetworkEchRejectedException.kt index 2acd455369ac..54bc7324d9a1 100644 --- a/okhttp-testing-support/src/main/kotlin/okhttp3/sockets/FakeNetworkEchRejectedException.kt +++ b/okhttp-testing-support/src/main/kotlin/okhttp3/sockets/FakeNetworkEchRejectedException.kt @@ -29,4 +29,4 @@ import okio.ByteString class FakeNetworkEchRejectedException( val publicName: String, val nextEchConfigList: ByteString?, -) : SSLException("ECH rejected") +) : SSLException("Encrypted Client Hello (ECH) rejected") diff --git a/okhttp-testing-support/src/main/kotlin/okhttp3/sockets/FakeNetworkPlatform.kt b/okhttp-testing-support/src/main/kotlin/okhttp3/sockets/FakeNetworkPlatform.kt index 949422e1953b..713c54094f22 100644 --- a/okhttp-testing-support/src/main/kotlin/okhttp3/sockets/FakeNetworkPlatform.kt +++ b/okhttp-testing-support/src/main/kotlin/okhttp3/sockets/FakeNetworkPlatform.kt @@ -33,7 +33,7 @@ import javax.net.ssl.TrustManager import javax.net.ssl.X509KeyManager import javax.net.ssl.X509TrustManager import okhttp3.Protocol -import okhttp3.internal.dns.EchRetryConfig +import okhttp3.internal.dns.EchRetryPlan import okhttp3.internal.platform.Platform import okhttp3.tls.internal.TlsUtil.newKeyManager import okio.ByteString @@ -91,11 +91,11 @@ class FakeNetworkPlatform : Platform() { return sslContext.socketFactory } - override fun getEchRetryConfig(exception: SSLException): EchRetryConfig? = + override fun echRetryPlan(exception: SSLException): EchRetryPlan? = when (exception) { is FakeNetworkEchRejectedException -> { - EchRetryConfig( - publicHostname = exception.publicName, + EchRetryPlan.getOrNull( + publicName = exception.publicName, configList = exception.nextEchConfigList, ) } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/internal/platform/Android10Platform.kt b/okhttp/src/androidMain/kotlin/okhttp3/internal/platform/Android10Platform.kt index b563de08d0b4..10f63dc6dc41 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/internal/platform/Android10Platform.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/internal/platform/Android10Platform.kt @@ -30,7 +30,7 @@ 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.dns.EchRetryPlan import okhttp3.internal.platform.AndroidPlatform.Companion.Tag import okhttp3.internal.platform.android.Android10SocketAdapter import okhttp3.internal.platform.android.Android17SocketAdapter @@ -91,20 +91,13 @@ class Android10Platform : } @SuppressLint("NewApi") - override fun getEchRetryConfig(exception: SSLException): EchRetryConfig? { + override fun echRetryPlan(exception: SSLException): EchRetryPlan? { 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(), + val publicName = exception.publicHostname ?: return null + return EchRetryPlan.getOrNull( + publicName = publicName, + configList = exception.retryConfigList?.toBytes()?.toByteString(), ) } diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt index 30f20ad174c1..ed6f4f7957c7 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt @@ -13,12 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +@file:OptIn(OkHttpInternalApi::class) + package okhttp3.internal.connection import java.io.IOException import java.net.ConnectException import java.net.HttpURLConnection -import java.net.NoRouteToHostException import java.net.ProtocolException import java.net.Proxy import java.net.Socket as JavaNetSocket @@ -35,11 +36,12 @@ import okhttp3.Handshake.Companion.handshake import okhttp3.Protocol import okhttp3.Request import okhttp3.Route +import okhttp3.internal.OkHttpInternalApi 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.dns.EchRetryPlan import okhttp3.internal.http.ExchangeCodec import okhttp3.internal.http1.Http1ExchangeCodec import okhttp3.internal.platform.Platform @@ -75,7 +77,7 @@ class ConnectPlan internal constructor( private val tunnelRequest: Request?, internal val connectionSpecIndex: Int, internal val isTlsFallback: Boolean, - private val echRetryConfig: EchRetryConfig? = null, + private val echRetryPlan: EchRetryPlan? = null, ) : RoutePlanner.Plan, ExchangeCodec.Carrier { /** True if this connect was canceled; typically because it lost a race. */ @@ -106,7 +108,7 @@ class ConnectPlan internal constructor( tunnelRequest: Request? = this.tunnelRequest, connectionSpecIndex: Int = this.connectionSpecIndex, isTlsFallback: Boolean = this.isTlsFallback, - echRetryConfig: EchRetryConfig? = this.echRetryConfig, + echRetryPlan: EchRetryPlan? = this.echRetryPlan, ): ConnectPlan = ConnectPlan( taskRunner = taskRunner, @@ -125,7 +127,7 @@ class ConnectPlan internal constructor( tunnelRequest = tunnelRequest, connectionSpecIndex = connectionSpecIndex, isTlsFallback = isTlsFallback, - echRetryConfig = echRetryConfig, + echRetryPlan = echRetryPlan, ) override fun connectTcp(): ConnectResult { @@ -503,26 +505,15 @@ class ConnectPlan internal constructor( ): 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 + // If this was an ECH retry, don't retry again. + if (echRetryPlan != null) 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 nextEchRetryPlan = Platform.get().echRetryPlan(sslException) + if (nextEchRetryPlan != 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)) { + if (!hostnameVerifier.verify(nextEchRetryPlan.publicName, sslSocket.session)) { return null } @@ -532,16 +523,14 @@ class ConnectPlan internal constructor( address = route.address, proxy = route.proxy, socketAddress = route.socketAddress, - echConfigList = offeredEchRetryConfig.configList, + echConfigList = nextEchRetryPlan.configList, ), - // echRetryConfig.configList is possibly null to retry with ECH disabled - echRetryConfig = offeredEchRetryConfig, + echRetryPlan = nextEchRetryPlan, ) } - // If this was already in response to an ech retry, we are done for this - // connection - if (echRetryConfig != null || !retryTlsHandshake(sslException)) return null + // If the exception is not recoverable, don't retry. + if (!retryTlsHandshake(sslException)) return null return nextCompatibleConnectionSpec(connectionSpecs, sslSocket) } @@ -619,7 +608,7 @@ class ConnectPlan internal constructor( tunnelRequest = tunnelRequest, connectionSpecIndex = connectionSpecIndex, isTlsFallback = isTlsFallback, - echRetryConfig = echRetryConfig, + echRetryPlan = echRetryPlan, ) fun closeQuietly() { diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/EchRetryConfig.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/EchRetryConfig.kt deleted file mode 100644 index d1f6a4448553..000000000000 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/EchRetryConfig.kt +++ /dev/null @@ -1,44 +0,0 @@ -/* - * 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 okhttp3.internal.OkHttpInternalApi -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 - */ -@OkHttpInternalApi -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/dns/EchRetryPlan.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/EchRetryPlan.kt new file mode 100644 index 000000000000..2eff391d0b5a --- /dev/null +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/EchRetryPlan.kt @@ -0,0 +1,59 @@ +/* + * 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 okhttp3.internal.OkHttpInternalApi +import okhttp3.internal.canParseAsIpAddress +import okhttp3.internal.toCanonicalHost +import okio.ByteString + +/** + * A plan to retry after a failed Encrypted Client Hello TLS handshake. + * + * The [publicName] comes from the failed attempt's ECH config list. This is available locally by + * unpacking [okhttp3.Dns.Record.ServiceMetadata.echConfigList]. + * + * The [configList] comes from the server that couldn't successfully handshake with ECH. It will be + * null if the server has directed us to securely disable ECH. + * + * See RFC 9849, section 6.1.6. + */ +@OkHttpInternalApi +class EchRetryPlan private constructor( + /** The client-facing server's name from `ECHConfig.contents.public_name`. */ + val publicName: String, + /** The ECH config list to retry with, or null to retry without ECH. */ + val configList: ByteString?, +) { + companion object { + private val INVALID_PUBLIC_NAME = + "(\\..*)|(.*\\.)|((.*\\.)?[0-9]+)|((.*\\.)?0[xX][0-9a-fA-F]*)".toRegex() + + /** Returns a new config if the inputs are valid, and null otherwise. */ + fun getOrNull( + publicName: String, + configList: ByteString?, + ): EchRetryPlan? { + val canonicalHost = publicName.toCanonicalHost() ?: return null + if (canonicalHost.canParseAsIpAddress()) return null + if (INVALID_PUBLIC_NAME.matches(canonicalHost)) return null + return EchRetryPlan( + publicName, + configList, + ) + } + } +} diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/platform/Platform.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/platform/Platform.kt index dc66ebe274ab..65687e2ce2b9 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/platform/Platform.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/platform/Platform.kt @@ -38,7 +38,7 @@ import okhttp3.Dns import okhttp3.OkHttpClient import okhttp3.Protocol import okhttp3.internal.OkHttpInternalApi -import okhttp3.internal.dns.EchRetryConfig +import okhttp3.internal.dns.EchRetryPlan import okhttp3.internal.publicsuffix.PublicSuffixDatabase import okhttp3.internal.readFieldOrNull import okhttp3.internal.tls.BasicCertificateChainCleaner @@ -135,8 +135,8 @@ open class Platform { ) { } - /** Returns the ECH retry configuration carried by [exception]. */ - open fun getEchRetryConfig(exception: SSLException): EchRetryConfig? = null + /** Returns a plan to recover when a handshake that failed due to Encrypted Client Hello. */ + open fun echRetryPlan(exception: SSLException): EchRetryPlan? = null /** Called after the TLS handshake to release resources allocated by [configureTlsExtensions]. */ open fun afterHandshake(sslSocket: SSLSocket) { diff --git a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/EchRetryPlanTest.kt b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/EchRetryPlanTest.kt new file mode 100644 index 000000000000..6b4a86377789 --- /dev/null +++ b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/EchRetryPlanTest.kt @@ -0,0 +1,76 @@ +/* + * 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. + */ +@file:OptIn(OkHttpInternalApi::class) + +package okhttp3.internal.dns + +import assertk.assertThat +import assertk.assertions.isNotNull +import assertk.assertions.isNull +import kotlin.test.Test +import okhttp3.internal.OkHttpInternalApi + +/** + * Confirm we correctly validate the requirements of RFC 9849 public names. We're stricter with + * these than on regular DNS names, because we expect the DNS servers will reject those for us. + * + * https://www.rfc-editor.org/rfc/rfc9849.html#section-6.1.7 + */ +class EchRetryPlanTest { + @Test + fun `valid public name`() { + assertValid("ech.example.com") + assertValid("ECH.EXAMPLE.COM") + assertValid("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijk.example.com") + assertValid("0xG") + assertValid("0xG.example.com") + assertValid("0XG") + assertValid("0XG.example.com") + } + + @Test + fun `invalid public name`() { + assertInvalid("1:2::3:4") + assertInvalid("10.20.30.40") + assertInvalid("ech.example.com.") + assertInvalid(".ech.example.com") + assertInvalid("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijkl.example.com") + assertInvalid("ech.example.123") + assertInvalid("ech.example.0x123") + assertInvalid("123") + assertInvalid("0xab") + assertInvalid("0XAB") + assertInvalid("0X") + } + + private fun assertValid(publicName: String) { + assertThat( + EchRetryPlan.getOrNull( + publicName = publicName, + configList = null, + ), + ).isNotNull() + } + + private fun assertInvalid(publicName: String) { + assertThat( + EchRetryPlan.getOrNull( + publicName = publicName, + configList = null, + ), + ).isNull() + } +} diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/EchOnFakeNetworkTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/EchOnFakeNetworkTest.kt index 51474432ed39..4567bc5c32b9 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/EchOnFakeNetworkTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/EchOnFakeNetworkTest.kt @@ -16,10 +16,13 @@ package okhttp3 import assertk.assertThat +import assertk.assertions.hasMessage import assertk.assertions.isEqualTo import assertk.assertions.isNull import java.net.InetAddress import java.util.concurrent.LinkedBlockingQueue +import javax.net.ssl.SSLException +import kotlin.test.assertFailsWith import mockwebserver3.MockResponse import mockwebserver3.MockWebServer import okhttp3.HttpUrl.Companion.toHttpUrl @@ -48,9 +51,16 @@ class EchOnFakeNetworkTest { @RegisterExtension val clientTestRule = OkHttpClientTestRule() + private val certificateAuthority = + HeldCertificate + .Builder() + .certificateAuthority(0) + .build() + private val publicServerCertificate = HeldCertificate .Builder() + .signedBy(certificateAuthority) .addSubjectAlternativeName("public.ech.example.com") .build() private val publicServerCertificates = @@ -62,6 +72,7 @@ class EchOnFakeNetworkTest { private val privateServerCertificate = HeldCertificate .Builder() + .signedBy(certificateAuthority) .addSubjectAlternativeName("private.ech.example.com") .build() private val privateServerCertificates = @@ -70,11 +81,33 @@ class EchOnFakeNetworkTest { .heldCertificate(privateServerCertificate) .build() + private val untrustedServerCertificate = + HeldCertificate + .Builder() + .addSubjectAlternativeName("untrusted.ech.example.com") + .build() + private val untrustedServerCertificates = + HandshakeCertificates + .Builder() + .heldCertificate(untrustedServerCertificate) + .build() + + private val ipAddressServerCertificate = + HeldCertificate + .Builder() + .signedBy(certificateAuthority) + .addSubjectAlternativeName("10.20.30.40") + .build() + private val ipAddressServerCertificates = + HandshakeCertificates + .Builder() + .heldCertificate(ipAddressServerCertificate) + .build() + private val clientCertificates = HandshakeCertificates .Builder() - .addTrustedCertificate(privateServerCertificate.certificate) - .addTrustedCertificate(publicServerCertificate.certificate) + .addTrustedCertificate(certificateAuthority.certificate) .build() private val serverIpAddress = InetAddress.getByName("1:2::3:4") @@ -137,27 +170,7 @@ class EchOnFakeNetworkTest { } } - server.enqueue( - MockResponse - .Builder() - .body("abc") - .build(), - ) - - val request = - Request( - url = "https://private.ech.example.com/".toHttpUrl(), - ) - - val call = client.newCall(request) - - val response = call.execute() - assertThat(response.code).isEqualTo(200) - assertThat(response.body.string()).isEqualTo("abc") - - val recordedRequest = server.takeRequest() - assertThat(recordedRequest.method).isEqualTo("GET") - assertThat(recordedRequest.body).isNull() + executeHttpExchange() assertThat(events.take()) .isEqualTo("handshake hostname=private.ech.example.com echConfigList=$echConfigList") @@ -222,6 +235,258 @@ class EchOnFakeNetworkTest { } } + executeHttpExchange() + + assertThat(events.take()) + .isEqualTo("handshake hostname=private.ech.example.com echConfigList=$echConfigList") + assertThat(events.take()) + .isEqualTo("handshake hostname=private.ech.example.com echConfigList=null") + } + + @Test + fun `server updates ech config for retry`() { + val updatedEchConfigList = "new key to encrypt 'private.ech.example.com'".encodeUtf8() + platform.handshaker = + object : Handshaker { + val delegate = InsecureHandshaker() + var handshakeCount = 0 + + override fun handshake( + client: Handshaker.ClientInputs, + server: Handshaker.ServerInputs, + ): Handshaker.Result { + events.put("handshake hostname=${client.hostname} echConfigList=${client.echConfigList}") + + when (handshakeCount++) { + 0 -> { + val publicClient = + client.copy( + hostname = "public.ech.example.com", + ) + val publicServer = + server.copy( + keyManager = publicServerCertificates.keyManager, + ) + val publicNameHandshake = delegate.handshake(publicClient, publicServer) + return Handshaker.Result.Failure( + exception = + FakeNetworkEchRejectedException( + publicName = "public.ech.example.com", + nextEchConfigList = updatedEchConfigList, + ), + clientHandshake = publicNameHandshake.clientHandshake, + serverHandshake = publicNameHandshake.serverHandshake, + selectedProtocol = publicNameHandshake.selectedProtocol, + ) + } + + 1 -> { + return delegate.handshake(client, server) + } + + else -> { + error("unexpected handshake") + } + } + } + } + + executeHttpExchange() + + assertThat(events.take()) + .isEqualTo("handshake hostname=private.ech.example.com echConfigList=$echConfigList") + assertThat(events.take()) + .isEqualTo("handshake hostname=private.ech.example.com echConfigList=$updatedEchConfigList") + } + + @Test + fun `server rejected because public name is not verified`() { + platform.handshaker = + object : Handshaker { + val delegate = InsecureHandshaker() + var handshakeCount = 0 + + override fun handshake( + client: Handshaker.ClientInputs, + server: Handshaker.ServerInputs, + ): Handshaker.Result { + check(handshakeCount++ == 0) + events.put("handshake hostname=${client.hostname} echConfigList=${client.echConfigList}") + + val publicClient = + client.copy( + hostname = "public.ech.example.com", + ) + val publicServer = + server.copy( + keyManager = untrustedServerCertificates.keyManager, + ) + val publicNameHandshake = delegate.handshake(publicClient, publicServer) + return Handshaker.Result.Failure( + exception = + FakeNetworkEchRejectedException( + publicName = "public.ech.example.com", + nextEchConfigList = null, + ), + clientHandshake = publicNameHandshake.clientHandshake, + serverHandshake = publicNameHandshake.serverHandshake, + selectedProtocol = publicNameHandshake.selectedProtocol, + ) + } + } + + val e = failHttpExchange() + assertThat(e).hasMessage("Encrypted Client Hello (ECH) rejected") + + assertThat(events.take()) + .isEqualTo("handshake hostname=private.ech.example.com echConfigList=$echConfigList") + } + + @Test + fun `only two attempts are made`() { + val echConfigList2 = "key #2 to encrypt 'private.ech.example.com'".encodeUtf8() + val echConfigList3 = "key #3 to encrypt 'private.ech.example.com'".encodeUtf8() + platform.handshaker = + object : Handshaker { + val delegate = InsecureHandshaker() + var handshakeCount = 0 + + override fun handshake( + client: Handshaker.ClientInputs, + server: Handshaker.ServerInputs, + ): Handshaker.Result { + events.put("handshake hostname=${client.hostname} echConfigList=${client.echConfigList}") + + val publicClient = + client.copy( + hostname = "public.ech.example.com", + ) + val publicServer = + server.copy( + keyManager = publicServerCertificates.keyManager, + ) + val publicNameHandshake = delegate.handshake(publicClient, publicServer) + return Handshaker.Result.Failure( + exception = + FakeNetworkEchRejectedException( + publicName = "public.ech.example.com", + nextEchConfigList = + when (handshakeCount++) { + 0 -> echConfigList2 + 1 -> echConfigList3 + else -> error("unexpected handshake") + }, + ), + clientHandshake = publicNameHandshake.clientHandshake, + serverHandshake = publicNameHandshake.serverHandshake, + selectedProtocol = publicNameHandshake.selectedProtocol, + ) + } + } + + val e = failHttpExchange() + assertThat(e).hasMessage("Encrypted Client Hello (ECH) rejected") + + assertThat(events.take()) + .isEqualTo("handshake hostname=private.ech.example.com echConfigList=$echConfigList") + assertThat(events.take()) + .isEqualTo("handshake hostname=private.ech.example.com echConfigList=$echConfigList2") + } + + @Test + fun `only two attempts are made even if third attempt would disable ech`() { + val echConfigList2 = "key #2 to encrypt 'private.ech.example.com'".encodeUtf8() + platform.handshaker = + object : Handshaker { + val delegate = InsecureHandshaker() + var handshakeCount = 0 + + override fun handshake( + client: Handshaker.ClientInputs, + server: Handshaker.ServerInputs, + ): Handshaker.Result { + events.put("handshake hostname=${client.hostname} echConfigList=${client.echConfigList}") + + val publicClient = + client.copy( + hostname = "public.ech.example.com", + ) + val publicServer = + server.copy( + keyManager = publicServerCertificates.keyManager, + ) + val publicNameHandshake = delegate.handshake(publicClient, publicServer) + return Handshaker.Result.Failure( + exception = + FakeNetworkEchRejectedException( + publicName = "public.ech.example.com", + nextEchConfigList = + when (handshakeCount++) { + 0 -> echConfigList2 + 1 -> null + else -> error("unexpected handshake") + }, + ), + clientHandshake = publicNameHandshake.clientHandshake, + serverHandshake = publicNameHandshake.serverHandshake, + selectedProtocol = publicNameHandshake.selectedProtocol, + ) + } + } + + val e = failHttpExchange() + assertThat(e).hasMessage("Encrypted Client Hello (ECH) rejected") + + assertThat(events.take()) + .isEqualTo("handshake hostname=private.ech.example.com echConfigList=$echConfigList") + assertThat(events.take()) + .isEqualTo("handshake hostname=private.ech.example.com echConfigList=$echConfigList2") + } + + @Test + fun `no retry if public name is not a DNS hostname`() { + platform.handshaker = + object : Handshaker { + val delegate = InsecureHandshaker() + var handshakeCount = 0 + + override fun handshake( + client: Handshaker.ClientInputs, + server: Handshaker.ServerInputs, + ): Handshaker.Result { + check(handshakeCount++ == 0) + events.put("handshake hostname=${client.hostname} echConfigList=${client.echConfigList}") + + val publicClient = + client.copy( + hostname = "public.ech.example.com", + ) + val publicServer = + server.copy( + keyManager = ipAddressServerCertificates.keyManager, + ) + val publicNameHandshake = delegate.handshake(publicClient, publicServer) + return Handshaker.Result.Failure( + exception = + FakeNetworkEchRejectedException( + publicName = "10.20.30.40", + nextEchConfigList = null, + ), + clientHandshake = publicNameHandshake.clientHandshake, + serverHandshake = publicNameHandshake.serverHandshake, + selectedProtocol = publicNameHandshake.selectedProtocol, + ) + } + } + + val e = failHttpExchange() + assertThat(e).hasMessage("Encrypted Client Hello (ECH) rejected") + + assertThat(events.take()) + .isEqualTo("handshake hostname=private.ech.example.com echConfigList=$echConfigList") + } + + private fun executeHttpExchange() { server.enqueue( MockResponse .Builder() @@ -243,10 +508,25 @@ class EchOnFakeNetworkTest { val recordedRequest = server.takeRequest() assertThat(recordedRequest.method).isEqualTo("GET") assertThat(recordedRequest.body).isNull() + } - assertThat(events.take()) - .isEqualTo("handshake hostname=private.ech.example.com echConfigList=$echConfigList") - assertThat(events.take()) - .isEqualTo("handshake hostname=private.ech.example.com echConfigList=null") + private fun failHttpExchange(): SSLException { + server.enqueue( + MockResponse + .Builder() + .body("abc") + .build(), + ) + + val request = + Request( + url = "https://private.ech.example.com/".toHttpUrl(), + ) + + val call = client.newCall(request) + + return assertFailsWith { + call.execute() + } } } diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RetryConnectionTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RetryConnectionTest.kt index 83e2cf6df7f7..bb09f7eaa8db 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RetryConnectionTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/RetryConnectionTest.kt @@ -35,7 +35,7 @@ import okhttp3.OkHttpClientTestRule import okhttp3.Route import okhttp3.TestValueFactory import okhttp3.TlsVersion -import okhttp3.internal.dns.EchRetryConfig +import okhttp3.internal.dns.EchRetryPlan import okhttp3.internal.dns.ResourceRecord import okhttp3.internal.platform.Platform import okhttp3.testing.PlatformRule @@ -52,16 +52,16 @@ class RetryConnectionTest { 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( + private val echRetryPlan = + EchRetryPlan.getOrNull( + publicName = "public.tls-ech.dev", configList = "retry config".encodeUtf8(), - publicHostname = "public.tls-ech.dev", - ) + )!! private val echDisabledConfig = - EchRetryConfig( + EchRetryPlan.getOrNull( + publicName = "public.tls-ech.dev", configList = null, - publicHostname = "public.tls-ech.dev", - ) + )!! @RegisterExtension val clientTestRule = OkHttpClientTestRule() @@ -71,9 +71,9 @@ class RetryConnectionTest { PlatformRule( platform = object : Platform() { - override fun getEchRetryConfig(exception: SSLException): EchRetryConfig? = + override fun echRetryPlan(exception: SSLException): EchRetryPlan? = when { - exception === echRetryException -> echRetryConfig + exception === echRetryException -> echRetryPlan exception === echDisabledException -> echDisabledConfig else -> null } @@ -117,9 +117,9 @@ class RetryConnectionTest { val attempt1 = attempt0.nextConnectionSpec(connectionSpecs, socket, echRetryException) assertThat(attempt1).isNotNull() - assertThat(attempt1!!.route.echConfigList).isEqualTo(echRetryConfig.configList) + assertThat(attempt1!!.route.echConfigList).isEqualTo(echRetryPlan.configList) assertThat(attempt1.isTlsFallback).isFalse() - assertThat(verifiedHostnames).isEqualTo(listOf(echRetryConfig.publicHostname)) + assertThat(verifiedHostnames).isEqualTo(listOf(echRetryPlan.publicName)) verifiedHostnames.clear() val attempt2 = attempt1.nextConnectionSpec(connectionSpecs, socket, retryableException) @@ -169,7 +169,7 @@ class RetryConnectionTest { assertThat(attempt1).isNotNull() assertThat(attempt1!!.route.socketAddress.address).isEqualTo(originalAddresses[0]) assertThat(attempt1.route.socketAddress.address).isNotEqualTo(newAddress) - assertThat(verifiedHostnames).isEqualTo(listOf(echRetryConfig.publicHostname)) + assertThat(verifiedHostnames).isEqualTo(listOf(echRetryPlan.publicName)) dns.assertRequests(hostname) socket.close() } @@ -189,7 +189,7 @@ class RetryConnectionTest { val attempt1 = attempt0.nextConnectionSpec(connectionSpecs, socket, echRetryException) assertThat(attempt1).isNull() - assertThat(verifiedHostnames).isEqualTo(listOf(echRetryConfig.publicHostname)) + assertThat(verifiedHostnames).isEqualTo(listOf(echRetryPlan.publicName)) socket.close() } @@ -214,7 +214,7 @@ class RetryConnectionTest { assertThat(attempt1).isNotNull() assertThat(attempt1!!.route.echConfigList).isNull() assertThat(attempt1.isTlsFallback).isFalse() - assertThat(verifiedHostnames).isEqualTo(listOf(echDisabledConfig.publicHostname)) + assertThat(verifiedHostnames).isEqualTo(listOf(echDisabledConfig.publicName)) // Having disabled ECH once, we don't do it again. verifiedHostnames.clear() @@ -243,16 +243,11 @@ class RetryConnectionTest { val attempt1 = attempt0.nextConnectionSpec(connectionSpecs, socket, echRetryException) assertThat(attempt1).isNotNull() - assertThat(attempt1!!.route.echConfigList).isEqualTo(echRetryConfig.configList) + assertThat(attempt1!!.route.echConfigList).isEqualTo(echRetryPlan.configList) - // A second retry config is not honored. + // At most two attempts are made. 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() + assertThat(attempt1.nextConnectionSpec(connectionSpecs, socket, echDisabledException)).isNull() } @Test fun someFallbacksSupported() {