From ceab0903cfcfd3ad20b39624c1e427e904de253e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 27 Feb 2020 18:09:37 +0100 Subject: [PATCH 01/10] Improve code - TU passed --- .../internal/crypto/verification/qrcode/Extensions.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 da926a0e10..d94e4c5c69 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 @@ -94,11 +94,11 @@ fun String.toQrCodeData(): QrCodeData? { val mode = byteArray[cursor].toInt() cursor++ - // Get transaction length - val bigEndian1 = byteArray[cursor].toUnsignedInt() - val bigEndian2 = byteArray[cursor + 1].toUnsignedInt() + // Get transaction length, Big Endian format + val msb = byteArray[cursor].toUnsignedInt() + val lsb = byteArray[cursor + 1].toUnsignedInt() - val transactionLength = bigEndian1 * 0x0100 + bigEndian2 + val transactionLength = msb.shl(8) + lsb cursor++ cursor++ From 8299487f6dead3f4010c014a5c58c0fa86c4e9ba Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 27 Feb 2020 18:13:42 +0100 Subject: [PATCH 02/10] Avoid using encoder flag to decode Base64 string... --- .../matrix/android/internal/crypto/crosssigning/Extensions.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6ffc341881..7d55ebb1ad 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 @@ -33,5 +33,5 @@ fun ByteArray.toBase64NoPadding(): String { } fun String.fromBase64NoPadding(): ByteArray { - return Base64.decode(this, Base64.NO_PADDING or Base64.NO_WRAP) + return Base64.decode(this, Base64.DEFAULT) } From 1ead2778c2f5c22600d648c32b91a445bcd4301e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 27 Feb 2020 18:15:39 +0100 Subject: [PATCH 03/10] ... and rename the method fromBase64NoPadding() to fromBase64() --- .../crosssigning/DefaultCrossSigningService.kt | 14 +++++++------- .../internal/crypto/crosssigning/Extensions.kt | 2 +- .../secrets/DefaultSharedSecretStorageService.kt | 10 +++++----- .../crypto/verification/qrcode/Extensions.kt | 8 ++++---- .../VerificationBottomSheetViewModel.kt | 4 ++-- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/DefaultCrossSigningService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/DefaultCrossSigningService.kt index a29f27ddd6..acc9f4134d 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/DefaultCrossSigningService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/DefaultCrossSigningService.kt @@ -80,7 +80,7 @@ internal class DefaultCrossSigningService @Inject constructor( cryptoStore.getCrossSigningPrivateKeys()?.let { privateKeysInfo -> privateKeysInfo.master - ?.fromBase64NoPadding() + ?.fromBase64() ?.let { privateKeySeed -> val pkSigning = OlmPkSigning() if (pkSigning.initWithSeed(privateKeySeed) == mxCrossSigningInfo.masterKey()?.unpaddedBase64PublicKey) { @@ -93,7 +93,7 @@ internal class DefaultCrossSigningService @Inject constructor( } } privateKeysInfo.user - ?.fromBase64NoPadding() + ?.fromBase64() ?.let { privateKeySeed -> val pkSigning = OlmPkSigning() if (pkSigning.initWithSeed(privateKeySeed) == mxCrossSigningInfo.userKey()?.unpaddedBase64PublicKey) { @@ -106,7 +106,7 @@ internal class DefaultCrossSigningService @Inject constructor( } } privateKeysInfo.selfSigned - ?.fromBase64NoPadding() + ?.fromBase64() ?.let { privateKeySeed -> val pkSigning = OlmPkSigning() if (pkSigning.initWithSeed(privateKeySeed) == mxCrossSigningInfo.selfSigningKey()?.unpaddedBase64PublicKey) { @@ -307,7 +307,7 @@ internal class DefaultCrossSigningService @Inject constructor( var userKeyIsTrusted = false var selfSignedKeyIsTrusted = false - masterKeyPrivateKey?.fromBase64NoPadding() + masterKeyPrivateKey?.fromBase64() ?.let { privateKeySeed -> val pkSigning = OlmPkSigning() try { @@ -324,7 +324,7 @@ internal class DefaultCrossSigningService @Inject constructor( } } - uskKeyPrivateKey?.fromBase64NoPadding() + uskKeyPrivateKey?.fromBase64() ?.let { privateKeySeed -> val pkSigning = OlmPkSigning() try { @@ -341,7 +341,7 @@ internal class DefaultCrossSigningService @Inject constructor( } } - sskPrivateKey?.fromBase64NoPadding() + sskPrivateKey?.fromBase64() ?.let { privateKeySeed -> val pkSigning = OlmPkSigning() try { @@ -450,7 +450,7 @@ internal class DefaultCrossSigningService @Inject constructor( // 1) check if I know the private key val masterPrivateKey = cryptoStore.getCrossSigningPrivateKeys() ?.master - ?.fromBase64NoPadding() + ?.fromBase64() var isMaterKeyTrusted = false if (myMasterKey.trustLevel?.locallyVerified == true) { 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 7d55ebb1ad..9125a99e70 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 @@ -32,6 +32,6 @@ fun ByteArray.toBase64NoPadding(): String { return Base64.encodeToString(this, Base64.NO_PADDING or Base64.NO_WRAP) } -fun String.fromBase64NoPadding(): ByteArray { +fun String.fromBase64(): ByteArray { return Base64.decode(this, Base64.DEFAULT) } 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 9627492dc7..b76b66b830 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 @@ -35,7 +35,7 @@ import im.vector.matrix.android.api.session.securestorage.SsssKeySpec import im.vector.matrix.android.api.session.securestorage.SsssPassphrase import im.vector.matrix.android.internal.crypto.SSSS_ALGORITHM_AES_HMAC_SHA2 import im.vector.matrix.android.internal.crypto.SSSS_ALGORITHM_CURVE25519_AES_SHA2 -import im.vector.matrix.android.internal.crypto.crosssigning.fromBase64NoPadding +import im.vector.matrix.android.internal.crypto.crosssigning.fromBase64 import im.vector.matrix.android.internal.crypto.crosssigning.toBase64NoPadding import im.vector.matrix.android.internal.crypto.keysbackup.generatePrivateKeyWithPassword import im.vector.matrix.android.internal.crypto.keysbackup.util.computeRecoveryKey @@ -268,7 +268,7 @@ internal class DefaultSharedSecretStorageService @Inject constructor( val ivParameterSpec = IvParameterSpec(iv) cipher.init(Cipher.ENCRYPT_MODE, secretKeySpec, ivParameterSpec) // secret are not that big, just do Final - val cipherBytes = cipher.doFinal(clearDataBase64.fromBase64NoPadding()) + val cipherBytes = cipher.doFinal(clearDataBase64.fromBase64()) require(cipherBytes.isNotEmpty()) val macKeySpec = SecretKeySpec(macKey, "HmacSHA256") @@ -295,9 +295,9 @@ internal class DefaultSharedSecretStorageService @Inject constructor( val aesKey = pseudoRandomKey.copyOfRange(0, 32) val macKey = pseudoRandomKey.copyOfRange(32, 64) - val iv = cipherContent.initializationVector?.fromBase64NoPadding() ?: ByteArray(16) + val iv = cipherContent.initializationVector?.fromBase64() ?: ByteArray(16) - val cipherRawBytes = cipherContent.ciphertext!!.fromBase64NoPadding() + val cipherRawBytes = cipherContent.ciphertext!!.fromBase64() val cipher = Cipher.getInstance("AES/CTR/NoPadding") @@ -314,7 +314,7 @@ internal class DefaultSharedSecretStorageService @Inject constructor( val mac = Mac.getInstance("HmacSHA256").apply { init(macKeySpec) } val digest = mac.doFinal(cipherRawBytes) - if (!cipherContent.mac?.fromBase64NoPadding()?.contentEquals(digest).orFalse()) { + if (!cipherContent.mac?.fromBase64()?.contentEquals(digest).orFalse()) { throw SharedSecretStorageError.BadMac } else { // we are good 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 d94e4c5c69..567fcdbf74 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 @@ -16,7 +16,7 @@ package im.vector.matrix.android.internal.crypto.verification.qrcode -import im.vector.matrix.android.internal.crypto.crosssigning.fromBase64NoPadding +import im.vector.matrix.android.internal.crypto.crosssigning.fromBase64 import im.vector.matrix.android.internal.crypto.crosssigning.toBase64NoPadding import im.vector.matrix.android.internal.extensions.toUnsignedInt @@ -52,15 +52,15 @@ fun QrCodeData.toEncodedString(): String { } // Keys - firstKey.fromBase64NoPadding().forEach { + firstKey.fromBase64().forEach { result += it } - secondKey.fromBase64NoPadding().forEach { + secondKey.fromBase64().forEach { result += it } // Secret - sharedSecret.fromBase64NoPadding().forEach { + sharedSecret.fromBase64().forEach { result += it } diff --git a/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheetViewModel.kt b/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheetViewModel.kt index c5fd167f39..f4670b90e1 100644 --- a/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheetViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheetViewModel.kt @@ -42,7 +42,7 @@ import im.vector.matrix.android.api.session.events.model.LocalEcho import im.vector.matrix.android.api.session.room.model.create.CreateRoomParams import im.vector.matrix.android.api.util.MatrixItem import im.vector.matrix.android.api.util.toMatrixItem -import im.vector.matrix.android.internal.crypto.crosssigning.fromBase64NoPadding +import im.vector.matrix.android.internal.crypto.crosssigning.fromBase64 import im.vector.matrix.android.internal.crypto.crosssigning.isVerified import im.vector.matrix.android.internal.crypto.verification.PendingVerificationRequest import im.vector.riotx.core.extensions.exhaustive @@ -265,7 +265,7 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(@Assisted ini } is VerificationAction.GotResultFromSsss -> { try { - action.cypherData.fromBase64NoPadding().inputStream().use { ins -> + action.cypherData.fromBase64().inputStream().use { ins -> val res = session.loadSecureSecret>(ins, action.alias) val trustResult = session.cryptoService().crossSigningService().checkTrustFromPrivateKeys( res?.get(MASTER_KEY_SSSS_NAME), From 0a9008a73d2a702527e44ec94a98395a133f5f6f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 27 Feb 2020 18:35:05 +0100 Subject: [PATCH 04/10] Be robust if other client sends padded base64 in the reciprocate --- .../crypto/crosssigning/ExtensionsKtTest.kt | 32 +++++++++++++++++++ .../DefaultQrCodeVerificationTransaction.kt | 3 +- 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/crosssigning/ExtensionsKtTest.kt 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 new file mode 100644 index 0000000000..ca83af2f42 --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/crosssigning/ExtensionsKtTest.kt @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2020 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 im.vector.matrix.android.internal.crypto.crosssigning + +import org.amshove.kluent.shouldBeTrue +import org.junit.Test + +@Suppress("SpellCheckingInspection") +class ExtensionsKtTest { + + @Test + fun testComparingBase64StringWithOrWithoutPadding() { + // Without padding + "NMJyumnhMic".fromBase64().contentEquals("NMJyumnhMic".fromBase64()).shouldBeTrue() + // With padding + "NMJyumnhMic".fromBase64().contentEquals("NMJyumnhMic=".fromBase64()).shouldBeTrue() + } +} 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 9c2a40a4e8..95438bf88d 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 @@ -24,6 +24,7 @@ import im.vector.matrix.android.api.session.crypto.verification.VerificationTxSt 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.store.IMXCryptoStore import im.vector.matrix.android.internal.crypto.verification.DefaultVerificationTransaction import im.vector.matrix.android.internal.crypto.verification.VerificationInfo @@ -199,7 +200,7 @@ internal class DefaultQrCodeVerificationTransaction( return } - if (startReq.sharedSecret == qrCodeData.sharedSecret) { + if ((startReq.sharedSecret?.fromBase64()?.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 From b1b8513da4842e7439c57633af326876933b4213 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 27 Feb 2020 19:17:14 +0100 Subject: [PATCH 05/10] 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++ From 151fec5ce0c021c07bff5ec6cc915c4703e3559f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 28 Feb 2020 11:38:30 +0100 Subject: [PATCH 06/10] Fix crash on attachment preview screen (#1088) --- CHANGES.md | 2 +- .../features/attachments/preview/AttachmentsPreviewFragment.kt | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 65f6e96f69..4f8838deb6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,7 +8,7 @@ Improvements 🙌: - Bugfix 🐛: - - + - Fix crash on attachment preview screen (#1088) Translations 🗣: - diff --git a/vector/src/main/java/im/vector/riotx/features/attachments/preview/AttachmentsPreviewFragment.kt b/vector/src/main/java/im/vector/riotx/features/attachments/preview/AttachmentsPreviewFragment.kt index 1d525dddf7..e52b497df4 100644 --- a/vector/src/main/java/im/vector/riotx/features/attachments/preview/AttachmentsPreviewFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/attachments/preview/AttachmentsPreviewFragment.kt @@ -39,6 +39,7 @@ import com.airbnb.mvrx.fragmentViewModel import com.airbnb.mvrx.withState import com.yalantis.ucrop.UCrop import com.yalantis.ucrop.UCropActivity +import im.vector.matrix.android.api.extensions.orFalse import im.vector.matrix.android.api.session.content.ContentAttachmentData import im.vector.riotx.R import im.vector.riotx.core.extensions.cleanup @@ -115,7 +116,7 @@ class AttachmentsPreviewFragment @Inject constructor( override fun onPrepareOptionsMenu(menu: Menu) { withState(viewModel) { state -> val editMenuItem = menu.findItem(R.id.attachmentsPreviewEditAction) - val showEditMenuItem = state.attachments[state.currentAttachmentIndex].isEditable() + val showEditMenuItem = state.attachments.getOrNull(state.currentAttachmentIndex)?.isEditable().orFalse() editMenuItem.setVisible(showEditMenuItem) } From 779026b0afdff168bceb963ba1e59ededdb9320f Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 28 Feb 2020 11:46:32 +0100 Subject: [PATCH 07/10] Fix / mark master key as trusted after self verif --- .../SASDefaultVerificationTransaction.kt | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SASDefaultVerificationTransaction.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SASDefaultVerificationTransaction.kt index 7856e571eb..f56e414a26 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SASDefaultVerificationTransaction.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SASDefaultVerificationTransaction.kt @@ -299,14 +299,21 @@ internal abstract class SASDefaultVerificationTransaction( } // If not me sign his MSK and upload the signature - if (otherMasterKeyIsVerified && otherUserId != userId) { + if (otherMasterKeyIsVerified) { // we should trust this master key // And check verification MSK -> SSK? - crossSigningService.trustUser(otherUserId, object : MatrixCallback { - override fun onFailure(failure: Throwable) { - Timber.e(failure, "## SAS Verification: Failed to trust User $otherUserId") + if (otherUserId != userId) { + crossSigningService.trustUser(otherUserId, object : MatrixCallback { + override fun onFailure(failure: Throwable) { + Timber.e(failure, "## SAS Verification: Failed to trust User $otherUserId") + } + }) + } else { + // Notice other master key is mine because other is me + if (otherMasterKey?.trustLevel?.isVerified() == false) { + crossSigningService.markMyMasterKeyAsTrusted() } - }) + } } if (otherUserId == userId) { From b7ff546f6608390f1d1fd5fe88615afa1c029a08 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Sun, 1 Mar 2020 09:59:00 +0100 Subject: [PATCH 08/10] Add support for `/plain` command (#12) --- CHANGES.md | 2 +- .../java/im/vector/riotx/features/command/Command.kt | 1 + .../im/vector/riotx/features/command/CommandParser.kt | 9 +++++++++ .../im/vector/riotx/features/command/ParsedCommand.kt | 1 + .../features/home/room/detail/RoomDetailViewModel.kt | 6 ++++++ vector/src/main/res/values/strings_riotX.xml | 2 +- 6 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 4f8838deb6..b2ad0766c4 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,7 +5,7 @@ Features ✨: - Improvements 🙌: - - + - Add support for `/plain` command (#12) Bugfix 🐛: - Fix crash on attachment preview screen (#1088) diff --git a/vector/src/main/java/im/vector/riotx/features/command/Command.kt b/vector/src/main/java/im/vector/riotx/features/command/Command.kt index f39f1cb7cd..72f686c2c8 100644 --- a/vector/src/main/java/im/vector/riotx/features/command/Command.kt +++ b/vector/src/main/java/im/vector/riotx/features/command/Command.kt @@ -43,6 +43,7 @@ enum class Command(val command: String, val parameters: String, @StringRes val d SPOILER("/spoiler", "", R.string.command_description_spoiler), POLL("/poll", "Question | Option 1 | Option 2 ...", R.string.command_description_poll), SHRUG("/shrug", "", R.string.command_description_shrug), + PLAIN("/plain", "", R.string.command_description_plain), // TODO temporary command VERIFY_USER("/verify", "", R.string.command_description_verify); diff --git a/vector/src/main/java/im/vector/riotx/features/command/CommandParser.kt b/vector/src/main/java/im/vector/riotx/features/command/CommandParser.kt index abc047e273..875fe92610 100644 --- a/vector/src/main/java/im/vector/riotx/features/command/CommandParser.kt +++ b/vector/src/main/java/im/vector/riotx/features/command/CommandParser.kt @@ -57,6 +57,15 @@ object CommandParser { } return when (val slashCommand = messageParts.first()) { + Command.PLAIN.command -> { + val text = textMessage.substring(Command.PLAIN.command.length).trim() + + if (text.isNotEmpty()) { + ParsedCommand.SendPlainText(text) + } else { + ParsedCommand.ErrorSyntax(Command.PLAIN) + } + } Command.CHANGE_DISPLAY_NAME.command -> { val newDisplayName = textMessage.substring(Command.CHANGE_DISPLAY_NAME.command.length).trim() diff --git a/vector/src/main/java/im/vector/riotx/features/command/ParsedCommand.kt b/vector/src/main/java/im/vector/riotx/features/command/ParsedCommand.kt index d823429ac9..e4fee27ee6 100644 --- a/vector/src/main/java/im/vector/riotx/features/command/ParsedCommand.kt +++ b/vector/src/main/java/im/vector/riotx/features/command/ParsedCommand.kt @@ -33,6 +33,7 @@ sealed class ParsedCommand { // Valid commands: + class SendPlainText(val message: CharSequence) : ParsedCommand() class SendEmote(val message: CharSequence) : ParsedCommand() class SendRainbow(val message: CharSequence) : ParsedCommand() class SendRainbowEmote(val message: CharSequence) : ParsedCommand() diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt index 8a231fb25d..1fcae90e95 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt @@ -340,6 +340,12 @@ class RoomDetailViewModel @AssistedInject constructor(@Assisted initialState: Ro is ParsedCommand.ErrorUnknownSlashCommand -> { _viewEvents.post(RoomDetailViewEvents.SlashCommandUnknown(slashCommandResult.slashCommand)) } + is ParsedCommand.SendPlainText -> { + // Send the text message to the room, without markdown + room.sendTextMessage(slashCommandResult.message, autoMarkdown = false) + _viewEvents.post(RoomDetailViewEvents.MessageSent) + popDraft() + } is ParsedCommand.Invite -> { handleInviteSlashCommand(slashCommandResult) popDraft() diff --git a/vector/src/main/res/values/strings_riotX.xml b/vector/src/main/res/values/strings_riotX.xml index 00bf65e121..69d821c0ca 100644 --- a/vector/src/main/res/values/strings_riotX.xml +++ b/vector/src/main/res/values/strings_riotX.xml @@ -26,7 +26,7 @@ - + Sends a message as plain text, without interpreting it as markdown From 30e46445ca1c3f24324ce039febbdf32a5df5077 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 4 Mar 2020 09:35:09 +0100 Subject: [PATCH 09/10] Add documentation to install environment and run integration tests (#1098) Add documentation to install environment and run integration tests --- CONTRIBUTING.md | 2 + docs/integration_tests.md | 97 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 docs/integration_tests.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8e08a921fb..176a6ee9c1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -82,6 +82,8 @@ Make sure the following commands execute without any error: RiotX is currently supported on Android KitKat (API 19+): please test your change on an Android device (or Android emulator) running with API 19. Many issues can happen (including crashes) on older devices. Also, if possible, please test your change on a real device. Testing on Android emulator may not be sufficient. +You should consider adding Unit tests with your PR, and also integration tests (AndroidTest). Please refer to [this document](./docs/integration_tests.md) to install and run the integration test environment. + ### Internationalisation When adding new string resources, please only add new entries in file `value/strings.xml`. Translations will be added later by the community of translators with a specific tool named [Weblate](https://translate.riot.im/projects/riot-android/). diff --git a/docs/integration_tests.md b/docs/integration_tests.md new file mode 100644 index 0000000000..f7557a87a2 --- /dev/null +++ b/docs/integration_tests.md @@ -0,0 +1,97 @@ +# Integration tests + +Integration tests are useful to ensure that the code works well for any use cases. + +They can also be used as sample on how to use the Matrix SDK. + +In a ideal world, every API of the SDK should be covered by integration tests. For the moment, we have test mainly for the Crypto part, which is the tricky part. But it covers quite a lot of features: accounts creation, login to existing account, send encrypted messages, keys backup, verification, etc. + +The Matrix SDK is able to open multiple sessions, for the same user, of for different users. This way we can test communication between several sessions on a single device. + +## Pre requirements + +Integration tests need a homeserver running on localhost. + +The documentation describes what we do to have one, using [Synapse](https://github.com/matrix-org/synapse/), which is the Matrix reference homeserver. + +## Install and run Synapse + +Steps: + +- Install virtualenv + +```bash +python3 -m pip install virtualenv +``` + +- Clone Synapse repository + +```bash +git clone -b develop https://github.com/matrix-org/synapse.git +``` +or +```bash +git clone -b develop git@github.com:matrix-org/synapse.git +``` + +You should have the develop branch cloned by default. + +- Run synapse, from the Synapse folder you just cloned + +```bash +virtualenv -p python3 env +source env/bin/activate +pip install -e . +demo/start.sh --no-rate-limit +``` + +Alternatively, to install the latest Synapse release package (and not a cloned branch) you can run the following instead of `pip install -e .`: + +```bash +pip install matrix-synapse +``` + +You should now have 3 running federated Synapse instances 🎉, at http://127.0.0.1:8080/, http://127.0.0.1:8081/ and http://127.0.0.1:8082/, which should display a "It Works! Synapse is running" message. + +## Run the test + +It's recommended to run tests using an Android Emulator and not a real device. First reason for that is that the tests will use http://10.0.2.2:8080 to connect to Synapse, which run locally on your machine. + +You can run all the tests in the `androidTest` folders. + +## Stop Synapse + +To stop Synapse, you can run the following commands: + +```bash +./demo/stop.sh +``` + +And you can deactivate the virtualenv: + +```bash +deactivate +``` + +## Troubleshoot + +You'll need python3 to be able to run synapse + +### Android Emulator does cannot reach the homeserver + +Try on the Emulator browser to open "http://10.0.2.2:8080". You should see the "Synapse is running" message. + +### virtualenv command fails + +You can try using +```bash +python3 -m venv env +``` +or +```bash +python3 -m virtualenv env +``` +instead of +```bash +virtualenv -p python3 env +``` From 0121eee5b83e36af7b2c97899198ec5f4ad834c5 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 4 Mar 2020 09:37:32 +0100 Subject: [PATCH 10/10] changelog --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index b2ad0766c4..37ff98758b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -20,7 +20,7 @@ Build 🧱: - Other changes: - - + - Add a [documentation](./docs/integration_tests.md) to run integration tests Changes in RiotX 0.17.0 (2020-02-27) ===================================================