Merge pull request #20 from AckeeCZ/feature/synchronized-keystore

🐛 Synchronize Android KeyStore operations

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 All notable changes to this project will be documented in this file.
 
-The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres
 to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
@@ -11,6 +11,42 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 ### datastore-preferences
 ### jetpack
 
+
+
+## BOM [1.1.0] - 2025-01-17
+
+### core
+#### Added
+- `AndroidKeyStoreSemaphore` that is used to synchronize all Android KeyStore operations in Guardian
+- Abstractions over a few JCA APIs that synchronize operations with Android KeyStore
+- Possibility to provide a custom `Semaphore` to `MasterKey`
+
+#### Fixed
+- Android KeyStore synchronization in `MasterKey`
+
+### datastore
+#### Added
+- Possibility to provide a custom `Semaphore` to `encryptedDataStore` and `DataStoreFactory.createEncrypted`
+
+#### Fixed
+- Android KeyStore synchronization in `encryptedDataStore` and `DataStoreFactory.createEncrypted`
+
+### datastore-preferences
+#### Added
+- Possibility to provide a custom `Semaphore` to `encryptedPreferencesDataStore` and `PreferenceDataStoreFactory.createEncrypted`
+
+#### Fixed
+- Android KeyStore synchronization in `encryptedPreferencesDataStore` and `PreferenceDataStoreFactory.createEncrypted`
+
+### jetpack
+#### Added
+- Possibility to provide a custom `Semaphore` to `EncryptedFile` and `EncryptedSharedPreferences`
+
+#### Fixed
+- Android KeyStore synchronization in `EncryptedFile` and `EncryptedSharedPreferences`
+
+
+
 ## BOM [1.0.0] - 2024-12-10
 
 ### core

README.md

