From 76db5297649b10d0438e8e4406a693742598e7d4 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 10:34:54 +0100 Subject: [PATCH 01/21] add long fingerprints for test keys --- passKitTests/Testbase/TestPGPKeys.swift | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/passKitTests/Testbase/TestPGPKeys.swift b/passKitTests/Testbase/TestPGPKeys.swift index da2177c..fd6d8bf 100644 --- a/passKitTests/Testbase/TestPGPKeys.swift +++ b/passKitTests/Testbase/TestPGPKeys.swift @@ -16,6 +16,7 @@ struct PGPTestSet { let publicKey: String let privateKey: String let fingerprint: String + let longFingerprint: String let passphrase: String fileprivate func collect() -> Self { // swiftlint:disable:this strict_fileprivate @@ -28,6 +29,7 @@ struct MultiKeyPGPTestSet { let publicKeys: String let privateKeys: String let fingerprints: [String] + let longFingerprints: [String] let passphrases: [String] } @@ -35,6 +37,7 @@ let RSA2048 = PGPTestSet( publicKey: PGP_RSA2048_PUBLIC_KEY, privateKey: PGP_RSA2048_PRIVATE_KEY, fingerprint: "a1024dae", + longFingerprint: "4712286271220db299883ea7062e678da1024dae", passphrase: "passforios" ).collect() @@ -42,6 +45,7 @@ let RSA2048_SUB = PGPTestSet( publicKey: PGP_RSA2048_PUBLIC_KEY, privateKey: PGP_RSA2048_PRIVATE_SUBKEY, fingerprint: "a1024dae", + longFingerprint: "4712286271220db299883ea7062e678da1024dae", passphrase: "passforios" ) @@ -49,6 +53,7 @@ let RSA3072_NO_PASSPHRASE = PGPTestSet( publicKey: PGP_RSA3072_PUBLIC_KEY_NO_PASSPHRASE, privateKey: PGP_RSA3072_PRIVATE_KEY_NO_PASSPHRASE, fingerprint: "be0f9402", + longFingerprint: "b37cd5669a03f0d46735a2ba35fba3d0be0f9402", passphrase: "" ) @@ -56,6 +61,7 @@ let RSA4096 = PGPTestSet( publicKey: PGP_RSA4096_PUBLIC_KEY, privateKey: PGP_RSA4096_PRIVATE_KEY, fingerprint: "d862027e", + longFingerprint: "787eae1a5fa3e749aa34cc6aa0645ebed862027e", passphrase: "passforios" ).collect() @@ -63,6 +69,7 @@ let RSA4096_SUB = PGPTestSet( publicKey: PGP_RSA4096_PUBLIC_KEY, privateKey: PGP_RSA4096_PRIVATE_SUBKEY, fingerprint: "d862027e", + longFingerprint: "787eae1a5fa3e749aa34cc6aa0645ebed862027e", passphrase: "passforios" ) @@ -70,6 +77,7 @@ let ED25519 = PGPTestSet( publicKey: PGP_ED25519_PUBLIC_KEY, privateKey: PGP_ED25519_PRIVATE_KEY, fingerprint: "e9444483", + longFingerprint: "5fccb081ab8af48972999e2ae750acbfe9444483", passphrase: "passforios" ).collect() @@ -77,6 +85,7 @@ let ED25519_SUB = PGPTestSet( publicKey: PGP_ED25519_PUBLIC_KEY, privateKey: PGP_ED25519_PRIVATE_SUBKEY, fingerprint: "e9444483", + longFingerprint: "5fccb081ab8af48972999e2ae750acbfe9444483", passphrase: "passforios" ) @@ -84,6 +93,7 @@ let NISTP384 = PGPTestSet( publicKey: PGP_NISTP384_PUBLIC_KEY, privateKey: PGP_NISTP384_PRIVATE_KEY, fingerprint: "5af3c085", + longFingerprint: "bcd364c078585c0607e19c67171c07d25af3c085", passphrase: "soirofssap" ).collect() @@ -91,6 +101,7 @@ let RSA2048_RSA4096 = MultiKeyPGPTestSet( publicKeys: PGP_RSA2048_PUBLIC_KEY | PGP_RSA4096_PUBLIC_KEY, privateKeys: PGP_RSA2048_PRIVATE_KEY | PGP_RSA4096_PRIVATE_KEY, fingerprints: ["a1024dae", "d862027e"], + longFingerprints: ["4712286271220db299883ea7062e678da1024dae", "787eae1a5fa3e749aa34cc6aa0645ebed862027e"], passphrases: ["passforios", "passforios"] ) @@ -98,6 +109,7 @@ let ED25519_NISTP384 = MultiKeyPGPTestSet( publicKeys: PGP_ED25519_PUBLIC_KEY | PGP_NISTP384_PUBLIC_KEY, privateKeys: PGP_ED25519_PRIVATE_KEY | PGP_NISTP384_PRIVATE_KEY, fingerprints: ["e9444483", "5af3c085"], + longFingerprints: ["5fccb081ab8af48972999e2ae750acbfe9444483", "bcd364c078585c0607e19c67171c07d25af3c085"], passphrases: ["passforios", "soirofssap"] ) From d136175d93e0c396246b5ef2190e24f061d557da Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 11:36:36 +0100 Subject: [PATCH 02/21] add detailed API tests checking how calls to PGPAgent propagate to the underlying interface this is refactoring support, so that we can notice changes in how the underlying APIs are called, and make changes intentionally when needed, instead of accidentally. --- pass.xcodeproj/project.pbxproj | 24 + passKit/Crypto/PGPAgent.swift | 5 + .../LowLevel/PGPAgentLowLevelTests.swift | 661 ++++++++++++++++++ passKitTests/Mocks/MockPGPInterface.swift | 70 ++ 4 files changed, 760 insertions(+) create mode 100644 passKitTests/LowLevel/PGPAgentLowLevelTests.swift create mode 100644 passKitTests/Mocks/MockPGPInterface.swift diff --git a/pass.xcodeproj/project.pbxproj b/pass.xcodeproj/project.pbxproj index d04dd81..4b5755f 100644 --- a/pass.xcodeproj/project.pbxproj +++ b/pass.xcodeproj/project.pbxproj @@ -116,6 +116,8 @@ 5F9D7B0F27AF6FD200A8AB22 /* CryptoTokenKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5F9D7B0C27AF6F7300A8AB22 /* CryptoTokenKit.framework */; settings = {ATTRIBUTES = (Weak, ); }; }; 8A4716692F5EF56900C7A64D /* AppKeychainTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A4716682F5EF56900C7A64D /* AppKeychainTest.swift */; }; 8A4716712F5EF7A900C7A64D /* PersistenceControllerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A47166F2F5EF7A900C7A64D /* PersistenceControllerTest.swift */; }; + 8AB3AD8C2F615FA50081DE16 /* MockPGPInterface.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AB3AD8A2F615FA50081DE16 /* MockPGPInterface.swift */; }; + 8AB3AD8D2F615FA50081DE16 /* PGPAgentLowLevelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AB3AD8B2F615FA50081DE16 /* PGPAgentLowLevelTests.swift */; }; 8AD8EBF32F5E2723007475AB /* Fixtures in Resources */ = {isa = PBXBuildFile; fileRef = 8AD8EBF22F5E268D007475AB /* Fixtures */; }; 9A1D1CE526E5D1CE0052028E /* OneTimePassword in Frameworks */ = {isa = PBXBuildFile; productRef = 9A1D1CE426E5D1CE0052028E /* OneTimePassword */; }; 9A1D1CE726E5D2230052028E /* OneTimePassword in Frameworks */ = {isa = PBXBuildFile; productRef = 9A1D1CE626E5D2230052028E /* OneTimePassword */; }; @@ -427,6 +429,8 @@ 5F9D7B0C27AF6F7300A8AB22 /* CryptoTokenKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CryptoTokenKit.framework; path = System/Library/Frameworks/CryptoTokenKit.framework; sourceTree = SDKROOT; }; 8A4716682F5EF56900C7A64D /* AppKeychainTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppKeychainTest.swift; sourceTree = ""; }; 8A47166F2F5EF7A900C7A64D /* PersistenceControllerTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistenceControllerTest.swift; sourceTree = ""; }; + 8AB3AD8A2F615FA50081DE16 /* MockPGPInterface.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockPGPInterface.swift; sourceTree = ""; }; + 8AB3AD8B2F615FA50081DE16 /* PGPAgentLowLevelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PGPAgentLowLevelTests.swift; sourceTree = ""; }; 8AD8EBF22F5E268D007475AB /* Fixtures */ = {isa = PBXFileReference; lastKnownFileType = folder; path = Fixtures; sourceTree = ""; }; 9A1EF0B324C50DD80074FEAC /* passBeta.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = passBeta.entitlements; sourceTree = ""; }; 9A1EF0B424C50E780074FEAC /* passBetaAutoFillExtension.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = passBetaAutoFillExtension.entitlements; sourceTree = ""; }; @@ -776,6 +780,22 @@ path = Controllers; sourceTree = ""; }; + 8AB3AD8E2F615FD70081DE16 /* Mocks */ = { + isa = PBXGroup; + children = ( + 8AB3AD8A2F615FA50081DE16 /* MockPGPInterface.swift */, + ); + path = Mocks; + sourceTree = ""; + }; + 8AB3AD8F2F61600B0081DE16 /* LowLevel */ = { + isa = PBXGroup; + children = ( + 8AB3AD8B2F615FA50081DE16 /* PGPAgentLowLevelTests.swift */, + ); + path = LowLevel; + sourceTree = ""; + }; 9A58664F25AADB66006719C2 /* Services */ = { isa = PBXGroup; children = ( @@ -901,6 +921,8 @@ 30697C5521F63F870064FCAC /* Extensions */, 8AD8EBF22F5E268D007475AB /* Fixtures */, 301F6464216164670071A4CE /* Helpers */, + 8AB3AD8F2F61600B0081DE16 /* LowLevel */, + 8AB3AD8E2F615FD70081DE16 /* Mocks */, 30C015A7214ED378005BB6DF /* Models */, 30C015A6214ED32A005BB6DF /* Parser */, 30B4C7BB24085A3C008B86F7 /* Passwords */, @@ -1644,6 +1666,8 @@ 8A4716712F5EF7A900C7A64D /* PersistenceControllerTest.swift in Sources */, 301F646D216166AA0071A4CE /* AdditionFieldTest.swift in Sources */, 9ADC954124418A5F0005402E /* PasswordStoreTest.swift in Sources */, + 8AB3AD8C2F615FA50081DE16 /* MockPGPInterface.swift in Sources */, + 8AB3AD8D2F615FA50081DE16 /* PGPAgentLowLevelTests.swift in Sources */, 30BAC8CB22E3BB6C00438475 /* DictBasedKeychain.swift in Sources */, DC6474612D46A8F8004B4BBC /* GitRepositoryTest.swift in Sources */, A2699ACF24027D9500F36323 /* PasswordTableEntryTest.swift in Sources */, diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index f193515..67088ab 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -17,6 +17,11 @@ public class PGPAgent { self.keyStore = keyStore } + init(keyStore: KeyStore, pgpInterface: PGPInterface) { + self.keyStore = keyStore + self.pgpInterface = pgpInterface + } + public func initKeys() throws { guard let publicKey: String = keyStore.get(for: PGPKey.PUBLIC.getKeychainKey()), let privateKey: String = keyStore.get(for: PGPKey.PRIVATE.getKeychainKey()) else { diff --git a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift new file mode 100644 index 0000000..85ed721 --- /dev/null +++ b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift @@ -0,0 +1,661 @@ +// +// PGPAgentLowLevelTests.swift +// passKitTests +// +// Detailed unit tests tracking the exact API call behavior of PGPAgent.decrypt. +// Uses MockPGPInterface to verify what arguments are passed to the underlying +// PGPInterface methods, and how passphrase resolution interacts with the keystore +// and the requestPGPKeyPassphrase callback. +// + +import XCTest + +@testable import passKit + +final class PGPAgentLowLevelTests: XCTestCase { + private var keychain: DictBasedKeychain! + private var mockPGP: MockPGPInterface! + private var agent: PGPAgent! + + private let testEncryptedData = Data("encrypted-payload".utf8) + private let testDecryptedData = Data("decrypted-payload".utf8) + + /// Tracks all calls to requestPGPKeyPassphrase closures created via `passphraseCallback(_:)`. + private var passphraseRequests: [String] = [] + + /// Creates a requestPGPKeyPassphrase closure that records the keyID it's called with + /// into `passphraseRequests` and returns `response`. + private func passphraseCallback(_ response: String) -> (String) -> String { + { [self] keyID in + passphraseRequests.append(keyID) + return response + } + } + + override func setUp() { + super.setUp() + + keychain = DictBasedKeychain() + // Set pgpKeyPassphrase key so checkAndInit() doesn't re-init and overwrite our mock. + keychain.add(string: "dummy", for: Globals.pgpKeyPassphrase) + + mockPGP = MockPGPInterface() + // some defaults + mockPGP.decryptResult = testDecryptedData + mockPGP.encryptResult = Data("mock-encrypted".utf8) + + passphraseRequests = [] + + agent = PGPAgent(keyStore: keychain, pgpInterface: mockPGP) + } + + override func tearDown() { + keychain.removeAllContent() + super.tearDown() + } + + // MARK: - decrypt(encryptedData:keyID:requestPGPKeyPassphrase:) - Key Resolution + + /// When the private key is found, decrypt is called with the provided keyID. + func testDecryptWithKeyID_keyFound_usesProvidedKeyID() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + + let result = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("pass")) + + XCTAssertEqual(result, testDecryptedData) + XCTAssertEqual(mockPGP.decryptCalls.count, 1) + XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) + XCTAssertEqual(mockPGP.decryptCalls[0].encryptedData, testEncryptedData) + XCTAssertEqual(passphraseRequests, [longFingerprint]) + } + + /// When the private key is NOT found but there's exactly one key, falls back to that key. + func testDecryptWithKeyID_keyNotFound_singleKey_fallsBackToOnlyKey() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [] // requested key not found + mockPGP.keyIDs = [longFingerprint] + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: "UNKNOWN", requestPGPKeyPassphrase: passphraseCallback("pass")) + + XCTAssertEqual(mockPGP.decryptCalls.count, 1) + // The keyID passed to pgpInterface.decrypt should be the fallback key, not the requested one. + XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) + XCTAssertEqual(passphraseRequests, [longFingerprint]) + } + + /// When the private key is NOT found and there are multiple keys, throws pgpPrivateKeyNotFound. + func testDecryptWithKeyID_keyNotFound_multipleKeys_throws() { + mockPGP.privateKeyIDs = [] + mockPGP.keyIDs = ["4712286271220db299883ea7062e678da1024dae", "787eae1a5fa3e749aa34cc6aa0645ebed862027e"] + + XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, keyID: "UNKNOWN", requestPGPKeyPassphrase: passphraseCallback("pass"))) { error in + XCTAssertEqual(error as? AppError, AppError.pgpPrivateKeyNotFound(keyID: "UNKNOWN")) + } + // pgpInterface.decrypt should NOT have been called + XCTAssertEqual(mockPGP.decryptCalls.count, 0) + XCTAssertEqual(passphraseRequests, [], "requestPGPKeyPassphrase should not be called when key resolution fails") + } + + /// containsPrivateKey is called with the provided keyID to check membership. + func testDecryptWithKeyID_checksContainsPrivateKey() throws { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("pass")) + + XCTAssertEqual(mockPGP.containsPrivateKeyCalls, [shortID]) + XCTAssertEqual(passphraseRequests, [shortID]) + } + + // MARK: - decrypt(encryptedData:keyID:requestPGPKeyPassphrase:) - Passphrase Resolution + + /// On first decrypt (latestDecryptStatus=true), the passphrase is looked up from keystore first. + /// If found in keystore, requestPGPKeyPassphrase is NOT called. + func testDecryptWithKeyID_firstCall_passphraseFromKeystore() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + keychain.add(string: "stored-passphrase", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("requested-passphrase")) + + XCTAssertEqual(passphraseRequests, [], "requestPGPKeyPassphrase should not be called when passphrase is in keystore") + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "stored-passphrase") + } + + /// On first decrypt, if keystore doesn't have the passphrase, requestPGPKeyPassphrase is called. + /// The keyID passed to requestPGPKeyPassphrase is the (possibly resolved) keyID. + func testDecryptWithKeyID_firstCall_passphraseFromRequest() throws { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + // No passphrase in keystore for this key. + XCTAssertFalse(keychain.contains(key: AppKeychain.getPGPKeyPassphraseKey(keyID: shortID))) + XCTAssertFalse(keychain.contains(key: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint))) + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("my-passphrase")) + + XCTAssertEqual(passphraseRequests, [shortID]) + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "my-passphrase") + } + + /// On first decrypt with key fallback, the resolved keyID is used for passphrase lookup and request. + func testDecryptWithKeyID_firstCall_keyFallback_usesResolvedKeyIDForPassphrase() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [] + mockPGP.keyIDs = [longFingerprint] + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: "UNKNOWN", requestPGPKeyPassphrase: passphraseCallback("pass")) + + // The passphrase and decrypt calls should use the resolved key, not the originally requested one. + XCTAssertEqual(passphraseRequests, [longFingerprint]) + XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) + } + + /// After a failed decrypt (latestDecryptStatus=false), requestPGPKeyPassphrase is ALWAYS called, + /// even if the keystore has a cached passphrase. + func testDecryptWithKeyID_afterFailure_alwaysRequestsPassphrase() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + keychain.add(string: "stored-passphrase", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) + + // First call: force a failure by making decrypt throw. + mockPGP.decryptError = AppError.wrongPassphrase + XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("bad"))) + + // Now latestDecryptStatus=false. Second call should always request. + mockPGP.decryptError = nil + mockPGP.decryptCalls.removeAll() + passphraseRequests.removeAll() + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("fresh-passphrase")) + + XCTAssertEqual(passphraseRequests, [longFingerprint], "After failure, passphrase should always be requested") + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "fresh-passphrase") + } + + /// After a successful decrypt, the next call uses keystore first (latestDecryptStatus=true). + func testDecryptWithKeyID_afterSuccess_usesKeystoreFirst() throws { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + + // First call succeeds. + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("pass1")) + + // Store a passphrase in keystore under the short ID (matching what PGPAgent used for lookup). + keychain.add(string: "pass1", for: AppKeychain.getPGPKeyPassphraseKey(keyID: shortID)) + + mockPGP.decryptCalls.removeAll() + passphraseRequests.removeAll() + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("ignored-passphrase")) + + XCTAssertEqual(passphraseRequests, []) + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "pass1") + } + + /// The passphrase keystore key is constructed with the resolved keyID (after fallback). + func testDecryptWithKeyID_passphraseKeystoreKey_usesResolvedKeyID() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [] + mockPGP.keyIDs = [longFingerprint] + keychain.add(string: "fallback-pass", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: "e9444483", requestPGPKeyPassphrase: passphraseCallback("should-not-use")) + + XCTAssertEqual(passphraseRequests, []) + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "fallback-pass") + } + + // MARK: - decrypt(encryptedData:keyID:requestPGPKeyPassphrase:) - Return Values & Error Propagation + + /// When pgpInterface.decrypt returns nil, agent.decrypt returns nil. + func testDecryptWithKeyID_interfaceReturnsNil_returnsNil() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + mockPGP.decryptResult = nil + + let result = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("pass")) + + XCTAssertNil(result) + XCTAssertEqual(passphraseRequests, [longFingerprint]) + } + + /// When pgpInterface.decrypt returns nil, latestDecryptStatus stays false + /// (next call will always request passphrase). + func testDecryptWithKeyID_interfaceReturnsNil_statusStaysFalse() throws { + let shortID = "d862027e" + let longFingerprint = "787eae1a5fa3e749aa34cc6aa0645ebed862027e" + mockPGP.privateKeyIDs = [longFingerprint] + mockPGP.decryptResult = nil + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("pass")) + + // Second call - should always request (latestDecryptStatus=false because nil return doesn't set it to true). + keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: shortID)) + keychain.add(string: "cached-long", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) + mockPGP.decryptResult = testDecryptedData + mockPGP.decryptCalls.removeAll() + passphraseRequests.removeAll() + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("fresh")) + + XCTAssertEqual(passphraseRequests, [shortID], "After nil return, passphrase should always be requested") + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "fresh") + } + + /// When pgpInterface.decrypt throws, the error propagates and latestDecryptStatus stays false. + func testDecryptWithKeyID_interfaceThrows_propagatesError() throws { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + mockPGP.decryptError = AppError.wrongPassphrase + + XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("pass"))) { error in + XCTAssertEqual(error as? AppError, AppError.wrongPassphrase) + } + XCTAssertEqual(passphraseRequests, [shortID]) + + // Verify latestDecryptStatus stayed false: next call should always request passphrase, + // even though the keystore has one cached. + keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: shortID)) + keychain.add(string: "cached-long", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) + mockPGP.decryptError = nil + mockPGP.decryptCalls.removeAll() + passphraseRequests.removeAll() + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("fresh")) + + XCTAssertEqual(passphraseRequests, [shortID], "After throw, passphrase should always be requested (latestDecryptStatus=false)") + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "fresh") + } + + /// After successful decrypt, latestDecryptStatus is true. + func testDecryptWithKeyID_success_setsStatusTrue() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + + // Force latestDecryptStatus to false first. + mockPGP.decryptError = AppError.wrongPassphrase + _ = try? agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("bad")) + mockPGP.decryptError = nil + mockPGP.decryptCalls.removeAll() + passphraseRequests.removeAll() + + // Now succeed. + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("good")) + + // Third call: latestDecryptStatus=true, so should try keystore first. + keychain.add(string: "good", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) + mockPGP.decryptCalls.removeAll() + passphraseRequests.removeAll() + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("should-not-use")) + + XCTAssertEqual(passphraseRequests, [], "After success, should try keystore first") + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "good") + } + + // MARK: - decrypt(encryptedData:keyID:requestPGPKeyPassphrase:) - checkAndInit behavior + + /// checkAndInit re-initializes if pgpKeyPassphrase is missing from keystore. + /// Since we're using a mock as pgpInterface, initKeys would overwrite it; verify the precondition holds. + func testDecryptWithKeyID_checkAndInit_requiresPGPKeyPassphraseInKeystore() throws { + // Remove the pgpKeyPassphrase sentinel, which will trigger checkAndInit -> initKeys. + keychain.removeContent(for: Globals.pgpKeyPassphrase) + // initKeys needs real PGP keys, which we don't have. It should throw keyImport. + XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, keyID: "a1024dae", requestPGPKeyPassphrase: passphraseCallback("pass"))) { error in + XCTAssertEqual(error as? AppError, AppError.keyImport) + } + XCTAssertEqual(passphraseRequests, [], "requestPGPKeyPassphrase should not be called when checkAndInit fails") + } + + // MARK: - decrypt(encryptedData:requestPGPKeyPassphrase:) - No KeyID Overload + + /// The no-keyID overload passes nil as keyID to pgpInterface.decrypt + func testDecryptNoKeyID_passesNilKeyIDToInterface() throws { + let result = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass")) + + XCTAssertEqual(result, testDecryptedData) + XCTAssertEqual(mockPGP.decryptCalls.count, 1) + XCTAssertNil(mockPGP.decryptCalls[0].keyID) + } + + /// The no-keyID overload requests passphrase with empty string keyID. + func testDecryptNoKeyID_requestsPassphraseWithEmptyString() throws { + _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass")) + + XCTAssertEqual(passphraseRequests, [""]) + } + + /// The no-keyID overload uses empty string as the keyID for passphrase lookup in keyStore/AppKeychain. + func testDecryptNoKeyID_usesEmptyStringForPassphraseLookup() throws { + keychain.add(string: "empty-key-pass", for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) + + _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("should-not-use")) + + XCTAssertEqual(passphraseRequests, []) + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "empty-key-pass") + } + + /// After failure, the no-keyID overload always requests passphrase (with empty string). + func testDecryptNoKeyID_afterFailure_alwaysRequestsPassphrase() throws { + // Force a failure. + mockPGP.decryptError = AppError.wrongPassphrase + _ = try? agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("bad")) + + // Next one can succeed + keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) + mockPGP.decryptError = nil + mockPGP.decryptCalls.removeAll() + passphraseRequests.removeAll() + + _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("fresh")) + + XCTAssertEqual(passphraseRequests, [""]) + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "fresh") + } + + /// The no-keyID overload returns nil when interface returns nil. + func testDecryptNoKeyID_interfaceReturnsNil_returnsNil() throws { + mockPGP.decryptResult = nil + + let result = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass")) + + XCTAssertNil(result) + XCTAssertEqual(passphraseRequests, [""]) + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "pass") + } + + /// The no-keyID overload propagates errors from the interface. + func testDecryptNoKeyID_interfaceThrows_propagatesError() { + mockPGP.decryptError = AppError.wrongPassphrase + + XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass"))) { error in + XCTAssertEqual(error as? AppError, AppError.wrongPassphrase) + } + XCTAssertEqual(passphraseRequests, [""]) + } + + /// The no-keyID overload sets latestDecryptStatus to true on success, + /// which affects subsequent passphrase lookups. + func testDecryptNoKeyID_success_setsStatusTrue() throws { + // Force failure first. + mockPGP.decryptError = AppError.wrongPassphrase + _ = try? agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("bad")) + mockPGP.decryptError = nil + mockPGP.decryptCalls.removeAll() + passphraseRequests.removeAll() + + // Succeed. + _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("good")) + + // Third call: should try keystore. + keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) + mockPGP.decryptCalls.removeAll() + passphraseRequests.removeAll() + + _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("nope")) + + XCTAssertEqual(passphraseRequests, []) + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "cached") + } + + /// The no-keyID overload doesn't check containsPrivateKey and doesn't resolve key. + func testDecryptNoKeyID_doesNotCheckPrivateKey() throws { + _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass")) + + XCTAssertEqual(mockPGP.containsPrivateKeyCalls.count, 0) + XCTAssertEqual(passphraseRequests, [""]) + } + + // MARK: - Key resolution error vs decrypt status ordering + + /// When pgpPrivateKeyNotFound is thrown (key not found, multiple keys), + /// latestDecryptStatus is NOT changed because the error occurs BEFORE the status update. + func testDecryptWithKeyID_keyNotFound_multipleKeys_doesNotChangeDecryptStatus() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [] + mockPGP.keyIDs = [longFingerprint, "787eae1a5fa3e749aa34cc6aa0645ebed862027e"] + + // This throws pgpPrivateKeyNotFound without changing latestDecryptStatus. + XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, keyID: "UNKNOWN", requestPGPKeyPassphrase: passphraseCallback("pass"))) + + // latestDecryptStatus should still be true (initial value). + // Next call should try keystore first. + mockPGP.privateKeyIDs = [longFingerprint] + keychain.add(string: "cached-pass", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) + passphraseRequests.removeAll() + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("fresh")) + + XCTAssertEqual(passphraseRequests, [], "After pgpPrivateKeyNotFound, latestDecryptStatus should be unchanged (still true)") + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "cached-pass") + } + + /// After failure + key fallback: passphrase is always requested using the RESOLVED (fallback) keyID. + func testDecryptWithKeyID_afterFailure_keyFallback_requestsWithResolvedKeyID() throws { + let shortID = "a1024dae" + let longFingerprint1 = "4712286271220db299883ea7062e678da1024dae" + let longFingerprint2 = "5fccb081ab8af48972999e2ae750acbfe9444483" + mockPGP.privateKeyIDs = [longFingerprint1] + + // Force a failure using a short ID that suffix-matches longFingerprint1. + mockPGP.decryptError = AppError.wrongPassphrase + _ = try? agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("bad")) + + // Now try with an unknown key that falls back to a different long fingerprint. + mockPGP.decryptError = nil + mockPGP.privateKeyIDs = [] + mockPGP.keyIDs = [longFingerprint2] + mockPGP.decryptCalls.removeAll() + passphraseRequests.removeAll() + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: "e9444483", requestPGPKeyPassphrase: passphraseCallback("pass")) + + XCTAssertEqual(passphraseRequests, [longFingerprint2]) + XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint2) + } + + // MARK: - Cross-overload latestDecryptStatus interaction + + /// latestDecryptStatus is shared between both overloads. + /// A failure in the keyID overload affects the no-keyID overload. + func testDecryptStatusSharedBetweenOverloads_failureInKeyIDOverload() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + + // Fail via keyID overload. + mockPGP.decryptError = AppError.wrongPassphrase + _ = try? agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("bad")) + + // Next call via no-keyID overload should always request. + keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) + mockPGP.decryptError = nil + mockPGP.decryptCalls.removeAll() + passphraseRequests.removeAll() + + _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("fresh")) + + XCTAssertEqual(passphraseRequests, [""], "Failure in keyID overload should affect no-keyID overload") + } + + /// A failure in the no-keyID overload affects the keyID overload. + func testDecryptStatusSharedBetweenOverloads_failureInNoKeyIDOverload() throws { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + + // Fail via no-keyID overload. + mockPGP.decryptError = AppError.wrongPassphrase + _ = try? agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("bad")) + + // Next call via keyID overload should always request. + mockPGP.decryptError = nil + mockPGP.decryptCalls.removeAll() + keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: shortID)) + passphraseRequests.removeAll() + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("fresh")) + + XCTAssertEqual(passphraseRequests, [shortID], "Failure in no-keyID overload should affect keyID overload") + } + + // MARK: - Short vs long key ID behavior + + /// When caller passes a short ID and containsPrivateKey matches it (via suffix), the short ID + /// is used for passphrase lookup, request callback, and pgpInterface.decrypt — it is NOT resolved + /// to a longer fingerprint. + func testDecryptWithKeyID_shortIDRecognized_shortIDFlowsThrough() throws { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + mockPGP.keyIDs = [longFingerprint] + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("pass")) + + // The short ID passes through everywhere — no resolution to the long fingerprint. + XCTAssertEqual(mockPGP.containsPrivateKeyCalls, [shortID]) + XCTAssertEqual(mockPGP.decryptCalls[0].keyID, shortID) + XCTAssertEqual(passphraseRequests, [shortID]) + } + + /// When caller passes a short ID and containsPrivateKey does NOT match, but there's one key, + /// the long fingerprint from keyID[0] is used everywhere instead. + func testDecryptWithKeyID_shortIDNotRecognized_singleKey_resolvesToLongFingerprint() throws { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [] // short ID doesn't match + mockPGP.keyIDs = [longFingerprint] + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("pass")) + + XCTAssertEqual(mockPGP.containsPrivateKeyCalls, [shortID]) + XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) + XCTAssertEqual(passphraseRequests, [longFingerprint]) + } + + /// Passphrase stored under long fingerprint is NOT found when the short ID is used for lookup + /// (because PGPAgent uses the caller's short ID as-is when containsPrivateKey matches). + func testDecryptWithKeyID_shortIDRecognized_passphraseStoredUnderLongID_missesKeystore() throws { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [longFingerprint] + mockPGP.keyIDs = [longFingerprint] + + // Store passphrase under the LONG fingerprint. + keychain.add(string: "stored-under-long", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("from-request")) + + // Keystore lookup uses the short ID, which doesn't match the long key → falls through to request. + XCTAssertEqual(passphraseRequests, [shortID]) + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "from-request") + } + + /// When short ID is NOT recognized and fallback resolves to long fingerprint, + /// passphrase stored under the long fingerprint IS found. + func testDecryptWithKeyID_shortIDNotRecognized_fallbackToLong_passphraseFoundInKeystore() throws { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.privateKeyIDs = [] + mockPGP.keyIDs = [longFingerprint] + + keychain.add(string: "stored-under-long", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) + + _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("should-not-use")) + + // Keystore lookup uses the resolved long fingerprint → finds the passphrase. + XCTAssertEqual(passphraseRequests, []) + XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "stored-under-long") + XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) + } + + /// Encrypt with short ID: when containsPublicKey matches (via suffix), the short ID is passed to interface. + func testEncryptWithKeyID_shortIDRecognized_shortIDFlowsThrough() throws { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.publicKeyIDs = [longFingerprint] + mockPGP.keyIDs = [longFingerprint] + + _ = try agent.encrypt(plainData: testDecryptedData, keyID: shortID) + + XCTAssertEqual(mockPGP.containsPublicKeyCalls, [shortID]) + XCTAssertEqual(mockPGP.encryptCalls[0].keyID, shortID) + } + + /// Encrypt with short ID: when containsPublicKey doesn't match and single key, + /// falls back to long fingerprint. + func testEncryptWithKeyID_shortIDNotRecognized_singleKey_resolvesToLongFingerprint() throws { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.publicKeyIDs = [] + mockPGP.keyIDs = [longFingerprint] + + _ = try agent.encrypt(plainData: testDecryptedData, keyID: shortID) + + XCTAssertEqual(mockPGP.containsPublicKeyCalls, [shortID]) + XCTAssertEqual(mockPGP.encryptCalls[0].keyID, longFingerprint) + } + + // MARK: - Encrypt passthrough tests (for completeness of mock interaction) + + /// encrypt(plainData:keyID:) calls containsPublicKey and passes data through. + func testEncryptWithKeyID_keyFound_callsInterface() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.publicKeyIDs = [longFingerprint] + + let result = try agent.encrypt(plainData: testDecryptedData, keyID: longFingerprint) + + XCTAssertEqual(result, mockPGP.encryptResult) + XCTAssertEqual(mockPGP.encryptCalls.count, 1) + XCTAssertEqual(mockPGP.encryptCalls[0].keyID, longFingerprint) + XCTAssertEqual(mockPGP.encryptCalls[0].plainData, testDecryptedData) + } + + /// encrypt with unknown key and single available key falls back. + func testEncryptWithKeyID_keyNotFound_singleKey_fallsBack() throws { + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.publicKeyIDs = [] + mockPGP.keyIDs = [longFingerprint] + + let result = try agent.encrypt(plainData: testDecryptedData, keyID: "e9444483") + + XCTAssertEqual(result, mockPGP.encryptResult) + XCTAssertEqual(mockPGP.encryptCalls[0].keyID, longFingerprint) + } + + /// encrypt with unknown key and multiple keys throws. + func testEncryptWithKeyID_keyNotFound_multipleKeys_throws() { + mockPGP.publicKeyIDs = [] + mockPGP.keyIDs = ["4712286271220db299883ea7062e678da1024dae", "787eae1a5fa3e749aa34cc6aa0645ebed862027e"] + + XCTAssertThrowsError(try agent.encrypt(plainData: testDecryptedData, keyID: "a1024dae")) { error in + XCTAssertEqual(error as? AppError, AppError.pgpPublicKeyNotFound(keyID: "a1024dae")) + } + XCTAssertEqual(mockPGP.encryptCalls.count, 0) + } + + /// encrypt(plainData:) without keyID passes nil to interface. + func testEncryptNoKeyID_passesNilToInterface() throws { + let result = try agent.encrypt(plainData: testDecryptedData) + + XCTAssertEqual(result, mockPGP.encryptResult) + XCTAssertEqual(mockPGP.encryptCalls.count, 1) + XCTAssertNil(mockPGP.encryptCalls[0].keyID) + } + + /// encrypt propagates errors from interface. + func testEncrypt_interfaceThrows_propagatesError() { + let shortID = "a1024dae" + let longFingerprint = "4712286271220db299883ea7062e678da1024dae" + mockPGP.publicKeyIDs = [longFingerprint] + mockPGP.encryptError = AppError.encryption + + XCTAssertThrowsError(try agent.encrypt(plainData: testDecryptedData, keyID: shortID)) { error in + XCTAssertEqual(error as? AppError, AppError.encryption) + } + } +} diff --git a/passKitTests/Mocks/MockPGPInterface.swift b/passKitTests/Mocks/MockPGPInterface.swift new file mode 100644 index 0000000..8ea0b53 --- /dev/null +++ b/passKitTests/Mocks/MockPGPInterface.swift @@ -0,0 +1,70 @@ +// +// MockPGPInterface.swift +// passKitTests +// + +import Foundation +@testable import passKit + +class MockPGPInterface: PGPInterface { + // MARK: - Configuration + + var keyIDs: [String] = [] + var shortKeyIDs: [String] = [] + var publicKeyIDs: Set = [] + var privateKeyIDs: Set = [] + + var decryptResult: Data? + var decryptError: Error? + var encryptResult = Data() + var encryptError: Error? + + // MARK: - Call tracking + + struct DecryptCall { + let encryptedData: Data + let keyID: String? + let passphrase: String + } + + struct EncryptCall { + let plainData: Data + let keyID: String? + } + + var decryptCalls: [DecryptCall] = [] + var encryptCalls: [EncryptCall] = [] + var containsPublicKeyCalls: [String] = [] + var containsPrivateKeyCalls: [String] = [] + + // MARK: - PGPInterface + + func decrypt(encryptedData: Data, keyID: String?, passphrase: String) throws -> Data? { + decryptCalls.append(DecryptCall(encryptedData: encryptedData, keyID: keyID, passphrase: passphrase)) + if let error = decryptError { + throw error + } + return decryptResult + } + + func encrypt(plainData: Data, keyID: String?) throws -> Data { + encryptCalls.append(EncryptCall(plainData: plainData, keyID: keyID)) + if let error = encryptError { + throw error + } + return encryptResult + } + + func containsPublicKey(with keyID: String) -> Bool { + containsPublicKeyCalls.append(keyID) + return publicKeyIDs.contains { $0.hasSuffix(keyID.lowercased()) } + } + + func containsPrivateKey(with keyID: String) -> Bool { + containsPrivateKeyCalls.append(keyID) + return privateKeyIDs.contains { $0.hasSuffix(keyID.lowercased()) } + } + + var keyID: [String] { keyIDs } + var shortKeyID: [String] { shortKeyIDs } +} From 2ae751044c2c3500f3552822343a85ab28ea7638 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Tue, 10 Mar 2026 17:14:11 +0100 Subject: [PATCH 03/21] decryption: always request key passphrase based on key ID --- passKit/Crypto/GopenPGPInterface.swift | 3 +- passKit/Crypto/ObjectivePGPInterface.swift | 9 +- passKit/Crypto/PGPAgent.swift | 26 +-- passKit/Crypto/PGPInterface.swift | 2 +- .../LowLevel/PGPAgentLowLevelTests.swift | 161 ++++++------------ passKitTests/Mocks/MockPGPInterface.swift | 14 +- 6 files changed, 85 insertions(+), 130 deletions(-) diff --git a/passKit/Crypto/GopenPGPInterface.swift b/passKit/Crypto/GopenPGPInterface.swift index 34f0622..9ec6a77 100644 --- a/passKit/Crypto/GopenPGPInterface.swift +++ b/passKit/Crypto/GopenPGPInterface.swift @@ -70,7 +70,7 @@ struct GopenPGPInterface: PGPInterface { privateKeys.keys.contains { key in key.hasSuffix(keyID.lowercased()) } } - func decrypt(encryptedData: Data, keyID: String?, passphrase: String) throws -> Data? { + func decrypt(encryptedData: Data, keyID: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? { let key: CryptoKey? = { if let keyID { return privateKeys.first(where: { key, _ in key.hasSuffix(keyID.lowercased()) })?.value @@ -87,6 +87,7 @@ struct GopenPGPInterface: PGPInterface { try privateKey.isLocked(&isLocked) var unlockedKey: CryptoKey! if isLocked.boolValue { + let passphrase = passPhraseForKey(privateKey.getFingerprint()) unlockedKey = try privateKey.unlock(passphrase.data(using: .utf8)) } else { unlockedKey = privateKey diff --git a/passKit/Crypto/ObjectivePGPInterface.swift b/passKit/Crypto/ObjectivePGPInterface.swift index 768d785..f94de71 100644 --- a/passKit/Crypto/ObjectivePGPInterface.swift +++ b/passKit/Crypto/ObjectivePGPInterface.swift @@ -24,8 +24,13 @@ struct ObjectivePGPInterface: PGPInterface { } } - func decrypt(encryptedData: Data, keyID _: String?, passphrase: String) throws -> Data? { - try ObjectivePGP.decrypt(encryptedData, andVerifySignature: false, using: keyring.keys) { _ in passphrase } + func decrypt(encryptedData: Data, keyID _: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? { + try ObjectivePGP.decrypt(encryptedData, andVerifySignature: false, using: keyring.keys) { selectedKey in + guard let selectedKey else { + return nil + } + return passPhraseForKey(selectedKey.keyID.longIdentifier) + } } func encrypt(plainData: Data, keyID _: String?) throws -> Data { diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index 67088ab..d668053 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -74,14 +74,14 @@ public class PGPAgent { latestDecryptStatus = false // Get the PGP key passphrase. - var passphrase = "" - if previousDecryptStatus == false { - passphrase = requestPGPKeyPassphrase(keyID) - } else { - passphrase = keyStore.get(for: AppKeychain.getPGPKeyPassphraseKey(keyID: keyID)) ?? requestPGPKeyPassphrase(keyID) + let providePassPhraseForKey = { (selectedKeyID: String) -> String in + if previousDecryptStatus == false { + return requestPGPKeyPassphrase(selectedKeyID) + } + return self.keyStore.get(for: AppKeychain.getPGPKeyPassphraseKey(keyID: selectedKeyID)) ?? requestPGPKeyPassphrase(selectedKeyID) } // Decrypt. - guard let result = try pgpInterface.decrypt(encryptedData: encryptedData, keyID: keyID, passphrase: passphrase) else { + guard let result = try pgpInterface.decrypt(encryptedData: encryptedData, keyID: keyID, passPhraseForKey: providePassPhraseForKey) else { return nil } // The decryption step has succeed. @@ -105,21 +105,21 @@ public class PGPAgent { return try pgpInterface.encrypt(plainData: plainData, keyID: keyID) } - public func decrypt(encryptedData: Data, requestPGPKeyPassphrase: (String) -> String) throws -> Data? { + public func decrypt(encryptedData: Data, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? { // Remember the previous status and set the current status let previousDecryptStatus = latestDecryptStatus latestDecryptStatus = false // Init keys. try checkAndInit() // Get the PGP key passphrase. - var passphrase = "" - if previousDecryptStatus == false { - passphrase = requestPGPKeyPassphrase("") - } else { - passphrase = keyStore.get(for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) ?? requestPGPKeyPassphrase("") + let providePassPhraseForKey = { (selectedKeyID: String) -> String in + if previousDecryptStatus == false { + return requestPGPKeyPassphrase(selectedKeyID) + } + return self.keyStore.get(for: AppKeychain.getPGPKeyPassphraseKey(keyID: selectedKeyID)) ?? requestPGPKeyPassphrase(selectedKeyID) } // Decrypt. - guard let result = try pgpInterface!.decrypt(encryptedData: encryptedData, keyID: nil, passphrase: passphrase) else { + guard let result = try pgpInterface!.decrypt(encryptedData: encryptedData, keyID: nil, passPhraseForKey: providePassPhraseForKey) else { return nil } // The decryption step has succeed. diff --git a/passKit/Crypto/PGPInterface.swift b/passKit/Crypto/PGPInterface.swift index b77831d..cb0d107 100644 --- a/passKit/Crypto/PGPInterface.swift +++ b/passKit/Crypto/PGPInterface.swift @@ -7,7 +7,7 @@ // protocol PGPInterface { - func decrypt(encryptedData: Data, keyID: String?, passphrase: String) throws -> Data? + func decrypt(encryptedData: Data, keyID: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? func encrypt(plainData: Data, keyID: String?) throws -> Data diff --git a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift index 85ed721..3e4b891 100644 --- a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift +++ b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift @@ -67,7 +67,6 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(mockPGP.decryptCalls.count, 1) XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) XCTAssertEqual(mockPGP.decryptCalls[0].encryptedData, testEncryptedData) - XCTAssertEqual(passphraseRequests, [longFingerprint]) } /// When the private key is NOT found but there's exactly one key, falls back to that key. @@ -81,7 +80,6 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(mockPGP.decryptCalls.count, 1) // The keyID passed to pgpInterface.decrypt should be the fallback key, not the requested one. XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) - XCTAssertEqual(passphraseRequests, [longFingerprint]) } /// When the private key is NOT found and there are multiple keys, throws pgpPrivateKeyNotFound. @@ -94,7 +92,6 @@ final class PGPAgentLowLevelTests: XCTestCase { } // pgpInterface.decrypt should NOT have been called XCTAssertEqual(mockPGP.decryptCalls.count, 0) - XCTAssertEqual(passphraseRequests, [], "requestPGPKeyPassphrase should not be called when key resolution fails") } /// containsPrivateKey is called with the provided keyID to check membership. @@ -106,7 +103,6 @@ final class PGPAgentLowLevelTests: XCTestCase { _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("pass")) XCTAssertEqual(mockPGP.containsPrivateKeyCalls, [shortID]) - XCTAssertEqual(passphraseRequests, [shortID]) } // MARK: - decrypt(encryptedData:keyID:requestPGPKeyPassphrase:) - Passphrase Resolution @@ -116,12 +112,13 @@ final class PGPAgentLowLevelTests: XCTestCase { func testDecryptWithKeyID_firstCall_passphraseFromKeystore() throws { let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [longFingerprint] + mockPGP.selectedKeyForPassphrase = longFingerprint keychain.add(string: "stored-passphrase", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("requested-passphrase")) + XCTAssertEqual(mockPGP.resolvedPassphrases, ["stored-passphrase"]) XCTAssertEqual(passphraseRequests, [], "requestPGPKeyPassphrase should not be called when passphrase is in keystore") - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "stored-passphrase") } /// On first decrypt, if keystore doesn't have the passphrase, requestPGPKeyPassphrase is called. @@ -130,27 +127,15 @@ final class PGPAgentLowLevelTests: XCTestCase { let shortID = "a1024dae" let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [longFingerprint] + mockPGP.selectedKeyForPassphrase = shortID // No passphrase in keystore for this key. XCTAssertFalse(keychain.contains(key: AppKeychain.getPGPKeyPassphraseKey(keyID: shortID))) XCTAssertFalse(keychain.contains(key: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint))) _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("my-passphrase")) + XCTAssertEqual(mockPGP.resolvedPassphrases, ["my-passphrase"]) XCTAssertEqual(passphraseRequests, [shortID]) - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "my-passphrase") - } - - /// On first decrypt with key fallback, the resolved keyID is used for passphrase lookup and request. - func testDecryptWithKeyID_firstCall_keyFallback_usesResolvedKeyIDForPassphrase() throws { - let longFingerprint = "4712286271220db299883ea7062e678da1024dae" - mockPGP.privateKeyIDs = [] - mockPGP.keyIDs = [longFingerprint] - - _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: "UNKNOWN", requestPGPKeyPassphrase: passphraseCallback("pass")) - - // The passphrase and decrypt calls should use the resolved key, not the originally requested one. - XCTAssertEqual(passphraseRequests, [longFingerprint]) - XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) } /// After a failed decrypt (latestDecryptStatus=false), requestPGPKeyPassphrase is ALWAYS called, @@ -167,12 +152,14 @@ final class PGPAgentLowLevelTests: XCTestCase { // Now latestDecryptStatus=false. Second call should always request. mockPGP.decryptError = nil mockPGP.decryptCalls.removeAll() + mockPGP.resolvedPassphrases.removeAll() + mockPGP.selectedKeyForPassphrase = longFingerprint passphraseRequests.removeAll() _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("fresh-passphrase")) + XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh-passphrase"]) XCTAssertEqual(passphraseRequests, [longFingerprint], "After failure, passphrase should always be requested") - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "fresh-passphrase") } /// After a successful decrypt, the next call uses keystore first (latestDecryptStatus=true). @@ -188,25 +175,14 @@ final class PGPAgentLowLevelTests: XCTestCase { keychain.add(string: "pass1", for: AppKeychain.getPGPKeyPassphraseKey(keyID: shortID)) mockPGP.decryptCalls.removeAll() + mockPGP.resolvedPassphrases.removeAll() + mockPGP.selectedKeyForPassphrase = shortID passphraseRequests.removeAll() _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("ignored-passphrase")) + XCTAssertEqual(mockPGP.resolvedPassphrases, ["pass1"]) XCTAssertEqual(passphraseRequests, []) - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "pass1") - } - - /// The passphrase keystore key is constructed with the resolved keyID (after fallback). - func testDecryptWithKeyID_passphraseKeystoreKey_usesResolvedKeyID() throws { - let longFingerprint = "4712286271220db299883ea7062e678da1024dae" - mockPGP.privateKeyIDs = [] - mockPGP.keyIDs = [longFingerprint] - keychain.add(string: "fallback-pass", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) - - _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: "e9444483", requestPGPKeyPassphrase: passphraseCallback("should-not-use")) - - XCTAssertEqual(passphraseRequests, []) - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "fallback-pass") } // MARK: - decrypt(encryptedData:keyID:requestPGPKeyPassphrase:) - Return Values & Error Propagation @@ -220,7 +196,6 @@ final class PGPAgentLowLevelTests: XCTestCase { let result = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("pass")) XCTAssertNil(result) - XCTAssertEqual(passphraseRequests, [longFingerprint]) } /// When pgpInterface.decrypt returns nil, latestDecryptStatus stays false @@ -238,12 +213,14 @@ final class PGPAgentLowLevelTests: XCTestCase { keychain.add(string: "cached-long", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) mockPGP.decryptResult = testDecryptedData mockPGP.decryptCalls.removeAll() + mockPGP.resolvedPassphrases.removeAll() + mockPGP.selectedKeyForPassphrase = longFingerprint passphraseRequests.removeAll() _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("fresh")) - XCTAssertEqual(passphraseRequests, [shortID], "After nil return, passphrase should always be requested") - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "fresh") + XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh"]) + XCTAssertEqual(passphraseRequests, [longFingerprint], "After nil return, passphrase should always be requested") } /// When pgpInterface.decrypt throws, the error propagates and latestDecryptStatus stays false. @@ -252,11 +229,12 @@ final class PGPAgentLowLevelTests: XCTestCase { let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [longFingerprint] mockPGP.decryptError = AppError.wrongPassphrase + mockPGP.selectedKeyForPassphrase = longFingerprint XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("pass"))) { error in XCTAssertEqual(error as? AppError, AppError.wrongPassphrase) } - XCTAssertEqual(passphraseRequests, [shortID]) + XCTAssertEqual(passphraseRequests, [longFingerprint]) // Verify latestDecryptStatus stayed false: next call should always request passphrase, // even though the keystore has one cached. @@ -264,12 +242,14 @@ final class PGPAgentLowLevelTests: XCTestCase { keychain.add(string: "cached-long", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) mockPGP.decryptError = nil mockPGP.decryptCalls.removeAll() + mockPGP.resolvedPassphrases.removeAll() + mockPGP.selectedKeyForPassphrase = longFingerprint passphraseRequests.removeAll() _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("fresh")) - XCTAssertEqual(passphraseRequests, [shortID], "After throw, passphrase should always be requested (latestDecryptStatus=false)") - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "fresh") + XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh"]) + XCTAssertEqual(passphraseRequests, [longFingerprint], "After throw, passphrase should always be requested (latestDecryptStatus=false)") } /// After successful decrypt, latestDecryptStatus is true. @@ -290,12 +270,14 @@ final class PGPAgentLowLevelTests: XCTestCase { // Third call: latestDecryptStatus=true, so should try keystore first. keychain.add(string: "good", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) mockPGP.decryptCalls.removeAll() + mockPGP.resolvedPassphrases.removeAll() + mockPGP.selectedKeyForPassphrase = longFingerprint passphraseRequests.removeAll() _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("should-not-use")) + XCTAssertEqual(mockPGP.resolvedPassphrases, ["good"]) XCTAssertEqual(passphraseRequests, [], "After success, should try keystore first") - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "good") } // MARK: - decrypt(encryptedData:keyID:requestPGPKeyPassphrase:) - checkAndInit behavior @@ -323,24 +305,7 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertNil(mockPGP.decryptCalls[0].keyID) } - /// The no-keyID overload requests passphrase with empty string keyID. - func testDecryptNoKeyID_requestsPassphraseWithEmptyString() throws { - _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass")) - - XCTAssertEqual(passphraseRequests, [""]) - } - - /// The no-keyID overload uses empty string as the keyID for passphrase lookup in keyStore/AppKeychain. - func testDecryptNoKeyID_usesEmptyStringForPassphraseLookup() throws { - keychain.add(string: "empty-key-pass", for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) - - _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("should-not-use")) - - XCTAssertEqual(passphraseRequests, []) - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "empty-key-pass") - } - - /// After failure, the no-keyID overload always requests passphrase (with empty string). + /// After failure, the no-keyID overload always requests passphrase. func testDecryptNoKeyID_afterFailure_alwaysRequestsPassphrase() throws { // Force a failure. mockPGP.decryptError = AppError.wrongPassphrase @@ -350,12 +315,14 @@ final class PGPAgentLowLevelTests: XCTestCase { keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) mockPGP.decryptError = nil mockPGP.decryptCalls.removeAll() + mockPGP.resolvedPassphrases.removeAll() + mockPGP.selectedKeyForPassphrase = "" passphraseRequests.removeAll() _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("fresh")) + XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh"]) XCTAssertEqual(passphraseRequests, [""]) - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "fresh") } /// The no-keyID overload returns nil when interface returns nil. @@ -365,8 +332,6 @@ final class PGPAgentLowLevelTests: XCTestCase { let result = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass")) XCTAssertNil(result) - XCTAssertEqual(passphraseRequests, [""]) - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "pass") } /// The no-keyID overload propagates errors from the interface. @@ -376,7 +341,6 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass"))) { error in XCTAssertEqual(error as? AppError, AppError.wrongPassphrase) } - XCTAssertEqual(passphraseRequests, [""]) } /// The no-keyID overload sets latestDecryptStatus to true on success, @@ -395,20 +359,21 @@ final class PGPAgentLowLevelTests: XCTestCase { // Third call: should try keystore. keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) mockPGP.decryptCalls.removeAll() + mockPGP.resolvedPassphrases.removeAll() + mockPGP.selectedKeyForPassphrase = "" passphraseRequests.removeAll() _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("nope")) + XCTAssertEqual(mockPGP.resolvedPassphrases, ["cached"]) XCTAssertEqual(passphraseRequests, []) - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "cached") } - /// The no-keyID overload doesn't check containsPrivateKey and doesn't resolve key. + /// The no-keyID overload doesn't check containsPrivateKey. func testDecryptNoKeyID_doesNotCheckPrivateKey() throws { _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass")) XCTAssertEqual(mockPGP.containsPrivateKeyCalls.count, 0) - XCTAssertEqual(passphraseRequests, [""]) } // MARK: - Key resolution error vs decrypt status ordering @@ -427,12 +392,13 @@ final class PGPAgentLowLevelTests: XCTestCase { // Next call should try keystore first. mockPGP.privateKeyIDs = [longFingerprint] keychain.add(string: "cached-pass", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) + mockPGP.selectedKeyForPassphrase = longFingerprint passphraseRequests.removeAll() _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("fresh")) XCTAssertEqual(passphraseRequests, [], "After pgpPrivateKeyNotFound, latestDecryptStatus should be unchanged (still true)") - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "cached-pass") + XCTAssertEqual(mockPGP.resolvedPassphrases, ["cached-pass"]) } /// After failure + key fallback: passphrase is always requested using the RESOLVED (fallback) keyID. @@ -451,12 +417,15 @@ final class PGPAgentLowLevelTests: XCTestCase { mockPGP.privateKeyIDs = [] mockPGP.keyIDs = [longFingerprint2] mockPGP.decryptCalls.removeAll() + mockPGP.resolvedPassphrases.removeAll() + mockPGP.selectedKeyForPassphrase = longFingerprint2 passphraseRequests.removeAll() _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: "e9444483", requestPGPKeyPassphrase: passphraseCallback("pass")) - XCTAssertEqual(passphraseRequests, [longFingerprint2]) XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint2) + XCTAssertEqual(mockPGP.resolvedPassphrases, ["pass"]) + XCTAssertEqual(passphraseRequests, [longFingerprint2]) } // MARK: - Cross-overload latestDecryptStatus interaction @@ -475,10 +444,13 @@ final class PGPAgentLowLevelTests: XCTestCase { keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) mockPGP.decryptError = nil mockPGP.decryptCalls.removeAll() + mockPGP.resolvedPassphrases.removeAll() + mockPGP.selectedKeyForPassphrase = "" passphraseRequests.removeAll() _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("fresh")) + XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh"]) XCTAssertEqual(passphraseRequests, [""], "Failure in keyID overload should affect no-keyID overload") } @@ -495,19 +467,21 @@ final class PGPAgentLowLevelTests: XCTestCase { // Next call via keyID overload should always request. mockPGP.decryptError = nil mockPGP.decryptCalls.removeAll() + mockPGP.resolvedPassphrases.removeAll() + mockPGP.selectedKeyForPassphrase = shortID keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: shortID)) passphraseRequests.removeAll() _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("fresh")) + XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh"]) XCTAssertEqual(passphraseRequests, [shortID], "Failure in no-keyID overload should affect keyID overload") } // MARK: - Short vs long key ID behavior /// When caller passes a short ID and containsPrivateKey matches it (via suffix), the short ID - /// is used for passphrase lookup, request callback, and pgpInterface.decrypt — it is NOT resolved - /// to a longer fingerprint. + /// is forwarded to pgpInterface.decrypt. func testDecryptWithKeyID_shortIDRecognized_shortIDFlowsThrough() throws { let shortID = "a1024dae" let longFingerprint = "4712286271220db299883ea7062e678da1024dae" @@ -516,14 +490,12 @@ final class PGPAgentLowLevelTests: XCTestCase { _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("pass")) - // The short ID passes through everywhere — no resolution to the long fingerprint. XCTAssertEqual(mockPGP.containsPrivateKeyCalls, [shortID]) XCTAssertEqual(mockPGP.decryptCalls[0].keyID, shortID) - XCTAssertEqual(passphraseRequests, [shortID]) } /// When caller passes a short ID and containsPrivateKey does NOT match, but there's one key, - /// the long fingerprint from keyID[0] is used everywhere instead. + /// the long fingerprint from keyID[0] is forwarded instead. func testDecryptWithKeyID_shortIDNotRecognized_singleKey_resolvesToLongFingerprint() throws { let shortID = "a1024dae" let longFingerprint = "4712286271220db299883ea7062e678da1024dae" @@ -534,43 +506,24 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(mockPGP.containsPrivateKeyCalls, [shortID]) XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) - XCTAssertEqual(passphraseRequests, [longFingerprint]) } /// Passphrase stored under long fingerprint is NOT found when the short ID is used for lookup - /// (because PGPAgent uses the caller's short ID as-is when containsPrivateKey matches). func testDecryptWithKeyID_shortIDRecognized_passphraseStoredUnderLongID_missesKeystore() throws { let shortID = "a1024dae" let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [longFingerprint] mockPGP.keyIDs = [longFingerprint] + mockPGP.selectedKeyForPassphrase = shortID // Store passphrase under the LONG fingerprint. keychain.add(string: "stored-under-long", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("from-request")) - // Keystore lookup uses the short ID, which doesn't match the long key → falls through to request. + // Backend requests passphrase with short ID — keystore lookup misses, falls through to request. + XCTAssertEqual(mockPGP.resolvedPassphrases, ["from-request"]) XCTAssertEqual(passphraseRequests, [shortID]) - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "from-request") - } - - /// When short ID is NOT recognized and fallback resolves to long fingerprint, - /// passphrase stored under the long fingerprint IS found. - func testDecryptWithKeyID_shortIDNotRecognized_fallbackToLong_passphraseFoundInKeystore() throws { - let shortID = "a1024dae" - let longFingerprint = "4712286271220db299883ea7062e678da1024dae" - mockPGP.privateKeyIDs = [] - mockPGP.keyIDs = [longFingerprint] - - keychain.add(string: "stored-under-long", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) - - _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("should-not-use")) - - // Keystore lookup uses the resolved long fingerprint → finds the passphrase. - XCTAssertEqual(passphraseRequests, []) - XCTAssertEqual(mockPGP.decryptCalls[0].passphrase, "stored-under-long") - XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) } /// Encrypt with short ID: when containsPublicKey matches (via suffix), the short ID is passed to interface. @@ -586,20 +539,6 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(mockPGP.encryptCalls[0].keyID, shortID) } - /// Encrypt with short ID: when containsPublicKey doesn't match and single key, - /// falls back to long fingerprint. - func testEncryptWithKeyID_shortIDNotRecognized_singleKey_resolvesToLongFingerprint() throws { - let shortID = "a1024dae" - let longFingerprint = "4712286271220db299883ea7062e678da1024dae" - mockPGP.publicKeyIDs = [] - mockPGP.keyIDs = [longFingerprint] - - _ = try agent.encrypt(plainData: testDecryptedData, keyID: shortID) - - XCTAssertEqual(mockPGP.containsPublicKeyCalls, [shortID]) - XCTAssertEqual(mockPGP.encryptCalls[0].keyID, longFingerprint) - } - // MARK: - Encrypt passthrough tests (for completeness of mock interaction) /// encrypt(plainData:keyID:) calls containsPublicKey and passes data through. @@ -617,13 +556,15 @@ final class PGPAgentLowLevelTests: XCTestCase { /// encrypt with unknown key and single available key falls back. func testEncryptWithKeyID_keyNotFound_singleKey_fallsBack() throws { + let shortID = "e9444483" let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.publicKeyIDs = [] mockPGP.keyIDs = [longFingerprint] - let result = try agent.encrypt(plainData: testDecryptedData, keyID: "e9444483") + let result = try agent.encrypt(plainData: testDecryptedData, keyID: shortID) XCTAssertEqual(result, mockPGP.encryptResult) + XCTAssertEqual(mockPGP.containsPublicKeyCalls, [shortID]) XCTAssertEqual(mockPGP.encryptCalls[0].keyID, longFingerprint) } diff --git a/passKitTests/Mocks/MockPGPInterface.swift b/passKitTests/Mocks/MockPGPInterface.swift index 8ea0b53..95b55e0 100644 --- a/passKitTests/Mocks/MockPGPInterface.swift +++ b/passKitTests/Mocks/MockPGPInterface.swift @@ -19,12 +19,16 @@ class MockPGPInterface: PGPInterface { var encryptResult = Data() var encryptError: Error? + /// When set, the mock calls `passPhraseForKey` with this key ID during `decrypt`, + /// simulating the PGP backend selecting a key and requesting its passphrase. + var selectedKeyForPassphrase: String? + // MARK: - Call tracking struct DecryptCall { let encryptedData: Data let keyID: String? - let passphrase: String + let passPhraseForKey: (String) -> String } struct EncryptCall { @@ -33,14 +37,18 @@ class MockPGPInterface: PGPInterface { } var decryptCalls: [DecryptCall] = [] + var resolvedPassphrases: [String] = [] var encryptCalls: [EncryptCall] = [] var containsPublicKeyCalls: [String] = [] var containsPrivateKeyCalls: [String] = [] // MARK: - PGPInterface - func decrypt(encryptedData: Data, keyID: String?, passphrase: String) throws -> Data? { - decryptCalls.append(DecryptCall(encryptedData: encryptedData, keyID: keyID, passphrase: passphrase)) + func decrypt(encryptedData: Data, keyID: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? { + decryptCalls.append(DecryptCall(encryptedData: encryptedData, keyID: keyID, passPhraseForKey: passPhraseForKey)) + if let selectedKey = selectedKeyForPassphrase { + resolvedPassphrases.append(passPhraseForKey(selectedKey)) + } if let error = decryptError { throw error } From f1cb5d27be766ca6686d04924f75495fca9632e4 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Tue, 10 Mar 2026 21:55:51 +0100 Subject: [PATCH 04/21] reference new version of gopenpgp with a new helper (HelperPassGetHexSubkeyIDsJSON) --- scripts/gopenpgp_build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/gopenpgp_build.sh b/scripts/gopenpgp_build.sh index 98dc8bf..b851cb9 100755 --- a/scripts/gopenpgp_build.sh +++ b/scripts/gopenpgp_build.sh @@ -14,7 +14,7 @@ GOPENPGP_PATH="$CHECKOUT_PATH/gopenpgp" mkdir -p "$OUTPUT_PATH" mkdir -p "$CHECKOUT_PATH" -git clone --depth 1 --branch "$GOPENPGP_VERSION" https://github.com/mssun/gopenpgp.git "$GOPENPGP_PATH" +git clone --depth 1 --branch "$GOPENPGP_VERSION" https://forgejo.tranvouez.eu/lysann/passforios-gopenpgp.git "$GOPENPGP_PATH" pushd "$GOPENPGP_PATH" mkdir -p dist From 8d4f3af475252e17b16f3c5707785e8336ba546f Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Tue, 10 Mar 2026 22:16:42 +0100 Subject: [PATCH 05/21] decryption: GopenPGPInterface tries to identify decryption key from message metadata So the system can have multiple private keys, and the caller doesn't need to specify a specific one regardless. Ideally: If there are several matches we could also take into account which keys have already been unlocked (or passthrases saved in keychain). Right now it only grabs the first match. --- passKit/Crypto/GopenPGPInterface.swift | 62 ++++++++++++++++++---- passKit/Crypto/ObjectivePGPInterface.swift | 2 +- passKit/Crypto/PGPAgent.swift | 4 +- passKit/Crypto/PGPInterface.swift | 2 +- passKitTests/Crypto/PGPAgentTest.swift | 19 +++++++ passKitTests/Mocks/MockPGPInterface.swift | 2 +- 6 files changed, 76 insertions(+), 15 deletions(-) diff --git a/passKit/Crypto/GopenPGPInterface.swift b/passKit/Crypto/GopenPGPInterface.swift index 9ec6a77..79e976a 100644 --- a/passKit/Crypto/GopenPGPInterface.swift +++ b/passKit/Crypto/GopenPGPInterface.swift @@ -16,6 +16,7 @@ struct GopenPGPInterface: PGPInterface { private var publicKeys: [String: CryptoKey] = [:] private var privateKeys: [String: CryptoKey] = [:] + private var privateSubkeyToKeyIDMapping: [String: String] = [:] // value is the key in privateKeys map init(publicArmoredKey: String, privateArmoredKey: String) throws { let pubKeys = extractKeysFromArmored(str: publicArmoredKey) @@ -40,7 +41,24 @@ struct GopenPGPInterface: PGPInterface { } throw AppError.keyImport } - privateKeys[cryptoKey.getFingerprint().lowercased()] = cryptoKey + + let keyID = cryptoKey.getFingerprint().lowercased() + privateKeys[keyID] = cryptoKey + + guard let subkeyIDsJSON = HelperPassGetHexSubkeyIDsJSON(cryptoKey) else { + guard error == nil else { + throw error! + } + throw AppError.keyImport + } + do { + let subkeyIDs = try JSONDecoder().decode([String].self, from: subkeyIDsJSON) + for subkeyID in subkeyIDs { + privateSubkeyToKeyIDMapping[subkeyID] = keyID + } + } catch { + throw AppError.keyImport + } } } @@ -70,15 +88,13 @@ struct GopenPGPInterface: PGPInterface { privateKeys.keys.contains { key in key.hasSuffix(keyID.lowercased()) } } - func decrypt(encryptedData: Data, keyID: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? { - let key: CryptoKey? = { - if let keyID { - return privateKeys.first(where: { key, _ in key.hasSuffix(keyID.lowercased()) })?.value - } - return privateKeys.first?.value - }() + func decrypt(encryptedData: Data, keyIDHint: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? { + let message = createPGPMessage(from: encryptedData) + guard let message else { + throw AppError.decryption + } - guard let privateKey = key else { + guard let privateKey: CryptoKey = try findDecryptionKey(message: message, keyIDHint: keyIDHint) else { throw AppError.decryption } @@ -101,7 +117,6 @@ struct GopenPGPInterface: PGPInterface { throw AppError.decryption } - let message = createPGPMessage(from: encryptedData) return try keyRing.decrypt(message, verifyKey: nil, verifyTime: 0).data } catch { throw Self.errorMapping[error.localizedDescription, default: error] @@ -148,6 +163,33 @@ struct GopenPGPInterface: PGPInterface { var shortKeyID: [String] { publicKeys.keys.map { $0.suffix(8).uppercased() } } + + private func findDecryptionKey(message: CryptoPGPMessage, keyIDHint: String?) throws -> CryptoKey? { + var keyIDCandidates: any Collection = privateKeys.keys + do { + if let encryptionKeysJSON = message.getHexEncryptionKeyIDsJson() { + // these are the subkeys (encryption keys), not the primaries keys (whose fingerprints we have in the privateKeys map), + // so we need to map them back to the primary keyIDs using privateSubkeyToKeyIDMapping + let validSubkeys = try JSONDecoder().decode([String].self, from: encryptionKeysJSON) + let validKeyIDs = validSubkeys.compactMap { privateSubkeyToKeyIDMapping[$0] } + if #available(iOSApplicationExtension 16.0, *) { + assert(validKeyIDs.isEmpty || !Set(keyIDCandidates).isDisjoint(with: validKeyIDs)) + } + keyIDCandidates = validKeyIDs + } + } catch { + // fall back to legacy approach of trying first in privateKeys (or preferring hint) + } + + if let keyIDHint { + keyIDCandidates = keyIDCandidates.filter { key in key.hasSuffix(keyIDHint.lowercased()) } + } + guard let selectedKeyID = keyIDCandidates.first else { + throw keyIDHint != nil ? AppError.keyExpiredOrIncompatible : AppError.decryption + } + + return privateKeys[selectedKeyID] + } } public func createPGPMessage(from encryptedData: Data) -> CryptoPGPMessage? { diff --git a/passKit/Crypto/ObjectivePGPInterface.swift b/passKit/Crypto/ObjectivePGPInterface.swift index f94de71..06b1c6b 100644 --- a/passKit/Crypto/ObjectivePGPInterface.swift +++ b/passKit/Crypto/ObjectivePGPInterface.swift @@ -24,7 +24,7 @@ struct ObjectivePGPInterface: PGPInterface { } } - func decrypt(encryptedData: Data, keyID _: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? { + func decrypt(encryptedData: Data, keyIDHint _: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? { try ObjectivePGP.decrypt(encryptedData, andVerifySignature: false, using: keyring.keys) { selectedKey in guard let selectedKey else { return nil diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index d668053..f96784a 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -81,7 +81,7 @@ public class PGPAgent { return self.keyStore.get(for: AppKeychain.getPGPKeyPassphraseKey(keyID: selectedKeyID)) ?? requestPGPKeyPassphrase(selectedKeyID) } // Decrypt. - guard let result = try pgpInterface.decrypt(encryptedData: encryptedData, keyID: keyID, passPhraseForKey: providePassPhraseForKey) else { + guard let result = try pgpInterface.decrypt(encryptedData: encryptedData, keyIDHint: keyID, passPhraseForKey: providePassPhraseForKey) else { return nil } // The decryption step has succeed. @@ -119,7 +119,7 @@ public class PGPAgent { return self.keyStore.get(for: AppKeychain.getPGPKeyPassphraseKey(keyID: selectedKeyID)) ?? requestPGPKeyPassphrase(selectedKeyID) } // Decrypt. - guard let result = try pgpInterface!.decrypt(encryptedData: encryptedData, keyID: nil, passPhraseForKey: providePassPhraseForKey) else { + guard let result = try pgpInterface!.decrypt(encryptedData: encryptedData, keyIDHint: nil, passPhraseForKey: providePassPhraseForKey) else { return nil } // The decryption step has succeed. diff --git a/passKit/Crypto/PGPInterface.swift b/passKit/Crypto/PGPInterface.swift index cb0d107..88cfd5f 100644 --- a/passKit/Crypto/PGPInterface.swift +++ b/passKit/Crypto/PGPInterface.swift @@ -7,7 +7,7 @@ // protocol PGPInterface { - func decrypt(encryptedData: Data, keyID: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? + func decrypt(encryptedData: Data, keyIDHint: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? func encrypt(plainData: Data, keyID: String?) throws -> Data diff --git a/passKitTests/Crypto/PGPAgentTest.swift b/passKitTests/Crypto/PGPAgentTest.swift index 8acb2be..4fc1102 100644 --- a/passKitTests/Crypto/PGPAgentTest.swift +++ b/passKitTests/Crypto/PGPAgentTest.swift @@ -87,6 +87,25 @@ final class PGPAgentTest: XCTestCase { } } + func testMultiKeysSelectMatchingPrivateKeyToDecrypt() throws { + keychain.removeAllContent() + try importKeys(RSA2048_RSA4096.publicKeys, RSA2048_RSA4096.privateKeys) + try pgpAgent.initKeys() + try [ + (true, true), + (true, false), + (false, true), + (false, false), + ].forEach { encryptInArmored, decryptFromArmored in + passKit.Defaults.encryptInArmored = encryptInArmored + let encryptedData = try pgpAgent.encrypt(plainData: testData, keyID: RSA2048.fingerprint) + passKit.Defaults.encryptInArmored = decryptFromArmored + // Note: not specifying the keyID to decrypt, so that the agent needs to find the matching private key by itself. + let decryptedData = try pgpAgent.decrypt(encryptedData: encryptedData, requestPGPKeyPassphrase: requestPGPKeyPassphrase) + XCTAssertEqual(decryptedData, testData) + } + } + func testNoPrivateKey() throws { try KeyFileManager(keyType: PGPKey.PUBLIC, keyPath: "", keyHandler: keychain.add).importKey(from: RSA2048.publicKey) XCTAssertFalse(pgpAgent.isPrepared) diff --git a/passKitTests/Mocks/MockPGPInterface.swift b/passKitTests/Mocks/MockPGPInterface.swift index 95b55e0..432f678 100644 --- a/passKitTests/Mocks/MockPGPInterface.swift +++ b/passKitTests/Mocks/MockPGPInterface.swift @@ -44,7 +44,7 @@ class MockPGPInterface: PGPInterface { // MARK: - PGPInterface - func decrypt(encryptedData: Data, keyID: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? { + func decrypt(encryptedData: Data, keyIDHint keyID: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? { decryptCalls.append(DecryptCall(encryptedData: encryptedData, keyID: keyID, passPhraseForKey: passPhraseForKey)) if let selectedKey = selectedKeyForPassphrase { resolvedPassphrases.append(passPhraseForKey(selectedKey)) From 84eaf4ad7dfb22691685ac4b9fa231c97e69af27 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 00:21:30 +0100 Subject: [PATCH 06/21] PGPInterface can encrypt with multiple keys, PGPAgent can encrypt with all keys --- passKit/Crypto/GopenPGPInterface.swift | 35 +++++++++---- passKit/Crypto/ObjectivePGPInterface.swift | 19 ++++++- passKit/Crypto/PGPAgent.swift | 11 +++- passKit/Crypto/PGPInterface.swift | 4 ++ passKitTests/Crypto/PGPAgentTest.swift | 29 +++++++++++ .../LowLevel/PGPAgentLowLevelTests.swift | 52 ++++++++++++++++--- passKitTests/Mocks/MockPGPInterface.swift | 27 ++++++++++ 7 files changed, 158 insertions(+), 19 deletions(-) diff --git a/passKit/Crypto/GopenPGPInterface.swift b/passKit/Crypto/GopenPGPInterface.swift index 79e976a..638ce25 100644 --- a/passKit/Crypto/GopenPGPInterface.swift +++ b/passKit/Crypto/GopenPGPInterface.swift @@ -123,26 +123,43 @@ struct GopenPGPInterface: PGPInterface { } } + @available(*, deprecated, message: "Use encrypt(plainData:keyIDs:) instead.") func encrypt(plainData: Data, keyID: String?) throws -> Data { - let key: CryptoKey? = { - if let keyID { - return publicKeys.first(where: { key, _ in key.hasSuffix(keyID.lowercased()) })?.value - } - return publicKeys.first?.value - }() + guard let keyID = keyID ?? publicKeys.keys.first else { + // this is invalid, but we want the new function to throw the error for us + return try encrypt(plainData: plainData, keyIDs: []) + } + return try encrypt(plainData: plainData, keyIDs: [keyID]) + } - guard let publicKey = key else { + func encryptWithAllKeys(plainData: Data) throws -> Data { + let keyIDs = publicKeys.keys.filter { key in privateKeys.keys.contains(key) } + return try encrypt(plainData: plainData, keyIDs: keyIDs) + } + + func encrypt(plainData: Data, keyIDs: [String]) throws -> Data { + let keys: [CryptoKey] = keyIDs.compactMap { keyID in + publicKeys.first(where: { key, _ in key.hasSuffix(keyID.lowercased()) })?.value + } + guard let firstKey = keys.first else { throw AppError.encryption } + let otherKeys = keys.dropFirst() var error: NSError? - - guard let keyRing = CryptoNewKeyRing(publicKey, &error) else { + guard let keyRing = CryptoNewKeyRing(firstKey, &error) else { guard error == nil else { throw error! } throw AppError.encryption } + do { + try otherKeys.forEach { key in + try keyRing.add(key) + } + } catch { + throw AppError.encryption + } let encryptedData = try keyRing.encrypt(CryptoNewPlainMessage(plainData.mutable as Data), privateKey: nil) if Defaults.encryptInArmored { diff --git a/passKit/Crypto/ObjectivePGPInterface.swift b/passKit/Crypto/ObjectivePGPInterface.swift index 06b1c6b..01a5a2b 100644 --- a/passKit/Crypto/ObjectivePGPInterface.swift +++ b/passKit/Crypto/ObjectivePGPInterface.swift @@ -33,8 +33,25 @@ struct ObjectivePGPInterface: PGPInterface { } } + @available(*, deprecated, message: "Use encrypt(plainData:keyIDs:) instead.") func encrypt(plainData: Data, keyID _: String?) throws -> Data { - let encryptedData = try ObjectivePGP.encrypt(plainData, addSignature: false, using: keyring.keys, passphraseForKey: nil) + // Backwards compatibility: ignore keyID parameter and encrypted with all keys in the keyring + try encryptWithAllKeys(plainData: plainData) + } + + func encryptWithAllKeys(plainData: Data) throws -> Data { + try encrypt(plainData: plainData, keyIDs: keyID) + } + + func encrypt(plainData: Data, keyIDs: [String]) throws -> Data { + let keys = try keyIDs.map { keyID in + guard let key = keyring.findKey(keyID) else { + throw AppError.pgpPublicKeyNotFound(keyID: keyID) + } + return key + } + + let encryptedData = try ObjectivePGP.encrypt(plainData, addSignature: false, using: keys, passphraseForKey: nil) if Defaults.encryptInArmored { return Armor.armored(encryptedData, as: .message).data(using: .ascii)! } diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index f96784a..1c49260 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -102,7 +102,7 @@ public class PGPAgent { throw AppError.pgpPublicKeyNotFound(keyID: keyID) } } - return try pgpInterface.encrypt(plainData: plainData, keyID: keyID) + return try pgpInterface.encrypt(plainData: plainData, keyIDs: [keyID]) } public func decrypt(encryptedData: Data, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? { @@ -127,6 +127,7 @@ public class PGPAgent { return result } + @available(*, deprecated, message: "Use encrypt(plainData:keyID:) or encryptWithAllKeys(plainData:) instead.") public func encrypt(plainData: Data) throws -> Data { try checkAndInit() guard let pgpInterface else { @@ -135,6 +136,14 @@ public class PGPAgent { return try pgpInterface.encrypt(plainData: plainData, keyID: nil) } + public func encryptWithAllKeys(plainData: Data) throws -> Data { + try checkAndInit() + guard let pgpInterface else { + throw AppError.encryption + } + return try pgpInterface.encryptWithAllKeys(plainData: plainData) + } + public var isPrepared: Bool { keyStore.contains(key: PGPKey.PUBLIC.getKeychainKey()) && keyStore.contains(key: PGPKey.PRIVATE.getKeychainKey()) diff --git a/passKit/Crypto/PGPInterface.swift b/passKit/Crypto/PGPInterface.swift index 88cfd5f..f90c4bd 100644 --- a/passKit/Crypto/PGPInterface.swift +++ b/passKit/Crypto/PGPInterface.swift @@ -9,7 +9,11 @@ protocol PGPInterface { func decrypt(encryptedData: Data, keyIDHint: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? + @available(*, deprecated, message: "Use encrypt(plainData:keyIDs:) instead.") func encrypt(plainData: Data, keyID: String?) throws -> Data + // encrypt with all public keys for which we also have a private key + func encryptWithAllKeys(plainData: Data) throws -> Data + func encrypt(plainData: Data, keyIDs: [String]) throws -> Data func containsPublicKey(with keyID: String) -> Bool diff --git a/passKitTests/Crypto/PGPAgentTest.swift b/passKitTests/Crypto/PGPAgentTest.swift index 4fc1102..522ce1a 100644 --- a/passKitTests/Crypto/PGPAgentTest.swift +++ b/passKitTests/Crypto/PGPAgentTest.swift @@ -180,6 +180,35 @@ final class PGPAgentTest: XCTestCase { XCTAssertEqual(passphraseRequestCalledCount, 3) } + func testEncryptWithAllKeys() throws { + // When multiple keys are imported, the agent should be able to encrypt without specifying the keyID. + // It should use all public keys for which we also have private keys, and the encrypted message should be able to be decrypted by any of the private keys. + + keychain.removeAllContent() + // no private key for ED25519 + try importKeys(RSA2048_RSA4096.publicKeys | ED25519.publicKey, RSA2048_RSA4096.privateKeys) + try pgpAgent.initKeys() + + let encryptedData = try pgpAgent.encryptWithAllKeys(plainData: testData) + + try [RSA2048.fingerprint, RSA4096.fingerprint].forEach { keyID in + let decryptedData = try pgpAgent.decrypt(encryptedData: encryptedData, keyID: keyID, requestPGPKeyPassphrase: requestPGPKeyPassphrase) + XCTAssertEqual(decryptedData, testData) + } + + XCTAssertThrowsError(try pgpAgent.decrypt(encryptedData: encryptedData, keyID: ED25519.fingerprint, requestPGPKeyPassphrase: requestPGPKeyPassphrase)) { + XCTAssertEqual($0 as! AppError, AppError.pgpPrivateKeyNotFound(keyID: ED25519.fingerprint)) + } + + // add private key for ED25519 + try importKeys(RSA2048_RSA4096.publicKeys | ED25519.publicKey, RSA2048_RSA4096.privateKeys | ED25519.privateKey) + try pgpAgent.initKeys() + + XCTAssertThrowsError(try pgpAgent.decrypt(encryptedData: encryptedData, keyID: ED25519.fingerprint, requestPGPKeyPassphrase: requestPGPKeyPassphrase)) { + XCTAssertEqual($0 as! AppError, AppError.keyExpiredOrIncompatible) + } + } + private func importKeys(_ publicKey: String, _ privateKey: String) throws { try KeyFileManager(keyType: PGPKey.PUBLIC, keyPath: "", keyHandler: keychain.add).importKey(from: publicKey) try KeyFileManager(keyType: PGPKey.PRIVATE, keyPath: "", keyHandler: keychain.add).importKey(from: privateKey) diff --git a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift index 3e4b891..73253f6 100644 --- a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift +++ b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift @@ -536,12 +536,13 @@ final class PGPAgentLowLevelTests: XCTestCase { _ = try agent.encrypt(plainData: testDecryptedData, keyID: shortID) XCTAssertEqual(mockPGP.containsPublicKeyCalls, [shortID]) - XCTAssertEqual(mockPGP.encryptCalls[0].keyID, shortID) + XCTAssertEqual(mockPGP.encryptMultiKeyCalls.count, 1) + XCTAssertEqual(mockPGP.encryptMultiKeyCalls[0].keyIDs, [shortID]) } // MARK: - Encrypt passthrough tests (for completeness of mock interaction) - /// encrypt(plainData:keyID:) calls containsPublicKey and passes data through. + /// encrypt(plainData:keyID:) calls containsPublicKey and passes data through via encrypt(plainData:keyIDs:). func testEncryptWithKeyID_keyFound_callsInterface() throws { let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.publicKeyIDs = [longFingerprint] @@ -549,9 +550,9 @@ final class PGPAgentLowLevelTests: XCTestCase { let result = try agent.encrypt(plainData: testDecryptedData, keyID: longFingerprint) XCTAssertEqual(result, mockPGP.encryptResult) - XCTAssertEqual(mockPGP.encryptCalls.count, 1) - XCTAssertEqual(mockPGP.encryptCalls[0].keyID, longFingerprint) - XCTAssertEqual(mockPGP.encryptCalls[0].plainData, testDecryptedData) + XCTAssertEqual(mockPGP.encryptMultiKeyCalls.count, 1) + XCTAssertEqual(mockPGP.encryptMultiKeyCalls[0].keyIDs, [longFingerprint]) + XCTAssertEqual(mockPGP.encryptMultiKeyCalls[0].plainData, testDecryptedData) } /// encrypt with unknown key and single available key falls back. @@ -565,7 +566,7 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(result, mockPGP.encryptResult) XCTAssertEqual(mockPGP.containsPublicKeyCalls, [shortID]) - XCTAssertEqual(mockPGP.encryptCalls[0].keyID, longFingerprint) + XCTAssertEqual(mockPGP.encryptMultiKeyCalls[0].keyIDs, [longFingerprint]) } /// encrypt with unknown key and multiple keys throws. @@ -576,10 +577,10 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertThrowsError(try agent.encrypt(plainData: testDecryptedData, keyID: "a1024dae")) { error in XCTAssertEqual(error as? AppError, AppError.pgpPublicKeyNotFound(keyID: "a1024dae")) } - XCTAssertEqual(mockPGP.encryptCalls.count, 0) + XCTAssertEqual(mockPGP.encryptMultiKeyCalls.count, 0) } - /// encrypt(plainData:) without keyID passes nil to interface. + /// encrypt(plainData:) without keyID passes nil to the deprecated interface method. func testEncryptNoKeyID_passesNilToInterface() throws { let result = try agent.encrypt(plainData: testDecryptedData) @@ -599,4 +600,39 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(error as? AppError, AppError.encryption) } } + + // MARK: - encryptWithAllKeys + + /// encryptWithAllKeys delegates to pgpInterface.encryptWithAllKeys. + func testEncryptWithAllKeys_callsInterface() throws { + mockPGP.encryptResult = Data("all-keys-encrypted".utf8) + + let result = try agent.encryptWithAllKeys(plainData: testDecryptedData) + + XCTAssertEqual(result, Data("all-keys-encrypted".utf8)) + XCTAssertEqual(mockPGP.encryptWithAllKeysCalls.count, 1) + XCTAssertEqual(mockPGP.encryptWithAllKeysCalls[0].plainData, testDecryptedData) + // Does not call containsPublicKey or the single/multi-key encrypt methods. + XCTAssertEqual(mockPGP.containsPublicKeyCalls.count, 0) + XCTAssertEqual(mockPGP.encryptCalls.count, 0) + XCTAssertEqual(mockPGP.encryptMultiKeyCalls.count, 0) + } + + /// encryptWithAllKeys propagates errors from interface. + func testEncryptWithAllKeys_interfaceThrows_propagatesError() { + mockPGP.encryptError = AppError.encryption + + XCTAssertThrowsError(try agent.encryptWithAllKeys(plainData: testDecryptedData)) { error in + XCTAssertEqual(error as? AppError, AppError.encryption) + } + } + + /// encryptWithAllKeys throws encryption error when pgpInterface is nil (checkAndInit fails). + func testEncryptWithAllKeys_checkAndInit_requiresPGPKeyPassphraseInKeystore() throws { + keychain.removeContent(for: Globals.pgpKeyPassphrase) + + XCTAssertThrowsError(try agent.encryptWithAllKeys(plainData: testDecryptedData)) { error in + XCTAssertEqual(error as? AppError, AppError.keyImport) + } + } } diff --git a/passKitTests/Mocks/MockPGPInterface.swift b/passKitTests/Mocks/MockPGPInterface.swift index 432f678..481d052 100644 --- a/passKitTests/Mocks/MockPGPInterface.swift +++ b/passKitTests/Mocks/MockPGPInterface.swift @@ -36,9 +36,20 @@ class MockPGPInterface: PGPInterface { let keyID: String? } + struct EncryptMultiKeyCall { + let plainData: Data + let keyIDs: [String] + } + + struct EncryptWithAllKeysCall { + let plainData: Data + } + var decryptCalls: [DecryptCall] = [] var resolvedPassphrases: [String] = [] var encryptCalls: [EncryptCall] = [] + var encryptMultiKeyCalls: [EncryptMultiKeyCall] = [] + var encryptWithAllKeysCalls: [EncryptWithAllKeysCall] = [] var containsPublicKeyCalls: [String] = [] var containsPrivateKeyCalls: [String] = [] @@ -63,6 +74,22 @@ class MockPGPInterface: PGPInterface { return encryptResult } + func encryptWithAllKeys(plainData: Data) throws -> Data { + encryptWithAllKeysCalls.append(EncryptWithAllKeysCall(plainData: plainData)) + if let error = encryptError { + throw error + } + return encryptResult + } + + func encrypt(plainData: Data, keyIDs: [String]) throws -> Data { + encryptMultiKeyCalls.append(EncryptMultiKeyCall(plainData: plainData, keyIDs: keyIDs)) + if let error = encryptError { + throw error + } + return encryptResult + } + func containsPublicKey(with keyID: String) -> Bool { containsPublicKeyCalls.append(keyID) return publicKeyIDs.contains { $0.hasSuffix(keyID.lowercased()) } From e728f26a2041302377103301d6194478463d9833 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 00:26:03 +0100 Subject: [PATCH 07/21] move and rename test functions --- passKitTests/Crypto/PGPAgentTest.swift | 100 +++++++++++++------------ 1 file changed, 53 insertions(+), 47 deletions(-) diff --git a/passKitTests/Crypto/PGPAgentTest.swift b/passKitTests/Crypto/PGPAgentTest.swift index 522ce1a..b7ba158 100644 --- a/passKitTests/Crypto/PGPAgentTest.swift +++ b/passKitTests/Crypto/PGPAgentTest.swift @@ -31,34 +31,7 @@ final class PGPAgentTest: XCTestCase { super.tearDown() } - private func basicEncryptDecrypt(using pgpAgent: PGPAgent, keyID: String, encryptKeyID: String? = nil, requestPassphrase: @escaping (String) -> String = requestPGPKeyPassphrase, encryptInArmored: Bool = true, decryptFromArmored: Bool = true) throws -> Data? { - passKit.Defaults.encryptInArmored = encryptInArmored - let encryptedData = try pgpAgent.encrypt(plainData: testData, keyID: keyID) - passKit.Defaults.encryptInArmored = decryptFromArmored - return try pgpAgent.decrypt(encryptedData: encryptedData, keyID: encryptKeyID ?? keyID, requestPGPKeyPassphrase: requestPassphrase) - } - - func testMultiKeys() throws { - try [ - RSA2048_RSA4096, - ED25519_NISTP384, - ].forEach { testKeyInfo in - keychain.removeAllContent() - try importKeys(testKeyInfo.publicKeys, testKeyInfo.privateKeys) - XCTAssert(pgpAgent.isPrepared) - try pgpAgent.initKeys() - try [ - (true, true), - (true, false), - (false, true), - (false, false), - ].forEach { encryptInArmored, decryptFromArmored in - for id in testKeyInfo.fingerprints { - XCTAssertEqual(try basicEncryptDecrypt(using: pgpAgent, keyID: id, encryptInArmored: encryptInArmored, decryptFromArmored: decryptFromArmored), testData) - } - } - } - } + // - MARK: Basic encrypt and decrypt tests func testBasicEncryptDecrypt() throws { try [ @@ -87,25 +60,6 @@ final class PGPAgentTest: XCTestCase { } } - func testMultiKeysSelectMatchingPrivateKeyToDecrypt() throws { - keychain.removeAllContent() - try importKeys(RSA2048_RSA4096.publicKeys, RSA2048_RSA4096.privateKeys) - try pgpAgent.initKeys() - try [ - (true, true), - (true, false), - (false, true), - (false, false), - ].forEach { encryptInArmored, decryptFromArmored in - passKit.Defaults.encryptInArmored = encryptInArmored - let encryptedData = try pgpAgent.encrypt(plainData: testData, keyID: RSA2048.fingerprint) - passKit.Defaults.encryptInArmored = decryptFromArmored - // Note: not specifying the keyID to decrypt, so that the agent needs to find the matching private key by itself. - let decryptedData = try pgpAgent.decrypt(encryptedData: encryptedData, requestPGPKeyPassphrase: requestPGPKeyPassphrase) - XCTAssertEqual(decryptedData, testData) - } - } - func testNoPrivateKey() throws { try KeyFileManager(keyType: PGPKey.PUBLIC, keyPath: "", keyHandler: keychain.add).importKey(from: RSA2048.publicKey) XCTAssertFalse(pgpAgent.isPrepared) @@ -180,6 +134,49 @@ final class PGPAgentTest: XCTestCase { XCTAssertEqual(passphraseRequestCalledCount, 3) } + func testMultipleKeysLoaded() throws { + try [ + RSA2048_RSA4096, + ED25519_NISTP384, + ].forEach { testKeyInfo in + keychain.removeAllContent() + try importKeys(testKeyInfo.publicKeys, testKeyInfo.privateKeys) + XCTAssert(pgpAgent.isPrepared) + try pgpAgent.initKeys() + try [ + (true, true), + (true, false), + (false, true), + (false, false), + ].forEach { encryptInArmored, decryptFromArmored in + for id in testKeyInfo.fingerprints { + XCTAssertEqual(try basicEncryptDecrypt(using: pgpAgent, keyID: id, encryptInArmored: encryptInArmored, decryptFromArmored: decryptFromArmored), testData) + } + } + } + } + + func testMultiKeysSelectMatchingPrivateKeyToDecrypt() throws { + keychain.removeAllContent() + try importKeys(RSA2048_RSA4096.publicKeys, RSA2048_RSA4096.privateKeys) + try pgpAgent.initKeys() + try [ + (true, true), + (true, false), + (false, true), + (false, false), + ].forEach { encryptInArmored, decryptFromArmored in + passKit.Defaults.encryptInArmored = encryptInArmored + let encryptedData = try pgpAgent.encrypt(plainData: testData, keyID: RSA2048.fingerprint) + passKit.Defaults.encryptInArmored = decryptFromArmored + // Note: not specifying the keyID to decrypt, so that the agent needs to find the matching private key by itself. + let decryptedData = try pgpAgent.decrypt(encryptedData: encryptedData, requestPGPKeyPassphrase: requestPGPKeyPassphrase) + XCTAssertEqual(decryptedData, testData) + } + } + + // - MARK: Encrypt with multiple keys + func testEncryptWithAllKeys() throws { // When multiple keys are imported, the agent should be able to encrypt without specifying the keyID. // It should use all public keys for which we also have private keys, and the encrypted message should be able to be decrypted by any of the private keys. @@ -209,8 +206,17 @@ final class PGPAgentTest: XCTestCase { } } + // - MARK: Helpers + private func importKeys(_ publicKey: String, _ privateKey: String) throws { try KeyFileManager(keyType: PGPKey.PUBLIC, keyPath: "", keyHandler: keychain.add).importKey(from: publicKey) try KeyFileManager(keyType: PGPKey.PRIVATE, keyPath: "", keyHandler: keychain.add).importKey(from: privateKey) } + + private func basicEncryptDecrypt(using pgpAgent: PGPAgent, keyID: String, encryptKeyID: String? = nil, requestPassphrase: @escaping (String) -> String = requestPGPKeyPassphrase, encryptInArmored: Bool = true, decryptFromArmored: Bool = true) throws -> Data? { + passKit.Defaults.encryptInArmored = encryptInArmored + let encryptedData = try pgpAgent.encrypt(plainData: testData, keyID: keyID) + passKit.Defaults.encryptInArmored = decryptFromArmored + return try pgpAgent.decrypt(encryptedData: encryptedData, keyID: encryptKeyID ?? keyID, requestPGPKeyPassphrase: requestPassphrase) + } } From b7873e6d72a450a54eec4244bd9c0910b5be7304 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 00:29:49 +0100 Subject: [PATCH 08/21] move functions around --- passKit/Crypto/PGPAgent.swift | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index 1c49260..de79b8e 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -105,6 +105,23 @@ public class PGPAgent { return try pgpInterface.encrypt(plainData: plainData, keyIDs: [keyID]) } + @available(*, deprecated, message: "Use encrypt(plainData:keyIDs:) or encryptWithAllKeys(plainData:) instead.") + public func encrypt(plainData: Data) throws -> Data { + try checkAndInit() + guard let pgpInterface else { + throw AppError.encryption + } + return try pgpInterface.encrypt(plainData: plainData, keyID: nil) + } + + public func encryptWithAllKeys(plainData: Data) throws -> Data { + try checkAndInit() + guard let pgpInterface else { + throw AppError.encryption + } + return try pgpInterface.encryptWithAllKeys(plainData: plainData) + } + public func decrypt(encryptedData: Data, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? { // Remember the previous status and set the current status let previousDecryptStatus = latestDecryptStatus @@ -127,23 +144,6 @@ public class PGPAgent { return result } - @available(*, deprecated, message: "Use encrypt(plainData:keyID:) or encryptWithAllKeys(plainData:) instead.") - public func encrypt(plainData: Data) throws -> Data { - try checkAndInit() - guard let pgpInterface else { - throw AppError.encryption - } - return try pgpInterface.encrypt(plainData: plainData, keyID: nil) - } - - public func encryptWithAllKeys(plainData: Data) throws -> Data { - try checkAndInit() - guard let pgpInterface else { - throw AppError.encryption - } - return try pgpInterface.encryptWithAllKeys(plainData: plainData) - } - public var isPrepared: Bool { keyStore.contains(key: PGPKey.PUBLIC.getKeychainKey()) && keyStore.contains(key: PGPKey.PRIVATE.getKeychainKey()) From 09b0b150cef7fe6da2292f7185fb6b3a37717ebd Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 00:48:21 +0100 Subject: [PATCH 09/21] PGPAgent can encrypt with multiple keys --- passKit/Crypto/GopenPGPInterface.swift | 7 ++++-- passKit/Crypto/PGPAgent.swift | 9 ++++++++ passKitTests/Crypto/PGPAgentTest.swift | 30 +++++++++++++++++++++++--- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/passKit/Crypto/GopenPGPInterface.swift b/passKit/Crypto/GopenPGPInterface.swift index 638ce25..3d34a58 100644 --- a/passKit/Crypto/GopenPGPInterface.swift +++ b/passKit/Crypto/GopenPGPInterface.swift @@ -138,8 +138,11 @@ struct GopenPGPInterface: PGPInterface { } func encrypt(plainData: Data, keyIDs: [String]) throws -> Data { - let keys: [CryptoKey] = keyIDs.compactMap { keyID in - publicKeys.first(where: { key, _ in key.hasSuffix(keyID.lowercased()) })?.value + let keys: [CryptoKey] = try keyIDs.map { keyID in + guard let key = publicKeys.first(where: { key, _ in key.hasSuffix(keyID.lowercased()) })?.value else { + throw AppError.pgpPublicKeyNotFound(keyID: keyID) + } + return key } guard let firstKey = keys.first else { throw AppError.encryption diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index de79b8e..d482d9f 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -89,6 +89,7 @@ public class PGPAgent { return result } + @available(*, deprecated, message: "Use encrypt(plainData:keyIDs:) instead.") public func encrypt(plainData: Data, keyID: String) throws -> Data { try checkAndInit() guard let pgpInterface else { @@ -105,6 +106,14 @@ public class PGPAgent { return try pgpInterface.encrypt(plainData: plainData, keyIDs: [keyID]) } + public func encrypt(plainData: Data, keyIDs: [String]) throws -> Data { + try checkAndInit() + guard let pgpInterface else { + throw AppError.encryption + } + return try pgpInterface.encrypt(plainData: plainData, keyIDs: keyIDs) + } + @available(*, deprecated, message: "Use encrypt(plainData:keyIDs:) or encryptWithAllKeys(plainData:) instead.") public func encrypt(plainData: Data) throws -> Data { try checkAndInit() diff --git a/passKitTests/Crypto/PGPAgentTest.swift b/passKitTests/Crypto/PGPAgentTest.swift index b7ba158..b90e17f 100644 --- a/passKitTests/Crypto/PGPAgentTest.swift +++ b/passKitTests/Crypto/PGPAgentTest.swift @@ -167,7 +167,7 @@ final class PGPAgentTest: XCTestCase { (false, false), ].forEach { encryptInArmored, decryptFromArmored in passKit.Defaults.encryptInArmored = encryptInArmored - let encryptedData = try pgpAgent.encrypt(plainData: testData, keyID: RSA2048.fingerprint) + let encryptedData = try pgpAgent.encrypt(plainData: testData, keyIDs: [RSA2048.fingerprint]) passKit.Defaults.encryptInArmored = decryptFromArmored // Note: not specifying the keyID to decrypt, so that the agent needs to find the matching private key by itself. let decryptedData = try pgpAgent.decrypt(encryptedData: encryptedData, requestPGPKeyPassphrase: requestPGPKeyPassphrase) @@ -177,6 +177,30 @@ final class PGPAgentTest: XCTestCase { // - MARK: Encrypt with multiple keys + func testEncryptWithMultipleKeys() throws { + keychain.removeAllContent() + // no private key for ED25519 + try importKeys(RSA2048_RSA4096.publicKeys | ED25519.publicKey, RSA2048_RSA4096.privateKeys) + try pgpAgent.initKeys() + + let encryptedData = try pgpAgent.encrypt(plainData: testData, keyIDs: RSA2048_RSA4096.fingerprints + [ED25519.fingerprint]) + + try [RSA2048.fingerprint, RSA4096.fingerprint].forEach { keyID in + let decryptedData = try pgpAgent.decrypt(encryptedData: encryptedData, keyID: keyID, requestPGPKeyPassphrase: requestPGPKeyPassphrase) + XCTAssertEqual(decryptedData, testData) + } + + XCTAssertThrowsError(try pgpAgent.decrypt(encryptedData: encryptedData, keyID: ED25519.fingerprint, requestPGPKeyPassphrase: requestPGPKeyPassphrase)) { + XCTAssertEqual($0 as! AppError, AppError.pgpPrivateKeyNotFound(keyID: ED25519.fingerprint)) + } + + // load private key for ED25519 + try importKeys(RSA2048_RSA4096.publicKeys | ED25519.publicKey, RSA2048_RSA4096.privateKeys | ED25519.privateKey) + try pgpAgent.initKeys() + let decryptedData = try pgpAgent.decrypt(encryptedData: encryptedData, keyID: ED25519.fingerprint, requestPGPKeyPassphrase: requestPGPKeyPassphrase) + XCTAssertEqual(decryptedData, testData) + } + func testEncryptWithAllKeys() throws { // When multiple keys are imported, the agent should be able to encrypt without specifying the keyID. // It should use all public keys for which we also have private keys, and the encrypted message should be able to be decrypted by any of the private keys. @@ -197,7 +221,7 @@ final class PGPAgentTest: XCTestCase { XCTAssertEqual($0 as! AppError, AppError.pgpPrivateKeyNotFound(keyID: ED25519.fingerprint)) } - // add private key for ED25519 + // load private key for ED25519 try importKeys(RSA2048_RSA4096.publicKeys | ED25519.publicKey, RSA2048_RSA4096.privateKeys | ED25519.privateKey) try pgpAgent.initKeys() @@ -215,7 +239,7 @@ final class PGPAgentTest: XCTestCase { private func basicEncryptDecrypt(using pgpAgent: PGPAgent, keyID: String, encryptKeyID: String? = nil, requestPassphrase: @escaping (String) -> String = requestPGPKeyPassphrase, encryptInArmored: Bool = true, decryptFromArmored: Bool = true) throws -> Data? { passKit.Defaults.encryptInArmored = encryptInArmored - let encryptedData = try pgpAgent.encrypt(plainData: testData, keyID: keyID) + let encryptedData = try pgpAgent.encrypt(plainData: testData, keyIDs: [keyID]) passKit.Defaults.encryptInArmored = decryptFromArmored return try pgpAgent.decrypt(encryptedData: encryptedData, keyID: encryptKeyID ?? keyID, requestPGPKeyPassphrase: requestPassphrase) } From e69e590e36f6675dd90b3e0bb8af2bd971f825f6 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 09:01:12 +0100 Subject: [PATCH 10/21] replace calls to deprecated function changes PasswordStore.encrypt behavior when .gpg-id support is off (default): old: * ignores passed in keyID * encrypt with first public key in keychain (gopenPGP), or entire keychain (ObjectivePGP) new: * honor passed in keyID * encrypt with all keys in keychain --- passKit/Crypto/GopenPGPInterface.swift | 9 --- passKit/Crypto/ObjectivePGPInterface.swift | 6 -- passKit/Crypto/PGPAgent.swift | 26 -------- passKit/Crypto/PGPInterface.swift | 2 - passKit/Models/PasswordStore.swift | 9 ++- .../LowLevel/PGPAgentLowLevelTests.swift | 60 ++----------------- passKitTests/Mocks/MockPGPInterface.swift | 9 --- 7 files changed, 11 insertions(+), 110 deletions(-) diff --git a/passKit/Crypto/GopenPGPInterface.swift b/passKit/Crypto/GopenPGPInterface.swift index 3d34a58..c185add 100644 --- a/passKit/Crypto/GopenPGPInterface.swift +++ b/passKit/Crypto/GopenPGPInterface.swift @@ -123,15 +123,6 @@ struct GopenPGPInterface: PGPInterface { } } - @available(*, deprecated, message: "Use encrypt(plainData:keyIDs:) instead.") - func encrypt(plainData: Data, keyID: String?) throws -> Data { - guard let keyID = keyID ?? publicKeys.keys.first else { - // this is invalid, but we want the new function to throw the error for us - return try encrypt(plainData: plainData, keyIDs: []) - } - return try encrypt(plainData: plainData, keyIDs: [keyID]) - } - func encryptWithAllKeys(plainData: Data) throws -> Data { let keyIDs = publicKeys.keys.filter { key in privateKeys.keys.contains(key) } return try encrypt(plainData: plainData, keyIDs: keyIDs) diff --git a/passKit/Crypto/ObjectivePGPInterface.swift b/passKit/Crypto/ObjectivePGPInterface.swift index 01a5a2b..cf13082 100644 --- a/passKit/Crypto/ObjectivePGPInterface.swift +++ b/passKit/Crypto/ObjectivePGPInterface.swift @@ -33,12 +33,6 @@ struct ObjectivePGPInterface: PGPInterface { } } - @available(*, deprecated, message: "Use encrypt(plainData:keyIDs:) instead.") - func encrypt(plainData: Data, keyID _: String?) throws -> Data { - // Backwards compatibility: ignore keyID parameter and encrypted with all keys in the keyring - try encryptWithAllKeys(plainData: plainData) - } - func encryptWithAllKeys(plainData: Data) throws -> Data { try encrypt(plainData: plainData, keyIDs: keyID) } diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index d482d9f..51d6e6a 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -89,23 +89,6 @@ public class PGPAgent { return result } - @available(*, deprecated, message: "Use encrypt(plainData:keyIDs:) instead.") - public func encrypt(plainData: Data, keyID: String) throws -> Data { - try checkAndInit() - guard let pgpInterface else { - throw AppError.encryption - } - var keyID = keyID - if !pgpInterface.containsPublicKey(with: keyID) { - if pgpInterface.keyID.count == 1 { - keyID = pgpInterface.keyID.first! - } else { - throw AppError.pgpPublicKeyNotFound(keyID: keyID) - } - } - return try pgpInterface.encrypt(plainData: plainData, keyIDs: [keyID]) - } - public func encrypt(plainData: Data, keyIDs: [String]) throws -> Data { try checkAndInit() guard let pgpInterface else { @@ -114,15 +97,6 @@ public class PGPAgent { return try pgpInterface.encrypt(plainData: plainData, keyIDs: keyIDs) } - @available(*, deprecated, message: "Use encrypt(plainData:keyIDs:) or encryptWithAllKeys(plainData:) instead.") - public func encrypt(plainData: Data) throws -> Data { - try checkAndInit() - guard let pgpInterface else { - throw AppError.encryption - } - return try pgpInterface.encrypt(plainData: plainData, keyID: nil) - } - public func encryptWithAllKeys(plainData: Data) throws -> Data { try checkAndInit() guard let pgpInterface else { diff --git a/passKit/Crypto/PGPInterface.swift b/passKit/Crypto/PGPInterface.swift index f90c4bd..aff2344 100644 --- a/passKit/Crypto/PGPInterface.swift +++ b/passKit/Crypto/PGPInterface.swift @@ -9,8 +9,6 @@ protocol PGPInterface { func decrypt(encryptedData: Data, keyIDHint: String?, passPhraseForKey: @escaping (String) -> String) throws -> Data? - @available(*, deprecated, message: "Use encrypt(plainData:keyIDs:) instead.") - func encrypt(plainData: Data, keyID: String?) throws -> Data // encrypt with all public keys for which we also have a private key func encryptWithAllKeys(plainData: Data) throws -> Data func encrypt(plainData: Data, keyIDs: [String]) throws -> Data diff --git a/passKit/Models/PasswordStore.swift b/passKit/Models/PasswordStore.swift index c822ffa..9a0e0f3 100644 --- a/passKit/Models/PasswordStore.swift +++ b/passKit/Models/PasswordStore.swift @@ -420,12 +420,15 @@ public class PasswordStore { } public func encrypt(password: Password, keyID: String? = nil) throws -> Data { + var keyID = keyID if Defaults.isEnableGPGIDOn { let encryptedDataPath = password.fileURL(in: storeURL) - let keyID = keyID ?? findGPGID(from: encryptedDataPath) - return try PGPAgent.shared.encrypt(plainData: password.plainData, keyID: keyID) + keyID = keyID ?? findGPGID(from: encryptedDataPath) } - return try PGPAgent.shared.encrypt(plainData: password.plainData) + if let keyID { + return try PGPAgent.shared.encrypt(plainData: password.plainData, keyIDs: [keyID]) + } + return try PGPAgent.shared.encryptWithAllKeys(plainData: password.plainData) } public func removeGitSSHKeys() { diff --git a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift index 73253f6..f562621 100644 --- a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift +++ b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift @@ -526,28 +526,13 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(passphraseRequests, [shortID]) } - /// Encrypt with short ID: when containsPublicKey matches (via suffix), the short ID is passed to interface. - func testEncryptWithKeyID_shortIDRecognized_shortIDFlowsThrough() throws { - let shortID = "a1024dae" - let longFingerprint = "4712286271220db299883ea7062e678da1024dae" - mockPGP.publicKeyIDs = [longFingerprint] - mockPGP.keyIDs = [longFingerprint] - - _ = try agent.encrypt(plainData: testDecryptedData, keyID: shortID) - - XCTAssertEqual(mockPGP.containsPublicKeyCalls, [shortID]) - XCTAssertEqual(mockPGP.encryptMultiKeyCalls.count, 1) - XCTAssertEqual(mockPGP.encryptMultiKeyCalls[0].keyIDs, [shortID]) - } - // MARK: - Encrypt passthrough tests (for completeness of mock interaction) - /// encrypt(plainData:keyID:) calls containsPublicKey and passes data through via encrypt(plainData:keyIDs:). - func testEncryptWithKeyID_keyFound_callsInterface() throws { + func testEncryptWithKeyIDs_passesThrough() throws { let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.publicKeyIDs = [longFingerprint] - let result = try agent.encrypt(plainData: testDecryptedData, keyID: longFingerprint) + let result = try agent.encrypt(plainData: testDecryptedData, keyIDs: [longFingerprint]) XCTAssertEqual(result, mockPGP.encryptResult) XCTAssertEqual(mockPGP.encryptMultiKeyCalls.count, 1) @@ -555,48 +540,14 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(mockPGP.encryptMultiKeyCalls[0].plainData, testDecryptedData) } - /// encrypt with unknown key and single available key falls back. - func testEncryptWithKeyID_keyNotFound_singleKey_fallsBack() throws { - let shortID = "e9444483" - let longFingerprint = "4712286271220db299883ea7062e678da1024dae" - mockPGP.publicKeyIDs = [] - mockPGP.keyIDs = [longFingerprint] - - let result = try agent.encrypt(plainData: testDecryptedData, keyID: shortID) - - XCTAssertEqual(result, mockPGP.encryptResult) - XCTAssertEqual(mockPGP.containsPublicKeyCalls, [shortID]) - XCTAssertEqual(mockPGP.encryptMultiKeyCalls[0].keyIDs, [longFingerprint]) - } - - /// encrypt with unknown key and multiple keys throws. - func testEncryptWithKeyID_keyNotFound_multipleKeys_throws() { - mockPGP.publicKeyIDs = [] - mockPGP.keyIDs = ["4712286271220db299883ea7062e678da1024dae", "787eae1a5fa3e749aa34cc6aa0645ebed862027e"] - - XCTAssertThrowsError(try agent.encrypt(plainData: testDecryptedData, keyID: "a1024dae")) { error in - XCTAssertEqual(error as? AppError, AppError.pgpPublicKeyNotFound(keyID: "a1024dae")) - } - XCTAssertEqual(mockPGP.encryptMultiKeyCalls.count, 0) - } - - /// encrypt(plainData:) without keyID passes nil to the deprecated interface method. - func testEncryptNoKeyID_passesNilToInterface() throws { - let result = try agent.encrypt(plainData: testDecryptedData) - - XCTAssertEqual(result, mockPGP.encryptResult) - XCTAssertEqual(mockPGP.encryptCalls.count, 1) - XCTAssertNil(mockPGP.encryptCalls[0].keyID) - } - /// encrypt propagates errors from interface. - func testEncrypt_interfaceThrows_propagatesError() { + func testEncryptWithKeyIDs_interfaceThrows_propagatesError() { let shortID = "a1024dae" let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.publicKeyIDs = [longFingerprint] mockPGP.encryptError = AppError.encryption - XCTAssertThrowsError(try agent.encrypt(plainData: testDecryptedData, keyID: shortID)) { error in + XCTAssertThrowsError(try agent.encrypt(plainData: testDecryptedData, keyIDs: [shortID])) { error in XCTAssertEqual(error as? AppError, AppError.encryption) } } @@ -614,7 +565,6 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(mockPGP.encryptWithAllKeysCalls[0].plainData, testDecryptedData) // Does not call containsPublicKey or the single/multi-key encrypt methods. XCTAssertEqual(mockPGP.containsPublicKeyCalls.count, 0) - XCTAssertEqual(mockPGP.encryptCalls.count, 0) XCTAssertEqual(mockPGP.encryptMultiKeyCalls.count, 0) } @@ -627,7 +577,7 @@ final class PGPAgentLowLevelTests: XCTestCase { } } - /// encryptWithAllKeys throws encryption error when pgpInterface is nil (checkAndInit fails). + /// encryptWithAllKeys throws keyImport when checkAndInit triggers initKeys without PGP keys. func testEncryptWithAllKeys_checkAndInit_requiresPGPKeyPassphraseInKeystore() throws { keychain.removeContent(for: Globals.pgpKeyPassphrase) diff --git a/passKitTests/Mocks/MockPGPInterface.swift b/passKitTests/Mocks/MockPGPInterface.swift index 481d052..6640986 100644 --- a/passKitTests/Mocks/MockPGPInterface.swift +++ b/passKitTests/Mocks/MockPGPInterface.swift @@ -47,7 +47,6 @@ class MockPGPInterface: PGPInterface { var decryptCalls: [DecryptCall] = [] var resolvedPassphrases: [String] = [] - var encryptCalls: [EncryptCall] = [] var encryptMultiKeyCalls: [EncryptMultiKeyCall] = [] var encryptWithAllKeysCalls: [EncryptWithAllKeysCall] = [] var containsPublicKeyCalls: [String] = [] @@ -66,14 +65,6 @@ class MockPGPInterface: PGPInterface { return decryptResult } - func encrypt(plainData: Data, keyID: String?) throws -> Data { - encryptCalls.append(EncryptCall(plainData: plainData, keyID: keyID)) - if let error = encryptError { - throw error - } - return encryptResult - } - func encryptWithAllKeys(plainData: Data) throws -> Data { encryptWithAllKeysCalls.append(EncryptWithAllKeysCall(plainData: plainData)) if let error = encryptError { From 9e3e3d1134392b1f61dcc67f0b864efd731c7675 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 16:10:08 +0100 Subject: [PATCH 11/21] remove fallback behavior this logic should not be relevant anywhere --- passKit/Crypto/PGPAgent.swift | 7 +-- .../LowLevel/PGPAgentLowLevelTests.swift | 62 +------------------ 2 files changed, 4 insertions(+), 65 deletions(-) diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index 51d6e6a..44b3443 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -60,13 +60,8 @@ public class PGPAgent { throw AppError.decryption } - var keyID = keyID if !pgpInterface.containsPrivateKey(with: keyID) { - if pgpInterface.keyID.count == 1 { - keyID = pgpInterface.keyID.first! - } else { - throw AppError.pgpPrivateKeyNotFound(keyID: keyID) - } + throw AppError.pgpPrivateKeyNotFound(keyID: keyID) } // Remember the previous status and set the current status diff --git a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift index f562621..7ef764c 100644 --- a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift +++ b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift @@ -69,21 +69,7 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(mockPGP.decryptCalls[0].encryptedData, testEncryptedData) } - /// When the private key is NOT found but there's exactly one key, falls back to that key. - func testDecryptWithKeyID_keyNotFound_singleKey_fallsBackToOnlyKey() throws { - let longFingerprint = "4712286271220db299883ea7062e678da1024dae" - mockPGP.privateKeyIDs = [] // requested key not found - mockPGP.keyIDs = [longFingerprint] - - _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: "UNKNOWN", requestPGPKeyPassphrase: passphraseCallback("pass")) - - XCTAssertEqual(mockPGP.decryptCalls.count, 1) - // The keyID passed to pgpInterface.decrypt should be the fallback key, not the requested one. - XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) - } - - /// When the private key is NOT found and there are multiple keys, throws pgpPrivateKeyNotFound. - func testDecryptWithKeyID_keyNotFound_multipleKeys_throws() { + func testDecryptWithKeyID_keyNotFound_throws() { mockPGP.privateKeyIDs = [] mockPGP.keyIDs = ["4712286271220db299883ea7062e678da1024dae", "787eae1a5fa3e749aa34cc6aa0645ebed862027e"] @@ -378,9 +364,8 @@ final class PGPAgentLowLevelTests: XCTestCase { // MARK: - Key resolution error vs decrypt status ordering - /// When pgpPrivateKeyNotFound is thrown (key not found, multiple keys), - /// latestDecryptStatus is NOT changed because the error occurs BEFORE the status update. - func testDecryptWithKeyID_keyNotFound_multipleKeys_doesNotChangeDecryptStatus() throws { + /// When pgpPrivateKeyNotFound is thrown, latestDecryptStatus is NOT changed because the error occurs BEFORE the status update. + func testDecryptWithKeyID_keyNotFound_doesNotChangeDecryptStatus() throws { let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [] mockPGP.keyIDs = [longFingerprint, "787eae1a5fa3e749aa34cc6aa0645ebed862027e"] @@ -401,33 +386,6 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(mockPGP.resolvedPassphrases, ["cached-pass"]) } - /// After failure + key fallback: passphrase is always requested using the RESOLVED (fallback) keyID. - func testDecryptWithKeyID_afterFailure_keyFallback_requestsWithResolvedKeyID() throws { - let shortID = "a1024dae" - let longFingerprint1 = "4712286271220db299883ea7062e678da1024dae" - let longFingerprint2 = "5fccb081ab8af48972999e2ae750acbfe9444483" - mockPGP.privateKeyIDs = [longFingerprint1] - - // Force a failure using a short ID that suffix-matches longFingerprint1. - mockPGP.decryptError = AppError.wrongPassphrase - _ = try? agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("bad")) - - // Now try with an unknown key that falls back to a different long fingerprint. - mockPGP.decryptError = nil - mockPGP.privateKeyIDs = [] - mockPGP.keyIDs = [longFingerprint2] - mockPGP.decryptCalls.removeAll() - mockPGP.resolvedPassphrases.removeAll() - mockPGP.selectedKeyForPassphrase = longFingerprint2 - passphraseRequests.removeAll() - - _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: "e9444483", requestPGPKeyPassphrase: passphraseCallback("pass")) - - XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint2) - XCTAssertEqual(mockPGP.resolvedPassphrases, ["pass"]) - XCTAssertEqual(passphraseRequests, [longFingerprint2]) - } - // MARK: - Cross-overload latestDecryptStatus interaction /// latestDecryptStatus is shared between both overloads. @@ -494,20 +452,6 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(mockPGP.decryptCalls[0].keyID, shortID) } - /// When caller passes a short ID and containsPrivateKey does NOT match, but there's one key, - /// the long fingerprint from keyID[0] is forwarded instead. - func testDecryptWithKeyID_shortIDNotRecognized_singleKey_resolvesToLongFingerprint() throws { - let shortID = "a1024dae" - let longFingerprint = "4712286271220db299883ea7062e678da1024dae" - mockPGP.privateKeyIDs = [] // short ID doesn't match - mockPGP.keyIDs = [longFingerprint] - - _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("pass")) - - XCTAssertEqual(mockPGP.containsPrivateKeyCalls, [shortID]) - XCTAssertEqual(mockPGP.decryptCalls[0].keyID, longFingerprint) - } - /// Passphrase stored under long fingerprint is NOT found when the short ID is used for lookup func testDecryptWithKeyID_shortIDRecognized_passphraseStoredUnderLongID_missesKeystore() throws { let shortID = "a1024dae" From 4e19d9e714e4dc094d0c57f4a93aee962b24b24d Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 16:15:49 +0100 Subject: [PATCH 12/21] remove irrelevant properties from mocks --- passKitTests/LowLevel/PGPAgentLowLevelTests.swift | 4 ---- passKitTests/Mocks/MockPGPInterface.swift | 6 ++---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift index 7ef764c..8b03310 100644 --- a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift +++ b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift @@ -71,7 +71,6 @@ final class PGPAgentLowLevelTests: XCTestCase { func testDecryptWithKeyID_keyNotFound_throws() { mockPGP.privateKeyIDs = [] - mockPGP.keyIDs = ["4712286271220db299883ea7062e678da1024dae", "787eae1a5fa3e749aa34cc6aa0645ebed862027e"] XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, keyID: "UNKNOWN", requestPGPKeyPassphrase: passphraseCallback("pass"))) { error in XCTAssertEqual(error as? AppError, AppError.pgpPrivateKeyNotFound(keyID: "UNKNOWN")) @@ -368,7 +367,6 @@ final class PGPAgentLowLevelTests: XCTestCase { func testDecryptWithKeyID_keyNotFound_doesNotChangeDecryptStatus() throws { let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [] - mockPGP.keyIDs = [longFingerprint, "787eae1a5fa3e749aa34cc6aa0645ebed862027e"] // This throws pgpPrivateKeyNotFound without changing latestDecryptStatus. XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, keyID: "UNKNOWN", requestPGPKeyPassphrase: passphraseCallback("pass"))) @@ -444,7 +442,6 @@ final class PGPAgentLowLevelTests: XCTestCase { let shortID = "a1024dae" let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [longFingerprint] - mockPGP.keyIDs = [longFingerprint] _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("pass")) @@ -457,7 +454,6 @@ final class PGPAgentLowLevelTests: XCTestCase { let shortID = "a1024dae" let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [longFingerprint] - mockPGP.keyIDs = [longFingerprint] mockPGP.selectedKeyForPassphrase = shortID // Store passphrase under the LONG fingerprint. diff --git a/passKitTests/Mocks/MockPGPInterface.swift b/passKitTests/Mocks/MockPGPInterface.swift index 6640986..b6402b9 100644 --- a/passKitTests/Mocks/MockPGPInterface.swift +++ b/passKitTests/Mocks/MockPGPInterface.swift @@ -9,8 +9,6 @@ import Foundation class MockPGPInterface: PGPInterface { // MARK: - Configuration - var keyIDs: [String] = [] - var shortKeyIDs: [String] = [] var publicKeyIDs: Set = [] var privateKeyIDs: Set = [] @@ -91,6 +89,6 @@ class MockPGPInterface: PGPInterface { return privateKeyIDs.contains { $0.hasSuffix(keyID.lowercased()) } } - var keyID: [String] { keyIDs } - var shortKeyID: [String] { shortKeyIDs } + var keyID: [String] { [] } // currently not relevant in these tests + var shortKeyID: [String] { [] } // currently not relevant in these tests } From 5a92b6fda742b8c49b1d1589acb6af314ff561e5 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 16:16:50 +0100 Subject: [PATCH 13/21] clarify public vs private keys + make prvate key IDs available --- .../PasswordDetailTableViewController.swift | 4 ++-- .../SettingsTableViewController.swift | 10 ++++++---- pass/Services/PasswordDecryptor.swift | 2 +- pass/Services/PasswordEncryptor.swift | 2 +- passKit/Crypto/GopenPGPInterface.swift | 13 ++++++++---- passKit/Crypto/ObjectivePGPInterface.swift | 20 ++++++++++++++----- passKit/Crypto/PGPAgent.swift | 8 ++++---- passKit/Crypto/PGPInterface.swift | 6 ++---- .../Extensions/UIAlertActionExtension.swift | 4 ++-- passKitTests/Crypto/PGPAgentTest.swift | 6 +++++- passKitTests/Mocks/MockPGPInterface.swift | 11 ++++++++-- 11 files changed, 56 insertions(+), 30 deletions(-) diff --git a/pass/Controllers/PasswordDetailTableViewController.swift b/pass/Controllers/PasswordDetailTableViewController.swift index 98aa66a..37efdd1 100644 --- a/pass/Controllers/PasswordDetailTableViewController.swift +++ b/pass/Controllers/PasswordDetailTableViewController.swift @@ -128,7 +128,7 @@ class PasswordDetailTableViewController: UITableViewController, UIGestureRecogni // alert: cancel or try again let alert = UIAlertController(title: "CannotShowPassword".localize(), message: AppError.pgpPrivateKeyNotFound(keyID: key).localizedDescription, preferredStyle: .alert) alert.addAction(UIAlertAction.cancelAndPopView(controller: self)) - let selectKey = UIAlertAction.selectKey(controller: self) { action in + let selectKey = UIAlertAction.selectKey(type: .PRIVATE, controller: self) { action in self.decryptThenShowPasswordLocalKey(keyID: action.title) } alert.addAction(selectKey) @@ -223,7 +223,7 @@ class PasswordDetailTableViewController: UITableViewController, UIGestureRecogni SVProgressHUD.dismiss() let alert = UIAlertController(title: "Cannot Edit Password", message: AppError.pgpPublicKeyNotFound(keyID: key).localizedDescription, preferredStyle: .alert) alert.addAction(UIAlertAction.cancelAndPopView(controller: self)) - let selectKey = UIAlertAction.selectKey(controller: self) { action in + let selectKey = UIAlertAction.selectKey(type: .PUBLIC, controller: self) { action in self.saveEditPassword(password: password, keyID: action.title) } alert.addAction(selectKey) diff --git a/pass/Controllers/SettingsTableViewController.swift b/pass/Controllers/SettingsTableViewController.swift index b715069..b0a3e61 100644 --- a/pass/Controllers/SettingsTableViewController.swift +++ b/pass/Controllers/SettingsTableViewController.swift @@ -89,10 +89,12 @@ class SettingsTableViewController: UITableViewController, UITabBarControllerDele private func setPGPKeyTableViewCellDetailText() { var label = "NotSet".localize() - let keyID = (try? PGPAgent.shared.getShortKeyID()) ?? [] - if keyID.count == 1 { - label = keyID.first ?? "" - } else if keyID.count > 1 { + var keyIDs = Set((try? PGPAgent.shared.getShortKeyIDs(type: .PRIVATE)) ?? []) + keyIDs.formUnion((try? PGPAgent.shared.getShortKeyIDs(type: .PUBLIC)) ?? []) + + if keyIDs.count == 1 { + label = keyIDs.first ?? "" + } else if keyIDs.count > 1 { label = "Multiple" } if Defaults.isYubiKeyEnabled { diff --git a/pass/Services/PasswordDecryptor.swift b/pass/Services/PasswordDecryptor.swift index 49e7845..8824bb3 100644 --- a/pass/Services/PasswordDecryptor.swift +++ b/pass/Services/PasswordDecryptor.swift @@ -40,7 +40,7 @@ func decryptPassword( DispatchQueue.main.async { let alert = UIAlertController(title: "CannotShowPassword".localize(), message: AppError.pgpPrivateKeyNotFound(keyID: key).localizedDescription, preferredStyle: .alert) alert.addAction(UIAlertAction.cancelAndPopView(controller: controller)) - let selectKey = UIAlertAction.selectKey(controller: controller) { action in + let selectKey = UIAlertAction.selectKey(type: PGPKey.PRIVATE, controller: controller) { action in decryptPassword(in: controller, with: passwordPath, using: action.title, completion: completion) } alert.addAction(selectKey) diff --git a/pass/Services/PasswordEncryptor.swift b/pass/Services/PasswordEncryptor.swift index 254f045..6159881 100644 --- a/pass/Services/PasswordEncryptor.swift +++ b/pass/Services/PasswordEncryptor.swift @@ -19,7 +19,7 @@ func encryptPassword(in controller: UIViewController, with password: Password, k DispatchQueue.main.async { let alert = UIAlertController(title: "Cannot Encrypt Password", message: AppError.pgpPublicKeyNotFound(keyID: key).localizedDescription, preferredStyle: .alert) alert.addAction(UIAlertAction.cancelAndPopView(controller: controller)) - let selectKey = UIAlertAction.selectKey(controller: controller) { action in + let selectKey = UIAlertAction.selectKey(type: .PUBLIC, controller: controller) { action in encryptPassword(in: controller, with: password, keyID: action.title, completion: completion) } alert.addAction(selectKey) diff --git a/passKit/Crypto/GopenPGPInterface.swift b/passKit/Crypto/GopenPGPInterface.swift index c185add..3917df1 100644 --- a/passKit/Crypto/GopenPGPInterface.swift +++ b/passKit/Crypto/GopenPGPInterface.swift @@ -167,12 +167,17 @@ struct GopenPGPInterface: PGPInterface { return encryptedData.getBinary()! } - var keyID: [String] { - publicKeys.keys.map { $0.uppercased() } + func getKeyIDs(type: PGPKey) -> [String] { + switch type { + case .PUBLIC: + return publicKeys.keys.map { $0.uppercased() } + case .PRIVATE: + return privateKeys.keys.map { $0.uppercased() } + } } - var shortKeyID: [String] { - publicKeys.keys.map { $0.suffix(8).uppercased() } + func getShortKeyIDs(type: PGPKey) -> [String] { + getKeyIDs(type: type).map { $0.suffix(8).uppercased() } } private func findDecryptionKey(message: CryptoPGPMessage, keyIDHint: String?) throws -> CryptoKey? { diff --git a/passKit/Crypto/ObjectivePGPInterface.swift b/passKit/Crypto/ObjectivePGPInterface.swift index cf13082..e1425c3 100644 --- a/passKit/Crypto/ObjectivePGPInterface.swift +++ b/passKit/Crypto/ObjectivePGPInterface.swift @@ -34,7 +34,8 @@ struct ObjectivePGPInterface: PGPInterface { } func encryptWithAllKeys(plainData: Data) throws -> Data { - try encrypt(plainData: plainData, keyIDs: keyID) + let keys = keyring.keys.filter { $0.isPublic && $0.isSecret } + return try encrypt(plainData: plainData, keyIDs: keys.map(\.keyID.longIdentifier)) } func encrypt(plainData: Data, keyIDs: [String]) throws -> Data { @@ -60,11 +61,20 @@ struct ObjectivePGPInterface: PGPInterface { keyring.findKey(keyID)?.isSecret ?? false } - var keyID: [String] { - keyring.keys.map(\.keyID.longIdentifier) + func getKeyIDs(type: PGPKey) -> [String] { + getKeys(type: type).map(\.keyID.longIdentifier) } - var shortKeyID: [String] { - keyring.keys.map(\.keyID.shortIdentifier) + func getShortKeyIDs(type: PGPKey) -> [String] { + getKeys(type: type).map(\.keyID.shortIdentifier) + } + + private func getKeys(type: PGPKey) -> [Key] { + switch type { + case .PUBLIC: + keyring.keys.filter(\.isPublic) + case .PRIVATE: + keyring.keys.filter(\.isSecret) + } } } diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index 44b3443..9ce335d 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -43,14 +43,14 @@ public class PGPAgent { pgpInterface != nil } - public func getKeyID() throws -> [String] { + public func getKeyIDs(type: PGPKey) throws -> [String] { try checkAndInit() - return pgpInterface?.keyID ?? [] + return pgpInterface?.getKeyIDs(type: type).sorted() ?? [] } - public func getShortKeyID() throws -> [String] { + public func getShortKeyIDs(type: PGPKey) throws -> [String] { try checkAndInit() - return pgpInterface?.shortKeyID.sorted() ?? [] + return pgpInterface?.getShortKeyIDs(type: type).sorted() ?? [] } public func decrypt(encryptedData: Data, keyID: String, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? { diff --git a/passKit/Crypto/PGPInterface.swift b/passKit/Crypto/PGPInterface.swift index aff2344..2afce6d 100644 --- a/passKit/Crypto/PGPInterface.swift +++ b/passKit/Crypto/PGPInterface.swift @@ -14,10 +14,8 @@ protocol PGPInterface { func encrypt(plainData: Data, keyIDs: [String]) throws -> Data func containsPublicKey(with keyID: String) -> Bool - func containsPrivateKey(with keyID: String) -> Bool - var keyID: [String] { get } - - var shortKeyID: [String] { get } + func getKeyIDs(type: PGPKey) -> [String] + func getShortKeyIDs(type: PGPKey) -> [String] } diff --git a/passKit/Extensions/UIAlertActionExtension.swift b/passKit/Extensions/UIAlertActionExtension.swift index b49ba07..d258e3b 100644 --- a/passKit/Extensions/UIAlertActionExtension.swift +++ b/passKit/Extensions/UIAlertActionExtension.swift @@ -38,10 +38,10 @@ public extension UIAlertAction { } } - static func selectKey(controller: UIViewController, handler: ((UIAlertAction) -> Void)?) -> UIAlertAction { + static func selectKey(type: PGPKey, controller: UIViewController, handler: ((UIAlertAction) -> Void)?) -> UIAlertAction { UIAlertAction(title: "Select Key", style: .default) { _ in let selectKeyAlert = UIAlertController(title: "Select from imported keys", message: nil, preferredStyle: .actionSheet) - try? PGPAgent.shared.getShortKeyID().forEach { keyID in + try? PGPAgent.shared.getShortKeyIDs(type: type).forEach { keyID in let action = UIAlertAction(title: keyID, style: .default, handler: handler) selectKeyAlert.addAction(action) } diff --git a/passKitTests/Crypto/PGPAgentTest.swift b/passKitTests/Crypto/PGPAgentTest.swift index b90e17f..531ecc1 100644 --- a/passKitTests/Crypto/PGPAgentTest.swift +++ b/passKitTests/Crypto/PGPAgentTest.swift @@ -48,7 +48,8 @@ final class PGPAgentTest: XCTestCase { try importKeys(testKeyInfo.publicKey, testKeyInfo.privateKey) XCTAssert(pgpAgent.isPrepared) try pgpAgent.initKeys() - XCTAssert(try pgpAgent.getKeyID().first!.lowercased().hasSuffix(testKeyInfo.fingerprint)) + XCTAssert(try pgpAgent.getKeyIDs(type: .PUBLIC).first!.lowercased().hasSuffix(testKeyInfo.fingerprint)) + XCTAssert(try pgpAgent.getKeyIDs(type: .PRIVATE).first!.lowercased().hasSuffix(testKeyInfo.fingerprint)) try [ (true, true), (true, false), @@ -183,6 +184,9 @@ final class PGPAgentTest: XCTestCase { try importKeys(RSA2048_RSA4096.publicKeys | ED25519.publicKey, RSA2048_RSA4096.privateKeys) try pgpAgent.initKeys() + XCTAssertEqual(try pgpAgent.getKeyIDs(type: .PUBLIC).map(\.localizedLowercase).sorted(), (RSA2048_RSA4096.longFingerprints + [ED25519.longFingerprint]).sorted()) + XCTAssertEqual(try pgpAgent.getKeyIDs(type: .PRIVATE).map(\.localizedLowercase).sorted(), RSA2048_RSA4096.longFingerprints.sorted()) + let encryptedData = try pgpAgent.encrypt(plainData: testData, keyIDs: RSA2048_RSA4096.fingerprints + [ED25519.fingerprint]) try [RSA2048.fingerprint, RSA4096.fingerprint].forEach { keyID in diff --git a/passKitTests/Mocks/MockPGPInterface.swift b/passKitTests/Mocks/MockPGPInterface.swift index b6402b9..9593669 100644 --- a/passKitTests/Mocks/MockPGPInterface.swift +++ b/passKitTests/Mocks/MockPGPInterface.swift @@ -89,6 +89,13 @@ class MockPGPInterface: PGPInterface { return privateKeyIDs.contains { $0.hasSuffix(keyID.lowercased()) } } - var keyID: [String] { [] } // currently not relevant in these tests - var shortKeyID: [String] { [] } // currently not relevant in these tests + func getKeyIDs(type _: PGPKey) -> [String] { + // currently irrelevant for the tests + [] + } + + func getShortKeyIDs(type _: PGPKey) -> [String] { + // currently irrelevant for the tests + [] + } } From b103337083e19e1eded64de74a12731122a42977 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 16:25:17 +0100 Subject: [PATCH 14/21] move function to be closer to related one --- passKit/Crypto/PGPAgent.swift | 44 +++++++++++++++++------------------ 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index 9ce335d..be7f52d 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -53,6 +53,28 @@ public class PGPAgent { return pgpInterface?.getShortKeyIDs(type: type).sorted() ?? [] } + public func decrypt(encryptedData: Data, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? { + // Remember the previous status and set the current status + let previousDecryptStatus = latestDecryptStatus + latestDecryptStatus = false + // Init keys. + try checkAndInit() + // Get the PGP key passphrase. + let providePassPhraseForKey = { (selectedKeyID: String) -> String in + if previousDecryptStatus == false { + return requestPGPKeyPassphrase(selectedKeyID) + } + return self.keyStore.get(for: AppKeychain.getPGPKeyPassphraseKey(keyID: selectedKeyID)) ?? requestPGPKeyPassphrase(selectedKeyID) + } + // Decrypt. + guard let result = try pgpInterface!.decrypt(encryptedData: encryptedData, keyIDHint: nil, passPhraseForKey: providePassPhraseForKey) else { + return nil + } + // The decryption step has succeed. + latestDecryptStatus = true + return result + } + public func decrypt(encryptedData: Data, keyID: String, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? { // Init keys. try checkAndInit() @@ -100,28 +122,6 @@ public class PGPAgent { return try pgpInterface.encryptWithAllKeys(plainData: plainData) } - public func decrypt(encryptedData: Data, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? { - // Remember the previous status and set the current status - let previousDecryptStatus = latestDecryptStatus - latestDecryptStatus = false - // Init keys. - try checkAndInit() - // Get the PGP key passphrase. - let providePassPhraseForKey = { (selectedKeyID: String) -> String in - if previousDecryptStatus == false { - return requestPGPKeyPassphrase(selectedKeyID) - } - return self.keyStore.get(for: AppKeychain.getPGPKeyPassphraseKey(keyID: selectedKeyID)) ?? requestPGPKeyPassphrase(selectedKeyID) - } - // Decrypt. - guard let result = try pgpInterface!.decrypt(encryptedData: encryptedData, keyIDHint: nil, passPhraseForKey: providePassPhraseForKey) else { - return nil - } - // The decryption step has succeed. - latestDecryptStatus = true - return result - } - public var isPrepared: Bool { keyStore.contains(key: PGPKey.PUBLIC.getKeychainKey()) && keyStore.contains(key: PGPKey.PRIVATE.getKeychainKey()) From 054f333bacde06718571aa553760bc9cb3b23682 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 16:30:53 +0100 Subject: [PATCH 15/21] deduplicate decrypt logic in PGPAgent --- passKit/Crypto/PGPAgent.swift | 27 +--- .../LowLevel/PGPAgentLowLevelTests.swift | 130 ++---------------- 2 files changed, 10 insertions(+), 147 deletions(-) diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index be7f52d..d8e40f8 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -53,36 +53,13 @@ public class PGPAgent { return pgpInterface?.getShortKeyIDs(type: type).sorted() ?? [] } - public func decrypt(encryptedData: Data, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? { - // Remember the previous status and set the current status - let previousDecryptStatus = latestDecryptStatus - latestDecryptStatus = false - // Init keys. - try checkAndInit() - // Get the PGP key passphrase. - let providePassPhraseForKey = { (selectedKeyID: String) -> String in - if previousDecryptStatus == false { - return requestPGPKeyPassphrase(selectedKeyID) - } - return self.keyStore.get(for: AppKeychain.getPGPKeyPassphraseKey(keyID: selectedKeyID)) ?? requestPGPKeyPassphrase(selectedKeyID) - } - // Decrypt. - guard let result = try pgpInterface!.decrypt(encryptedData: encryptedData, keyIDHint: nil, passPhraseForKey: providePassPhraseForKey) else { - return nil - } - // The decryption step has succeed. - latestDecryptStatus = true - return result - } - - public func decrypt(encryptedData: Data, keyID: String, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? { - // Init keys. + public func decrypt(encryptedData: Data, keyID: String? = nil, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? { try checkAndInit() guard let pgpInterface else { throw AppError.decryption } - if !pgpInterface.containsPrivateKey(with: keyID) { + if let keyID, !pgpInterface.containsPrivateKey(with: keyID) { throw AppError.pgpPrivateKeyNotFound(keyID: keyID) } diff --git a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift index 8b03310..a1e6e99 100644 --- a/passKitTests/LowLevel/PGPAgentLowLevelTests.swift +++ b/passKitTests/LowLevel/PGPAgentLowLevelTests.swift @@ -125,7 +125,7 @@ final class PGPAgentLowLevelTests: XCTestCase { /// After a failed decrypt (latestDecryptStatus=false), requestPGPKeyPassphrase is ALWAYS called, /// even if the keystore has a cached passphrase. - func testDecryptWithKeyID_afterFailure_alwaysRequestsPassphrase() throws { + func testDecrypt_afterFailure_alwaysRequestsPassphrase() throws { let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [longFingerprint] keychain.add(string: "stored-passphrase", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint)) @@ -148,7 +148,7 @@ final class PGPAgentLowLevelTests: XCTestCase { } /// After a successful decrypt, the next call uses keystore first (latestDecryptStatus=true). - func testDecryptWithKeyID_afterSuccess_usesKeystoreFirst() throws { + func testDecrypt_afterSuccess_usesKeystoreFirst() throws { let shortID = "a1024dae" let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [longFingerprint] @@ -173,7 +173,7 @@ final class PGPAgentLowLevelTests: XCTestCase { // MARK: - decrypt(encryptedData:keyID:requestPGPKeyPassphrase:) - Return Values & Error Propagation /// When pgpInterface.decrypt returns nil, agent.decrypt returns nil. - func testDecryptWithKeyID_interfaceReturnsNil_returnsNil() throws { + func testDecrypt_interfaceReturnsNil_returnsNil() throws { let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [longFingerprint] mockPGP.decryptResult = nil @@ -185,7 +185,7 @@ final class PGPAgentLowLevelTests: XCTestCase { /// When pgpInterface.decrypt returns nil, latestDecryptStatus stays false /// (next call will always request passphrase). - func testDecryptWithKeyID_interfaceReturnsNil_statusStaysFalse() throws { + func testDecrypt_interfaceReturnsNil_statusStaysFalse() throws { let shortID = "d862027e" let longFingerprint = "787eae1a5fa3e749aa34cc6aa0645ebed862027e" mockPGP.privateKeyIDs = [longFingerprint] @@ -209,7 +209,7 @@ final class PGPAgentLowLevelTests: XCTestCase { } /// When pgpInterface.decrypt throws, the error propagates and latestDecryptStatus stays false. - func testDecryptWithKeyID_interfaceThrows_propagatesError() throws { + func testDecrypt_interfaceThrows_propagatesError() throws { let shortID = "a1024dae" let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [longFingerprint] @@ -238,7 +238,7 @@ final class PGPAgentLowLevelTests: XCTestCase { } /// After successful decrypt, latestDecryptStatus is true. - func testDecryptWithKeyID_success_setsStatusTrue() throws { + func testDecrypt_success_setsStatusTrue() throws { let longFingerprint = "4712286271220db299883ea7062e678da1024dae" mockPGP.privateKeyIDs = [longFingerprint] @@ -269,7 +269,7 @@ final class PGPAgentLowLevelTests: XCTestCase { /// checkAndInit re-initializes if pgpKeyPassphrase is missing from keystore. /// Since we're using a mock as pgpInterface, initKeys would overwrite it; verify the precondition holds. - func testDecryptWithKeyID_checkAndInit_requiresPGPKeyPassphraseInKeystore() throws { + func testDecrypt_checkAndInit_requiresPGPKeyPassphraseInKeystore() throws { // Remove the pgpKeyPassphrase sentinel, which will trigger checkAndInit -> initKeys. keychain.removeContent(for: Globals.pgpKeyPassphrase) // initKeys needs real PGP keys, which we don't have. It should throw keyImport. @@ -279,7 +279,7 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(passphraseRequests, [], "requestPGPKeyPassphrase should not be called when checkAndInit fails") } - // MARK: - decrypt(encryptedData:requestPGPKeyPassphrase:) - No KeyID Overload + // MARK: - decrypt(encryptedData:keyID:requestPGPKeyPassphrase:) - nil keyID /// The no-keyID overload passes nil as keyID to pgpInterface.decrypt func testDecryptNoKeyID_passesNilKeyIDToInterface() throws { @@ -290,70 +290,6 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertNil(mockPGP.decryptCalls[0].keyID) } - /// After failure, the no-keyID overload always requests passphrase. - func testDecryptNoKeyID_afterFailure_alwaysRequestsPassphrase() throws { - // Force a failure. - mockPGP.decryptError = AppError.wrongPassphrase - _ = try? agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("bad")) - - // Next one can succeed - keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) - mockPGP.decryptError = nil - mockPGP.decryptCalls.removeAll() - mockPGP.resolvedPassphrases.removeAll() - mockPGP.selectedKeyForPassphrase = "" - passphraseRequests.removeAll() - - _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("fresh")) - - XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh"]) - XCTAssertEqual(passphraseRequests, [""]) - } - - /// The no-keyID overload returns nil when interface returns nil. - func testDecryptNoKeyID_interfaceReturnsNil_returnsNil() throws { - mockPGP.decryptResult = nil - - let result = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass")) - - XCTAssertNil(result) - } - - /// The no-keyID overload propagates errors from the interface. - func testDecryptNoKeyID_interfaceThrows_propagatesError() { - mockPGP.decryptError = AppError.wrongPassphrase - - XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass"))) { error in - XCTAssertEqual(error as? AppError, AppError.wrongPassphrase) - } - } - - /// The no-keyID overload sets latestDecryptStatus to true on success, - /// which affects subsequent passphrase lookups. - func testDecryptNoKeyID_success_setsStatusTrue() throws { - // Force failure first. - mockPGP.decryptError = AppError.wrongPassphrase - _ = try? agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("bad")) - mockPGP.decryptError = nil - mockPGP.decryptCalls.removeAll() - passphraseRequests.removeAll() - - // Succeed. - _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("good")) - - // Third call: should try keystore. - keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) - mockPGP.decryptCalls.removeAll() - mockPGP.resolvedPassphrases.removeAll() - mockPGP.selectedKeyForPassphrase = "" - passphraseRequests.removeAll() - - _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("nope")) - - XCTAssertEqual(mockPGP.resolvedPassphrases, ["cached"]) - XCTAssertEqual(passphraseRequests, []) - } - /// The no-keyID overload doesn't check containsPrivateKey. func testDecryptNoKeyID_doesNotCheckPrivateKey() throws { _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass")) @@ -384,56 +320,6 @@ final class PGPAgentLowLevelTests: XCTestCase { XCTAssertEqual(mockPGP.resolvedPassphrases, ["cached-pass"]) } - // MARK: - Cross-overload latestDecryptStatus interaction - - /// latestDecryptStatus is shared between both overloads. - /// A failure in the keyID overload affects the no-keyID overload. - func testDecryptStatusSharedBetweenOverloads_failureInKeyIDOverload() throws { - let longFingerprint = "4712286271220db299883ea7062e678da1024dae" - mockPGP.privateKeyIDs = [longFingerprint] - - // Fail via keyID overload. - mockPGP.decryptError = AppError.wrongPassphrase - _ = try? agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("bad")) - - // Next call via no-keyID overload should always request. - keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: "")) - mockPGP.decryptError = nil - mockPGP.decryptCalls.removeAll() - mockPGP.resolvedPassphrases.removeAll() - mockPGP.selectedKeyForPassphrase = "" - passphraseRequests.removeAll() - - _ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("fresh")) - - XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh"]) - XCTAssertEqual(passphraseRequests, [""], "Failure in keyID overload should affect no-keyID overload") - } - - /// A failure in the no-keyID overload affects the keyID overload. - func testDecryptStatusSharedBetweenOverloads_failureInNoKeyIDOverload() throws { - let shortID = "a1024dae" - let longFingerprint = "4712286271220db299883ea7062e678da1024dae" - mockPGP.privateKeyIDs = [longFingerprint] - - // Fail via no-keyID overload. - mockPGP.decryptError = AppError.wrongPassphrase - _ = try? agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("bad")) - - // Next call via keyID overload should always request. - mockPGP.decryptError = nil - mockPGP.decryptCalls.removeAll() - mockPGP.resolvedPassphrases.removeAll() - mockPGP.selectedKeyForPassphrase = shortID - keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: shortID)) - passphraseRequests.removeAll() - - _ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("fresh")) - - XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh"]) - XCTAssertEqual(passphraseRequests, [shortID], "Failure in no-keyID overload should affect keyID overload") - } - // MARK: - Short vs long key ID behavior /// When caller passes a short ID and containsPrivateKey matches it (via suffix), the short ID From e32402b8079b8eba17e28b2f63b0804e49c133fc Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 22:56:21 +0100 Subject: [PATCH 16/21] streamline gpg-id handing * decrypt should not care about it at all * PasswordStore.decrypt always forwards the passed in keyID, even when gpg-id handling is disabled * PasswordStore.encrypt: streamlined, but should be same behavior --- passKit/Models/PasswordStore.swift | 32 +++++++++++++----------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/passKit/Models/PasswordStore.swift b/passKit/Models/PasswordStore.swift index 9a0e0f3..fdf321a 100644 --- a/passKit/Models/PasswordStore.swift +++ b/passKit/Models/PasswordStore.swift @@ -395,13 +395,7 @@ public class PasswordStore { public func decrypt(passwordEntity: PasswordEntity, keyID: String? = nil, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Password { let url = passwordEntity.fileURL(in: storeURL) let encryptedData = try Data(contentsOf: url) - let data: Data? = try { - if Defaults.isEnableGPGIDOn { - let keyID = keyID ?? findGPGID(from: url) - return try PGPAgent.shared.decrypt(encryptedData: encryptedData, keyID: keyID, requestPGPKeyPassphrase: requestPGPKeyPassphrase) - } - return try PGPAgent.shared.decrypt(encryptedData: encryptedData, requestPGPKeyPassphrase: requestPGPKeyPassphrase) - }() + let data: Data? = try PGPAgent.shared.decrypt(encryptedData: encryptedData, keyID: keyID, requestPGPKeyPassphrase: requestPGPKeyPassphrase) guard let decryptedData = data else { throw AppError.decryption } @@ -413,20 +407,22 @@ public class PasswordStore { guard let passwordEntity = fetchPasswordEntity(with: path) else { throw AppError.decryption } - if Defaults.isEnableGPGIDOn { - return try decrypt(passwordEntity: passwordEntity, keyID: keyID, requestPGPKeyPassphrase: requestPGPKeyPassphrase) - } - return try decrypt(passwordEntity: passwordEntity, requestPGPKeyPassphrase: requestPGPKeyPassphrase) + return try decrypt(passwordEntity: passwordEntity, keyID: keyID, requestPGPKeyPassphrase: requestPGPKeyPassphrase) } public func encrypt(password: Password, keyID: String? = nil) throws -> Data { - var keyID = keyID - if Defaults.isEnableGPGIDOn { - let encryptedDataPath = password.fileURL(in: storeURL) - keyID = keyID ?? findGPGID(from: encryptedDataPath) - } - if let keyID { - return try PGPAgent.shared.encrypt(plainData: password.plainData, keyIDs: [keyID]) + let keyIDs: [String] = { + if let keyID { + return [keyID] + } + if Defaults.isEnableGPGIDOn { + let encryptedDataPath = password.fileURL(in: storeURL) + return [findGPGID(from: encryptedDataPath)] + } + return [] + }() + if !keyIDs.isEmpty { + return try PGPAgent.shared.encrypt(plainData: password.plainData, keyIDs: keyIDs) } return try PGPAgent.shared.encryptWithAllKeys(plainData: password.plainData) } From 566b7253f5bb4db9cf573c11f38f0aad681ed25d Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 22:56:57 +0100 Subject: [PATCH 17/21] remove overload, caller can take care of it in 2 step process --- pass/Services/PasswordDecryptor.swift | 5 ++++- passKit/Models/PasswordStore.swift | 7 ------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/pass/Services/PasswordDecryptor.swift b/pass/Services/PasswordDecryptor.swift index 8824bb3..e18aaba 100644 --- a/pass/Services/PasswordDecryptor.swift +++ b/pass/Services/PasswordDecryptor.swift @@ -30,8 +30,11 @@ func decryptPassword( } DispatchQueue.global(qos: .userInteractive).async { do { + guard let passwordEntity = PasswordStore.shared.fetchPasswordEntity(with: passwordPath) else { + throw AppError.decryption + } let requestPGPKeyPassphrase = Utils.createRequestPGPKeyPassphraseHandler(controller: controller) - let decryptedPassword = try PasswordStore.shared.decrypt(path: passwordPath, keyID: keyID, requestPGPKeyPassphrase: requestPGPKeyPassphrase) + let decryptedPassword = try PasswordStore.shared.decrypt(passwordEntity: passwordEntity, keyID: keyID, requestPGPKeyPassphrase: requestPGPKeyPassphrase) DispatchQueue.main.async { completion(decryptedPassword) diff --git a/passKit/Models/PasswordStore.swift b/passKit/Models/PasswordStore.swift index fdf321a..f71b8a6 100644 --- a/passKit/Models/PasswordStore.swift +++ b/passKit/Models/PasswordStore.swift @@ -403,13 +403,6 @@ public class PasswordStore { return Password(name: passwordEntity.name, path: passwordEntity.path, plainText: plainText) } - public func decrypt(path: String, keyID: String? = nil, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Password { - guard let passwordEntity = fetchPasswordEntity(with: path) else { - throw AppError.decryption - } - return try decrypt(passwordEntity: passwordEntity, keyID: keyID, requestPGPKeyPassphrase: requestPGPKeyPassphrase) - } - public func encrypt(password: Password, keyID: String? = nil) throws -> Data { let keyIDs: [String] = { if let keyID { From 77f85ccdd1034ce48b3d9690e2f89335057d110b Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 22:57:51 +0100 Subject: [PATCH 18/21] support reading several key IDs in .gpg-id files --- passKit/Models/PasswordStore.swift | 10 +++++++--- .../23/44f8116ab49ad30dd96d92e306a2b28839ee71 | Bin 0 -> 175 bytes .../34/22e35e46f42b12fdca050c9bc424028d0e89c8 | Bin 0 -> 87 bytes .../8b/08bf485e19720eae26df62ddceefbb8b4d8247 | Bin 0 -> 897 bytes .../bb/1f455127743a2e202840fc39201937a2457ed2 | 1 + .../c7/c52ac6962d08d69e5651eedd6cbaf2f8bd05c3 | 3 +++ .../refs/heads/master | 1 + passKitTests/Models/PasswordStoreTest.swift | 13 +++++++------ 8 files changed, 19 insertions(+), 9 deletions(-) create mode 100644 passKitTests/Fixtures/password-store-with-gpgid.git/objects/23/44f8116ab49ad30dd96d92e306a2b28839ee71 create mode 100644 passKitTests/Fixtures/password-store-with-gpgid.git/objects/34/22e35e46f42b12fdca050c9bc424028d0e89c8 create mode 100644 passKitTests/Fixtures/password-store-with-gpgid.git/objects/8b/08bf485e19720eae26df62ddceefbb8b4d8247 create mode 100644 passKitTests/Fixtures/password-store-with-gpgid.git/objects/bb/1f455127743a2e202840fc39201937a2457ed2 create mode 100644 passKitTests/Fixtures/password-store-with-gpgid.git/objects/c7/c52ac6962d08d69e5651eedd6cbaf2f8bd05c3 create mode 100644 passKitTests/Fixtures/password-store-with-gpgid.git/refs/heads/master diff --git a/passKit/Models/PasswordStore.swift b/passKit/Models/PasswordStore.swift index f71b8a6..b6d01b1 100644 --- a/passKit/Models/PasswordStore.swift +++ b/passKit/Models/PasswordStore.swift @@ -410,7 +410,7 @@ public class PasswordStore { } if Defaults.isEnableGPGIDOn { let encryptedDataPath = password.fileURL(in: storeURL) - return [findGPGID(from: encryptedDataPath)] + return findGPGIDs(from: encryptedDataPath) } return [] }() @@ -461,7 +461,7 @@ extension PasswordStore { } } -func findGPGID(from url: URL) -> String { +func findGPGIDs(from url: URL) -> [String] { var path = url while !FileManager.default.fileExists(atPath: path.appendingPathComponent(".gpg-id").path), path.path != "file:///" { @@ -469,5 +469,9 @@ func findGPGID(from url: URL) -> String { } path = path.appendingPathComponent(".gpg-id") - return (try? String(contentsOf: path))?.trimmed ?? "" + let allKeysSeparatedByNewline = (try? String(contentsOf: path)) ?? "" + return allKeysSeparatedByNewline + .split(separator: "\n") + .map { String($0).trimmed } + .filter { !$0.isEmpty } } diff --git a/passKitTests/Fixtures/password-store-with-gpgid.git/objects/23/44f8116ab49ad30dd96d92e306a2b28839ee71 b/passKitTests/Fixtures/password-store-with-gpgid.git/objects/23/44f8116ab49ad30dd96d92e306a2b28839ee71 new file mode 100644 index 0000000000000000000000000000000000000000..d798c2c294d12b85ea7a3759cf4f4cc42e3232e3 GIT binary patch literal 175 zcmV;g08syU0V^p=O;s>7GiNX~FfcPQQP4{-NY~9wVF;MEM)0C}-pWn7_ih~&=68LQ z*ehsa00atYiMg3Ml?-x8=jI8R38ly#^Hnq0JnMr^giHZMK|yL!aeiK64#T`WPp%NY z85-IVyE~Z$k6D;aulWa2Qk;=kl$yd|qVzbFUn?6 dUZFU*LVVqsSv|i>wR^Y5+wXDK0|3aiKI4&zP~89k literal 0 HcmV?d00001 diff --git a/passKitTests/Fixtures/password-store-with-gpgid.git/objects/34/22e35e46f42b12fdca050c9bc424028d0e89c8 b/passKitTests/Fixtures/password-store-with-gpgid.git/objects/34/22e35e46f42b12fdca050c9bc424028d0e89c8 new file mode 100644 index 0000000000000000000000000000000000000000..69995d0a287cad2ca6f8ab7be65d971c47169976 GIT binary patch literal 87 zcmV-d0I2_X0V^p=O;s>AXD~D{Ff%bx&`U2!*Ud~}*e&lGs9s{Fr=a2R$5KJke35J2 tC8(10%#w`KB)#PPT##ahZjSvPags%R>(uTi-97hyceihoI{-x<9TKf%DR%$> literal 0 HcmV?d00001 diff --git a/passKitTests/Fixtures/password-store-with-gpgid.git/objects/8b/08bf485e19720eae26df62ddceefbb8b4d8247 b/passKitTests/Fixtures/password-store-with-gpgid.git/objects/8b/08bf485e19720eae26df62ddceefbb8b4d8247 new file mode 100644 index 0000000000000000000000000000000000000000..ea752fb59f479221ee814ac65c743cb495168ece GIT binary patch literal 897 zcmV-{1AhE?0ReUciTq-0Z(<-gH#Y!<0Sp7X@s5I6Xj^>&2mrSx8ci2rXT~yI0?r=U zB7^)Ywla$k7VwNAk;QJ2>1!cRe3`dbQ77lFfqxzC3>hg9*hU;h6zB3I331U=q4{9A z3_i`XHsP61g@dgTV2XPMCbf+?cJQqY$Q1(N&t7=Yr(=GDQ?KweP$NXTlv5kx_TYhw zQ6Kr`NMTMIaZ2v=K!8Qs6sBuY^(GSx%f2!T+apH|xC)x_0HUc_IB;wYd!x6W?M6mt z>2h;~ygQgIFRNQOSH|-6(Ar|9C1gWuZh2*-*4udsiR+LMcZ`u_YrJ+NlSw_q%fN)) zqa^;Mr8`-Zyan=SE)B&bYfO7D97D-E*}mAUHpI|#;R1yM3p(+pUf+*1kQdG=rdA&*DV&};_tyO zFKXQMK4?WynL^WzE`Haj121xS-4>dW-%|U{)+42w-uL;try7TXr=gjCZc{E3PW#~-B zAxk8Kuz=R{=*8Z6mxS>bz0Af#xb<+=2{7pE`H)>H&_u5j-M0}x_i#BGbRrmxJ*p}f zBH5($1FGc=ut-hFD9ueHxev=(2NPIG`*yFo_@9!(j=#_r2ldmLDo;+ve~2k;AvU+nOZU3rTPI~e#~n z+ntE1`D=z1t{)^cW5k+xeg64Ffp=pDf!)&V@I zwJjJBV~d@)5So_=at(N#Xxs5NM|z2fI6e+M7PnJNi|P}n%G8637cl%kN9rNRN;3IbgM&o`FH4tVZqpuh-|Tq@e#H+ literal 0 HcmV?d00001 diff --git a/passKitTests/Fixtures/password-store-with-gpgid.git/objects/bb/1f455127743a2e202840fc39201937a2457ed2 b/passKitTests/Fixtures/password-store-with-gpgid.git/objects/bb/1f455127743a2e202840fc39201937a2457ed2 new file mode 100644 index 0000000..61d0371 --- /dev/null +++ b/passKitTests/Fixtures/password-store-with-gpgid.git/objects/bb/1f455127743a2e202840fc39201937a2457ed2 @@ -0,0 +1 @@ +x1 @k^BI(9Bj]lEUMC{lD+=rKd8.4uy: n痈Ky1F \ No newline at end of file diff --git a/passKitTests/Fixtures/password-store-with-gpgid.git/objects/c7/c52ac6962d08d69e5651eedd6cbaf2f8bd05c3 b/passKitTests/Fixtures/password-store-with-gpgid.git/objects/c7/c52ac6962d08d69e5651eedd6cbaf2f8bd05c3 new file mode 100644 index 0000000..89b191d --- /dev/null +++ b/passKitTests/Fixtures/password-store-with-gpgid.git/objects/c7/c52ac6962d08d69e5651eedd6cbaf2f8bd05c3 @@ -0,0 +1,3 @@ +xAj0s+bF#Y < |@m]Hry}B>C_ +~O S+"@yڅh}`ޱ'1EGEP$7KEiyh8f +G[g 9g {;^oH'a0d{D ϨlUKS5T B-BfH’z9RS9v qkcJ \ No newline at end of file diff --git a/passKitTests/Fixtures/password-store-with-gpgid.git/refs/heads/master b/passKitTests/Fixtures/password-store-with-gpgid.git/refs/heads/master new file mode 100644 index 0000000..2b6288a --- /dev/null +++ b/passKitTests/Fixtures/password-store-with-gpgid.git/refs/heads/master @@ -0,0 +1 @@ +c7c52ac6962d08d69e5651eedd6cbaf2f8bd05c3 diff --git a/passKitTests/Models/PasswordStoreTest.swift b/passKitTests/Models/PasswordStoreTest.swift index 4fe2c80..bb78bbf 100644 --- a/passKitTests/Models/PasswordStoreTest.swift +++ b/passKitTests/Models/PasswordStoreTest.swift @@ -32,7 +32,7 @@ final class PasswordStoreTest: XCTestCase { try cloneRepository(.withGPGID) XCTAssertEqual(passwordStore.numberOfPasswords, 4) - XCTAssertEqual(passwordStore.numberOfCommits, 16) + XCTAssertEqual(passwordStore.numberOfCommits, 17) XCTAssertEqual(passwordStore.numberOfLocalCommits, 0) let entity = passwordStore.fetchPasswordEntity(with: "personal/github.com.gpg") @@ -319,11 +319,12 @@ final class PasswordStoreTest: XCTestCase { Defaults.isEnableGPGIDOn = true [ - ("work/github.com", "4712286271220DB299883EA7062E678DA1024DAE"), - ("personal/github.com", "787EAE1A5FA3E749AA34CC6AA0645EBED862027E"), - ].forEach { path, id in - let keyID = findGPGID(from: localRepoURL.appendingPathComponent(path)) - XCTAssertEqual(keyID, id) + ("work/github.com", ["4712286271220DB299883EA7062E678DA1024DAE"]), + ("personal/github.com", ["787EAE1A5FA3E749AA34CC6AA0645EBED862027E"]), + ("shared/github.com", ["4712286271220DB299883EA7062E678DA1024DAE", "787EAE1A5FA3E749AA34CC6AA0645EBED862027E"]), + ].forEach { path, keyIDs in + let foundKeyIDs = findGPGIDs(from: localRepoURL.appendingPathComponent(path)) + XCTAssertEqual(foundKeyIDs, keyIDs) } let personal = try decrypt(path: "personal/github.com.gpg") From e3de11f71cc0c18a0deb1a5b1f4118e8a7f7fee6 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Wed, 11 Mar 2026 23:25:46 +0100 Subject: [PATCH 19/21] PasswordStore: allow to pass in a specific PGPAgent implementation (for testing) --- passKit/Crypto/PGPAgent.swift | 2 +- passKit/Models/PasswordStore.swift | 14 ++++++++------ passKitTests/Models/PasswordStoreTest.swift | 9 ++++++--- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index d8e40f8..6ee8c2b 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -9,7 +9,7 @@ public class PGPAgent { public static let shared = PGPAgent() - private let keyStore: KeyStore + let keyStore: KeyStore private var pgpInterface: PGPInterface? private var latestDecryptStatus = true diff --git a/passKit/Models/PasswordStore.swift b/passKit/Models/PasswordStore.swift index b6d01b1..742f13e 100644 --- a/passKit/Models/PasswordStore.swift +++ b/passKit/Models/PasswordStore.swift @@ -23,6 +23,7 @@ public class PasswordStore { }() public var storeURL: URL + private let pgpAgent: PGPAgent public var gitRepository: GitRepository? @@ -84,8 +85,9 @@ public class PasswordStore { gitRepository?.numberOfCommits() } - init(url: URL = Globals.repositoryURL) { + init(url: URL = Globals.repositoryURL, pgpAgent: PGPAgent = .shared) { self.storeURL = url + self.pgpAgent = pgpAgent // Migration importExistingKeysIntoKeychain() @@ -359,14 +361,14 @@ public class PasswordStore { eraseStoreData() // Delete PGP key, SSH key and other secrets from the keychain. - AppKeychain.shared.removeAllContent() + pgpAgent.keyStore.removeAllContent() // Delete default settings. Defaults.removeAll() // Delete cache explicitly. PasscodeLock.shared.delete() - PGPAgent.shared.uninitKeys() + pgpAgent.uninitKeys() } // return the number of discarded commits @@ -395,7 +397,7 @@ public class PasswordStore { public func decrypt(passwordEntity: PasswordEntity, keyID: String? = nil, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Password { let url = passwordEntity.fileURL(in: storeURL) let encryptedData = try Data(contentsOf: url) - let data: Data? = try PGPAgent.shared.decrypt(encryptedData: encryptedData, keyID: keyID, requestPGPKeyPassphrase: requestPGPKeyPassphrase) + let data: Data? = try pgpAgent.decrypt(encryptedData: encryptedData, keyID: keyID, requestPGPKeyPassphrase: requestPGPKeyPassphrase) guard let decryptedData = data else { throw AppError.decryption } @@ -415,9 +417,9 @@ public class PasswordStore { return [] }() if !keyIDs.isEmpty { - return try PGPAgent.shared.encrypt(plainData: password.plainData, keyIDs: keyIDs) + return try pgpAgent.encrypt(plainData: password.plainData, keyIDs: keyIDs) } - return try PGPAgent.shared.encryptWithAllKeys(plainData: password.plainData) + return try pgpAgent.encryptWithAllKeys(plainData: password.plainData) } public func removeGitSSHKeys() { diff --git a/passKitTests/Models/PasswordStoreTest.swift b/passKitTests/Models/PasswordStoreTest.swift index bb78bbf..accd779 100644 --- a/passKitTests/Models/PasswordStoreTest.swift +++ b/passKitTests/Models/PasswordStoreTest.swift @@ -15,15 +15,18 @@ import XCTest final class PasswordStoreTest: XCTestCase { private let localRepoURL: URL = Globals.sharedContainerURL.appendingPathComponent("Library/password-store-test/") + private var pgpAgent: PGPAgent! = nil private var passwordStore: PasswordStore! = nil override func setUp() { - passwordStore = PasswordStore(url: localRepoURL) + pgpAgent = PGPAgent() + passwordStore = PasswordStore(url: localRepoURL, pgpAgent: pgpAgent) } override func tearDown() { passwordStore.erase() passwordStore = nil + pgpAgent = nil Defaults.removeAll() } @@ -78,7 +81,7 @@ final class PasswordStoreTest: XCTestCase { XCTAssertTrue(AppKeychain.shared.contains(key: PGPKey.PUBLIC.getKeychainKey())) XCTAssertEqual(Defaults.gitSignatureName, "Test User") XCTAssertTrue(PasscodeLock.shared.hasPasscode) - XCTAssertTrue(PGPAgent.shared.isInitialized()) + XCTAssertTrue(pgpAgent.isInitialized()) expectation(forNotification: .passwordStoreUpdated, object: nil) expectation(forNotification: .passwordStoreErased, object: nil) @@ -88,7 +91,7 @@ final class PasswordStoreTest: XCTestCase { XCTAssertFalse(AppKeychain.shared.contains(key: PGPKey.PUBLIC.getKeychainKey())) XCTAssertFalse(Defaults.hasKey(\.gitSignatureName)) XCTAssertFalse(PasscodeLock.shared.hasPasscode) - XCTAssertFalse(PGPAgent.shared.isInitialized()) + XCTAssertFalse(pgpAgent.isInitialized()) waitForExpectations(timeout: 1, handler: nil) } From 38649f96fe77dec05b7a756f3bda9542938eb13a Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Thu, 12 Mar 2026 09:30:54 +0100 Subject: [PATCH 20/21] mocking prep: PasswordStoreTest has its own KeyStore --- passKitTests/Models/PasswordStoreTest.swift | 27 ++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/passKitTests/Models/PasswordStoreTest.swift b/passKitTests/Models/PasswordStoreTest.swift index accd779..e02039e 100644 --- a/passKitTests/Models/PasswordStoreTest.swift +++ b/passKitTests/Models/PasswordStoreTest.swift @@ -15,11 +15,15 @@ import XCTest final class PasswordStoreTest: XCTestCase { private let localRepoURL: URL = Globals.sharedContainerURL.appendingPathComponent("Library/password-store-test/") + private var keyStore: KeyStore! = nil private var pgpAgent: PGPAgent! = nil private var passwordStore: PasswordStore! = nil override func setUp() { - pgpAgent = PGPAgent() + super.setUp() + + keyStore = DictBasedKeychain() + pgpAgent = PGPAgent(keyStore: keyStore) passwordStore = PasswordStore(url: localRepoURL, pgpAgent: pgpAgent) } @@ -27,8 +31,11 @@ final class PasswordStoreTest: XCTestCase { passwordStore.erase() passwordStore = nil pgpAgent = nil + keyStore = nil Defaults.removeAll() + + super.tearDown() } func testInitPasswordEntityCoreData() throws { @@ -78,7 +85,7 @@ final class PasswordStoreTest: XCTestCase { PasscodeLock.shared.save(passcode: "1234") XCTAssertGreaterThan(passwordStore.numberOfPasswords, 0) - XCTAssertTrue(AppKeychain.shared.contains(key: PGPKey.PUBLIC.getKeychainKey())) + XCTAssertTrue(keyStore.contains(key: PGPKey.PUBLIC.getKeychainKey())) XCTAssertEqual(Defaults.gitSignatureName, "Test User") XCTAssertTrue(PasscodeLock.shared.hasPasscode) XCTAssertTrue(pgpAgent.isInitialized()) @@ -88,7 +95,7 @@ final class PasswordStoreTest: XCTestCase { passwordStore.erase() XCTAssertEqual(passwordStore.numberOfPasswords, 0) - XCTAssertFalse(AppKeychain.shared.contains(key: PGPKey.PUBLIC.getKeychainKey())) + XCTAssertFalse(keyStore.contains(key: PGPKey.PUBLIC.getKeychainKey())) XCTAssertFalse(Defaults.hasKey(\.gitSignatureName)) XCTAssertFalse(PasscodeLock.shared.hasPasscode) XCTAssertFalse(pgpAgent.isInitialized()) @@ -382,17 +389,15 @@ final class PasswordStoreTest: XCTestCase { } private func importSinglePGPKey() throws { - let keychain = AppKeychain.shared - try KeyFileManager(keyType: PGPKey.PUBLIC, keyPath: "", keyHandler: keychain.add).importKey(from: RSA4096.publicKey) - try KeyFileManager(keyType: PGPKey.PRIVATE, keyPath: "", keyHandler: keychain.add).importKey(from: RSA4096.privateKey) - try PGPAgent.shared.initKeys() + try KeyFileManager(keyType: PGPKey.PUBLIC, keyPath: "", keyHandler: keyStore.add).importKey(from: RSA4096.publicKey) + try KeyFileManager(keyType: PGPKey.PRIVATE, keyPath: "", keyHandler: keyStore.add).importKey(from: RSA4096.privateKey) + try pgpAgent.initKeys() } private func importMultiplePGPKeys() throws { - let keychain = AppKeychain.shared - try KeyFileManager(keyType: PGPKey.PUBLIC, keyPath: "", keyHandler: keychain.add).importKey(from: RSA2048_RSA4096.publicKeys) - try KeyFileManager(keyType: PGPKey.PRIVATE, keyPath: "", keyHandler: keychain.add).importKey(from: RSA2048_RSA4096.privateKeys) - try PGPAgent.shared.initKeys() + try KeyFileManager(keyType: PGPKey.PUBLIC, keyPath: "", keyHandler: keyStore.add).importKey(from: RSA2048_RSA4096.publicKeys) + try KeyFileManager(keyType: PGPKey.PRIVATE, keyPath: "", keyHandler: keyStore.add).importKey(from: RSA2048_RSA4096.privateKeys) + try pgpAgent.initKeys() } private func decrypt(path: String, keyID: String? = nil) throws -> Password { From 5c416bfb21995de15004715bbb0c3c0195e95774 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Thu, 12 Mar 2026 09:31:37 +0100 Subject: [PATCH 21/21] test .gpg-id support mostly using mocks --- passKitTests/Crypto/PGPAgentTest.swift | 4 +- passKitTests/Models/PasswordStoreTest.swift | 119 +++++++++++++++++--- 2 files changed, 105 insertions(+), 18 deletions(-) diff --git a/passKitTests/Crypto/PGPAgentTest.swift b/passKitTests/Crypto/PGPAgentTest.swift index 531ecc1..267742c 100644 --- a/passKitTests/Crypto/PGPAgentTest.swift +++ b/passKitTests/Crypto/PGPAgentTest.swift @@ -184,8 +184,8 @@ final class PGPAgentTest: XCTestCase { try importKeys(RSA2048_RSA4096.publicKeys | ED25519.publicKey, RSA2048_RSA4096.privateKeys) try pgpAgent.initKeys() - XCTAssertEqual(try pgpAgent.getKeyIDs(type: .PUBLIC).map(\.localizedLowercase).sorted(), (RSA2048_RSA4096.longFingerprints + [ED25519.longFingerprint]).sorted()) - XCTAssertEqual(try pgpAgent.getKeyIDs(type: .PRIVATE).map(\.localizedLowercase).sorted(), RSA2048_RSA4096.longFingerprints.sorted()) + XCTAssertEqual(try pgpAgent.getKeyIDs(type: .PUBLIC).map { $0.lowercased() }.sorted(), (RSA2048_RSA4096.longFingerprints + [ED25519.longFingerprint]).sorted()) + XCTAssertEqual(try pgpAgent.getKeyIDs(type: .PRIVATE).map { $0.lowercased() }.sorted(), RSA2048_RSA4096.longFingerprints.sorted()) let encryptedData = try pgpAgent.encrypt(plainData: testData, keyIDs: RSA2048_RSA4096.fingerprints + [ED25519.fingerprint]) diff --git a/passKitTests/Models/PasswordStoreTest.swift b/passKitTests/Models/PasswordStoreTest.swift index e02039e..e9b3c44 100644 --- a/passKitTests/Models/PasswordStoreTest.swift +++ b/passKitTests/Models/PasswordStoreTest.swift @@ -27,6 +27,18 @@ final class PasswordStoreTest: XCTestCase { passwordStore = PasswordStore(url: localRepoURL, pgpAgent: pgpAgent) } + private func setUpMockedPGPInterface() -> MockPGPInterface { + let mockPGPInterface = MockPGPInterface() + keyStore = DictBasedKeychain() + pgpAgent = PGPAgent(keyStore: keyStore, pgpInterface: mockPGPInterface) + passwordStore = PasswordStore(url: localRepoURL, pgpAgent: pgpAgent) + + // Set pgpKeyPassphrase key so checkAndInit() doesn't re-init and overwrite our mock. + keyStore.add(string: "dummy", for: Globals.pgpKeyPassphrase) + + return mockPGPInterface + } + override func tearDown() { passwordStore.erase() passwordStore = nil @@ -322,31 +334,106 @@ final class PasswordStoreTest: XCTestCase { // MARK: - .gpg-id support - func testCloneAndDecryptMultiKeys() throws { + func testReadGPGIDFile() throws { try cloneRepository(.withGPGID) - try importMultiplePGPKeys() - Defaults.isEnableGPGIDOn = true [ - ("work/github.com", ["4712286271220DB299883EA7062E678DA1024DAE"]), - ("personal/github.com", ["787EAE1A5FA3E749AA34CC6AA0645EBED862027E"]), - ("shared/github.com", ["4712286271220DB299883EA7062E678DA1024DAE", "787EAE1A5FA3E749AA34CC6AA0645EBED862027E"]), - ].forEach { path, keyIDs in + ("", [RSA4096.longFingerprint]), + ("family", [String(NISTP384.longFingerprint.suffix(16))]), + ("personal", [RSA4096.longFingerprint]), + ("shared", [RSA2048.longFingerprint, RSA4096.longFingerprint]), + ("work", [RSA2048.longFingerprint]), + ].forEach { path, expectedKeyIDs in let foundKeyIDs = findGPGIDs(from: localRepoURL.appendingPathComponent(path)) - XCTAssertEqual(foundKeyIDs, keyIDs) + XCTAssertEqual(foundKeyIDs, expectedKeyIDs.map { $0.uppercased() }) } + } - let personal = try decrypt(path: "personal/github.com.gpg") - XCTAssertEqual(personal.plainText, "passwordforpersonal\n") - - let work = try decrypt(path: "work/github.com.gpg") - XCTAssertEqual(work.plainText, "passwordforwork\n") + func testAddPasswordInRoot_WithSingleEntryInPGPIDFile_EncryptsWithThatKey() throws { + let mockPGPInterface = setUpMockedPGPInterface() + mockPGPInterface.publicKeyIDs = Set(RSA2048_RSA4096.fingerprints) + try cloneRepository(.withGPGID) + Defaults.isEnableGPGIDOn = true let testPassword = Password(name: "test", path: "test.gpg", plainText: "testpassword") - let testPasswordEntity = try passwordStore.add(password: testPassword)! - let testPasswordPlain = try passwordStore.decrypt(passwordEntity: testPasswordEntity, requestPGPKeyPassphrase: requestPGPKeyPassphrase) - XCTAssertEqual(testPasswordPlain.plainText, "testpassword") + _ = try passwordStore.add(password: testPassword) + + XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls.count, 1) + let encryptCall = mockPGPInterface.encryptMultiKeyCalls.first + XCTAssertEqual(encryptCall?.plainData, testPassword.plainData) + XCTAssertEqual(encryptCall?.keyIDs, [RSA4096.longFingerprint].map { $0.uppercased() }) + } + + func testEncryptWithSingleKeyViaGPGIDFileInSubDirectory() throws { + let mockPGPInterface = setUpMockedPGPInterface() + mockPGPInterface.publicKeyIDs = Set(RSA2048_RSA4096.fingerprints) + try cloneRepository(.withGPGID) + Defaults.isEnableGPGIDOn = true + + let testPassword = Password(name: "test", path: "family/test.gpg", plainText: "testpassword") + _ = try passwordStore.add(password: testPassword) + + XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls.count, 1) + let encryptCall = mockPGPInterface.encryptMultiKeyCalls.first + XCTAssertEqual(encryptCall?.plainData, testPassword.plainData) + XCTAssertEqual(encryptCall?.keyIDs, [String(NISTP384.longFingerprint.suffix(16))].map { $0.uppercased() }) + } + + func testEncryptWithSingleKeyViaGPGIDFileInParentDir() throws { + let mockPGPInterface = setUpMockedPGPInterface() + mockPGPInterface.publicKeyIDs = Set(RSA2048_RSA4096.fingerprints) + try cloneRepository(.withGPGID) + Defaults.isEnableGPGIDOn = true + + // /personal doesn't have its own .gpg-id file, but should inherit from the root .gpg-id file + let testPassword = Password(name: "test", path: "personal/test.gpg", plainText: "testpassword") + _ = try passwordStore.add(password: testPassword) + + XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls.count, 1) + let encryptCall = mockPGPInterface.encryptMultiKeyCalls.first + XCTAssertEqual(encryptCall?.plainData, testPassword.plainData) + XCTAssertEqual(encryptCall?.keyIDs, [RSA4096.longFingerprint].map { $0.uppercased() }) + } + + func testEncryptWithMultipleKeysViaGPGIDFile() throws { + let mockPGPInterface = setUpMockedPGPInterface() + mockPGPInterface.publicKeyIDs = Set(RSA2048_RSA4096.fingerprints) + try cloneRepository(.withGPGID) + Defaults.isEnableGPGIDOn = true + + // /shared uses both RSA2048 and RSA4096 + let testPassword = Password(name: "test", path: "shared/test.gpg", plainText: "testpassword") + _ = try passwordStore.add(password: testPassword) + + XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls.count, 1) + let encryptCall = mockPGPInterface.encryptMultiKeyCalls.first + XCTAssertEqual(encryptCall?.plainData, testPassword.plainData) + XCTAssertEqual(encryptCall?.keyIDs, RSA2048_RSA4096.longFingerprints.map { $0.uppercased() }) + } + + func testEncryptWithSingleKeyViaGPGFile_MissingKey() throws { + try cloneRepository(.withGPGID) + try importSinglePGPKey() // Only import RSA4096, but not RSA2048 + Defaults.isEnableGPGIDOn = true + + // /work uses RSA2048, but we didn't import that one + let testPassword = Password(name: "test", path: "work/test.gpg", plainText: "testpassword") + XCTAssertThrowsError(try passwordStore.add(password: testPassword)) { + XCTAssertEqual($0 as? AppError, .pgpPublicKeyNotFound(keyID: RSA2048.longFingerprint.uppercased())) + } + } + + func testEncryptWithMultipleKeysViaGPGFile_MissingKey() throws { + try cloneRepository(.withGPGID) + try importSinglePGPKey() // Only import RSA4096, but not RSA2048 + Defaults.isEnableGPGIDOn = true + + // /shared uses both RSA2048 and RSA4096, but we only imported RSA4096, so encryption should fail since one of the keys is missing + let testPassword = Password(name: "test", path: "shared/test.gpg", plainText: "testpassword") + XCTAssertThrowsError(try passwordStore.add(password: testPassword)) { + XCTAssertEqual($0 as? AppError, .pgpPublicKeyNotFound(keyID: RSA2048.longFingerprint.uppercased())) + } } // MARK: - Helpers