From b1b8513da4842e7439c57633af326876933b4213 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 27 Feb 2020 19:17:14 +0100 Subject: [PATCH] Create fromBase64Safe() to parse data received from external source --- .../crypto/crosssigning/ExtensionsKtTest.kt | 6 ++++++ .../securestorage/SharedSecretStorageError.kt | 2 ++ .../internal/crypto/crosssigning/Extensions.kt | 13 +++++++++++++ .../secrets/DefaultSharedSecretStorageService.kt | 2 +- .../qrcode/DefaultQrCodeVerificationTransaction.kt | 3 ++- .../crypto/verification/qrcode/Extensions.kt | 2 +- 6 files changed, 25 insertions(+), 3 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/crosssigning/ExtensionsKtTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/crosssigning/ExtensionsKtTest.kt index ca83af2f42..84da654c4c 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/crosssigning/ExtensionsKtTest.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/crosssigning/ExtensionsKtTest.kt @@ -16,6 +16,7 @@ package im.vector.matrix.android.internal.crypto.crosssigning +import org.amshove.kluent.shouldBeNull import org.amshove.kluent.shouldBeTrue import org.junit.Test @@ -29,4 +30,9 @@ class ExtensionsKtTest { // With padding "NMJyumnhMic".fromBase64().contentEquals("NMJyumnhMic=".fromBase64()).shouldBeTrue() } + + @Test + fun testBadBase64() { + "===".fromBase64Safe().shouldBeNull() + } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/securestorage/SharedSecretStorageError.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/securestorage/SharedSecretStorageError.kt index abd12789a5..a6bf75f2c0 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/securestorage/SharedSecretStorageError.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/securestorage/SharedSecretStorageError.kt @@ -28,5 +28,7 @@ sealed class SharedSecretStorageError(message: String?) : Throwable(message) { object BadKeyFormat : SharedSecretStorageError("Bad Key Format") object ParsingError : SharedSecretStorageError("parsing Error") object BadMac : SharedSecretStorageError("Bad mac") + object BadCipherText : SharedSecretStorageError("Bad cipher text") + data class OtherError(val reason: Throwable) : SharedSecretStorageError(reason.localizedMessage) } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/Extensions.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/Extensions.kt index 9125a99e70..fadfb567b3 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/Extensions.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/Extensions.kt @@ -19,6 +19,7 @@ import android.util.Base64 import im.vector.matrix.android.internal.crypto.model.CryptoCrossSigningKey import im.vector.matrix.android.internal.crypto.model.CryptoDeviceInfo import im.vector.matrix.android.internal.util.JsonCanonicalizer +import timber.log.Timber fun CryptoDeviceInfo.canonicalSignable(): String { return JsonCanonicalizer.getCanonicalJson(Map::class.java, signalableJSONDictionary()) @@ -35,3 +36,15 @@ fun ByteArray.toBase64NoPadding(): String { fun String.fromBase64(): ByteArray { return Base64.decode(this, Base64.DEFAULT) } + +/** + * Decode the base 64. Return null in case of bad format. Should be used when parsing received data from external source + */ +fun String.fromBase64Safe(): ByteArray? { + return try { + Base64.decode(this, Base64.DEFAULT) + } catch (throwable: Throwable) { + Timber.e(throwable, "Unable to decode base64 string") + null + } +} diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/secrets/DefaultSharedSecretStorageService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/secrets/DefaultSharedSecretStorageService.kt index b76b66b830..649f60887d 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/secrets/DefaultSharedSecretStorageService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/secrets/DefaultSharedSecretStorageService.kt @@ -297,7 +297,7 @@ internal class DefaultSharedSecretStorageService @Inject constructor( val iv = cipherContent.initializationVector?.fromBase64() ?: ByteArray(16) - val cipherRawBytes = cipherContent.ciphertext!!.fromBase64() + val cipherRawBytes = cipherContent.ciphertext?.fromBase64() ?: throw SharedSecretStorageError.BadCipherText val cipher = Cipher.getInstance("AES/CTR/NoPadding") diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/DefaultQrCodeVerificationTransaction.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/DefaultQrCodeVerificationTransaction.kt index 95438bf88d..241c741757 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/DefaultQrCodeVerificationTransaction.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/DefaultQrCodeVerificationTransaction.kt @@ -25,6 +25,7 @@ import im.vector.matrix.android.api.session.events.model.EventType import im.vector.matrix.android.internal.crypto.actions.SetDeviceVerificationAction import im.vector.matrix.android.internal.crypto.crosssigning.DeviceTrustLevel import im.vector.matrix.android.internal.crypto.crosssigning.fromBase64 +import im.vector.matrix.android.internal.crypto.crosssigning.fromBase64Safe import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore import im.vector.matrix.android.internal.crypto.verification.DefaultVerificationTransaction import im.vector.matrix.android.internal.crypto.verification.VerificationInfo @@ -200,7 +201,7 @@ internal class DefaultQrCodeVerificationTransaction( return } - if ((startReq.sharedSecret?.fromBase64()?.contentEquals(qrCodeData.sharedSecret.fromBase64()) == true)) { + if (startReq.sharedSecret?.fromBase64Safe()?.contentEquals(qrCodeData.sharedSecret.fromBase64()) == true) { // Ok, we can trust the other user // We can only trust the master key in this case // But first, ask the user for a confirmation diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/Extensions.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/Extensions.kt index 567fcdbf74..a4c4e649cc 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/Extensions.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/Extensions.kt @@ -98,7 +98,7 @@ fun String.toQrCodeData(): QrCodeData? { val msb = byteArray[cursor].toUnsignedInt() val lsb = byteArray[cursor + 1].toUnsignedInt() - val transactionLength = msb.shl(8) + lsb + val transactionLength = msb.shl(8) + lsb cursor++ cursor++