From 4e8da9b5f9fb19ba09abb52c0dd10955a291c7a8 Mon Sep 17 00:00:00 2001 From: Fabian Henneke Date: Wed, 12 Aug 2020 16:41:11 +0200 Subject: [PATCH] Create only one SSH session per GitOperation (#1012) --- .../zeapo/pwdstore/git/GitCommandExecutor.kt | 12 +-- .../pwdstore/git/config/SshjSessionFactory.kt | 78 +++++++------------ .../com/zeapo/pwdstore/utils/Extensions.kt | 6 -- 3 files changed, 35 insertions(+), 61 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 { + private var isRetry = false + abstract fun askForPassword(cont: Continuation, isRetry: Boolean) - private var isRetry = false - private var lastPassword: CharArray? = null - private val rememberToWipe: MutableList = mutableListOf() - - fun resetForReuse() { - isRetry = false - } - - fun clearPasswords() { - rememberToWipe.forEach { it.clear() } - lastPassword = null - } - 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 { 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 { return split("\n".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray() } -fun CharArray.clear() { - forEachIndexed { i, _ -> - this[i] = 0.toChar() - } -} - val Context.clipboard get() = getSystemService() fun FragmentActivity.snackbar(