Skip to content

Kotlin: consolidate the two cert-exception clients into one host-aware trust manager #1556

Description

@jkmassel

Extracted from the review of #1527. Not a vulnerability — the current behavior is fail-safe; this is a cross-platform parity and simplification issue.

Summary

  • Kotlin's disableCertificateValidation routes opted-out hosts to a separate all-trusting OkHttpClient and guards it with a redirect interceptor that refuses every cross-host redirect — including redirects to hosts with a valid public certificate.
  • Swift makes the trust decision per server-trust challenge, so it follows such redirects and validates the new host normally. Same input, opposite outcome by platform.
  • Replace the two-client split with a single client whose X509ExtendedTrustManager decides trust per host. This matches Swift, and retires two other review findings (the GenericError flattening and the connection-pool drift) at the same time.

Current behavior

WpHttpClient.DefaultHttpClient keeps a strict client and a lazily-built all-trusting insecureClient, and routes the initial request by host (WpHttpClient.kt:141). Because OkHttp follows redirects internally on whichever client took the call, a network interceptor on the insecure client throws when a redirect leaves the opt-out set (WpHttpClient.kt:129):

disableCertificateValidation("dev.example.test")
GET https://dev.example.test/                → insecure client, trust-all handshake OK
← 301 Location: https://www.example.test/    (valid, publicly-trusted cert)
interceptor: "www.example.test" not in opt-out set → throw IOException
→ RequestExecutionErrorReason.GenericError   (WpRequestExecutor.kt:260)

The identical flow succeeds on iOS: the redirect target gets a fresh server-trust challenge and passes default validation. The canonical apex→www redirect (opt out the self-signed dev apex; it 301s to a www that has a real cert) therefore works on iOS and dead-ends on Android as an opaque error.

Root cause

OkHttp configures trust per client, so the design encodes "which trust policy" in which client took the request and then applies that one policy to every redirect hop. The guard is a patch over that: the all-trusting client can't tell "valid cert, accept normally" from "invalid cert, bypassing," so it refuses all cross-host hops.

Proposed change

One client. Move the opt-out decision into the trust layer, keyed on the connecting host:

  • X509ExtendedTrustManager: read the host from the handshake ((socket as SSLSocket).handshakeSession.peerHost, and the SSLEngine overload). Opted-out host → return (trust all). Otherwise → delegate to the platform default X509ExtendedTrustManager. Fail closed: if the host can't be determined, use strict validation.
  • HostnameVerifier: opted-out host → true; otherwise the existing allow-list + OkHostnameVerifier path (unchanged).

Delete insecureClient, the redirect network-interceptor, and getClient(host)'s routing.

Behavior after (per hop, across redirects — matches Swift)

Redirect target Result
Opted-out host trust-all (as configured)
Non-opted host, valid cert validated normally → works
Non-opted host, invalid cert rejected as InvalidSslError (not GenericError)

The trust-all bypass is now scoped to the exact host in the set and re-checked at every handshake, so it structurally cannot extend to a non-opted host — the same invariant the guard enforced by refusing, now enforced by correctly validating.

Also retires

  • The redirect refusal throwing a bare IOException that flattens into GenericError (WpRequestExecutor.kt:260): there is no thrown refusal anymore.
  • insecureClient is built once from client v0 (WpHttpClient.kt:94) and stops sharing the pool/dispatcher after a later addAllowedAlternativeNamesForHostname rebuild: there is only one client.

Risks / must validate before merging

  • SSLSession.getPeerHost() reliability. The whole decision hinges on it returning OkHttp's target host. It is reliable in OkHttp's usage (sockets are created with the hostname for SNI) but can be null in some JSSE paths — hence fail-closed. Add a test asserting peerHost matches the target across direct, redirected, and HTTP/2-coalesced connections.
  • HTTP/2 coalescing under a single client. With one SSLSocketFactory/HostnameVerifier instance, confirm a trust-all connection to an opted-out host cannot be coalesced for a non-opted host (the per-host verifier should gate it; prove it).

Acceptance criteria

  • A request to an opted-out host that 301s to a valid-cert host completes (parity with Swift), on the real OkHttpClient path.
  • A redirect to a non-opted host with an invalid cert surfaces as InvalidSslError, not GenericError.
  • A redirect from a non-opted host to an opted-out host still gets normal validation (i.e. the opt-out does not silently apply on a hop the router did not start on).
  • The peerHost-unavailable path falls back to strict validation (fail closed).
  • insecureClient, the redirect interceptor, and getClient(host) routing are gone.

Context: supersedes the redirect guard introduced in #1527.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions