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