Do not forget pgp passphrase on decryption error #296

pass/Controllers/PasswordDetailTableViewController.swift

@@ -18,6 +18,7 @@ class PasswordDetailTableViewController: UITableViewController, UIGestureRecogni
     private var oneTimePasswordIndexPath : IndexPath?
     private var shouldPopCurrentView = false
     private let passwordStore = PasswordStore.shared
+    private let keychain = AppKeychain.shared
 
     private lazy var editUIBarButtonItem: UIBarButtonItem = {
         let uiBarButtonItem = UIBarButtonItem(barButtonSystemItem: .edit, target: self, action: #selector(pressEdit(_:)))
@@ -96,14 +97,14 @@ class PasswordDetailTableViewController: UITableViewController, UIGestureRecogni
                 sem.signal()
             }))
             alert.addTextField(configurationHandler: {(textField: UITextField!) in
-                textField.text = ""
+                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 SharedDefaults[.isRememberPGPPassphraseOn] {
-            AppKeychain.shared.add(string: passphrase, for: Globals.pgpKeyPassphrase)
+            self.keychain.add(string: passphrase, for: Globals.pgpKeyPassphrase)
         }
         return passphrase
     }
@@ -121,8 +122,6 @@ class PasswordDetailTableViewController: UITableViewController, UIGestureRecogni
                 self.password = try self.passwordStore.decrypt(passwordEntity: passwordEntity, requestPGPKeyPassphrase: self.requestPGPKeyPassphrase)
             } catch {
                 DispatchQueue.main.async {
-                    // remove the wrong passphrase so that users could enter it next time
-                    AppKeychain.shared.removeContent(for: Globals.pgpKeyPassphrase)
                     // alert: cancel or try again
                     let alert = UIAlertController(title: "CannotShowPassword".localize(), message: error.localizedDescription, preferredStyle: UIAlertController.Style.alert)
                     alert.addAction(UIAlertAction(title: "Cancel".localize(), style: UIAlertAction.Style.default) { _ in

pass/Controllers/PasswordsViewController.swift

@@ -167,7 +167,7 @@ class PasswordsViewController: UIViewController, UITableViewDataSource, UITableV
         SVProgressHUD.setDefaultStyle(.light)
         SVProgressHUD.show(withStatus: "SyncingPasswordStore".localize())
         var gitCredential: GitCredential
-        let privateKey: String? = AppKeychain.shared.get(for: SshKey.PRIVATE.getKeychainKey())
+        let privateKey: String? = self.keychain.get(for: SshKey.PRIVATE.getKeychainKey())
         if SharedDefaults[.gitAuthenticationMethod] == "Password" || privateKey == nil {
             gitCredential = GitCredential(credential: GitCredential.Credential.http(userName: SharedDefaults[.gitUsername]!))
         } else {
@@ -406,20 +406,14 @@ class PasswordsViewController: UIViewController, UITableViewDataSource, UITableV
                 sem.signal()
             }))
             alert.addTextField(configurationHandler: {(textField: UITextField!) in
-                textField.text = ""
+                textField.text = self.keychain.get(for: Globals.pgpKeyPassphrase) ?? ""
                 textField.isSecureTextEntry = true
             })
-            // hide it so that alert is on the top of the view
-            SVProgressHUD.dismiss()
             self.present(alert, animated: true, completion: nil)
         }
         let _ = sem.wait(timeout: DispatchTime.distantFuture)
-        DispatchQueue.main.async {
-            // bring back
-            SVProgressHUD.show(withStatus: "Decrypting".localize())
-        }
         if SharedDefaults[.isRememberPGPPassphraseOn] {
-            keychain.add(string: passphrase, for: Globals.pgpKeyPassphrase)
+            self.keychain.add(string: passphrase, for: Globals.pgpKeyPassphrase)
         }
         return passphrase
     }
@@ -431,22 +425,20 @@ class PasswordsViewController: UIViewController, UITableViewDataSource, UITableV
         }
         let passwordEntity = getPasswordEntry(by: indexPath).passwordEntity!
         UIImpactFeedbackGenerator(style: .medium).impactOccurred()
-        SVProgressHUD.setDefaultMaskType(.black)
+        SVProgressHUD.dismiss()
-        SVProgressHUD.setDefaultStyle(.dark)
-        SVProgressHUD.show(withStatus: "Decrypting".localize())
         DispatchQueue.global(qos: .userInteractive).async {
             var decryptedPassword: Password?
             do {
                 decryptedPassword = try self.passwordStore.decrypt(passwordEntity: passwordEntity, requestPGPKeyPassphrase: self.requestPGPKeyPassphrase)
                 DispatchQueue.main.async {
                     SecurePasteboard.shared.copy(textToCopy: decryptedPassword?.password)
+                    SVProgressHUD.setDefaultMaskType(.black)
+                    SVProgressHUD.setDefaultStyle(.dark)
                     SVProgressHUD.showSuccess(withStatus: "PasswordCopiedToPasteboard.".localize())
                     SVProgressHUD.dismiss(withDelay: 0.6)
                 }
             } catch {
                 DispatchQueue.main.async {
-                    // remove the wrong passphrase so that users could enter it next time
-                    self.keychain.removeContent(for: Globals.pgpKeyPassphrase)
                     Utils.alert(title: "CannotCopyPassword".localize(), message: error.localizedDescription, controller: self, completion: nil)
                 }
             }

passAutoFillExtension/Controllers/CredentialProviderViewController.swift

@@ -27,6 +27,7 @@ class CredentialProviderViewController: ASCredentialProviderViewController, UITa
     @IBOutlet weak var tableView: UITableView!
 
     private let passwordStore = PasswordStore.shared
+    private let keychain = AppKeychain.shared
 
     private var searchActive = false
     private var passwordsTableEntries: [PasswordsTableEntry] = []
@@ -164,8 +165,6 @@ class CredentialProviderViewController: ASCredentialProviderViewController, UITa
                 }
             } catch {
                 DispatchQueue.main.async {
-                    // remove the wrong passphrase so that users could enter it next time
-                    AppKeychain.shared.removeContent(for: Globals.pgpKeyPassphrase)
                     Utils.alert(title: "CannotCopyPassword".localize(), message: error.localizedDescription, controller: self, completion: nil)
                 }
             }
