Skip to content

Commit 103e064

Browse files
authored
fix: parse Common-Name-less certificates and report their real hostnames (#1531)
* fix: parse Common-Name-less certificates and report their real hostnames A SAN-only leaf certificate (no subject Common Name) failed to parse, so a TLS name-mismatch error reported the wrong hostname — the intermediate CA's CN on Swift, an empty list on Kotlin. Make the subject and issuer `common_name` `Option<String>`, add `SslCertificateInfo.presented_hostnames()` (Common Name plus SANs, de-duplicated), and have both executors report the leaf's presented hostnames instead of only its Common Name. The Swift executor also parses the raw leaf directly rather than assuming it survived the chain's `compactMap`. Fixes #1508. * test: breadcrumb the #1498 coupling on the SAN-only SSL spec no-common-name.badssl.com's certificate is expired, so the failure is a bad-date one that currently surfaces as certificateNotValidForName. When #1498 remaps bad-date failures to genericSslError, this endpoint yields no presented hostnames and the assertion breaks; note to move it to a non-expired CN-less cert (local mock, #1208) then. Per @oguzkocer review. * test: prefer expect over unwrap in the ssl presented-hostnames test Matches the module's other tests and the repo convention. Per @oguzkocer review.
1 parent 0bb178a commit 103e064

6 files changed

Lines changed: 304 additions & 21 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2727
- **Internal:** Corrected `GET /all-domains/` fixtures that claimed subtypes the endpoint never returns (`site_redirect`, `domain_mapping`).
2828
- Kotlin: The request executor now classifies cancelling an in-flight request via `CancellableCall.cancel()` as `CancellationError` instead of `GenericError`, matching Swift's handling of `URLError.cancelled`. Whole-call `callTimeout` expiry is classified as `HttpTimeoutError` rather than being mistaken for a cancellation, and a `CancellationException` surfacing synchronously inside the executor (e.g. from an upload callback) is classified as `CancellationError` rather than flattened into a `GenericError` ([#1492](https://github.com/Automattic/wordpress-rs/issues/1492)).
2929
- **Internal:** Run clippy's `--tests` lint pass with `--jobs 1` to cap the Rust lint step's peak memory at ~4.7 GiB (down from ~9 GiB) on CI.
30+
- **BREAKING:** The `commonName` on `SslCertificateInfo` and its `issuer` changed from `String` to `Option<String>` (`String?` on Swift and Kotlin). A certificate that omits its subject Common Name — the direction CAs are moving, toward SAN-only certificates — now parses instead of failing outright, so callers reading `commonName` must handle its absence. `SslCertificateInfo` gained `presentedHostnames()`, which returns the Common Name (when present) followed by the Subject Alternative Names, de-duplicated; prefer it over `commonName` when reporting which names a certificate covers.
3031
- **Internal:** Build the Android JNI libraries with `cargo-ndk` instead of the `rust-android-gradle` Gradle plugin.
3132
- **Internal:** Upgraded the Android/Kotlin build to Android Gradle Plugin `9.3.0` / Gradle `9.5.0` (Kotlin `2.3.21`, `compileSdk` 36), migrating `api/android` to the AGP 9 variant APIs and splitting the example app into a `com.android.kotlin.multiplatform.library` shared module and a standalone `com.android.application` module.
3233
- **Internal:** Bumped `syn` from `2.0` to `3.0`, updating the proc-macro crates for its breaking changes.
@@ -53,6 +54,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5354
- **Internal:** Upgraded `reqwest` from `0.12` to `0.13` and removed the direct `hickory-resolver` dependency, moving `hickory-proto` to `0.26.1` to clear [RUSTSEC-2026-0119](https://rustsec.org/advisories/RUSTSEC-2026-0119.html) ([`GHSA-q2qq-hmj6-3wpp`](https://github.com/hickory-dns/hickory-dns/security/advisories/GHSA-q2qq-hmj6-3wpp)), a medium-severity O(n²) CPU-exhaustion DoS in DNS message encoding. Only the Rust `reqwest` request executor — used by the CLI, web tool, and integration tests — pulls in `hickory`; the shipping iOS/Android bindings don't compile `reqwest`, so they were never affected. `reqwest` 0.13 also drops the `native-tls`/`openssl` stack in favor of rustls-only, so the executor no longer links OpenSSL.
5455
- **Internal:** Bumped the transitive `rand` dependency to `0.9.3` and `0.8.6` to clear [RUSTSEC-2026-0097](https://rustsec.org/advisories/RUSTSEC-2026-0097.html) (`GHSA-cq8v-f236-94qc`), a low-severity unsoundness in `rand` 0.9.2 / 0.8.5. Lockfile-only; the affected code path (a custom `log` logger calling `rand::rng()` during reseed) is not exercised here.
5556
- **Internal:** Bumped `wp_rs_web`'s transitive `nanoid` dependency from `3.3.16` to `3.3.18` to clear [CVE-2026-67213](https://nvd.nist.gov/vuln/detail/CVE-2026-67213) ([GHSA-2v37-7h3g-55p8](https://github.com/advisories/GHSA-2v37-7h3g-55p8)), a denial-of-service via an infinite loop in `nanoid`'s `customAlphabet`/`customRandom` when called with a size of `0`. Lockfile-only; `nanoid` is pulled in only by Tailwind's build-time `postcss`, which never reaches the affected functions.
57+
- A SAN-only (Common-Name-less) leaf certificate is now parsed rather than dropped, so a TLS name-mismatch error reports the site certificate's own hostnames. Previously the whole parse failed on the missing Common Name: the Swift executor could then report an intermediate CA's Common Name (e.g. `R10`) as the presented hostname, and the Kotlin executor reported an empty list. Both executors now report the leaf certificate's Common Name plus its Subject Alternative Names, and the Swift executor no longer assumes the leaf survived parsing when picking the site certificate out of the chain.
5658

5759
## [0.6.0] - 2026-07-16
5860

native/kotlin/api/kotlin/src/integrationTest/kotlin/ApiUrlDiscoveryTest.kt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,33 @@ class ApiUrlDiscoveryTest {
283283
assertContains(presentedHostnames, "vanilla.wpmt.co")
284284
}
285285

286+
// `wrong.host.badssl.com` serves a valid `*.badssl.com` certificate on a host
287+
// it doesn't cover — a genuine name mismatch whose identities live in the SANs.
288+
// The old code reported only the Common Name; assert the `badssl.com` SAN is
289+
// now included too.
290+
//
291+
// The CN-less case (`no-common-name.badssl.com`) isn't exercised here: that
292+
// certificate is expired, which OkHttp raises as an `SSLHandshakeException` —
293+
// not the `SSLPeerUnverifiedException` that routes to certificate inspection —
294+
// and the inspection's re-connect would fail on the expiry regardless. The
295+
// Rust `parse_certificate` unit test covers the CN-less parse directly.
296+
@Test
297+
fun testNameMismatchReportsAllPresentedNames() = runTest {
298+
val reason = loginClient.apiDiscovery("https://wrong.host.badssl.com")
299+
.assertFailureFindApiRoot().getRequestExecutionErrorReason()
300+
assertInstanceOf(RequestExecutionErrorReason.InvalidSslError::class.java, reason)
301+
302+
val sslError = (reason as RequestExecutionErrorReason.InvalidSslError).reason
303+
assertInstanceOf(
304+
InvalidSslErrorReason.CertificateNotValidForName::class.java,
305+
sslError
306+
)
307+
308+
val presentedHostnames =
309+
(sslError as InvalidSslErrorReason.CertificateNotValidForName).presentedHostnames
310+
assertContains(presentedHostnames, "badssl.com")
311+
}
312+
286313
@Test // Spec Example 17 (with exception)
287314
fun testInvalidHttpsWithExceptionWorks() = runTest {
288315
val httpClient = WpHttpClient.DefaultHttpClient(emptyList())

native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,11 +356,15 @@ private fun RequestExecutionErrorReason.Companion.invalidSSLError(
356356

357357
try {
358358
// Certificate is parsed by the Rust shared implementation.
359+
// `serverCertificates` is leaf-first, so the site's certificate is the
360+
// first element. Report every hostname it presents (Common Name plus
361+
// SANs), not only its Common Name — a modern SAN-only certificate may
362+
// omit the Common Name entirely.
359363
val certificates = newConnection.serverCertificates.map { parseCertificate(it.encoded) }
360364
RequestExecutionErrorReason.InvalidSslError(
361365
reason = InvalidSslErrorReason.CertificateNotValidForName(
362366
hostname = requestUrl.host,
363-
presentedHostnames = listOfNotNull(certificates.first()?.commonName())
367+
presentedHostnames = certificates.firstOrNull()?.presentedHostnames() ?: emptyList()
364368
)
365369
)
366370
} finally {

native/swift/Sources/wordpress-api/SafeRequestExecutor.swift

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -197,10 +197,7 @@ public final class WpRequestExecutor: SafeRequestExecutor {
197197
for request: NetworkRequestContent
198198
) -> Result<WpNetworkResponse, RequestExecutionError> {
199199

200-
guard
201-
var peerCertificateChain = getPeerCertificateChain(error),
202-
!peerCertificateChain.isEmpty
203-
else {
200+
guard let siteCertificate = leafCertificate(from: error) else {
204201
return .failure(
205202
.RequestExecutionFailed(
206203
statusCode: nil,
@@ -212,16 +209,14 @@ public final class WpRequestExecutor: SafeRequestExecutor {
212209
)
213210
}
214211

215-
let siteCertificate = peerCertificateChain.remove(at: 0)
216-
217212
return .failure(
218213
.RequestExecutionFailed(
219214
statusCode: nil,
220215
redirects: executorDelegate.redirects(for: request.requestId()),
221216
reason: RequestExecutionErrorReason.invalidSslError(
222217
reason: .certificateNotValidForName(
223218
hostname: URL(string: request.url())?.host ?? "unknown host",
224-
presentedHostnames: [siteCertificate.commonName()]
219+
presentedHostnames: siteCertificate.presentedHostnames()
225220
)
226221
),
227222
requestUrl: request.url(),
@@ -346,19 +341,28 @@ public final class WpRequestExecutor: SafeRequestExecutor {
346341
)
347342
}
348343

349-
private func getPeerCertificateChain(_ error: Error) -> [SslCertificateInfo]? {
344+
/// Parse the site (leaf) certificate out of a failed TLS handshake.
345+
///
346+
/// The peer certificate chain is leaf-first, so the site's certificate is
347+
/// element 0 of the *raw* chain. We parse that element directly rather than
348+
/// parsing the whole chain and taking element 0 of whatever survived: a leaf
349+
/// we can't parse must degrade to `genericSslError`, never silently promote an
350+
/// intermediate CA's certificate into the site's position and report the CA's
351+
/// name (e.g. `R10`) as the presented hostname.
352+
private func leafCertificate(from error: Error) -> SslCertificateInfo? {
350353
#if os(Linux) // Linux doesn't support `SecCertificate`
351-
return []
354+
return nil
352355
#else
353356
// The certificate chain the server presented during the failed TLS handshake.
354357
guard
355358
let trust = (error as? URLError)?.failureURLPeerTrust,
356-
let certChainArray = SecTrustCopyCertificateChain(trust) as? [SecCertificate]
359+
let certChainArray = SecTrustCopyCertificateChain(trust) as? [SecCertificate],
360+
let leaf = certChainArray.first
357361
else {
358362
return nil
359363
}
360364

361-
return certChainArray.compactMap { parseCertificate(data: SecCertificateCopyData($0) as Data) }
365+
return parseCertificate(data: SecCertificateCopyData(leaf) as Data)
362366
#endif
363367
}
364368
}
@@ -396,8 +400,12 @@ private final class RequestExecutorDelegate:
396400
}
397401

398402
private func alternateNames(forCertificate cert: SslCertificateInfo) -> Set<String> {
399-
lock.withLock {
400-
additionalAlternativeNames[cert.commonName()] ?? []
403+
// The allowlist is keyed on the certificate's Common Name, so a SAN-only
404+
// certificate that omits its CN can't be matched here — there's no key to
405+
// look it up under. Fall through to default handling in that case.
406+
guard let commonName = cert.commonName() else { return [] }
407+
return lock.withLock {
408+
additionalAlternativeNames[commonName] ?? []
401409
}
402410
}
403411

native/swift/Tests/wordpress-api/LoginTests.swift

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,12 @@ class LoginTests {
317317
}
318318

319319
#expect(hostname == "wordpress-1315525-4803651.cloudwaysapps.com")
320-
#expect(presentedHostnames == ["vanilla.wpmt.co"])
320+
// The presented names are the leaf certificate's Common Name and
321+
// its SANs, not only the CN. Assert the certificate's identity is
322+
// reported and the requested host is absent, without pinning the
323+
// exact SAN list (it changes when the certificate is reissued).
324+
#expect(presentedHostnames.contains("vanilla.wpmt.co"))
325+
#expect(!presentedHostnames.contains("wordpress-1315525-4803651.cloudwaysapps.com"))
321326
#endif
322327

323328
return true
@@ -341,6 +346,97 @@ class LoginTests {
341346
_ = try await self.client.details(ofSite: "https://vanilla1.wpmt.co")
342347
}
343348

349+
/// Regression for a SAN-only (Common-Name-less) leaf certificate — see #1508.
350+
/// `no-common-name.badssl.com` serves a certificate whose subject carries no
351+
/// Common Name and a single SAN. Before the fix the leaf failed to parse, the
352+
/// `compactMap` dropped it, and element 0 of the survivors — the issuer CA —
353+
/// was reported, so `presentedHostnames` was the CA's name (`COMODO ...`)
354+
/// rather than the site's.
355+
@Test("SAN-only certificate reports its SAN, not the issuer CA")
356+
func testCommonNameLessCertificateReportsSan() async throws {
357+
await #expect(
358+
performing: {
359+
_ = try await self.client.details(ofSite: "https://no-common-name.badssl.com")
360+
},
361+
throws: { error in
362+
let reason = try #require(try self.getRequestExecutionErrorReason(from: error))
363+
364+
guard case .invalidSslError(let underlyingReason) = reason else {
365+
Issue.record("The transport error must be `invalidSslError`")
366+
return false
367+
}
368+
369+
#if os(watchOS) // watchOS doesn't make the underlying certificate available to us
370+
guard case .genericSslError = underlyingReason else {
371+
Issue.record("The underlying error must be `genericSslError`")
372+
return false
373+
}
374+
#else
375+
// Breadcrumb (#1498): this endpoint's certificate is expired, so the failure is a
376+
// bad-date one that the executor currently reports as `certificateNotValidForName`.
377+
// Once #1498 remaps bad-date failures to `genericSslError`, this endpoint yields no
378+
// presented hostnames and the assertion below breaks — move it to a non-expired
379+
// Common-Name-less certificate then (e.g. a local mock, #1208). The parsing itself
380+
// is already covered #1498-proof by the Rust `ssl` unit tests.
381+
guard case .certificateNotValidForName(_, let presentedHostnames) = underlyingReason else {
382+
Issue.record("The underlying error must be `certificateNotValidForName`")
383+
return false
384+
}
385+
386+
// The leaf carries no Common Name and exactly one SAN, so that SAN
387+
// is the entire presented-hostname list. The bug reported the
388+
// COMODO issuer CA's name here instead.
389+
#expect(presentedHostnames == ["no-common-name.badssl.com"])
390+
#endif
391+
392+
return true
393+
}
394+
)
395+
}
396+
397+
/// Regression for the SANs half of the same payload — see #1507.
398+
/// `wrong.host.badssl.com` serves a valid `*.badssl.com` certificate on a host
399+
/// it doesn't cover, so it's a genuine name mismatch whose identities live in
400+
/// the SANs. `presentedHostnames` must include them, not only the Common Name.
401+
@Test("Name-mismatch certificate reports its SANs, not only its CN")
402+
func testNameMismatchReportsAllPresentedNames() async throws {
403+
await #expect(
404+
performing: {
405+
_ = try await self.client.details(ofSite: "https://wrong.host.badssl.com")
406+
},
407+
throws: { error in
408+
let reason = try #require(try self.getRequestExecutionErrorReason(from: error))
409+
410+
guard case .invalidSslError(let underlyingReason) = reason else {
411+
Issue.record("The transport error must be `invalidSslError`")
412+
return false
413+
}
414+
415+
#if os(watchOS) // watchOS doesn't make the underlying certificate available to us
416+
guard case .genericSslError = underlyingReason else {
417+
Issue.record("The underlying error must be `genericSslError`")
418+
return false
419+
}
420+
#else
421+
guard case .certificateNotValidForName(let hostname, let presentedHostnames) = underlyingReason else {
422+
Issue.record("The underlying error must be `certificateNotValidForName`")
423+
return false
424+
}
425+
426+
#expect(hostname == "wrong.host.badssl.com")
427+
// The certificate is for `*.badssl.com` with SANs `*.badssl.com`
428+
// and `badssl.com`. The old code reported only the CN, so the
429+
// `badssl.com` SAN is the discriminator that proves SANs are now
430+
// included.
431+
#expect(presentedHostnames.contains("badssl.com"))
432+
#expect(presentedHostnames.contains("*.badssl.com"))
433+
#endif
434+
435+
return true
436+
}
437+
)
438+
}
439+
344440
@Test("Cancel API discovery process")
345441
func testCancellation() async throws {
346442
let task = Task { [client] in

0 commit comments

Comments
 (0)