Skip to content

feat: rework TLS certificate exceptions and fix the #1512 trust bypass - #1527

Open
jkmassel wants to merge 1 commit into
trunkfrom
issue-1512
Open

feat: rework TLS certificate exceptions and fix the #1512 trust bypass#1527
jkmassel wants to merge 1 commit into
trunkfrom
issue-1512

Conversation

@jkmassel

@jkmassel jkmassel commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Description

WpRequestExecutor.allowSSL(altNames:forCommonName:) — the Swift exception apps use to reach a self-hosted site whose certificate doesn't list the hostname — accepted the certificate as soon as its common name matched the allow-list. It never called SecTrustEvaluateWithError, so the chain signature, issuer, and validity dates went unchecked. A man-in-the-middle could self-sign a certificate copying a legitimate common name (public information) and be accepted for every host previously passed to the exception (#1512).

The exception now validates the full chain and waives only the hostname check — the trust is re-evaluated under a hostname-relaxed SecPolicyCreateSSL(true, nil) policy, and a certificate that fails falls through to default handling.

The old name and behavior conflated two different needs — "accept a valid certificate for an extra hostname" and "accept anything" — which was the root of the confusion. This splits them into two explicit methods and brings the same split to Kotlin.

Changes

Swift

  • allowAlternativeNames(_:forCommonName:) (renamed from allowSSL) accepts an otherwise-valid certificate whose name doesn't cover the host. The chain is still validated.
  • disableCertificateValidation(forHost:) — new — accepts any certificate for a named host (self-signed, expired, untrusted root), for hosts the caller controls.
  • allowSSL(altNames:forCommonName:) is deprecated, forwarding to allowAlternativeNames. Its @available message names both successors, so a caller who does nothing gets the secure behavior plus a warning, and anyone who relied on accept-anything is pointed at disableCertificateValidation.

Kotlin — the same two layers

  • WpHttpClient.DefaultHttpClient.disableCertificateValidation(host:) accepts any certificate for a host by routing it to a separate, all-trusting OkHttpClient. Layer 1 (addAllowedAlternativeNamesForHostname) was already safe — its HostnameVerifier runs after the default X509TrustManager, so it never had the The allowSSL server-trust override accepts the challenge without evaluating the trust #1512 bypass.
  • The all-trusting client refuses to follow a redirect to a host that isn't itself opted in, so the bypass can't be extended past the host the caller named. Swift needs no equivalent: URLSession issues a fresh server-trust challenge per host, so a redirect target already gets default validation.
  • Chain-validation failures are now reported as InvalidSslError. Self-signed / expired / untrusted-root certificates surface as an SSLException that previously fell through to GenericError; only hostname mismatches were classified as SSL problems.

Both platforms

  • The name allow-list matches the certificate's common name and the connecting host case-insensitively — DNS names are case-insensitive and the presented host's case isn't guaranteed — matching how the opt-out already lower-cased hosts.
  • The Kotlin opt-out state is guarded by a lock (Swift already used an NSLock), so concurrent configuration can't drop an opt-out and silently leave a host on strict validation.

Layer 2 needs a second client because OkHttp checks the chain before the hostname. The X509TrustManager never sees the target host, so a per-host bypass can't live there; the alternative — accept every chain globally and re-validate in the HostnameVerifier — would mean re-implementing chain validation by hand. Routing opted-in hosts to an all-trusting client keeps the default client fully strict.

Tests & docs

  • SSL-validation tests on all three platforms — and the login spec's reference implementation — now use badssl.com's dedicated hosts (wrong.host, self-signed, and the apex via its SAN) instead of the bespoke vanilla1.wpmt.co subdomain and wordpress-1315525-4803651.cloudwaysapps.com instance, which this frees for decommissioning.
  • Added a regression test (Swift and Kotlin): a self-signed certificate is rejected even when its host is allow-listed through the name exception.

Test plan

  • Swift: swift build and BUILDKITE=1 swift test — 81 tests pass, including the live badssl.com cases: testInvalidHTTPsFails (wrong.host.badssl.com, valid chain / wrong name → invalidSslError / certificateNotValidForName), testInvalidHttpsWithExceptionWorks (name exception gets the mismatched host past the handshake, then fails only as probablyNotAWordPressSite), testAllowAlternativeNamesStillValidatesChain (self-signed.badssl.com allow-listed by name → still rejected as invalidSslError — the The allowSSL server-trust override accepts the challenge without evaluating the trust #1512 regression), and testDisableCertificateValidationWorks (opt-out → self-signed accepted, discovery reaches the host). swift-format --strict clean.
  • Kotlin: :api:kotlin:compileKotlin, :compileIntegrationTestKotlin, and :detekt clean. Live against badssl.com, testDisableCertificateValidationWorks (self-signed accepted → ProbablyNotAWordPressSite) and testAllowedHostnamesStillValidatesCertificateChain (self-signed rejected as InvalidSslError despite allow-listing) pass.
  • Rust: cargo check -p wp_api_integration_tests --tests.
  • The Rust #[ignore]d login_spec_17_* cases hit live hosts and run in the dedicated soft-fail CI step (make test-rust-integration-remote-login), not locally. Reproduction: without the fix, allowAlternativeNames accepts self-signed.badssl.com; with it, that connection is rejected while wrong.host.badssl.com (valid chain, wrong name) is still accepted.

Changelog

Stacked on #1531 (issue-1508); the base retargets to trunk once that merges.

Fixes #1512.

@wpmobilebot

wpmobilebot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

XCFramework Build

This PR's XCFramework is available for testing. Add to your Package.swift:

.package(url: "https://github.com/automattic/wordpress-rs", branch: "pr-build/1527")

Built from d73dfaa

@jkmassel
jkmassel changed the base branch from trunk to issue-1508 August 7, 2026 23:15
@jkmassel
jkmassel marked this pull request as ready for review August 8, 2026 20:41
@jkmassel
jkmassel requested a review from crazytonyli August 8, 2026 20:41
@jkmassel jkmassel self-assigned this Aug 8, 2026
@jkmassel
jkmassel requested a review from oguzkocer August 8, 2026 20:42

@oguzkocer oguzkocer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkmassel Claude found some issues. I looked through them and they seemed reasonable to address and the first one seemed important enough to block the merge.

I usually combine my own review with Claude's to leave review manual feedback, but the list this time is long enough that sharing as-is felt more efficient. Having said that, some of these are lower value, so it's up to you whether to tackle all of them.

1527-009d7d39.md

Splits the ambiguous `allowSSL(altNames:forCommonName:)` into two explicit
opt-ins on both platforms and closes the #1512 trust bypass:

- `allowAlternativeNames(_:forCommonName:)` (Swift) / the Kotlin allow-list
  accept an otherwise-valid certificate whose name doesn't cover the host; the
  chain is still validated. Swift re-evaluates the chain under a hostname-relaxed
  `SecPolicyCreateSSL(true, nil)` policy before accepting, so a self-signed
  certificate copying a legitimate common name is rejected.
- `disableCertificateValidation(forHost:)` accepts any certificate for a named
  host the caller controls; Kotlin routes those hosts to a separate all-trusting
  client that refuses cross-host redirects.
- Kotlin classifies chain-validation failures as `InvalidSslError`.
- Allow-list matching is case-insensitive and the Kotlin opt-out state is guarded
  by a lock, matching Swift's `NSLock`.
- SSL-validation tests (all platforms) and the login spec point at badssl.com.

Fixes #1512.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The allowSSL server-trust override accepts the challenge without evaluating the trust

3 participants