Skip to content

Commit 1ad0659

Browse files
kablizkabliz
committed
[APT-10658] Better EncryptedSharedPreference Resilience
The earlier failsafe does not protect against the app crashing on startup. This time, the exception is caught and the SecureStorage can handle the SharedPref being null.
1 parent 9e70e2b commit 1ad0659

File tree

6 files changed

+130
-59
lines changed

6 files changed

+130
-59
lines changed

‎Armadillo/src/main/java/com/scribd/armadillo/Constants.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ object Constants {
3030
internal object Keys {
3131
const val KEY_ARMADILLO_CONFIG = "armadillo_config"
3232
const val KEY_AUDIO_PLAYABLE = "audio_playable"
33+
const val ANDROID_KEYSTORE_NAME= "AndroidKeyStore"
3334
}
3435

3536
internal object DI {
@@ -41,6 +42,11 @@ object Constants {
4142

4243
const val GLOBAL_SCOPE = "global_scope"
4344

45+
const val DOWNLOAD_STORE_ALIAS="armadillo"
46+
const val DOWNLOAD_STORE_FILENAME="armadillo.download.secure"
47+
const val STANDARD_STORE_ALIAS="armadilloStandard"
48+
const val STANDARD_STORE_FILENAME="armadillo.standard.secure"
49+
4450
const val STANDARD_STORAGE = "standard_storage"
4551
const val STANDARD_SECURE_STORAGE = "standard_secure_storage"
4652
const val DRM_DOWNLOAD_STORAGE = "drm_download_storage"

‎Armadillo/src/main/java/com/scribd/armadillo/di/DownloadModule.kt‎

Lines changed: 13 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,6 @@ package com.scribd.armadillo.di
22

33
import android.content.Context
44
import android.content.SharedPreferences
5-
import android.security.keystore.KeyGenParameterSpec
6-
import android.security.keystore.KeyProperties.BLOCK_MODE_GCM
7-
import android.security.keystore.KeyProperties.ENCRYPTION_PADDING_NONE
8-
import android.security.keystore.KeyProperties.PURPOSE_DECRYPT
9-
import android.security.keystore.KeyProperties.PURPOSE_ENCRYPT
10-
import androidx.security.crypto.EncryptedSharedPreferences
11-
import androidx.security.crypto.MasterKeys
125
import com.google.android.exoplayer2.offline.DownloadManager
136
import com.google.android.exoplayer2.offline.DownloadService
147
import com.google.android.exoplayer2.offline.DownloaderFactory
@@ -17,6 +10,10 @@ import com.google.android.exoplayer2.upstream.cache.Cache
1710
import com.google.android.exoplayer2.upstream.cache.NoOpCacheEvictor
1811
import com.google.android.exoplayer2.upstream.cache.SimpleCache
1912
import com.scribd.armadillo.Constants
13+
import com.scribd.armadillo.Constants.DI.DOWNLOAD_STORE_ALIAS
14+
import com.scribd.armadillo.Constants.DI.DOWNLOAD_STORE_FILENAME
15+
import com.scribd.armadillo.Constants.DI.STANDARD_STORE_ALIAS
16+
import com.scribd.armadillo.Constants.DI.STANDARD_STORE_FILENAME
2017
import com.scribd.armadillo.download.ArmadilloDatabaseProvider
2118
import com.scribd.armadillo.download.ArmadilloDatabaseProviderImpl
2219
import com.scribd.armadillo.download.ArmadilloDownloadManagerFactory
@@ -35,10 +32,10 @@ import com.scribd.armadillo.encryption.ExoplayerEncryption
3532
import com.scribd.armadillo.encryption.ExoplayerEncryptionImpl
3633
import com.scribd.armadillo.encryption.SecureStorage
3734
import com.scribd.armadillo.exoplayerExternalDirectory
35+
import com.scribd.armadillo.extensions.createEncryptedSharedPrefKeyStoreWithRetry
3836
import dagger.Module
3937
import dagger.Provides
4038
import java.io.File
41-
import java.security.KeyStore
4239
import javax.inject.Named
4340
import javax.inject.Qualifier
4441
import javax.inject.Singleton
@@ -122,46 +119,19 @@ internal class DownloadModule {
122119
@Singleton
123120
@Provides
124121
@Named(Constants.DI.STANDARD_SECURE_STORAGE)
125-
fun standardSecureStorage(context: Context): SharedPreferences {
126-
val keystoreAlias = "armadilloStandard"
127-
val fileName = "armadillo.standard.secure"
128-
return createEncryptedSharedPrefsKeyStore(context = context, fileName = fileName, keystoreAlias = keystoreAlias)
122+
fun standardSecureStorage(context: Context): SharedPreferences? {
123+
val keystoreAlias = STANDARD_STORE_ALIAS
124+
val fileName = STANDARD_STORE_FILENAME
125+
return createEncryptedSharedPrefKeyStoreWithRetry(context = context, fileName = fileName, keystoreAlias = keystoreAlias)
129126
}
130127

131128
@Singleton
132129
@Provides
133130
@Named(Constants.DI.DRM_SECURE_STORAGE)
134-
fun drmSecureStorage(context: Context): SharedPreferences {
135-
val keystoreAlias = "armadillo"
136-
val fileName = "armadillo.download.secure"
137-
return createEncryptedSharedPrefsKeyStore(context = context, fileName = fileName, keystoreAlias = keystoreAlias)
138-
}
139-
140-
private fun createEncryptedSharedPrefsKeyStore(context: Context, fileName: String, keystoreAlias: String)
141-
: SharedPreferences {
142-
val keySpec = KeyGenParameterSpec.Builder(keystoreAlias, PURPOSE_ENCRYPT or PURPOSE_DECRYPT)
143-
.setKeySize(256)
144-
.setBlockModes(BLOCK_MODE_GCM)
145-
.setEncryptionPaddings(ENCRYPTION_PADDING_NONE)
146-
.build()
147-
148-
val keys = try {
149-
MasterKeys.getOrCreate(keySpec)
150-
} catch (ex: Exception) {
151-
//clear corrupted store, contents will be lost
152-
val keyStore = KeyStore.getInstance("AndroidKeyStore")
153-
keyStore.load(null)
154-
keyStore.deleteEntry(keystoreAlias)
155-
context.getSharedPreferences(fileName, Context.MODE_PRIVATE).edit().clear().apply()
156-
MasterKeys.getOrCreate(keySpec)
157-
}
158-
return EncryptedSharedPreferences.create(
159-
fileName,
160-
keys,
161-
context,
162-
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
163-
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
164-
)
131+
fun drmSecureStorage(context: Context): SharedPreferences? {
132+
val keystoreAlias = DOWNLOAD_STORE_ALIAS
133+
val fileName = DOWNLOAD_STORE_FILENAME
134+
return createEncryptedSharedPrefKeyStoreWithRetry(context = context, fileName = fileName, keystoreAlias = keystoreAlias)
165135
}
166136

167137
@Singleton

‎Armadillo/src/main/java/com/scribd/armadillo/encryption/SecureStorage.kt‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ internal interface SecureStorage {
2828
@Singleton
2929
internal class ArmadilloSecureStorage @Inject constructor(
3030
@Named(Constants.DI.STANDARD_STORAGE) private val legacyStandardStorage: SharedPreferences,
31-
@Named(Constants.DI.STANDARD_SECURE_STORAGE) private val secureStandardStorage: SharedPreferences,
31+
@Named(Constants.DI.STANDARD_SECURE_STORAGE) private val secureStandardStorage: SharedPreferences?,
3232
@Named(Constants.DI.DRM_DOWNLOAD_STORAGE) private val legacyDrmStorage: SharedPreferences,
33-
@Named(Constants.DI.DRM_SECURE_STORAGE) private val secureDrmStorage: SharedPreferences
33+
@Named(Constants.DI.DRM_SECURE_STORAGE) private val secureDrmStorage: SharedPreferences?
3434
) : SecureStorage {
3535
companion object {
3636
const val DOWNLOAD_KEY = "download_key"
@@ -41,26 +41,31 @@ internal class ArmadilloSecureStorage @Inject constructor(
4141
}
4242

4343
override fun downloadSecretKey(context: Context): ByteArray {
44-
return if (secureStandardStorage.contains(DOWNLOAD_KEY)) {
44+
return if (secureStandardStorage?.contains(DOWNLOAD_KEY) == true) {
4545
val storedKey = secureStandardStorage.getString(DOWNLOAD_KEY, DEFAULT) ?: DEFAULT
4646
if (storedKey == DEFAULT) {
4747
Log.e(TAG, "Storage Is Out of Alignment")
4848
}
4949
storedKey.toSecretByteArray
50-
} else if(legacyStandardStorage.contains(DOWNLOAD_KEY)) {
50+
} else if (legacyStandardStorage.contains(DOWNLOAD_KEY)) {
5151
//migrate to secured version
5252
val storedKey = legacyStandardStorage.getString(DOWNLOAD_KEY, DEFAULT) ?: DEFAULT
5353
if (storedKey == DEFAULT) {
5454
Log.e(TAG, "Storage Is Out of Alignment")
5555
}
56-
secureStandardStorage.edit().putString(DOWNLOAD_KEY, storedKey).apply()
57-
legacyStandardStorage.edit().remove(DOWNLOAD_KEY).apply()
56+
if (secureStandardStorage != null) {
57+
secureStandardStorage.edit().putString(DOWNLOAD_KEY, storedKey).apply()
58+
legacyStandardStorage.edit().remove(DOWNLOAD_KEY).apply()
59+
}
5860
storedKey.toSecretByteArray
59-
} else {
61+
} else if (secureStandardStorage != null) {
6062
//no key exists anywhere yet
6163
createRandomString().also {
6264
secureStandardStorage.edit().putString(DOWNLOAD_KEY, it).apply()
6365
}.toSecretByteArray
66+
} else {
67+
"".toSecretByteArray
68+
//we've attempted to create 2 sharedPrefs by this point, so this shouldn't happen. Let exoplayer fail to decrypt
6469
}
6570
}
6671

@@ -73,28 +78,30 @@ internal class ArmadilloSecureStorage @Inject constructor(
7378
override fun saveDrmDownload(context: Context, id: String, drmDownload: DrmDownload) {
7479
val alias = getDrmDownloadAlias(id, drmDownload.drmType)
7580
val value = Base64.encodeToString(Json.encodeToString(drmDownload).toByteArray(StandardCharsets.UTF_8), Base64.NO_WRAP)
76-
secureDrmStorage.edit().putString(alias, value).apply()
81+
secureDrmStorage?.edit()?.putString(alias, value)?.apply()
7782
}
7883

7984
override fun getDrmDownload(context: Context, id: String, drmType: DrmType): DrmDownload? {
8085
val alias = getDrmDownloadAlias(id, drmType)
81-
var download = secureDrmStorage.getString(alias, null)?.decodeToDrmDownload()
86+
var download = secureDrmStorage?.getString(alias, null)?.decodeToDrmDownload()
8287
if (download == null && legacyDrmStorage.contains(alias)) {
8388
//migrate old storage to secure storage
8489
val downloadValue = legacyDrmStorage.getString(alias, null)
8590
download = downloadValue?.decodeToDrmDownload()
86-
secureDrmStorage.edit().putString(alias, downloadValue).apply()
87-
legacyDrmStorage.edit().remove(alias).apply()
91+
if (secureDrmStorage != null) {
92+
secureDrmStorage.edit().putString(alias, downloadValue).apply()
93+
legacyDrmStorage.edit().remove(alias).apply()
94+
}
8895
}
8996
return download
9097
}
9198

9299
override fun getAllDrmDownloads(context: Context): Map<String, DrmDownload> {
93-
val drmDownloads = secureDrmStorage.all.keys.mapNotNull { alias ->
100+
val drmDownloads = secureDrmStorage?.all?.keys?.mapNotNull { alias ->
94101
secureDrmStorage.getString(alias, null)?.let { drmResult ->
95102
alias to drmResult.decodeToDrmDownload()
96103
}
97-
}.toMap()
104+
}?.toMap() ?: emptyMap()
98105
val legacyDownloads = legacyDrmStorage.all.keys.mapNotNull { alias ->
99106
legacyDrmStorage.getString(alias, null)?.let { drmResult ->
100107
alias to drmResult.decodeToDrmDownload()
@@ -107,12 +114,12 @@ internal class ArmadilloSecureStorage @Inject constructor(
107114
override fun removeDrmDownload(context: Context, id: String, drmType: DrmType) {
108115
val alias = getDrmDownloadAlias(id, drmType)
109116
legacyDrmStorage.edit().remove(alias).apply()
110-
secureDrmStorage.edit().remove(alias).apply()
117+
secureDrmStorage?.edit()?.remove(alias)?.apply()
111118
}
112119

113120
override fun removeDrmDownload(context: Context, key: String) {
114121
legacyDrmStorage.edit().remove(key).apply()
115-
secureDrmStorage.edit().remove(key).apply()
122+
secureDrmStorage?.edit()?.remove(key)?.apply()
116123
}
117124

118125
private val String.toSecretByteArray: ByteArray

‎Armadillo/src/main/java/com/scribd/armadillo/extensions/SharedPrefExt.kt‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
package com.scribd.armadillo.extensions
2+
3+
import android.content.Context
4+
import android.content.SharedPreferences
5+
import android.security.keystore.KeyGenParameterSpec
6+
import android.security.keystore.KeyProperties.BLOCK_MODE_GCM
7+
import android.security.keystore.KeyProperties.ENCRYPTION_PADDING_NONE
8+
import android.security.keystore.KeyProperties.PURPOSE_DECRYPT
9+
import android.security.keystore.KeyProperties.PURPOSE_ENCRYPT
10+
import android.util.Log
11+
import androidx.security.crypto.EncryptedSharedPreferences
12+
import androidx.security.crypto.MasterKeys
13+
import com.scribd.armadillo.Constants.DI.STANDARD_STORE_FILENAME
14+
import com.scribd.armadillo.Constants.Keys.ANDROID_KEYSTORE_NAME
15+
import java.io.File
16+
import java.security.KeyStore
17+
18+
fun SharedPreferences.deleteSharedPreference(context: Context, filename: String, keystoreAlias: String) {
19+
val tag = "DeletingSharedPrefs"
20+
try {
21+
val sharedPrefsFile = File(
22+
(context.filesDir.getParent()?.plus("/shared_prefs/")) + filename + ".xml"
23+
)
24+
25+
edit().clear().commit()
26+
27+
if (sharedPrefsFile.exists()) {
28+
val deleted = sharedPrefsFile.delete()
29+
Log.d(tag, "resetStorage() Shared prefs file deleted: $deleted; path: ${sharedPrefsFile.absolutePath}")
30+
} else {
31+
Log.d(tag,"resetStorage() Shared prefs file non-existent; path: ${sharedPrefsFile.absolutePath}")
32+
}
33+
34+
val keyStore = KeyStore.getInstance(ANDROID_KEYSTORE_NAME)
35+
keyStore.load(null)
36+
keyStore.deleteEntry(keystoreAlias)
37+
} catch (e: Exception) {
38+
Log.e(tag, "Error occurred while trying to reset shared prefs", e)
39+
}
40+
}
41+
42+
fun createEncryptedSharedPrefKeyStoreWithRetry(context: Context, fileName: String, keystoreAlias: String): SharedPreferences? {
43+
val firstAttempt = createEncryptedSharedPrefsKeyStore(context = context, fileName = fileName, keystoreAlias = keystoreAlias)
44+
return if(firstAttempt != null) {
45+
firstAttempt
46+
} else {
47+
context.getSharedPreferences(fileName, Context.MODE_PRIVATE).deleteSharedPreference(
48+
context = context,
49+
filename = fileName,
50+
keystoreAlias = keystoreAlias
51+
)
52+
createEncryptedSharedPrefsKeyStore(context = context, fileName = fileName, keystoreAlias = keystoreAlias)
53+
}
54+
}
55+
56+
fun createEncryptedSharedPrefsKeyStore(context: Context, fileName: String, keystoreAlias: String)
57+
: SharedPreferences? {
58+
val keySpec = KeyGenParameterSpec.Builder(keystoreAlias, PURPOSE_ENCRYPT or PURPOSE_DECRYPT)
59+
.setKeySize(256)
60+
.setBlockModes(BLOCK_MODE_GCM)
61+
.setEncryptionPaddings(ENCRYPTION_PADDING_NONE)
62+
.build()
63+
64+
val keys = try {
65+
MasterKeys.getOrCreate(keySpec)
66+
} catch (ex: Exception) {
67+
//clear corrupted store, contents will be lost
68+
context.getSharedPreferences(fileName, Context.MODE_PRIVATE).deleteSharedPreference(
69+
context = context,
70+
filename = fileName,
71+
keystoreAlias = keystoreAlias )
72+
MasterKeys.getOrCreate(keySpec)
73+
}
74+
return try {
75+
EncryptedSharedPreferences.create(
76+
fileName,
77+
keys,
78+
context,
79+
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
80+
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
81+
)
82+
} catch(ex: Exception) {
83+
null
84+
}
85+
}

‎RELEASE.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# Project Armadillo Release Notes
22

3+
## 1.6.8
4+
- Fixes an app startup crash to EncryptedSharedPreference faults.
5+
36
## 1.6.7
47
- Adds additional data in audio player errors: HttpResponseCodeException, DownloadFailed
58
- Add new ParsingException for internal ParserException

‎gradle.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ org.gradle.jvmargs=-Xmx1536m
1313
# org.gradle.parallel=true
1414
PACKAGE_NAME=com.scribd.armadillo
1515
GRADLE_PLUGIN_VERSION=7.2.0
16-
LIBRARY_VERSION=1.6.7
16+
LIBRARY_VERSION=1.6.8
1717
EXOPLAYER_VERSION=2.19.1
1818
RXJAVA_VERSION=2.2.4
1919
RXANDROID_VERSION=2.0.1

0 commit comments

Comments
 (0)