Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix encrypted storage bug #75

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ apply plugin: 'com.android.application'
apply plugin: 'kotlin-android'

android {
compileSdkVersion 32
compileSdkVersion 33

defaultConfig {
applicationId "com.schibsted.account"
minSdkVersion 21
targetSdkVersion 30
targetSdkVersion 31
versionCode 1
versionName "1.0"

Expand Down
4 changes: 2 additions & 2 deletions webflows/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ apply plugin: 'maven-publish'
apply plugin: 'signing'

android {
compileSdkVersion 32
compileSdkVersion 33

defaultConfig {
minSdkVersion 21
Expand Down Expand Up @@ -52,7 +52,7 @@ dependencies {
implementation 'org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4'
implementation 'androidx.core:core-ktx:1.8.0'
implementation 'androidx.appcompat:appcompat:1.5.0'
implementation 'androidx.security:security-crypto:1.1.0-alpha03'
implementation 'androidx.security:security-crypto:1.1.0-alpha06'

implementation "com.squareup.retrofit2:retrofit:2.9.0"
implementation "com.squareup.retrofit2:converter-gson:2.9.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.schibsted.account.webflows.persistence

import android.content.Context
import android.content.SharedPreferences
import android.os.Build
import androidx.security.crypto.EncryptedSharedPreferences
import androidx.security.crypto.MasterKey
import com.google.gson.Gson
Expand All @@ -11,6 +12,7 @@ import com.schibsted.account.webflows.user.StoredUserSession
import com.schibsted.account.webflows.util.Either
import timber.log.Timber
import java.security.GeneralSecurityException
import java.security.KeyStore

internal typealias StorageReadCallback = (Either<StorageError, StoredUserSession?>) -> Unit

Expand All @@ -28,27 +30,10 @@ internal interface SessionStorage {

internal class EncryptedSharedPrefsStorage(context: Context) : SessionStorage {
private val gson = GsonBuilder().setDateFormat("MM dd, yyyy HH:mm:ss").create()
private val appContext: Context = context.applicationContext
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a fan of having to keep reference to app context here, but alternative was putting context as parameter in get, and that would cause a snowball of changes that wouldn't really make sense


private val prefs: SharedPreferences by lazy {
val masterKey = MasterKey.Builder(context.applicationContext)
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
.build()

try {
EncryptedSharedPreferences.create(
context.applicationContext,
PREFERENCE_FILENAME,
masterKey,
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
)
} catch (e: GeneralSecurityException) {
Timber.e(
"Error occurred while trying to build encrypted shared preferences",
e
)
throw e
}
createEncryptedSharedPrefDestructively(context.applicationContext)
}

override fun save(session: StoredUserSession) {
Expand Down Expand Up @@ -80,15 +65,8 @@ internal class EncryptedSharedPrefsStorage(context: Context) : SessionStorage {
"Error occurred while trying to read from encrypted shared preferences",
e
)
callback(Either.Left(StorageError.UnexpectedError(e)))
}
}

private fun Gson.getStoredUserSession(json: String): StoredUserSession? {
return try {
fromJson(json, StoredUserSession::class.java)
} catch (e: JsonSyntaxException) {
null
createEncryptedSharedPrefDestructively(this.appContext)
}
}

Expand All @@ -105,6 +83,70 @@ internal class EncryptedSharedPrefsStorage(context: Context) : SessionStorage {
}
}

private fun createEncryptedSharedPrefDestructively(context: Context): SharedPreferences {
try {
return createEncryptedSharedPref(context)
} catch (e: GeneralSecurityException) {
Timber.e(
"Error occurred while trying to build encrypted shared preferences. Cleaning corrupted data",
e
)
deleteMasterKeyEntry()
deleteExistingPref(context)
}

try {
return createEncryptedSharedPref(context)
} catch (e: GeneralSecurityException) {
Timber.e(
"Encrypted shared preferences could not be created",
e
)

throw e
}
}

private fun deleteExistingPref(context: Context) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
context.deleteSharedPreferences(PREFERENCE_FILENAME)
} else {
context.getSharedPreferences(PREFERENCE_FILENAME, Context.MODE_PRIVATE)
.edit()
.clear()
.commit()
}
}

private fun deleteMasterKeyEntry() {
KeyStore.getInstance("AndroidKeyStore").apply {
load(null)
deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS)
}
}

private fun createEncryptedSharedPref(context: Context): SharedPreferences {
val masterKey = MasterKey.Builder(context)
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
.build()

return EncryptedSharedPreferences.create(
context,
PREFERENCE_FILENAME,
masterKey,
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
)
}

private fun Gson.getStoredUserSession(json: String): StoredUserSession? {
return try {
fromJson(json, StoredUserSession::class.java)
} catch (e: JsonSyntaxException) {
null
}
}

companion object {
const val PREFERENCE_FILENAME = "SCHACC_TOKENS"
}
Expand Down