From 898e0d51f57f6d39c4aa4cd09b7af241a631b042 Mon Sep 17 00:00:00 2001 From: CC Date: Mon, 15 Jun 2026 15:47:09 +0200 Subject: [PATCH] fix(security): fail closed on identity key load errors --- .../android/crypto/EncryptionService.kt | 31 +++--- .../identity/SecureIdentityStateManager.kt | 94 +++++++++---------- .../SecureIdentityStateManagerTest.kt | 57 +++++++++++ 3 files changed, 117 insertions(+), 65 deletions(-) create mode 100644 app/src/test/java/com/bitchat/android/identity/SecureIdentityStateManagerTest.kt diff --git a/app/src/main/java/com/bitchat/android/crypto/EncryptionService.kt b/app/src/main/java/com/bitchat/android/crypto/EncryptionService.kt index 4b51fd31..bf3d3425 100644 --- a/app/src/main/java/com/bitchat/android/crypto/EncryptionService.kt +++ b/app/src/main/java/com/bitchat/android/crypto/EncryptionService.kt @@ -415,21 +415,19 @@ open class EncryptionService(private val context: Context) { private fun loadOrCreateEd25519KeyPair(): AsymmetricCipherKeyPair { // Migrate legacy plaintext Ed25519 key to encrypted storage if present migrateOldEd25519KeyIfNeeded() - try { - val storedKey = prefs.getString(ED25519_PRIVATE_KEY_PREF, null) - if (storedKey != null) { - // Load existing key - val privateKeyBytes = Base64.decode(storedKey, Base64.DEFAULT) - val privateKey = Ed25519PrivateKeyParameters(privateKeyBytes, 0) - val publicKey = privateKey.generatePublicKey() - Log.d(TAG, "✅ Loaded existing Ed25519 signing key pair") - return AsymmetricCipherKeyPair(publicKey, privateKey) + val storedKey = prefs.getString(ED25519_PRIVATE_KEY_PREF, null) + if (storedKey != null) { + val privateKeyBytes = Base64.decode(storedKey, Base64.DEFAULT) + check(privateKeyBytes.size == 32) { + "Invalid Ed25519 private key size: ${privateKeyBytes.size}" } - } catch (e: Exception) { - Log.w(TAG, "⚠️ Failed to load existing Ed25519 key, creating new one: ${e.message}") + val privateKey = Ed25519PrivateKeyParameters(privateKeyBytes, 0) + val publicKey = privateKey.generatePublicKey() + Log.d(TAG, "✅ Loaded existing Ed25519 signing key pair") + return AsymmetricCipherKeyPair(publicKey, privateKey) } - + // Create new key pair return generateAndSaveEd25519KeyPair() } @@ -445,10 +443,14 @@ open class EncryptionService(private val context: Context) { val privateKeyBytes = privateKey.encoded val encodedKey = Base64.encodeToString(privateKeyBytes, Base64.DEFAULT) - prefs.edit { putString(ED25519_PRIVATE_KEY_PREF, encodedKey) } + val saved = prefs.edit() + .putString(ED25519_PRIVATE_KEY_PREF, encodedKey) + .commit() + check(saved) { "Failed to commit Ed25519 private key" } Log.d(TAG, "✅ Created and stored new Ed25519 signing key pair") } catch (e: Exception) { Log.e(TAG, "❌ Failed to store Ed25519 private key: ${e.message}") + throw e } return keyPair @@ -471,7 +473,8 @@ open class EncryptionService(private val context: Context) { Log.d(TAG, "🔁 Migrated Ed25519 key to EncryptedSharedPreferences") } } catch (e: Exception) { - Log.w(TAG, "⚠️ Failed to migrate Ed25519 key; generating new identity: ${e.message}") + Log.e(TAG, "❌ Failed to migrate Ed25519 key: ${e.message}") + throw e } } } diff --git a/app/src/main/java/com/bitchat/android/identity/SecureIdentityStateManager.kt b/app/src/main/java/com/bitchat/android/identity/SecureIdentityStateManager.kt index 012ea3c4..6fd67f74 100644 --- a/app/src/main/java/com/bitchat/android/identity/SecureIdentityStateManager.kt +++ b/app/src/main/java/com/bitchat/android/identity/SecureIdentityStateManager.kt @@ -60,30 +60,11 @@ class SecureIdentityStateManager(private val context: Context) { * Returns (privateKey, publicKey) or null if none exists */ fun loadStaticKey(): Pair? { - return try { - val privateKeyString = prefs.getString(KEY_STATIC_PRIVATE_KEY, null) - val publicKeyString = prefs.getString(KEY_STATIC_PUBLIC_KEY, null) - - if (privateKeyString != null && publicKeyString != null) { - val privateKey = android.util.Base64.decode(privateKeyString, android.util.Base64.DEFAULT) - val publicKey = android.util.Base64.decode(publicKeyString, android.util.Base64.DEFAULT) - - // Validate key sizes - if (privateKey.size == 32 && publicKey.size == 32) { - Log.d(TAG, "Loaded static identity key from secure storage") - Pair(privateKey, publicKey) - } else { - Log.w(TAG, "Invalid key sizes in storage, returning null") - null - } - } else { - Log.d(TAG, "No static identity key found in storage") - null - } - } catch (e: Exception) { - Log.e(TAG, "Failed to load static key: ${e.message}") - null - } + return loadKeyPair( + privateKeyName = KEY_STATIC_PRIVATE_KEY, + publicKeyName = KEY_STATIC_PUBLIC_KEY, + keyDescription = "static identity key" + ) } /** @@ -99,10 +80,11 @@ class SecureIdentityStateManager(private val context: Context) { val privateKeyString = android.util.Base64.encodeToString(privateKey, android.util.Base64.DEFAULT) val publicKeyString = android.util.Base64.encodeToString(publicKey, android.util.Base64.DEFAULT) - prefs.edit() + val saved = prefs.edit() .putString(KEY_STATIC_PRIVATE_KEY, privateKeyString) .putString(KEY_STATIC_PUBLIC_KEY, publicKeyString) - .apply() + .commit() + check(saved) { "Failed to commit static identity key" } Log.d(TAG, "Saved static identity key to secure storage") } catch (e: Exception) { @@ -118,30 +100,39 @@ class SecureIdentityStateManager(private val context: Context) { * Returns (privateKey, publicKey) or null if none exists */ fun loadSigningKey(): Pair? { - return try { - val privateKeyString = prefs.getString(KEY_SIGNING_PRIVATE_KEY, null) - val publicKeyString = prefs.getString(KEY_SIGNING_PUBLIC_KEY, null) - - if (privateKeyString != null && publicKeyString != null) { - val privateKey = android.util.Base64.decode(privateKeyString, android.util.Base64.DEFAULT) - val publicKey = android.util.Base64.decode(publicKeyString, android.util.Base64.DEFAULT) - - // Validate key sizes - if (privateKey.size == 32 && publicKey.size == 32) { - Log.d(TAG, "Loaded Ed25519 signing key from secure storage") - Pair(privateKey, publicKey) - } else { - Log.w(TAG, "Invalid signing key sizes in storage, returning null") - null - } - } else { - Log.d(TAG, "No Ed25519 signing key found in storage") - null - } - } catch (e: Exception) { - Log.e(TAG, "Failed to load signing key: ${e.message}") - null + return loadKeyPair( + privateKeyName = KEY_SIGNING_PRIVATE_KEY, + publicKeyName = KEY_SIGNING_PUBLIC_KEY, + keyDescription = "Ed25519 signing key" + ) + } + + private fun loadKeyPair( + privateKeyName: String, + publicKeyName: String, + keyDescription: String + ): Pair? { + val privateKeyString = prefs.getString(privateKeyName, null) + val publicKeyString = prefs.getString(publicKeyName, null) + + if (privateKeyString == null && publicKeyString == null) { + Log.d(TAG, "No $keyDescription found in storage") + return null } + + val privateEncoded = privateKeyString + ?: error("Incomplete $keyDescription in secure storage") + val publicEncoded = publicKeyString + ?: error("Incomplete $keyDescription in secure storage") + + val privateKey = Base64.decode(privateEncoded, Base64.DEFAULT) + val publicKey = Base64.decode(publicEncoded, Base64.DEFAULT) + check(privateKey.size == 32 && publicKey.size == 32) { + "Invalid $keyDescription sizes: private=${privateKey.size}, public=${publicKey.size}" + } + + Log.d(TAG, "Loaded $keyDescription from secure storage") + return Pair(privateKey, publicKey) } /** @@ -157,10 +148,11 @@ class SecureIdentityStateManager(private val context: Context) { val privateKeyString = android.util.Base64.encodeToString(privateKey, android.util.Base64.DEFAULT) val publicKeyString = android.util.Base64.encodeToString(publicKey, android.util.Base64.DEFAULT) - prefs.edit() + val saved = prefs.edit() .putString(KEY_SIGNING_PRIVATE_KEY, privateKeyString) .putString(KEY_SIGNING_PUBLIC_KEY, publicKeyString) - .apply() + .commit() + check(saved) { "Failed to commit Ed25519 signing key" } Log.d(TAG, "Saved Ed25519 signing key to secure storage") } catch (e: Exception) { diff --git a/app/src/test/java/com/bitchat/android/identity/SecureIdentityStateManagerTest.kt b/app/src/test/java/com/bitchat/android/identity/SecureIdentityStateManagerTest.kt new file mode 100644 index 00000000..efa68de1 --- /dev/null +++ b/app/src/test/java/com/bitchat/android/identity/SecureIdentityStateManagerTest.kt @@ -0,0 +1,57 @@ +package com.bitchat.android.identity + +import android.content.Context +import android.util.Base64 +import androidx.test.core.app.ApplicationProvider +import org.junit.After +import org.junit.Assert.assertNull +import org.junit.Assert.assertThrows +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class SecureIdentityStateManagerTest { + private lateinit var manager: SecureIdentityStateManager + + @Before + fun setup() { + val context = ApplicationProvider.getApplicationContext() + manager = SecureIdentityStateManager(context) + manager.clearIdentityData() + } + + @After + fun tearDown() { + manager.clearIdentityData() + } + + @Test + fun loadStaticKey_returnsNullWhenBothKeysAreAbsent() { + assertNull(manager.loadStaticKey()) + } + + @Test + fun loadStaticKey_throwsWhenOnlyOneKeyExists() { + manager.storeSecureValue("static_private_key", encoded(ByteArray(32) { 1 })) + + assertThrows(IllegalStateException::class.java) { + manager.loadStaticKey() + } + } + + @Test + fun loadSigningKey_throwsWhenStoredKeyHasInvalidSize() { + manager.storeSecureValue("signing_private_key", encoded(ByteArray(31) { 1 })) + manager.storeSecureValue("signing_public_key", encoded(ByteArray(32) { 2 })) + + assertThrows(IllegalStateException::class.java) { + manager.loadSigningKey() + } + } + + private fun encoded(bytes: ByteArray): String { + return Base64.encodeToString(bytes, Base64.DEFAULT) + } +}