@@ -32,6 +32,29 @@ Contains basic core cryptographic logic like `MasterKey` class (rewritten from J
 that is used by other modules to encrypt the data. You don't have to depend on this module directly,
 if you use `datastore` modules or `jetpack`.
 
+#### Android KeyStore synchronization
+
+Android KeyStore is not thread-safe and its operations must be synchronized to avoid errors on various
+range of devices. Ackee Guardian synchronizes all Android KeyStore operations performed under the hood
+using a single `AndroidKeyStoreSemaphore` object.
+
+It is important to know that you need to synchronize all KeyStore operations, not only those using 
+`KeyStore` class, but even all others using various classes from JCA that are backed-up by 
+AndroidKeyStore provider implementation. This includes e.g. `KeyGenerator` for key generation in 
+Android KeyStore or `Cipher` for encryption/decryption using keys stored in Android KeyStore. 
+These operations have to be synchronized across your whole app, so even though Ackee Guardian 
+synchronizes operations under the hood, you need to synchronize your custom operations involving 
+Android KeyStore together with those in Ackee Guardian. Guardian already provides some abstractions 
+over JCA APIs backed by AndroidKeyStore provider, that are properly synchronized like 
+`SynchronizedAndroidKeyStore` or `SynchronizedAndroidKeyGenerator`, that you can use without any 
+other synchronization code. However, not all JCA APIs are covered or maybe you can't use provided 
+abstractions for some reason. In these cases, the simplest way to synchronize everything correctly 
+is to wrap all your Android KeyStore operations in the `AndroidKeyStoreSemaphore.withPermit` calls.
+
+There is more options how you can approach the synchronization using Ackee Guardian, which are
+discussed in more detail in `AndroidKeyStoreSemaphore` documentation, that also provides more
+information about this topic and implementation in Guardian.
+
 ### DataStore
 
 DataStore modules provide an encrypted version of `DataStore`s. They use [Tink](https://github.com/tink-crypto/tink-java)

core-internal/build.gradle.kts

@@ -2,6 +2,7 @@ plugins {
     alias(libs.plugins.ackeecz.guardian.android.library)
     alias(libs.plugins.ackeecz.guardian.publishing)
     alias(libs.plugins.ackeecz.guardian.testfixtures)
+    alias(libs.plugins.ackeecz.guardian.tink)
 }
 
 android {

core-internal/src/main/java/io/github/ackeecz/guardian/core/internal/AndroidKeysetManagerSynchronizedBuilder.kt

@@ -0,0 +1,36 @@
+package io.github.ackeecz.guardian.core.internal
+
+import android.content.Context
+import com.google.crypto.tink.KeyTemplate
+import com.google.crypto.tink.integration.android.AndroidKeysetManager
+import kotlinx.coroutines.sync.Semaphore
+import kotlinx.coroutines.sync.withPermit
+
+/**
+ * Decorator of [AndroidKeysetManager.Builder] that synchronizes Android KeyStore operations using
+ * [semaphore].
+ */
+public class AndroidKeysetManagerSynchronizedBuilder(private val semaphore: Semaphore) {
+
+    private val delegate = AndroidKeysetManager.Builder()
+
+    public fun withKeyTemplate(template: KeyTemplate): AndroidKeysetManagerSynchronizedBuilder = apply {
+        delegate.withKeyTemplate(template)
+    }
+
+    public fun withSharedPref(
+        context: Context,
+        keysetName: String,
+        prefFileName: String,
+    ): AndroidKeysetManagerSynchronizedBuilder = apply {
+        delegate.withSharedPref(context, keysetName, prefFileName)
+    }
+
+    public fun withMasterKeyUri(uri: String): AndroidKeysetManagerSynchronizedBuilder = apply {
+        delegate.withMasterKeyUri(uri)
+    }
+
+    public suspend fun build(): AndroidKeysetManager {
+        return semaphore.withPermit { delegate.build() }
+    }
+}

core-internal/src/main/java/io/github/ackeecz/guardian/core/internal/SynchronizedDataHolder.kt

@@ -1,19 +1,15 @@
 package io.github.ackeecz.guardian.core.internal
 
-import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
-import kotlinx.coroutines.withContext
 
 /**
  * Holder of [T], which creation is synchronized in the scope of the instance of
  * this class. Creation is not synchronized among multiple instances of this class, i.e. creation of
  * [T] can run concurrently in multiple threads, where each operates on different
  * instance of this class.
  */
-public abstract class SynchronizedDataHolder<T : Any>(
-    private val defaultDispatcher: CoroutineDispatcher,
-) {
+public abstract class SynchronizedDataHolder<T : Any> {
 
     private val mutex = Mutex()
 
@@ -26,16 +22,15 @@ public abstract class SynchronizedDataHolder<T : Any>(
     public suspend fun getOrCreate(): T {
         return synchronizedData ?: mutex.withLock {
             if (synchronizedData == null) {
-                synchronizedData = withContext(defaultDispatcher) { createSynchronizedData() }
+                synchronizedData = createSynchronizedData()
             }
             requireNotNull(synchronizedData)
         }
     }
 
     /**
-     * Creates [T]. This method is called from [defaultDispatcher] and is synchronized in the scope
-     * of this instance, i.e. for each [SynchronizedDataHolder] instance this will be called just
-     * once.
+     * Creates [T]. This method is synchronized in the scope of this instance, i.e. for each
+     * [SynchronizedDataHolder] instance this will be called just once.
      */
     protected abstract suspend fun createSynchronizedData(): T
 }

core/api/core.api

@@ -11,6 +11,76 @@ public final class io/github/ackeecz/guardian/core/MasterKey$Companion {
 	public final fun createSafeDefaultSpecBuilder (Ljava/lang/String;)Landroid/security/keystore/KeyGenParameterSpec$Builder;
 	public static synthetic fun createSafeDefaultSpecBuilder$default (Lio/github/ackeecz/guardian/core/MasterKey$Companion;Ljava/lang/String;ILjava/lang/Object;)Landroid/security/keystore/KeyGenParameterSpec$Builder;
 	public final fun getOrCreate (Landroid/security/keystore/KeyGenParameterSpec;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
-	public static synthetic fun getOrCreate$default (Lio/github/ackeecz/guardian/core/MasterKey$Companion;Landroid/security/keystore/KeyGenParameterSpec;Lkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object;
+	public final fun getOrCreate (Landroid/security/keystore/KeyGenParameterSpec;Lkotlinx/coroutines/sync/Semaphore;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public final fun getOrCreate (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public static synthetic fun getOrCreate$default (Lio/github/ackeecz/guardian/core/MasterKey$Companion;Landroid/security/keystore/KeyGenParameterSpec;Lkotlinx/coroutines/sync/Semaphore;Lkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object;
+}
+
+public final class io/github/ackeecz/guardian/core/keystore/android/AndroidKeyStoreSemaphore : kotlinx/coroutines/sync/Semaphore {
+	public static final field INSTANCE Lio/github/ackeecz/guardian/core/keystore/android/AndroidKeyStoreSemaphore;
+	public fun acquire (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public fun getAvailablePermits ()I
+	public fun release ()V
+	public final fun setPermits (I)V
+	public fun tryAcquire ()Z
+}
+
+public abstract interface class io/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyGenerator {
+	public static final field Companion Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyGenerator$Companion;
+	public abstract fun generateKey (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun getAlgorithm ()Ljava/lang/String;
+	public abstract fun init (I)V
+	public abstract fun init (ILjava/security/SecureRandom;)V
+	public abstract fun init (Ljava/security/SecureRandom;)V
+	public abstract fun init (Ljava/security/spec/AlgorithmParameterSpec;)V
+	public abstract fun init (Ljava/security/spec/AlgorithmParameterSpec;Ljava/security/SecureRandom;)V
+}
+
+public final class io/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyGenerator$Companion {
+	public final fun getInstance (Ljava/lang/String;Lkotlinx/coroutines/sync/Semaphore;)Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyGenerator;
+	public static synthetic fun getInstance$default (Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyGenerator$Companion;Ljava/lang/String;Lkotlinx/coroutines/sync/Semaphore;ILjava/lang/Object;)Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyGenerator;
+}
+
+public abstract interface class io/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyPairGenerator {
+	public static final field Companion Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyPairGenerator$Companion;
+	public abstract fun genKeyPair (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun generateKeyPair (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun getAlgorithm ()Ljava/lang/String;
+	public abstract fun initialize (I)V
+	public abstract fun initialize (ILjava/security/SecureRandom;)V
+	public abstract fun initialize (Ljava/security/spec/AlgorithmParameterSpec;)V
+	public abstract fun initialize (Ljava/security/spec/AlgorithmParameterSpec;Ljava/security/SecureRandom;)V
+}
+
+public final class io/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyPairGenerator$Companion {
+	public final fun getInstance (Ljava/lang/String;Lkotlinx/coroutines/sync/Semaphore;)Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyPairGenerator;
+	public static synthetic fun getInstance$default (Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyPairGenerator$Companion;Ljava/lang/String;Lkotlinx/coroutines/sync/Semaphore;ILjava/lang/Object;)Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyPairGenerator;
+}
+
+public abstract interface class io/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyStore {
+	public static final field Companion Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyStore$Companion;
+	public abstract fun aliases (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun containsAlias (Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun deleteEntry (Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun entryInstanceOf (Ljava/lang/String;Ljava/lang/Class;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun getCertificate (Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun getCertificateAlias (Ljava/security/cert/Certificate;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun getCertificateChain (Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun getCreationDate (Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun getEntry (Ljava/lang/String;Ljava/security/KeyStore$ProtectionParameter;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun getKey (Ljava/lang/String;[CLkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun getType (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun isCertificateEntry (Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun isKeyEntry (Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun setCertificateEntry (Ljava/lang/String;Ljava/security/cert/Certificate;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun setEntry (Ljava/lang/String;Ljava/security/KeyStore$Entry;Ljava/security/KeyStore$ProtectionParameter;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun setKeyEntry (Ljava/lang/String;Ljava/security/Key;[C[Ljava/security/cert/Certificate;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun setKeyEntry (Ljava/lang/String;[B[Ljava/security/cert/Certificate;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun size (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+}
+
+public final class io/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyStore$Companion {
+	public final fun invoke (Lkotlinx/coroutines/sync/Semaphore;)Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyStore;
+	public static synthetic fun invoke$default (Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyStore$Companion;Lkotlinx/coroutines/sync/Semaphore;ILjava/lang/Object;)Lio/github/ackeecz/guardian/core/keystore/android/SynchronizedAndroidKeyStore;
 }
 

core/build.gradle.kts

@@ -11,6 +11,8 @@ android {
 
 dependencies {
 
+    implementation(projects.coreInternal)
+
     implementation(libs.androidx.annotation)
 
     implementation(platform(libs.coroutines.bom))

core/src/main/kotlin/io/github/ackeecz/guardian/core/MasterKey.kt

@@ -21,18 +21,20 @@ package io.github.ackeecz.guardian.core
 import android.security.keystore.KeyGenParameterSpec
 import android.security.keystore.KeyProperties
 import androidx.annotation.VisibleForTesting
+import io.github.ackeecz.guardian.core.MasterKey.Companion.createSafeDefaultSpecBuilder
+import io.github.ackeecz.guardian.core.MasterKey.Companion.getOrCreate
+import io.github.ackeecz.guardian.core.keystore.android.AndroidKeyStoreSemaphore
+import io.github.ackeecz.guardian.core.keystore.android.SynchronizedAndroidKeyGenerator
+import io.github.ackeecz.guardian.core.keystore.android.SynchronizedAndroidKeyStore
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.Semaphore
 import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
 import java.security.GeneralSecurityException
 import java.security.KeyStore
 import java.security.ProviderException
-import javax.crypto.KeyGenerator
-import kotlin.also
-import kotlin.collections.contentEquals
-import kotlin.collections.contentToString
 
 private val keyMutex = Mutex()
 
@@ -42,6 +44,9 @@ private val keyMutex = Mutex()
  * by calling [getOrCreate] method and pass [KeyGenParameterSpec], which can be built manually or
  * by using [createSafeDefaultSpecBuilder] method for getting a [KeyGenParameterSpec.Builder]
  * configured with safe default parameters.
+ *
+ * Operations with Android [KeyStore] are synchronized using a provided [Semaphore]. More info about
+ * this topic can be found in [AndroidKeyStoreSemaphore] documentation.
  */
 public class MasterKey private constructor(public val alias: String) {
 
@@ -92,22 +97,29 @@ public class MasterKey private constructor(public val alias: String) {
         /**
          * Gets the [MasterKey]. If it does not exist yet, it is created with the provided
          * [keyGenParameterSpec].
+         *
+         * Android [KeyStore] operations are synchronized using [keyStoreSemaphore]. It is recommended
+         * to use a default [AndroidKeyStoreSemaphore], if you really don't need to provide a custom
+         * [Semaphore].
          */
+        @JvmOverloads
         public suspend fun getOrCreate(
             keyGenParameterSpec: KeyGenParameterSpec = createSafeDefaultSpecBuilder().build(),
+            keyStoreSemaphore: Semaphore = AndroidKeyStoreSemaphore,
         ): MasterKey {
-            return provider.getOrCreate(keyGenParameterSpec)
+            return provider.getOrCreate(keyGenParameterSpec, keyStoreSemaphore)
         }
     }
 
     @VisibleForTesting
     internal class Provider(private val defaultDispatcher: CoroutineDispatcher) {
 
-        private val keyStore = KeyStore.getInstance(ANDROID_KEY_STORE).also { it.load(null) }
-
-        suspend fun getOrCreate(keyGenParameterSpec: KeyGenParameterSpec): MasterKey {
+        suspend fun getOrCreate(
+            keyGenParameterSpec: KeyGenParameterSpec,
+            keyStoreSemaphore: Semaphore,
+        ): MasterKey {
             validate(keyGenParameterSpec)
-            generateKeyIfNeeded(keyGenParameterSpec)
+            generateKeyIfNeeded(keyGenParameterSpec, keyStoreSemaphore)
             return MasterKey(keyGenParameterSpec.keystoreAlias)
         }
 
@@ -129,24 +141,29 @@ public class MasterKey private constructor(public val alias: String) {
             }
         }
 
-        private suspend fun generateKeyIfNeeded(spec: KeyGenParameterSpec) {
-            fun keyDoesNotExist() = !keyStore.containsAlias(spec.keystoreAlias)
+        private suspend fun generateKeyIfNeeded(spec: KeyGenParameterSpec, keyStoreSemaphore: Semaphore) {
+            val keyStore = SynchronizedAndroidKeyStore(keyStoreSemaphore)
+
+            suspend fun keyDoesNotExist() = !keyStore.containsAlias(spec.keystoreAlias)
 
             if (keyDoesNotExist()) {
                 keyMutex.withLock {
                     if (keyDoesNotExist()) {
-                        generateKey(spec)
+                        generateKey(spec, keyStoreSemaphore)
                     }
                 }
             }
         }
 
-        private suspend fun generateKey(keyGenParameterSpec: KeyGenParameterSpec) {
+        private suspend fun generateKey(
+            keyGenParameterSpec: KeyGenParameterSpec,
+            keyStoreSemaphore: Semaphore,
+        ) {
             withContext(defaultDispatcher) {
                 try {
-                    val keyGenerator = KeyGenerator.getInstance(
+                    val keyGenerator = SynchronizedAndroidKeyGenerator.getInstance(
                         KeyProperties.KEY_ALGORITHM_AES,
-                        ANDROID_KEY_STORE,
+                        keyStoreSemaphore,
                     )
                     keyGenerator.init(keyGenParameterSpec)
                     keyGenerator.generateKey()
@@ -157,10 +174,5 @@ public class MasterKey private constructor(public val alias: String) {
                 }
             }
         }
-
-        companion object {
-
-            private const val ANDROID_KEY_STORE = "AndroidKeyStore"
-        }
     }
 }