From e9c5b63c4b8cb7e7a78ce4b5a7ee598f673712a3 Mon Sep 17 00:00:00 2001 From: Mingshen Sun Date: Mon, 13 Apr 2020 15:16:03 -0700 Subject: [PATCH] Refactor logic of request PGP key passphrase --- .../PasswordDetailTableViewController.swift | 25 ++-------------- .../Controllers/PasswordsViewController.swift | 29 +++---------------- .../CredentialProviderViewController.swift | 25 ++-------------- .../Controllers/ExtensionViewController.swift | 25 ++-------------- passKit/Crypto/PGPAgent.swift | 6 ++-- passKit/Helpers/Utils.swift | 24 +++++++++++++++ passKit/Models/PasswordStore.swift | 2 +- passKitTests/Crypto/PGPAgentTest.swift | 6 ++-- passKitTests/Models/PasswordStoreTest.swift | 4 +-- passKitTests/Testbase/TestPGPKeys.swift | 2 +- 10 files changed, 44 insertions(+), 104 deletions(-) diff --git a/pass/Controllers/PasswordDetailTableViewController.swift b/pass/Controllers/PasswordDetailTableViewController.swift index be1b89e..95c8083 100644 --- a/pass/Controllers/PasswordDetailTableViewController.swift +++ b/pass/Controllers/PasswordDetailTableViewController.swift @@ -87,28 +87,6 @@ class PasswordDetailTableViewController: UITableViewController, UIGestureRecogni } } - private func requestPGPKeyPassphrase() -> String { - let sem = DispatchSemaphore(value: 0) - var passphrase = "" - DispatchQueue.main.async { - let alert = UIAlertController(title: "Passphrase".localize(), message: "FillInPgpPassphrase.".localize(), preferredStyle: UIAlertController.Style.alert) - alert.addAction(UIAlertAction(title: "Ok".localize(), style: UIAlertAction.Style.default, handler: {_ in - passphrase = alert.textFields!.first!.text! - sem.signal() - })) - alert.addTextField(configurationHandler: {(textField: UITextField!) in - textField.text = self.keychain.get(for: Globals.pgpKeyPassphrase) ?? "" - textField.isSecureTextEntry = true - }) - self.present(alert, animated: true, completion: nil) - } - let _ = sem.wait(timeout: DispatchTime.distantFuture) - if Defaults.isRememberPGPPassphraseOn { - self.keychain.add(string: passphrase, for: Globals.pgpKeyPassphrase) - } - return passphrase - } - @objc private func decryptThenShowPassword() { guard let passwordEntity = passwordEntity else { Utils.alert(title: "CannotShowPassword".localize(), message: "PasswordDoesNotExist".localize(), controller: self, handler: {(UIAlertAction) -> Void in @@ -119,7 +97,8 @@ class PasswordDetailTableViewController: UITableViewController, UIGestureRecogni DispatchQueue.global(qos: .userInitiated).async { // decrypt password do { - self.password = try self.passwordStore.decrypt(passwordEntity: passwordEntity, requestPGPKeyPassphrase: self.requestPGPKeyPassphrase) + let requestPGPKeyPassphrase = Utils.createRequestPGPKeyPassphraseHandler(controller: self) + self.password = try self.passwordStore.decrypt(passwordEntity: passwordEntity, requestPGPKeyPassphrase: requestPGPKeyPassphrase) } catch { DispatchQueue.main.async { // alert: cancel or try again diff --git a/pass/Controllers/PasswordsViewController.swift b/pass/Controllers/PasswordsViewController.swift index 4492c95..7f9027c 100644 --- a/pass/Controllers/PasswordsViewController.swift +++ b/pass/Controllers/PasswordsViewController.swift @@ -433,28 +433,6 @@ class PasswordsViewController: UIViewController, UITableViewDataSource, UITableV decryptThenCopyPassword(from: indexPath) } - private func requestPGPKeyPassphrase() -> String { - let sem = DispatchSemaphore(value: 0) - var passphrase = "" - DispatchQueue.main.async { - let alert = UIAlertController(title: "Passphrase".localize(), message: "FillInPgpPassphrase.".localize(), preferredStyle: UIAlertController.Style.alert) - alert.addAction(UIAlertAction(title: "Ok".localize(), style: UIAlertAction.Style.default, handler: {_ in - passphrase = alert.textFields!.first!.text! - sem.signal() - })) - alert.addTextField(configurationHandler: {(textField: UITextField!) in - textField.text = self.keychain.get(for: Globals.pgpKeyPassphrase) ?? "" - textField.isSecureTextEntry = true - }) - self.present(alert, animated: true, completion: nil) - } - let _ = sem.wait(timeout: DispatchTime.distantFuture) - if Defaults.isRememberPGPPassphraseOn { - self.keychain.add(string: passphrase, for: Globals.pgpKeyPassphrase) - } - return passphrase - } - private func decryptThenCopyPassword(from indexPath: IndexPath) { guard PGPAgent.shared.isPrepared else { Utils.alert(title: "CannotCopyPassword".localize(), message: "PgpKeyNotSet.".localize(), controller: self, completion: nil) @@ -466,7 +444,8 @@ class PasswordsViewController: UIViewController, UITableViewDataSource, UITableV DispatchQueue.global(qos: .userInteractive).async { var decryptedPassword: Password? do { - decryptedPassword = try self.passwordStore.decrypt(passwordEntity: passwordEntity, requestPGPKeyPassphrase: self.requestPGPKeyPassphrase) + let requestPGPKeyPassphrase = Utils.createRequestPGPKeyPassphraseHandler(controller: self) + decryptedPassword = try self.passwordStore.decrypt(passwordEntity: passwordEntity, requestPGPKeyPassphrase: requestPGPKeyPassphrase) DispatchQueue.main.async { SecurePasteboard.shared.copy(textToCopy: decryptedPassword?.password) SVProgressHUD.setDefaultMaskType(.black) @@ -617,10 +596,10 @@ class PasswordsViewController: UIViewController, UITableViewDataSource, UITableV } @objc func actOnReloadTableViewRelatedNotification() { - // Reset selectedScopeButtonIndex to make sure the correct reloadTableView - searchController.searchBar.selectedScopeButtonIndex = 0 DispatchQueue.main.async { [weak weakSelf = self] in guard let strongSelf = weakSelf else { return } + // Reset selectedScopeButtonIndex to make sure the correct reloadTableView + strongSelf.searchController.searchBar.selectedScopeButtonIndex = 0 strongSelf.initPasswordsTableEntries(parent: nil) strongSelf.reloadTableView(data: strongSelf.passwordsTableEntries) } diff --git a/passAutoFillExtension/Controllers/CredentialProviderViewController.swift b/passAutoFillExtension/Controllers/CredentialProviderViewController.swift index f447f81..655ccc0 100644 --- a/passAutoFillExtension/Controllers/CredentialProviderViewController.swift +++ b/passAutoFillExtension/Controllers/CredentialProviderViewController.swift @@ -142,7 +142,8 @@ class CredentialProviderViewController: ASCredentialProviderViewController, UITa DispatchQueue.global(qos: .userInteractive).async { var decryptedPassword: Password? do { - decryptedPassword = try self.passwordStore.decrypt(passwordEntity: passwordEntity, requestPGPKeyPassphrase: self.requestPGPKeyPassphrase) + let requestPGPKeyPassphrase = Utils.createRequestPGPKeyPassphraseHandler(controller: self) + decryptedPassword = try self.passwordStore.decrypt(passwordEntity: passwordEntity, requestPGPKeyPassphrase: requestPGPKeyPassphrase) let username = decryptedPassword?.username ?? decryptedPassword?.login ?? "" let password = decryptedPassword?.password ?? "" DispatchQueue.main.async {// prepare a dictionary to return @@ -168,28 +169,6 @@ class CredentialProviderViewController: ASCredentialProviderViewController, UITa return passwordsTableEntries.count; } - private func requestPGPKeyPassphrase() -> String { - let sem = DispatchSemaphore(value: 0) - var passphrase = "" - DispatchQueue.main.async { - let alert = UIAlertController(title: "Passphrase".localize(), message: "FillInPgpPassphrase.".localize(), preferredStyle: UIAlertController.Style.alert) - alert.addAction(UIAlertAction(title: "Ok".localize(), style: UIAlertAction.Style.default, handler: {_ in - passphrase = alert.textFields!.first!.text! - sem.signal() - })) - alert.addTextField(configurationHandler: {(textField: UITextField!) in - textField.text = self.keychain.get(for: Globals.pgpKeyPassphrase) ?? "" - textField.isSecureTextEntry = true - }) - self.present(alert, animated: true, completion: nil) - } - let _ = sem.wait(timeout: DispatchTime.distantFuture) - if Defaults.isRememberPGPPassphraseOn { - self.keychain.add(string: passphrase, for: Globals.pgpKeyPassphrase) - } - return passphrase - } - func searchBarCancelButtonClicked(_ searchBar: UISearchBar) { searchBar.text = "" searchActive = false diff --git a/passExtension/Controllers/ExtensionViewController.swift b/passExtension/Controllers/ExtensionViewController.swift index 0c17d62..9ffe6c9 100644 --- a/passExtension/Controllers/ExtensionViewController.swift +++ b/passExtension/Controllers/ExtensionViewController.swift @@ -150,7 +150,8 @@ class ExtensionViewController: UIViewController, UITableViewDataSource, UITableV DispatchQueue.global(qos: .userInteractive).async { var decryptedPassword: Password? do { - decryptedPassword = try self.passwordStore.decrypt(passwordEntity: passwordEntity, requestPGPKeyPassphrase: self.requestPGPKeyPassphrase) + let requestPGPKeyPassphrase = Utils.createRequestPGPKeyPassphraseHandler(controller: self) + decryptedPassword = try self.passwordStore.decrypt(passwordEntity: passwordEntity, requestPGPKeyPassphrase: requestPGPKeyPassphrase) let username = decryptedPassword?.username ?? decryptedPassword?.login ?? decryptedPassword?.nameFromPath ?? "" let password = decryptedPassword?.password ?? "" DispatchQueue.main.async {// prepare a dictionary to return @@ -195,28 +196,6 @@ class ExtensionViewController: UIViewController, UITableViewDataSource, UITableV return passwordsTableEntries.count; } - private func requestPGPKeyPassphrase() -> String { - let sem = DispatchSemaphore(value: 0) - var passphrase = "" - DispatchQueue.main.async { - let alert = UIAlertController(title: "Passphrase".localize(), message: "FillInPgpPassphrase.".localize(), preferredStyle: UIAlertController.Style.alert) - alert.addAction(UIAlertAction(title: "Ok".localize(), style: UIAlertAction.Style.default, handler: {_ in - passphrase = alert.textFields!.first!.text! - sem.signal() - })) - alert.addTextField(configurationHandler: {(textField: UITextField!) in - textField.text = self.keychain.get(for: Globals.pgpKeyPassphrase) ?? "" - textField.isSecureTextEntry = true - }) - self.present(alert, animated: true, completion: nil) - } - let _ = sem.wait(timeout: DispatchTime.distantFuture) - if Defaults.isRememberPGPPassphraseOn { - self.keychain.add(string: passphrase, for: Globals.pgpKeyPassphrase) - } - return passphrase - } - @IBAction func cancelExtension(_ sender: Any) { extensionContext!.completeRequest(returningItems: [], completionHandler: nil) } diff --git a/passKit/Crypto/PGPAgent.swift b/passKit/Crypto/PGPAgent.swift index de4bf23..aac9bf9 100644 --- a/passKit/Crypto/PGPAgent.swift +++ b/passKit/Crypto/PGPAgent.swift @@ -45,7 +45,7 @@ public class PGPAgent { return pgpInterface?.shortKeyId } - public func decrypt(encryptedData: Data, keyID: String, requestPGPKeyPassphrase: () -> String) throws -> Data? { + public func decrypt(encryptedData: Data, keyID: String, requestPGPKeyPassphrase: (String) -> String) throws -> Data? { // Remember the previous status and set the current status let previousDecryptStatus = self.latestDecryptStatus self.latestDecryptStatus = false @@ -54,9 +54,9 @@ public class PGPAgent { // Get the PGP key passphrase. var passphrase = "" if previousDecryptStatus == false { - passphrase = requestPGPKeyPassphrase() + passphrase = requestPGPKeyPassphrase(keyID) } else { - passphrase = keyStore.get(for: Globals.pgpKeyPassphrase) ?? requestPGPKeyPassphrase() + passphrase = keyStore.get(for: Globals.pgpKeyPassphrase) ?? requestPGPKeyPassphrase(keyID) } // Decrypt. guard let result = try pgpInterface!.decrypt(encryptedData: encryptedData, keyID: keyID, passphrase: passphrase) else { diff --git a/passKit/Helpers/Utils.swift b/passKit/Helpers/Utils.swift index bd80802..d5cd1fe 100644 --- a/passKit/Helpers/Utils.swift +++ b/passKit/Helpers/Utils.swift @@ -38,5 +38,29 @@ public class Utils { alert.addAction(UIAlertAction(title: "Ok".localize(), style: UIAlertAction.Style.default, handler: handler)) controller.present(alert, animated: true, completion: completion) } + + public static func createRequestPGPKeyPassphraseHandler(controller: UIViewController) -> (String) -> String { + return { keyID in + let sem = DispatchSemaphore(value: 0) + var passphrase = "" + DispatchQueue.main.async { + let alert = UIAlertController(title: "Passphrase".localize() + " (\(keyID.suffix(8)))", message: "FillInPgpPassphrase.".localize(), preferredStyle: UIAlertController.Style.alert) + alert.addAction(UIAlertAction(title: "Ok".localize(), style: UIAlertAction.Style.default, handler: {_ in + passphrase = alert.textFields!.first!.text! + sem.signal() + })) + alert.addTextField(configurationHandler: {(textField: UITextField!) in + textField.text = AppKeychain.shared.get(for: Globals.pgpKeyPassphrase) ?? "" + textField.isSecureTextEntry = true + }) + controller.present(alert, animated: true, completion: nil) + } + let _ = sem.wait(timeout: DispatchTime.distantFuture) + if Defaults.isRememberPGPPassphraseOn { + AppKeychain.shared.add(string: passphrase, for: Globals.pgpKeyPassphrase) + } + return passphrase + } + } } diff --git a/passKit/Models/PasswordStore.swift b/passKit/Models/PasswordStore.swift index c6e5923..db2a7a0 100644 --- a/passKit/Models/PasswordStore.swift +++ b/passKit/Models/PasswordStore.swift @@ -698,7 +698,7 @@ public class PasswordStore { return try storeRepository.localCommitsRelative(toRemoteBranch: remoteBranch) } - public func decrypt(passwordEntity: PasswordEntity, requestPGPKeyPassphrase: () -> String) throws -> Password? { + public func decrypt(passwordEntity: PasswordEntity, requestPGPKeyPassphrase: (String) -> String) throws -> Password? { let encryptedDataPath = storeURL.appendingPathComponent(passwordEntity.getPath()) let keyID = findGPGID(from: encryptedDataPath) let encryptedData = try Data(contentsOf: encryptedDataPath) diff --git a/passKitTests/Crypto/PGPAgentTest.swift b/passKitTests/Crypto/PGPAgentTest.swift index 0efd17b..76427a1 100644 --- a/passKitTests/Crypto/PGPAgentTest.swift +++ b/passKitTests/Crypto/PGPAgentTest.swift @@ -31,7 +31,7 @@ class PGPAgentTest: XCTestCase { super.tearDown() } - func basicEncryptDecrypt(using pgpAgent: PGPAgent, keyID: String, encryptKeyID: String? = nil, requestPassphrase: () -> String = requestPGPKeyPassphrase, encryptInArmored: Bool = true, encryptInArmoredNow: Bool = true) throws -> Data? { + func basicEncryptDecrypt(using pgpAgent: PGPAgent, keyID: String, encryptKeyID: String? = nil, requestPassphrase: (String) -> String = requestPGPKeyPassphrase, encryptInArmored: Bool = true, encryptInArmoredNow: Bool = true) throws -> Data? { passKit.Defaults.encryptInArmored = encryptInArmored let encryptedData = try pgpAgent.encrypt(plainData: testData, keyID: keyID) passKit.Defaults.encryptInArmored = encryptInArmoredNow @@ -132,11 +132,11 @@ class PGPAgentTest: XCTestCase { try importKeys(RSA2048.publicKey, RSA2048.privateKey) var passphraseRequestCalledCount = 0 - let provideCorrectPassphrase: () -> String = { + let provideCorrectPassphrase: (String) -> String = { _ in passphraseRequestCalledCount = passphraseRequestCalledCount + 1 return requestPGPKeyPassphrase() } - let provideIncorrectPassphrase: () -> String = { + let provideIncorrectPassphrase: (String) -> String = { _ in passphraseRequestCalledCount = passphraseRequestCalledCount + 1 return "incorrect passphrase" } diff --git a/passKitTests/Models/PasswordStoreTest.swift b/passKitTests/Models/PasswordStoreTest.swift index 93377df..c42d9e8 100644 --- a/passKitTests/Models/PasswordStoreTest.swift +++ b/passKitTests/Models/PasswordStoreTest.swift @@ -57,7 +57,7 @@ class PasswordStoreTest: XCTestCase { let testPassword = Password(name: "test", url: URL(string: "test.gpg")!, plainText: "testpassword") let testPasswordEntity = try passwordStore.add(password: testPassword)! - let testPasswordPlain = try passwordStore.decrypt(passwordEntity: testPasswordEntity, requestPGPKeyPassphrase: { "passforios" } )! + let testPasswordPlain = try passwordStore.decrypt(passwordEntity: testPasswordEntity, requestPGPKeyPassphrase: requestPGPKeyPassphrase )! XCTAssertEqual(testPasswordPlain.plainText, "testpassword") passwordStore.erase() @@ -65,7 +65,7 @@ class PasswordStoreTest: XCTestCase { private func decrypt(passwordStore: PasswordStore, path: String, passphrase: String) throws -> Password { let entity = passwordStore.getPasswordEntity(by: path, isDir: false)! - return try passwordStore.decrypt(passwordEntity: entity, requestPGPKeyPassphrase: { passphrase } )! + return try passwordStore.decrypt(passwordEntity: entity, requestPGPKeyPassphrase: requestPGPKeyPassphrase )! } diff --git a/passKitTests/Testbase/TestPGPKeys.swift b/passKitTests/Testbase/TestPGPKeys.swift index 57f2b7f..7a81103 100644 --- a/passKitTests/Testbase/TestPGPKeys.swift +++ b/passKitTests/Testbase/TestPGPKeys.swift @@ -68,7 +68,7 @@ let RSA2048_RSA4096 = MultiPGPKeyTestTriple( passphrase: ["passforios", "passforios"] ) -func requestPGPKeyPassphrase() -> String { +func requestPGPKeyPassphrase(keyID: String = "") -> String { return "passforios" }