feat: put new SSH layer behind a feature flag

There hasn't been nearly enough regression testing on this and
it is causing breakage, let's move it out of the default path
until we've ironed out the kinks.
This commit is contained in:
Harsh Shandilya 2023-04-10 16:54:13 +05:30
parent 5e3c08f1da
commit fb8d74fc1f
No known key found for this signature in database
9 changed files with 118 additions and 29 deletions

View file

@ -16,7 +16,6 @@ import androidx.fragment.app.FragmentActivity
import app.passwordstore.R
import app.passwordstore.data.repo.PasswordRepository
import app.passwordstore.injection.prefs.GitPreferences
import app.passwordstore.ssh.SSHKeyManager
import app.passwordstore.ui.git.config.GitConfigActivity
import app.passwordstore.ui.git.config.GitServerConfigActivity
import app.passwordstore.ui.proxy.ProxySelectorActivity
@ -30,6 +29,7 @@ import app.passwordstore.util.extensions.snackbar
import app.passwordstore.util.extensions.unsafeLazy
import app.passwordstore.util.settings.GitSettings
import app.passwordstore.util.settings.PreferenceKeys
import app.passwordstore.util.ssh.SSHFacade
import com.github.michaelbull.result.onFailure
import com.github.michaelbull.result.runCatching
import com.google.android.material.dialog.MaterialAlertDialogBuilder
@ -45,11 +45,11 @@ import de.Maxr1998.modernpreferences.helpers.switch
class RepositorySettings(
private val activity: FragmentActivity,
private val sshKeyManager: SSHKeyManager,
private val sshFacade: SSHFacade,
) : SettingsProvider {
private val generateSshKey =
activity.registerForActivityResult(StartActivityForResult()) {
showSshKeyPref?.visible = sshKeyManager.canShowPublicKey()
showSshKeyPref?.visible = sshFacade.canShowPublicKey()
}
private val hiltEntryPoint by unsafeLazy {
@ -114,7 +114,7 @@ class RepositorySettings(
showSshKeyPref =
pref(PreferenceKeys.SSH_SEE_KEY) {
titleRes = R.string.pref_ssh_see_key_title
visible = PasswordRepository.isGitRepo() && sshKeyManager.canShowPublicKey()
visible = PasswordRepository.isGitRepo() && sshFacade.canShowPublicKey()
onClick {
ShowSshKeyFragment().show(activity.supportFragmentManager, "public_key")
true

View file

@ -11,8 +11,8 @@ import androidx.appcompat.app.AppCompatActivity
import androidx.core.os.BundleCompat
import app.passwordstore.R
import app.passwordstore.databinding.ActivityPreferenceRecyclerviewBinding
import app.passwordstore.ssh.SSHKeyManager
import app.passwordstore.util.extensions.viewBinding
import app.passwordstore.util.ssh.SSHFacade
import com.google.android.material.dialog.MaterialAlertDialogBuilder
import dagger.hilt.android.AndroidEntryPoint
import de.Maxr1998.modernpreferences.Preference
@ -24,7 +24,7 @@ import javax.inject.Inject
@AndroidEntryPoint
class SettingsActivity : AppCompatActivity() {
@Inject lateinit var sshKeyManager: SSHKeyManager
@Inject lateinit var sshFacade: SSHFacade
private lateinit var repositorySettings: RepositorySettings
private val miscSettings = MiscSettings(this)
private val autofillSettings = AutofillSettings(this)
@ -40,7 +40,7 @@ class SettingsActivity : AppCompatActivity() {
super.onCreate(savedInstanceState)
setContentView(binding.root)
Preference.Config.dialogBuilderFactory = { context -> MaterialAlertDialogBuilder(context) }
repositorySettings = RepositorySettings(this, sshKeyManager)
repositorySettings = RepositorySettings(this, sshFacade)
val screen =
screen(this) {
subScreen {

View file

@ -9,7 +9,7 @@ import android.content.Intent
import android.os.Bundle
import androidx.fragment.app.DialogFragment
import app.passwordstore.R
import app.passwordstore.ssh.SSHKeyManager
import app.passwordstore.util.ssh.SSHFacade
import com.google.android.material.dialog.MaterialAlertDialogBuilder
import dagger.hilt.android.AndroidEntryPoint
import javax.inject.Inject
@ -17,11 +17,11 @@ import javax.inject.Inject
@AndroidEntryPoint
class ShowSshKeyFragment : DialogFragment() {
@Inject lateinit var sshKeyManager: SSHKeyManager
@Inject lateinit var sshFacade: SSHFacade
override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
val activity = requireActivity()
val publicKey = sshKeyManager.publicKey()
val publicKey = sshFacade.publicKey()
return MaterialAlertDialogBuilder(requireActivity()).run {
setMessage(getString(R.string.ssh_keygen_message, publicKey))
setTitle(R.string.your_public_key)

View file

@ -18,11 +18,11 @@ import app.passwordstore.R
import app.passwordstore.databinding.ActivitySshKeygenBinding
import app.passwordstore.injection.prefs.GitPreferences
import app.passwordstore.ssh.SSHKeyAlgorithm
import app.passwordstore.ssh.SSHKeyManager
import app.passwordstore.util.auth.BiometricAuthenticator
import app.passwordstore.util.auth.BiometricAuthenticator.Result
import app.passwordstore.util.extensions.keyguardManager
import app.passwordstore.util.extensions.viewBinding
import app.passwordstore.util.ssh.SSHFacade
import com.github.michaelbull.result.fold
import com.github.michaelbull.result.runCatching
import com.google.android.material.dialog.MaterialAlertDialogBuilder
@ -40,7 +40,7 @@ class SshKeyGenActivity : AppCompatActivity() {
private var sshKeyAlgorithm = SSHKeyAlgorithm.ECDSA
private val binding by viewBinding(ActivitySshKeygenBinding::inflate)
@GitPreferences @Inject lateinit var gitPrefs: SharedPreferences
@Inject lateinit var sshKeyManager: SSHKeyManager
@Inject lateinit var sshFacade: SSHFacade
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
@ -48,7 +48,7 @@ class SshKeyGenActivity : AppCompatActivity() {
supportActionBar?.setDisplayHomeAsUpEnabled(true)
with(binding) {
generate.setOnClickListener {
if (sshKeyManager.keyExists()) {
if (sshFacade.keyExists()) {
MaterialAlertDialogBuilder(this@SshKeyGenActivity).run {
setTitle(R.string.ssh_keygen_existing_title)
setMessage(R.string.ssh_keygen_existing_message)
@ -126,7 +126,7 @@ class SshKeyGenActivity : AppCompatActivity() {
if (result !is Result.Success)
throw UserNotAuthenticatedException(getString(R.string.biometric_auth_generic_failure))
}
sshKeyManager.generateKey(sshKeyAlgorithm, requireAuthentication)
sshFacade.generateKey(sshKeyAlgorithm, requireAuthentication)
}
}
// Check if we still need this

View file

@ -12,7 +12,7 @@ import androidx.activity.result.contract.ActivityResultContracts
import androidx.appcompat.app.AppCompatActivity
import androidx.lifecycle.lifecycleScope
import app.passwordstore.R
import app.passwordstore.ssh.SSHKeyManager
import app.passwordstore.util.ssh.SSHFacade
import com.github.michaelbull.result.onFailure
import com.github.michaelbull.result.runCatching
import com.google.android.material.dialog.MaterialAlertDialogBuilder
@ -23,7 +23,7 @@ import kotlinx.coroutines.launch
@AndroidEntryPoint
class SshKeyImportActivity : AppCompatActivity() {
@Inject lateinit var sshKeyManager: SSHKeyManager
@Inject lateinit var sshFacade: SSHFacade
private val sshKeyImportAction =
registerForActivityResult(ActivityResultContracts.OpenDocument()) { uri: Uri? ->
@ -33,7 +33,7 @@ class SshKeyImportActivity : AppCompatActivity() {
}
runCatching {
lifecycleScope.launch {
sshKeyManager.importKey(uri)
sshFacade.importKey(uri)
Toast.makeText(
this@SshKeyImportActivity,
resources.getString(R.string.ssh_key_success_dialog_title),
@ -55,7 +55,7 @@ class SshKeyImportActivity : AppCompatActivity() {
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
if (sshKeyManager.keyExists()) {
if (sshFacade.keyExists()) {
MaterialAlertDialogBuilder(this@SshKeyImportActivity).run {
setTitle(R.string.ssh_keygen_existing_title)
setMessage(R.string.ssh_keygen_existing_message)

View file

@ -19,6 +19,9 @@ enum class Feature(
level = DeprecationLevel.ERROR
)
EnablePGPainlessBackend(true, "enable_pgp_v2_backend"),
/** Opt into the new SSH layer implemented as a freestanding module. */
EnableNewSSHLayer(false, "enable_new_ssh"),
;
companion object {

View file

@ -10,7 +10,6 @@ import androidx.appcompat.app.AppCompatActivity
import androidx.fragment.app.FragmentActivity
import app.passwordstore.R
import app.passwordstore.data.repo.PasswordRepository
import app.passwordstore.ssh.SSHKeyManager
import app.passwordstore.ui.sshkeygen.SshKeyGenActivity
import app.passwordstore.ui.sshkeygen.SshKeyImportActivity
import app.passwordstore.util.auth.BiometricAuthenticator
@ -22,6 +21,7 @@ import app.passwordstore.util.git.GitCommandExecutor
import app.passwordstore.util.git.sshj.SshAuthMethod
import app.passwordstore.util.git.sshj.SshjSessionFactory
import app.passwordstore.util.settings.AuthMode
import app.passwordstore.util.ssh.SSHFacade
import com.github.michaelbull.result.Err
import com.github.michaelbull.result.Ok
import com.github.michaelbull.result.Result
@ -71,7 +71,7 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) {
callingActivity.applicationContext,
GitOperationEntryPoint::class.java
)
private val sshKeyManager = hiltEntryPoint.sshKeyManager()
private val sshFacade = hiltEntryPoint.sshFacade()
protected val repository = PasswordRepository.repository!!
protected val git = Git(repository)
private val authActivity
@ -125,7 +125,7 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) {
authMethod: SshAuthMethod,
credentialsProvider: CredentialsProvider? = null
) {
sshSessionFactory = SshjSessionFactory(authMethod, hostKeyFile, sshKeyManager)
sshSessionFactory = SshjSessionFactory(authMethod, hostKeyFile, sshFacade)
commands.filterIsInstance<TransportCommand<*, *>>().forEach { command ->
command.setTransportConfigCallback { transport: Transport ->
(transport as? SshTransport)?.sshSessionFactory = sshSessionFactory
@ -173,8 +173,8 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) {
suspend fun executeAfterAuthentication(authMode: AuthMode): Result<Unit, Throwable> {
when (authMode) {
AuthMode.SshKey ->
if (sshKeyManager.keyExists()) {
if (sshKeyManager.needsAuthentication()) {
if (sshFacade.keyExists()) {
if (sshFacade.needsAuthentication()) {
val result =
withContext(Dispatchers.Main) {
suspendCoroutine<BiometricAuthenticator.Result> { cont ->
@ -245,6 +245,6 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) {
@EntryPoint
@InstallIn(SingletonComponent::class)
interface GitOperationEntryPoint {
fun sshKeyManager(): SSHKeyManager
fun sshFacade(): SSHFacade
}
}

View file

@ -6,9 +6,9 @@ package app.passwordstore.util.git.sshj
import android.util.Base64
import androidx.appcompat.app.AppCompatActivity
import app.passwordstore.ssh.SSHKeyManager
import app.passwordstore.util.git.operation.CredentialFinder
import app.passwordstore.util.settings.AuthMode
import app.passwordstore.util.ssh.SSHFacade
import com.github.michaelbull.result.getOrElse
import com.github.michaelbull.result.runCatching
import java.io.File
@ -69,7 +69,7 @@ abstract class InteractivePasswordFinder : PasswordFinder {
class SshjSessionFactory(
private val authMethod: SshAuthMethod,
private val hostKeyFile: File,
private val sshKeyManager: SSHKeyManager,
private val sshFacade: SSHFacade,
) : SshSessionFactory() {
private var currentSession: SshjSession? = null
@ -81,7 +81,7 @@ class SshjSessionFactory(
tms: Int
): RemoteSession {
return currentSession
?: SshjSession(uri, uri.user, authMethod, hostKeyFile, sshKeyManager).connect().also {
?: SshjSession(uri, uri.user, authMethod, hostKeyFile, sshFacade).connect().also {
logcat { "New SSH connection created" }
currentSession = it
}
@ -125,7 +125,7 @@ private class SshjSession(
private val username: String,
private val authMethod: SshAuthMethod,
private val hostKeyFile: File,
private val sshKeyManager: SSHKeyManager,
private val sshFacade: SSHFacade,
) : RemoteSession {
private lateinit var ssh: SSHClient
@ -160,7 +160,7 @@ private class SshjSession(
is SshAuthMethod.SshKey -> {
val pubkeyAuth =
AuthPublickey(
sshKeyManager.keyProvider(ssh, CredentialFinder(authMethod.activity, AuthMode.SshKey))
sshFacade.keyProvider(ssh, CredentialFinder(authMethod.activity, AuthMode.SshKey))
)
ssh.auth(username, pubkeyAuth, passwordAuth)
}

View file

@ -0,0 +1,86 @@
package app.passwordstore.util.ssh
import android.net.Uri
import app.passwordstore.ssh.SSHKeyAlgorithm
import app.passwordstore.ssh.SSHKeyManager
import app.passwordstore.util.features.Feature
import app.passwordstore.util.features.Features
import app.passwordstore.util.git.operation.CredentialFinder
import app.passwordstore.util.git.sshj.SshKey
import javax.inject.Inject
import javax.inject.Provider
import net.schmizz.sshj.SSHClient
import net.schmizz.sshj.userauth.keyprovider.KeyProvider
/** A wrapper around [SshKey] and [SSHKeyManager] to allow switching between them at runtime. */
class SSHFacade
@Inject
constructor(
private val features: Provider<Features>,
private val sshKeyManager: SSHKeyManager,
) {
private val useNewSSH
get() = features.get().isEnabled(Feature.EnableNewSSHLayer)
fun canShowPublicKey(): Boolean {
return if (useNewSSH) {
sshKeyManager.canShowPublicKey()
} else {
SshKey.canShowSshPublicKey
}
}
fun publicKey(): String? {
return if (useNewSSH) {
sshKeyManager.publicKey()
} else {
SshKey.sshPublicKey
}
}
fun keyExists(): Boolean {
return if (useNewSSH) {
sshKeyManager.keyExists()
} else {
SshKey.exists
}
}
suspend fun generateKey(keyAlgorithm: SSHKeyAlgorithm, requireAuthentication: Boolean) {
if (useNewSSH) {
sshKeyManager.generateKey(keyAlgorithm, requireAuthentication)
} else {
when (keyAlgorithm) {
SSHKeyAlgorithm.RSA ->
SshKey.generateKeystoreNativeKey(SshKey.Algorithm.Rsa, requireAuthentication)
SSHKeyAlgorithm.ECDSA ->
SshKey.generateKeystoreNativeKey(SshKey.Algorithm.Ecdsa, requireAuthentication)
SSHKeyAlgorithm.ED25519 -> SshKey.generateKeystoreWrappedEd25519Key(requireAuthentication)
}
}
}
suspend fun importKey(uri: Uri) {
if (useNewSSH) {
sshKeyManager.importKey(uri)
} else {
SshKey.import(uri)
}
}
fun needsAuthentication(): Boolean {
return if (useNewSSH) {
sshKeyManager.needsAuthentication()
} else {
SshKey.mustAuthenticate
}
}
fun keyProvider(client: SSHClient, credentialFinder: CredentialFinder): KeyProvider? {
return if (useNewSSH) {
sshKeyManager.keyProvider(client, credentialFinder)
} else {
SshKey.provide(client, credentialFinder)
}
}
}