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
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import Foundation

extension Blog {

/// The XML-RPC endpoint URL to use for network requests.
///
/// Older app versions could silently downgrade the discovered XML-RPC endpoint to
/// http for an https site and persist it, so later XML-RPC traffic and the
/// credentials it carries crossed plaintext (GHSA-qxpr-7v78-mh5g). For such a site
/// this returns the https-upgraded endpoint, so a client built from it starts every
/// request over https. The persisted `xmlrpc` value and the Keychain entry keyed by
/// it are left untouched (so credential lookups still resolve); only the request
/// endpoint is upgraded, at the point a client is built. Every credential-bearing
/// XML-RPC client must be constructed from this, not from the raw `xmlrpc` string.
///
/// This upgrades only the initial endpoint. It does not stop an https-to-http
/// redirect issued by the server during a request; blocking that in the XML-RPC
/// client is tracked separately.
@objc public var xmlrpcURL: URL? {
guard let xmlrpc, let endpoint = URL(string: xmlrpc) else { return nil }
// URL schemes are case-insensitive, so compare normalized schemes and rewrite
// through URL components: an http endpoint (in any spelling) for an https site
// is upgraded to https; everything else is returned unchanged.
guard endpoint.scheme?.lowercased() == "http",
let siteAddress = url, URL(string: siteAddress)?.scheme?.lowercased() == "https",
var components = URLComponents(url: endpoint, resolvingAgainstBaseURL: false)
else {
return endpoint
}
components.scheme = "https"
return components.url ?? endpoint
}
}
28 changes: 28 additions & 0 deletions Modules/Sources/WordPressData/Swift/Blog+SelfHostedRestApi.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import Foundation

