From 0d328b565ea6905ed6cbb1c22f3d4d1ee6a9ed93 Mon Sep 17 00:00:00 2001 From: phantompatch <289516701+phantompatch@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:31:33 +0000 Subject: [PATCH] Feat: Implement SSH Host Key verification --- app/src/main/AndroidManifest.xml | 5 + .../provider/sftp/client/Authenticator.kt | 10 ++ .../files/provider/sftp/client/Client.kt | 53 ++++++++- .../SftpHostKeyChangedDialogActivity.kt | 44 +++++++ .../SftpHostKeyChangedDialogFragment.kt | 109 ++++++++++++++++++ .../files/storage/SftpServerAuthenticator.kt | 52 +++++++++ app/src/main/res/values/strings.xml | 4 + 7 files changed, 274 insertions(+), 3 deletions(-) create mode 100644 app/src/main/java/me/zhanghai/android/files/storage/SftpHostKeyChangedDialogActivity.kt create mode 100644 app/src/main/java/me/zhanghai/android/files/storage/SftpHostKeyChangedDialogFragment.kt diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index f4b7b77d5..f2d25a900 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -295,6 +295,11 @@ android:autoRemoveFromRecents="true" android:theme="@style/Theme.MaterialFiles.Translucent" /> + + { + authenticator.putHostKey(authority, hostKey) + true + } + + knownHostKey == hostKey -> true + + authenticator.confirmChangedHostKey( + authority, + knownHostKey, + hostKey + ) -> true + + else -> { + exception = IOException( + "Host key for $authority has changed: expected $knownHostKey, got $hostKey. " + + "This may indicate a man-in-the-middle attack." + ) + false + } + } + } + + override fun findExistingAlgorithms(hostname: String, port: Int): List = + authenticator.getHostKey(authority)?.substringBefore(' ')?.let { listOf(it) } + ?: emptyList() + + private fun PublicKey.toHostKey(): String = + "${KeyType.fromKey(this)} ${SecurityUtils.getFingerprint(this)}" + } + interface Path { val authority: Authority val remotePath: String diff --git a/app/src/main/java/me/zhanghai/android/files/storage/SftpHostKeyChangedDialogActivity.kt b/app/src/main/java/me/zhanghai/android/files/storage/SftpHostKeyChangedDialogActivity.kt new file mode 100644 index 000000000..0e0b13230 --- /dev/null +++ b/app/src/main/java/me/zhanghai/android/files/storage/SftpHostKeyChangedDialogActivity.kt @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2026 Material Files contributors + * SPDX‑License‑Identifier: GPL-3.0-or-later + */ + +package me.zhanghai.android.files.storage + +import android.os.Bundle +import android.view.View +import androidx.fragment.app.commit +import me.zhanghai.android.files.app.AppActivity +import me.zhanghai.android.files.util.args +import me.zhanghai.android.files.util.putArgs + +class SftpHostKeyChangedDialogActivity : AppActivity() { + private val args by args() + + private lateinit var fragment: SftpHostKeyChangedDialogFragment + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + + // Calls ensureSubDecor(). + findViewById(android.R.id.content) + if (savedInstanceState == null) { + fragment = SftpHostKeyChangedDialogFragment().putArgs(args) + supportFragmentManager.commit { + add(fragment, SftpHostKeyChangedDialogFragment::class.java.name) + } + } else { + fragment = supportFragmentManager.findFragmentByTag( + SftpHostKeyChangedDialogFragment::class.java.name + ) as SftpHostKeyChangedDialogFragment + } + } + + override fun onDestroy() { + super.onDestroy() + + if (isFinishing) { + fragment.onFinish() + } + } +} diff --git a/app/src/main/java/me/zhanghai/android/files/storage/SftpHostKeyChangedDialogFragment.kt b/app/src/main/java/me/zhanghai/android/files/storage/SftpHostKeyChangedDialogFragment.kt new file mode 100644 index 000000000..7f566600a --- /dev/null +++ b/app/src/main/java/me/zhanghai/android/files/storage/SftpHostKeyChangedDialogFragment.kt @@ -0,0 +1,109 @@ +/* + * Copyright (c) 2026 Material Files contributors + * SPDX‑License‑Identifier: GPL-3.0-or-later + */ + +package me.zhanghai.android.files.storage + +import android.app.Dialog +import android.content.Context +import android.content.DialogInterface +import android.os.Bundle +import android.os.Parcel +import androidx.appcompat.app.AppCompatDialogFragment +import com.google.android.material.dialog.MaterialAlertDialogBuilder +import kotlinx.parcelize.Parceler +import kotlinx.parcelize.Parcelize +import kotlinx.parcelize.WriteWith +import me.zhanghai.android.files.R +import me.zhanghai.android.files.util.ParcelableArgs +import me.zhanghai.android.files.util.RemoteCallback +import me.zhanghai.android.files.util.args +import me.zhanghai.android.files.util.finish +import me.zhanghai.android.files.util.getArgs +import me.zhanghai.android.files.util.putArgs +import me.zhanghai.android.files.util.readParcelable + +class SftpHostKeyChangedDialogFragment : AppCompatDialogFragment() { + private val args by args() + + private var isListenerNotified = false + + override fun onCreateDialog(savedInstanceState: Bundle?): Dialog = + MaterialAlertDialogBuilder(requireContext(), theme) + .setTitle(R.string.storage_sftp_host_key_changed_title) + .setMessage( + getMessage(args.authority, args.oldHostKey, args.newHostKey, requireContext()) + ) + .setPositiveButton(R.string.storage_sftp_host_key_changed_trust) { _, _ -> + notifyListenerOnce(true) + finish() + } + .setNegativeButton(android.R.string.cancel) { _, _ -> + notifyListenerOnce(false) + finish() + } + .create() + .apply { setCanceledOnTouchOutside(false) } + + override fun onCancel(dialog: DialogInterface) { + super.onCancel(dialog) + + notifyListenerOnce(false) + finish() + } + + fun onFinish() { + notifyListenerOnce(false) + } + + private fun notifyListenerOnce(trust: Boolean) { + if (isListenerNotified) { + return + } + args.listener(trust) + isListenerNotified = true + } + + companion object { + fun getMessage(authority: String, context: Context): String = + context.getString(R.string.storage_sftp_host_key_changed_message_format, authority) + + private fun getMessage( + authority: String, + oldHostKey: String, + newHostKey: String, + context: Context + ): String = context.getString( + R.string.storage_sftp_host_key_changed_message_with_keys_format, + authority, + oldHostKey, + newHostKey + ) + } + + @Parcelize + class Args( + val authority: String, + val oldHostKey: String, + val newHostKey: String, + val listener: @WriteWith() (Boolean) -> Unit + ) : ParcelableArgs { + object ListenerParceler : Parceler<(Boolean) -> Unit> { + override fun create(parcel: Parcel): (Boolean) -> Unit = + parcel.readParcelable()!!.let { + { trust -> it.sendResult(Bundle().putArgs(ListenerArgs(trust))) } + } + + override fun ((Boolean) -> Unit).write(parcel: Parcel, flags: Int) { + parcel.writeParcelable(RemoteCallback { + val args = it.getArgs() + this(args.trust) + }, flags) + } + + @Parcelize + private class ListenerArgs(val trust: Boolean) : ParcelableArgs + } + } +} diff --git a/app/src/main/java/me/zhanghai/android/files/storage/SftpServerAuthenticator.kt b/app/src/main/java/me/zhanghai/android/files/storage/SftpServerAuthenticator.kt index effb7f8ae..39eb70101 100644 --- a/app/src/main/java/me/zhanghai/android/files/storage/SftpServerAuthenticator.kt +++ b/app/src/main/java/me/zhanghai/android/files/storage/SftpServerAuthenticator.kt @@ -5,14 +5,27 @@ package me.zhanghai.android.files.storage +import androidx.core.content.edit +import kotlinx.coroutines.runBlocking +import me.zhanghai.android.files.R +import me.zhanghai.android.files.app.BackgroundActivityStarter +import me.zhanghai.android.files.app.application +import me.zhanghai.android.files.app.defaultSharedPreferences import me.zhanghai.android.files.provider.sftp.client.Authentication import me.zhanghai.android.files.provider.sftp.client.Authenticator import me.zhanghai.android.files.provider.sftp.client.Authority import me.zhanghai.android.files.settings.Settings +import me.zhanghai.android.files.util.createIntent +import me.zhanghai.android.files.util.putArgs import me.zhanghai.android.files.util.valueCompat +import kotlin.coroutines.resume +import kotlin.coroutines.suspendCoroutine object SftpServerAuthenticator : Authenticator { + private const val HOST_KEY_PREFIX = "sftp_host_key_" + private val transientServers = mutableSetOf() + private val hostKeyLock = Any() override fun getAuthentication(authority: Authority): Authentication? { val server = synchronized(transientServers) { @@ -23,6 +36,43 @@ object SftpServerAuthenticator : Authenticator { return server?.authentication } + override fun getHostKey(authority: Authority): String? = synchronized(hostKeyLock) { + defaultSharedPreferences.getString(authority.toHostKeyPreferenceKey(), null) + } + + override fun putHostKey(authority: Authority, hostKey: String) { + synchronized(hostKeyLock) { + defaultSharedPreferences.edit { putString(authority.toHostKeyPreferenceKey(), hostKey) } + } + } + + override fun confirmChangedHostKey( + authority: Authority, + oldHostKey: String, + newHostKey: String + ): Boolean = runBlocking { + suspendCoroutine { continuation -> + val authorityString = authority.toString() + val onTrustDecision: (Boolean) -> Unit = { trust -> + if (trust) { + putHostKey(authority, newHostKey) + } + continuation.resume(trust) + } + + BackgroundActivityStarter.startActivity( + SftpHostKeyChangedDialogActivity::class.createIntent().putArgs( + SftpHostKeyChangedDialogFragment.Args( + authorityString, oldHostKey, newHostKey, onTrustDecision + ) + ), + application.getString(R.string.storage_sftp_host_key_changed_title), + SftpHostKeyChangedDialogFragment.getMessage(authorityString, application), + application + ) + } + } + fun addTransientServer(server: SftpServer) { synchronized(transientServers) { transientServers += server } } @@ -30,4 +80,6 @@ object SftpServerAuthenticator : Authenticator { fun removeTransientServer(server: SftpServer) { synchronized(transientServers) { transientServers -= server } } + + private fun Authority.toHostKeyPreferenceKey(): String = "$HOST_KEY_PREFIX$this" } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 964179529..ecfedbe25 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -613,6 +613,10 @@ Invalid private key password Connect and add Add + SFTP host key changed + The host key for %1$s has changed. This may indicate a man-in-the-middle attack. Only trust the new key if you expected this change. + The host key for %1$s has changed. This may indicate a man-in-the-middle attack. Only trust the new key if you expected this change.\n\nOld key:\n%2$s\n\nNew key:\n%3$s + Trust @string/storage_edit_smb_server_title_add Searching for SMB servers… Add manually