fix(security): fail closed on identity key load errors

2026-07-24 21:45:21 +00:00 · 2026-06-15 15:47:09 +02:00
parent 5db58ad7ca
commit 898e0d51f5
3 changed files with 117 additions and 65 deletions
@@ -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
        }
    }
 }
@@ -60,30 +60,11 @@ class SecureIdentityStateManager(private val context: Context) {
     * Returns (privateKey, publicKey) or null if none exists
     */
    fun loadStaticKey(): Pair<ByteArray, ByteArray>? {
-        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<ByteArray, ByteArray>? {
-        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<ByteArray, ByteArray>? {
+        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) {
@@ -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<Context>()
+        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)
+    }
+}