Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions .github/workflows/android-ech.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ concurrency:
jobs:
android-ech:
name: android-ech (${{ inputs.okhttpVersion || 'pinned snapshot' }})
# Declared once and used twice — by the emulator that runs the suite, and by the metadata
# that tells the status page what the suite ran on. Two copies of an API level is how a
# page ends up confidently reporting the wrong device.
env:
EMULATOR_API_LEVEL: '37.0'
EMULATOR_TARGET: google_apis_playstore_ps16k
EMULATOR_ARCH: x86_64
runs-on: ubuntu-latest
timeout-minutes: 45

Expand Down Expand Up @@ -59,9 +66,9 @@ jobs:
- name: Run the ECH suite against the fixture containers
uses: reactivecircus/android-emulator-runner@v2
with:
api-level: '37.0'
target: google_apis_playstore_ps16k
arch: x86_64
api-level: ${{ env.EMULATOR_API_LEVEL }}
target: ${{ env.EMULATOR_TARGET }}
arch: ${{ env.EMULATOR_ARCH }}
disable-animations: true
emulator-options: >-
-no-window
Expand Down Expand Up @@ -91,7 +98,7 @@ jobs:
"workflow": "android-ech",
"label": "${{ inputs.okhttpVersion || 'pinned-snapshot' }}",
"okhttpVersion": "$version",
"javaVersion": "21",
"platform": "Android emulator API $EMULATOR_API_LEVEL · $EMULATOR_ARCH",
"jobStatus": "${{ job.status }}",
"runNumber": ${{ github.run_number }},
"runUrl": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}",
Expand Down
9 changes: 8 additions & 1 deletion .github/workflows/containers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,20 @@ jobs:
if [ -z "$version" ]; then
version=$(sed -n 's/^okhttp = "\(.*\)"$/\1/p' gradle/libs.versions.toml)
fi
# What this actually ran on. Measured rather than written down: a runner image bump
# or a toolchain change should show up on the status page as a different platform,
# not as the same string next to different results.
props=$(java -XshowSettings:properties -version 2>&1)
prop() { printf '%s\n' "$props" | sed -n "s/^ *$1 = //p" | head -1; }
platform="$(prop java.vendor) JDK $(prop java.version) · $(prop os.arch)"

mkdir -p containers/build/test-results
cat > containers/build/test-results/run-metadata.json <<JSON
{
"workflow": "containers",
"label": "${{ matrix.okhttpVersion || 'pinned' }}",
"okhttpVersion": "$version",
"javaVersion": "21",
"platform": "$platform",
"jobStatus": "${{ job.status }}",
"runNumber": ${{ github.run_number }},
"runUrl": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}",
Expand Down
11 changes: 9 additions & 2 deletions .github/workflows/network.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ jobs:
# failures that happen before a test runs, so one suite failing to compile or to
# resolve doesn't rob the others of a result.
run: >
./gradlew network:networkTest network:echTest network:echConscryptTest --continue
./gradlew network:networkTest network:echTest network:echConscryptTest network:echPlatformTest --continue
${{ matrix.okhttpVersion && format('-PokhttpVersion={0}', matrix.okhttpVersion) || '' }}

# What the XML can't say: which OkHttp version 'pinned' actually resolved to, and
Expand All @@ -129,13 +129,20 @@ jobs:
if [ -z "$version" ]; then
version=$(sed -n 's/^okhttp = "\(.*\)"$/\1/p' gradle/libs.versions.toml)
fi
# What this actually ran on. Measured rather than written down: a runner image bump
# or a toolchain change should show up on the status page as a different platform,
# not as the same string next to different results.
props=$(java -XshowSettings:properties -version 2>&1)
prop() { printf '%s\n' "$props" | sed -n "s/^ *$1 = //p" | head -1; }
platform="$(prop java.vendor) JDK $(prop java.version) · $(prop os.arch)"

