diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b95e6a..72b8324 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,83 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] -### Changed +## [0.13.0] — 2026-10-03 + +### Read this first: `trustSelfSignedCertificates` is gone, and it never did what it said + +**Breaking, and a security fix (CWE-295).** All three network transports took +`trustSelfSignedCertificates: Bool`. Passing `true` set NIOSSL's `certificateVerification` to +`.none`. That does not trust a self-signed certificate — it stops checking certificates, so the +connection accepts whatever is presented by whoever answers it. Anyone on the path between a +client and its "development" server could read and rewrite the session, bearer token included. + +The parameter is removed rather than deprecated. A deprecated flag that quietly began verifying +would compile, warn, and then fail at run time in exactly the environments that set it; a +deprecated flag that kept working would keep the hole. Each transport instead carries an +`unavailable` initializer under the old label, so the old spelling is a compile error whose +message says what to write. + +**Migration.** Where the argument was `false`, delete it. Where it was `true`, supply the +certificate the server presents — or the private authority that issued it — as a trust root: + +```swift +// before +let transport = StreamableHTTPTransport(url: url, trustSelfSignedCertificates: true) + +// after +let trust = try ServerTrust.onlyRoots([.pemFile("/etc/mcp/dev-server.pem")]) +let transport = StreamableHTTPTransport(url: url, serverTrust: trust) +``` +Two things that used to be skipped are now checked, and a development server has to pass both: + +- **The name.** The certificate must carry the host in the URL as a subject alternative name — + a DNS name for `https://dev.internal`, an IP address for `https://127.0.0.1`. To export what a + server presents: `openssl s_client -connect host:443 -showcerts + server.pem`. To make one that names its host: `openssl req -x509 -newkey ec -pkeyopt + ec_paramgen_curve:prime256v1 -nodes -keyout key.pem -out server.pem -days 365 -subj + "/CN=dev.internal" -addext "subjectAltName=DNS:dev.internal,IP:127.0.0.1" -addext + "extendedKeyUsage=serverAuth"`. +- **The chain.** It must end at a root you supplied (or, for `additionalRoots`, at one of those + or a system root). + +There is no replacement for "verify nothing", by design. + +### Added +- **`ServerTrust` — what a transport is prepared to believe about its server.** One type, used + by `StreamableHTTPTransport`, `HTTPSSETransport` and `WebSocketTransport` through a new + `serverTrust:` parameter that defaults to `.system`: + - `ServerTrust.system` — the platform root store. The default; behaviour is unchanged. + - `ServerTrust.additionalRoots(_:)` — the platform root store plus supplied certificates. + - `ServerTrust.onlyRoots(_:)` — the supplied certificates and nothing else. With a self-signed + server's own certificate this is a pin: a different self-signed certificate is refused. + + Certificates come from `ServerTrust.CertificateSource`: `.pem(String)`, `.der([UInt8])`, + `.pemFile(String)`, `.derFile(String)`. They are read and parsed when the value is made, so a + bad path or a mangled certificate throws `ServerTrustError` where the configuration is + written. An empty list throws too; nothing falls back to the system roots. + `rootFingerprints` exposes the SHA-256 of each supplied root — the value `openssl x509 + -fingerprint -sha256` prints — so an application can show what it trusts. +- **No hash pinning, deliberately.** Pinning by SHA-256 of a key or certificate needs NIOSSL's + verification callback, which neither `AsyncHTTPClient` nor `WebSocketKit` lets a caller + install, and which replaces chain validation rather than adding to it. A pin that only some + transports could enforce would be worse than none. + +### Changed +- **A transport with supplied roots runs on NIO's event loops, on every platform.** On Apple + platforms `AsyncHTTPClient` normally runs on Network.framework and *translates* a NIOSSL + `TLSConfiguration` for it; that translation ignores `additionalTrustRoots` altogether. With + supplied roots the client is now built on `MultiThreadedEventLoopGroup.singleton`, so NIOSSL + enforces the configuration and macOS behaves as Linux does. `.system` keeps the platform + stack, so nothing changes for a caller who does not pass `serverTrust`. +- **`swift-nio-ssl` is a declared dependency.** The transports already imported it and received + it transitively. `swift-certificates` and `swift-asn1` are added for the test target only, + which mints its certificates at run time rather than committing a private key. All three were + already in the resolved graph; `Package.resolved` is unchanged. +- **MCPExplorer's "Trust self-signed certificates" toggle is now a "Trusted certificate file" + field.** It takes the path of a PEM file and trusts only what is in it; an unreadable path + stops the connection instead of connecting without it. It applies to WebSocket connections + too, which the toggle never reached. - One `// SECURITY:` acknowledgement now says what was decided, ahead of the quality gate requiring a reason of at least eight words on every one. @@ -471,3 +546,13 @@ and Linux. - Phase 2 spec compliance: notifications, ping, pagination, protocol version negotiation - TransportGuide DocC article - Initial MCPClient package with HTTP/SSE transport + +[Unreleased]: https://github.com/jpurnell/SwiftMCPClient/compare/v0.12.0...HEAD +[0.13.0]: https://github.com/jpurnell/SwiftMCPClient/compare/v0.12.0...v0.13.0 +[0.12.0]: https://github.com/jpurnell/SwiftMCPClient/compare/v0.11.0...v0.12.0 +[0.11.0]: https://github.com/jpurnell/SwiftMCPClient/compare/v0.10.0...v0.11.0 +[0.10.0]: https://github.com/jpurnell/SwiftMCPClient/compare/v0.9.0...v0.10.0 +[0.9.0]: https://github.com/jpurnell/SwiftMCPClient/compare/v0.4.0...v0.9.0 +[0.4.0]: https://github.com/jpurnell/SwiftMCPClient/compare/v0.3.0...v0.4.0 +[0.3.0]: https://github.com/jpurnell/SwiftMCPClient/compare/v0.2.0...v0.3.0 +[0.2.0]: https://github.com/jpurnell/SwiftMCPClient/releases/tag/v0.2.0 diff --git a/Package.swift b/Package.swift index 964b86e..c7e2309 100644 --- a/Package.swift +++ b/Package.swift @@ -14,6 +14,10 @@ var targets: [Target] = [ .product(name: "NIOCore", package: "swift-nio"), .product(name: "NIOPosix", package: "swift-nio"), .product(name: "NIOHTTP1", package: "swift-nio"), + // The transports import NIOSSL to say what a TLS connection trusts. It arrived + // transitively for as long as nothing here named a type of its own; `ServerTrust` + // does, so the dependency is declared rather than borrowed. + .product(name: "NIOSSL", package: "swift-nio-ssl"), .product(name: "WebSocketKit", package: "websocket-kit"), ], // Declared rather than left implicit. SwiftPM does not claim this catalog on a @@ -37,6 +41,13 @@ var targets: [Target] = [ .product(name: "NIOCore", package: "swift-nio"), .product(name: "NIOPosix", package: "swift-nio"), .product(name: "NIOHTTP1", package: "swift-nio"), + // What a transport trusts is also wire behaviour: whether a handshake with a + // given certificate completes. The tests stand up a TLS server on loopback and + // mint its certificates at run time, so no private key is ever committed. + .product(name: "NIOSSL", package: "swift-nio-ssl"), + .product(name: "X509", package: "swift-certificates"), + .product(name: "SwiftASN1", package: "swift-asn1"), + .product(name: "Crypto", package: "swift-crypto"), ] ), ] @@ -118,6 +129,14 @@ let package = Package( // listener uses it directly. .package(url: "https://github.com/apple/swift-nio.git", from: "2.100.0"), .package(url: "https://github.com/apple/swift-crypto.git", from: "3.0.0"), + // Already present transitively via async-http-client and websocket-kit; declared + // because `ServerTrust` builds a `TLSConfiguration` directly. + .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.36.0"), + // Test-only: mints the self-signed and private-CA certificates the trust tests + // handshake against. Both were already in the graph through async-http-client's + // dependencies; neither reaches the library target. + .package(url: "https://github.com/apple/swift-certificates.git", from: "1.18.0"), + .package(url: "https://github.com/apple/swift-asn1.git", from: "1.6.0"), .package(url: "https://github.com/vapor/websocket-kit.git", from: "2.15.0"), .package(url: "https://github.com/swiftlang/swift-docc-plugin", from: "1.4.3"), ], diff --git a/README.md b/README.md index 0ab7ae0..bf436a6 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ A Swift 6 client library for the [Model Context Protocol (MCP)](https://modelcon - Swift 6.0+ - macOS 14+ / iOS 17+ / tvOS 17+ / watchOS 10+ -- Linux (Ubuntu 22.04+) — builds; CI verification is still outstanding, so treat Linux as unproven +- Linux (Ubuntu 22.04+) — built and tested in CI ## Installation @@ -114,10 +114,35 @@ let transport = HTTPSSETransport( connectionTimeout: 30, maxReconnectAttempts: 3, reconnectBaseDelay: 1.0, - trustSelfSignedCertificates: false // true only for dev/testing + serverTrust: .system // the default; see below for self-signed and private-CA servers ) ``` +### Self-signed and private-CA servers + +Every network transport takes a `serverTrust:`. It defaults to `.system`, the platform's root +store. For a server whose certificate is self-signed, or issued by a private authority, supply +that certificate and it becomes a trust root: + +```swift +// Trust this certificate and nothing else. For a self-signed server this is a pin. +let trust = try ServerTrust.onlyRoots([.pemFile("/etc/mcp/dev-server.pem")]) + +// Or: the system roots plus a private authority. +let trust = try ServerTrust.additionalRoots([.pem(corporateRootPEM)]) + +let transport = StreamableHTTPTransport(url: url, serverTrust: trust) +``` + +Sources are `.pem(String)`, `.der([UInt8])`, `.pemFile(String)` and `.derFile(String)`. They are +read when the value is made, so a bad path throws there, not at connect time. + +There is no option that turns verification off. The chain is validated and the certificate is +checked against the host in the URL in every case, so a development certificate must name its +host in a subject alternative name. Versions before 0.13.0 had a `trustSelfSignedCertificates` +flag that disabled verification outright; it is removed — see the +[changelog](CHANGELOG.md) for the migration. + ## MCP Operations ### Tools @@ -279,6 +304,7 @@ MCPClient/ │ ├── HTTPSSETransport # HTTP POST + Server-Sent Events │ ├── WebSocketTransport # WebSocket text frames │ ├── StdioTransport # Subprocess stdin/stdout +│ ├── ServerTrust # Which certificate roots a TLS server may chain to │ └── SSEParser # SSE event stream parser ├── AnyCodableValue # Type-safe JSON value wrapper ├── JSONRPCTypes # JSON-RPC 2.0 envelope types diff --git a/Sources/MCPClient/MCPClient.docc/MCPClient.md b/Sources/MCPClient/MCPClient.docc/MCPClient.md index d879ca2..71b2543 100644 --- a/Sources/MCPClient/MCPClient.docc/MCPClient.md +++ b/Sources/MCPClient/MCPClient.docc/MCPClient.md @@ -66,9 +66,12 @@ Swift project that needs MCP integration. - - ``MCPTransport`` +- ``StreamableHTTPTransport`` - ``HTTPSSETransport`` - ``WebSocketTransport`` - ``StdioTransport`` +- ``ServerTrust`` +- ``ServerTrustError`` ### JSON-RPC Protocol diff --git a/Sources/MCPClient/MCPClient.docc/MigrationGuide.md b/Sources/MCPClient/MCPClient.docc/MigrationGuide.md index 245f816..c4b2ae1 100644 --- a/Sources/MCPClient/MCPClient.docc/MigrationGuide.md +++ b/Sources/MCPClient/MCPClient.docc/MigrationGuide.md @@ -226,3 +226,28 @@ func connectOverWebSocket() throws { _ = webSocketTransport } ``` + +## Migrating to 0.13.0: `trustSelfSignedCertificates` is removed + +The network transports no longer accept `trustSelfSignedCertificates`. Passing +`true` disabled certificate verification entirely — it did not trust a +self-signed certificate, it trusted whoever answered. The old spelling is now a +compile error. + +Where the argument was `false`, delete it. Where it was `true`, supply the +certificate the server presents, or the authority that issued it, through +``ServerTrust``: + +```swift +func connectWithSuppliedCertificate() throws { + guard let url = URL(string: "https://dev.internal:8443/mcp") else { return } + + // Before: StreamableHTTPTransport(url: url, trustSelfSignedCertificates: true) + let trust = try ServerTrust.onlyRoots([.pemFile("/etc/mcp/dev-server.pem")]) + _ = StreamableHTTPTransport(url: url, serverTrust: trust) +} +``` + +The certificate must name the host in the URL as a subject alternative name; +that check was skipped before and is not skippable now. See + for the details. diff --git a/Sources/MCPClient/MCPClient.docc/TransportGuide.md b/Sources/MCPClient/MCPClient.docc/TransportGuide.md index a5d09cd..b939201 100644 --- a/Sources/MCPClient/MCPClient.docc/TransportGuide.md +++ b/Sources/MCPClient/MCPClient.docc/TransportGuide.md @@ -144,6 +144,84 @@ an exponential backoff — three attempts by default, configurable through - Servers that offer only the legacy HTTP+SSE endpoints - Cross-platform (macOS, iOS, tvOS, watchOS, Linux) +## Self-Signed and Private-CA Servers + +The three network transports — ``StreamableHTTPTransport``, ``HTTPSSETransport`` +and ``WebSocketTransport`` — each take a ``ServerTrust``, which says which +certificate roots the server's chain may end at. It defaults to +``ServerTrust/system``: the platform's root store, which is what any publicly +certified server needs. + +A development server with a self-signed certificate, or a server behind a +private certificate authority, is trusted by *supplying the certificate*: + +```swift +// Shown inside functions this guide never calls: one reads a file that is not +// there when the guide runs, and both would open a connection. +func connectToSelfSignedServer() async throws { + guard let url = URL(string: "https://dev.internal:8443/mcp") else { return } + + // The server's own certificate, and nothing else. A server presenting any + // other certificate — including another self-signed one — is refused. + let trust = try ServerTrust.onlyRoots([.pemFile("/etc/mcp/dev-server.pem")]) + + let transport = StreamableHTTPTransport(url: url, serverTrust: trust) + let client = MCPClientConnection(transport: transport) + _ = try await client.initialize(clientName: "my-app", clientVersion: "1.0.0") +} + +func connectBehindPrivateAuthority(rootPEM: String) throws { + guard let url = URL(string: "wss://mcp.corp.example/ws") else { return } + + // The system roots as well as the private one, for a client that reaches + // public servers too. + let trust = try ServerTrust.additionalRoots([.pem(rootPEM)]) + + // What was just trusted, as `openssl x509 -fingerprint -sha256` prints it. + print(trust.rootFingerprints) + + _ = WebSocketTransport(url: url, serverTrust: trust) +} +``` + +### What is still checked + +Everything. A supplied root changes who is allowed to have signed the server's +certificate; it does not change whether that is checked, and there is no option +that does. In particular: + +- **The hostname.** The certificate must carry the host in the URL as a subject + alternative name — a DNS name, or an IP address if the URL uses one. A trusted + certificate issued for another name is refused. +- **The chain.** It must end at a supplied root, or for + ``ServerTrust/additionalRoots(_:)`` at a supplied root or a system one. +- **The dates.** An expired certificate is refused even if it is the one you + supplied. + +On Apple platforms ``ServerTrust/additionalRoots(_:)`` is evaluated by the +system verifier, which applies Apple's own requirements for TLS server +certificates — among them an extended key usage of `serverAuth`. +``ServerTrust/onlyRoots(_:)`` is evaluated by BoringSSL on every platform. + +### When it fails + +A ``ServerTrust`` that cannot be built — a missing file, text holding no +certificate, an empty list — throws ``ServerTrustError`` where it is +constructed, before any transport exists. Nothing falls back to the system +roots. + +A server that is refused surfaces as ``MCPError/connectionFailed(reason:)`` +from the first request. The HTTP transports retry a failed connection until +`connectionTimeout` elapses, so a refusal is reported after that long rather +than at once. + +### Why there is no "trust anything" switch + +Versions before 0.13.0 had `trustSelfSignedCertificates: Bool`. It did not +trust a self-signed certificate; it turned verification off, which accepts any +certificate from anyone able to answer the connection. It has been removed — +the old spelling is a compile error that points here. + ## Stdio — Local Subprocess ``StdioTransport`` launches an MCP server as a local child process and diff --git a/Sources/MCPClient/Transport/HTTPSSETransport.swift b/Sources/MCPClient/Transport/HTTPSSETransport.swift index 53a1205..a9a391c 100644 --- a/Sources/MCPClient/Transport/HTTPSSETransport.swift +++ b/Sources/MCPClient/Transport/HTTPSSETransport.swift @@ -27,9 +27,11 @@ import Logging /// /// ## Cross-Platform /// -/// Uses `AsyncHTTPClient` (Swift NIO) for HTTP and TLS, providing identical -/// behavior on macOS and Linux. Self-signed certificate support works on -/// all platforms via NIO SSL. +/// Uses `AsyncHTTPClient` for HTTP on every platform. TLS is the platform's own +/// on Apple platforms and NIOSSL's on Linux, both verifying against the system +/// roots. A self-signed or privately issued server certificate is trusted by +/// supplying it — see ``ServerTrust`` — and a transport given one verifies with +/// NIOSSL everywhere, so that case behaves identically on macOS and Linux. public actor HTTPSSETransport: MCPTransport { private let url: URL private let headers: [String: String] @@ -44,7 +46,7 @@ public actor HTTPSSETransport: MCPTransport { private let connectionTimeout: TimeAmount private let maxReconnectAttempts: Int private let reconnectBaseDelay: TimeInterval - private let trustSelfSignedCertificates: Bool + private let serverTrust: ServerTrust /// The endpoint URL extracted from the SSE `endpoint` event during connect. private var endpointURL: URL? @@ -76,9 +78,10 @@ public actor HTTPSSETransport: MCPTransport { /// - connectionTimeout: Maximum time to wait for the initial endpoint event. Default 30s. /// - maxReconnectAttempts: Number of reconnection attempts on stream drop. Default 3. /// - reconnectBaseDelay: Base delay for exponential backoff in seconds. Default 1.0. - /// - trustSelfSignedCertificates: Accept self-signed or invalid TLS certificates. - /// **Use only for development/testing** — this disables certificate validation. - /// Works on both macOS and Linux. + /// - serverTrust: Which certificate roots an `https://` server may chain to. Defaults + /// to ``ServerTrust/system``. For a self-signed or privately issued certificate, + /// supply it with ``ServerTrust/onlyRoots(_:)`` or ``ServerTrust/additionalRoots(_:)``; + /// the chain and the hostname are verified either way. public init( url: URL, headers: [String: String] = [:], @@ -86,7 +89,7 @@ public actor HTTPSSETransport: MCPTransport { connectionTimeout: TimeInterval = 30.0, maxReconnectAttempts: Int = 3, reconnectBaseDelay: TimeInterval = 1.0, - trustSelfSignedCertificates: Bool = false + serverTrust: ServerTrust = .system ) { self.url = url self.headers = headers @@ -94,7 +97,44 @@ public actor HTTPSSETransport: MCPTransport { self.connectionTimeout = .seconds(clamping: connectionTimeout) self.maxReconnectAttempts = maxReconnectAttempts self.reconnectBaseDelay = reconnectBaseDelay - self.trustSelfSignedCertificates = trustSelfSignedCertificates + self.serverTrust = serverTrust + } + + /// Removed: this did not trust a self-signed certificate, it disabled verification. + /// + /// Passing `true` set NIOSSL's `certificateVerification` to `.none`, which accepts any + /// certificate from anyone. Supply the certificate instead — see ``ServerTrust``. + /// + /// - Parameters: + /// - url: The SSE endpoint URL. + /// - headers: Custom HTTP headers sent with all requests. + /// - authorization: Asked for a current `Authorization` header. + /// - connectionTimeout: Maximum time to wait for the initial endpoint event. + /// - maxReconnectAttempts: Number of reconnection attempts on stream drop. + /// - reconnectBaseDelay: Base delay for exponential backoff in seconds. + /// - trustSelfSignedCertificates: Ignored. Verification is never disabled. + @available(*, unavailable, message: "This disabled certificate verification entirely. Pass serverTrust: try .onlyRoots([.pemFile(path)]) (or .additionalRoots) with the server's certificate; omit the argument where it was false.") + public init( + url: URL, + headers: [String: String] = [:], + authorization: AuthorizationProvider? = nil, + connectionTimeout: TimeInterval = 30.0, + maxReconnectAttempts: Int = 3, + reconnectBaseDelay: TimeInterval = 1.0, + trustSelfSignedCertificates: Bool + ) { + self.url = url + self.headers = headers + self.authorization = authorization + self.connectionTimeout = .seconds(clamping: connectionTimeout) + self.maxReconnectAttempts = maxReconnectAttempts + self.reconnectBaseDelay = reconnectBaseDelay + self.serverTrust = .system + } + + /// The TLS configuration every request is made with. + nonisolated var tlsConfiguration: TLSConfiguration { + serverTrust.makeTLSConfiguration() } /// Open the SSE connection to the MCP server, retrying with exponential backoff on failure. @@ -389,17 +429,7 @@ public actor HTTPSSETransport: MCPTransport { // MARK: - HTTP Client Factory private func makeHTTPClient() -> HTTPClient { - var tlsConfig = TLSConfiguration.makeClientConfiguration() - if trustSelfSignedCertificates { - tlsConfig.certificateVerification = .none - } - - var config = HTTPClient.Configuration( - tlsConfiguration: tlsConfig - ) - config.timeout.connect = connectionTimeout - - return HTTPClient(configuration: config) + serverTrust.makeHTTPClient(connectTimeout: connectionTimeout) } // MARK: - Message Handling diff --git a/Sources/MCPClient/Transport/ServerTrust.swift b/Sources/MCPClient/Transport/ServerTrust.swift new file mode 100644 index 0000000..81d065e --- /dev/null +++ b/Sources/MCPClient/Transport/ServerTrust.swift @@ -0,0 +1,265 @@ +import Foundation +import AsyncHTTPClient +import Crypto +import NIOCore +import NIOPosix +import NIOSSL + +/// What a transport is prepared to believe about the server it is talking to. +/// +/// Every network transport takes one of these, and there are exactly three things it can say: +/// +/// - ``system`` — the platform's root store, which is the default and what a publicly +/// certified server needs. +/// - ``additionalRoots(_:)`` — the platform's root store *and* certificates you supply. +/// - ``onlyRoots(_:)`` — the certificates you supply and nothing else. +/// +/// What it cannot say is "do not verify". In all three the certificate chain is validated and +/// the certificate is checked against the host being connected to; a supplied root changes +/// *who is allowed to have signed* the server's certificate, never *whether* that is checked. +/// +/// ## Trusting a self-signed server +/// +/// A self-signed certificate is its own issuer, so trusting it means supplying it as a root: +/// +/// ```swift +/// func selfSignedTransport(url: URL) throws -> StreamableHTTPTransport { +/// let trust = try ServerTrust.onlyRoots([.pemFile("/etc/mcp/dev-server.pem")]) +/// return StreamableHTTPTransport(url: url, serverTrust: trust) +/// } +/// ``` +/// +/// With ``onlyRoots(_:)`` that is also a pin: the server must present a chain ending at that +/// exact certificate, and a different self-signed certificate — which is all an interposed +/// attacker can offer — is refused. +/// +/// ## Trusting a private certificate authority +/// +/// Supply the authority's certificate. Any server certificate it issued is then trusted for +/// the names that certificate carries: +/// +/// ```swift +/// func privateAuthorityTrust(rootPEM: String) throws -> ServerTrust { +/// try ServerTrust.additionalRoots([.pem(rootPEM)]) +/// } +/// ``` +/// +/// ## The hostname still has to match +/// +/// A development certificate has to name the host it is reached by, as a subject alternative +/// name — a DNS name for `https://dev.internal`, an IP address for `https://127.0.0.1`. A +/// certificate that names neither is refused however it is trusted. That is deliberate: a +/// root you trust says who signed the certificate, and the name says who it was signed *for*. +/// +/// ## Why there is no hash pin +/// +/// Pinning by the SHA-256 of a certificate or public key needs a verification callback, and +/// neither `AsyncHTTPClient` nor `WebSocketKit` lets a caller install one — and NIOSSL's +/// callback *replaces* chain validation rather than adding to it. A pin that some transports +/// could not enforce would be worse than none, so this type offers only what all three can +/// honour. ``onlyRoots(_:)`` with the server's own certificate is the pin; and +/// ``rootFingerprints`` is there so an application can show, or compare, exactly what it has +/// been told to trust. +public struct ServerTrust: Sendable, Hashable { + + /// Where a trusted certificate comes from. + public enum CertificateSource: Sendable, Hashable { + /// PEM text holding one or more certificates. + case pem(String) + /// One DER-encoded certificate. + case der([UInt8]) + /// The path of a PEM file holding one or more certificates. + case pemFile(String) + /// The path of a file holding one DER-encoded certificate. + case derFile(String) + } + + /// Whether the platform's root store is consulted. + /// + /// `false` only for ``onlyRoots(_:)``, where a certificate signed by a public authority + /// is refused like any other the caller did not name. + public let usesSystemRoots: Bool + + /// The SHA-256 of each supplied root's DER encoding, as lowercase hex, in the order the + /// roots were supplied. + /// + /// Empty for ``system``. This is the same value `openssl x509 -fingerprint -sha256` + /// prints, without the colons — so what an application trusts can be shown to a person + /// and compared against what they expected. + public let rootFingerprints: [String] + + /// The parsed roots. Parsed once, when the value is made, so that nothing about what is + /// trusted is left to be discovered at connect time. + private let roots: [NIOSSLCertificate] + + private init(usesSystemRoots: Bool, roots: [NIOSSLCertificate], rootFingerprints: [String]) { + self.usesSystemRoots = usesSystemRoots + self.roots = roots + self.rootFingerprints = rootFingerprints + } + + /// Trust the platform's root store, and nothing else. + /// + /// The default for every transport, and the right answer for any server with a + /// certificate from a public authority. + public static let system = ServerTrust(usesSystemRoots: true, roots: [], rootFingerprints: []) + + /// Trust the platform's root store and the supplied certificates. + /// + /// For a client that talks to public servers and to ones behind a private authority. + /// Prefer ``onlyRoots(_:)`` when the transport will only ever reach the private one. + /// + /// - Parameter sources: The certificates to add. Read and parsed before this returns. + /// - Returns: The trust. + /// - Throws: ``ServerTrustError/noCertificates`` if `sources` is empty, or + /// ``ServerTrustError/unreadable(_:)`` naming the first source that held no certificate. + public static func additionalRoots(_ sources: [CertificateSource]) throws -> ServerTrust { + try make(usesSystemRoots: true, sources: sources) + } + + /// Trust the supplied certificates and nothing else. + /// + /// The server's chain must end at one of them. Supplying a self-signed server's own + /// certificate pins the connection to that certificate. + /// + /// - Parameter sources: The only certificates to trust. Read and parsed before this + /// returns. + /// - Returns: The trust. + /// - Throws: ``ServerTrustError/noCertificates`` if `sources` is empty, or + /// ``ServerTrustError/unreadable(_:)`` naming the first source that held no certificate. + public static func onlyRoots(_ sources: [CertificateSource]) throws -> ServerTrust { + try make(usesSystemRoots: false, sources: sources) + } + + /// The NIOSSL client configuration this trust amounts to. + /// + /// The one place in the package a client `TLSConfiguration` is built. Verification is + /// whatever `makeClientConfiguration()` set — chain and hostname — and nothing here + /// touches it; the only fields written are the two that say which roots to verify against. + func makeTLSConfiguration() -> TLSConfiguration { + var configuration = TLSConfiguration.makeClientConfiguration() + guard !roots.isEmpty else { return configuration } + if usesSystemRoots { + configuration.additionalTrustRoots = [.certificates(roots)] + } else { + configuration.trustRoots = .certificates(roots) + } + return configuration + } + + /// An HTTP client that verifies servers against this trust. + /// + /// Shared by both HTTP transports, so there is one answer to "what does a request + /// verify against" rather than one per transport. + /// + /// Supplied roots are always enforced by NIOSSL. On Apple platforms `AsyncHTTPClient` + /// otherwise runs on Network.framework and *translates* a `TLSConfiguration` for it, and + /// that translation does not carry `additionalTrustRoots` at all — so the configuration + /// would say one thing and the connection would verify against another. Running on NIO's + /// own event loops makes the configuration built above the one that is enforced, and + /// makes it the same one on macOS and Linux. ``system`` keeps the platform's stack. + /// + /// - Parameter connectTimeout: How long a connection attempt may take. + /// - Returns: The client. The caller owns it and must shut it down. + func makeHTTPClient(connectTimeout: TimeAmount) -> HTTPClient { + var configuration = HTTPClient.Configuration(tlsConfiguration: makeTLSConfiguration()) + configuration.timeout.connect = connectTimeout + guard !roots.isEmpty else { + return HTTPClient(configuration: configuration) + } + return HTTPClient( + eventLoopGroup: MultiThreadedEventLoopGroup.singleton, + configuration: configuration) + } + + // MARK: - Loading + + private static func make(usesSystemRoots: Bool, sources: [CertificateSource]) throws -> ServerTrust { + // An empty list is refused rather than interpreted. Under `onlyRoots` it would be a + // client that trusts nothing; under `additionalRoots` it would quietly mean `.system`. + guard !sources.isEmpty else { throw ServerTrustError.noCertificates } + + var roots: [NIOSSLCertificate] = [] + var fingerprints: [String] = [] + for source in sources { + // All or nothing: a subset of the roots someone asked for is a different trust + // decision from the one they made. + let loaded = try load(source) + guard !loaded.isEmpty else { throw ServerTrustError.unreadable(source) } + for certificate in loaded { + fingerprints.append(try fingerprint(of: certificate, from: source)) + roots.append(certificate) + } + } + return ServerTrust(usesSystemRoots: usesSystemRoots, roots: roots, rootFingerprints: fingerprints) + } + + private static func load(_ source: CertificateSource) throws -> [NIOSSLCertificate] { + do { + switch source { + case .pem(let text): + return try NIOSSLCertificate.fromPEMBytes(Array(text.utf8)) + case .der(let bytes): + return [try NIOSSLCertificate(bytes: bytes, format: .der)] + case .pemFile(let path): + return try NIOSSLCertificate.fromPEMFile(path) + case .derFile(let path): + return [try NIOSSLCertificate.fromDERFile(path)] + } + } catch { + // The underlying error is BoringSSL's and says little a caller can act on; which + // source it was is the useful part, and the error carries it. + throw ServerTrustError.unreadable(source) + } + } + + private static func fingerprint( + of certificate: NIOSSLCertificate, + from source: CertificateSource + ) throws -> String { + let der: [UInt8] + do { + der = try certificate.toDERBytes() + } catch { + throw ServerTrustError.unreadable(source) + } + return SHA256.hash(data: der).map { byte in + let hex = String(byte, radix: 16) + return hex.count == 1 ? "0" + hex : hex + }.joined() + } +} + +/// Why a ``ServerTrust`` could not be made. +/// +/// Thrown when the trust is constructed, not when a transport connects — a certificate that +/// cannot be read is a mistake in configuration, and it is reported where the configuration +/// is written. Nothing falls back to ``ServerTrust/system``. +public enum ServerTrustError: Error, Sendable, Equatable { + /// No certificate sources were supplied. + case noCertificates + + /// A source did not yield a certificate: the text or bytes did not parse, the file could + /// not be read, or it held none. + /// + /// - Parameter source: The source that failed. + case unreadable(ServerTrust.CertificateSource) +} + +extension ServerTrustError: CustomStringConvertible { + /// A description suitable for a log line or an error alert. + public var description: String { + switch self { + case .noCertificates: + return "No certificates were supplied to trust." + case .unreadable(.pem): + return "The supplied PEM text holds no readable certificate." + case .unreadable(.der): + return "The supplied DER bytes are not a certificate." + case .unreadable(.pemFile(let path)): + return "No certificate could be read from the PEM file at \(path)." + case .unreadable(.derFile(let path)): + return "No certificate could be read from the DER file at \(path)." + } + } +} diff --git a/Sources/MCPClient/Transport/StreamableHTTPTransport.swift b/Sources/MCPClient/Transport/StreamableHTTPTransport.swift index 2e8854a..7ebda0a 100644 --- a/Sources/MCPClient/Transport/StreamableHTTPTransport.swift +++ b/Sources/MCPClient/Transport/StreamableHTTPTransport.swift @@ -90,7 +90,7 @@ public actor StreamableHTTPTransport: MCPTransport { /// Asked for a current `Authorization` header before each request. private let authorization: AuthorizationProvider? private let connectionTimeout: TimeAmount - private let trustSelfSignedCertificates: Bool + private let serverTrust: ServerTrust /// The HTTP client used for all requests. private var httpClient: HTTPClient? @@ -129,22 +129,58 @@ public actor StreamableHTTPTransport: MCPTransport { /// server-initiated messages. Passing `false` restores the POST-only behaviour of /// ADR-001 exactly. /// - connectionTimeout: Maximum time to wait for each HTTP request. Default 30s. - /// - trustSelfSignedCertificates: Accept self-signed or invalid TLS certificates. - /// **Use only for development/testing** — this disables certificate validation. + /// - serverTrust: Which certificate roots an `https://` server may chain to. Defaults + /// to ``ServerTrust/system``. For a self-signed or privately issued certificate, + /// supply it with ``ServerTrust/onlyRoots(_:)`` or ``ServerTrust/additionalRoots(_:)``; + /// the chain and the hostname are verified either way. public init( url: URL, headers: [String: String] = [:], authorization: AuthorizationProvider? = nil, openServerStream: Bool = true, connectionTimeout: TimeInterval = 30.0, - trustSelfSignedCertificates: Bool = false + serverTrust: ServerTrust = .system ) { self.url = url self.headers = headers self.authorization = authorization self.opensServerStream = openServerStream self.connectionTimeout = .seconds(clamping: connectionTimeout) - self.trustSelfSignedCertificates = trustSelfSignedCertificates + self.serverTrust = serverTrust + } + + /// Removed: this did not trust a self-signed certificate, it disabled verification. + /// + /// Passing `true` set NIOSSL's `certificateVerification` to `.none`, which accepts any + /// certificate from anyone. Supply the certificate instead — see ``ServerTrust``. + /// + /// - Parameters: + /// - url: The MCP endpoint URL. + /// - headers: Custom HTTP headers sent with all requests. + /// - authorization: Asked for a current `Authorization` header. + /// - openServerStream: Whether to open the client-initiated `GET` stream. + /// - connectionTimeout: Maximum time to wait for each HTTP request. + /// - trustSelfSignedCertificates: Ignored. Verification is never disabled. + @available(*, unavailable, message: "This disabled certificate verification entirely. Pass serverTrust: try .onlyRoots([.pemFile(path)]) (or .additionalRoots) with the server's certificate; omit the argument where it was false.") + public init( + url: URL, + headers: [String: String] = [:], + authorization: AuthorizationProvider? = nil, + openServerStream: Bool = true, + connectionTimeout: TimeInterval = 30.0, + trustSelfSignedCertificates: Bool + ) { + self.url = url + self.headers = headers + self.authorization = authorization + self.opensServerStream = openServerStream + self.connectionTimeout = .seconds(clamping: connectionTimeout) + self.serverTrust = .system + } + + /// The TLS configuration every request is made with. + nonisolated var tlsConfiguration: TLSConfiguration { + serverTrust.makeTLSConfiguration() } /// Replaces the `Authorization` header used by subsequent requests. @@ -680,17 +716,7 @@ public actor StreamableHTTPTransport: MCPTransport { // MARK: - HTTP Client Factory private func makeHTTPClient() -> HTTPClient { - var tlsConfig = TLSConfiguration.makeClientConfiguration() - if trustSelfSignedCertificates { - tlsConfig.certificateVerification = .none - } - - var config = HTTPClient.Configuration( - tlsConfiguration: tlsConfig - ) - config.timeout.connect = connectionTimeout - - return HTTPClient(configuration: config) + serverTrust.makeHTTPClient(connectTimeout: connectionTimeout) } // MARK: - Message Handling diff --git a/Sources/MCPClient/Transport/WebSocketTransport.swift b/Sources/MCPClient/Transport/WebSocketTransport.swift index 081f671..36e056d 100644 --- a/Sources/MCPClient/Transport/WebSocketTransport.swift +++ b/Sources/MCPClient/Transport/WebSocketTransport.swift @@ -28,7 +28,7 @@ import NIOPosix public actor WebSocketTransport: MCPTransport { private let url: URL private let headers: [String: String] - private let trustSelfSignedCertificates: Bool + private let serverTrust: ServerTrust private var eventLoopGroup: (any EventLoopGroup)? private var webSocket: WebSocket? private var isConnected: Bool = false @@ -44,16 +44,43 @@ public actor WebSocketTransport: MCPTransport { /// - Parameters: /// - url: The WebSocket URL to connect to (ws:// or wss://). /// - headers: Optional HTTP headers to include in the upgrade request. - /// - trustSelfSignedCertificates: Accept self-signed or invalid TLS certificates. - /// Works on both macOS and Linux. + /// - serverTrust: Which certificate roots a `wss://` server may chain to. Defaults to + /// ``ServerTrust/system``. For a self-signed or privately issued certificate, supply + /// it with ``ServerTrust/onlyRoots(_:)`` or ``ServerTrust/additionalRoots(_:)``; the + /// chain and the hostname are verified either way. public init( url: URL, headers: [String: String] = [:], - trustSelfSignedCertificates: Bool = false + serverTrust: ServerTrust = .system ) { self.url = url self.headers = headers - self.trustSelfSignedCertificates = trustSelfSignedCertificates + self.serverTrust = serverTrust + } + + /// Removed: this did not trust a self-signed certificate, it disabled verification. + /// + /// Passing `true` set NIOSSL's `certificateVerification` to `.none`, which accepts any + /// certificate from anyone. Supply the certificate instead — see ``ServerTrust``. + /// + /// - Parameters: + /// - url: The WebSocket URL to connect to. + /// - headers: Optional HTTP headers to include in the upgrade request. + /// - trustSelfSignedCertificates: Ignored. Verification is never disabled. + @available(*, unavailable, message: "This disabled certificate verification entirely. Pass serverTrust: try .onlyRoots([.pemFile(path)]) (or .additionalRoots) with the server's certificate; omit the argument where it was false.") + public init( + url: URL, + headers: [String: String] = [:], + trustSelfSignedCertificates: Bool + ) { + self.url = url + self.headers = headers + self.serverTrust = .system + } + + /// The TLS configuration a `wss://` connection is made with. + nonisolated var tlsConfiguration: TLSConfiguration { + serverTrust.makeTLSConfiguration() } /// Establish a WebSocket connection to the MCP server. @@ -61,10 +88,7 @@ public actor WebSocketTransport: MCPTransport { let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) self.eventLoopGroup = group - var tlsConfig = TLSConfiguration.makeClientConfiguration() - if trustSelfSignedCertificates { - tlsConfig.certificateVerification = .none - } + let tlsConfig = tlsConfiguration var upgradeHeaders = HTTPHeaders() for (key, value) in headers { diff --git a/Sources/MCPExplorer/ConnectionView.swift b/Sources/MCPExplorer/ConnectionView.swift index 175a241..70b8a95 100644 --- a/Sources/MCPExplorer/ConnectionView.swift +++ b/Sources/MCPExplorer/ConnectionView.swift @@ -46,14 +46,17 @@ struct ConnectionView: View { ? "Signed in with OAuth; the token is managed for you." : "Used only if you are not signed in with OAuth.") - Toggle("Trust self-signed certificates", isOn: $vm.trustSelfSignedCertificates) - .font(.callout) } else { Text("Example: \(viewModel.transportType.exampleURL)") .font(.caption) .foregroundStyle(.secondary) } + TextField("Trusted certificate file (PEM, optional)", text: $vm.trustedCertificateFile) + .textFieldStyle(.roundedBorder) + .autocorrectionDisabled() + .help("For a self-signed or privately issued server: the path of its certificate, or of the authority that issued it. When set, only certificates in this file are trusted. The certificate must still name the host in the URL.") + case .stdio: TextField("Command", text: $vm.stdioCommand) .textFieldStyle(.roundedBorder) diff --git a/Sources/MCPExplorer/MCPViewModel.swift b/Sources/MCPExplorer/MCPViewModel.swift index 4cc1807..f0c0939 100644 --- a/Sources/MCPExplorer/MCPViewModel.swift +++ b/Sources/MCPExplorer/MCPViewModel.swift @@ -119,7 +119,9 @@ final class MCPViewModel { var stdioCommand: String = "" var stdioArguments: String = "" var transportType: TransportType = .httpSSE - var trustSelfSignedCertificates: Bool = false + /// Path of a PEM file holding the server's certificate, or the private authority that + /// issued it. Empty means the system roots. See ``TrustedCertificateFile``. + var trustedCertificateFile: String = "" var connectionState: ConnectionState = .disconnected var serverCapabilities: ServerCapabilities? @@ -164,6 +166,9 @@ final class MCPViewModel { do { let transport: MCPTransport + // Resolved before any transport exists, so an unreadable certificate file stops + // the connection here rather than becoming a connection made without it. + let serverTrust = try TrustedCertificateFile.serverTrust(for: trustedCertificateFile) switch transportType { case .httpSSE, .streamableHTTP: // SECURITY: URL is user-provided configuration entered in the UI @@ -199,9 +204,9 @@ final class MCPViewModel { url: url, headers: headers, authorization: provider, - trustSelfSignedCertificates: trustSelfSignedCertificates) + serverTrust: serverTrust) } else { - transport = HTTPSSETransport(url: url, headers: headers, trustSelfSignedCertificates: trustSelfSignedCertificates) + transport = HTTPSSETransport(url: url, headers: headers, serverTrust: serverTrust) } case .webSocket: @@ -210,7 +215,7 @@ final class MCPViewModel { connectionState = .error("Invalid URL") return } - transport = WebSocketTransport(url: url) + transport = WebSocketTransport(url: url, serverTrust: serverTrust) case .stdio: #if os(macOS) || os(Linux) diff --git a/Sources/MCPExplorer/TrustedCertificateFile.swift b/Sources/MCPExplorer/TrustedCertificateFile.swift new file mode 100644 index 0000000..5c50955 --- /dev/null +++ b/Sources/MCPExplorer/TrustedCertificateFile.swift @@ -0,0 +1,25 @@ +import Foundation +import MCPClient + +/// Turns the connection form's "trusted certificate" field into what a transport is given. +/// +/// The field replaces a toggle that read "Trust self-signed certificates" and switched +/// certificate verification off. Naming a file is the honest form of the same wish: the +/// server's certificate — or the private authority that issued it — is what gets trusted, and +/// the chain and hostname are checked as they are for any other server. +enum TrustedCertificateFile { + + /// The trust a connection should be made with. + /// + /// - Parameter field: What is in the form field: a path to a PEM file, or nothing. + /// - Returns: ``ServerTrust/system`` for an empty field; otherwise trust in the + /// certificates in that file and in nothing else, so a field left filled in cannot + /// quietly widen what a public server is checked against. + /// - Throws: ``ServerTrustError`` if a path was given and no certificate could be read + /// from it. The connection is not attempted. + static func serverTrust(for field: String) throws -> ServerTrust { + let path = field.trimmingCharacters(in: .whitespacesAndNewlines) + guard !path.isEmpty else { return .system } + return try .onlyRoots([.pemFile(path)]) + } +} diff --git a/Tests/MCPClientTests/HTTPSSETransportTests.swift b/Tests/MCPClientTests/HTTPSSETransportTests.swift index 9e899bd..1da3c5a 100644 --- a/Tests/MCPClientTests/HTTPSSETransportTests.swift +++ b/Tests/MCPClientTests/HTTPSSETransportTests.swift @@ -25,15 +25,6 @@ struct HTTPSSETransportTests { #expect(Bool(true), "Transport initialized successfully") } - @Test("Initializes with self-signed certificate trust") - func initWithSelfSignedTrust() throws { - _ = HTTPSSETransport( - url: try requireURL("https://mcp.example.com/sse"), - trustSelfSignedCertificates: true - ) - #expect(Bool(true), "Transport initialized successfully") - } - // MARK: - Send/Receive Before Connect @Test("Send throws connectionFailed when not connected") diff --git a/Tests/MCPClientTests/ServerTrustTests.swift b/Tests/MCPClientTests/ServerTrustTests.swift new file mode 100644 index 0000000..82f24d5 --- /dev/null +++ b/Tests/MCPClientTests/ServerTrustTests.swift @@ -0,0 +1,210 @@ +import Foundation +import Testing +import Crypto +import NIOSSL +@testable import MCPClient + +/// What a transport is prepared to believe about the server it is talking to. +/// +/// These replace three tests that constructed a transport with `trustSelfSignedCertificates: +/// true` and asserted nothing. What that flag did was set NIOSSL's verification to `.none`, +/// which is not trust in a self-signed certificate — it is trust in whoever answers. Every +/// test here asserts the configuration NIOSSL is actually handed, because the old defect was +/// invisible everywhere else. +@Suite("ServerTrust") +struct ServerTrustTests { + + // MARK: - The default + + @Test("The default verifies the chain and the hostname against the system roots") + func systemVerifiesFully() { + let configuration = ServerTrust.system.makeTLSConfiguration() + + #expect(configuration.certificateVerification == .fullVerification) + #expect(configuration.trustRoots == .default) + #expect(configuration.additionalTrustRoots == []) + #expect(ServerTrust.system.usesSystemRoots == true) + #expect(ServerTrust.system.rootFingerprints == []) + } + + // MARK: - Supplied roots + + @Test("Additional roots are added to the system roots, with verification left on") + func additionalRootsKeepSystemRoots() throws { + let identity = try TestIdentity.selfSigned() + let trust = try ServerTrust.additionalRoots([.pem(identity.pem)]) + let configuration = trust.makeTLSConfiguration() + + #expect(configuration.certificateVerification == .fullVerification) + #expect(configuration.trustRoots == .default) + #expect(configuration.additionalTrustRoots == [.certificates([try certificate(identity)])]) + #expect(trust.usesSystemRoots == true) + #expect(trust.rootFingerprints == [fingerprint(identity)]) + } + + @Test("Only-roots replaces the system roots, with verification left on") + func onlyRootsReplaceSystemRoots() throws { + let identity = try TestIdentity.selfSigned() + let trust = try ServerTrust.onlyRoots([.der(identity.der)]) + let configuration = trust.makeTLSConfiguration() + + #expect(configuration.certificateVerification == .fullVerification) + #expect(configuration.trustRoots == .certificates([try certificate(identity)])) + #expect(configuration.additionalTrustRoots == []) + #expect(trust.usesSystemRoots == false) + #expect(trust.rootFingerprints == [fingerprint(identity)]) + } + + @Test("A PEM bundle contributes every certificate in it, in order") + func pemBundleContributesEveryCertificate() throws { + let first = try TestIdentity.selfSigned(commonName: "first") + let second = try TestIdentity.selfSigned(commonName: "second") + let trust = try ServerTrust.onlyRoots([.pem(first.pem + "\n" + second.pem)]) + + #expect(trust.rootFingerprints == [fingerprint(first), fingerprint(second)]) + #expect(trust.makeTLSConfiguration().trustRoots + == .certificates([try certificate(first), try certificate(second)])) + } + + @Test("Several sources are concatenated in the order given") + func sourcesAreConcatenated() throws { + let first = try TestIdentity.selfSigned(commonName: "first") + let second = try TestIdentity.selfSigned(commonName: "second") + let trust = try ServerTrust.additionalRoots([.der(second.der), .pem(first.pem)]) + + #expect(trust.rootFingerprints == [fingerprint(second), fingerprint(first)]) + } + + @Test("A PEM file is read when the trust is made") + func pemFileIsRead() throws { + let identity = try TestIdentity.selfSigned() + let path = try TestIdentity.temporaryFile(Array(identity.pem.utf8), fileExtension: "pem") + defer { try? FileManager.default.removeItem(atPath: path) } // silent: best-effort cleanup of a temporary file + + let trust = try ServerTrust.onlyRoots([.pemFile(path)]) + + #expect(trust.rootFingerprints == [fingerprint(identity)]) + #expect(trust.makeTLSConfiguration().certificateVerification == .fullVerification) + } + + @Test("A DER file is read when the trust is made") + func derFileIsRead() throws { + let identity = try TestIdentity.selfSigned() + let path = try TestIdentity.temporaryFile(identity.der, fileExtension: "der") + defer { try? FileManager.default.removeItem(atPath: path) } // silent: best-effort cleanup of a temporary file + + let trust = try ServerTrust.additionalRoots([.derFile(path)]) + + #expect(trust.rootFingerprints == [fingerprint(identity)]) + } + + @Test("Equal trust compares equal, and different roots do not") + func equality() throws { + let identity = try TestIdentity.selfSigned() + let other = try TestIdentity.selfSigned() + + #expect(try ServerTrust.onlyRoots([.pem(identity.pem)]) == ServerTrust.onlyRoots([.der(identity.der)])) + #expect(try ServerTrust.onlyRoots([.pem(identity.pem)]) != ServerTrust.onlyRoots([.pem(other.pem)])) + #expect(try ServerTrust.onlyRoots([.pem(identity.pem)]) != ServerTrust.additionalRoots([.pem(identity.pem)])) + #expect(ServerTrust.system == ServerTrust.system) + } + + // MARK: - Failing closed + + /// An empty list under `onlyRoots` would be a client that trusts nothing, and under + /// `additionalRoots` one that silently means `.system`. Neither is what anyone wrote. + @Test("No sources is an error, not a silent default", arguments: [true, false]) + func emptySourcesThrow(only: Bool) { + #expect(throws: ServerTrustError.noCertificates) { + _ = only ? try ServerTrust.onlyRoots([]) : try ServerTrust.additionalRoots([]) + } + } + + @Test("Text that holds no certificate is refused") + func garbagePEMThrows() { + let source = ServerTrust.CertificateSource.pem("not a certificate") + #expect(throws: ServerTrustError.unreadable(source)) { + _ = try ServerTrust.onlyRoots([source]) + } + } + + @Test("Bytes that are not a DER certificate are refused") + func garbageDERThrows() { + let source = ServerTrust.CertificateSource.der([0x30, 0x03, 0x02, 0x01, 0x01]) + #expect(throws: ServerTrustError.unreadable(source)) { + _ = try ServerTrust.additionalRoots([source]) + } + } + + @Test("A file that is not there is refused rather than skipped") + func missingFileThrows() { + let source = ServerTrust.CertificateSource.pemFile("/nonexistent/mcp-trust-\(UUID().uuidString).pem") + #expect(throws: ServerTrustError.unreadable(source)) { + _ = try ServerTrust.onlyRoots([source]) + } + } + + /// One bad source must not be dropped while the good ones are kept: the caller asked for + /// a set of roots, and a subset of it is a different trust decision. + @Test("One unreadable source fails the whole set") + func oneBadSourceFailsTheSet() throws { + let identity = try TestIdentity.selfSigned() + let bad = ServerTrust.CertificateSource.pem("") + #expect(throws: ServerTrustError.unreadable(bad)) { + _ = try ServerTrust.additionalRoots([.pem(identity.pem), bad]) + } + } + + // MARK: - Every transport uses it + + @Test("Each transport verifies fully by default") + func transportsDefaultToFullVerification() throws { + let expected = ServerTrust.system.makeTLSConfiguration() + let http = try requireURL("https://mcp.example.com/mcp") + let socket = try requireURL("wss://mcp.example.com/ws") + + for configuration in [ + StreamableHTTPTransport(url: http).tlsConfiguration, + HTTPSSETransport(url: http).tlsConfiguration, + WebSocketTransport(url: socket).tlsConfiguration, + ] { + #expect(configuration.certificateVerification == .fullVerification) + #expect(configuration.trustRoots == expected.trustRoots) + #expect(configuration.additionalTrustRoots == []) + } + } + + @Test("Each transport hands NIOSSL the supplied roots with verification on") + func transportsCarrySuppliedRoots() throws { + let identity = try TestIdentity.selfSigned() + let trust = try ServerTrust.onlyRoots([.pem(identity.pem)]) + let roots = NIOSSLTrustRoots.certificates([try certificate(identity)]) + let http = try requireURL("https://mcp.example.com/mcp") + let socket = try requireURL("wss://mcp.example.com/ws") + + for configuration in [ + StreamableHTTPTransport(url: http, serverTrust: trust).tlsConfiguration, + HTTPSSETransport(url: http, serverTrust: trust).tlsConfiguration, + WebSocketTransport(url: socket, serverTrust: trust).tlsConfiguration, + ] { + #expect(configuration.certificateVerification == .fullVerification) + #expect(configuration.trustRoots == roots) + #expect(configuration.additionalTrustRoots == []) + } + } + + // MARK: - Helpers + + private func certificate(_ identity: TestIdentity) throws -> NIOSSLCertificate { + try NIOSSLCertificate(bytes: identity.der, format: .der) + } + + /// Computed here from the DER rather than read back from `ServerTrust`, so the expected + /// value does not come from the code under test. + private func fingerprint(_ identity: TestIdentity) -> String { + SHA256.hash(data: identity.der).map { byte in + let hex = String(byte, radix: 16) + return hex.count == 1 ? "0" + hex : hex + }.joined() + } +} diff --git a/Tests/MCPClientTests/ServerTrustWireTests.swift b/Tests/MCPClientTests/ServerTrustWireTests.swift new file mode 100644 index 0000000..94692ee --- /dev/null +++ b/Tests/MCPClientTests/ServerTrustWireTests.swift @@ -0,0 +1,169 @@ +import Foundation +import Testing +@testable import MCPClient + +/// Whether a handshake completes — asked of a real TLS server on loopback. +/// +/// A configuration can say `.fullVerification` and still be wrong about what it verifies +/// against, so these assert the only thing a server can observe: whether the request arrived. +/// A request that reached the server was sent over a connection the client agreed to; a count +/// of zero means the client refused before sending anything. +@Suite("ServerTrust — on the wire") +struct ServerTrustWireTests { + + // MARK: - Trusted + + @Test("A self-signed certificate supplied as the only root is trusted") + func selfSignedAsOnlyRoot() async throws { + let server = try TestIdentity.selfSigned() + let arrived = try await requests( + presenting: server, trusting: try .onlyRoots([.pem(server.pem)])) + #expect(arrived == 1) + } + + @Test("A self-signed certificate that claims to be a CA is trusted the same way") + func selfSignedAuthorityAsOnlyRoot() async throws { + let server = try TestIdentity.selfSigned(isAuthority: true) + let arrived = try await requests( + presenting: server, trusting: try .onlyRoots([.der(server.der)])) + #expect(arrived == 1) + } + + @Test("A self-signed certificate supplied as an additional root is trusted") + func selfSignedAsAdditionalRoot() async throws { + let server = try TestIdentity.selfSigned() + let arrived = try await requests( + presenting: server, trusting: try .additionalRoots([.pem(server.pem)])) + #expect(arrived == 1) + } + + @Test("A private CA is trusted for a leaf the client has never seen") + func privateAuthority() async throws { + let (authority, leaf) = try TestIdentity.authorityAndLeaf() + let arrived = try await requests( + presenting: leaf, trusting: try .onlyRoots([.pem(authority.pem)])) + #expect(arrived == 1) + } + + // MARK: - Refused + + /// The case the old flag existed for, and the one it got wrong: with nothing supplied, a + /// self-signed server is refused. + @Test("The default refuses a self-signed server") + func systemRefusesSelfSigned() async throws { + let server = try TestIdentity.selfSigned() + await #expect { + _ = try await requests(presenting: server, trusting: .system) + } throws: { Self.isTLSRefusal($0) } + } + + /// What `certificateVerification = .none` could not do. Trusting one self-signed + /// certificate is not trusting another: a server presenting a different one — which is + /// what an interposed attacker has — is refused before a byte of the request is sent. + @Test("Trusting one self-signed certificate does not trust another", + arguments: [true, false]) + func wrongCertificateFailsClosed(only: Bool) async throws { + let server = try TestIdentity.selfSigned() + let impostor = try TestIdentity.selfSigned() + let sources = [ServerTrust.CertificateSource.pem(server.pem)] + let trust = only ? try ServerTrust.onlyRoots(sources) : try ServerTrust.additionalRoots(sources) + + let recorder = ArrivalCount() + await #expect { + _ = try await requests(presenting: impostor, trusting: trust, recorder: recorder) + } throws: { Self.isTLSRefusal($0) } + #expect(await recorder.count == 0) + } + + @Test("A leaf from some other private CA is refused") + func wrongAuthorityFailsClosed() async throws { + let (authority, _) = try TestIdentity.authorityAndLeaf() + let (_, strangerLeaf) = try TestIdentity.authorityAndLeaf() + + let recorder = ArrivalCount() + await #expect { + _ = try await requests( + presenting: strangerLeaf, trusting: try .onlyRoots([.pem(authority.pem)]), + recorder: recorder) + } throws: { Self.isTLSRefusal($0) } + #expect(await recorder.count == 0) + } + + /// Supplying a root narrows *who signed it*, never *who it is for*. A trusted certificate + /// issued for another name is still the wrong server. + @Test("A trusted certificate for a different host is refused", arguments: [true, false]) + func hostnameIsStillChecked(only: Bool) async throws { + let server = try TestIdentity.selfSigned(names: [.dnsName("elsewhere.example")]) + let sources = [ServerTrust.CertificateSource.pem(server.pem)] + let trust = only ? try ServerTrust.onlyRoots(sources) : try ServerTrust.additionalRoots(sources) + + let recorder = ArrivalCount() + await #expect { + _ = try await requests(presenting: server, trusting: trust, recorder: recorder) + } throws: { Self.isTLSRefusal($0) } + #expect(await recorder.count == 0) + } + + // MARK: - Helpers + + /// Whether an error is the transport reporting a failed TLS handshake. + /// + /// "It threw" is not enough: a stub that never started, or a request that timed out, + /// throws too. The transport reports a refusal as `connectionFailed` carrying the TLS + /// layer's own error — NIOSSL's wherever roots were supplied, and Network.framework's on + /// an Apple platform when the system roots are in use. + private static func isTLSRefusal(_ error: any Error) -> Bool { + guard case .connectionFailed(let reason) = error as? MCPError else { return false } + return reason.contains("NIOSSL") || reason.contains("NWTLSError") + } + + /// Sends one request through a Streamable HTTP transport to an HTTPS stub. + /// + /// - Parameters: + /// - identity: What the server presents. + /// - trust: What the client is told to trust. + /// - recorder: Told how many requests arrived, on the failure path too — a throwing + /// call returns nothing, and "it threw" does not by itself say nothing was sent. + /// - Returns: How many requests reached the server. + private func requests( + presenting identity: TestIdentity, + trusting trust: ServerTrust, + recorder: ArrivalCount? = nil + ) async throws -> Int { + let server = try await StubHTTPServer.start( + replies: [.ok(#"{"jsonrpc":"2.0","id":1}"#)], + tls: try identity.serverContext()) + let transport = StreamableHTTPTransport( + url: try await server.url, + openServerStream: false, + connectionTimeout: 3, + serverTrust: trust) + try await transport.connect() + + do { + try await transport.send(Data(#"{"id":1}"#.utf8)) + _ = try await transport.receive() + } catch { + await recorder?.set(await server.received.count) + // silent: teardown after the failure this test is about + try? await transport.disconnect() + await server.stop() + throw error + } + let arrived = await server.received.count + await recorder?.set(arrived) + // silent: the DELETE that ends a session is not what is being asserted + try? await transport.disconnect() + await server.stop() + return arrived + } +} + +/// Carries a count out of a call that throws. +private actor ArrivalCount { + /// Starts at a value no server can report, so an assertion of zero cannot pass because + /// the helper never ran. + private(set) var count = -1 + + func set(_ value: Int) { count = value } +} diff --git a/Tests/MCPClientTests/StreamableHTTPTransportTests.swift b/Tests/MCPClientTests/StreamableHTTPTransportTests.swift index 5a31df5..2a21e6c 100644 --- a/Tests/MCPClientTests/StreamableHTTPTransportTests.swift +++ b/Tests/MCPClientTests/StreamableHTTPTransportTests.swift @@ -18,17 +18,7 @@ struct StreamableHTTPTransportTests { _ = StreamableHTTPTransport( url: try requireURL("https://mcp.example.com/mcp"), headers: ["Authorization": "Bearer token123"], - connectionTimeout: 60.0, - trustSelfSignedCertificates: false - ) - #expect(Bool(true), "Transport initialized successfully") - } - - @Test("Initializes with self-signed certificate trust") - func initWithSelfSignedTrust() throws { - _ = StreamableHTTPTransport( - url: try requireURL("https://mcp.example.com/mcp"), - trustSelfSignedCertificates: true + connectionTimeout: 60.0 ) #expect(Bool(true), "Transport initialized successfully") } diff --git a/Tests/MCPClientTests/StubHTTPServer.swift b/Tests/MCPClientTests/StubHTTPServer.swift index 81a295a..488ee52 100644 --- a/Tests/MCPClientTests/StubHTTPServer.swift +++ b/Tests/MCPClientTests/StubHTTPServer.swift @@ -2,6 +2,7 @@ import Foundation import NIOCore import NIOPosix import NIOHTTP1 +import NIOSSL /// An HTTP server on loopback that answers from a script and records what it was asked. /// @@ -105,27 +106,41 @@ actor StubHTTPServer { private var channel: Channel? private let recorder: Recorder - private init(recorder: Recorder) { + /// Whether the listener speaks TLS, which decides the scheme of ``url``. + private let servesTLS: Bool + + private init(recorder: Recorder, servesTLS: Bool) { self.recorder = recorder + self.servesTLS = servesTLS } /// Starts a server answering with `replies`. /// - /// - Parameter replies: The scripted responses, in order. + /// - Parameters: + /// - replies: The scripted responses, in order. + /// - serverStream: How to answer a `GET`, if at all. + /// - tls: The identity to present. Supplying one makes this an HTTPS server, which is + /// what the trust tests need: whether a request arrived at all is the only honest + /// evidence of what a client was willing to handshake with. /// - Returns: The running server. static func start( replies: [Reply], - serverStream: ServerStream? = nil + serverStream: ServerStream? = nil, + tls: NIOSSLContext? = nil ) async throws -> StubHTTPServer { let recorder = Recorder(replies: replies, serverStream: serverStream) - let server = StubHTTPServer(recorder: recorder) + let server = StubHTTPServer(recorder: recorder, servesTLS: tls != nil) let bootstrap = ServerBootstrap(group: MultiThreadedEventLoopGroup.singleton) .serverChannelOption(.backlog, value: 8) .serverChannelOption(.socketOption(.so_reuseaddr), value: 1) .childChannelInitializer { channel in - channel.pipeline.configureHTTPServerPipeline().flatMap { - channel.pipeline.addHandler(StubHandler(recorder: recorder)) + channel.eventLoop.makeCompletedFuture { + if let tls { + try channel.pipeline.syncOperations.addHandler(NIOSSLServerHandler(context: tls)) + } + try channel.pipeline.syncOperations.configureHTTPServerPipeline() + try channel.pipeline.syncOperations.addHandler(StubHandler(recorder: recorder)) } } @@ -145,7 +160,7 @@ actor StubHTTPServer { throw StubServerError.notListening } var components = URLComponents() - components.scheme = "http" + components.scheme = servesTLS ? "https" : "http" components.host = "127.0.0.1" components.port = port components.path = "/mcp" diff --git a/Tests/MCPClientTests/TestCertificates.swift b/Tests/MCPClientTests/TestCertificates.swift new file mode 100644 index 0000000..bc3b5d4 --- /dev/null +++ b/Tests/MCPClientTests/TestCertificates.swift @@ -0,0 +1,140 @@ +import Foundation +import Crypto +import NIOSSL +import SwiftASN1 +import X509 + +/// A certificate minted for one test run, with the key that goes with it. +/// +/// Generated rather than checked in. A fixture would mean a private key in the repository, +/// and one with an expiry date — a suite that goes red on a calendar day nobody remembers +/// choosing. These are valid for an hour around the moment they are made. +struct TestIdentity { + + /// The certificate, DER-encoded. + let der: [UInt8] + + /// The certificate, PEM-encoded. + let pem: String + + /// The private key, PEM-encoded. Only a server ever needs this. + let keyPEM: String + + /// A self-signed certificate naming the loopback address. + /// + /// - Parameters: + /// - commonName: The subject, and — being self-signed — the issuer. + /// - names: The subject alternative names. Defaults to what the loopback stub servers + /// are reached by, `127.0.0.1`; a test of hostname checking passes something else. + /// - isAuthority: Whether the certificate claims to be a CA. A self-signed server + /// certificate made by hand usually does not, so both shapes have to be trusted. + /// - Returns: The identity. + /// - Throws: If the certificate cannot be built or serialized. + static func selfSigned( + commonName: String = "SwiftMCPClient test server", + names: [GeneralName] = TestIdentity.loopbackNames, + isAuthority: Bool = false + ) throws -> TestIdentity { + let key = P256.Signing.PrivateKey() + let name = try DistinguishedName { CommonName(commonName) } + return try issue( + subject: name, subjectKey: key, issuer: name, issuerKey: key, + names: names, isAuthority: isAuthority) + } + + /// A private certificate authority and a server certificate it signed. + /// + /// - Returns: The authority, which a client is told to trust, and the leaf, which the + /// server presents. The client never sees the leaf ahead of time. + /// - Throws: If either certificate cannot be built or serialized. + static func authorityAndLeaf() throws -> (authority: TestIdentity, leaf: TestIdentity) { + let authorityKey = P256.Signing.PrivateKey() + let authorityName = try DistinguishedName { CommonName("SwiftMCPClient test CA") } + let authority = try issue( + subject: authorityName, subjectKey: authorityKey, + issuer: authorityName, issuerKey: authorityKey, + names: [], isAuthority: true) + + let leafKey = P256.Signing.PrivateKey() + let leaf = try issue( + subject: try DistinguishedName { CommonName("SwiftMCPClient test leaf") }, + subjectKey: leafKey, issuer: authorityName, issuerKey: authorityKey, + names: loopbackNames, isAuthority: false) + return (authority, leaf) + } + + /// The names the loopback stub servers answer to. + static let loopbackNames: [GeneralName] = [ + .ipAddress(ASN1OctetString(contentBytes: [127, 0, 0, 1])), + .dnsName("localhost"), + ] + + /// A server-side TLS context presenting this identity. + /// + /// - Returns: The context. + /// - Throws: If the certificate or key does not load. + func serverContext() throws -> NIOSSLContext { + let certificate = try NIOSSLCertificate(bytes: der, format: .der) + let key = try NIOSSLPrivateKey(bytes: Array(keyPEM.utf8), format: .pem) + let configuration = TLSConfiguration.makeServerConfiguration( + certificateChain: [.certificate(certificate)], + privateKey: .privateKey(key)) + return try NIOSSLContext(configuration: configuration) + } + + /// Writes `bytes` to a fresh file in the temporary directory. + /// + /// - Parameters: + /// - bytes: What to write. + /// - fileExtension: The extension, without the dot. + /// - Returns: The path. + /// - Throws: If the file cannot be written. + static func temporaryFile(_ bytes: [UInt8], fileExtension: String) throws -> String { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("mcp-trust-\(UUID().uuidString).\(fileExtension)") + try Data(bytes).write(to: url) + return url.path + } + + private static func issue( + subject: DistinguishedName, + subjectKey: P256.Signing.PrivateKey, + issuer: DistinguishedName, + issuerKey: P256.Signing.PrivateKey, + names: [GeneralName], + isAuthority: Bool + ) throws -> TestIdentity { + let now = Date() + let extensions = try Certificate.Extensions { + if isAuthority { + Critical(BasicConstraints.isCertificateAuthority(maxPathLength: nil)) + } else { + Critical(BasicConstraints.notCertificateAuthority) + } + if !names.isEmpty { + SubjectAlternativeNames(names) + // Apple's verifier refuses a TLS server certificate without this, and a + // certificate made for a server ought to say so anyway. + try ExtendedKeyUsage([.serverAuth]) + } + } + let certificate = try Certificate( + version: .v3, + serialNumber: Certificate.SerialNumber(), + publicKey: Certificate.PublicKey(subjectKey.publicKey), + notValidBefore: now.addingTimeInterval(-1800), + notValidAfter: now.addingTimeInterval(1800), + issuer: issuer, + subject: subject, + signatureAlgorithm: .ecdsaWithSHA256, + extensions: extensions, + issuerPrivateKey: Certificate.PrivateKey(issuerKey)) + + var serializer = DER.Serializer() + try serializer.serialize(certificate) + return TestIdentity( + der: serializer.serializedBytes, + pem: try certificate.serializeAsPEM().pemString, + keyPEM: subjectKey.pemRepresentation) + } +} diff --git a/Tests/MCPClientTests/WebSocketTransportTests.swift b/Tests/MCPClientTests/WebSocketTransportTests.swift index 50d0888..4cb0a34 100644 --- a/Tests/MCPClientTests/WebSocketTransportTests.swift +++ b/Tests/MCPClientTests/WebSocketTransportTests.swift @@ -19,13 +19,6 @@ struct WebSocketTransportTests { #expect(Bool(true), "Transport initialized successfully") } - @Test("WebSocketTransport initializes with self-signed certificate trust") - func initWithSelfSignedTrust() throws { - let url = try requireURL("wss://mcp.example.com/ws") - _ = WebSocketTransport(url: url, trustSelfSignedCertificates: true) - #expect(Bool(true), "Transport initialized successfully") - } - @Test("Send before connect throws") func sendBeforeConnect() async throws { let url = try requireURL("ws://localhost:1/mcp") diff --git a/Tests/MCPExplorerTests/TrustedCertificateFileTests.swift b/Tests/MCPExplorerTests/TrustedCertificateFileTests.swift new file mode 100644 index 0000000..e2abdf1 --- /dev/null +++ b/Tests/MCPExplorerTests/TrustedCertificateFileTests.swift @@ -0,0 +1,62 @@ +import Foundation +import Testing +import MCPClient +@testable import MCPExplorer + +/// What the Explorer's "trusted certificate" field turns into. +/// +/// The field replaces a toggle labelled "Trust self-signed certificates", which did not do +/// that: it turned certificate verification off. A file path cannot be mistaken for an off +/// switch, and these check that it is not one. +@Suite("Trusted certificate file") +struct TrustedCertificateFileTests { + + /// A public certificate with no key, valid until 2126. The fingerprint below is what + /// `openssl x509 -fingerprint -sha256` printed for it, so the expected value comes from + /// outside this package. + private static let fixture = """ + -----BEGIN CERTIFICATE----- + MIIBnjCCAUOgAwIBAgIUOectdds2c9YRe7YidIl1xm08aTYwCgYIKoZIzj0EAwIw + IzEhMB8GA1UEAwwYTUNQRXhwbG9yZXIgdGVzdCBmaXh0dXJlMCAXDTI2MTAwNDAz + NDQyMloYDzIxMjYwOTEwMDM0NDIyWjAjMSEwHwYDVQQDDBhNQ1BFeHBsb3JlciB0 + ZXN0IGZpeHR1cmUwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARwFCzOLzThrelP + LoO5bw5fFhnjaIrVt9VQBQRSA5ayW3ImvIVscAn4vbfelNykRjVMPk4nwMytBxmc + LZoJBsPPo1MwUTAdBgNVHQ4EFgQUQVCEFgY695yGih37zZBQkebov+IwHwYDVR0j + BBgwFoAUQVCEFgY695yGih37zZBQkebov+IwDwYDVR0TAQH/BAUwAwEB/zAKBggq + hkjOPQQDAgNJADBGAiEA1ef7enXlzDdYKV1wBhZTnzPKBWou0g7W4bGin5MKXKcC + IQDWwg6Z4tYtX8rPuMDW9APsO34xUCRAXzyfIv7KqaST4g== + -----END CERTIFICATE----- + + """ + private static let fixtureFingerprint = + "3efc4002d9fd0161d91cc8280d95d3df56e5ebcb2f55e4ca51bcab5045da53de" + + @Test("An empty field means the system roots", arguments: ["", " ", "\n\t"]) + func emptyMeansSystem(field: String) throws { + #expect(try TrustedCertificateFile.serverTrust(for: field) == ServerTrust.system) + } + + @Test("A certificate file becomes the only root") + func fileBecomesOnlyRoot() throws { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("explorer-trust-\(UUID().uuidString).pem") + try Data(Self.fixture.utf8).write(to: url) + defer { try? FileManager.default.removeItem(at: url) } // silent: best-effort cleanup of a temporary file + + // Padded, as a pasted path often is. + let trust = try TrustedCertificateFile.serverTrust(for: " \(url.path)\n") + + #expect(trust.usesSystemRoots == false) + #expect(trust.rootFingerprints == [Self.fixtureFingerprint]) + } + + /// A path that names nothing must stop the connection. Falling back to the system roots + /// would connect to a public server the user believed they had restricted. + @Test("A file that cannot be read is an error, not a fallback") + func unreadableFileThrows() { + let path = "/nonexistent/explorer-trust-\(UUID().uuidString).pem" + #expect(throws: ServerTrustError.unreadable(.pemFile(path))) { + _ = try TrustedCertificateFile.serverTrust(for: path) + } + } +}