extension Blog {

/// The WordPress REST API root URL to use for self-hosted network requests.
///
/// Prefers `restApiRootURL` — the root observed during REST API discovery, which is
/// served over https for an https site — over a URL derived from the raw `xmlrpc`
/// string. An older app version could silently downgrade and persist an http `xmlrpc`
/// endpoint for an https site (GHSA-qxpr-7v78-mh5g), and `url(withPath:)` copies that
/// scheme verbatim, so a REST client built from it would carry the application password
/// (sent as a Basic auth header) over plaintext http.
///
/// `restApiRootURL` is written together with the application token (see
/// `ApplicationPasswordRepository.assign` and `Blog.createRestApiBlog`), so it is always
/// present when the token is. That makes this a discovery-backed choice rather than a
/// scheme-rewriting guess: an intentionally-http site is left on http (its discovered
/// root is http), and an https site uses the https root that discovery observed.
///
/// Falls back to the `xmlrpc`-derived `wp-json/` URL only when no discovered root was
/// persisted (legacy XML-RPC sign-ins), leaving behavior unchanged for those sites.
public var selfHostedRestApiRootURL: URL? {
if let restApiRootURL, let url = URL(string: restApiRootURL) {
return url
}
return url(withPath: "wp-json/").flatMap { URL(string: $0) }
}
}
12 changes: 10 additions & 2 deletions Modules/Sources/WordPressData/Swift/Blog.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,25 @@ public class Blog: NSManagedObject {
// MARK: - Non-Core Data Properties

private var _xmlrpcApi: WordPressOrgXMLRPCApi?
private var _xmlrpcApiEndpoint: URL?
private var _selfHostedSiteRestApi: WordPressOrgRestApi?

@objc public var xmlrpcApi: WordPressOrgXMLRPCApi? {
get {
if _xmlrpcApi == nil, let endpoint = xmlrpc.flatMap(URL.init(string:)) {
_xmlrpcApi = WordPressOrgXMLRPCApi(endpoint: endpoint, userAgent: WPUserAgent.wordPress())
let endpoint = xmlrpcURL
// The endpoint depends on both `xmlrpc` and `url` (via `xmlrpcURL`), and
// `url` can change without clearing this cache (including through a merge
// from another Core Data context). Rebuild whenever the computed endpoint
// changes so a cached client never keeps sending to a stale endpoint.
if _xmlrpcApiEndpoint != endpoint {
_xmlrpcApi = endpoint.map { WordPressOrgXMLRPCApi(endpoint: $0, userAgent: WPUserAgent.wordPress()) }
_xmlrpcApiEndpoint = endpoint
}
return _xmlrpcApi
}
set {
_xmlrpcApi = newValue
_xmlrpcApiEndpoint = newValue == nil ? nil : xmlrpcURL
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ private func apiBase(blog: Blog) -> URL? {
return nil
}

guard let urlString = blog.url(withPath: "wp-json/") else {
return nil
}

return URL(string: urlString)
// Prefer the REST root observed during discovery over one derived from the raw
// `xmlrpc` string, which an older app version could have persisted as http for an
// https site (GHSA-qxpr-7v78-mh5g). See `Blog.selfHostedRestApiRootURL`.
return blog.selfHostedRestApiRootURL
}

extension WordPressOrgRestApi {
Expand Down
27 changes: 23 additions & 4 deletions Modules/Sources/WordPressKit/WordPressOrgXMLRPCValidator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import Foundation
case blocked = 405 // Server returned a 405 error while reading xmlrpc file
case invalid // Doesn't look to be valid XMLRPC Endpoint.
case xmlrpc_missing // site contains RSD link but XML-RPC information is missing
case insecureEndpoint // The endpoint resolved to plain http for a site that was requested over https

public var localizedDescription: String {
switch self {
Expand All @@ -34,6 +35,11 @@ import Foundation
return NSLocalizedString("Couldn't connect. We received a 403 error when trying to access your site's XMLRPC endpoint. The app needs that in order to communicate with your site. Please contact your hosting provider to solve this problem.", comment: "Message to show to user when he tries to add a self-hosted site but the host returned a 403 error, meaning that the access to the /xmlrpc.php file is forbidden.")
case .xmlrpc_missing:
return NSLocalizedString("Couldn't connect. Required XML-RPC methods are missing on the server. Please contact your hosting provider to solve this problem.", comment: "Message to show to user when he tries to add a self-hosted site with RSD link present, but xmlrpc is missing.")
case .insecureEndpoint:
return NSLocalizedString(
"Couldn't establish a secure connection to your site's XML-RPC endpoint.",
comment: "Error message shown when the discovered XML-RPC endpoint is not served over HTTPS even though the site address uses HTTPS."
)
}
}
}
Expand Down Expand Up @@ -95,15 +101,28 @@ open class WordPressOrgXMLRPCValidator: NSObject {
sitesToTry.append(site.replacingOccurrences(of: "http://", with: "https://"))
} else if site.hasPrefix("https://") {
sitesToTry.append(site)
if !secureAccessOnly {
sitesToTry.append(site.replacingOccurrences(of: "https://", with: "http://"))
}
} else {
failure(WordPressOrgXMLRPCValidatorError.invalidScheme as NSError)
return
}

tryGuessXMLRPCURLForSites(sitesToTry, userAgent: userAgent, success: success, failure: failure)
// Never hand back a plaintext endpoint when the caller asked for a secure site.
// Besides the (removed) http probe candidate, redirects and RSD discovery can
// also resolve to http:// for an https:// input (GHSA-qxpr-7v78-mh5g).
let validatedSuccess: (URL) -> Void
if site.hasPrefix("https://") {
validatedSuccess = { xmlrpcURL in
if xmlrpcURL.scheme?.lowercased() == "https" {
success(xmlrpcURL)
} else {
failure(WordPressOrgXMLRPCValidatorError.insecureEndpoint as NSError)
}
}
} else {
validatedSuccess = success
}

tryGuessXMLRPCURLForSites(sitesToTry, userAgent: userAgent, success: validatedSuccess, failure: failure)
}

/// Helper for `guessXMLRPCURLForSite(_:userAgent:success:failure)`
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import CoreData
import Testing
@testable import WordPressData

@MainActor
struct BlogInsecureXMLRPCEndpointTests {
private let contextManager = ContextManager.forTesting()

private func makeBlog(url: String?, xmlrpc: String?) -> Blog {
let blog = BlogBuilder(contextManager.mainContext, dotComID: nil).build()
blog.account = nil
blog.url = url
blog.xmlrpc = xmlrpc
return blog
}

@Test func upgradesHTTPEndpointForHTTPSSite() {
let blog = makeBlog(url: "https://example.com", xmlrpc: "http://example.com/xmlrpc.php")
#expect(blog.xmlrpcURL?.absoluteString == "https://example.com/xmlrpc.php")
}

@Test func leavesSecureEndpointUnchanged() {
let blog = makeBlog(url: "https://example.com", xmlrpc: "https://example.com/xmlrpc.php")
#expect(blog.xmlrpcURL?.absoluteString == "https://example.com/xmlrpc.php")
}

@Test func leavesHTTPSiteEndpointUnchanged() {
// The site itself is http (user-asserted), so the endpoint is left as-is.
let blog = makeBlog(url: "http://example.com", xmlrpc: "http://example.com/xmlrpc.php")
#expect(blog.xmlrpcURL?.absoluteString == "http://example.com/xmlrpc.php")
}

@Test func returnsNilWhenNoEndpoint() {
let blog = makeBlog(url: "https://example.com", xmlrpc: nil)
#expect(blog.xmlrpcURL == nil)
}

@Test func upgradesMixedCaseHTTPEndpoint() {
// URL schemes are case-insensitive, so an uppercase http scheme must still upgrade.
let blog = makeBlog(url: "https://example.com", xmlrpc: "HTTP://example.com/xmlrpc.php")
#expect(blog.xmlrpcURL?.scheme == "https")
#expect(blog.xmlrpcURL?.host == "example.com")
#expect(blog.xmlrpcURL?.path == "/xmlrpc.php")
}

@Test func upgradesForMixedCaseHTTPSSite() {
let blog = makeBlog(url: "HTTPS://example.com", xmlrpc: "http://example.com/xmlrpc.php")
#expect(blog.xmlrpcURL?.absoluteString == "https://example.com/xmlrpc.php")
}

@Test func rebuildsCachedClientWhenEndpointChanges() throws {
// Realize the client while the site is http (no upgrade), then flip the site
// address to https. The cached client must be rebuilt for the upgraded endpoint
// rather than keep sending to the stale http one.
let blog = makeBlog(url: "http://example.com", xmlrpc: "http://example.com/xmlrpc.php")
let httpClient = try #require(blog.xmlrpcApi)

blog.url = "https://example.com"

let upgradedClient = try #require(blog.xmlrpcApi)
#expect(upgradedClient !== httpClient)
}

@Test func reusesCachedClientWhenEndpointUnchanged() throws {
let blog = makeBlog(url: "https://example.com", xmlrpc: "http://example.com/xmlrpc.php")
let first = try #require(blog.xmlrpcApi)
let second = try #require(blog.xmlrpcApi)
#expect(first === second)
}
}
44 changes: 44 additions & 0 deletions Modules/Tests/WordPressDataTests/BlogSelfHostedRestApiTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import CoreData
import Testing
@testable import WordPressData

@MainActor
struct BlogSelfHostedRestApiTests {
private let contextManager = ContextManager.forTesting()

private func makeBlog(url: String?, xmlrpc: String?, restApiRootURL: String?) -> Blog {
let blog = BlogBuilder(contextManager.mainContext, dotComID: nil).build()
blog.account = nil
blog.url = url
blog.xmlrpc = xmlrpc
blog.restApiRootURL = restApiRootURL
return blog
}

@Test func prefersDiscoveredHTTPSRootOverDowngradedXMLRPCEndpoint() {
// An older app version could persist an http xmlrpc endpoint for an https site; the
// https REST root observed during discovery must win so the application password is
// never sent over plaintext http.
let blog = makeBlog(
url: "https://example.com",
xmlrpc: "http://example.com/xmlrpc.php",
restApiRootURL: "https://example.com/wp-json/"
)
#expect(blog.selfHostedRestApiRootURL?.absoluteString == "https://example.com/wp-json/")
}

@Test func fallsBackToXMLRPCDerivedRootWhenNoDiscoveredRoot() {
// Legacy XML-RPC sign-ins never persisted a REST root, so behavior is unchanged.
let blog = makeBlog(
url: "https://example.com",
xmlrpc: "https://example.com/xmlrpc.php",
restApiRootURL: nil
)
#expect(blog.selfHostedRestApiRootURL?.absoluteString == "https://example.com/wp-json/")
}

@Test func returnsNilWhenNeitherRootIsAvailable() {
let blog = makeBlog(url: "https://example.com", xmlrpc: nil, restApiRootURL: nil)
#expect(blog.selfHostedRestApiRootURL == nil)
}
}
2 changes: 2 additions & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

27.2
-----
* [*] Fix an issue where an https self-hosted site's application password could be sent over insecure http when using its REST API [#25914]
* [*] Custom Post Types: Make custom post types with REST API and editor support available in My Site [#25849]
* [*] Stop the media picker from removing gallery images when you cancel it in the experimental editor [#25866]
* [*] Stats: Fix the screen getting stuck on a loading indicator for self-hosted sites that are not connected to Jetpack [#25858]
* [*] Fix an issue where XML-RPC credentials for an https self-hosted site could be sent over insecure http [#25869]
* [*] Stats: Fix an issue where new post may not appear in the Subscribers -> Emails card [#25895]
* [*] [internal] Experimental Gutenberg editor: add a per-site "Use Third-Party Blocks (Beta)" toggle to Site Settings, matching Android [#25889]
* [*] Fixes an isuse where back up logs incorrectly show up in some Personal plan sites
Expand Down
Loading