Skip to content

Commit 48594b5

Browse files
CrisBarreiroaitorvs
CrisBarreiro
and
aitorvs
authored
FF removing synchronized implementations for encrypted shared preferences (#5962)
Task/Issue URL: https://app.asana.com/1/137249556945/project/1202552961248957/task/1210067151874996?focus=true ### Description ### Steps to test this PR * Smoke test autofill _Feature 1_ - [ ] - [ ] ### UI changes | Before | After | | ------ | ----- | !(Upload before screenshot)|(Upload after screenshot)| --------- Co-authored-by: Aitor Viana <[email protected]>
1 parent f0ff57c commit 48594b5

File tree

13 files changed

+320
-157
lines changed

13 files changed

+320
-157
lines changed

‎autofill/autofill-api/src/main/java/com/duckduckgo/autofill/api/AutofillFeature.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,4 +137,7 @@ interface AutofillFeature {
137137

138138
@Toggle.DefaultValue(defaultValue = false)
139139
fun siteSpecificFixes(): Toggle
140+
141+
@Toggle.DefaultValue(defaultValue = true)
142+
fun createAsyncPreferences(): Toggle
140143
}

‎autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/impl/securestorage/L2DataTransformer.kt

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,31 @@
1616

1717
package com.duckduckgo.autofill.impl.securestorage
1818

19+
import com.duckduckgo.app.di.AppCoroutineScope
1920
import com.duckduckgo.autofill.impl.securestorage.SecureStorageException.InternalSecureStorageException
2021
import com.duckduckgo.autofill.impl.securestorage.encryption.EncryptionHelper
2122
import com.duckduckgo.autofill.impl.securestorage.encryption.EncryptionHelper.EncryptedBytes
2223
import com.duckduckgo.autofill.impl.securestorage.encryption.EncryptionHelper.EncryptedString
24+
import com.duckduckgo.common.utils.DispatcherProvider
2325
import com.duckduckgo.di.scopes.AppScope
2426
import com.squareup.anvil.annotations.ContributesBinding
2527
import dagger.SingleInstanceIn
28+
import java.security.Key
2629
import javax.inject.Inject
30+
import kotlinx.coroutines.CoroutineScope
31+
import kotlinx.coroutines.Deferred
32+
import kotlinx.coroutines.async
2733
import okio.ByteString.Companion.decodeBase64
2834
import okio.ByteString.Companion.toByteString
2935

3036
interface L2DataTransformer {
31-
fun canProcessData(): Boolean
37+
suspend fun canProcessData(): Boolean
3238

3339
@Throws(SecureStorageException::class)
34-
fun encrypt(data: String): EncryptedString
40+
suspend fun encrypt(data: String): EncryptedString
3541

3642
@Throws(SecureStorageException::class)
37-
fun decrypt(
43+
suspend fun decrypt(
3844
data: String,
3945
iv: String,
4046
): String
@@ -45,31 +51,37 @@ interface L2DataTransformer {
4551
class RealL2DataTransformer @Inject constructor(
4652
private val encryptionHelper: EncryptionHelper,
4753
private val secureStorageKeyProvider: SecureStorageKeyProvider,
54+
@AppCoroutineScope private val appCoroutineScope: CoroutineScope,
55+
private val dispatcherProvider: DispatcherProvider,
4856
) : L2DataTransformer {
49-
private val l2Key by lazy {
50-
secureStorageKeyProvider.getl2Key()
57+
private val l2KeyDeferred: Deferred<Key> by lazy {
58+
appCoroutineScope.async(dispatcherProvider.io()) {
59+
secureStorageKeyProvider.getl2Key()
60+
}
5161
}
5262

53-
override fun canProcessData(): Boolean = secureStorageKeyProvider.canAccessKeyStore()
63+
suspend fun getL2Key(): Key = l2KeyDeferred.await()
64+
65+
override suspend fun canProcessData(): Boolean = secureStorageKeyProvider.canAccessKeyStore()
5466

5567
// get ByteArray -> encrypt -> encode to String
56-
override fun encrypt(data: String): EncryptedString = encryptionHelper.encrypt(data.toByteArray(), l2Key).run {
68+
override suspend fun encrypt(data: String): EncryptedString = encryptionHelper.encrypt(data.toByteArray(), getL2Key()).run {
5769
EncryptedString(
5870
this.data.transformToString(),
5971
this.iv.transformToString(),
6072
)
6173
}
6274

6375
// decode to ByteArray -> decrypt -> get String
64-
override fun decrypt(
76+
override suspend fun decrypt(
6577
data: String,
6678
iv: String,
6779
): String = encryptionHelper.decrypt(
6880
EncryptedBytes(
6981
data = data.transformToByteArray(),
7082
iv = iv.transformToByteArray(),
7183
),
72-
l2Key,
84+
getL2Key(),
7385
).run { String(this) }
7486

7587
private fun ByteArray.transformToString(): String = this.toByteString().base64()

‎autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/impl/securestorage/SecureStorage.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ class RealSecureStorage @Inject constructor(
291291
}
292292
}
293293

294-
private fun WebsiteLoginDetailsWithCredentials.toDataEntity(): WebsiteLoginCredentialsEntity {
294+
private suspend fun WebsiteLoginDetailsWithCredentials.toDataEntity(): WebsiteLoginCredentialsEntity {
295295
val encryptedPassword = encryptData(password)
296296
val encryptedNotes = encryptData(notes)
297297
return WebsiteLoginCredentialsEntity(
@@ -308,7 +308,7 @@ class RealSecureStorage @Inject constructor(
308308
)
309309
}
310310

311-
private fun WebsiteLoginCredentialsEntity.toCredentials(): WebsiteLoginDetailsWithCredentials =
311+
private suspend fun WebsiteLoginCredentialsEntity.toCredentials(): WebsiteLoginDetailsWithCredentials =
312312
WebsiteLoginDetailsWithCredentials(
313313
details = toDetails(),
314314
password = decryptData(password, passwordIv),
@@ -326,9 +326,9 @@ class RealSecureStorage @Inject constructor(
326326
)
327327

328328
// only encrypt when there's data
329-
private fun encryptData(data: String?): EncryptedString? = data?.let { l2DataTransformer.encrypt(it) }
329+
private suspend fun encryptData(data: String?): EncryptedString? = data?.let { l2DataTransformer.encrypt(it) }
330330

331-
private fun decryptData(
331+
private suspend fun decryptData(
332332
data: String?,
333333
iv: String?,
334334
): String? {

‎autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/impl/securestorage/SecureStorageDatabaseFactory.kt

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,28 +18,56 @@ package com.duckduckgo.autofill.impl.securestorage
1818

1919
import android.content.Context
2020
import androidx.room.Room
21+
import com.duckduckgo.autofill.api.AutofillFeature
2122
import com.duckduckgo.di.scopes.AppScope
2223
import com.duckduckgo.securestorage.store.db.ALL_MIGRATIONS
2324
import com.duckduckgo.securestorage.store.db.SecureStorageDatabase
2425
import com.squareup.anvil.annotations.ContributesBinding
2526
import dagger.SingleInstanceIn
2627
import javax.inject.Inject
28+
import kotlinx.coroutines.runBlocking
29+
import kotlinx.coroutines.sync.Mutex
30+
import kotlinx.coroutines.sync.withLock
2731
import net.sqlcipher.database.SupportFactory
2832

2933
interface SecureStorageDatabaseFactory {
30-
fun getDatabase(): SecureStorageDatabase?
34+
suspend fun getDatabase(): SecureStorageDatabase?
3135
}
3236

3337
@SingleInstanceIn(AppScope::class)
3438
@ContributesBinding(AppScope::class)
3539
class RealSecureStorageDatabaseFactory @Inject constructor(
3640
private val context: Context,
3741
private val keyProvider: SecureStorageKeyProvider,
42+
private val autofillFeature: AutofillFeature,
3843
) : SecureStorageDatabaseFactory {
3944
private var _database: SecureStorageDatabase? = null
4045

46+
private val mutex = Mutex()
47+
48+
override suspend fun getDatabase(): SecureStorageDatabase? {
49+
return if (autofillFeature.createAsyncPreferences().isEnabled()) {
50+
getAsyncDatabase()
51+
} else {
52+
getDatabaseSynchronized()
53+
}
54+
}
55+
4156
@Synchronized
42-
override fun getDatabase(): SecureStorageDatabase? {
57+
private fun getDatabaseSynchronized(): SecureStorageDatabase? {
58+
return runBlocking {
59+
getInnerDatabase()
60+
}
61+
}
62+
63+
private suspend fun getAsyncDatabase(): SecureStorageDatabase? {
64+
_database?.let { return it }
65+
return mutex.withLock {
66+
getInnerDatabase()
67+
}
68+
}
69+
70+
private suspend fun getInnerDatabase(): SecureStorageDatabase? {
4371
// If we have already the DB instance then let's use it
4472
if (_database != null) {
4573
return _database

‎autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/impl/securestorage/SecureStorageKeyProvider.kt

Lines changed: 70 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.duckduckgo.autofill.impl.securestorage
1818

19+
import com.duckduckgo.autofill.api.AutofillFeature
1920
import com.duckduckgo.autofill.impl.securestorage.encryption.EncryptionHelper
2021
import com.duckduckgo.autofill.impl.securestorage.encryption.EncryptionHelper.EncryptedBytes
2122
import com.duckduckgo.di.scopes.AppScope
@@ -24,23 +25,26 @@ import com.duckduckgo.securestorage.store.SecureStorageKeyRepository
2425
import com.squareup.anvil.annotations.ContributesBinding
2526
import java.security.Key
2627
import javax.inject.Inject
28+
import kotlinx.coroutines.runBlocking
29+
import kotlinx.coroutines.sync.Mutex
30+
import kotlinx.coroutines.sync.withLock
2731
import okio.ByteString.Companion.toByteString
2832

2933
/**
3034
* This class provides the usable decrypted keys to be used in various levels on encryption
3135
*/
3236
interface SecureStorageKeyProvider {
33-
fun canAccessKeyStore(): Boolean
37+
suspend fun canAccessKeyStore(): Boolean
3438

3539
/**
3640
* Ready to use key for L1 encryption
3741
*/
38-
fun getl1Key(): ByteArray
42+
suspend fun getl1Key(): ByteArray
3943

4044
/**
4145
* Ready to use key for L2 encryption using the generated user password
4246
*/
43-
fun getl2Key(): Key
47+
suspend fun getl2Key(): Key
4448
}
4549

4650
@ContributesBinding(AppScope::class)
@@ -49,73 +53,116 @@ class RealSecureStorageKeyProvider @Inject constructor(
4953
private val secureStorageKeyRepository: SecureStorageKeyRepository,
5054
private val encryptionHelper: EncryptionHelper,
5155
private val secureStorageKeyGenerator: SecureStorageKeyGenerator,
56+
private val autofillFeature: AutofillFeature,
5257
) : SecureStorageKeyProvider {
5358

54-
override fun canAccessKeyStore(): Boolean = secureStorageKeyRepository.canUseEncryption()
59+
override suspend fun canAccessKeyStore(): Boolean = secureStorageKeyRepository.canUseEncryption()
60+
private val l1KeyMutex = Mutex()
61+
private val l2KeyMutex = Mutex()
62+
63+
override suspend fun getl1Key(): ByteArray {
64+
if (autofillFeature.createAsyncPreferences().isEnabled()) {
65+
return getl1KeyAsync()
66+
} else {
67+
return getl1KeySync()
68+
}
69+
}
70+
71+
private suspend fun getl1KeyAsync(): ByteArray {
72+
l1KeyMutex.withLock {
73+
return innerGetL1Key()
74+
}
75+
}
5576

5677
@Synchronized
57-
override fun getl1Key(): ByteArray {
78+
private fun getl1KeySync(): ByteArray {
79+
return runBlocking {
80+
innerGetL1Key()
81+
}
82+
}
83+
84+
private suspend fun innerGetL1Key(): ByteArray {
5885
// If no key exists in the keystore, we generate a new one and store it
59-
return if (secureStorageKeyRepository.l1Key == null) {
86+
return if (secureStorageKeyRepository.getL1Key() == null) {
6087
randomBytesGenerator.generateBytes(L1_PASSPHRASE_SIZE).also {
61-
secureStorageKeyRepository.l1Key = it
88+
secureStorageKeyRepository.setL1Key(it)
6289
}
6390
} else {
64-
secureStorageKeyRepository.l1Key!!
91+
secureStorageKeyRepository.getL1Key()!!
92+
}
93+
}
94+
95+
override suspend fun getl2Key(): Key {
96+
if (autofillFeature.createAsyncPreferences().isEnabled()) {
97+
return getl2KeyAsync()
98+
} else {
99+
return getl2KeySync()
100+
}
101+
}
102+
103+
private suspend fun getl2KeyAsync(): Key {
104+
return l2KeyMutex.withLock {
105+
innerGetL2Key()
65106
}
66107
}
67108

68109
@Synchronized
69-
override fun getl2Key(): Key {
70-
val userPassword = if (secureStorageKeyRepository.password == null) {
110+
private fun getl2KeySync(): Key {
111+
return runBlocking {
112+
innerGetL2Key()
113+
}
114+
}
115+
116+
private suspend fun innerGetL2Key(): Key {
117+
val userPassword = if (secureStorageKeyRepository.getPassword() == null) {
71118
randomBytesGenerator.generateBytes(PASSWORD_SIZE).also {
72-
secureStorageKeyRepository.password = it
119+
secureStorageKeyRepository.setPassword(it)
73120
}
74121
} else {
75-
secureStorageKeyRepository.password
122+
secureStorageKeyRepository.getPassword()
76123
}
77124

78125
return getl2Key(userPassword!!.toByteString().base64())
79126
}
80127

81-
private fun getl2Key(password: String): Key {
82-
val keyMaterial = if (secureStorageKeyRepository.encryptedL2Key == null) {
128+
private suspend fun getl2Key(password: String): Key {
129+
val keyMaterial = if (secureStorageKeyRepository.getEncryptedL2Key() == null) {
83130
secureStorageKeyGenerator.generateKey().encoded.also {
84131
encryptAndStoreL2Key(it, password)
85132
}
86133
} else {
87134
encryptionHelper.decrypt(
88135
EncryptedBytes(
89-
secureStorageKeyRepository.encryptedL2Key!!,
90-
secureStorageKeyRepository.encryptedL2KeyIV!!,
136+
secureStorageKeyRepository.getEncryptedL2Key()!!,
137+
secureStorageKeyRepository.getEncryptedL2KeyIV()!!,
91138
),
92139
deriveKeyFromPassword(password),
93140
)
94141
}
95142
return secureStorageKeyGenerator.generateKeyFromKeyMaterial(keyMaterial)
96143
}
97144

98-
private fun encryptAndStoreL2Key(
145+
private suspend fun encryptAndStoreL2Key(
99146
keyBytes: ByteArray,
100147
password: String,
101148
): ByteArray =
102149
encryptionHelper.encrypt(
103150
keyBytes,
104151
deriveKeyFromPassword(password),
105152
).also {
106-
secureStorageKeyRepository.encryptedL2Key = it.data
107-
secureStorageKeyRepository.encryptedL2KeyIV = it.iv
153+
secureStorageKeyRepository.setEncryptedL2Key(it.data)
154+
secureStorageKeyRepository.setEncryptedL2KeyIV(it.iv)
108155
}.data
109156

110-
private fun getPasswordSalt() = if (secureStorageKeyRepository.passwordSalt == null) {
157+
private suspend fun getPasswordSalt() = if (secureStorageKeyRepository.getPasswordSalt() == null) {
111158
randomBytesGenerator.generateBytes(PASSWORD_KEY_SALT_SIZE).also {
112-
secureStorageKeyRepository.passwordSalt = it
159+
secureStorageKeyRepository.setPasswordSalt(it)
113160
}
114161
} else {
115-
secureStorageKeyRepository.passwordSalt!!
162+
secureStorageKeyRepository.getPasswordSalt()!!
116163
}
117164

118-
private fun deriveKeyFromPassword(password: String) =
165+
private suspend fun deriveKeyFromPassword(password: String) =
119166
secureStorageKeyGenerator.generateKeyFromPassword(password, getPasswordSalt())
120167

121168
companion object {

‎autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/impl/securestorage/di/SecureStorageModule.kt

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@
1717
package com.duckduckgo.autofill.impl.securestorage.di
1818

1919
import android.content.Context
20+
import com.duckduckgo.app.di.AppCoroutineScope
21+
import com.duckduckgo.autofill.api.AutofillFeature
2022
import com.duckduckgo.autofill.impl.securestorage.DerivedKeySecretFactory
2123
import com.duckduckgo.autofill.impl.securestorage.RealDerivedKeySecretFactory
24+
import com.duckduckgo.common.utils.DispatcherProvider
2225
import com.duckduckgo.di.scopes.AppScope
2326
import com.duckduckgo.securestorage.store.RealSecureStorageKeyRepository
2427
import com.duckduckgo.securestorage.store.SecureStorageKeyRepository
@@ -28,15 +31,23 @@ import dagger.Module
2831
import dagger.Provides
2932
import dagger.SingleInstanceIn
3033
import javax.inject.Named
34+
import kotlinx.coroutines.CoroutineScope
3135

3236
@Module
3337
@ContributesTo(AppScope::class)
3438
object SecureStorageModule {
3539

3640
@Provides
3741
@SingleInstanceIn(AppScope::class)
38-
fun providesSecureStorageKeyStore(context: Context): SecureStorageKeyRepository =
39-
RealSecureStorageKeyRepository(RealSecureStorageKeyStore(context))
42+
fun providesSecureStorageKeyStore(
43+
context: Context,
44+
@AppCoroutineScope coroutineScope: CoroutineScope,
45+
dispatcherProvider: DispatcherProvider,
46+
autofillFeature: AutofillFeature,
47+
): SecureStorageKeyRepository =
48+
RealSecureStorageKeyRepository(
49+
RealSecureStorageKeyStore(context, coroutineScope, dispatcherProvider, autofillFeature),
50+
)
4051
}
4152

4253
@Module

0 commit comments

Comments
 (0)