Skip to content
Open
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
}
}
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
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)
}
}
2 changes: 1 addition & 1 deletion RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
27.2
-----
* [*] Stop the media picker from removing gallery images when you cancel it in the experimental editor [#25866]

* [*] Fix an issue where XML-RPC credentials for an https self-hosted site could be sent over insecure http [#25869]

27.1
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,83 @@ final class WordPressOrgXMLRPCValidatorTests: XCTestCase {
wait(for: [failure], timeout: 0.3)
}

func testItWillNotGuessXMLRPCOnHTTPForHTTPSInput() {
// Given
var schemes = Set<String>()
// Stub all, we only care about the URL schemes that are being tested.
stub(condition: { request -> Bool in
if let scheme = request.url?.scheme {
schemes.insert(scheme)
}
return true
}, response: { _ in
let error = NSError(domain: NSURLErrorDomain, code: NSURLErrorNotConnectedToInternet, userInfo: nil)
return HTTPStubsResponse(error: error)
})

// Unsecured ATS settings armed the http fallback before the fix (GHSA-qxpr-7v78-mh5g).
let validator = WordPressOrgXMLRPCValidator(makeUnsecuredAppTransportSecuritySettings())

// When
let expectation = self.expectation(description: "Wait for failure")
validator.guessXMLRPCURLForSite("https://example.com", userAgent: "", success: {
XCTFail("Unexpected result: \($0)")
expectation.fulfill()
}, failure: { _ in
expectation.fulfill()
})

wait(for: [expectation], timeout: 2.0)

// Then
XCTAssertEqual(schemes, ["https"])
}

func testItRejectsInsecureEndpointDiscoveredViaRSDForHTTPSInput() throws {
let responseInvalidPath = try XCTUnwrap(xmlrpcResponseInvalidPath)
stub(condition: isScheme("https") && isHost("www.apple.com") && isPath("/blog/xmlrpc.php")) { _ in
fixture(filePath: responseInvalidPath, status: 403, headers: nil)
}

stub(condition: isScheme("https") && isHost("www.apple.com") && isPath("/blog")) { _ in
let html = """
<!DOCTYPE html>
<html>
<head>
<link rel="EditURI" type="application/rsd+xml" title="RSD" href="http://www.apple.com/xmlrpc.php?rsd" />
<title>test site</title>
</head>
<body>hello world</body>
</html>
"""
return HTTPStubsResponse(data: html.data(using: .utf8)!, statusCode: 200, headers: nil)
}

// The plaintext endpoint answers with a valid methods list, but it must not be returned.
let responseListPath = try XCTUnwrap(
OHPathForFileInBundle("xmlrpc-response-list-methods.xml", Bundle.coreAPITestsBundle)
)
stub(condition: isScheme("http") && isHost("www.apple.com") && isPath("/xmlrpc.php")) { _ in
fixture(
filePath: responseListPath,
status: 200,
headers: [
"Content-Type": "application/xml"
]
)
}

let failure = self.expectation(description: "returns error")
let validator = WordPressOrgXMLRPCValidator(makeUnsecuredAppTransportSecuritySettings())
validator.guessXMLRPCURLForSite("https://www.apple.com/blog", userAgent: "test/1.0", success: {
XCTFail("Unexpected result: \($0)")
}) { error in
XCTAssertEqual(error as? WordPressOrgXMLRPCValidatorError, .insecureEndpoint)
failure.fulfill()
}
wait(for: [failure], timeout: 0.3)
}

let xmlrpcResponseInvalidPath = OHPathForFileInBundle(
"xmlrpc-response-invalid.html",
Bundle.coreAPITestsBundle
Expand Down
2 changes: 1 addition & 1 deletion WordPress/Classes/Utility/ZendeskUtils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ private extension ZendeskUtils {
// Get email address from remote profile
guard let username = blog.username,
let password = blog.password,
let xmlrpc = blog.xmlrpc,
let xmlrpc = blog.xmlrpcURL?.absoluteString,
let service = UsersService(username: username, password: password, xmlrpc: xmlrpc) else {
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,7 @@ - (void)validateLoginCredentials
[SVProgressHUD setDefaultMaskType:SVProgressHUDMaskTypeBlack];
[SVProgressHUD showWithStatus:NSLocalizedString(@"Authenticating", @"")];

NSURL *xmlRpcURL = [NSURL URLWithString:self.blog.xmlrpc];
NSURL *xmlRpcURL = self.blog.xmlrpcURL;
WordPressOrgXMLRPCApi *api = [[WordPressOrgXMLRPCApi alloc] initWithEndpoint:xmlRpcURL userAgent:[WPUserAgent wordPressUserAgent]];
__weak __typeof__(self) weakSelf = self;
[api checkCredentials:self.username password:self.password success:^(id __unused responseObject, NSHTTPURLResponse *__unused httpResponse) {
Expand Down