diff --git a/app/build.gradle b/app/build.gradle index 544f91dc3..027ac83fa 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -5,7 +5,7 @@ apply plugin: 'kotlin-kapt' android { namespace 'com.orgzly' - compileSdkVersion 32 + compileSdkVersion 33 defaultConfig { minSdkVersion 21 // Lollipop (5.0) @@ -190,16 +190,20 @@ dependencies { implementation "io.github.rburgst:okhttp-digest:$versions.okhttp_digest" - // Git sync via SSH - implementation "org.eclipse.jgit:org.eclipse.jgit:$versions.jgit" - implementation "org.eclipse.jgit:org.eclipse.jgit.ssh.apache:$versions.jgit" + implementation("org.eclipse.jgit:org.eclipse.jgit:$versions.jgit") { + because 'Git repo support' + } + implementation("org.eclipse.jgit:org.eclipse.jgit.ssh.apache:$versions.jgit") { + because 'SSH transport support for Git repos' + // Avoid DuplicatePlatformClasses error: + exclude group: 'org.apache.sshd', module: 'sshd-sftp' + } implementation("androidx.security:security-crypto:$versions.security_crypto") { because 'SSH key generation' } implementation("androidx.biometric:biometric-ktx:$versions.biometric_ktx") { because 'Protect SSH key with biometric prompt' } - } repositories { diff --git a/app/proguard-rules.pro b/app/proguard-rules.pro index 16566ebdc..5deb032a2 100644 --- a/app/proguard-rules.pro +++ b/app/proguard-rules.pro @@ -47,6 +47,7 @@ -dontwarn org.joda.convert.** +-dontwarn org.eclipse.jgit.** -dontwarn com.jcraft.** -dontwarn org.slf4j.** diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 08649d4bd..85876a26c 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -14,7 +14,7 @@ + android:maxSdkVersion="29" /> diff --git a/app/src/main/java/com/orgzly/android/git/GitSshKeyTransportSetter.kt b/app/src/main/java/com/orgzly/android/git/GitSshKeyTransportSetter.kt index dc80579de..b13e0565b 100644 --- a/app/src/main/java/com/orgzly/android/git/GitSshKeyTransportSetter.kt +++ b/app/src/main/java/com/orgzly/android/git/GitSshKeyTransportSetter.kt @@ -3,7 +3,6 @@ package com.orgzly.android.git import android.os.Build import androidx.annotation.RequiresApi import com.orgzly.android.App -import org.apache.sshd.common.util.OsUtils import org.eclipse.jgit.annotations.NonNull import org.eclipse.jgit.api.TransportCommand import org.eclipse.jgit.api.TransportConfigCallback @@ -53,8 +52,6 @@ class GitSshKeyTransportSetter: GitTransportSetter { // org.apache.sshd.common.config.keys.IdentityUtils freaks out if user.home is not set System.setProperty("user.home", context.filesDir.toString()) - // org.apache.sshd.common.util.OsUtils has trouble recognizing Android - OsUtils.setAndroid(true) configCallback = TransportConfigCallback { transport: Transport -> val sshTransport = transport as SshTransport diff --git a/app/src/main/java/com/orgzly/android/git/SshKey.kt b/app/src/main/java/com/orgzly/android/git/SshKey.kt index 45babcf14..f8906b58e 100644 --- a/app/src/main/java/com/orgzly/android/git/SshKey.kt +++ b/app/src/main/java/com/orgzly/android/git/SshKey.kt @@ -5,10 +5,8 @@ import android.content.Intent import android.content.pm.PackageManager import android.os.Build import android.security.keystore.KeyGenParameterSpec -import android.security.keystore.KeyInfo import android.security.keystore.KeyProperties import android.security.keystore.UserNotAuthenticatedException -import android.util.Log import androidx.annotation.RequiresApi import androidx.core.content.edit import androidx.security.crypto.EncryptedFile @@ -22,6 +20,7 @@ import com.orgzly.android.util.BiometricAuthenticator import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext +import net.i2p.crypto.eddsa.EdDSAEngine import net.i2p.crypto.eddsa.EdDSAPrivateKey import net.i2p.crypto.eddsa.spec.EdDSANamedCurveTable import net.i2p.crypto.eddsa.spec.EdDSAPrivateKeySpec @@ -31,8 +30,6 @@ import java.io.File import java.io.IOException import java.security.* import java.security.interfaces.RSAKey -import javax.crypto.SecretKey -import javax.crypto.SecretKeyFactory private const val PROVIDER_ANDROID_KEY_STORE = "AndroidKeyStore" private const val KEYSTORE_ALIAS = "orgzly_sshkey" @@ -60,63 +57,29 @@ fun toSshPublicKey(publicKey: PublicKey): String { return PublicKeyEntry.toString(publicKey) } +@RequiresApi(Build.VERSION_CODES.N) object SshKey { - private val TAG = SshKey::class.java.name val sshPublicKey get() = if (publicKeyFile.exists()) publicKeyFile.readText() else null val canShowSshPublicKey get() = type in listOf(Type.KeystoreNative, Type.KeystoreWrappedEd25519) val exists get() = type != null - private val mustAuthenticate: Boolean - @RequiresApi(Build.VERSION_CODES.N) - get() { - return runCatching { - if (type !in listOf(Type.KeystoreNative, Type.KeystoreWrappedEd25519)) return false - when (val key = androidKeystore.getKey(KEYSTORE_ALIAS, null)) { - is PrivateKey -> { - val factory = - KeyFactory.getInstance(key.algorithm, PROVIDER_ANDROID_KEY_STORE) - return factory.getKeySpec( - key, - KeyInfo::class.java - ).isUserAuthenticationRequired - } - is SecretKey -> { - val factory = - SecretKeyFactory.getInstance(key.algorithm, PROVIDER_ANDROID_KEY_STORE) - (factory.getKeySpec( - key, - KeyInfo::class.java - ) as KeyInfo).isUserAuthenticationRequired - } - else -> throw IllegalStateException("SSH key does not exist in Keystore") - } - } - .getOrElse { - // It is fine to swallow the exception here since it will reappear when the key - // is used for SSH authentication and can then be shown in the UI. - false - } - } - private val context: Context get() = App.getAppContext() - private val privateKeyFile get() = File(context.filesDir, "ssh_key") private val publicKeyFile get() = File(context.filesDir, "ssh_key.pub") - private var type: Type? get() = Type.fromValue(AppPreferences.gitSshKeyType(context)) set(value) = AppPreferences.gitSshKeyType(context, value?.value) - private val isStrongBoxSupported by unsafeLazy { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) context.packageManager.hasSystemFeature(PackageManager.FEATURE_STRONGBOX_KEYSTORE) else false } + private var privateKeyLoadAttempts = 0 private enum class Type(val value: String) { KeystoreNative("keystore_native"), @@ -128,7 +91,6 @@ object SshKey { } } - @RequiresApi(Build.VERSION_CODES.N) enum class Algorithm( val algorithm: String, val applyToSpec: KeyGenParameterSpec.Builder.() -> Unit @@ -217,7 +179,6 @@ object SshKey { type = Type.KeystoreWrappedEd25519 } - @RequiresApi(Build.VERSION_CODES.N) fun generateKeystoreNativeKey(algorithm: Algorithm, requireAuthentication: Boolean) { delete() @@ -248,10 +209,9 @@ object SshKey { type = Type.KeystoreNative } - @RequiresApi(Build.VERSION_CODES.N) fun getKeyPair(): KeyPair { + privateKeyLoadAttempts = 0 var privateKey: PrivateKey? = null - var privateKeyLoadAttempts = 0 val publicKey: PublicKey? = when (type) { Type.KeystoreNative -> { kotlin.runCatching { androidKeystore.sshPublicKey } @@ -282,70 +242,75 @@ object SshKey { } } Type.KeystoreWrappedEd25519 -> { - runCatching { - // The current MasterKey API does not allow getting a reference to an existing - // one without specifying the KeySpec for a new one. However, the value for - // passed here for `requireAuthentication` is not used as the key already exists - // at this point. - val encryptedPrivateKeyFile = runBlocking { - getOrCreateWrappedPrivateKeyFile(false) - } - val rawPrivateKey = - encryptedPrivateKeyFile.openFileInput().use { it.readBytes() } - EdDSAPrivateKey( - EdDSAPrivateKeySpec( - rawPrivateKey, EdDSANamedCurveTable.ED_25519_CURVE_SPEC - ) - ) - }.getOrElse { error -> - throw IOException(context.getString(R.string.ssh_key_failed_get_private), error) + try { + tryToReadEd25519PrivateKey() + } catch (e: UserNotAuthenticatedException) { + tryBiometricAuthOrFail(e) + tryToReadEd25519PrivateKey() + } catch (e: Exception) { + throw IOException(context.getString(R.string.ssh_key_failed_get_private), e) } } else -> throw IllegalStateException("SSH key does not exist in Keystore") } try { // Try to sign something to see if the key is unlocked - val algorithm: String = if (privateKey is RSAKey) { - "SHA256withRSA" - } else { - "SHA256withECDSA" + val signature = when (privateKey) { + is EdDSAPrivateKey -> EdDSAEngine(MessageDigest.getInstance( + EdDSANamedCurveTable.getByName(EdDSANamedCurveTable.ED_25519).hashAlgorithm)) + is RSAKey -> Signature.getInstance("SHA256withRSA") + else -> Signature.getInstance("SHA256withECDSA") } - Signature.getInstance(algorithm).apply { + signature.apply { initSign(privateKey) update("loremipsum".toByteArray()) }.sign() // The key is unlocked; exit the loop. break } catch (e: UserNotAuthenticatedException) { - if (privateKeyLoadAttempts > 0) { - // We expect this exception before trying auth, but after that, this means - // we have failed to unlock the SSH key. - Log.e(TAG, context.getString(R.string.ssh_key_failed_unlock_private), e) - throw IOException(context.getString(R.string.ssh_key_failed_unlock_private)) - } + tryBiometricAuthOrFail(e) } catch (e: Exception) { - // Our attempt to use the key for signing may go wrong in many unforeseen ways. - // Such failures are unimportant. - e.printStackTrace() + throw e } - if (mustAuthenticate && privateKeyLoadAttempts == 0) { - // Time to try biometric auth - val currentActivity = App.getCurrentActivity() - checkNotNull(currentActivity) { - throw IOException(context.getString(R.string.ssh_key_locked_and_no_activity)) - } - val biometricAuthenticator = BiometricAuthenticator(currentActivity) - runBlocking(Dispatchers.Main) { - biometricAuthenticator.authenticate( - context.getString( - R.string.biometric_prompt_title_unlock_ssh_key - ) + } + return KeyPair(publicKey, privateKey) + } + + private fun tryToReadEd25519PrivateKey(): EdDSAPrivateKey { + // The current MasterKey API does not allow getting a reference to an existing + // one without specifying the KeySpec for a new one. However, the value for + // passed here for `requireAuthentication` is not used as the key already exists + // at this point. + val encryptedPrivateKeyFile = runBlocking { + getOrCreateWrappedPrivateKeyFile(false) + } + val rawPrivateKey = + encryptedPrivateKeyFile.openFileInput().use { it.readBytes() } + return EdDSAPrivateKey( + EdDSAPrivateKeySpec( + rawPrivateKey, EdDSANamedCurveTable.ED_25519_CURVE_SPEC + ) + ) + } + + private fun tryBiometricAuthOrFail(e: UserNotAuthenticatedException) { + if (privateKeyLoadAttempts == 0) { + val currentActivity = App.getCurrentActivity() + checkNotNull(currentActivity) { + throw IOException(context.getString(R.string.ssh_key_locked_and_no_activity)) + } + val biometricAuthenticator = BiometricAuthenticator(currentActivity) + runBlocking(Dispatchers.Main) { + biometricAuthenticator.authenticate( + context.getString( + R.string.biometric_prompt_title_unlock_ssh_key ) - } + ) } privateKeyLoadAttempts++ + } else { + throw e } - return KeyPair(publicKey, privateKey) } @RequiresApi(Build.VERSION_CODES.N) diff --git a/app/src/main/java/com/orgzly/android/repos/GitRepo.java b/app/src/main/java/com/orgzly/android/repos/GitRepo.java index 652c89c98..c466e324c 100644 --- a/app/src/main/java/com/orgzly/android/repos/GitRepo.java +++ b/app/src/main/java/com/orgzly/android/repos/GitRepo.java @@ -158,7 +158,6 @@ private static Git cloneRepo(Uri repoUri, File directoryFile, GitTransportSetter } catch (IOException ex) { ex.printStackTrace(); } - Log.e(TAG, "JGit error:", e); throw new IOException(e); } } diff --git a/app/src/main/java/com/orgzly/android/ui/repo/git/GitRepoActivity.kt b/app/src/main/java/com/orgzly/android/ui/repo/git/GitRepoActivity.kt index 415ca08db..0493289d4 100644 --- a/app/src/main/java/com/orgzly/android/ui/repo/git/GitRepoActivity.kt +++ b/app/src/main/java/com/orgzly/android/ui/repo/git/GitRepoActivity.kt @@ -3,6 +3,7 @@ package com.orgzly.android.ui.repo.git import android.app.Activity import android.app.ProgressDialog +import android.content.DialogInterface import android.content.Intent import android.content.SharedPreferences import android.net.Uri @@ -21,6 +22,7 @@ import android.widget.EditText import androidx.annotation.RequiresApi import androidx.lifecycle.Observer import androidx.lifecycle.ViewModelProvider +import com.google.android.material.dialog.MaterialAlertDialogBuilder import com.google.android.material.textfield.TextInputLayout import com.orgzly.R import com.orgzly.android.App @@ -38,15 +40,14 @@ import com.orgzly.android.ui.repo.BrowserActivity import com.orgzly.android.ui.repo.RepoViewModel import com.orgzly.android.ui.repo.RepoViewModelFactory import com.orgzly.android.ui.showSnackbar +import com.orgzly.android.ui.util.copyPlainTextToClipboard import com.orgzly.android.util.AppPermissions import com.orgzly.android.util.MiscUtils import com.orgzly.databinding.ActivityRepoGitBinding -import org.eclipse.jgit.errors.TransportException import org.eclipse.jgit.errors.NoRemoteRepositoryException import org.eclipse.jgit.errors.NotSupportedException import org.eclipse.jgit.lib.ProgressMonitor import java.io.File -import java.io.FileNotFoundException import java.io.IOException class GitRepoActivity : CommonActivity(), GitPreferences { @@ -124,10 +125,14 @@ class GitRepoActivity : CommonActivity(), GitPreferences { createDefaultRepoFolder() binding.activityRepoGitAuthor.setText("Orgzly") binding.activityRepoGitBranch.setText(R.string.git_default_branch) - val userDeviceName: String = if (Build.VERSION.SDK_INT > Build.VERSION_CODES.S) { - Settings.Global.getString(contentResolver, Settings.Global.DEVICE_NAME) - } else { - Settings.Secure.getString(contentResolver, "bluetooth_name") + val userDeviceName: String = try { + if (Build.VERSION.SDK_INT > Build.VERSION_CODES.S) { + Settings.Global.getString(contentResolver, Settings.Global.DEVICE_NAME) + } else { + Settings.Secure.getString(contentResolver, "bluetooth_name") + } + } catch (e: Exception) { + "MyPhone" } binding.activityRepoGitEmail.setText(String.format("orgzly@%s", userDeviceName)) } @@ -275,8 +280,14 @@ class GitRepoActivity : CommonActivity(), GitPreferences { save() } else { val targetDirectory = File(binding.activityRepoGitDirectory.text.toString()) + if (!targetDirectory.exists()) { + binding.activityRepoGitDirectoryLayout.error = + getString(R.string.git_clone_error_invalid_target_dir) + return + } if (targetDirectory.list()!!.isNotEmpty()) { - binding.activityRepoGitDirectoryLayout.error = getString(R.string.git_clone_error_target_not_empty) + binding.activityRepoGitDirectoryLayout.error = + getString(R.string.git_clone_error_target_not_empty) return } val repoScheme = getRepoScheme() @@ -295,28 +306,28 @@ class GitRepoActivity : CommonActivity(), GitPreferences { if (e == null) { save() } else { - val error = when (e.cause) { - is NoRemoteRepositoryException -> R.string.git_clone_error_invalid_repo - is TransportException -> { - // JGit's catch-all "remote hung up unexpectedly" message is not very useful. - if (Regex("hung up unexpectedly").containsMatchIn(e.cause!!.message!!)) { - String.format(getString(R.string.git_clone_error_ssh), e.cause!!.cause!!.message) - } else { - String.format(getString(R.string.git_clone_error_ssh), e.cause!!.message) - } - } - // TODO: This should be checked when the user enters a directory by hand - is FileNotFoundException -> R.string.git_clone_error_invalid_target_dir - is GitRepo.DirectoryNotEmpty -> R.string.git_clone_error_target_not_empty - is NotSupportedException -> R.string.git_clone_error_uri_not_supported - else -> R.string.git_clone_error_unknown - } - when (error) { - is Int -> { showSnackbar(error) } - is String -> { showSnackbar(error) } + val error = when (val rootException = getRootException(e)) { + is NoRemoteRepositoryException -> getString(R.string.git_clone_error_invalid_repo) + is NotSupportedException -> getString(R.string.git_clone_error_uri_not_supported) + else -> rootException.localizedMessage?.toString() } - e.printStackTrace() + MaterialAlertDialogBuilder(this) + .setPositiveButton(R.string.ok, null) + .setNeutralButton("Copy stack trace") { _: DialogInterface?, _: Int -> + this.copyPlainTextToClipboard("Error during cloning", e.stackTraceToString()) + } + .setMessage(error) + .show() + Log.e(TAG, "Error during repo cloning:", e) + } + } + + private fun getRootException(e: Throwable): Throwable { + var result = e + while (result.cause != null) { + result = result.cause as Throwable } + return result } private fun saveToPreferences(id: Long): Boolean { @@ -355,7 +366,7 @@ class GitRepoActivity : CommonActivity(), GitPreferences { for (field in fields) { if (field.layout.visibility == View.GONE || field.allowEmpty) { - continue; + continue } if (errorIfEmpty(field.editText, field.layout)) { hasEmptyFields = true @@ -472,10 +483,8 @@ class GitRepoActivity : CommonActivity(), GitPreferences { try { GitRepo.ensureRepositoryExists(fragment, true, this) } catch (e: IOException) { - Log.e(TAG, "Error while cloning Git repository:", e) return e } - return null } @@ -520,10 +529,6 @@ class GitRepoActivity : CommonActivity(), GitPreferences { override fun endTask() { } - - override fun showDuration(enabled: Boolean) { - TODO("Not yet implemented") - } } companion object { diff --git a/build.gradle b/build.gradle index cfb4dd0d9..3926e82c0 100644 --- a/build.gradle +++ b/build.gradle @@ -65,9 +65,9 @@ buildscript { versions.okhttp_digest = '2.7' - versions.jgit = '6.7.0.202309050840-r' + versions.jgit = '5.13.3.202401111512-r' - versions.security_crypto = '1.1.0-alpha03' + versions.security_crypto = '1.1.0-alpha06' versions.biometric_ktx = '1.2.0-alpha04' diff --git a/gradle.properties b/gradle.properties index 807552ddd..877be9ba9 100644 --- a/gradle.properties +++ b/gradle.properties @@ -16,5 +16,6 @@ android.enableJetifier = true android.databinding.incremental = true kotlin.code.style = official +org.gradle.unsafe.configuration-cache=true # org.gradle.warning.mode=all \ No newline at end of file