diff --git a/passKit/Models/PasswordStore.swift b/passKit/Models/PasswordStore.swift index 742f13e..65bf31c 100644 --- a/passKit/Models/PasswordStore.swift +++ b/passKit/Models/PasswordStore.swift @@ -411,8 +411,7 @@ public class PasswordStore { return [keyID] } if Defaults.isEnableGPGIDOn { - let encryptedDataPath = password.fileURL(in: storeURL) - return findGPGIDs(from: encryptedDataPath) + return findGPGIDs(underPath: password.path) } return [] }() @@ -430,6 +429,37 @@ public class PasswordStore { AppKeychain.shared.removeContent(for: SSHKey.PRIVATE.getKeychainKey()) gitSSHPrivateKeyPassphrase = nil } + + func findGPGIDs(underPath relativePath: String) -> [String] { + guard let gpgIDFileURL = findGPGIDFile(atPath: relativePath) else { + return [] + } + let allKeysSeparatedByNewline = (try? String(contentsOf: gpgIDFileURL)) ?? "" + return allKeysSeparatedByNewline + .split(separator: "\n") + .map { String($0).trimmed } + .filter { !$0.isEmpty } + } + + func findGPGIDFile(atPath relativePath: String) -> URL? { + // Walk up the directory hierarchy, but never escape the store root + let storeRoot = storeURL.absoluteURL.resolvingSymlinksInPath() + var current = storeRoot.appendingPathComponent(relativePath).resolvingSymlinksInPath() + + while current.path.hasPrefix(storeRoot.path) { + let candidate = current.appendingPathComponent(".gpg-id") + var isDir: ObjCBool = false + if FileManager.default.fileExists(atPath: candidate.path, isDirectory: &isDir), !isDir.boolValue { + return candidate.standardizedFileURL + } + let parent = current.deletingLastPathComponent().resolvingSymlinksInPath() + if parent.path == current.path { + break + } + current = parent + } + return nil + } } extension PasswordStore { @@ -462,18 +492,3 @@ extension PasswordStore { return try gitRepository.commit(signature: gitSignatureForNow, message: message) } } - -func findGPGIDs(from url: URL) -> [String] { - var path = url - while !FileManager.default.fileExists(atPath: path.appendingPathComponent(".gpg-id").path), - path.path != "file:///" { - path = path.deletingLastPathComponent() - } - path = path.appendingPathComponent(".gpg-id") - - let allKeysSeparatedByNewline = (try? String(contentsOf: path)) ?? "" - return allKeysSeparatedByNewline - .split(separator: "\n") - .map { String($0).trimmed } - .filter { !$0.isEmpty } -} diff --git a/passKitTests/Models/PasswordStoreTest.swift b/passKitTests/Models/PasswordStoreTest.swift index e9b3c44..52b683c 100644 --- a/passKitTests/Models/PasswordStoreTest.swift +++ b/passKitTests/Models/PasswordStoreTest.swift @@ -332,12 +332,85 @@ final class PasswordStoreTest: XCTestCase { waitForExpectations(timeout: 1, handler: nil) } - // MARK: - .gpg-id support + // MARK: - Find .gpg-id + + func testFindGPGIDFile() throws { + try FileManager.default.createDirectory(at: localRepoURL, withIntermediateDirectories: true) + XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data("under root".utf8))) + + try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("foo/bar/baz"), withIntermediateDirectories: true) + XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path, contents: Data("under foo".utf8))) + + try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/"), withIntermediateDirectories: true) + try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/hey"), withIntermediateDirectories: true) + XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/hey/.gpg-id").path, contents: Data("under hey".utf8))) + + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path)) + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "/")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path)) + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "doesnt-exist")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path)) + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/..")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path)) + + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path)) + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/bar")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path)) + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/bar/baz")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path)) + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/doesnt-exist")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path)) + + // there is a _drectory_ called .gpg-id in here + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "weird-subdir")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path)) + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "weird-subdir/.gpg-id")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path)) + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "weird-subdir/.gpg-id/hey")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/hey/.gpg-id").path)) + + // "foo/bar/../../baz" resolves to "baz" which has no .gpg-id, so should find root's. + // Without path resolution, the walk ["foo","bar","..","..","baz"] → remove "baz" → remove ".." → + // "foo/bar/.." → remove ".." → "foo/bar" → finds foo/.gpg-id (wrong). + try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("baz"), withIntermediateDirectories: true) + XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/bar/../../baz")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path)) + } + + func testMissingGPGIDFile() throws { + XCTAssertFalse(FileManager.default.fileExists(atPath: localRepoURL.appendingPathComponent(".gpg-id").path)) + try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("subdir"), withIntermediateDirectories: true) + + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "")) + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "subdir")) + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "missing")) + } + + func testFindGPGIDFileStopsAtRoot() throws { + // Place a .gpg-id file ABOVE the store root, this should not be found + let parentDir = localRepoURL.deletingLastPathComponent() + let escapedGPGIDURL = parentDir.appendingPathComponent(".gpg-id") + XCTAssertTrue(FileManager.default.createFile(atPath: escapedGPGIDURL.path, contents: Data("ESCAPED_KEY".utf8))) + defer { try? FileManager.default.removeItem(at: escapedGPGIDURL) } + + // Store has no .gpg-id at all + try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("sub/deep"), withIntermediateDirectories: true) + // Direct paths, should not find the escaped .gpg-id since it's outside the store root + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "")) + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub")) + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/deep")) + + // Path traversal attempts via ".." + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "..")) + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "../..")) + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/../..")) + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/deep/../../..")) + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/deep/../../../../../etc")) + + // Symlink escape: create a symlink inside the store pointing outside + let evilDir = parentDir.appendingPathComponent("evil") + try FileManager.default.createDirectory(at: evilDir, withIntermediateDirectories: true) + XCTAssertTrue(FileManager.default.createFile(atPath: evilDir.appendingPathComponent(".gpg-id").path, contents: Data("EVIL_KEY".utf8))) + defer { try? FileManager.default.removeItem(at: evilDir) } + try FileManager.default.createSymbolicLink(at: localRepoURL.appendingPathComponent("sub/escape"), withDestinationURL: evilDir) + // Following the symlink would find evil/.gpg-id — must not happen + XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/escape")) + } + + // MARK: Parse .gpg-id func testReadGPGIDFile() throws { try cloneRepository(.withGPGID) - Defaults.isEnableGPGIDOn = true - [ ("", [RSA4096.longFingerprint]), ("family", [String(NISTP384.longFingerprint.suffix(16))]), @@ -345,11 +418,36 @@ final class PasswordStoreTest: XCTestCase { ("shared", [RSA2048.longFingerprint, RSA4096.longFingerprint]), ("work", [RSA2048.longFingerprint]), ].forEach { path, expectedKeyIDs in - let foundKeyIDs = findGPGIDs(from: localRepoURL.appendingPathComponent(path)) - XCTAssertEqual(foundKeyIDs, expectedKeyIDs.map { $0.uppercased() }) + let foundKeyIDs = passwordStore.findGPGIDs(underPath: path) + XCTAssertEqual(foundKeyIDs, expectedKeyIDs.map { $0.uppercased() }, "Failed for path: \(path)") } } + func testReadEmptyGPGIDFile() throws { + try FileManager.default.createDirectory(at: localRepoURL, withIntermediateDirectories: true) + + XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: nil)) + XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), []) + + XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data(" \n\t".utf8))) + XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), []) + } + + func testReadGPGIDFileWithWhitespace() throws { + try FileManager.default.createDirectory(at: localRepoURL, withIntermediateDirectories: true) + + XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: nil)) + XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), []) + + XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data(" \n\t".utf8))) + XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), []) + + XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data(" \nbar foo\n\tbaz\n \n".utf8))) + XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), ["bar foo", "baz"]) + } + + // MARK: Handle .gpg-id + func testAddPasswordInRoot_WithSingleEntryInPGPIDFile_EncryptsWithThatKey() throws { let mockPGPInterface = setUpMockedPGPInterface() mockPGPInterface.publicKeyIDs = Set(RSA2048_RSA4096.fingerprints) @@ -436,6 +534,48 @@ final class PasswordStoreTest: XCTestCase { } } + func testGPGIDDisabledIgnoresGPGIDFile() throws { + try cloneRepository(.withGPGID) + try importSinglePGPKey() // Only import RSA4096, but not RSA2048 + Defaults.isEnableGPGIDOn = false + + // /work uses RSA2048, but we didn't import that one + let testPassword = Password(name: "test", path: "work/test.gpg", plainText: "testpassword") + // this would throw if isEnableGPGIDOn was true, since we are missing the key to encrypt it + _ = try passwordStore.add(password: testPassword) + + // check that we can decrypt it with the key we have, which confirms that it was encrypted without using the .gpg-id file + let decryptedPassword = try decrypt(path: "work/test.gpg", keyID: RSA4096.longFingerprint) + XCTAssertEqual(decryptedPassword.plainText, "testpassword") + + // we can't even decrypt it with RSA2048 + try importMultiplePGPKeys() + XCTAssertThrowsError(try decrypt(path: "work/test.gpg", keyID: RSA2048.longFingerprint)) { + XCTAssertEqual($0 as? AppError, .keyExpiredOrIncompatible) + } + } + + func testEncryptWithExplicitKeyID_OverridesGPGIDFile() throws { + continueAfterFailure = false // avoid index out of bounds error below + + let mockPGPInterface = setUpMockedPGPInterface() + mockPGPInterface.publicKeyIDs = Set(RSA2048_RSA4096.fingerprints) + try cloneRepository(.withGPGID) + Defaults.isEnableGPGIDOn = true + + // Even though /personal would normally use RSA4096 from the root .gpg-id file, if we explicitly specify a key ID then that should be used instead + let testPassword1 = Password(name: "test1", path: "personal/test1.gpg", plainText: "testpassword1") + _ = try passwordStore.add(password: testPassword1) + let testPassword2 = Password(name: "test2", path: "personal/test2.gpg", plainText: "testpassword2") + _ = try passwordStore.add(password: testPassword2, keyID: RSA2048.longFingerprint) + + XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls.count, 2) + XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[0].plainData, testPassword1.plainData) + XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[0].keyIDs, [RSA4096.longFingerprint].map { $0.uppercased() }) + XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[1].plainData, testPassword2.plainData) + XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[1].keyIDs, [RSA2048.longFingerprint]) + } + // MARK: - Helpers private enum RemoteRepo {