Remove ability to save SSH private key passwords

This brings AVNC's behavior inline with other SSH clients, e.g. OpenSSH.

app/src/androidTest/java/com/gaurav/avnc/ui/home/ProfileEditorTest.kt

@@ -154,15 +154,13 @@ class PrivateKeyTest {
         setupFileOpenIntent(SAMPLE_KEY)
         onView(withId(R.id.key_import_btn)).doClick()
         onView(withText(R.string.msg_imported)).checkWillBeDisplayed()
-        onView(withId(R.id.ssh_key_password)).checkIsNotDisplayed()
     }
 
     @Test
     fun importEncryptedPK() {
         setupFileOpenIntent(SAMPLE_ENCRYPTED_KEY)
         onView(withId(R.id.key_import_btn)).doClick()
         onView(withText(R.string.msg_imported)).checkWillBeDisplayed()
-        onView(withId(R.id.ssh_key_password)).checkWillBeDisplayed()
     }
 
     @Test

app/src/androidTest/java/com/gaurav/avnc/ui/vnc/LoginFragmentTest.kt

@@ -22,11 +22,20 @@ import androidx.test.espresso.Espresso.onIdle
 import androidx.test.espresso.Espresso.onView
 import androidx.test.espresso.matcher.ViewMatchers.withId
 import androidx.test.espresso.matcher.ViewMatchers.withText
-import com.gaurav.avnc.*
+import com.gaurav.avnc.R
+import com.gaurav.avnc.checkIsDisplayed
+import com.gaurav.avnc.checkIsNotDisplayed
+import com.gaurav.avnc.checkWillBeDisplayed
+import com.gaurav.avnc.doClick
+import com.gaurav.avnc.doTypeText
+import com.gaurav.avnc.inDialog
 import com.gaurav.avnc.model.LoginInfo
 import com.gaurav.avnc.model.ServerProfile
 import com.gaurav.avnc.model.db.MainDb
+import com.gaurav.avnc.runOnMainSync
+import com.gaurav.avnc.targetContext
 import com.gaurav.avnc.viewmodel.VncViewModel
+import com.gaurav.avnc.withActivity
 import kotlinx.coroutines.runBlocking
 import org.junit.Assert.assertEquals
 import org.junit.Test
@@ -129,17 +138,20 @@ class LoginFragmentTest {
         assertEquals(SAMPLE_PASSWORD, p.sshPassword)
     }
 
+    // We no longer save Private Key password. It is always asked from user and a message is shown to users
+    // who have previously saved key password.
     @Test
-    fun sshKeyPasswordLoginWithRememberChecked() = Scenario().use { scenario ->
+    fun sshKeyPasswordMigrationMessage() = Scenario(ServerProfile(sshPrivateKeyPassword = "foo")).use { scenario ->
         scenario.triggerLoginInfoRequest(LoginInfo.Type.SSH_KEY_PASSWORD)
         onView(withId(R.id.password)).inDialog().checkWillBeDisplayed().doTypeText(SAMPLE_PASSWORD)
-        onView(withId(R.id.remember)).inDialog().checkIsDisplayed().doClick()
+        onView(withId(R.id.remember)).inDialog().checkIsNotDisplayed()
+        onView(withId(R.id.pk_password_msg)).inDialog().checkIsDisplayed()
         onView(withText(android.R.string.ok)).inDialog().checkIsDisplayed().doClick()
 
         val l = scenario.waitForLoginInfo()
         val p = scenario.triggerLoginSave()
         assertEquals(SAMPLE_PASSWORD, l.password)
-        assertEquals(SAMPLE_PASSWORD, p.sshPrivateKeyPassword)
+        assertEquals("", p.sshPrivateKeyPassword) // Saved password should have been cleared
     }
 
     /**
@@ -148,14 +160,12 @@ class LoginFragmentTest {
      */
     @Test(timeout = 5000)
     fun savedLoginTest() {
-        val profile = ServerProfile(username = "AB", password = "BC", sshPassword = "CD", sshPrivateKeyPassword = "DE")
+        val profile = ServerProfile(username = "AB", password = "BC", sshPassword = "CD")
         val viewModel = runOnMainSync { VncViewModel(profile, ApplicationProvider.getApplicationContext()) }
 
         assertEquals("AB", viewModel.getLoginInfo(LoginInfo.Type.VNC_CREDENTIAL).username)
         assertEquals("BC", viewModel.getLoginInfo(LoginInfo.Type.VNC_CREDENTIAL).password)
         assertEquals("BC", viewModel.getLoginInfo(LoginInfo.Type.VNC_PASSWORD).password)
-
         assertEquals("CD", viewModel.getLoginInfo(LoginInfo.Type.SSH_PASSWORD).password)
-        assertEquals("DE", viewModel.getLoginInfo(LoginInfo.Type.SSH_KEY_PASSWORD).password)
     }
 }

