Build Conscrypt with ECH, and test it on the JVM and on Android

.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

.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 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
+# 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

.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

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

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)

android-ech/run-ech-test.sh

@@ -92,9 +92,76 @@ 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=$?
+
+  # 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"
+  fi
+  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
+# 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"