mkdir -p network/build/test-results
cat > network/build/test-results/run-metadata.json <<JSON
{
"workflow": "network",
"label": "${{ matrix.okhttpVersion || 'pinned' }}",
"okhttpVersion": "$version",
"javaVersion": "21",
"platform": "$platform",
"jobStatus": "${{ job.status }}",
"runNumber": ${{ github.run_number }},
"runUrl": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}",
Expand Down
20 changes: 18 additions & 2 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,24 @@ subprojects {
doLast {
val offences =
sources.files.sorted().flatMap { file ->
file
.readLines()
val lines = file.readLines()

// A file may opt out by saying why, on its own line:
//
// // USES-OKHTTP-INTERNALS: reimplements ConscryptPlatform's missing ECH call.
//
// The rule is about suites: a test that reaches into `okhttp3.internal` is testing
// something no caller can rely on. A file whose subject *is* an internal — a platform
// OkHttp doesn't ship yet — can't be written any other way, and the alternative to
// this marker is deleting the rule for everybody. The reason is required, and the
// exemptions are printed on every run so they stay visible rather than accumulating.
val exemption = lines.firstOrNull { it.trim().startsWith("// USES-OKHTTP-INTERNALS:") }
if (exemption != null) {
logger.lifecycle("public-api-only: ${file.name} exempt — ${exemption.substringAfter(":").trim()}")
return@flatMap emptyList<String>()
}

lines
.withIndex()
.filter { (_, line) ->
line.startsWith("import ") && forbiddenImports.any { line.removePrefix("import ").startsWith("$it.") }
Expand Down
8 changes: 8 additions & 0 deletions conscrypt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ 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.

`network:echPlatformTest` measures that second row rather than describing it. `EchConscryptPlatform`
is a `Platform` that makes exactly that call and nothing else new; `EchPlatformTest` then runs
`EchTest`'s requests through ordinary public API with it installed. The two suites are the same
client against the same servers, so the difference between their results is the one call. That
platform is the only file here allowed to import `okhttp3.internal` — it is a `Platform`, which
OkHttp declares nowhere else — and it says so with a `USES-OKHTTP-INTERNALS:` marker that
`checkPublicApiOnly` reports on every run.

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
Expand Down
31 changes: 30 additions & 1 deletion network/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ fun compareVersions(
val echTestPattern = "EchTest"
val echConscryptTestPattern = "EchConscryptTest"
val echClientHelloTestPattern = "EchClientHelloTest"
val echPlatformTestPattern = "EchPlatformTest"

// 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
Expand All @@ -63,7 +64,9 @@ sourceSets {
exclude(
"**/$echConscryptTestPattern.kt",
"**/$echClientHelloTestPattern.kt",
"**/$echPlatformTestPattern.kt",
"**/ConscryptEch.kt",
"**/EchConscryptPlatform.kt",
)
}
}
Expand Down Expand Up @@ -107,6 +110,7 @@ val networkTest =
"**/$echTestPattern.class",
"**/$echConscryptTestPattern.class",
"**/$echClientHelloTestPattern.class",
"**/$echPlatformTestPattern.class",
)

reportEndpointsTo("networkTest")
Expand Down Expand Up @@ -164,6 +168,31 @@ val echConscryptTest =
}
}

// The third reading of the same servers, and the one that answers the question the other two
// only bracket. echTest says OkHttp as shipped doesn't encrypt a client hello on the JVM;
// echConscryptTest says Conscrypt can, from outside OkHttp. Neither says what happens when the
// config list OkHttp resolved reaches Conscrypt through OkHttp's own platform, because no
// platform does that yet. EchConscryptPlatform is one that does, so this task is the difference
// between echTest and a fixed OkHttp, measured rather than argued.
val echPlatformTest =
tasks.register<Test>("echPlatformTest") {
group = "verification"
description = "Reports whether OkHttp does ECH when its platform makes the Conscrypt call."

val testSourceSet = sourceSets.test.get()
testClassesDirs = testSourceSet.output.classesDirs
classpath = testSourceSet.runtimeClasspath
include("**/$echPlatformTestPattern.class")

reportEndpointsTo("echPlatformTest")
enabled = supportsEch && hasConscrypt
ignoreFailures = true

doFirst {
logger.lifecycle("Testing ECH against OkHttp $okhttpVersion on Conscrypt, through OkHttp's platform")
}
}

if (!supportsEch) {
logger.lifecycle("Skipping EchTest: OkHttp $okhttpVersion predates the ECH API")
}
Expand All @@ -173,7 +202,7 @@ if (!hasConscrypt) {
}

tasks.check {
dependsOn(networkTest, echTest, echConscryptTest)
dependsOn(networkTest, echTest, echConscryptTest, echPlatformTest)
}

dependencies {
Expand Down
118 changes: 118 additions & 0 deletions network/src/test/kotlin/okhttp/testbed/network/EchConscryptPlatform.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
* 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.
*/
// USES-OKHTTP-INTERNALS: is a Platform, which OkHttp only declares internally.
package okhttp.testbed.network

import javax.net.ssl.SSLContext
import javax.net.ssl.SSLSocket
import javax.net.ssl.SSLSocketFactory
import javax.net.ssl.X509TrustManager
import okhttp3.Protocol
import okhttp3.internal.OkHttpInternalApi
import okhttp3.internal.platform.Platform
import okio.ByteString
import org.conscrypt.Conscrypt

/**
* OkHttp's `ConscryptPlatform`, plus the one call it doesn't make.
*
* [EchConscryptTest] shows Conscrypt can encrypt a client hello, but it shows it from a socket
* factory — outside OkHttp, on a socket OkHttp then uses. That leaves the interesting question
* open, because the config list it passes is one the suite kept for itself rather than the one
* OkHttp resolved. This closes it: the config list arrives the way OkHttp delivers it, as the
* `echConfigList` argument to [configureTlsExtensions], and the only thing added is the
* [Conscrypt.setEchConfigList] call that `ConscryptPlatform` omits.
*
* So [EchPlatformTest] and [EchTest] make the same requests through the same public API, and
* differ only by which platform is installed. What that difference measures is one call.
*
* Why this exists here rather than upstream: OkHttp's `master` can't compile against a Conscrypt
* that has these methods, because no published Conscrypt does. [lysine-dev/okhttp#9559][pr] is
* the change on the OkHttp side, and it builds against a Conscrypt built from source. This
* reaches the same place from the other direction, with a platform written here.
*
* It is not a proposal for how OkHttp should do it. `ConscryptPlatform` is `final` and its
* constructor is private, so this can't extend it and reimplements the parts it needs instead;
* the upstream change is four lines in the class itself, and uses `setEchParameters`, a newer
* Conscrypt API than the `setEchConfigList` the `google3-export` build here exposes.
*
* [pr]: https://github.com/lysine-dev/okhttp/pull/9559
*/
@OptIn(OkHttpInternalApi::class)
class EchConscryptPlatform : Platform() {
private val provider = Conscrypt.newProvider()

override fun newSSLContext(): SSLContext = SSLContext.getInstance("TLS", provider)

/**
* Conscrypt's own, wrapped so Conscrypt can find the policy that permits ECH.
*
* Both halves are load-bearing and both are covered by [ConscryptEch]: the JDK's trust manager
* rejects the authType Conscrypt uses for TLS 1.3, and without the policy Conscrypt treats ECH
* as not allowed and sends the hello in the clear whatever is set on the socket.
*/
override fun platformTrustManager(): X509TrustManager = EchEnablingTrustManager(ConscryptEch.platformTrustManager())

override fun trustManager(sslSocketFactory: SSLSocketFactory): X509TrustManager? = null

override fun configureTlsExtensions(
sslSocket: SSLSocket,
hostname: String?,
protocols: List<Protocol>,
echConfigList: ByteString?,
) {
if (!Conscrypt.isConscrypt(sslSocket)) {
super.configureTlsExtensions(sslSocket, hostname, protocols, echConfigList)
return
}

Conscrypt.setUseSessionTickets(sslSocket, true)
Conscrypt.setApplicationProtocols(sslSocket, alpnProtocolNames(protocols).toTypedArray())

// The line this whole file exists for. `ConscryptPlatform` takes this argument and returns.
if (echConfigList != null) {
Conscrypt.setEchConfigList(sslSocket, echConfigList.toByteArray())
}
}

override fun getSelectedProtocol(sslSocket: SSLSocket): String? =
when {
Conscrypt.isConscrypt(sslSocket) -> Conscrypt.getApplicationProtocol(sslSocket)
else -> super.getSelectedProtocol(sslSocket)
}

override fun newSslSocketFactory(trustManager: X509TrustManager): SSLSocketFactory =
newSSLContext()
.apply { init(null, arrayOf(trustManager), null) }
.socketFactory

override fun toString(): String = "EchConscryptPlatform"

companion object {
/**
* Makes this the platform every client built afterwards will use.
*
* `Platform.get()` is a process-wide singleton, read when a client builds its socket factory,
* so a client built before this call keeps the platform it was built with. Pair with
* [uninstall]; both live here so that this file stays the only one reaching into
* `okhttp3.internal`.
*/
fun install() = Platform.resetForTests(EchConscryptPlatform())

/** Puts back whichever platform OkHttp would have chosen for itself. */
fun uninstall() = Platform.resetForTests()
}
}
Loading
Loading