Create only one SSH session per GitOperation (#1012)

3 files changed, 34 insertions, 60 deletions

diff --git a/app/src/main/java/com/zeapo/pwdstore/git/GitCommandExecutor.kt b/app/src/main/java/com/zeapo/pwdstore/git/GitCommandExecutor.kt
index 3f41aaec..97ce6602 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/GitCommandExecutor.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/GitCommandExecutor.kt
@@ -44,8 +44,8 @@ class GitCommandExecutor(
             length = Snackbar.LENGTH_INDEFINITE,
         )
         var operationResult: Result = Result.Ok
-        for (command in operation.commands) {
-            try {
+        try {
+            for (command in operation.commands) {
                 when (command) {
                     is CommitCommand -> {
                         // the previous status will eventually be used to avoid a commit
@@ -108,9 +108,9 @@ class GitCommandExecutor(
                         }
                     }
                 }
-            } catch (e: Exception) {
-                operationResult = Result.Err(e)
             }
+        } catch (e: Exception) {
+            operationResult = Result.Err(e)
         }
         when (operationResult) {
             is Result.Err -> {
@@ -131,7 +131,9 @@ class GitCommandExecutor(
             }
         }
         snackbar.dismiss()
-        (SshSessionFactory.getInstance() as? SshjSessionFactory)?.clearCredentials()
+        withContext(Dispatchers.IO) {
+            (SshSessionFactory.getInstance() as? SshjSessionFactory)?.close()
+        }
         SshSessionFactory.setInstance(null)
     }
 
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/config/SshjSessionFactory.kt b/app/src/main/java/com/zeapo/pwdstore/git/config/SshjSessionFactory.kt
index 066f8ac6..85b5f753 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/config/SshjSessionFactory.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/config/SshjSessionFactory.kt
@@ -7,7 +7,6 @@ package com.zeapo.pwdstore.git.config
 import android.util.Base64
 import com.github.ajalt.timberkt.d
 import com.github.ajalt.timberkt.w
-import com.zeapo.pwdstore.utils.clear
 import java.io.File
 import java.io.IOException
 import java.io.InputStream
@@ -36,60 +35,25 @@ import org.eclipse.jgit.transport.URIish
 import org.eclipse.jgit.util.FS
 
 sealed class SshAuthData {
-    class Password(val passwordFinder: InteractivePasswordFinder) : SshAuthData() {
-
-        override fun clearCredentials() {
-            passwordFinder.clearPasswords()
-        }
-    }
-
-    class PublicKeyFile(val keyFile: File, val passphraseFinder: InteractivePasswordFinder) : SshAuthData() {
-
-        override fun clearCredentials() {
-            passphraseFinder.clearPasswords()
-        }
-    }
-
-    abstract fun clearCredentials()
+    class Password(val passwordFinder: InteractivePasswordFinder) : SshAuthData()
+    class PublicKeyFile(val keyFile: File, val passphraseFinder: InteractivePasswordFinder) : SshAuthData()
 }
 
 abstract class InteractivePasswordFinder : PasswordFinder {
 
-    abstract fun askForPassword(cont: Continuation<String?>, isRetry: Boolean)
-
     private var isRetry = false
-    private var lastPassword: CharArray? = null
-    private val rememberToWipe: MutableList<CharArray> = mutableListOf()
 
-    fun resetForReuse() {
-        isRetry = false
-    }
-
-    fun clearPasswords() {
-        rememberToWipe.forEach { it.clear() }
-        lastPassword = null
-    }
+    abstract fun askForPassword(cont: Continuation<String?>, isRetry: Boolean)
 
     final override fun reqPassword(resource: Resource<*>?): CharArray {
-        if (lastPassword != null && !isRetry) {
-            // This instance successfully authenticated in a previous authentication step and is
-            // now being reused for a new one. We try the previous password so that the user
-            // does not have to type it again.
-            isRetry = true
-            return lastPassword!!.clone().also { rememberToWipe.add(it) }
-        }
-        clearPasswords()
         val password = runBlocking(Dispatchers.Main) {
             suspendCoroutine<String?> { cont ->
                 askForPassword(cont, isRetry)
             }
         }
         isRetry = true
-        if (password == null)
-            throw SSHException(DisconnectReason.AUTH_CANCELLED_BY_USER)
-        val passwordChars = password.toCharArray().also { rememberToWipe.add(it) }
-        lastPassword = passwordChars
-        return passwordChars.clone().also { rememberToWipe.add(it) }
+        return password?.toCharArray()
+            ?: throw SSHException(DisconnectReason.AUTH_CANCELLED_BY_USER)
     }
 
     final override fun shouldRetry(resource: Resource<*>?) = true
@@ -97,12 +61,17 @@ abstract class InteractivePasswordFinder : PasswordFinder {
 
 class SshjSessionFactory(private val authData: SshAuthData, private val hostKeyFile: File) : SshSessionFactory() {
 
+    private var currentSession: SshjSession? = null
+
     override fun getSession(uri: URIish, credentialsProvider: CredentialsProvider?, fs: FS?, tms: Int): RemoteSession {
-        return SshjSession(uri, uri.user, authData, hostKeyFile).connect()
+        return currentSession ?: SshjSession(uri, uri.user, authData, hostKeyFile).connect().also {
+            d { "New SSH connection created" }
+            currentSession = it
+        }
     }
 
-    fun clearCredentials() {
-        authData.clearCredentials()
+    fun close() {
+        currentSession?.close()
     }
 }
 
@@ -155,11 +124,9 @@ private class SshjSession(uri: URIish, private val username: String, private val
         when (authData) {
             is SshAuthData.Password -> {
                 ssh.authPassword(username, authData.passwordFinder)
-                authData.passwordFinder.resetForReuse()
             }
             is SshAuthData.PublicKeyFile -> {
                 ssh.authPublickey(username, ssh.loadKeys(authData.keyFile.absolutePath, authData.passphraseFinder))
-                authData.passphraseFinder.resetForReuse()
             }
         }
         return this
@@ -167,17 +134,28 @@ private class SshjSession(uri: URIish, private val username: String, private val
 
     override fun exec(commandName: String?, timeout: Int): Process {
         if (currentCommand != null) {
-            w { "Killing old session" }
-            currentCommand?.close()
-            currentCommand = null
+            w { "Killing old command" }
+            disconnect()
         }
         val session = ssh.startSession()
         currentCommand = session
         return SshjProcess(session.exec(commandName), timeout.toLong())
     }
 
+    /**
+     * Kills the current command if one is running and returns the session into a state where `exec`
+     * can be called.
+     *
+     * Note that this does *not* disconnect the session. Unfortunately, the function has to be
+     * called `disconnect` to override the corresponding abstract function in `RemoteSession`.
+     */
     override fun disconnect() {
         currentCommand?.close()
+        currentCommand = null
+    }
+
+    fun close() {
+        disconnect()
         ssh.close()
     }
 }
diff --git a/app/src/main/java/com/zeapo/pwdstore/utils/Extensions.kt b/app/src/main/java/com/zeapo/pwdstore/utils/Extensions.kt
index e088097b..da6be410 100644
--- a/app/src/main/java/com/zeapo/pwdstore/utils/Extensions.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/utils/Extensions.kt
@@ -42,12 +42,6 @@ fun String.splitLines(): Array<String> {
     return split("\n".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray()
 }
 
-fun CharArray.clear() {
-    forEachIndexed { i, _ ->
-        this[i] = 0.toChar()
-    }
-}
-
 val Context.clipboard get() = getSystemService<ClipboardManager>()
 
 fun FragmentActivity.snackbar(