From 1131c7485a973232a1c9247d70c1d86da94f5bd4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 09:40:09 +0000 Subject: [PATCH 1/3] Build Conscrypt with ECH, and test it on the JVM and on Android MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `network:echTest` cannot pass on the JVM, and saying so is less useful than saying why. Three things are in the way and only one of them is OkHttp's: no published TLS stack a JVM can load will encrypt a client hello; `ConscryptPlatform.configureTlsExtensions` takes an `echConfigList` and ignores it; and Conscrypt's OpenJDK `Platform` throws away the retry configs a server offers when it rejects a stale one. `conscrypt/` closes the first. Conscrypt's `google3-export` branch has `Conscrypt.setEchConfigList(SSLSocket, byte[])`, which is in no release, so `build-conscrypt.sh` builds it against a pinned BoringSSL and the `conscrypt` workflow caches the jars as a release keyed by both shas. Suites download that rather than rebuilding: the build is fifteen minutes and its output changes only when `pinned.properties` does. `network:echConscryptTest` runs on it. `EchClientHelloTest` reads the client hello off a local socket that answers nothing and asserts the name is not in the bytes — no DNS, no server, no internet, so it is the one ECH result that is never about somebody else's uptime. `EchConscryptTest` is `EchTest`'s cases against the public servers, with the two things the JVM lacks supplied from outside OkHttp. The gap between it and `echTest` is then the second item above and nothing else. Neither claims OkHttp does ECH on the JVM: the suite makes the `setEchConfigList` call itself, from a socket factory, precisely because OkHttp doesn't. The third item has no counterpart at all, which is why the stale-config case doesn't cross over. `android-ech` gains `PublicEncryptedClientHelloTest`, the same public-server cases on the API 37 emulator, so the JVM and Android rows of the status page are the same assertions against the same servers with only the TLS stack differing. It runs first and does not fail the job, for the reason nothing in `network` gates; the fixture suite after it still does. All of `conscrypt/` should be deleted the day Conscrypt ships ECH. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CqiK79k9uoWsn2AzgXpHMA --- .github/workflows/android-ech.yml | 2 +- .github/workflows/conscrypt.yml | 124 +++++++++++ .github/workflows/network.yml | 21 +- README.md | 51 ++++- android-ech/run-ech-test.sh | 41 +++- .../ech/PublicEncryptedClientHelloTest.kt | 200 +++++++++++++++++ conscrypt/README.md | 86 ++++++++ conscrypt/RELEASE_NOTES.md | 16 ++ conscrypt/build-conscrypt.sh | 108 ++++++++++ conscrypt/fetch-conscrypt.sh | 70 ++++++ conscrypt/pinned.properties | 15 ++ conscrypt/release-tag.sh | 17 ++ network/build.gradle.kts | 61 +++++- .../okhttp/testbed/network/ConscryptEch.kt | 202 ++++++++++++++++++ .../testbed/network/EchClientHelloTest.kt | 170 +++++++++++++++ .../testbed/network/EchConscryptTest.kt | 155 ++++++++++++++ site/tools/collect_results.py | 10 +- site/topics/ech.html | 54 +++++ 18 files changed, 1386 insertions(+), 17 deletions(-) create mode 100644 .github/workflows/conscrypt.yml create mode 100644 android-ech/src/androidTest/kotlin/okhttp/testbed/android/ech/PublicEncryptedClientHelloTest.kt create mode 100644 conscrypt/README.md create mode 100644 conscrypt/RELEASE_NOTES.md create mode 100755 conscrypt/build-conscrypt.sh create mode 100755 conscrypt/fetch-conscrypt.sh create mode 100644 conscrypt/pinned.properties create mode 100755 conscrypt/release-tag.sh create mode 100644 network/src/test/kotlin/okhttp/testbed/network/ConscryptEch.kt create mode 100644 network/src/test/kotlin/okhttp/testbed/network/EchClientHelloTest.kt create mode 100644 network/src/test/kotlin/okhttp/testbed/network/EchConscryptTest.kt diff --git a/.github/workflows/android-ech.yml b/.github/workflows/android-ech.yml index 9b86759..e8c7915 100644 --- a/.github/workflows/android-ech.yml +++ b/.github/workflows/android-ech.yml @@ -110,6 +110,6 @@ jobs: name: android-ech-test-results-${{ inputs.okhttpVersion || 'pinned-snapshot' }} path: | android-ech/build/run-metadata.json - android-ech/build/outputs/androidTest-results/connected/**/*.xml + android-ech/build/outputs/androidTest-results/connected*/**/*.xml android-ech/build/reports/androidTests/connected/ retention-days: 30 diff --git a/.github/workflows/conscrypt.yml b/.github/workflows/conscrypt.yml new file mode 100644 index 0000000..ab10b14 --- /dev/null +++ b/.github/workflows/conscrypt.yml @@ -0,0 +1,124 @@ +name: conscrypt + +# Builds the Conscrypt the ECH suites need, and caches it as a release on this repository. +# +# Not on every commit, and not as part of any test workflow: the build takes around fifteen +# minutes, needs a C++ toolchain, a cross compiler and two source trees, and its output depends +# on nothing in this repository except `conscrypt/pinned.properties`. So it runs when that file +# changes, or on demand, and everything else downloads what it published. The release tag carries +# both pinned shas, which makes the cache self-invalidating: bump a pin and the tag no longer +# exists, so this workflow builds and publishes it, and the suites pick it up on their next run. +# +# See conscrypt/README.md for why this is being built from a branch at all. +on: + push: + branches: + - main + paths: + - 'conscrypt/**' + - '.github/workflows/conscrypt.yml' + pull_request: + paths: + - 'conscrypt/**' + - '.github/workflows/conscrypt.yml' + workflow_dispatch: + inputs: + force: + description: 'Rebuild and replace the release even if the tag already exists' + required: false + default: false + type: boolean + +permissions: + contents: write + +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: false + +jobs: + conscrypt: + name: conscrypt + runs-on: ubuntu-latest + timeout-minutes: 60 + + steps: + - name: Checkout + uses: actions/checkout@v7 + + # Conscrypt's own build wants a JDK to compile against and JNI headers to build against. + - name: Configure JDK + uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: 17 + + - name: Resolve the release tag + id: pin + run: | + echo "tag=$(conscrypt/release-tag.sh)" >> "$GITHUB_OUTPUT" + + # A release for these two shas is the whole point of the cache, so having one already is + # the good outcome, not a reason to do the work again. `force` is for the case where a + # published asset is wrong rather than missing. + - name: Check for an existing release + id: existing + env: + GH_TOKEN: ${{ github.token }} + run: | + if gh release view "${{ steps.pin.outputs.tag }}" --repo "$GITHUB_REPOSITORY" >/dev/null 2>&1; then + echo "exists=true" >> "$GITHUB_OUTPUT" + else + echo "exists=false" >> "$GITHUB_OUTPUT" + fi + + - name: Install the build toolchain + if: steps.existing.outputs.exists == 'false' || inputs.force + run: | + sudo apt-get -qq update + sudo apt-get -qq install -y --no-install-recommends \ + clang \ + cmake \ + g++-aarch64-linux-gnu \ + binutils-aarch64-linux-gnu \ + ninja-build + + - name: Build Conscrypt + if: steps.existing.outputs.exists == 'false' || inputs.force + run: conscrypt/build-conscrypt.sh + + # A pull request touching the pin gets the build checked and nothing published: a release + # is a fact about the repository, and a proposed pin isn't one yet. + - name: Publish the release + if: >- + github.event_name != 'pull_request' + && (steps.existing.outputs.exists == 'false' || inputs.force) + env: + GH_TOKEN: ${{ github.token }} + run: | + tag='${{ steps.pin.outputs.tag }}' + gh release delete "$tag" --repo "$GITHUB_REPOSITORY" --yes --cleanup-tag || true + gh release create "$tag" \ + --repo "$GITHUB_REPOSITORY" \ + --title "Conscrypt for ECH ($tag)" \ + --notes-file conscrypt/RELEASE_NOTES.md \ + conscrypt/build/dist/* + + # The suites consume this by tag, so a run that built something is worth proving can be + # fetched back — including on a pull request, where it exercises the fallback path. + - name: Upload the build + if: always() && (steps.existing.outputs.exists == 'false' || inputs.force) + uses: actions/upload-artifact@v4 + with: + name: conscrypt-${{ steps.pin.outputs.tag }} + path: conscrypt/build/dist/ + retention-days: 30 + if-no-files-found: warn + + - name: Report + run: | + if [ '${{ steps.existing.outputs.exists }}' = 'true' ] && [ '${{ inputs.force }}' != 'true' ]; then + echo "Release ${{ steps.pin.outputs.tag }} already exists; nothing to build." >> "$GITHUB_STEP_SUMMARY" + else + echo "Built ${{ steps.pin.outputs.tag }}." >> "$GITHUB_STEP_SUMMARY" + fi diff --git a/.github/workflows/network.yml b/.github/workflows/network.yml index 5e45de0..69a176e 100644 --- a/.github/workflows/network.yml +++ b/.github/workflows/network.yml @@ -58,6 +58,13 @@ jobs: - name: Setup Gradle uses: gradle/actions/setup-gradle@v6 + # Without this the Conscrypt suites are left out of the source set and a change that + # breaks them compiles clean. Failure is not fatal here: a pull request that bumps the + # pin has no release to fetch yet, and the conscrypt workflow is what builds that one. + - name: Fetch Conscrypt + continue-on-error: true + run: conscrypt/fetch-conscrypt.sh + - name: Compile Network Suites run: ./gradlew network:compileTestKotlin network:checkPublicApiOnly @@ -95,13 +102,21 @@ jobs: - name: Setup Gradle uses: gradle/actions/setup-gradle@v6 + # The ECH suites' other half: a Conscrypt that can encrypt a client hello, which is not + # published anywhere. Downloaded rather than built — the build is its own workflow, and + # this one runs daily. A missing release leaves echConscryptTest out of the build and + # the rest of the run unaffected, which is why this doesn't fail the job. + - name: Fetch Conscrypt + continue-on-error: true + run: conscrypt/fetch-conscrypt.sh + - name: Run Network Tests - # Neither task gates — both carry ignoreFailures, because everything here calls a + # No task here gates — all of them carry ignoreFailures, because everything here calls a # server someone else operates. --continue still earns its place: it covers the # failures that happen before a test runs, so one suite failing to compile or to - # resolve doesn't rob the other of a result. + # resolve doesn't rob the others of a result. run: > - ./gradlew network:networkTest network:echTest --continue + ./gradlew network:networkTest network:echTest network:echConscryptTest --continue ${{ matrix.okhttpVersion && format('-PokhttpVersion={0}', matrix.okhttpVersion) || '' }} # What the XML can't say: which OkHttp version 'pinned' actually resolved to, and diff --git a/README.md b/README.md index 79d45de..01fd044 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ Suites |---------------|-----------------------------|---------------------------------------------------------------------| | `containers` | Docker | SOCKS5 and HTTP proxies, TLS via MockServer, virtual threads (Loom) | | `network` | Outbound network | ALPN and SNI overrides, Let's Encrypt trust, ECH on the public servers | -| `android-ech` | Docker, an API 37 emulator | Encrypted Client Hello over DoH: accepted, retried, and declined | +| `android-ech` | Docker, an API 37 emulator | Encrypted Client Hello over DoH: accepted, retried, and declined, plus the public servers | The `network` suites call servers other people operate — Google, Cloudflare, Let's Encrypt, and the ECH test servers at `tls-ech.dev` and `defo.ie`. They came from OkHttp's @@ -89,10 +89,11 @@ Requires Docker and JDK 21+. ``` ./gradlew containers:test containers:loomTest ./gradlew network:networkTest network:echTest +./gradlew network:echConscryptTest # after conscrypt/fetch-conscrypt.sh; see ECH on the JVM ``` -`test` covers the gating suites and fails the build. `loomTest`, `networkTest` and `echTest` -all run with `ignoreFailures`, because what they report is not this repository being broken — +`test` covers the gating suites and fails the build. `loomTest`, `networkTest`, `echTest` and +`echConscryptTest` all run with `ignoreFailures`, because what they report is not this repository being broken — see [Suites that report rather than gate](#suites-that-report-rather-than-gate). `network` has no gating task at all: its `test` task is disabled, so those two are the only way to run it. @@ -187,14 +188,52 @@ A and AAAA records only, so there is no HTTPS record to carry an ECH config list at whatever you like with `-PokhttpVersion`, and drop `ech-okhttp` once a release ships the API. +`android-ech` also runs `PublicEncryptedClientHelloTest`, which is `EchTest`'s cases against +the same public servers the JVM suite calls — `tls-ech.dev`, `defo.ie`, `cloudflare-ech.com`. +It is there so the public-server results can be read across platforms: the JVM row of the +status page and the Android row are then the same assertions against the same servers, and +the only variable left between them is the TLS stack. It runs first, and its failures do not +fail the job, for the same reason nothing in `network` gates — those servers belong to other +people. The fixture suite that follows it does gate. + ECH is Android-only in OkHttp today: JVM platforms accept the config list and ignore it. The `network` suite is where that shows up, from the other direction — `EchTest` came from OkHttp's `android-test` with its assertions intact, so on the JVM the route assertions pass, the assertions about what the server saw fail, and the difference is recorded rather than fixed up. See [Suites that report rather than gate](#suites-that-report-rather-than-gate). -The two ECH suites are not duplicates: `android-ech` proves the client behaviour against a -fixture nobody else can change, and `network` is what notices when `tls-ech.dev` or `defo.ie` -does change. +The ECH suites are not duplicates: `android-ech` proves the client behaviour against a +fixture nobody else can change, `network` is what notices when `tls-ech.dev` or `defo.ie` +does change, and `echConscryptTest` below is what says *why* the JVM ones are red. + +ECH on the JVM +-------------- + +`network:echTest` cannot pass on the JVM, and it is worth being precise about what is +missing, because it is less than it looks. + +There is no published TLS stack a JVM can load that will encrypt a client hello. Conscrypt's +`google3-export` branch has one — `Conscrypt.setEchConfigList(SSLSocket, byte[])` is public +API there and in no release. `conscrypt/` builds that branch and caches the result as a +release on this repository, and `network:echConscryptTest` runs the ECH cases against it: + +``` +conscrypt/fetch-conscrypt.sh +./gradlew network:echConscryptTest -PokhttpVersion=5.5.0-SNAPSHOT +``` + +Two suites run under that task. `EchClientHelloTest` reads the bytes of the client hello +against a local socket that accepts a connection and says nothing — no DNS, no internet, no +server — and asserts that the name is not in them. `EchConscryptTest` is `EchTest`'s cases +against the public servers, with the two things the JVM lacks supplied from outside OkHttp: +this Conscrypt, and a network security policy saying ECH is allowed. When those pass and +`echTest` doesn't, the difference between them is one call OkHttp's `ConscryptPlatform` +doesn't make. It is not a claim that OkHttp does ECH on the JVM — the suite makes that call +itself, from a socket factory, precisely because OkHttp doesn't. + +The whole arrangement is temporary and `conscrypt/` should be deleted the day Conscrypt ships +ECH. [`conscrypt/README.md`](conscrypt/README.md) has the detail: what is missing where, why +the build is cached as a release rather than run per commit, and why the stale-config retry +case has no counterpart on the JVM at all. That suite is also the one place a version matters to compilation. `Route.echConfigList` and `DnsOverHttps.Builder.includeServiceMetadata` arrived after 5.4.0, so diff --git a/android-ech/run-ech-test.sh b/android-ech/run-ech-test.sh index 8d10740..e906566 100755 --- a/android-ech/run-ech-test.sh +++ b/android-ech/run-ech-test.sh @@ -92,9 +92,44 @@ adb reverse tcp:8053 "tcp:$doh_host_port" adb reverse tcp:443 "tcp:$target_host_port" adb reverse tcp:8443 "tcp:$target_host_port" -"$repository_root/gradlew" -p "$repository_root" :android-ech:connectedDebugAndroidTest \ - "${gradle_arguments[@]}" \ - -Pandroid.testInstrumentationRunnerArguments.class=okhttp.testbed.android.ech.EncryptedClientHelloTest \ +# One instrumentation run per suite, because the two report differently and Gradle writes both +# to the same place. `results_dir` is moved aside after each run so the workflow can upload them +# together; without that the second run would overwrite the first. +results_dir="$repository_root/android-ech/build/outputs/androidTest-results/connected" + +run_suite() { + local class="$1" + shift + local status=0 + + rm -rf "$results_dir" + # `|| status=$?` rather than a bare call: this runs under `set -e`, and a failing suite whose + # results were never moved aside is a failing suite nobody can read. + "$repository_root/gradlew" -p "$repository_root" :android-ech:connectedDebugAndroidTest \ + "${gradle_arguments[@]}" \ + -Pandroid.testInstrumentationRunnerArguments.class="okhttp.testbed.android.ech.$class" \ + "$@" || status=$? + + if [ -d "$results_dir" ]; then + rm -rf "$results_dir-$class" + mv "$results_dir" "$results_dir-$class" + fi + return $status +} + +# The public servers first, and not allowed to fail the run. tls-ech.dev, defo.ie and +# cloudflare-ech.com belong to other people; an outage there is not a result about OkHttp, and +# the JVM `network` suites treat the same servers the same way. The XML still records what +# happened, which is what the status page reads. +public_status=0 +run_suite PublicEncryptedClientHelloTest || public_status=$? +if [ "$public_status" -ne 0 ]; then + echo "PublicEncryptedClientHelloTest failed; recorded, not fatal." >&2 +fi + +# The fixture suite does gate: it runs against containers this repository starts, so a failure +# is about OkHttp or about this repository, and there is nobody else to blame for it. +run_suite EncryptedClientHelloTest \ -Pandroid.testInstrumentationRunnerArguments.ech=true \ -Pandroid.testInstrumentationRunnerArguments.dohPort=8053 \ -Pandroid.testInstrumentationRunnerArguments.caCertificate="$ca_certificate" diff --git a/android-ech/src/androidTest/kotlin/okhttp/testbed/android/ech/PublicEncryptedClientHelloTest.kt b/android-ech/src/androidTest/kotlin/okhttp/testbed/android/ech/PublicEncryptedClientHelloTest.kt new file mode 100644 index 0000000..16da8d2 --- /dev/null +++ b/android-ech/src/androidTest/kotlin/okhttp/testbed/android/ech/PublicEncryptedClientHelloTest.kt @@ -0,0 +1,200 @@ +/* + * Copyright (C) 2026 Block, Inc. + * + * 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 okhttp.testbed.android.ech + +import android.os.Build +import assertk.assertThat +import assertk.assertions.contains +import assertk.assertions.doesNotContain +import assertk.assertions.hasSize +import assertk.assertions.isEqualTo +import assertk.assertions.isNotNull +import assertk.assertions.isNull +import okhttp3.Call +import okhttp3.HttpUrl.Companion.toHttpUrl +import okhttp3.Interceptor +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.Response +import okhttp3.Route +import okhttp3.dnsoverhttps.DnsOverHttps +import org.junit.jupiter.api.Assumptions.assumeTrue +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test + +/** + * [okhttp.testbed.network.EchTest]'s cases, on the one platform where they can all pass. + * + * The JVM suite reaches the same servers through the JDK's TLS stack, which cannot encrypt a + * client hello: its route assertions pass and its assertions about what the server saw do not. + * That is a finding about the platform rather than about OkHttp, and it is only legible against + * a platform where the whole thing works. This is that platform — Android 16 QPR2, API 37, where + * `android.net.ssl.EchConfigList` is what OkHttp's Android platform hands the config list to. + * + * [EncryptedClientHelloTest] covers the same feature against containers on the host, which is a + * different job: it can arrange a stale config and a server that refuses to offer a new one, and + * it doesn't depend on anyone else's uptime. This one covers the servers real clients meet, and + * makes the public-server results comparable across the JVM and Android rows of the status page. + * + * Two cases from the JVM suite don't cross over in the other direction, and remain in + * OkHttp's own `android-test`: the `AndroidDns` variant, and the one covering a host excluded by + * `network_security_config.xml`. Both are Android platform behaviour, not OkHttp's. + */ +class PublicEncryptedClientHelloTest { + private lateinit var client: OkHttpClient + + @BeforeEach + fun setUp() { + assumeTrue(Build.VERSION.SDK_INT >= 37, "ECH requires Android API 37") + + val bootstrapClient = OkHttpClient() + + // DNS server is addressed by IP, so resolving the resolver doesn't need a resolver. + val dns = + DnsOverHttps + .Builder() + .client(bootstrapClient) + .url("https://1.1.1.1/dns-query".toHttpUrl()) + // HTTPS records, which is where the ECH config list arrives. + .includeServiceMetadata(true) + .build() + + client = + bootstrapClient + .newBuilder() + .addNetworkInterceptor(RouteTagger) + .dns(dns) + .build() + } + + @Test + fun cloudflareUsesEch() { + val call = client.newCall(Request("https://cloudflare-ech.com/cdn-cgi/trace".toHttpUrl())) + call.execute().use { response -> + assertThat(call.routeList.routes.single().echConfigList).isNotNull() + + val body = response.body.string() + assertThat(body).contains("sni=encrypted") + } + } + + @Test + fun echIsAcceptedOnTlsEchDev() { + val call = client.newCall(Request("https://tls-ech.dev/".toHttpUrl())) + call.execute().use { response -> + assertThat(call.routeList.routes.single().echConfigList).isNotNull() + + val body = response.body.string() + + // 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") + } + } + + @Test + fun echIsRetriedOnStaleTlsEchDev() { + val call = client.newCall(Request("https://stale.tls-ech.dev/".toHttpUrl())) + call.execute().use { response -> + val routes = call.routeList.routes + assertThat(routes).hasSize(2) + assertThat(routes[0].echConfigList).isNotNull() + assertThat(routes[1].echConfigList).isNotNull() + + val body = response.body.string() + assertThat(body).contains("

stale.tls-ech.dev

") + assertThat(body).contains("You are using ECH") + assertThat(body).doesNotContain("not using ECH") + } + } + + /** + * This page redirects to 'https://wrong.tls-ech.dev:445/', but nothing is listening on that port + * on that server. + */ + @Test + fun echIsAcceptedOnWrongTlsEchDev() { + val verifiedHostnames = mutableListOf() + val hostnameVerifier = client.hostnameVerifier + val client = + client + .newBuilder() + .hostnameVerifier { hostname, session -> + verifiedHostnames += hostname + hostnameVerifier.verify(hostname, session) + }.followRedirects(false) + .build() + + val call = client.newCall(Request("https://wrong.tls-ech.dev/".toHttpUrl())) + call.execute().use { response -> + assertThat(call.routeList.routes.single().echConfigList).isNotNull() + + assertThat(response.code).isEqualTo(302) + assertThat(response.headers["Location"]) + .isEqualTo("https://wrong.tls-ech.dev:445/") + assertThat(verifiedHostnames).contains("wrong.tls-ech.dev") + } + } + + /** TLS 1.2 cannot carry ECH. */ + @Test + fun tlsIsNotUsedOnTls12TlsEchDev() { + val call = client.newCall(Request("https://tls12.tls-ech.dev/".toHttpUrl())) + call.execute().use { response -> + val routes = call.routeList.routes + assertThat(routes).hasSize(2) + assertThat(routes[0].echConfigList).isNotNull() + assertThat(routes[1].echConfigList).isNull() + + val body = response.body.string() + assertThat(body).contains("

tls12.tls-ech.dev

") + assertThat(body).contains("You are not using ECH") + assertThat(body).doesNotContain("You are using ECH") + } + } + + @Test + fun echIsAcceptedOnDefoIe() { + val call = client.newCall(Request("https://defo.ie/ech-check.php".toHttpUrl())) + call.execute().use { response -> + assertThat(call.routeList.routes.single().echConfigList).isNotNull() + + val body = response.body.string() + assertThat(body).contains("SSL_ECH_STATUS: success") + } + } +} + +/** + * Collect Route information to confirm we sent an ECH config list to our TLS stack. Unlike the + * JVM suite, here the assertions about what the *server* saw are expected to hold too. + */ +private object RouteTagger : Interceptor { + override fun intercept(chain: Interceptor.Chain): Response { + val routeList = chain.call().routeList + routeList.routes += chain.connection()!!.route() + return chain.proceed(chain.request()) + } +} + +private val Call.routeList: RouteList + get() = tag(RouteList::class) { RouteList() } + +/** All of the routes used to retrieve an HTTP response. */ +private class RouteList { + val routes = mutableListOf() +} diff --git a/conscrypt/README.md b/conscrypt/README.md new file mode 100644 index 0000000..4f1c024 --- /dev/null +++ b/conscrypt/README.md @@ -0,0 +1,86 @@ +Conscrypt, for ECH +================== + +A build of Conscrypt's OpenJDK artifact from a branch, cached as a release on this repository. +It exists for one reason: **there is no published TLS stack a JVM can load that will encrypt a +client hello**, so `network:echTest` cannot pass on the JVM, and without something like this +there is no way to say whether that is OkHttp's problem or the platform's. + +This directory should be deleted the day Conscrypt ships ECH. + +What's missing, and where +------------------------- + +Encrypted Client Hello takes two halves. OkHttp supplies the first on every platform: it reads +the ECH config list out of the DNS `HTTPS` record and carries it on the [`Route`][route]. The TLS +stack supplies the second, and on the JVM three separate things are in the way. + +| What | Where | Status | +|------|-------|--------| +| A TLS stack that can encrypt a client hello | Conscrypt `google3-export` | Exists, unpublished — this directory | +| Handing the config list to that stack | OkHttp `ConscryptPlatform.configureTlsExtensions` | Takes an `echConfigList` and ignores it | +| Reading a server's retry config back | Conscrypt OpenJDK `Platform.wrapEchRejectedException` | Discards the retry configs and the public name | + +The first is why this directory exists. The second is the small piece of work +[lysine-dev/okhttp#9559][okhttp-pr] does, one call to `Conscrypt.setEchConfigList` alongside the +ALPN and session-ticket configuration that method already does — `Android10Platform` is the model. + +The third is a Conscrypt change rather than an OkHttp one, and it is why `network:echConscryptTest` +has no counterpart to `EchTest.echIsRetriedOnStaleTlsEchDev`. On Android, a rejected ECH config +arrives as an `EchConfigMismatchException` carrying the config the server offered instead, which +is what `Android10Platform.getEchRetryConfig` reads. On OpenJDK the same code path throws an +`EchRejectedException` with the retry configs dropped on the floor, and nothing public exposes +`SSL_get0_ech_retry_configs` — so a stale config can be detected and not recovered from, on any +JVM client, however OkHttp is changed. + +The ECH work in Conscrypt was proposed as [google/conscrypt#1406][conscrypt-pr], which is still +open; what landed on `google3-export` is Google's internal version of it, exported by Copybara. +The API this suite uses is `Conscrypt.setEchConfigList(SSLSocket, byte[])`. + +How it's built and cached +------------------------- + +The build takes about fifteen minutes, needs a C++ toolchain and two source trees, and its output +depends on nothing in this repository except `pinned.properties`. So it does not happen in a test +workflow. The `conscrypt` workflow builds it when that file changes and publishes the jars as a +release; the release tag carries both pinned shas, so bumping a pin invalidates the cache by +construction, and a run that finds its tag already published does nothing. + +| File | What it does | +|------|--------------| +| `pinned.properties` | The Conscrypt and BoringSSL commits. The cache key. | +| `build-conscrypt.sh` | Builds BoringSSL for x86-64 and aarch64, then `conscrypt-openjdk`, into `build/dist`. | +| `fetch-conscrypt.sh` | Downloads the release for the pinned shas into `build/dist`, checksums it. | +| `release-tag.sh` | The tag those two agree on. | + +To run the JVM ECH suite locally: + +``` +conscrypt/fetch-conscrypt.sh # or --build-if-missing, if no release exists yet +./gradlew network:echConscryptTest -PokhttpVersion=5.5.0-SNAPSHOT +``` + +`network/build.gradle.kts` picks up `build/dist/conscrypt-openjdk-*.jar` if it is there and leaves +`EchConscryptTest` out of the build if it isn't, so neither the fetch nor the build is on anyone's +critical path. + +Building it needs `cmake`, `ninja`, `clang`, a JDK and an aarch64 cross compiler. Conscrypt's own +`jar` task builds a native library per architecture the host can target and fails if any of them +won't link, which is the only reason aarch64 is built at all — nothing here uses it. + +What a pass means +----------------- + +`network:echConscryptTest` supplies the two things the JVM lacks — this Conscrypt, and a network +security policy saying ECH is allowed, which on Android comes from `network_security_config.xml` +and on the JVM defaults to a value Conscrypt reads as "no" — and leaves the rest to OkHttp. When +it passes and `network:echTest` doesn't, the difference between them is the second row of the +table above and nothing else. + +It is not a claim that OkHttp does ECH on the JVM. It cannot be: the suite makes the +`setEchConfigList` call itself, from a socket factory, because OkHttp doesn't. What it does say is +that everything else is in place, and that the remaining change is worth making. + +[route]: https://square.github.io/okhttp/5.x/okhttp/okhttp3/-route/ +[okhttp-pr]: https://github.com/lysine-dev/okhttp/pull/9559 +[conscrypt-pr]: https://github.com/google/conscrypt/pull/1406 diff --git a/conscrypt/RELEASE_NOTES.md b/conscrypt/RELEASE_NOTES.md new file mode 100644 index 0000000..fd2fc50 --- /dev/null +++ b/conscrypt/RELEASE_NOTES.md @@ -0,0 +1,16 @@ +A build of [Conscrypt][conscrypt]'s OpenJDK artifact from the `google3-export` branch, for the +testbed's Encrypted Client Hello suites. **Not a Conscrypt release.** It is published here only +because ECH is not in one yet. + +`Conscrypt.setEchConfigList(SSLSocket, byte[])` exists on that branch and in no published +Conscrypt artifact, so the JVM has no TLS stack that can encrypt a client hello. That is what +`network:echTest` reports, and what `network:echConscryptTest` uses this to isolate. + +The exact commits, of both Conscrypt and BoringSSL, are in the tag and in `build-info.json`. +Verify the jars against `SHA256SUMS`. Delete this release once Conscrypt ships ECH: at that point +`conscrypt/` should go and the suite should depend on the release instead. + +Built by [`conscrypt/build-conscrypt.sh`][script] in the `conscrypt` workflow. + +[conscrypt]: https://github.com/google/conscrypt +[script]: https://github.com/yschimke/okhttp-testbed/blob/main/conscrypt/build-conscrypt.sh diff --git a/conscrypt/build-conscrypt.sh b/conscrypt/build-conscrypt.sh new file mode 100755 index 0000000..d350f63 --- /dev/null +++ b/conscrypt/build-conscrypt.sh @@ -0,0 +1,108 @@ +#!/usr/bin/env bash + +# Builds Conscrypt's OpenJDK artifact from the pinned google3-export commit, against the +# pinned BoringSSL commit, and stages the result under conscrypt/build/dist. +# +# This is deliberately not part of the Gradle build. It takes around fifteen minutes, needs +# a C++ toolchain and a cross compiler, and its output changes only when `pinned.properties` +# changes — so it runs in its own workflow and its output is cached as a release. Suites get +# the jar from `fetch-conscrypt.sh`, which downloads that release. See README.md. +# +# Requires: git, cmake, ninja, clang, a JDK, and g++-aarch64-linux-gnu. On Ubuntu: +# sudo apt-get install -y cmake ninja-build clang g++-aarch64-linux-gnu binutils-aarch64-linux-gnu + +set -euo pipefail + +conscrypt_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +build_dir="$conscrypt_dir/build" +dist_dir="$build_dir/dist" + +# shellcheck disable=SC1091 +source "$conscrypt_dir/pinned.properties" + +: "${conscryptRef:?pinned.properties must set conscryptRef}" +: "${conscryptVersion:?pinned.properties must set conscryptVersion}" +: "${boringsslRef:?pinned.properties must set boringsslRef}" + +# One checkout per sha, so a bumped pin doesn't build on top of the previous tree and a +# rerun with an unchanged pin reuses what's already there. +boringssl_dir="$build_dir/boringssl-$boringsslRef" +conscrypt_src_dir="$build_dir/conscrypt-$conscryptRef" + +# Fetches exactly one commit. `git clone --depth 1` can only take a branch tip, and both of +# these are pinned to a sha that may be behind it. +checkout() { + local url="$1" ref="$2" target="$3" + + if [ -e "$target/.git" ]; then + echo "Reusing $target" + return + fi + + rm -rf "$target" + mkdir -p "$target" + git -C "$target" init --quiet + git -C "$target" remote add origin "$url" + git -C "$target" fetch --quiet --depth 1 origin "$ref" + git -C "$target" checkout --quiet FETCH_HEAD +} + +echo "==> BoringSSL $boringsslRef" +checkout https://github.com/google/boringssl.git "$boringsslRef" "$boringssl_dir" + +# Both architectures, because Conscrypt's `jar` task builds a native library for each and +# fails the build if either can't link. Only x86_64 is used by the suites today. +if [ ! -f "$boringssl_dir/build64/libssl.a" ]; then + echo "==> BoringSSL x86_64" + cmake -S "$boringssl_dir" -B "$boringssl_dir/build64" -GNinja \ + -DCMAKE_POSITION_INDEPENDENT_CODE=TRUE \ + -DCMAKE_BUILD_TYPE=Release \ + -DCMAKE_C_COMPILER=clang \ + -DCMAKE_CXX_COMPILER=clang++ + ninja -C "$boringssl_dir/build64" crypto ssl +fi + +if [ ! -f "$boringssl_dir/build.arm/libssl.a" ]; then + echo "==> BoringSSL aarch64" + cmake -S "$boringssl_dir" -B "$boringssl_dir/build.arm" -GNinja \ + -DCMAKE_SYSTEM_NAME=Linux \ + -DCMAKE_SYSTEM_PROCESSOR=aarch64 \ + -DCMAKE_C_COMPILER=aarch64-linux-gnu-gcc \ + -DCMAKE_CXX_COMPILER=aarch64-linux-gnu-g++ \ + -DCMAKE_POSITION_INDEPENDENT_CODE=TRUE \ + -DCMAKE_BUILD_TYPE=Release + ninja -C "$boringssl_dir/build.arm" crypto ssl +fi + +echo "==> Conscrypt $conscryptRef" +checkout https://github.com/google/conscrypt.git "$conscryptRef" "$conscrypt_src_dir" + +# `jar` covers every native jar the host can build, which on Linux is x86_64 and aarch64. +# The Android modules are only included when an SDK is visible, and this build has no use +# for them, so ANDROID_HOME is cleared to keep them out of the task graph. +( + cd "$conscrypt_src_dir" + unset ANDROID_HOME ANDROID_SDK_ROOT + BORINGSSL_HOME="$boringssl_dir" CC=clang CXX=clang++ \ + ./gradlew --no-daemon :conscrypt-openjdk:jar +) + +rm -rf "$dist_dir" +mkdir -p "$dist_dir" +cp "$conscrypt_src_dir/openjdk/build/libs/"conscrypt-openjdk-*.jar "$dist_dir/" + +# What the jars can't say: which commits they came from. `fetch-conscrypt.sh` writes the +# same file after a download, so a suite can report its provenance either way. +cat > "$dist_dir/build-info.json" < SHA256SUMS) + +echo "==> Staged in $dist_dir" +ls -l "$dist_dir" diff --git a/conscrypt/fetch-conscrypt.sh b/conscrypt/fetch-conscrypt.sh new file mode 100755 index 0000000..d0eaacb --- /dev/null +++ b/conscrypt/fetch-conscrypt.sh @@ -0,0 +1,70 @@ +#!/usr/bin/env bash + +# Puts the pinned Conscrypt build under conscrypt/build/dist, downloading it from this +# repository's releases rather than building it. +# +# The release tag is derived from `pinned.properties`, so this and `build-conscrypt.sh` +# always agree on what "the pinned build" means, and a bumped pin misses the cache rather +# than silently returning the old jar. `--build-if-missing` falls back to building; without +# it a missing release is an error, which is what a test workflow wants — a suite that +# quietly builds Conscrypt on every checkin is the thing this arrangement exists to avoid. +# +# Nothing here needs a token: the releases of a public repository are public. + +set -euo pipefail + +build_if_missing=false +if [ "${1:-}" = "--build-if-missing" ]; then + build_if_missing=true +elif [ $# -gt 0 ]; then + echo "usage: $0 [--build-if-missing]" >&2 + exit 2 +fi + +conscrypt_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +dist_dir="$conscrypt_dir/build/dist" + +# shellcheck disable=SC1091 +source "$conscrypt_dir/pinned.properties" + +repository="${GITHUB_REPOSITORY:-yschimke/okhttp-testbed}" +tag="$("$conscrypt_dir/release-tag.sh")" +base_url="https://github.com/$repository/releases/download/$tag" + +if [ -f "$dist_dir/build-info.json" ] && grep -q "\"$conscryptRef\"" "$dist_dir/build-info.json"; then + echo "Already have $tag in $dist_dir" + exit 0 +fi + +echo "==> Fetching $tag from $repository" +staging="$(mktemp -d)" +trap 'rm -rf "$staging"' EXIT + +if ! curl -fsSL --retry 3 --retry-delay 2 "$base_url/SHA256SUMS" -o "$staging/SHA256SUMS"; then + echo "No release $tag on $repository." >&2 + if [ "$build_if_missing" = true ]; then + echo "Building it instead." >&2 + exec "$conscrypt_dir/build-conscrypt.sh" + fi + echo "Run conscrypt/build-conscrypt.sh, or the conscrypt workflow, to publish it." >&2 + exit 1 +fi + +# The names come from the checksum file rather than being guessed, so adding an +# architecture to the build doesn't need a matching change here. +while read -r _ name; do + name="${name#./}" + curl -fsSL --retry 3 --retry-delay 2 "$base_url/$name" -o "$staging/$name" +done < "$staging/SHA256SUMS" + +curl -fsSL --retry 3 --retry-delay 2 "$base_url/build-info.json" -o "$staging/build-info.json" + +(cd "$staging" && sha256sum --check --quiet SHA256SUMS) + +rm -rf "$dist_dir" +mkdir -p "$(dirname "$dist_dir")" +mv "$staging" "$dist_dir" +trap - EXIT + +echo "==> Fetched into $dist_dir" +ls -l "$dist_dir" diff --git a/conscrypt/pinned.properties b/conscrypt/pinned.properties new file mode 100644 index 0000000..f132b3d --- /dev/null +++ b/conscrypt/pinned.properties @@ -0,0 +1,15 @@ +# What `build-conscrypt.sh` builds, and what `fetch-conscrypt.sh` looks for. +# +# Pinned rather than tracked: the build takes about fifteen minutes and its output is +# cached as a release on this repository, so the pin is the cache key. Moving either sha +# means the next run rebuilds and publishes a new release. Bump them deliberately. + +# google/conscrypt, branch google3-export. This is where ECH lives until it reaches a +# Conscrypt release: `Conscrypt.setEchConfigList(SSLSocket, byte[])` is public API here and +# exists in no published artifact. See README.md. +conscryptRef=d65d6b602c174408a6beaa9feab445fb500ce2bb +conscryptVersion=2.6-SNAPSHOT + +# google/boringssl. Conscrypt builds its JNI library against BoringSSL's source tree; the +# upstream CI tracks main, which is why this is pinned here rather than left to float. +boringsslRef=3c31f33f61cdb22e48437c4347d17c6cebab1506 diff --git a/conscrypt/release-tag.sh b/conscrypt/release-tag.sh new file mode 100755 index 0000000..d112541 --- /dev/null +++ b/conscrypt/release-tag.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash + +# Prints the release tag for the currently pinned build. +# +# One place, because three things have to agree on it: the workflow that publishes the +# release, the script that downloads it, and anyone looking at the releases page trying to +# work out which commits a jar came from. Both shas are in the tag because the artifact +# depends on both. + +set -euo pipefail + +conscrypt_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +# shellcheck disable=SC1091 +source "$conscrypt_dir/pinned.properties" + +echo "conscrypt-${conscryptRef:0:12}-boringssl-${boringsslRef:0:12}" diff --git a/network/build.gradle.kts b/network/build.gradle.kts index 7e0a5dd..033309c 100644 --- a/network/build.gradle.kts +++ b/network/build.gradle.kts @@ -39,6 +39,19 @@ fun compareVersions( ): Int = a.zip(b).firstNotNullOfOrNull { (x, y) -> (x - y).takeIf { it != 0 } } ?: 0 val echTestPattern = "EchTest" +val echConscryptTestPattern = "EchConscryptTest" +val echClientHelloTestPattern = "EchClientHelloTest" + +// The Conscrypt built from `google3-export`, if someone has fetched or built it. It is not on +// any repository — `Conscrypt.setEchConfigList` exists on that branch and in no release — so +// `conscrypt/fetch-conscrypt.sh` stages it here. Absent, the suite that needs it is left out of +// the build entirely rather than failing to compile. See conscrypt/README.md. +val conscryptJars = + fileTree(rootProject.layout.projectDirectory.dir("conscrypt/build/dist")) { + include("conscrypt-openjdk-*.jar") + } + +val hasConscrypt = !conscryptJars.isEmpty sourceSets { test { @@ -46,6 +59,13 @@ sourceSets { if (!supportsEch) { exclude("**/$echTestPattern.kt") } + if (!supportsEch || !hasConscrypt) { + exclude( + "**/$echConscryptTestPattern.kt", + "**/$echClientHelloTestPattern.kt", + "**/ConscryptEch.kt", + ) + } } } } @@ -83,7 +103,11 @@ val networkTest = val testSourceSet = sourceSets.test.get() testClassesDirs = testSourceSet.output.classesDirs classpath = testSourceSet.runtimeClasspath - exclude("**/$echTestPattern.class") + exclude( + "**/$echTestPattern.class", + "**/$echConscryptTestPattern.class", + "**/$echClientHelloTestPattern.class", + ) reportEndpointsTo("networkTest") ignoreFailures = true @@ -114,12 +138,42 @@ val echTest = } } +// The same servers, through Conscrypt rather than the JDK. Its own task for the same reason +// echTest is: its result answers a different question. echTest says whether OkHttp can do ECH as +// shipped, which on the JVM it cannot; this says whether the missing piece is the TLS stack and +// nothing else. Reading them together is the point, so they report side by side. +val echConscryptTest = + tasks.register("echConscryptTest") { + group = "verification" + description = "Reports whether ECH works on the JVM with a Conscrypt that supports it." + + val testSourceSet = sourceSets.test.get() + testClassesDirs = testSourceSet.output.classesDirs + classpath = testSourceSet.runtimeClasspath + // EchClientHelloTest runs here rather than under networkTest despite calling nobody: it + // answers the same question as the rest of this task and needs the same Conscrypt, and + // reading a "the servers agree" result next to a "the bytes are right" one is the point. + include("**/$echConscryptTestPattern.class", "**/$echClientHelloTestPattern.class") + + reportEndpointsTo("echConscryptTest") + enabled = supportsEch && hasConscrypt + ignoreFailures = true + + doFirst { + logger.lifecycle("Testing ECH against OkHttp $okhttpVersion on Conscrypt") + } + } + if (!supportsEch) { logger.lifecycle("Skipping EchTest: OkHttp $okhttpVersion predates the ECH API") } +if (!hasConscrypt) { + logger.lifecycle("Skipping EchConscryptTest: no Conscrypt build. Run conscrypt/fetch-conscrypt.sh.") +} + tasks.check { - dependsOn(networkTest, echTest) + dependsOn(networkTest, echTest, echConscryptTest) } dependencies { @@ -127,6 +181,9 @@ dependencies { testImplementation("com.squareup.okhttp3:okhttp-tls:$okhttpVersion") testImplementation("com.squareup.okhttp3:okhttp-dnsoverhttps:$okhttpVersion") + // Absent unless someone staged it; `hasConscrypt` leaves EchConscryptTest out when it is. + testImplementation(conscryptJars) + testImplementation(libs.junit.jupiter.api) testImplementation(libs.junit.jupiter.params) testRuntimeOnly(libs.junit.jupiter.engine) diff --git a/network/src/test/kotlin/okhttp/testbed/network/ConscryptEch.kt b/network/src/test/kotlin/okhttp/testbed/network/ConscryptEch.kt new file mode 100644 index 0000000..b180ea4 --- /dev/null +++ b/network/src/test/kotlin/okhttp/testbed/network/ConscryptEch.kt @@ -0,0 +1,202 @@ +/* + * 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 okhttp.testbed.network + +import java.io.IOException +import java.net.Socket +import java.security.KeyStore +import java.util.concurrent.ConcurrentHashMap +import javax.net.ssl.SSLContext +import javax.net.ssl.SSLSocket +import javax.net.ssl.SSLSocketFactory +import javax.net.ssl.TrustManagerFactory +import javax.net.ssl.X509TrustManager +import okhttp3.Dns +import okio.ByteString +import org.conscrypt.Conscrypt +import org.conscrypt.DomainEncryptionMode +import org.conscrypt.NetworkSecurityPolicy +import org.conscrypt.metrics.CertificateTransparencyVerificationReason + +/** + * Everything needed to encrypt a client hello on the JVM, which as of this writing is not + * something OkHttp can arrange on its own. + * + * OkHttp resolves the ECH config list out of the DNS HTTPS record and hands it to its platform; + * on Android that platform passes it to Conscrypt and ECH happens. On the JVM `ConscryptPlatform` + * takes the same parameter and drops it — `configureTlsExtensions` sets session tickets and ALPN + * and returns. So the config list reaches [okhttp3.Route] and stops there, which is exactly what + * [EchTest] reports. + * + * These types close that gap from outside OkHttp, using published API on both sides: OkHttp's + * [Dns.Record.ServiceMetadata.echConfigList] for the config, and Conscrypt's + * `Conscrypt.setEchConfigList` for the socket. What that measures is *Conscrypt's* half of ECH, + * not OkHttp's — see [EchConscryptTest] for what does and doesn't follow from a pass here. + */ +object ConscryptEch { + /** + * True if the Conscrypt on the classpath can encrypt a client hello. + * + * `setEchConfigList` exists only on the `google3-export` branch. A released Conscrypt has the + * rest of this class's API and not this method, so the check is for the method rather than for + * the provider — anything else would report "Conscrypt is missing" for a Conscrypt that is + * there and merely too old. + */ + val isSupported: Boolean by lazy { + try { + Conscrypt::class.java.getMethod("setEchConfigList", SSLSocket::class.java, ByteArray::class.java) + Conscrypt.isAvailable() + } catch (_: NoSuchMethodException) { + false + } catch (_: NoClassDefFoundError) { + false + } + } + + val version: String + get() = Conscrypt.version().let { "${it.major()}.${it.minor()}.${it.patch()}" } + + /** An [SSLContext] from Conscrypt, trusting whatever the JVM trusts. */ + fun sslContext(trustManager: X509TrustManager): SSLContext = + SSLContext.getInstance("TLS", Conscrypt.newProvider()).apply { + init(null, arrayOf(trustManager), null) + } + + /** The JVM's default trust manager. */ + fun platformTrustManager(): X509TrustManager = + TrustManagerFactory + .getInstance(TrustManagerFactory.getDefaultAlgorithm()) + .apply { init(null as KeyStore?) } + .trustManagers + .filterIsInstance() + .first() +} + +/** + * A trust manager that also answers Conscrypt's question about whether ECH is allowed here. + * + * Conscrypt asks the platform, through a [NetworkSecurityPolicy] it looks for by calling a + * `getNetworkSecurityPolicy` method on the configured trust manager. On Android the platform + * supplies one and the answer comes from `network_security_config.xml`. On the JVM the default + * answers `UNKNOWN`, which Conscrypt reads as "no" — so a config list set on the socket is + * ignored and the handshake goes out in the clear. That is the second half of why ECH does not + * work on the JVM today, and it is not something a caller can fix by configuring OkHttp. + * + * The method is found reflectively, which is why it has to be public on a public class. + */ +class EchEnablingTrustManager( + private val delegate: X509TrustManager, +) : X509TrustManager by delegate { + @Suppress("unused") // Called by Conscrypt, reflectively. + fun getNetworkSecurityPolicy(): NetworkSecurityPolicy = Policy + + private object Policy : NetworkSecurityPolicy { + override fun isCertificateTransparencyVerificationRequired(hostname: String): Boolean = false + + override fun getCertificateTransparencyVerificationReason( + hostname: String, + ): CertificateTransparencyVerificationReason = CertificateTransparencyVerificationReason.UNKNOWN + + // ENABLED rather than REQUIRED: a hostname with no config list should still connect, in the + // clear, the way a browser does. REQUIRED would turn tls12.tls-ech.dev into a failure to + // connect rather than the negative result it is there to give. + override fun getDomainEncryptionMode(hostname: String): DomainEncryptionMode = DomainEncryptionMode.ENABLED + } +} + +/** + * A [Dns] that remembers the ECH config list it saw for each hostname. + * + * OkHttp reads the config list out of the HTTPS record and carries it on the [okhttp3.Route], but + * a route is only visible once a connection exists — too late to configure the socket that makes + * it. So this watches the same records on their way past, and [EchSocketFactory] reads them back + * when a socket for that hostname is created. + * + * Only [Dns.newCall] carries service metadata; [Dns.lookup] is addresses only. Both are delegated, + * because OkHttp uses each in different circumstances. + */ +class EchRecordingDns( + private val delegate: Dns, +) : Dns { + private val echConfigLists = ConcurrentHashMap() + + operator fun get(hostname: String): ByteString? = echConfigLists[hostname.lowercase()] + + override fun lookup(hostname: String) = delegate.lookup(hostname) + + override fun newCall(request: Dns.Request): Dns.Call = RecordingCall(delegate.newCall(request)) + + private inner class RecordingCall( + private val delegate: Dns.Call, + ) : Dns.Call by delegate { + override fun enqueue(callback: Dns.Callback) { + delegate.enqueue( + object : Dns.Callback { + override fun onRecords( + call: Dns.Call, + last: Boolean, + records: List, + ) { + for (record in records) { + if (record !is Dns.Record.ServiceMetadata) continue + val echConfigList = record.echConfigList ?: continue + echConfigLists[record.hostname.lowercase()] = echConfigList + } + callback.onRecords(call, last, records) + } + + override fun onFailure( + call: Dns.Call, + e: IOException, + ) { + callback.onFailure(call, e) + } + }, + ) + } + } +} + +/** + * Applies the config list [dns] saw for a hostname to the socket about to connect to it. + * + * This is the call OkHttp's `ConscryptPlatform` doesn't make. Doing it here means the handshake + * is encrypted, and means the retry path isn't covered: when a server rejects a stale config it + * offers a fresh one, and reading that back needs `SSL_get0_ech_retry_configs`, which Conscrypt + * exposes on Android as `EchConfigMismatchException` and does not expose at all on the JVM. + */ +class EchSocketFactory( + private val delegate: SSLSocketFactory, + private val dns: EchRecordingDns, +) : DelegatingSSLSocketFactory(delegate) { + /** The hostnames a config list was applied to, so a test can assert it was applied at all. */ + val encryptedHostnames: MutableList = mutableListOf() + + override fun createSocket( + socket: Socket, + host: String, + port: Int, + autoClose: Boolean, + ): SSLSocket { + val sslSocket = delegate.createSocket(socket, host, port, autoClose) as SSLSocket + val echConfigList = dns[host] + if (echConfigList != null) { + Conscrypt.setEchConfigList(sslSocket, echConfigList.toByteArray()) + synchronized(encryptedHostnames) { encryptedHostnames += host } + } + return sslSocket + } +} diff --git a/network/src/test/kotlin/okhttp/testbed/network/EchClientHelloTest.kt b/network/src/test/kotlin/okhttp/testbed/network/EchClientHelloTest.kt new file mode 100644 index 0000000..d2d1328 --- /dev/null +++ b/network/src/test/kotlin/okhttp/testbed/network/EchClientHelloTest.kt @@ -0,0 +1,170 @@ +/* + * 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 okhttp.testbed.network + +import assertk.assertThat +import assertk.assertions.isFalse +import assertk.assertions.isTrue +import java.io.ByteArrayOutputStream +import java.io.IOException +import java.io.OutputStream +import java.net.InetAddress +import java.net.ServerSocket +import java.net.Socket +import java.security.SecureRandom +import javax.net.ssl.SSLSocket +import kotlin.concurrent.thread +import org.junit.jupiter.api.Assumptions.assumeTrue +import org.junit.jupiter.api.Test + +/** + * Reads the client hello Conscrypt actually sends, and checks that the name is not in it. + * + * Every other ECH suite here asks a server whether it saw an encrypted client hello, which means + * every other ECH suite depends on somebody's uptime and on a network that doesn't intercept TLS. + * This one asks the only question that has to be true for any of the rest to be — did the bytes + * on the wire carry the name in the clear — and answers it against a socket that accepts a + * connection and says nothing. It needs no DNS, no internet and no server, so it is the suite + * that says whether a red result elsewhere is about ECH or about the network the run was on. + * + * The config list here is synthetic. A real one comes from a DNS `HTTPS` record and is paired + * with a key the server holds; nothing here completes a handshake, so an unpaired public key is + * fine. It does have to be a well-formed `ECHConfigList` — BoringSSL rejects a malformed one, and + * a rejected config means a client hello with no ECH in it, which is exactly the failure this + * would otherwise miss. + */ +class EchClientHelloTest { + @Test + fun theNameIsNotSentInTheClear() { + assumeTrue(ConscryptEch.isSupported) { + "requires a Conscrypt with ECH. Run conscrypt/fetch-conscrypt.sh." + } + + val trustManager = EchEnablingTrustManager(ConscryptEch.platformTrustManager()) + val sslContext = ConscryptEch.sslContext(trustManager) + + val clientHello = captureClientHello { server -> + val socket = + sslContext.socketFactory.createSocket( + Socket(InetAddress.getLoopbackAddress(), server.localPort), + INNER_NAME, + server.localPort, + true, + ) as SSLSocket + org.conscrypt.Conscrypt.setEchConfigList(socket, echConfigList(PUBLIC_NAME)) + try { + socket.startHandshake() + } catch (_: IOException) { + // Nothing answers. The first flight is all this needs. + } + } + + // 0xfe0d is both the `encrypted_client_hello` extension type and the ECHConfig version, which + // is why matching the two bytes anywhere in the hello is enough: an ECH-less hello from this + // stack contains neither. + assertThat(clientHello.containsBytes(byteArrayOf(0xfe.toByte(), 0x0d))).isTrue() + + // The outer hello names the public name, and the real one is encrypted. Both halves matter: + // a client that sent no SNI at all would pass the second check on its own. + assertThat(clientHello.containsBytes(PUBLIC_NAME.toByteArray(Charsets.US_ASCII))).isTrue() + assertThat(clientHello.containsBytes(INNER_NAME.toByteArray(Charsets.US_ASCII))).isFalse() + } + + /** Runs [connect] against a socket that accepts once, reads the first flight, and closes. */ + private fun captureClientHello(connect: (ServerSocket) -> Unit): ByteArray { + ServerSocket(0, 1, InetAddress.getLoopbackAddress()).use { server -> + var captured = ByteArray(0) + val accepter = + thread { + try { + server.accept().use { accepted -> + val buffer = ByteArray(16 * 1024) + val read = accepted.getInputStream().read(buffer) + if (read > 0) captured = buffer.copyOf(read) + } + } catch (_: IOException) { + } + } + + connect(server) + accepter.join(5_000) + return captured + } + } + + private fun ByteArray.containsBytes(needle: ByteArray): Boolean = + (0..size - needle.size).any { start -> + needle.indices.all { this[start + it] == needle[it] } + } + + /** + * An `ECHConfigList` for [publicName], from RFC 9849 section 4. + * + * ``` + * ECHConfigList uint16 length, then ECHConfig* + * ECHConfig uint16 version, uint16 length, ECHConfigContents + * ``` + */ + private fun echConfigList(publicName: String): ByteArray { + val publicKey = ByteArray(32).also(SecureRandom()::nextBytes) + + val contents = + ByteArrayOutputStream().apply { + write(CONFIG_ID) + writeShort(KEM_X25519_HKDF_SHA256) + writeShort(publicKey.size) + write(publicKey) + writeShort(4) // cipher_suites, one suite of two uint16s. + writeShort(KDF_HKDF_SHA256) + writeShort(AEAD_AES_128_GCM) + write(MAXIMUM_NAME_LENGTH) + val name = publicName.toByteArray(Charsets.US_ASCII) + write(name.size) + write(name) + writeShort(0) // No extensions. + }.toByteArray() + + val config = + ByteArrayOutputStream().apply { + writeShort(ECH_CONFIG_VERSION) + writeShort(contents.size) + write(contents) + }.toByteArray() + + return ByteArrayOutputStream().apply { + writeShort(config.size) + write(config) + }.toByteArray() + } + + private fun OutputStream.writeShort(value: Int) { + write((value ushr 8) and 0xff) + write(value and 0xff) + } + + private companion object { + /** Neither name is resolved or connected to; they only have to be distinguishable. */ + private const val PUBLIC_NAME = "public.example.com" + private const val INNER_NAME = "secret.example.com" + + private const val ECH_CONFIG_VERSION = 0xfe0d + private const val CONFIG_ID = 42 + private const val KEM_X25519_HKDF_SHA256 = 0x0020 + private const val KDF_HKDF_SHA256 = 0x0001 + private const val AEAD_AES_128_GCM = 0x0001 + private const val MAXIMUM_NAME_LENGTH = 64 + } +} diff --git a/network/src/test/kotlin/okhttp/testbed/network/EchConscryptTest.kt b/network/src/test/kotlin/okhttp/testbed/network/EchConscryptTest.kt new file mode 100644 index 0000000..d89f9d6 --- /dev/null +++ b/network/src/test/kotlin/okhttp/testbed/network/EchConscryptTest.kt @@ -0,0 +1,155 @@ +/* + * 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 okhttp.testbed.network + +import assertk.assertThat +import assertk.assertions.contains +import assertk.assertions.doesNotContain +import assertk.assertions.isNotNull +import okhttp3.HttpUrl.Companion.toHttpUrl +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.dnsoverhttps.DnsOverHttps +import org.junit.jupiter.api.Assumptions.assumeTrue +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test + +/** + * The same servers [EchTest] calls, reached through a Conscrypt built from `google3-export` + * instead of through the JDK's TLS stack. + * + * [EchTest] fails its `sni=encrypted` assertions on the JVM, and that is a true result: no + * published TLS stack a JVM can load will encrypt a client hello. This suite exists to say what + * is missing rather than that something is. It supplies the two pieces the JVM lacks — a + * Conscrypt with ECH in it, and a network security policy that says ECH is allowed — and leaves + * everything else to OkHttp. When these pass and [EchTest] doesn't, the difference is the work + * still to be done, and it is a small and specific piece of work: + * + * * `ConscryptPlatform.configureTlsExtensions` accepts an `echConfigList` and ignores it. It + * needs to call `Conscrypt.setEchConfigList`, the way `Android10Platform` does. + * * `ConscryptPlatform.getEchRetryConfig` doesn't exist. Android's reads the retry config out + * of an `EchConfigMismatchException`; Conscrypt's OpenJDK `Platform` throws away both the + * retry configs and the public name, so on the JVM there is nothing to read. That one is a + * Conscrypt change, not an OkHttp change, which is why the stale-config case from [EchTest] + * has no counterpart here. + * + * The Conscrypt this needs is not published anywhere. `conscrypt/build-conscrypt.sh` builds it + * and the `conscrypt` workflow caches the result as a release; without it every test here skips. + * See `conscrypt/README.md`. + */ +@RequiresEndpoint(Endpoint.CLOUDFLARE_DOH) +class EchConscryptTest { + private lateinit var client: OkHttpClient + private lateinit var dns: EchRecordingDns + private lateinit var socketFactory: EchSocketFactory + + @BeforeEach + fun setUp() { + assumeTrue(ConscryptEch.isSupported) { + "requires a Conscrypt with ECH. Run conscrypt/fetch-conscrypt.sh." + } + + val trustManager = EchEnablingTrustManager(ConscryptEch.platformTrustManager()) + val sslContext = ConscryptEch.sslContext(trustManager) + + // Conscrypt for the bootstrap client too, so one TLS stack is under test rather than two. + // The resolver is addressed by IP, so resolving it doesn't need a resolver. + val bootstrapClient = + OkHttpClient + .Builder() + .sslSocketFactory(sslContext.socketFactory, trustManager) + .build() + + dns = + EchRecordingDns( + DnsOverHttps + .Builder() + .client(bootstrapClient) + .url("https://1.1.1.1/dns-query".toHttpUrl()) + .includeServiceMetadata(true) + .build(), + ) + + socketFactory = EchSocketFactory(sslContext.socketFactory, dns) + + client = + bootstrapClient + .newBuilder() + .sslSocketFactory(socketFactory, trustManager) + .dns(dns) + .build() + } + + @Test + @RequiresEndpoint(Endpoint.CLOUDFLARE_ECH) + fun cloudflareAcceptsAnEncryptedClientHello() { + val body = get("https://cloudflare-ech.com/cdn-cgi/trace") + + assertThat(socketFactory.encryptedHostnames).contains("cloudflare-ech.com") + assertThat(body).contains("sni=encrypted") + } + + @Test + @RequiresEndpoint(Endpoint.TLS_ECH_DEV) + fun tlsEchDevAcceptsAnEncryptedClientHello() { + val body = get("https://tls-ech.dev/") + + assertThat(socketFactory.encryptedHostnames).contains("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") + } + + @Test + @RequiresEndpoint(Endpoint.DEFO_IE) + fun defoIeAcceptsAnEncryptedClientHello() { + val body = get("https://defo.ie/ech-check.php") + + assertThat(socketFactory.encryptedHostnames).contains("defo.ie") + assertThat(body).contains("SSL_ECH_STATUS: success") + } + + /** + * TLS 1.2 cannot carry ECH, and this name publishes no config list. The point is that the + * connection still happens: a client that can do ECH must not break the servers that can't. + */ + @Test + @RequiresEndpoint(Endpoint.TLS_ECH_DEV) + fun tls12IsReachedWithoutEch() { + val body = get("https://tls12.tls-ech.dev/") + + assertThat(body).contains("

tls12.tls-ech.dev

") + assertThat(body).contains("You are not using ECH") + assertThat(body).doesNotContain("You are using ECH") + } + + /** + * The config list still has to reach OkHttp for any of this to be OkHttp's ECH rather than the + * suite's. This asserts the half that already works, on the same connection as the rest. + */ + @Test + @RequiresEndpoint(Endpoint.CLOUDFLARE_ECH) + fun okHttpResolvesTheConfigListFromDns() { + get("https://cloudflare-ech.com/cdn-cgi/trace") + + assertThat(dns["cloudflare-ech.com"]).isNotNull() + } + + private fun get(url: String): String = + client.newCall(Request(url.toHttpUrl())).execute().use { it.body.string() } +} diff --git a/site/tools/collect_results.py b/site/tools/collect_results.py index e2b0d59..2337c69 100644 --- a/site/tools/collect_results.py +++ b/site/tools/collect_results.py @@ -45,7 +45,13 @@ import xml.etree.ElementTree as ElementTree # Gradle test tasks whose failures are findings about OkHttp rather than breakage here. -REPORTING_TASKS = {"loomTest", "echTest", "networkTest"} +REPORTING_TASKS = {"loomTest", "echTest", "echConscryptTest", "networkTest"} + +# The same distinction for suites that can't make it with a task name. Android instrumentation +# runs under one task whatever it is testing, so the Android suite that calls tls-ech.dev and +# defo.ie has no way to say it reports rather than gates except by being named here. Everything +# else in the Android module runs against containers this repository starts. +REPORTING_CLASSES = {"PublicEncryptedClientHelloTest"} # How many collections the history keeps. Enough for the trend strip to show a few weeks of # daily runs without the file growing without bound. @@ -98,7 +104,7 @@ def parse_suite(path: pathlib.Path, task: str, workflow: str, run_url: str) -> d "workflow": workflow, "runUrl": run_url, "task": task, - "reporting": task in REPORTING_TASKS, + "reporting": task in REPORTING_TASKS or simple_name in REPORTING_CLASSES, "timeSeconds": float(root.get("time") or 0.0), "passed": sum(1 for c in cases if c["status"] == "passed"), "failed": sum(1 for c in cases if c["status"] == "failed"), diff --git a/site/topics/ech.html b/site/topics/ech.html index 2bdcaa9..b353704 100644 --- a/site/topics/ech.html +++ b/site/topics/ech.html @@ -96,6 +96,60 @@

What runs today

DnsOverHttps.Builder.includeServiceMetadata, and no release has it: 5.4.0 resolves A and AAAA records only, so there is no HTTPS record to carry a config list at all.

+

+ The same emulator also runs PublicEncryptedClientHelloTest, which is the JVM + suite's cases against tls-ech.dev, defo.ie and + cloudflare-ech.com. The fixture suite says whether the client is right; this one + makes the JVM and Android rows of the status page comparable, since they are then the same + assertions against the same servers with only the TLS stack differing. It reports rather than + gates, for the reason everything calling those servers does. +

+ +

What it would take on the JVM

+

+ network:echTest runs the same cases against the public servers on a JVM, and its + route assertions pass while its assertions about what the server saw fail. That is a true + result and a slightly misleading one: it reads as "OkHttp cannot do ECH on the JVM", where the + accurate version is that three separate things are in the way and only one of them is OkHttp's. +

+
+ + + + + + + + + + + + + + + + + + + + + +
WhatWhereStatus
A TLS stack that can encrypt a client helloConscrypt google3-exportExists, unpublished. Built and cached by this repository.
Handing the config list to that stackOkHttp ConscryptPlatform.configureTlsExtensionsTakes an echConfigList and ignores it. One call, next to the ALPN one.
Reading a server's retry config backConscrypt OpenJDK PlatformDiscards the retry configs and the public name. A Conscrypt change, not an OkHttp one.
+
+

+ network:echConscryptTest supplies the first and leaves the rest to OkHttp, so the + gap between it and echTest is the second row and nothing else. Under it, + EchClientHelloTest reads the client hello off a local socket that answers nothing + and asserts the name is not in the bytes — no DNS, no server, no network — which makes it the + one ECH result that is never about somebody else's uptime. +

+

+ The third row is why stale.tls-ech.dev has no JVM counterpart. On Android a + rejected config arrives as an EchConfigMismatchException carrying the config the + server offered instead; on OpenJDK the same path throws the retry configs away, and nothing + public exposes them. A stale config can be detected there and not recovered from, whatever + OkHttp does. +

Specifications

From adce25111d4e740f9e87c727a64bb25cb9724537 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 10:02:08 +0000 Subject: [PATCH 2/3] Wait for the device before the first instrumentation run, and retry a run that produced nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first PR run installed the test APK while the emulator's package service was still coming up, got `Broken pipe`, and reported zero tests for `PublicEncryptedClientHelloTest`. Zero tests is the worst failure mode available here: it isn't an ECH result, and on the status page it doesn't look like one either. Two changes, because the race has two ends. `wait_for_package_service` asks `pm` to answer before the first suite runs — `sys.boot_completed` is already true when the install fails, so it is the wrong thing to wait on. And a run that fails without writing any results is retried once, which covers the same failure wherever it turns up and costs nothing when it doesn't. Also drop the "fifteen minutes" figure for the Conscrypt build; the workflow does it in about six. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CqiK79k9uoWsn2AzgXpHMA --- android-ech/run-ech-test.sh | 32 ++++++++++++++++++++++++++++++++ conscrypt/README.md | 2 +- conscrypt/build-conscrypt.sh | 2 +- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/android-ech/run-ech-test.sh b/android-ech/run-ech-test.sh index e906566..4552837 100755 --- a/android-ech/run-ech-test.sh +++ b/android-ech/run-ech-test.sh @@ -110,6 +110,20 @@ run_suite() { -Pandroid.testInstrumentationRunnerArguments.class="okhttp.testbed.android.ech.$class" \ "$@" || status=$? + # A run that produced no results at all didn't fail its assertions — it never got as far as + # running them. The way that happens here is an APK install against an emulator whose package + # service is still coming up, which answers `Broken pipe` and leaves Gradle reporting zero + # tests. Retried once, because a suite that reported nothing is worse than a slow job: it + # looks like a pass on the status page and is not one. + if [ "$status" -ne 0 ] && [ ! -d "$results_dir" ]; then + echo "$class produced no results; retrying once." >&2 + status=0 + "$repository_root/gradlew" -p "$repository_root" :android-ech:connectedDebugAndroidTest \ + "${gradle_arguments[@]}" \ + -Pandroid.testInstrumentationRunnerArguments.class="okhttp.testbed.android.ech.$class" \ + "$@" || status=$? + fi + if [ -d "$results_dir" ]; then rm -rf "$results_dir-$class" mv "$results_dir" "$results_dir-$class" @@ -117,6 +131,24 @@ run_suite() { return $status } +# The emulator reports itself booted before its package service will accept an install, and the +# first `connectedDebugAndroidTest` of a run is what meets that. Waiting for `pm` to answer is +# the check that matches the failure — `sys.boot_completed` on its own is already true when the +# install fails. Best effort: on a machine where this can't be asked, the retry above still +# covers it. +wait_for_package_service() { + adb wait-for-device || return 0 + for _ in $(seq 1 90); do + if adb shell pm path android >/dev/null 2>&1; then + return 0 + fi + sleep 2 + done + echo "Timed out waiting for the device's package service; running anyway." >&2 +} + +wait_for_package_service + # The public servers first, and not allowed to fail the run. tls-ech.dev, defo.ie and # cloudflare-ech.com belong to other people; an outage there is not a result about OkHttp, and # the JVM `network` suites treat the same servers the same way. The XML still records what diff --git a/conscrypt/README.md b/conscrypt/README.md index 4f1c024..c62e720 100644 --- a/conscrypt/README.md +++ b/conscrypt/README.md @@ -40,7 +40,7 @@ The API this suite uses is `Conscrypt.setEchConfigList(SSLSocket, byte[])`. How it's built and cached ------------------------- -The build takes about fifteen minutes, needs a C++ toolchain and two source trees, and its output +The build takes several minutes, needs a C++ toolchain and two source trees, and its output depends on nothing in this repository except `pinned.properties`. So it does not happen in a test workflow. The `conscrypt` workflow builds it when that file changes and publishes the jars as a release; the release tag carries both pinned shas, so bumping a pin invalidates the cache by diff --git a/conscrypt/build-conscrypt.sh b/conscrypt/build-conscrypt.sh index d350f63..c9fcf26 100755 --- a/conscrypt/build-conscrypt.sh +++ b/conscrypt/build-conscrypt.sh @@ -3,7 +3,7 @@ # Builds Conscrypt's OpenJDK artifact from the pinned google3-export commit, against the # pinned BoringSSL commit, and stages the result under conscrypt/build/dist. # -# This is deliberately not part of the Gradle build. It takes around fifteen minutes, needs +# This is deliberately not part of the Gradle build. It takes several minutes, needs # a C++ toolchain and a cross compiler, and its output changes only when `pinned.properties` # changes — so it runs in its own workflow and its output is cached as a release. Suites get # the jar from `fetch-conscrypt.sh`, which downloads that release. See README.md. From db6b847349226dfaa425762a3295b7fa4a93a19b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 11:01:08 +0000 Subject: [PATCH 3/3] Depend on okhttp-android, so the Android public suite can resolve a hostname MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the install race fixed, the public suite ran for the first time and all six cases failed identically, none of them for a reason involving ECH: java.lang.IllegalStateException: Unable to load PublicSuffixDatabase.list at okhttp3.internal.publicsuffix.AssetPublicSuffixList.listSource at okhttp3.dnsoverhttps.DnsOverHttps$Companion.isPrivateHost On Android the public suffix list is an asset rather than a jar resource, and `assets/PublicSuffixDatabase.list` is shipped by `okhttp-android` — which this module didn't depend on, because nothing in the source names it. `DnsOverHttps` consults the list on every query to decide whether a host is private, so the suite failed before it opened a socket. Verified by assembling the test APK and finding the asset in it. What the six cases actually say about ECH on API 37 is now a question CI can answer, which it could not before. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CqiK79k9uoWsn2AzgXpHMA --- .github/workflows/conscrypt.yml | 4 ++-- android-ech/build.gradle.kts | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/conscrypt.yml b/.github/workflows/conscrypt.yml index ab10b14..723dbff 100644 --- a/.github/workflows/conscrypt.yml +++ b/.github/workflows/conscrypt.yml @@ -2,8 +2,8 @@ name: conscrypt # Builds the Conscrypt the ECH suites need, and caches it as a release on this repository. # -# Not on every commit, and not as part of any test workflow: the build takes around fifteen -# minutes, needs a C++ toolchain, a cross compiler and two source trees, and its output depends +# Not on every commit, and not as part of any test workflow: the build takes several minutes, +# needs a C++ toolchain, a cross compiler and two source trees, and its output depends # on nothing in this repository except `conscrypt/pinned.properties`. So it runs when that file # changes, or on demand, and everything else downloads what it published. The release tag carries # both pinned shas, which makes the cache self-invalidating: bump a pin and the tag no longer diff --git a/android-ech/build.gradle.kts b/android-ech/build.gradle.kts index 6feca96..28bcb63 100644 --- a/android-ech/build.gradle.kts +++ b/android-ech/build.gradle.kts @@ -55,6 +55,12 @@ dependencies { androidTestImplementation("com.squareup.okhttp3:okhttp:$okhttpVersion") androidTestImplementation("com.squareup.okhttp3:okhttp-dnsoverhttps:$okhttpVersion") + // Not optional, despite nothing here naming it. On Android the public suffix list is read + // from `assets/PublicSuffixDatabase.list`, which only this artifact ships; without it every + // `DnsOverHttps` query throws from `isPrivateHost` before a connection is attempted, and the + // whole suite fails on something that has nothing to do with ECH. + androidTestImplementation("com.squareup.okhttp3:okhttp-android:$okhttpVersion") + androidTestImplementation(libs.assertk) androidTestImplementation(libs.junit.jupiter.api) androidTestImplementation(libs.junit5android.core)