diff --git a/app/src/main/java/to/bitkit/App.kt b/app/src/main/java/to/bitkit/App.kt index bb7d85b1b3..aeb41632f7 100644 --- a/app/src/main/java/to/bitkit/App.kt +++ b/app/src/main/java/to/bitkit/App.kt @@ -15,6 +15,7 @@ import to.bitkit.appwidget.AppWidgetRefreshScheduler import to.bitkit.env.Env import to.bitkit.services.BluetoothInit import to.bitkit.services.PubkyAuthHandlerRegistrar +import to.bitkit.utils.Crypto import to.bitkit.utils.Logger import to.bitkit.utils.SubscriptionClockOffsetSync import javax.inject.Inject @@ -42,6 +43,8 @@ internal open class App : Application(), Configuration.Provider { .build() override fun onCreate() { + // Runs before super.onCreate(), where Hilt starts building services that open TLS connections + Crypto.installSecurityProvider() super.onCreate() Env.initAppStoragePath(filesDir.absolutePath) installUncaughtExceptionLogger() diff --git a/app/src/main/java/to/bitkit/utils/Crypto.kt b/app/src/main/java/to/bitkit/utils/Crypto.kt index 30903e024c..22b59d90f8 100644 --- a/app/src/main/java/to/bitkit/utils/Crypto.kt +++ b/app/src/main/java/to/bitkit/utils/Crypto.kt @@ -33,6 +33,35 @@ import javax.inject.Singleton @Suppress("SwallowedException", "MagicNumber", "TooGenericExceptionCaught") @Singleton class Crypto @Inject constructor() { + companion object { + /** + * Puts the bundled BouncyCastle in place of the outdated "BC" provider that Android registers. + * + * `App.onCreate` calls this before anything can open a TLS connection. While the swap runs no + * provider offers the "BKS" keystore, and a native TLS verifier that loads its classes in that + * window fails for the rest of the process. Later calls do nothing. + */ + @Synchronized + fun installSecurityProvider() { + // TODO show setup failure on UI? It throws from App.onCreate and stops start-up + try { + val provider = Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) + when { + provider == null -> Security.addProvider(BouncyCastleProvider()) + provider::class.java != BouncyCastleProvider::class.java -> { + // We substitute the outdated BC provider registered in Android. + // Build the replacement first so the gap without a "BC" provider stays short. + val replacement = BouncyCastleProvider() + Security.removeProvider(BouncyCastleProvider.PROVIDER_NAME) + Security.insertProviderAt(replacement, 1) + } + } + } catch (e: Exception) { + throw CryptoError.SecurityProviderSetupFailed() + } + } + } + @Suppress("ArrayInDataClass") data class KeyPair( val privateKey: ByteArray, @@ -50,20 +79,7 @@ class Crypto @Inject constructor() { private val transformation = "AES/GCM/NoPadding" init { - // TODO move init to VM (to enable error handling on UI)? - try { - val provider = Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) - when { - provider == null -> Security.addProvider(BouncyCastleProvider()) - provider::class.java != BouncyCastleProvider::class.java -> { - // We substitute the outdated BC provider registered in Android - Security.removeProvider(BouncyCastleProvider.PROVIDER_NAME) - Security.insertProviderAt(BouncyCastleProvider(), 1) - } - } - } catch (e: Exception) { - throw CryptoError.SecurityProviderSetupFailed() - } + installSecurityProvider() } fun generateKeyPair(): KeyPair { diff --git a/app/src/test/java/to/bitkit/AppTest.kt b/app/src/test/java/to/bitkit/AppTest.kt new file mode 100644 index 0000000000..1ebbb44dae --- /dev/null +++ b/app/src/test/java/to/bitkit/AppTest.kt @@ -0,0 +1,59 @@ +package to.bitkit + +import org.bouncycastle.jce.provider.BouncyCastleProvider +import org.junit.After +import org.junit.Before +import org.junit.Test +import to.bitkit.utils.AppError +import java.security.Provider +import java.security.Security +import kotlin.test.assertFailsWith +import kotlin.test.assertIs + +class AppTest { + private companion object { + const val BC = BouncyCastleProvider.PROVIDER_NAME + } + + private var baselineProvider: Provider? = null + private var baselinePosition = 0 + + @Before + fun setUp() { + baselineProvider = Security.getProvider(BC) + baselinePosition = Security.getProviders().indexOfFirst { it === baselineProvider } + 1 + } + + @After + fun tearDown() { + // The provider list is shared by the whole JVM, so restore BC as it was before this test started + Security.removeProvider(BC) + baselineProvider?.let { Security.insertProviderAt(it, baselinePosition) } + } + + @Test + fun `onCreate installs the security provider before Hilt injects the app`() { + Security.removeProvider(BC) + Security.addProvider(OutdatedBcProvider()) + val app = InjectionProbeApp() + + // The Hilt Gradle plugin rewrites App to extend the generated Hilt_App, whose onCreate() injects App through + // hiltInternalInject(). The probe's method of that name overrides it at runtime and stops onCreate() there. + assertFailsWith { app.onCreate() } + + assertIs(app.providerAtInjection) + } + + private class InjectionProbeApp : App() { + var providerAtInjection: Provider? = null + + fun hiltInternalInject() { + providerAtInjection = Security.getProvider(BC) + throw InjectionReached() + } + } + + private class InjectionReached : AppError("Reached Hilt injection") + + private class OutdatedBcProvider : Provider(BC, 1.0, "Stub for the BC provider that Android registers") +} diff --git a/app/src/test/java/to/bitkit/utils/CryptoTest.kt b/app/src/test/java/to/bitkit/utils/CryptoTest.kt index 482588db7c..483cf67fad 100644 --- a/app/src/test/java/to/bitkit/utils/CryptoTest.kt +++ b/app/src/test/java/to/bitkit/utils/CryptoTest.kt @@ -1,5 +1,7 @@ package to.bitkit.utils +import org.bouncycastle.jce.provider.BouncyCastleProvider +import org.junit.After import org.junit.Before import org.junit.Test import to.bitkit.env.Env.derivationName @@ -8,17 +10,37 @@ import to.bitkit.ext.fromHex import to.bitkit.ext.toBase64 import to.bitkit.ext.toHex import to.bitkit.fcm.EncryptedNotification +import java.security.Provider +import java.security.Security import kotlin.test.assertContentEquals import kotlin.test.assertEquals +import kotlin.test.assertIs +import kotlin.test.assertSame +import kotlin.test.assertTrue class CryptoTest { + private companion object { + const val BC = BouncyCastleProvider.PROVIDER_NAME + } + private lateinit var sut: Crypto + private var baselineProvider: Provider? = null + private var baselinePosition = 0 @Before fun setUp() { + baselineProvider = Security.getProvider(BC) + baselinePosition = positionOf(baselineProvider) sut = Crypto() } + @After + fun tearDown() { + // The provider list is shared by the whole JVM, so restore BC as it was before this test started + Security.removeProvider(BC) + baselineProvider?.let { Security.insertProviderAt(it, baselinePosition) } + } + @Test fun `it should generate valid shared secret from keypair`() { val (privateKey, publicKey) = sut.generateKeyPair() @@ -107,4 +129,48 @@ class CryptoTest { assertEquals(decryptedPayload, value.decodeToString()) } + + @Test + fun `installSecurityProvider adds BouncyCastle when no BC provider is registered`() { + Security.removeProvider(BC) + + Crypto.installSecurityProvider() + + assertIs(Security.getProvider(BC)) + } + + @Test + fun `installSecurityProvider replaces an outdated BC provider at position 1`() { + val outdated = OutdatedBcProvider() + Security.removeProvider(BC) + Security.addProvider(outdated) + + Crypto.installSecurityProvider() + + val installed = assertIs(Security.getProviders().first()) + assertSame(installed, Security.getProvider(BC)) + assertTrue(Security.getProviders().none { it === outdated }) + } + + @Test + fun `installSecurityProvider keeps the installed provider on later calls`() { + Security.removeProvider(BC) + Security.addProvider(OutdatedBcProvider()) + Crypto.installSecurityProvider() + val installed = Security.getProviders().toList() + + Crypto.installSecurityProvider() + Crypto() + + assertSameProviders(installed, Security.getProviders().toList()) + } + + private fun positionOf(provider: Provider?) = Security.getProviders().indexOfFirst { it === provider } + 1 + + private fun assertSameProviders(expected: List, actual: List) { + assertEquals(expected.size, actual.size) + expected.zip(actual).forEach { (want, got) -> assertSame(want, got) } + } + + private class OutdatedBcProvider : Provider(BC, 1.0, "Stub for the BC provider that Android registers") } diff --git a/changelog.d/next/1416.fixed.md b/changelog.d/next/1416.fixed.md new file mode 100644 index 0000000000..e5a83c4196 --- /dev/null +++ b/changelog.d/next/1416.fixed.md @@ -0,0 +1 @@ +Pubky profile, contact and payment features no longer fail after a cold start until the app is restarted.