Merge pull request #1087 from vector-im/feature/verification_cleanup

Verification cleanup

matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/crosssigning/ExtensionsKtTest.kt

@@ -0,0 +1,38 @@
+/*
+ * 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.shouldBeNull
+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()
+    }
+
+    @Test
+    fun testBadBase64() {
+        "===".fromBase64Safe().shouldBeNull()
+    }
+}

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)
 }

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) {

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())
@@ -32,6 +33,18 @@ fun ByteArray.toBase64NoPadding(): String {
     return Base64.encodeToString(this, Base64.NO_PADDING or Base64.NO_WRAP)
 }
 
-fun String.fromBase64NoPadding(): ByteArray {
-    return Base64.decode(this, Base64.NO_PADDING or Base64.NO_WRAP)
+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
+    }
 }

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() ?: throw SharedSecretStorageError.BadCipherText
 
         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

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?
+            if (otherUserId != userId) {
                 crossSigningService.trustUser(otherUserId, object : MatrixCallback<Unit> {
                     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) {

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/DefaultQrCodeVerificationTransaction.kt

@@ -24,6 +24,8 @@ 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.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
@@ -199,7 +201,7 @@ internal class DefaultQrCodeVerificationTransaction(
             return
         }
 
-        if (startReq.sharedSecret == qrCodeData.sharedSecret) {
+        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

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
     }
 
@@ -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++

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<Map<String, String>>(ins, action.alias)
                         val trustResult = session.cryptoService().crossSigningService().checkTrustFromPrivateKeys(
                                 res?.get(MASTER_KEY_SSSS_NAME),