From 34eb8e1fecce45ddfe19c836aa7253a8f695f3cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20M=C3=B6ller?= <jakob.moeller@sap.com> Date: Sun, 7 Jun 2020 10:52:41 +0200 Subject: [PATCH] Implement passphrase generation for db (closes #173) (#224) (#233) * Implement passphrase generation for db (closes #173) (#224) * Add failing tests (#173) * Fix AppDatabase instance not reset when method called (#173) * Implement passphrase generation and storage/retrieval of passphrase by encrypted shared preferences (#173) * Setup Retention while maintaining the DB Password Signed-off-by: d067928 <jakob.moeller@sap.com> * Setup Retention while maintaining the DB Password Signed-off-by: d067928 <jakob.moeller@sap.com> * Lint Signed-off-by: d067928 <jakob.moeller@sap.com> Co-authored-by: Andreas Schattney <a.schattney@gmail.com> Co-authored-by: Philipp Woessner <64482866+pwoessner@users.noreply.github.com> --- .../util/security/DBPasswordTest.kt | 132 ++++++++++++++++++ .../coronawarnapp/TestRiskLevelCalculation.kt | 3 +- .../rki/coronawarnapp/storage/AppDatabase.kt | 18 ++- .../coronawarnapp/util/DataRetentionHelper.kt | 10 +- .../util/security/SecurityHelper.kt | 100 +++++++------ 5 files changed, 210 insertions(+), 53 deletions(-) create mode 100644 Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/DBPasswordTest.kt diff --git a/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/DBPasswordTest.kt b/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/DBPasswordTest.kt new file mode 100644 index 000000000..bd9025f62 --- /dev/null +++ b/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/DBPasswordTest.kt @@ -0,0 +1,132 @@ +/****************************************************************************** + * Corona-Warn-App * + * * + * SAP SE and all other contributors / * + * copyright owners license this file to you under the Apache * + * License, Version 2.0 (the "License"); you may not use this * + * file except in compliance with the License. * + * You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ******************************************************************************/ + +package de.rki.coronawarnapp.util.security + +import android.content.Context +import android.database.sqlite.SQLiteDatabase +import androidx.test.core.app.ApplicationProvider +import de.rki.coronawarnapp.storage.AppDatabase +import de.rki.coronawarnapp.storage.DATABASE_NAME +import de.rki.coronawarnapp.storage.keycache.KeyCacheEntity +import kotlinx.coroutines.runBlocking +import net.sqlcipher.database.SQLiteException +import org.hamcrest.Matchers.equalTo +import org.hamcrest.Matchers.not +import org.junit.Assert.assertThat +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import java.io.File +import java.util.UUID +import kotlin.random.Random + +@RunWith(JUnit4::class) +class DBPasswordTest { + + private val appContext: Context + get() = ApplicationProvider.getApplicationContext() + + private val db: AppDatabase + get() = AppDatabase.getInstance(appContext) + + @Before + fun setUp() { + clearSharedPreferences() + AppDatabase.reset(appContext) + } + + @Test + fun generatesPassphraseInCorrectLength() { + val passphrase = SecurityHelper.getDBPassword() + assertTrue(passphrase.size in 32..48) + } + + @Test + fun secondPassphraseShouldBeDifferFromFirst() { + val passphrase1 = SecurityHelper.getDBPassword() + + clearSharedPreferences() + val passphrase2 = SecurityHelper.getDBPassword() + + assertThat(passphrase1, not(equalTo(passphrase2))) + } + + @Test + fun canLoadDataFromEncryptedDatabase() { + runBlocking { + val id = UUID.randomUUID().toString() + val path = UUID.randomUUID().toString() + val type = Random.nextInt(1000) + + insertFakeEntity(id, path, type) + val keyCacheEntity = loadFakeEntity() + + assertThat(keyCacheEntity.id, equalTo(id)) + assertThat(keyCacheEntity.path, equalTo(path)) + assertThat(keyCacheEntity.type, equalTo(type)) + } + } + + @Test + fun testDbInstanceIsActuallyResetWhenCalled() { + val before = this.db + AppDatabase.reset(appContext) + val after = this.db + + assertTrue(before != after) + } + + @Test(expected = SQLiteException::class) + fun loadingDataFromDatabaseWillFailWhenPassphraseIsIncorrect() { + runBlocking { + val id = UUID.randomUUID().toString() + val path = UUID.randomUUID().toString() + val type = Random.nextInt(1000) + insertFakeEntity(id, path, type) + + clearSharedPreferences() + AppDatabase.resetInstance() + + val keyCacheEntity = loadFakeEntity() + assertThat(keyCacheEntity.id, equalTo(id)) + assertThat(keyCacheEntity.path, equalTo(path)) + assertThat(keyCacheEntity.type, equalTo(type)) + } + } + + private suspend fun insertFakeEntity( + id: String, + path: String, + type: Int + ) { + db.dateDao().insertEntry(KeyCacheEntity().apply { + this.id = id + this.path = path + this.type = type + }) + } + + private suspend fun loadFakeEntity(): KeyCacheEntity = db.dateDao().getAllEntries().first() + + private fun clearSharedPreferences() = + SecurityHelper.globalEncryptedSharedPreferencesInstance.edit().clear().commit() +} diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/TestRiskLevelCalculation.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/TestRiskLevelCalculation.kt index 602f7322f..3e288bd87 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/TestRiskLevelCalculation.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/TestRiskLevelCalculation.kt @@ -36,6 +36,7 @@ import de.rki.coronawarnapp.ui.viewmodel.SettingsViewModel import de.rki.coronawarnapp.ui.viewmodel.SubmissionViewModel import de.rki.coronawarnapp.ui.viewmodel.TracingViewModel import de.rki.coronawarnapp.util.KeyFileHelper +import de.rki.coronawarnapp.util.security.SecurityHelper import kotlinx.android.synthetic.main.fragment_test_risk_level_calculation.transmission_number import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch @@ -107,7 +108,7 @@ class TestRiskLevelCalculation : Fragment() { // Database Reset AppDatabase.getInstance(requireContext()).clearAllTables() // Delete Database Instance - AppDatabase.resetInstance(requireContext()) + SecurityHelper.resetSharedPrefs() // Export File Reset FileStorageHelper.getAllFilesInKeyExportDirectory().forEach { it.delete() } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/AppDatabase.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/AppDatabase.kt index 15e12326f..113427e38 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/AppDatabase.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/AppDatabase.kt @@ -1,6 +1,7 @@ package de.rki.coronawarnapp.storage import android.content.Context +import android.database.sqlite.SQLiteDatabase import androidx.room.Database import androidx.room.Room import androidx.room.RoomDatabase @@ -11,8 +12,8 @@ import de.rki.coronawarnapp.storage.tracing.TracingIntervalDao import de.rki.coronawarnapp.storage.tracing.TracingIntervalEntity import de.rki.coronawarnapp.util.Converters import de.rki.coronawarnapp.util.security.SecurityHelper -import net.sqlcipher.database.SQLiteDatabase import net.sqlcipher.database.SupportFactory +import java.io.File @Database( entities = [ExposureSummaryEntity::class, KeyCacheEntity::class, TracingIntervalEntity::class], @@ -36,11 +37,22 @@ abstract class AppDatabase : RoomDatabase() { } } - fun resetInstance(context: Context) = { instance = null }.also { getInstance(context) } + fun resetInstance() = synchronized(this) { + instance = null + } + + fun reset(context: Context) { + val path: String = context.getDatabasePath(DATABASE_NAME).path + val dbFile = File(path) + if (dbFile.exists()) { + SQLiteDatabase.deleteDatabase(dbFile) + } + resetInstance() + } private fun buildDatabase(context: Context): AppDatabase { return Room.databaseBuilder(context, AppDatabase::class.java, DATABASE_NAME) - .openHelperFactory(SupportFactory(SQLiteDatabase.getBytes(SecurityHelper.getDBPassword()))) + .openHelperFactory(SupportFactory(SecurityHelper.getDBPassword())) .build() } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DataRetentionHelper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DataRetentionHelper.kt index a6891315c..863aa1a4e 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DataRetentionHelper.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DataRetentionHelper.kt @@ -19,11 +19,12 @@ package de.rki.coronawarnapp.util +import android.annotation.SuppressLint import android.content.Context import android.util.Log import de.rki.coronawarnapp.storage.AppDatabase import de.rki.coronawarnapp.storage.FileStorageHelper -import de.rki.coronawarnapp.storage.LocalData +import de.rki.coronawarnapp.util.security.SecurityHelper /** * Helper for supplying functionality regarding Data Retention @@ -35,14 +36,13 @@ object DataRetentionHelper { * Deletes all data known to the Application * */ + @SuppressLint("ApplySharedPref") // We need a commit here to ensure consistency fun clearAllLocalData(context: Context) { Log.w(TAG, "CWA LOCAL DATA DELETION INITIATED.") // Database Reset - AppDatabase.getInstance(context).clearAllTables() + AppDatabase.reset(context) // Shared Preferences Reset - LocalData.getSharedPreferenceInstance().edit().clear().apply() - // Delete Database Instance - AppDatabase.resetInstance(context) + SecurityHelper.resetSharedPrefs() // Export File Reset FileStorageHelper.getAllFilesInKeyExportDirectory().forEach { it.delete() } Log.w(TAG, "CWA LOCAL DATA DELETION COMPLETED.") diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SecurityHelper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SecurityHelper.kt index d8df176a7..91ac13d91 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SecurityHelper.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SecurityHelper.kt @@ -20,22 +20,19 @@ package de.rki.coronawarnapp.util.security import KeyExportFormat.TEKSignatureList +import android.annotation.SuppressLint import android.content.Context import android.content.SharedPreferences -import android.security.keystore.KeyGenParameterSpec -import android.security.keystore.KeyProperties +import android.os.Build +import android.util.Base64 import androidx.security.crypto.EncryptedSharedPreferences import androidx.security.crypto.MasterKeys import de.rki.coronawarnapp.BuildConfig import de.rki.coronawarnapp.CoronaWarnApplication import de.rki.coronawarnapp.exception.CwaSecurityException -import java.lang.Exception -import java.lang.NullPointerException import java.security.KeyStore import java.security.MessageDigest import java.security.SecureRandom -import javax.crypto.KeyGenerator -import javax.crypto.SecretKey import java.security.Signature import java.security.cert.Certificate @@ -44,7 +41,8 @@ import java.security.cert.Certificate */ object SecurityHelper { private const val CWA_APP_SQLITE_DB_PW = "CWA_APP_SQLITE_DB_PW" - private const val AES_KEY_SIZE = 256 + private const val DB_PASSWORD_MIN_LENGTH = 32 + private const val DB_PASSWORD_MAX_LENGTH = 48 private const val SHARED_PREF_NAME = "shared_preferences_cwa" private val keyGenParameterSpec = MasterKeys.AES256_GCM_SPEC private val masterKeyAlias = MasterKeys.getOrCreate(keyGenParameterSpec) @@ -53,13 +51,6 @@ object SecurityHelper { private const val CWA_EXPORT_CERTIFICATE_NAME_NON_PROD = "cwa non-prod certificate" private const val CWA_EXPORT_CERTIFICATE_KEY_STORE = "trusted-certs-cwa.bks" - private const val ANDROID_KEY_STORE = "AndroidKeyStore" - - private val androidKeyStore: KeyStore by lazy { - KeyStore.getInstance(ANDROID_KEY_STORE).also { - it.load(null) - } - } val globalEncryptedSharedPreferencesInstance: SharedPreferences by lazy { withSecurityCatch { @@ -79,33 +70,53 @@ object SecurityHelper { EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM ) + private val String.toPreservedByteArray: ByteArray + get() = Base64.decode(this, Base64.NO_WRAP) + + private val ByteArray.toPreservedString: String + get() = Base64.encodeToString(this, Base64.NO_WRAP) + /** - * Retrieves the Master Key from the Android KeyStore to use in SQLCipher + * Retrieves the db password from encrypted shared preferences. The password is automatically + * encrypted when storing the password and decrypted when retrieving the password. + * + * If no password exists yet, a new password is generated and stored into + * encrypted shared preferences. The length of the password will be in the interval + * of [[DB_PASSWORD_MIN_LENGTH], [DB_PASSWORD_MAX_LENGTH]] */ - fun getDBPassword() = getOrGenerateDBSecretKey() - .toString() - .toCharArray() - - private fun getOrGenerateDBSecretKey(): SecretKey = - androidKeyStore.getKey(CWA_APP_SQLITE_DB_PW, null).run { - return if (this == null) { - val kg: KeyGenerator = KeyGenerator.getInstance( - KeyProperties.KEY_ALGORITHM_AES, ANDROID_KEY_STORE - ) - val spec: KeyGenParameterSpec = KeyGenParameterSpec.Builder( - CWA_APP_SQLITE_DB_PW, - KeyProperties.PURPOSE_ENCRYPT and KeyProperties.PURPOSE_DECRYPT - ) - .setKeySize(AES_KEY_SIZE) - .setBlockModes(KeyProperties.BLOCK_MODE_CBC) - .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7) - .setRandomizedEncryptionRequired(true) - .setUserAuthenticationRequired(false) - .build() - kg.init(spec, SecureRandom()) - kg.generateKey() - } else this as SecretKey + fun getDBPassword(): ByteArray = + getStoredDbPassword() ?: storeDbPassword(generateDBPassword()) + + @SuppressLint("ApplySharedPref") + fun resetSharedPrefs() { + globalEncryptedSharedPreferencesInstance.edit().clear().commit() + } + + private fun getStoredDbPassword(): ByteArray? = + globalEncryptedSharedPreferencesInstance + .getString(CWA_APP_SQLITE_DB_PW, null)?.toPreservedByteArray + + private fun storeDbPassword(keyBytes: ByteArray): ByteArray { + globalEncryptedSharedPreferencesInstance + .edit() + .putString(CWA_APP_SQLITE_DB_PW, keyBytes.toPreservedString) + .apply() + return keyBytes + } + + private fun generateDBPassword(): ByteArray { + val secureRandom = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + SecureRandom.getInstanceStrong() + } else { + SecureRandom() } + val max = DB_PASSWORD_MAX_LENGTH + val min = DB_PASSWORD_MIN_LENGTH + val passwordLength = secureRandom.nextInt(max - min + 1) + min + val password = ByteArray(passwordLength) + secureRandom.nextBytes(password) + return password + } fun hash256(input: String): String = MessageDigest .getInstance("SHA-256") @@ -116,12 +127,13 @@ object SecurityHelper { Signature.getInstance(EXPORT_SIGNATURE_ALGORITHM).run { initVerify(trustedCertForSignature) update(export) - verify(TEKSignatureList - .parseFrom(sig) - .signaturesList - .first() - .signature - .toByteArray() + verify( + TEKSignatureList + .parseFrom(sig) + .signaturesList + .first() + .signature + .toByteArray() ) } } -- GitLab