@@ -193,14 +192,14 @@ class CredentialProviderViewController: ASCredentialProviderViewController, UITa
                 sem.signal()
             }))
             alert.addTextField(configurationHandler: {(textField: UITextField!) in
-                textField.text = ""
+                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 SharedDefaults[.isRememberPGPPassphraseOn] {
-            AppKeychain.shared.add(string: passphrase, for: Globals.pgpKeyPassphrase)
+            self.keychain.add(string: passphrase, for: Globals.pgpKeyPassphrase)
         }
         return passphrase
     }

passExtension/Controllers/ExtensionViewController.swift

@@ -191,8 +191,6 @@ class ExtensionViewController: UIViewController, UITableViewDataSource, UITableV
                 }
             } catch {
                 DispatchQueue.main.async {
-                    // remove the wrong passphrase so that users could enter it next time
-                    self.keychain.removeContent(for: Globals.pgpKeyPassphrase)
                     Utils.alert(title: "CannotCopyPassword".localize(), message: error.localizedDescription, controller: self, completion: nil)
                 }
             }
@@ -221,7 +219,7 @@ class ExtensionViewController: UIViewController, UITableViewDataSource, UITableV
                 sem.signal()
             }))
             alert.addTextField(configurationHandler: {(textField: UITextField!) in
-                textField.text = ""
+                textField.text = self.keychain.get(for: Globals.pgpKeyPassphrase) ?? ""
                 textField.isSecureTextEntry = true
             })
             self.present(alert, animated: true, completion: nil)

passKit/Crypto/PGPAgent.swift

@@ -12,6 +12,7 @@ public class PGPAgent {
 
     private let keyStore: KeyStore
     private var pgpInterface: PgpInterface?
+    private var latestDecryptStatus: Bool = true
 
     public init(keyStore: KeyStore = AppKeychain.shared) {
         self.keyStore = keyStore
@@ -38,9 +39,25 @@ public class PGPAgent {
     }
 
     public func decrypt(encryptedData: Data, requestPGPKeyPassphrase: () -> String) throws -> Data? {
+        // Remember the previous status and set the current status
+        let previousDecryptStatus = self.latestDecryptStatus
+        self.latestDecryptStatus = false
+        // Init keys.
         try checkAndInit()
-        let passphrase = keyStore.get(for: Globals.pgpKeyPassphrase) ?? requestPGPKeyPassphrase()
+        // Get the PGP key passphrase.
-        return try pgpInterface!.decrypt(encryptedData: encryptedData, passphrase: passphrase)
+        var passphrase = ""
+        if previousDecryptStatus == false {
+            passphrase = requestPGPKeyPassphrase()
+        } else {
+            passphrase = keyStore.get(for: Globals.pgpKeyPassphrase) ?? requestPGPKeyPassphrase()
+        }
+        // Decrypt.
+        guard let result = try pgpInterface!.decrypt(encryptedData: encryptedData, passphrase: passphrase) else {
+            return nil
+        }
+        // The decryption step has succeed.
+        self.latestDecryptStatus = true
+        return result
     }
 
     public func encrypt(plainData: Data) throws -> Data {

passKitTests/Crypto/PGPAgentTest.swift

@@ -112,23 +112,29 @@ class PGPAgentTest: XCTestCase {
     func testNoDecryptionWithIncorrectPassphrase() throws {
         try importKeys(RSA2048.publicKey, RSA2048.privateKey)
 
-        var passphraseRequestCalled = false
+        var passphraseRequestCalledCount = 0
         let provideCorrectPassphrase: () -> String = {
-            passphraseRequestCalled = true
+            passphraseRequestCalledCount = passphraseRequestCalledCount + 1
             return requestPGPKeyPassphrase()
         }
-        XCTAssertEqual(try basicEncryptDecrypt(using: pgpAgent, requestPassphrase: provideCorrectPassphrase), testData)
-        XCTAssert(passphraseRequestCalled)
-
-        passphraseRequestCalled = false
         let provideIncorrectPassphrase: () -> String = {
-            passphraseRequestCalled = true
+            passphraseRequestCalledCount = passphraseRequestCalledCount + 1
             return "incorrect passphrase"
         }
+        
+        // Provide the correct passphrase.
+        XCTAssertEqual(try basicEncryptDecrypt(using: pgpAgent, requestPassphrase: provideCorrectPassphrase), testData)
+        XCTAssertEqual(passphraseRequestCalledCount, 1)
+        
+        // Provide the wrong passphrase.
         XCTAssertThrowsError(try basicEncryptDecrypt(using: pgpAgent, requestPassphrase: provideIncorrectPassphrase)) {
             XCTAssert($0.localizedDescription.contains("openpgp: invalid data: private key checksum failure"))
         }
-        XCTAssert(passphraseRequestCalled)
+        XCTAssertEqual(passphraseRequestCalledCount, 2)
+        
+        // Ask for the passphrase because the previous decryption has failed.
+        XCTAssertEqual(try basicEncryptDecrypt(using: pgpAgent, requestPassphrase: provideCorrectPassphrase), testData)
+        XCTAssertEqual(passphraseRequestCalledCount, 3)
     }
 
     private func importKeys(_ publicKey: String, _ privateKey: String) throws {