app/src/main/java/com/gaurav/avnc/ui/home/ProfileEditor.kt

@@ -36,9 +36,9 @@ import com.gaurav.avnc.util.MsgDialog
 import com.gaurav.avnc.util.OpenableDocument
 import com.gaurav.avnc.viewmodel.EditorViewModel
 import com.gaurav.avnc.viewmodel.HomeViewModel
-import com.gaurav.avnc.viewmodel.isPrivateKeyEncrypted
 import com.google.android.material.dialog.MaterialAlertDialogBuilder
 import com.google.android.material.snackbar.Snackbar
+import com.trilead.ssh2.crypto.PEMDecoder
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
@@ -272,21 +272,19 @@ class AdvancedProfileEditor : Fragment() {
 
         lifecycleScope.launch(Dispatchers.IO) {
             var key = ""
-            var encrypted = false
             val result = runCatching {
                 requireContext().contentResolver.openAssetFileDescriptor(uri, "r")!!.use {
                     // Valid key files are only few KBs. So if selected file is too big,
                     // user has accidentally selected something else.
                     check(it.length < 2 * 1024 * 1024) { "File is too big [${it.length}]" }
                     key = it.createInputStream().use { s -> s.reader().use { r -> r.readText() } }
                 }
-                encrypted = isPrivateKeyEncrypted(key)
+                PEMDecoder.parsePEM(key.toCharArray()) //Try to parse key
             }
 
             withContext(Dispatchers.Main) {
                 result.onSuccess {
                     viewModel.profile.sshPrivateKey = key
-                    viewModel.isPrivateKeyEncrypted.value = encrypted
                     viewModel.hasSshPrivateKey.value = true
                     binding.keyImportBtn.error = null
                     Snackbar.make(binding.root, R.string.msg_imported, Snackbar.LENGTH_SHORT).show()

app/src/main/java/com/gaurav/avnc/ui/vnc/LoginFragment.kt

@@ -46,9 +46,12 @@ class LoginFragment : DialogFragment() {
         binding.loginInfo = loginInfo
         binding.usernameLayout.isVisible = loginInfo.username.isBlank() && loginType == LoginInfo.Type.VNC_CREDENTIAL
         binding.passwordLayout.isVisible = loginInfo.password.isBlank()
-        binding.remember.isVisible = viewModel.profile.ID != 0L
-        if (loginType == LoginInfo.Type.SSH_KEY_PASSWORD)
+        binding.remember.isVisible = viewModel.profile.ID != 0L && loginType != LoginInfo.Type.SSH_KEY_PASSWORD
+
+        if (loginType == LoginInfo.Type.SSH_KEY_PASSWORD) {
             binding.passwordLayout.setHint(R.string.hint_key_password)
+            binding.pkPasswordMsg.isVisible = viewModel.profile.sshPrivateKeyPassword.isNotBlank()
+        }
 
         setupAutoComplete()
         isCancelable = false
@@ -73,7 +76,7 @@ class LoginFragment : DialogFragment() {
             LoginInfo.Type.VNC_PASSWORD -> LoginInfo(p.name, p.host, "", p.password)
             LoginInfo.Type.VNC_CREDENTIAL -> LoginInfo(p.name, p.host, p.username, p.password)
             LoginInfo.Type.SSH_PASSWORD -> LoginInfo(p.name, p.sshHost, "", p.sshPassword)
-            LoginInfo.Type.SSH_KEY_PASSWORD -> LoginInfo(p.name, p.sshHost, "", p.sshPrivateKeyPassword)
+            LoginInfo.Type.SSH_KEY_PASSWORD -> LoginInfo(p.name, p.sshHost, "", "" /*p.sshPrivateKeyPassword*/)
         }
     }
 
@@ -85,14 +88,14 @@ class LoginFragment : DialogFragment() {
                 p.password = l.password
             }
             LoginInfo.Type.SSH_PASSWORD -> p.sshPassword = l.password
-            LoginInfo.Type.SSH_KEY_PASSWORD -> p.sshPrivateKeyPassword = l.password
+            LoginInfo.Type.SSH_KEY_PASSWORD -> p.sshPrivateKeyPassword = "" /* key password is not saved anymore */
         }
     }
 
     private fun onOk() {
         loginInfo.password = getRealPassword(loginInfo.password)
         viewModel.loginInfoRequest.offerResponse(loginInfo)
-        if (binding.remember.isChecked)
+        if (binding.remember.isChecked || binding.pkPasswordMsg.isVisible /* to forget saved password */)
             saveLoginInfo(loginInfo)
     }
 

app/src/main/java/com/gaurav/avnc/viewmodel/EditorViewModel.kt

@@ -37,7 +37,6 @@ class EditorViewModel(app: Application, state: SavedStateHandle, initialProfile:
     val sshUsePassword = state.getLiveData("sshUsePassword", profile.sshAuthType == ServerProfile.SSH_AUTH_PASSWORD)
     val sshUsePrivateKey = state.getLiveData("sshUsePrivateKey", profile.sshAuthType == ServerProfile.SSH_AUTH_KEY)
     val hasSshPrivateKey = state.getLiveData("hasSshPrivateKey", profile.sshPrivateKey.isNotBlank())
-    val isPrivateKeyEncrypted = state.getLiveData("isPrivateKeyEncrypted", profile.sshPrivateKey.isNotBlank() && isPrivateKeyEncrypted(profile.sshPrivateKey))
 
 
     fun prepareProfileForSave(): ServerProfile {

app/src/main/java/com/gaurav/avnc/viewmodel/VncViewModel.kt

@@ -316,7 +316,6 @@ class VncViewModel(val profile: ServerProfile, app: Application) : BaseViewModel
         val vu = profile.username
         val vp = profile.password
         val sp = profile.sshPassword
-        val kp = profile.sshPrivateKeyPassword
 
         if (type == LoginInfo.Type.VNC_PASSWORD && vp.isNotBlank())
             return LoginInfo(password = vp)
@@ -327,9 +326,6 @@ class VncViewModel(val profile: ServerProfile, app: Application) : BaseViewModel
         if (type == LoginInfo.Type.SSH_PASSWORD && sp.isNotBlank())
             return LoginInfo(password = sp)
 
-        if (type == LoginInfo.Type.SSH_KEY_PASSWORD && kp.isNotBlank())
-            return LoginInfo(password = kp)
-
         // Something is missing, so we have to ask the user
         return loginInfoRequest.requestResponse(type)  // Blocking call
     }

app/src/main/res/layout/fragment_credential.xml

@@ -7,7 +7,8 @@
   -->
 
 <layout xmlns:android="http://schemas.android.com/apk/res/android"
-    xmlns:app="http://schemas.android.com/apk/res-auto">
+    xmlns:app="http://schemas.android.com/apk/res-auto"
+    xmlns:tools="http://schemas.android.com/tools">
 
     <data>
 
@@ -70,5 +71,17 @@
             android:text="@string/title_remember"
             android:textAppearance="@style/TextAppearance.MaterialComponents.Body2" />
 
+        <TextView
+            android:id="@+id/pk_password_msg"
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content"
+            android:layout_margin="@dimen/margin_normal"
+            android:drawablePadding="@dimen/padding_small"
+            android:text="@string/msg_pk_password_not_saved"
+            android:visibility="gone"
+            app:drawableStartCompat="@drawable/ic_info"
+            app:drawableTint="@color/material_on_background_disabled"
+            tools:visibility="visible" />
+
     </LinearLayout>
 </layout>

app/src/main/res/layout/fragment_profile_editor_advanced.xml

@@ -442,19 +442,6 @@
                         app:layout_constraintStart_toStartOf="parent"
                         app:layout_constraintTop_toBottomOf="@id/ssh_auth_type" />
 
-                    <!--Key Password-->
-                    <EditText
-                        android:id="@+id/ssh_key_password"
-                        style="@style/FormField.EditText"
-                        android:autofillHints="password"
-                        android:drawableStart="@drawable/ic_password"
-                        android:hint="@string/hint_key_password"
-                        android:inputType="textPassword"
-                        android:text="@={viewModel.profile.sshPrivateKeyPassword}"
-                        app:isVisible="@{viewModel.sshUsePrivateKey &amp;&amp; viewModel.isPrivateKeyEncrypted}"
-                        app:layout_constraintBaseline_toBaselineOf="@id/key_import_btn"
-                        app:layout_constraintEnd_toEndOf="parent"
-                        app:layout_constraintStart_toEndOf="@id/key_import_btn" />
                 </androidx.constraintlayout.widget.ConstraintLayout>
             </androidx.constraintlayout.widget.ConstraintLayout>
         </ScrollView>

app/src/main/res/values/strings.xml

@@ -81,6 +81,7 @@
     <string name="msg_invalid_key_file">Invalid private key file</string>
     <string name="msg_invalid_vnc_uri">Invalid VNC URI</string>
     <string name="msg_copied_to_clipboard">Copied</string>
+    <string name="msg_pk_password_not_saved">AVNC no longer saves private key passwords to improve security.</string>
     <string name="msg_key_compat_mode_help">Some servers have limited Unicode support. Legacy key events can help input non-English text.</string>
     <string name="msg_button_up_delay_help">Delaying click events can help in some rare cases if an app is not responding to clicks.</string>
     <string name="msg_drag_gesture_help">Assigning an action to this gesture will change Long press detection:\n\n<b>Press-hold-release</b> → Long press\n<b>Press-hold-swipe</b> → Long press and swipe</string>