diff --git a/Tests/KeystoneTests/Tests/Login/URL+InsecureConnectionTests.swift b/Tests/KeystoneTests/Tests/Login/URL+InsecureConnectionTests.swift new file mode 100644 index 000000000000..841eba2a3ecb --- /dev/null +++ b/Tests/KeystoneTests/Tests/Login/URL+InsecureConnectionTests.swift @@ -0,0 +1,41 @@ +import Foundation +import Testing + +@testable import WordPress + +struct URLInsecureConnectionTests { + @Test(arguments: [ + "https://example.com", + "https://example.com:8443/wp-json", + "HTTPS://EXAMPLE.COM" + ]) + func secureURLs(_ string: String) throws { + let url = try #require(URL(string: string)) + #expect(!url.isInsecureConnection) + } + + @Test(arguments: [ + "http://example.com", + "HTTP://EXAMPLE.COM", + "http://example.com:8080/wp-json", + "http://mymac.local", + "http://mysite.test", + "http://192.168.1.10:8881" + ]) + func insecureURLs(_ string: String) throws { + let url = try #require(URL(string: string)) + #expect(url.isInsecureConnection) + } + + @Test(arguments: [ + "http://localhost", + "http://localhost:8881/wp-admin", + "http://LOCALHOST:8881", + "http://127.0.0.1:8881", + "http://[::1]:8881" + ]) + func loopbackURLsAreExempt(_ string: String) throws { + let url = try #require(URL(string: string)) + #expect(!url.isInsecureConnection) + } +} diff --git a/Tests/KeystoneTests/Tests/Utility/ApplicationPasswordsRepositoryTests.swift b/Tests/KeystoneTests/Tests/Utility/ApplicationPasswordsRepositoryTests.swift index b621bbc9ee1d..29c4158c8600 100644 --- a/Tests/KeystoneTests/Tests/Utility/ApplicationPasswordsRepositoryTests.swift +++ b/Tests/KeystoneTests/Tests/Utility/ApplicationPasswordsRepositoryTests.swift @@ -386,6 +386,183 @@ class ApplicationPasswordsRepositoryTests { let password = await password(of: blog) #expect(password == uuid) } + + @Test + func insecureSiteDoesNotTransmitCredentials() async throws { + defer { HTTPStubs.removeAllStubs() } + + let host = "insecure.example.com" + stub(condition: isHost(host)) { _ in + Issue.record("No request should be sent to an insecure site") + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + } + + let blog = try await coreDataStack.performAndSave { context in + let blog = Blog(context: context) + blog.url = "http://\(host)" + blog.xmlrpc = "http://\(host)/xmlrpc.php" + blog.username = "demo" + blog.password = "pass" + return TaggedManagedObjectID(blog) + } + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + await #expect(throws: ApplicationPasswordRepositoryError.insecureConnection) { + try await repository.createPasswordIfNeeded(for: blog) + } + } + + @Test + func insecureRestApiRootDoesNotTransmitCredentials() async throws { + defer { HTTPStubs.removeAllStubs() } + + let secureHost = "secure.example.com" + let insecureHost = "insecure-root.example.com" + stub(condition: isHost(secureHost) || isHost(insecureHost)) { _ in + Issue.record("No request should be sent when a credential destination is insecure") + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + } + + let blog = try await coreDataStack.performAndSave { context in + let blog = Blog(context: context) + blog.url = "https://\(secureHost)" + blog.xmlrpc = "https://\(secureHost)/xmlrpc.php" + blog.restApiRootURL = "http://\(insecureHost)/wp-json" + blog.username = "demo" + blog.password = "pass" + return TaggedManagedObjectID(blog) + } + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + await #expect(throws: ApplicationPasswordRepositoryError.insecureConnection) { + try await repository.createPasswordIfNeeded(for: blog) + } + } + + @Test + func insecureXMLRPCDerivedDestinationDoesNotTransmitCredentials() async throws { + defer { HTTPStubs.removeAllStubs() } + + // The site URL is https, so validation proceeds through an unauthenticated discovery over + // https. But login_url, admin_url, and the wp-json base are derived from the http xmlrpc + // endpoint, so self-hosted password creation is rejected before any credential-bearing + // request reaches the insecure host. + let secureHost = "secure-site.example.com" + let insecureHost = "insecure-xmlrpc.example.com" + stubApiDiscovery(siteHost: secureHost) + stub(condition: isHost(insecureHost)) { _ in + Issue.record("No request should be sent to an xmlrpc-derived insecure destination") + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + } + + let blog = try await coreDataStack.performAndSave { context in + let blog = Blog(context: context) + blog.url = "https://\(secureHost)" + blog.xmlrpc = "http://\(insecureHost)/xmlrpc.php" + blog.username = "demo" + blog.password = "pass" + return TaggedManagedObjectID(blog) + } + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + await #expect(throws: ApplicationPasswordRepositoryError.insecureConnection) { + try await repository.createPasswordIfNeeded(for: blog) + } + } + + @Test + func validTokenValidatesOverHttpsDespiteInsecureXMLRPC() async throws { + defer { HTTPStubs.removeAllStubs() } + + // A valid stored token validates over the https site URL and REST root. The http + // xmlrpc-derived login/admin/wp-json urls are only needed to create a password, so an + // existing token must not be blocked by them, and creation must not be attempted. + let secureHost = "valid-token.example.com" + let insecureHost = "insecure-xmlrpc.example.com" + stubCurrentApplicationPassword(host: secureHost) + stub(condition: isHost(insecureHost)) { _ in + Issue.record("No request should reach the insecure xmlrpc-derived host") + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + } + + let blog = try await coreDataStack.performAndSave { [keychain] context in + let blog = Blog(context: context) + blog.url = "https://\(secureHost)" + blog.xmlrpc = "http://\(insecureHost)/xmlrpc.php" + blog.restApiRootURL = "https://\(secureHost)/wp-json" + blog.username = "demo" + blog.password = "pass" + try blog.setApplicationToken("valid token", using: keychain) + return TaggedManagedObjectID(blog) + } + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + try await repository.createPasswordIfNeeded(for: blog) + + let password = await password(of: blog) + #expect(password == "valid token") + } + + @Test + func discoveredInsecureRestApiRootDoesNotTransmitCredentials() async throws { + defer { HTTPStubs.removeAllStubs() } + + let host = "discovered-insecure.example.com" + let blog = try await coreDataStack.performAndSave { context in + let blog = Blog(context: context) + blog.url = "https://\(host)" + blog.xmlrpc = "https://\(host)/xmlrpc.php" + blog.username = "demo" + blog.password = "pass" + return TaggedManagedObjectID(blog) + } + + // The blog has no stored REST root, so discovery resolves one. Discovery advertises an http + // root, and no credential-bearing request may follow. + stubApiDiscoveryWithInsecureRoot(siteHost: host) + stub( + condition: isPath("/wp-json/wp/v2/users/me") + || isPath("/wp-json/wp/v2/users/me/application-passwords") + ) { _ in + Issue.record("No credentials should be sent to a discovered insecure REST root") + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + } + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + await #expect(throws: ApplicationPasswordRepositoryError.insecureConnection) { + try await repository.createPasswordIfNeeded(for: blog) + } + + // The rejected http root must not be persisted, or other consumers could later use it. + let storedRoot = await coreDataStack.performQuery { context in + (try? context.existingObject(with: blog))?.restApiRootURL + } + #expect(storedRoot == nil) + } + + @Test + func loopbackHttpSiteCreatesPassword() async throws { + defer { HTTPStubs.removeAllStubs() } + + let blog = try await coreDataStack.performAndSave { context in + let blog = Blog(context: context) + blog.url = "http://localhost:8881" + blog.xmlrpc = "http://localhost:8881/xmlrpc.php" + blog.username = "demo" + blog.password = "pass" + return TaggedManagedObjectID(blog) + } + + stubApiDiscovery(siteHost: "localhost") + stubSelfHostedSiteWpV2GetUser() + stubSelfHostedSiteCreateApplicationPassword(host: "localhost", password: "abcd efgh") + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + try await repository.createPasswordIfNeeded(for: blog) + + let password = await password(of: blog) + #expect(password == "abcd efgh") + } } // MARK: - Helpers @@ -758,6 +935,39 @@ private extension ApplicationPasswordsRepositoryTests { } } + func stubApiDiscoveryWithInsecureRoot(siteHost: String) { + stub(condition: isHost(siteHost) && isPath("/")) { _ in + HTTPStubsResponse( + data: "homepage".data(using: .utf8)!, + statusCode: 200, + headers: ["Link": "; rel=\"https://api.w.org/\""] + ) + } + stub(condition: isHost(siteHost) && isPath("/wp-json")) { _ in + let json = """ + { + "name": "Site", + "description": "", + "url": "http://\(siteHost)", + "home": "http://\(siteHost)", + "gmt_offset": "0", + "timezone_string": "", + "namespaces": ["wp/v2"], + "authentication": { + "application-passwords": { + "endpoints": { + "authorization": "http://\(siteHost)/wp-admin/authorize-application.php" + } + } + }, + "routes": {}, + "_links": {} + } + """ + return HTTPStubsResponse(data: json.data(using: .utf8)!, statusCode: 200, headers: nil) + } + } + func stubApiDiscoveryFailure(siteHost: String) { stub(condition: isHost(siteHost) && isPath("/")) { _ in HTTPStubsResponse(data: "homepage".data(using: .utf8)!, statusCode: 200, headers: nil) diff --git a/WordPress/Classes/Login/SelfHostedSiteAuthenticator.swift b/WordPress/Classes/Login/SelfHostedSiteAuthenticator.swift index 7e0029a0c859..355b9e2922d0 100644 --- a/WordPress/Classes/Login/SelfHostedSiteAuthenticator.swift +++ b/WordPress/Classes/Login/SelfHostedSiteAuthenticator.swift @@ -10,6 +10,7 @@ import WordPressShared import BuildSettingsKit import SVProgressHUD import WordPressSharedUI +import WordPressUI struct SelfHostedSiteAuthenticator { @@ -193,7 +194,12 @@ struct SelfHostedSiteAuthenticator { { credentials = parsed } else { - credentials = try await authenticate(details: details, from: viewController) + let authenticated = try await authenticate(details: details, from: viewController) + credentials = WpApiApplicationPasswordDetails( + siteUrl: details.sanitizedSiteUrl(authenticated.siteUrl), + userLogin: authenticated.userLogin, + password: authenticated.password + ) } let apiRootURL = details.apiRootUrl.asURL() @@ -211,6 +217,36 @@ struct SelfHostedSiteAuthenticator { } } + @MainActor + private func confirmInsecureConnection(host: String, from viewController: UIViewController) async -> Bool { + // The continuation is resumed only by the alert actions, matching the app's existing + // alert-to-async bridging. The alert is app-modal, so the only way it resolves without an + // action is the login flow being torn down while it is on screen. We accept that narrow edge + // case (a suspended continuation on an already-cancelled task) rather than add cancellation + // plumbing here. + await withCheckedContinuation { continuation in + let alert = UIAlertController( + title: Strings.insecureConnectionTitle, + message: Strings.insecureConnectionMessage(host: host), + preferredStyle: .alert + ) + alert.addAction( + UIAlertAction(title: SharedStrings.Button.cancel, style: .cancel) { _ in + continuation.resume(returning: false) + } + ) + alert.addAction( + UIAlertAction(title: Strings.insecureConnectionContinue, style: .destructive) { _ in + continuation.resume(returning: true) + } + ) + // The sign-in entry points hand us a controller that is already presenting the SwiftUI + // login flow, so present from the topmost controller to avoid a no-op present that would + // leave the continuation suspended forever. + viewController.topmostPresentedViewController.present(alert, animated: true) + } + } + @MainActor private func authenticate( details: AutoDiscoveryAttemptSuccess, @@ -228,6 +264,14 @@ struct SelfHostedSiteAuthenticator { throw .authentication(failure) } + if let insecureDestination = details.insecureURL { + let host = insecureDestination.host(percentEncoded: false) ?? details.parsedSiteUrl.url() + let proceed = await confirmInsecureConnection(host: host, from: viewController) + guard proceed else { + throw .cancelled + } + } + let appId = Self.wordPressAppId let appName = Self.wordPressAppName @@ -679,3 +723,62 @@ private final class EmptyAppNotifier: WpAppNotifier { // Do nothing. } } + +private extension AutoDiscoveryAttemptSuccess { + /// The first pre-authorization destination that would receive credentials over an unencrypted + /// connection, or nil when the whole flow is secure. + /// + /// The API root is included because discovery takes it verbatim from the site's Link header, + /// and a misconfigured https site can advertise an http API root that would receive the + /// application password. + var insecureURL: URL? { + var destinations = [parsedSiteUrl.asURL(), apiRootUrl.asURL()] + if case let .applicationPasswords(authUrl) = authentication { + destinations.append(authUrl.asURL()) + } + return destinations.first(where: \.isInsecureConnection) + } + + /// Sanitizes the site URL returned by the authorization callback before it is persisted as the + /// blog URL and used for XML-RPC discovery. + /// + /// When the pre-authorization flow was fully secure (the user was never warned), an http value + /// here is site misconfiguration and must not silently downgrade later traffic, so its scheme is + /// upgraded to https. When the user consented to an insecure flow, the value is left alone. + func sanitizedSiteUrl(_ callbackSiteUrl: String) -> String { + guard insecureURL == nil, + let url = URL(string: callbackSiteUrl), + url.isInsecureConnection, + var components = URLComponents(string: callbackSiteUrl) + else { + return callbackSiteUrl + } + components.scheme = "https" + return components.string ?? callbackSiteUrl + } +} + +private enum Strings { + static let insecureConnectionTitle = NSLocalizedString( + "addSite.selfHosted.insecureConnectionAlert.title", + value: "This site doesn't use a secure connection", + comment: "Title of an alert warning the user that the self-hosted site uses an unencrypted HTTP connection" + ) + + static func insecureConnectionMessage(host: String) -> String { + let format = NSLocalizedString( + "addSite.selfHosted.insecureConnectionAlert.message", + value: + "%@ uses HTTP, which is not encrypted. Your username, password, and site data could be seen by others on the network. Do you want to continue?", + comment: + "Message of an alert warning the user that the self-hosted site uses an unencrypted HTTP connection. The first argument is the site's host name." + ) + return String(format: format, host) + } + + static let insecureConnectionContinue = NSLocalizedString( + "addSite.selfHosted.insecureConnectionAlert.continue", + value: "Continue Anyway", + comment: "Button to proceed with signing in to a self-hosted site over an unencrypted HTTP connection" + ) +} diff --git a/WordPress/Classes/Login/URL+InsecureConnection.swift b/WordPress/Classes/Login/URL+InsecureConnection.swift new file mode 100644 index 000000000000..3879dc6e517b --- /dev/null +++ b/WordPress/Classes/Login/URL+InsecureConnection.swift @@ -0,0 +1,21 @@ +import Foundation + +extension URL { + private static let loopbackHosts: Set = ["localhost", "127.0.0.1", "::1"] + + /// Whether sending credentials to this URL would use an unencrypted connection to a remote host. + /// + /// Only loopback destinations are exempt. Names like `*.local` (resolved over the LAN via mDNS) + /// and `*.test` (resolved by whatever DNS the network provides) do not guarantee a local + /// connection, so they are treated the same as any other remote host. + var isInsecureConnection: Bool { + guard scheme?.lowercased() == "http" else { + return false + } + guard let host = host(percentEncoded: false)?.lowercased() else { + // A scheme of "http" with no parseable host cannot be proven local. Treat it as insecure. + return true + } + return !Self.loopbackHosts.contains(host) + } +} diff --git a/WordPress/Classes/Services/ApplicationPasswordRepository.swift b/WordPress/Classes/Services/ApplicationPasswordRepository.swift index 829fcefd0c72..93676210e7f2 100644 --- a/WordPress/Classes/Services/ApplicationPasswordRepository.swift +++ b/WordPress/Classes/Services/ApplicationPasswordRepository.swift @@ -89,6 +89,17 @@ actor ApplicationPasswordRepository { /// When returning true, a valid application password is guaranteed to be returned by the `Blog.getApplicationToken` function. /// + /// This function must never transmit credentials over an unencrypted connection on its own, so + /// each path checks the destinations it contacts and throws `insecureConnection` for a + /// non-loopback http one. Validation, which every path performs, contacts the site URL and REST + /// root, so those are checked up front. Self-hosted password creation additionally contacts the + /// cookie/nonce URLs (`login_url`, `admin_url`, wp-json, all derived from `xmlrpc` and + /// independently http-capable), checked before creating. A Jetpack-connected site creates its + /// password through the WordPress.com proxy over https, so it is not gated on the site's own + /// scheme. Getting an application password for an insecure site instead goes through the + /// interactive sign-in flow, which shows an insecure-connection warning. The REST root discovered + /// at runtime is validated in `updateRestAPIURLIfNeeded`. + /// /// This function is safe to call multiple times, but every call performs real work, including /// HTTP requests (password validation, and REST API root rediscovery when the stored root is /// stale). Limit calls to once per "site launch" (app launch, switching site, etc.) to avoid @@ -162,7 +173,27 @@ actor ApplicationPasswordRepository { } private extension ApplicationPasswordRepository { + /// Throws `insecureConnection` when any URL would send credentials over a non-loopback http + /// connection. Callers pass only the destinations the request they are about to make contacts. + static func checkInsecureURL(_ urls: [URL]) throws { + if urls.contains(where: \.isInsecureConnection) { + throw ApplicationPasswordRepositoryError.insecureConnection + } + } + func validatePasswords(in blogId: TaggedManagedObjectID) async throws -> ApplicationPassword? { + // Validation contacts the site URL and stored REST root, here and in the token copy below. + // Reject before any request when either is insecure, without contacting the site. + let validationDestinations = try await coreDataStack.performQuery { context in + let blog = try context.existingObject(with: blogId) + var destinations = [try blog.getUrl()] + if let restApiRootURL = blog.restApiRootURL, let parsed = URL(string: restApiRootURL) { + destinations.append(parsed) + } + return destinations + } + try Self.checkInsecureURL(validationDestinations) + try await saveApplicationPassword(of: blogId) let (owners, siteUrl) = try await coreDataStack.performQuery { context in @@ -258,6 +289,28 @@ private extension ApplicationPasswordRepository { parameters: parameters ) } else if let dotOrgApi { + // Creating a password here uses cookie and nonce authentication, contacting the site's + // login_url, admin_url, and wp-json base. Each derives from `xmlrpc` (or its own option), + // so each can use http independently. Gate on every credential destination before + // sending anything. + let destinations = try await coreDataStack.performQuery { context in + let blog = try context.existingObject(with: blogId) + var destinations: [URL] = [try blog.getUrl()] + if let restApiRootURL = blog.restApiRootURL, let parsed = URL(string: restApiRootURL) { + destinations.append(parsed) + } + if let restBase = blog.url(withPath: "wp-json/"), let parsed = URL(string: restBase) { + destinations.append(parsed) + } + if let loginURL = blog.loginURL { + destinations.append(loginURL) + } + if let adminURL = blog.makeAdminURL() { + destinations.append(adminURL) + } + return destinations + } + try Self.checkInsecureURL(destinations) password = try await createPasswordOnSelfHostedSites(api: dotOrgApi, parameters: parameters) } else { // This error should never happen since a blog is accessible via either dot-com or a dot-org API. @@ -385,6 +438,13 @@ private extension ApplicationPasswordRepository { throw error } + // Discovery can resolve an http REST root even for an https site (e.g. an advertised http + // API root). Reject it before persisting, so an insecure value is never stored where other + // consumers (WordPressSite, EditorConfiguration, ...) could later send credentials to it. + if let url = URL(string: apiRootURL.url()), url.isInsecureConnection { + throw ApplicationPasswordRepositoryError.insecureConnection + } + if apiRootURL.url() != restApiRootUrl { try await coreDataStack.performAndSave { context in let blog = try context.existingObject(with: blogId) @@ -506,6 +566,7 @@ extension ApplicationPasswordStorage { enum ApplicationPasswordRepositoryError: LocalizedError { case usernameNotFound case restApiInaccessible + case insecureConnection case unknown var errorDescription: String? { @@ -516,6 +577,13 @@ enum ApplicationPasswordRepositoryError: LocalizedError { value: "Unable to find username for the site", comment: "Error message when the username cannot be found for application password creation" ) + case .insecureConnection: + return NSLocalizedString( + "applicationPasswordRepository.error.insecureConnection", + value: "The site uses an unencrypted connection (HTTP).", + comment: + "Error message when application password creation is skipped because the site uses an insecure HTTP connection" + ) case .restApiInaccessible: return NSLocalizedString( "applicationPasswordRepository.error.restApiInaccessible",