Skip to content
Draft
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
41 changes: 41 additions & 0 deletions Tests/KeystoneTests/Tests/Login/URL+InsecureConnectionTests.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,146 @@ 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 REST base, login_url, and admin_url are derived from the http xmlrpc endpoint, so
// credentials must not be sent even though the site URL itself is https.
let insecureHost = "insecure-xmlrpc.example.com"
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://secure-site.example.com"
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 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
Expand Down Expand Up @@ -758,6 +898,39 @@ private extension ApplicationPasswordsRepositoryTests {
}
}

func stubApiDiscoveryWithInsecureRoot(siteHost: String) {
stub(condition: isHost(siteHost) && isPath("/")) { _ in
HTTPStubsResponse(
data: "<html>homepage</html>".data(using: .utf8)!,
statusCode: 200,
headers: ["Link": "<http://\(siteHost)/wp-json/>; 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: "<html>homepage</html>".data(using: .utf8)!, statusCode: 200, headers: nil)
Expand Down
100 changes: 99 additions & 1 deletion WordPress/Classes/Login/SelfHostedSiteAuthenticator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import WordPressShared
import BuildSettingsKit
import SVProgressHUD
import WordPressSharedUI
import WordPressUI

struct SelfHostedSiteAuthenticator {

Expand Down Expand Up @@ -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()
Expand All @@ -211,6 +217,31 @@ struct SelfHostedSiteAuthenticator {
}
}

@MainActor
private func confirmInsecureConnection(host: String, from viewController: UIViewController) async -> Bool {
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,
Expand All @@ -228,6 +259,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

Expand Down Expand Up @@ -679,3 +718,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"
)
}
21 changes: 21 additions & 0 deletions WordPress/Classes/Login/URL+InsecureConnection.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import Foundation

extension URL {
private static let loopbackHosts: Set<String> = ["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)
}
}
Loading