Skip to content

Commit

Permalink
Merge pull request #161 from amberin/back-to-jgit-v5
Browse files Browse the repository at this point in the history 
Various fixes for Git repos
  • Loading branch information
@amberin
amberin authored Feb 15, 2024
2 parents d2112a7 + 038851d commit a751396
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 135 deletions.
14 changes: 9 additions & 5 deletions app/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ apply plugin: 'kotlin-kapt'
android {
namespace 'com.orgzly'

compileSdkVersion 32
compileSdkVersion 33

defaultConfig {
minSdkVersion 21 // Lollipop (5.0)
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions app/proguard-rules.pro
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@

-dontwarn org.joda.convert.**

-dontwarn org.eclipse.jgit.**
-dontwarn com.jcraft.**
-dontwarn org.slf4j.**

Expand Down
2 changes: 1 addition & 1 deletion app/src/main/AndroidManifest.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />

<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"
android:maxSdkVersion="28" />
android:maxSdkVersion="29" />

<uses-permission android:name="android.permission.MANAGE_EXTERNAL_STORAGE" />

Expand Down
3 changes: 0 additions & 3 deletions app/src/main/java/com/orgzly/android/git/GitSshKeyTransportSetter.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
143 changes: 54 additions & 89 deletions app/src/main/java/com/orgzly/android/git/SshKey.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -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"),
Expand All @@ -128,7 +91,6 @@ object SshKey {
}
}

@RequiresApi(Build.VERSION_CODES.N)
enum class Algorithm(
val algorithm: String,
val applyToSpec: KeyGenParameterSpec.Builder.() -> Unit
Expand Down Expand Up @@ -217,7 +179,6 @@ object SshKey {
type = Type.KeystoreWrappedEd25519
}

@RequiresApi(Build.VERSION_CODES.N)
fun generateKeystoreNativeKey(algorithm: Algorithm, requireAuthentication: Boolean) {
delete()

Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion app/src/main/java/com/orgzly/android/repos/GitRepo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
Loading

0 comments on commit a751396

Please sign in to comment.