From f31c44963bb522ea65079a4fe78af6dd36dcec71 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 11 May 2021 11:12:27 +0200 Subject: [PATCH 1/4] Cleanup the existing code --- .../sdk/internal/database/RealmKeysUtils.kt | 4 +- .../securestorage/SecretStoringUtils.kt | 70 +++---------------- 2 files changed, 12 insertions(+), 62 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmKeysUtils.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmKeysUtils.kt index 0e3a7a2c49..d5ff7a0f84 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmKeysUtils.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmKeysUtils.kt @@ -71,7 +71,7 @@ internal class RealmKeysUtils @Inject constructor(context: Context, val encodedKey = Base64.encodeToString(key, Base64.NO_PADDING) val toStore = secretStoringUtils.securelyStoreString(encodedKey, alias) sharedPreferences.edit { - putString("${ENCRYPTED_KEY_PREFIX}_$alias", Base64.encodeToString(toStore!!, Base64.NO_PADDING)) + putString("${ENCRYPTED_KEY_PREFIX}_$alias", Base64.encodeToString(toStore, Base64.NO_PADDING)) } return key } @@ -84,7 +84,7 @@ internal class RealmKeysUtils @Inject constructor(context: Context, val encryptedB64 = sharedPreferences.getString("${ENCRYPTED_KEY_PREFIX}_$alias", null) val encryptedKey = Base64.decode(encryptedB64, Base64.NO_PADDING) val b64 = secretStoringUtils.loadSecureSecret(encryptedKey, alias) - return Base64.decode(b64!!, Base64.NO_PADDING) + return Base64.decode(b64, Base64.NO_PADDING) } fun configureEncryption(realmConfigurationBuilder: RealmConfiguration.Builder, alias: String) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt index c3b2d7f161..c6c8cedf9d 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt @@ -58,23 +58,19 @@ import javax.security.auth.x500.X500Principal * is not available. * * Android [K-M[ - * For android >=KITKAT and =L and Older androids - * For older androids as a fallback we generate an AES key from the alias using PBKDF2 with random salt. - * The salt and iv are stored with encrypted data. - * * Sample usage: * * val secret = "The answer is 42" - * val KEncrypted = SecretStoringUtils.securelyStoreString(secret, "myAlias", context) + * val KEncrypted = SecretStoringUtils.securelyStoreString(secret, "myAlias") * //This can be stored anywhere e.g. encoded in b64 and stored in preference for example * * //to get back the secret, just call - * val kDecrypted = SecretStoringUtils.loadSecureSecret(KEncrypted!!, "myAlias", context) + * val kDecrypted = SecretStoringUtils.loadSecureSecret(KEncrypted, "myAlias") * * * You can also just use this utility to store a secret key, and use any encryption algorithm that you want. @@ -91,7 +87,6 @@ internal class SecretStoringUtils @Inject constructor(private val context: Conte private const val FORMAT_API_M: Byte = 0 private const val FORMAT_1: Byte = 1 - private const val FORMAT_2: Byte = 2 } private val keyStore: KeyStore by lazy { @@ -113,15 +108,14 @@ internal class SecretStoringUtils @Inject constructor(private val context: Conte /** * Encrypt the given secret using the android Keystore. * On android >= M, will directly use the keystore to generate a symmetric key - * On android >= KitKat and = Lollipop and = Build.VERSION_CODES.M -> encryptStringM(secret, keyAlias) else -> encryptString(secret, keyAlias) @@ -132,7 +126,7 @@ internal class SecretStoringUtils @Inject constructor(private val context: Conte * Decrypt a secret that was encrypted by #securelyStoreString() */ @Throws(Exception::class) - fun loadSecureSecret(encrypted: ByteArray, keyAlias: String): String? { + fun loadSecureSecret(encrypted: ByteArray, keyAlias: String): String { return when { Build.VERSION.SDK_INT >= Build.VERSION_CODES.M -> decryptStringM(encrypted, keyAlias) else -> decryptString(encrypted, keyAlias) @@ -180,7 +174,7 @@ internal class SecretStoringUtils @Inject constructor(private val context: Conte - Store the encrypted AES Generate a key pair for encryption */ - fun getOrGenerateKeyPairForAlias(alias: String): KeyStore.PrivateKeyEntry { + private fun getOrGenerateKeyPairForAlias(alias: String): KeyStore.PrivateKeyEntry { val privateKeyEntry = (keyStore.getEntry(alias, null) as? KeyStore.PrivateKeyEntry) if (privateKeyEntry != null) return privateKeyEntry @@ -205,7 +199,7 @@ internal class SecretStoringUtils @Inject constructor(private val context: Conte } @RequiresApi(Build.VERSION_CODES.M) - fun encryptStringM(text: String, keyAlias: String): ByteArray? { + private fun encryptStringM(text: String, keyAlias: String): ByteArray { val secretKey = getOrGenerateSymmetricKeyForAliasM(keyAlias) val cipher = Cipher.getInstance(AES_MODE) @@ -229,7 +223,7 @@ internal class SecretStoringUtils @Inject constructor(private val context: Conte return String(cipher.doFinal(encryptedText), Charsets.UTF_8) } - private fun encryptString(text: String, keyAlias: String): ByteArray? { + private fun encryptString(text: String, keyAlias: String): ByteArray { // we generate a random symmetric key val key = ByteArray(16) secureRandom.nextBytes(key) @@ -246,7 +240,7 @@ internal class SecretStoringUtils @Inject constructor(private val context: Conte return format1Make(encryptedKey, iv, encryptedBytes) } - private fun decryptString(data: ByteArray, keyAlias: String): String? { + private fun decryptString(data: ByteArray, keyAlias: String): String { val (encryptedKey, iv, encrypted) = format1Extract(ByteArrayInputStream(data)) // we need to decrypt the key @@ -307,22 +301,6 @@ internal class SecretStoringUtils @Inject constructor(private val context: Conte output.write(bos1.toByteArray()) } -// @RequiresApi(Build.VERSION_CODES.M) -// @Throws(IOException::class) -// fun saveSecureObjectM(keyAlias: String, file: File, writeObject: Any) { -// FileOutputStream(file).use { -// saveSecureObjectM(keyAlias, it, writeObject) -// } -// } -// -// @RequiresApi(Build.VERSION_CODES.M) -// @Throws(IOException::class) -// fun loadSecureObjectM(keyAlias: String, file: File): T? { -// FileInputStream(file).use { -// return loadSecureObjectM(keyAlias, it) -// } -// } - @RequiresApi(Build.VERSION_CODES.M) @Throws(IOException::class) private fun loadSecureObjectM(keyAlias: String, inputStream: InputStream): T? { @@ -443,32 +421,4 @@ internal class SecretStoringUtils @Inject constructor(private val context: Conte return bos.toByteArray() } - - private fun format2Make(salt: ByteArray, iv: ByteArray, encryptedBytes: ByteArray): ByteArray { - val bos = ByteArrayOutputStream(3 + salt.size + iv.size + encryptedBytes.size) - bos.write(FORMAT_2.toInt()) - bos.write(salt.size) - bos.write(salt) - bos.write(iv.size) - bos.write(iv) - bos.write(encryptedBytes) - - return bos.toByteArray() - } - - private fun format2Extract(bis: InputStream): Triple { - val format = bis.read() - assert(format.toByte() == FORMAT_2) - - val saltSize = bis.read() - val salt = ByteArray(saltSize) - bis.read(salt) - - val ivSize = bis.read() - val iv = ByteArray(ivSize) - bis.read(iv) - - val encrypted = bis.readBytes() - return Triple(salt, iv, encrypted) - } } From cef4cf09ec8f85e5f36eab2fd112709dfcf87642 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 11 May 2021 11:55:54 +0200 Subject: [PATCH 2/4] Create a BuildVersionSdkIntProvider to be able to inject it and do some test To merge with BuildVersionSdkIntProvider To merge with fix add module To merge with fix buildVersionSdkIntProvider --- .../android/sdk/common/TestMatrixComponent.kt | 2 ++ .../sdk/internal/di/MatrixComponent.kt | 2 ++ .../sdk/internal/session/SessionComponent.kt | 2 ++ .../securestorage/SecretStoringUtils.kt | 27 ++++++++++++------- .../util/system/BuildVersionSdkIntProvider.kt | 24 +++++++++++++++++ .../DefaultBuildVersionSdkIntProvider.kt | 25 +++++++++++++++++ .../sdk/internal/util/system/SystemModule.kt | 27 +++++++++++++++++++ 7 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/BuildVersionSdkIntProvider.kt create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/DefaultBuildVersionSdkIntProvider.kt create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/SystemModule.kt diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/TestMatrixComponent.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/TestMatrixComponent.kt index 33de345630..1d05e655af 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/TestMatrixComponent.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/TestMatrixComponent.kt @@ -26,6 +26,7 @@ import org.matrix.android.sdk.internal.di.MatrixModule import org.matrix.android.sdk.internal.di.MatrixScope import org.matrix.android.sdk.internal.di.NetworkModule import org.matrix.android.sdk.internal.raw.RawModule +import org.matrix.android.sdk.internal.util.system.SystemModule @Component(modules = [ TestModule::class, @@ -33,6 +34,7 @@ import org.matrix.android.sdk.internal.raw.RawModule NetworkModule::class, AuthModule::class, RawModule::class, + SystemModule::class, TestNetworkModule::class ]) @MatrixScope diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/di/MatrixComponent.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/di/MatrixComponent.kt index 9d6fa29bb2..5bc519e960 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/di/MatrixComponent.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/di/MatrixComponent.kt @@ -36,6 +36,7 @@ import org.matrix.android.sdk.internal.session.TestInterceptor import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.util.BackgroundDetectionObserver import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers +import org.matrix.android.sdk.internal.util.system.SystemModule import org.matrix.olm.OlmManager import java.io.File @@ -44,6 +45,7 @@ import java.io.File NetworkModule::class, AuthModule::class, RawModule::class, + SystemModule::class, NoOpTestModule::class ]) @MatrixScope diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionComponent.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionComponent.kt index 541c877b1d..9a936b73c2 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionComponent.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionComponent.kt @@ -64,6 +64,7 @@ import org.matrix.android.sdk.internal.session.user.accountdata.AccountDataModul import org.matrix.android.sdk.internal.session.widgets.WidgetModule import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers +import org.matrix.android.sdk.internal.util.system.SystemModule @Component(dependencies = [MatrixComponent::class], modules = [ @@ -80,6 +81,7 @@ import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers CacheModule::class, MediaModule::class, CryptoModule::class, + SystemModule::class, PushersModule::class, OpenIdModule::class, WidgetModule::class, diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt index c6c8cedf9d..5be608ce13 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt @@ -18,12 +18,14 @@ package org.matrix.android.sdk.internal.session.securestorage +import android.annotation.SuppressLint import android.content.Context import android.os.Build import android.security.KeyPairGeneratorSpec import android.security.keystore.KeyGenParameterSpec import android.security.keystore.KeyProperties import androidx.annotation.RequiresApi +import org.matrix.android.sdk.internal.util.system.BuildVersionSdkIntProvider import timber.log.Timber import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream @@ -78,7 +80,10 @@ import javax.security.auth.x500.X500Principal * Important: Keys stored in the keystore can be wiped out (depends of the OS version, like for example if you * add a pin or change the schema); So you might and with a useless pile of bytes. */ -internal class SecretStoringUtils @Inject constructor(private val context: Context) { +internal class SecretStoringUtils @Inject constructor( + private val context: Context, + private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider +) { companion object { private const val ANDROID_KEY_STORE = "AndroidKeyStore" @@ -114,36 +119,40 @@ internal class SecretStoringUtils @Inject constructor(private val context: Conte * * The secret is encrypted using the following method: AES/GCM/NoPadding */ + @SuppressLint("NewApi") @Throws(Exception::class) fun securelyStoreString(secret: String, keyAlias: String): ByteArray { return when { - Build.VERSION.SDK_INT >= Build.VERSION_CODES.M -> encryptStringM(secret, keyAlias) - else -> encryptString(secret, keyAlias) + buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M -> encryptStringM(secret, keyAlias) + else -> encryptString(secret, keyAlias) } } /** * Decrypt a secret that was encrypted by #securelyStoreString() */ + @SuppressLint("NewApi") @Throws(Exception::class) fun loadSecureSecret(encrypted: ByteArray, keyAlias: String): String { return when { - Build.VERSION.SDK_INT >= Build.VERSION_CODES.M -> decryptStringM(encrypted, keyAlias) - else -> decryptString(encrypted, keyAlias) + buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M -> decryptStringM(encrypted, keyAlias) + else -> decryptString(encrypted, keyAlias) } } + @SuppressLint("NewApi") fun securelyStoreObject(any: Any, keyAlias: String, output: OutputStream) { when { - Build.VERSION.SDK_INT >= Build.VERSION_CODES.M -> saveSecureObjectM(keyAlias, output, any) - else -> saveSecureObject(keyAlias, output, any) + buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M -> saveSecureObjectM(keyAlias, output, any) + else -> saveSecureObject(keyAlias, output, any) } } + @SuppressLint("NewApi") fun loadSecureSecret(inputStream: InputStream, keyAlias: String): T? { return when { - Build.VERSION.SDK_INT >= Build.VERSION_CODES.M -> loadSecureObjectM(keyAlias, inputStream) - else -> loadSecureObject(keyAlias, inputStream) + buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M -> loadSecureObjectM(keyAlias, inputStream) + else -> loadSecureObject(keyAlias, inputStream) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/BuildVersionSdkIntProvider.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/BuildVersionSdkIntProvider.kt new file mode 100644 index 0000000000..1736f10108 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/BuildVersionSdkIntProvider.kt @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2021 New Vector Ltd + * + * Licensed 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 org.matrix.android.sdk.internal.util.system + +internal interface BuildVersionSdkIntProvider { + /** + * Return the current version of the Android SDK + */ + fun get(): Int +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/DefaultBuildVersionSdkIntProvider.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/DefaultBuildVersionSdkIntProvider.kt new file mode 100644 index 0000000000..a206918dec --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/DefaultBuildVersionSdkIntProvider.kt @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2021 New Vector Ltd + * + * Licensed 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 org.matrix.android.sdk.internal.util.system + +import android.os.Build +import javax.inject.Inject + +internal class DefaultBuildVersionSdkIntProvider @Inject constructor() + : BuildVersionSdkIntProvider { + override fun get() = Build.VERSION.SDK_INT +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/SystemModule.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/SystemModule.kt new file mode 100644 index 0000000000..fdbb5c9d97 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/SystemModule.kt @@ -0,0 +1,27 @@ +/* + * Copyright 2021 The Matrix.org Foundation C.I.C. + * + * Licensed 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 org.matrix.android.sdk.internal.util.system + +import dagger.Binds +import dagger.Module + +@Module +internal abstract class SystemModule { + + @Binds + abstract fun bindBuildVersionSdkIntProvider(provider: DefaultBuildVersionSdkIntProvider): BuildVersionSdkIntProvider +} From 91be2b6f3f444a589ec22c415fb07b28ae5ea7b1 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 11 May 2021 13:55:29 +0200 Subject: [PATCH 3/4] Add test and handle system upgrade --- CHANGES.md | 1 + .../securestorage/SecretStoringUtilsTest.kt | 184 ++++++++++++++++++ .../TestBuildVersionSdkIntProvider.kt | 25 +++ .../securestorage/SecretStoringUtils.kt | 38 ++-- 4 files changed, 228 insertions(+), 20 deletions(-) create mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtilsTest.kt create mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/TestBuildVersionSdkIntProvider.kt diff --git a/CHANGES.md b/CHANGES.md index 89121bdf54..100c9972ee 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -27,6 +27,7 @@ Bugfix 🐛: - Properly clean the back stack if the user cancel registration when waiting for email validation - Fix read marker visibility/position when filtering some events - Fix user invitation in case of restricted profile api (#3306) + - Make sure the SDK can retrieve the secret storage if the system is upgraded (#3304) Translations 🗣: - diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtilsTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtilsTest.kt new file mode 100644 index 0000000000..7ee6caed0d --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtilsTest.kt @@ -0,0 +1,184 @@ +/* + * Copyright (c) 2021 The Matrix.org Foundation C.I.C. + * + * Licensed 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 org.matrix.android.sdk.internal.session.securestorage + +import android.os.Build +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.amshove.kluent.shouldBeEqualTo +import org.junit.FixMethodOrder +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.MethodSorters +import org.matrix.android.sdk.InstrumentedTest +import org.matrix.android.sdk.internal.crypto.crosssigning.fromBase64 +import org.matrix.android.sdk.internal.crypto.crosssigning.toBase64NoPadding +import java.io.ByteArrayOutputStream +import java.util.UUID + +@RunWith(AndroidJUnit4::class) +@FixMethodOrder(MethodSorters.JVM) +class SecretStoringUtilsTest : InstrumentedTest { + + private val buildVersionSdkIntProvider = TestBuildVersionSdkIntProvider() + private val secretStoringUtils = SecretStoringUtils(context(), buildVersionSdkIntProvider) + + companion object { + const val TEST_STR = "This is something I want to store safely!" + } + + @Test + fun testStringNominalCaseApi21() { + val alias = generateAlias() + buildVersionSdkIntProvider.value = Build.VERSION_CODES.LOLLIPOP + // Encrypt + val encrypted = secretStoringUtils.securelyStoreString(TEST_STR, alias) + // Decrypt + val decrypted = secretStoringUtils.loadSecureSecret(encrypted, alias) + decrypted shouldBeEqualTo TEST_STR + secretStoringUtils.safeDeleteKey(alias) + } + + @Test + fun testStringNominalCaseApi23() { + val alias = generateAlias() + buildVersionSdkIntProvider.value = Build.VERSION_CODES.M + // Encrypt + val encrypted = secretStoringUtils.securelyStoreString(TEST_STR, alias) + // Decrypt + val decrypted = secretStoringUtils.loadSecureSecret(encrypted, alias) + decrypted shouldBeEqualTo TEST_STR + secretStoringUtils.safeDeleteKey(alias) + } + + @Test + fun testStringNominalCaseApi30() { + val alias = generateAlias() + buildVersionSdkIntProvider.value = Build.VERSION_CODES.R + // Encrypt + val encrypted = secretStoringUtils.securelyStoreString(TEST_STR, alias) + // Decrypt + val decrypted = secretStoringUtils.loadSecureSecret(encrypted, alias) + decrypted shouldBeEqualTo TEST_STR + secretStoringUtils.safeDeleteKey(alias) + } + + @Test + fun testStringMigration21_23() { + val alias = generateAlias() + buildVersionSdkIntProvider.value = Build.VERSION_CODES.LOLLIPOP + // Encrypt + val encrypted = secretStoringUtils.securelyStoreString(TEST_STR, alias) + + // Simulate a system upgrade + buildVersionSdkIntProvider.value = Build.VERSION_CODES.M + + // Decrypt + val decrypted = secretStoringUtils.loadSecureSecret(encrypted, alias) + decrypted shouldBeEqualTo TEST_STR + secretStoringUtils.safeDeleteKey(alias) + } + + @Test + fun testObjectNominalCaseApi21() { + val alias = generateAlias() + buildVersionSdkIntProvider.value = Build.VERSION_CODES.LOLLIPOP + + // Encrypt + val encrypted = ByteArrayOutputStream().also { outputStream -> + outputStream.use { + secretStoringUtils.securelyStoreObject(TEST_STR, alias, it) + } + } + .toByteArray() + .toBase64NoPadding() + // Decrypt + val decrypted = encrypted.fromBase64().inputStream().use { + secretStoringUtils.loadSecureSecret(it, alias) + } + decrypted shouldBeEqualTo TEST_STR + secretStoringUtils.safeDeleteKey(alias) + } + + @Test + fun testObjectNominalCaseApi23() { + val alias = generateAlias() + buildVersionSdkIntProvider.value = Build.VERSION_CODES.M + + // Encrypt + val encrypted = ByteArrayOutputStream().also { outputStream -> + outputStream.use { + secretStoringUtils.securelyStoreObject(TEST_STR, alias, it) + } + } + .toByteArray() + .toBase64NoPadding() + // Decrypt + val decrypted = encrypted.fromBase64().inputStream().use { + secretStoringUtils.loadSecureSecret(it, alias) + } + decrypted shouldBeEqualTo TEST_STR + secretStoringUtils.safeDeleteKey(alias) + } + + @Test + fun testObjectNominalCaseApi30() { + val alias = generateAlias() + buildVersionSdkIntProvider.value = Build.VERSION_CODES.R + + // Encrypt + val encrypted = ByteArrayOutputStream().also { outputStream -> + outputStream.use { + secretStoringUtils.securelyStoreObject(TEST_STR, alias, it) + } + } + .toByteArray() + .toBase64NoPadding() + // Decrypt + val decrypted = encrypted.fromBase64().inputStream().use { + secretStoringUtils.loadSecureSecret(it, alias) + } + decrypted shouldBeEqualTo TEST_STR + secretStoringUtils.safeDeleteKey(alias) + } + + @Test + fun testObjectMigration21_23() { + val alias = generateAlias() + buildVersionSdkIntProvider.value = Build.VERSION_CODES.LOLLIPOP + + // Encrypt + val encrypted = ByteArrayOutputStream().also { outputStream -> + outputStream.use { + secretStoringUtils.securelyStoreObject(TEST_STR, alias, it) + } + } + .toByteArray() + .toBase64NoPadding() + + // Simulate a system upgrade + buildVersionSdkIntProvider.value = Build.VERSION_CODES.M + + // Decrypt + val decrypted = encrypted.fromBase64().inputStream().use { + secretStoringUtils.loadSecureSecret(it, alias) + } + decrypted shouldBeEqualTo TEST_STR + secretStoringUtils.safeDeleteKey(alias) + } + + private fun generateAlias() = UUID.randomUUID().toString() +} diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/TestBuildVersionSdkIntProvider.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/TestBuildVersionSdkIntProvider.kt new file mode 100644 index 0000000000..e44a62e24e --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/TestBuildVersionSdkIntProvider.kt @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2021 New Vector Ltd + * + * Licensed 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 org.matrix.android.sdk.internal.session.securestorage + +import org.matrix.android.sdk.internal.util.system.BuildVersionSdkIntProvider + +class TestBuildVersionSdkIntProvider : BuildVersionSdkIntProvider { + var value: Int = 0 + + override fun get() = value +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt index 5be608ce13..fad1840e51 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/securestorage/SecretStoringUtils.kt @@ -34,6 +34,7 @@ import java.io.InputStream import java.io.ObjectInputStream import java.io.ObjectOutputStream import java.io.OutputStream +import java.lang.IllegalArgumentException import java.math.BigInteger import java.security.KeyPairGenerator import java.security.KeyStore @@ -134,9 +135,13 @@ internal class SecretStoringUtils @Inject constructor( @SuppressLint("NewApi") @Throws(Exception::class) fun loadSecureSecret(encrypted: ByteArray, keyAlias: String): String { - return when { - buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M -> decryptStringM(encrypted, keyAlias) - else -> decryptString(encrypted, keyAlias) + encrypted.inputStream().use { inputStream -> + // First get the format + return when (val format = inputStream.read().toByte()) { + FORMAT_API_M -> decryptStringM(inputStream, keyAlias) + FORMAT_1 -> decryptString(inputStream, keyAlias) + else -> throw IllegalArgumentException("Unknown format $format") + } } } @@ -150,9 +155,11 @@ internal class SecretStoringUtils @Inject constructor( @SuppressLint("NewApi") fun loadSecureSecret(inputStream: InputStream, keyAlias: String): T? { - return when { - buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M -> loadSecureObjectM(keyAlias, inputStream) - else -> loadSecureObject(keyAlias, inputStream) + // First get the format + return when (val format = inputStream.read().toByte()) { + FORMAT_API_M -> loadSecureObjectM(keyAlias, inputStream) + FORMAT_1 -> loadSecureObject(keyAlias, inputStream) + else -> throw IllegalArgumentException("Unknown format $format") } } @@ -196,7 +203,7 @@ internal class SecretStoringUtils @Inject constructor( .setAlias(alias) .setSubject(X500Principal("CN=$alias")) .setSerialNumber(BigInteger.TEN) - // .setEncryptionRequired() requires that the phone as a pin/schema + // .setEncryptionRequired() requires that the phone has a pin/schema .setStartDate(start.time) .setEndDate(end.time) .build() @@ -220,8 +227,8 @@ internal class SecretStoringUtils @Inject constructor( } @RequiresApi(Build.VERSION_CODES.M) - private fun decryptStringM(encryptedChunk: ByteArray, keyAlias: String): String { - val (iv, encryptedText) = formatMExtract(encryptedChunk.inputStream()) + private fun decryptStringM(inputStream: InputStream, keyAlias: String): String { + val (iv, encryptedText) = formatMExtract(inputStream) val secretKey = getOrGenerateSymmetricKeyForAliasM(keyAlias) @@ -249,8 +256,8 @@ internal class SecretStoringUtils @Inject constructor( return format1Make(encryptedKey, iv, encryptedBytes) } - private fun decryptString(data: ByteArray, keyAlias: String): String { - val (encryptedKey, iv, encrypted) = format1Extract(ByteArrayInputStream(data)) + private fun decryptString(inputStream: InputStream, keyAlias: String): String { + val (encryptedKey, iv, encrypted) = format1Extract(inputStream) // we need to decrypt the key val sKeyBytes = rsaDecrypt(keyAlias, ByteArrayInputStream(encryptedKey)) @@ -315,9 +322,6 @@ internal class SecretStoringUtils @Inject constructor( private fun loadSecureObjectM(keyAlias: String, inputStream: InputStream): T? { val secretKey = getOrGenerateSymmetricKeyForAliasM(keyAlias) - val format = inputStream.read() - assert(format.toByte() == FORMAT_API_M) - val ivSize = inputStream.read() val iv = ByteArray(ivSize) inputStream.read(iv, 0, ivSize) @@ -380,9 +384,6 @@ internal class SecretStoringUtils @Inject constructor( } private fun formatMExtract(bis: InputStream): Pair { - val format = bis.read().toByte() - assert(format == FORMAT_API_M) - val ivSize = bis.read() val iv = ByteArray(ivSize) bis.read(iv, 0, ivSize) @@ -401,9 +402,6 @@ internal class SecretStoringUtils @Inject constructor( } private fun format1Extract(bis: InputStream): Triple { - val format = bis.read() - assert(format.toByte() == FORMAT_1) - val keySizeBig = bis.read() val keySizeLow = bis.read() val encryptedKeySize = keySizeBig.shl(8) + keySizeLow From 824a8a5c66a2d4afc6bd57ede404f2b902cb32f6 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 11 May 2021 13:50:27 +0200 Subject: [PATCH 4/4] Fix copyright --- .../session/securestorage/TestBuildVersionSdkIntProvider.kt | 2 +- .../sdk/internal/util/system/BuildVersionSdkIntProvider.kt | 2 +- .../internal/util/system/DefaultBuildVersionSdkIntProvider.kt | 2 +- .../org/matrix/android/sdk/internal/util/system/SystemModule.kt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/TestBuildVersionSdkIntProvider.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/TestBuildVersionSdkIntProvider.kt index e44a62e24e..b08c88fb24 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/TestBuildVersionSdkIntProvider.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/session/securestorage/TestBuildVersionSdkIntProvider.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021 New Vector Ltd + * Copyright (c) 2021 The Matrix.org Foundation C.I.C. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/BuildVersionSdkIntProvider.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/BuildVersionSdkIntProvider.kt index 1736f10108..b660796ad8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/BuildVersionSdkIntProvider.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/BuildVersionSdkIntProvider.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021 New Vector Ltd + * Copyright (c) 2021 The Matrix.org Foundation C.I.C. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/DefaultBuildVersionSdkIntProvider.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/DefaultBuildVersionSdkIntProvider.kt index a206918dec..d9f0064f38 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/DefaultBuildVersionSdkIntProvider.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/DefaultBuildVersionSdkIntProvider.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021 New Vector Ltd + * Copyright (c) 2021 The Matrix.org Foundation C.I.C. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/SystemModule.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/SystemModule.kt index fdbb5c9d97..8a7b50175a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/SystemModule.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/system/SystemModule.kt @@ -1,5 +1,5 @@ /* - * Copyright 2021 The Matrix.org Foundation C.I.C. + * Copyright (c) 2021 The Matrix.org Foundation C.I.C. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.