diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d5f1b8f..badb960f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 as installed, reported with heuristic confidence; a bottle whose log already says installed is never probed (#233). +### Security +- Importing a Game Porting Toolkit payload now verifies that the D3DMetal + framework and its shared library carry Apple's code signature before + anything is copied into the store. A payload assembled by hand or altered + after download is refused with the file named, instead of being deployed + into every bottle's Wine tree. The PE forwarders, which cannot be signed, + keep their builtin-marker check (#265). + ### Removed - ClickOnce support. Games do not arrive as `.appref-ms` deployments, and nobody spoke up for it during the window on #215. The manager, its diff --git a/Whisky/Localizable.xcstrings b/Whisky/Localizable.xcstrings index 8e6c21d2..00a77952 100644 --- a/Whisky/Localizable.xcstrings +++ b/Whisky/Localizable.xcstrings @@ -57131,6 +57131,23 @@ } } }, + "gptk.error.notAppleSigned" : { + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "This is not a genuine Game Porting Toolkit payload, a file is not signed by Apple:" + } + }, + "en-GB" : { + "stringUnit" : { + "state" : "translated", + "value" : "This is not a genuine Game Porting Toolkit payload, a file is not signed by Apple:" + } + } + } + }, "gptk.error.payloadIncomplete" : { "extractionState" : "manual", "localizations" : { diff --git a/WhiskyKit/Sources/WhiskyKit/WhiskyWine/GPTKImporter.swift b/WhiskyKit/Sources/WhiskyKit/WhiskyWine/GPTKImporter.swift index 3c45bdc4..70f2c353 100644 --- a/WhiskyKit/Sources/WhiskyKit/WhiskyWine/GPTKImporter.swift +++ b/WhiskyKit/Sources/WhiskyKit/WhiskyWine/GPTKImporter.swift @@ -18,6 +18,7 @@ import Foundation import os.log +import Security /// Errors thrown while importing or deploying a GPTK payload. public enum GPTKImportError: LocalizedError, Equatable { @@ -30,6 +31,10 @@ public enum GPTKImportError: LocalizedError, Equatable { case forwarderNotBuiltin(String) /// The D3DMetal framework's version could not be read. case versionUnreadable + /// An Apple binary in the payload (payload-relative path) does not carry a + /// valid Apple code signature, so this is not a genuine GPTK payload or it + /// was altered after download. + case notAppleSigned(String) /// No imported payload exists in the store to deploy or remove. case storeEmpty @@ -43,6 +48,8 @@ public enum GPTKImportError: LocalizedError, Equatable { String(localized: "gptk.error.forwarderNotBuiltin") + " " + name case .versionUnreadable: String(localized: "gptk.error.versionUnreadable") + case let .notAppleSigned(name): + String(localized: "gptk.error.notAppleSigned") + " " + name case .storeEmpty: String(localized: "gptk.error.storeEmpty") } @@ -170,12 +177,32 @@ public enum GPTKImporter { // MARK: - Validation + /// The Apple-built binaries whose code signature must chain to Apple's + /// root, payload-relative to `external/`. The unix bridge entries are + /// symlinks to the dylib, so checking it covers them. + static let appleSignedNames = ["libd3dshared.dylib", "D3DMetal.framework"] + /// Validates completeness and authenticity of the payload at `libRoot` and /// reads its version. /// + /// Authenticity has two halves. The PE forwarders cannot be code-signed, + /// so they are checked for the winebuild builtin marker Apple's build + /// leaves in them. The Mach-O half (the shared library and the D3DMetal + /// framework) is signed by Apple's own software-signing chain, so it is + /// verified against an `anchor apple` requirement; a payload assembled by + /// hand or altered after download fails here with the file named. The + /// signature check runs last so that the cheaper, more common mistakes + /// (a wrong folder, a missing file) are reported first. + /// + /// - Parameter isAppleSigned: the signature verifier, injectable so tests + /// can validate fixtures that are not real code objects. /// - Throws: ``GPTKImportError`` when files are missing, a forwarder is not - /// the builtin variant, or the version is unreadable. - public static func validatePayload(at libRoot: URL) throws -> GPTKPayload { + /// the builtin variant, the version is unreadable, or an Apple binary + /// fails the signature check. + public static func validatePayload( + at libRoot: URL, + isAppleSigned: (URL) -> Bool = GPTKImporter.isAppleSigned + ) throws -> GPTKPayload { let fileManager = FileManager.default let peDir = libRoot.appending(path: "wine").appending(path: "x86_64-windows") let external = libRoot.appending(path: "external") @@ -208,9 +235,39 @@ public enum GPTKImporter { guard let version = frameworkVersion(inExternal: external) else { throw GPTKImportError.versionUnreadable } + + for name in appleSignedNames where !isAppleSigned(external.appending(path: name)) { + throw GPTKImportError.notAppleSigned("external/\(name)") + } return GPTKPayload(libRoot: libRoot, version: version) } + /// Whether the code object at `url` (a Mach-O file or a bundle) carries a + /// valid signature that chains to Apple's root certificate. + /// + /// This is `codesign --verify -R="anchor apple"`: the static code must be + /// well-formed, every sealed resource must match, and the signing chain + /// must end at Apple. Anything unsigned, ad-hoc signed, or signed by a + /// third party fails, as does a path that is not a code object at all. + public static func isAppleSigned(_ url: URL) -> Bool { + var staticCode: SecStaticCode? + guard SecStaticCodeCreateWithPath(url as CFURL, [], &staticCode) == errSecSuccess, + let staticCode + else { return false } + + var requirement: SecRequirement? + guard SecRequirementCreateWithString("anchor apple" as CFString, [], &requirement) == errSecSuccess, + let requirement + else { return false } + + let status = SecStaticCodeCheckValidity(staticCode, [], requirement) + if status != errSecSuccess { + let name = url.lastPathComponent + logger.info("Apple signature check failed for \(name, privacy: .public): \(status, privacy: .public)") + } + return status == errSecSuccess + } + /// Reads `CFBundleShortVersionString` from the framework's Info.plist, /// looking in the versioned bundle layout first and the flat layout second. static func frameworkVersion(inExternal external: URL) -> String? { diff --git a/WhiskyKit/Tests/WhiskyKitTests/GPTKDeploymentTests.swift b/WhiskyKit/Tests/WhiskyKitTests/GPTKDeploymentTests.swift index 43256428..ea959609 100644 --- a/WhiskyKit/Tests/WhiskyKitTests/GPTKDeploymentTests.swift +++ b/WhiskyKit/Tests/WhiskyKitTests/GPTKDeploymentTests.swift @@ -110,7 +110,8 @@ struct GPTKDeploymentTests { let store = GPTKImporter.storeFolder(inApplicationFolder: appSupport) let lib = tempDir.appending(path: "payload") try makePayload(at: lib) - try GPTKImporter.importPayload(GPTKImporter.validatePayload(at: lib), intoStore: store) + let payload = try GPTKImporter.validatePayload(at: lib, isAppleSigned: { _ in true }) + try GPTKImporter.importPayload(payload, intoStore: store) let runtime = appSupport.appending(path: "Libraries") try makeRuntime(at: runtime) diff --git a/WhiskyKit/Tests/WhiskyKitTests/GPTKFixtures.swift b/WhiskyKit/Tests/WhiskyKitTests/GPTKFixtures.swift index 21644901..c17d425c 100644 --- a/WhiskyKit/Tests/WhiskyKitTests/GPTKFixtures.swift +++ b/WhiskyKit/Tests/WhiskyKitTests/GPTKFixtures.swift @@ -247,6 +247,7 @@ func makeImportedStore(in tempDir: URL) throws -> URL { let lib = tempDir.appending(path: "payload") let store = tempDir.appending(path: "store") try makePayload(at: lib) - try GPTKImporter.importPayload(GPTKImporter.validatePayload(at: lib), intoStore: store) + let payload = try GPTKImporter.validatePayload(at: lib, isAppleSigned: { _ in true }) + try GPTKImporter.importPayload(payload, intoStore: store) return store } diff --git a/WhiskyKit/Tests/WhiskyKitTests/GPTKImporterTests.swift b/WhiskyKit/Tests/WhiskyKitTests/GPTKImporterTests.swift index 6becef57..3147c863 100644 --- a/WhiskyKit/Tests/WhiskyKitTests/GPTKImporterTests.swift +++ b/WhiskyKit/Tests/WhiskyKitTests/GPTKImporterTests.swift @@ -53,12 +53,68 @@ struct GPTKImporterTests { let lib = tempDir.appending(path: "lib") try makePayload(at: lib) - let payload = try GPTKImporter.validatePayload(at: lib) + let payload = try GPTKImporter.validatePayload(at: lib, isAppleSigned: { _ in true }) #expect(payload.version == "4.0b2") #expect(payload.libRoot == lib) } + @Test("An unsigned shared library is rejected by the real signature check") + func validateUnsignedPayload() throws { + let lib = tempDir.appending(path: "lib") + try makePayload(at: lib) + + #expect(throws: GPTKImportError.notAppleSigned("external/libd3dshared.dylib")) { + try GPTKImporter.validatePayload(at: lib) + } + } + + @Test("The signature check covers the shared library and the framework, in that order") + func validateChecksBothAppleBinaries() throws { + let lib = tempDir.appending(path: "lib") + try makePayload(at: lib) + var checked: [String] = [] + + _ = try GPTKImporter.validatePayload(at: lib, isAppleSigned: { url in + checked.append(url.lastPathComponent) + return true + }) + + #expect(checked == ["libd3dshared.dylib", "D3DMetal.framework"]) + } + + @Test("A framework that fails the signature check is named") + func validateUnsignedFramework() throws { + let lib = tempDir.appending(path: "lib") + try makePayload(at: lib) + + #expect(throws: GPTKImportError.notAppleSigned("external/D3DMetal.framework")) { + try GPTKImporter.validatePayload(at: lib, isAppleSigned: { url in + url.lastPathComponent != "D3DMetal.framework" + }) + } + } + + @Test("Completeness and variant errors take precedence over the signature check") + func validateSignatureCheckRunsLast() throws { + let lib = tempDir.appending(path: "lib") + try makePayload(at: lib, builtinForwarders: false) + + #expect(throws: GPTKImportError.forwarderNotBuiltin("d3d10.dll")) { + try GPTKImporter.validatePayload(at: lib) + } + } + + @Test("The Apple signature check accepts a system binary and refuses a plain file") + func appleSignatureCheck() throws { + let plain = tempDir.appending(path: "plain.dylib") + try Data("not a code object".utf8).write(to: plain) + + #expect(GPTKImporter.isAppleSigned(URL(filePath: "/bin/ls"))) + #expect(!GPTKImporter.isAppleSigned(plain)) + #expect(!GPTKImporter.isAppleSigned(tempDir.appending(path: "missing.dylib"))) + } + @Test("Missing forwarders are reported by name") func validateMissingForwarder() throws { let lib = tempDir.appending(path: "lib") @@ -118,7 +174,7 @@ struct GPTKImporterTests { let lib = tempDir.appending(path: "lib") let store = tempDir.appending(path: "store") try makePayload(at: lib) - let payload = try GPTKImporter.validatePayload(at: lib) + let payload = try GPTKImporter.validatePayload(at: lib, isAppleSigned: { _ in true }) let record = try GPTKImporter.importPayload(payload, intoStore: store) @@ -138,7 +194,7 @@ struct GPTKImporterTests { let lib = tempDir.appending(path: "lib") let store = tempDir.appending(path: "store") try makePayload(at: lib, unixEntriesAsFiles: true) - let payload = try GPTKImporter.validatePayload(at: lib) + let payload = try GPTKImporter.validatePayload(at: lib, isAppleSigned: { _ in true }) try GPTKImporter.importPayload(payload, intoStore: store) @@ -157,7 +213,7 @@ struct GPTKImporterTests { let lib = tempDir.appending(path: "lib") try makePayload(at: lib) - let payload = try GPTKImporter.validatePayload(at: lib) + let payload = try GPTKImporter.validatePayload(at: lib, isAppleSigned: { _ in true }) try GPTKImporter.importPayload(payload, intoStore: store) try FileManager.default.removeItem( at: store.appending(path: "lib").appending(path: "external")