Automattic / Automattic/wordpress-rs

Every TLS failure with a peer certificate chain becomes `CertificateNotValidForName` — expired, self-signed, and unknown-root included

Open
#1,498 0 comments 0 reactions 0 assignees View on GitHub
Bug Error Handling Swift
Dominant language
Rust
Stars
36
Forks
5
Avg merge
17h 30m
Merged PRs (30d)
43

Description

> Split out from Automattic/wordpress-rs#1497 — *Swift executor URLSession error audit*. Section A — Converted to the wrong Rust error.
>
> File references are to `native/swift/Sources/wordpress-api/SafeRequestExecutor.swift` unless noted; `:NNN` line numbers were verified against `fix/converge-executor-error-classification`.

**Severity: high.** `errorIsHttpsError` (`:233-246`) matches five codes — `.secureConnectionFailed`, `.serverCertificateUntrusted`, `.serverCertificateHasBadDate`, `.serverCertificateNotYetValid`, `.serverCertificateHasUnknownRoot` — but `handleHttpsError` (`:172-208`) never inspects `urlError.code`: chain present → `.certificateNotValidForName`, chain missing → `.genericSslError`. An expired or self-signed certificate issued for the **correct** hostname — the common misconfigured self-hosted case — is reported as a hostname mismatch, often with `hostname == presentedHostnames[0]`, a self-contradictory payload. ❌

reqwest reserves `CertificateNotValidForName` for rustls's actual `NotValidForNameContext` and maps every other TLS error away from it (`wp_api/src/reqwest_request_executor.rs:230-247`). ✅

This also compounds with Automattic/wordpress-rs#1512: an app seeing `certificateNotValidForName` may offer the `allowSSL(altNames:forCommonName:)` remediation (`:75-77`), steering users into trusting an expired/untrusted certificate they believed was a mere name mismatch.

**Fix:** map `.serverCertificateHasBadDate` / `.serverCertificateNotYetValid` / `.serverCertificateHasUnknownRoot` to `.genericSslError` (they are by definition not name problems). For the remaining two codes, only claim `certificateNotValidForName` when the request host is genuinely absent from the certificate's CN + SANs — `SslCertificateInfo` already exposes both (`wp_api/src/ssl.rs:73-79`).

**Caveat:** genuine name mismatches surface as `.serverCertificateUntrusted` on Darwin, so a per-code switch alone would break the existing mismatch test (`LoginTests.swift`, `testInvalidHTTPsFails`) — the host-vs-names comparison is required to keep true mismatches while fixing the false positives.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in native/swift/Sources/wordpress-api/SafeRequestExecutor.swift, reading errorIsHttpsError and handleHttpsError at the cited lines, then compare the Rust mapping in wp_api/src/reqwest_request_executor.rs and SslCertificateInfo in wp_api/src/ssl.rs. Run the existing LoginTests.swift testInvalidHTTPsFails and inspect the certificate-related cases. Done means genuine hostname mismatches retain their classification while expired, not-yet-valid, unknown-root, and other non-name TLS failures are reported correctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, swift
Domain
api, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.