Merge pull request #8933 from element-hq/feature/bca/fix_previously_verified_users

feat(crypto): Add support for verification violation warnings

.github/workflows/post-pr.yml

@@ -54,7 +54,7 @@ jobs:
         with:
           cache-read-only: ${{ github.ref != 'refs/heads/develop' }}
       - name: Start synapse server
-        uses: michaelkaye/setup-matrix-synapse@v1.0.4
+        uses: michaelkaye/setup-matrix-synapse@v1.0.5
         with:
           uploadLogs: true
           httpPort: 8080

.github/workflows/tests.yml

@@ -55,7 +55,7 @@ jobs:
       - uses: actions/setup-python@v4
         with:
           python-version: 3.8
-      - uses: michaelkaye/setup-matrix-synapse@v1.0.4
+      - uses: michaelkaye/setup-matrix-synapse@v1.0.5
         with:
           uploadLogs: true
           httpPort: 8080

changelog.d/8933.bugfix

@@ -0,0 +1 @@
+Show a notice when a previously verified user is not anymore

matrix-sdk-android/build.gradle

@@ -221,7 +221,7 @@ dependencies {
 
     implementation libs.google.phonenumber
 
-    implementation("org.matrix.rustcomponents:crypto-android:0.4.3")
+    implementation("org.matrix.rustcomponents:crypto-android:0.5.0")
 //    api project(":library:rustCrypto")
 
     testImplementation libs.tests.junit

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/ComputeShieldForGroupUseCaseTest.kt

@@ -0,0 +1,262 @@
+/*
+ * Copyright 2024 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.crypto
+
+import io.mockk.coEvery
+import io.mockk.every
+import io.mockk.mockk
+import kotlinx.coroutines.test.runTest
+import org.amshove.kluent.shouldBeEqualTo
+import org.junit.Test
+import org.matrix.android.sdk.api.session.crypto.crosssigning.DeviceTrustLevel
+import org.matrix.android.sdk.api.session.crypto.crosssigning.MXCrossSigningInfo
+import org.matrix.android.sdk.api.session.crypto.model.CryptoDeviceInfo
+import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel
+
+class ComputeShieldForGroupUseCaseTest {
+
+    @Test
+    fun shouldReturnDefaultShieldWhenNoOneIsVerified() = runTest {
+        val mockMachine = mockk<OlmMachine> {
+            coEvery {
+                getIdentity("@me:localhost")
+            } returns mockk<UserIdentities>(relaxed = true)
+
+            coEvery {
+                getIdentity("@alice:localhost")
+            } returns fakeIdentity(isVerified = false, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@alice:localhost")
+            } returns listOf(fakeDevice("@alice:localhost", "A0", false))
+
+            coEvery {
+                getIdentity("@bob:localhost")
+            } returns fakeIdentity(isVerified = false, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@bob:localhost")
+            } returns listOf(fakeDevice("@bob:localhost", "B0", false))
+
+            coEvery {
+                getIdentity("@charly:localhost")
+            } returns fakeIdentity(isVerified = false, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@charly:localhost")
+            } returns listOf(fakeDevice("@charly:localhost", "C0", false))
+        }
+
+        val computeShieldOp = ComputeShieldForGroupUseCase("@me:localhost")
+
+        val shield = computeShieldOp.invoke(mockMachine, listOf("@alice:localhost", "@bob:localhost", "@charly:localhost"))
+
+        shield shouldBeEqualTo RoomEncryptionTrustLevel.Default
+    }
+
+    @Test
+    fun shouldReturnDefaultShieldWhenVerifiedUsersHaveSecureDevices() = runTest {
+        val mockMachine = mockk<OlmMachine> {
+            coEvery {
+                getIdentity("@me:localhost")
+            } returns mockk<UserIdentities>(relaxed = true)
+
+            // Alice is verified
+            coEvery {
+                getIdentity("@alice:localhost")
+            } returns fakeIdentity(isVerified = true, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@alice:localhost")
+            } returns listOf(
+                    fakeDevice("@alice:localhost", "A0", true),
+                    fakeDevice("@alice:localhost", "A1", true)
+            )
+
+            coEvery {
+                getIdentity("@bob:localhost")
+            } returns fakeIdentity(isVerified = false, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@bob:localhost")
+            } returns listOf(fakeDevice("@bob:localhost", "B0", false))
+
+            coEvery {
+                getIdentity("@charly:localhost")
+            } returns fakeIdentity(isVerified = false, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@charly:localhost")
+            } returns listOf(fakeDevice("@charly:localhost", "C0", false))
+        }
+
+        val computeShieldOp = ComputeShieldForGroupUseCase("@me:localhost")
+
+        val shield = computeShieldOp.invoke(mockMachine, listOf("@alice:localhost", "@bob:localhost", "@charly:localhost"))
+
+        shield shouldBeEqualTo RoomEncryptionTrustLevel.Default
+    }
+
+    @Test
+    fun shouldReturnWarningShieldWhenPreviouslyVerifiedUsersHaveInSecureDevices() = runTest {
+        val mockMachine = mockk<OlmMachine> {
+            coEvery {
+                getIdentity("@me:localhost")
+            } returns mockk<UserIdentities>(relaxed = true)
+
+            // Alice is verified
+            coEvery {
+                getIdentity("@alice:localhost")
+            } returns fakeIdentity(isVerified = false, hasVerificationViolation = true)
+
+            coEvery {
+                getUserDevices("@alice:localhost")
+            } returns listOf(
+                    fakeDevice("@alice:localhost", "A0", false),
+                    fakeDevice("@alice:localhost", "A1", false)
+            )
+
+            coEvery {
+                getIdentity("@bob:localhost")
+            } returns fakeIdentity(isVerified = false, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@bob:localhost")
+            } returns listOf(fakeDevice("@bob:localhost", "B0", false))
+
+            coEvery {
+                getIdentity("@charly:localhost")
+            } returns fakeIdentity(isVerified = false, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@charly:localhost")
+            } returns listOf(fakeDevice("@charly:localhost", "C0", false))
+        }
+
+        val computeShieldOp = ComputeShieldForGroupUseCase("@me:localhost")
+
+        val shield = computeShieldOp.invoke(mockMachine, listOf("@alice:localhost", "@bob:localhost", "@charly:localhost"))
+
+        shield shouldBeEqualTo RoomEncryptionTrustLevel.Warning
+    }
+
+    @Test
+    fun shouldReturnRedShieldWhenVerifiedUserHaveInsecureDevices() = runTest {
+        val mockMachine = mockk<OlmMachine> {
+            coEvery {
+                getIdentity("@me:localhost")
+            } returns mockk<UserIdentities>(relaxed = true)
+
+            // Alice is verified
+            coEvery {
+                getIdentity("@alice:localhost")
+            } returns fakeIdentity(isVerified = true, hasVerificationViolation = false)
+
+            // And has an insecure device
+            coEvery {
+                getUserDevices("@alice:localhost")
+            } returns listOf(
+                    fakeDevice("@alice:localhost", "A0", true),
+                    fakeDevice("@alice:localhost", "A1", false)
+            )
+
+            coEvery {
+                getIdentity("@bob:localhost")
+            } returns fakeIdentity(isVerified = false, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@bob:localhost")
+            } returns listOf(fakeDevice("@bob:localhost", "B0", false))
+
+            coEvery {
+                getIdentity("@charly:localhost")
+            } returns fakeIdentity(isVerified = false, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@charly:localhost")
+            } returns listOf(fakeDevice("@charly:localhost", "C0", false))
+        }
+
+        val computeShieldOp = ComputeShieldForGroupUseCase("@me:localhost")
+
+        val shield = computeShieldOp.invoke(mockMachine, listOf("@alice:localhost", "@bob:localhost", "@charly:localhost"))
+
+        shield shouldBeEqualTo RoomEncryptionTrustLevel.Warning
+    }
+
+    @Test
+    fun shouldReturnGreenShieldWhenAllUsersAreVerifiedAndHaveSecuredDevices() = runTest {
+        val mockMachine = mockk<OlmMachine> {
+            coEvery {
+                getIdentity("@me:localhost")
+            } returns mockk<UserIdentities>(relaxed = true)
+
+            // Alice is verified
+            coEvery {
+                getIdentity("@alice:localhost")
+            } returns fakeIdentity(isVerified = true, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@alice:localhost")
+            } returns listOf(
+                    fakeDevice("@alice:localhost", "A0", true),
+                    fakeDevice("@alice:localhost", "A1", false)
+            )
+
+            coEvery {
+                getIdentity("@bob:localhost")
+            } returns fakeIdentity(isVerified = true, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@bob:localhost")
+            } returns listOf(fakeDevice("@bob:localhost", "B0", true))
+
+            coEvery {
+                getIdentity("@charly:localhost")
+            } returns fakeIdentity(isVerified = true, hasVerificationViolation = false)
+
+            coEvery {
+                getUserDevices("@charly:localhost")
+            } returns listOf(fakeDevice("@charly:localhost", "C0", true))
+        }
+
+        val computeShieldOp = ComputeShieldForGroupUseCase("@me:localhost")
+
+        val shield = computeShieldOp.invoke(mockMachine, listOf("@alice:localhost", "@bob:localhost", "@charly:localhost"))
+
+        shield shouldBeEqualTo RoomEncryptionTrustLevel.Warning
+    }
+
+    companion object {
+        internal fun fakeDevice(userId: String, deviceId: String, isSecure: Boolean) = mockk<Device>(relaxed = true) {
+            every { toCryptoDeviceInfo() } returns CryptoDeviceInfo(
+                    deviceId = deviceId,
+                    userId = userId,
+                    trustLevel = DeviceTrustLevel(
+                            crossSigningVerified = isSecure, locallyVerified = null
+                    )
+            )
+        }
+
+        internal fun fakeIdentity(isVerified: Boolean, hasVerificationViolation: Boolean) = mockk<UserIdentities>(relaxed = true) {
+            coEvery { toMxCrossSigningInfo() } returns mockk<MXCrossSigningInfo> {
+                every { wasTrustedOnce } returns hasVerificationViolation
+                every { isTrusted() } returns isVerified
+            }
+        }
+    }
+}

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/crosssigning/XSigningTest.kt

@@ -24,7 +24,6 @@ import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Assert.fail
-import org.junit.Assume
 import org.junit.FixMethodOrder
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -202,9 +201,6 @@ class XSigningTest : InstrumentedTest {
         val aliceSession = cryptoTestData.firstSession
         val bobSession = cryptoTestData.secondSession
 
-        // Remove when https://github.com/matrix-org/matrix-rust-sdk/issues/1129
-        Assume.assumeTrue("Not yet supported by rust", aliceSession.cryptoService().name() != "rust-sdk")
-
         val aliceAuthParams = UserPasswordAuth(
                 user = aliceSession.myUserId,
                 password = TestConstants.PASSWORD

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/ComputeShieldForGroupUseCase.kt

@@ -29,7 +29,10 @@ internal class ComputeShieldForGroupUseCase @Inject constructor(
         val myIdentity = olmMachine.getIdentity(myUserId)
         val allTrustedUserIds = userIds
                 .filter { userId ->
-                    olmMachine.getIdentity(userId)?.verified() == true
+                    val identity = olmMachine.getIdentity(userId)?.toMxCrossSigningInfo()
+                    identity?.isTrusted() == true ||
+                            // Always take into account users that was previously verified but are not anymore
+                            identity?.wasTrustedOnce == true
                 }
 
         return if (allTrustedUserIds.isEmpty()) {

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/GetUserIdentityUseCase.kt

@@ -66,7 +66,8 @@ internal class GetUserIdentityUseCase @Inject constructor(
                         innerMachine = innerMachine,
                         requestSender = requestSender,
                         coroutineDispatchers = coroutineDispatchers,
-                        verificationRequestFactory = verificationRequestFactory
+                        verificationRequestFactory = verificationRequestFactory,
+                        hasVerificationViolation = identity.hasVerificationViolation
                 )
             }
             is InnerUserIdentity.Own -> {
@@ -89,7 +90,8 @@ internal class GetUserIdentityUseCase @Inject constructor(
                         innerMachine = innerMachine,
                         requestSender = requestSender,
                         coroutineDispatchers = coroutineDispatchers,
-                        verificationRequestFactory = verificationRequestFactory
+                        verificationRequestFactory = verificationRequestFactory,
+                        hasVerificationViolation = identity.hasVerificationViolation
                 )
             }
             null                             -> null

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt

@@ -84,7 +84,9 @@ import org.matrix.rustcomponents.sdk.crypto.ShieldState
 import org.matrix.rustcomponents.sdk.crypto.SignatureVerification
 import org.matrix.rustcomponents.sdk.crypto.setLogger
 import timber.log.Timber
+import uniffi.matrix_sdk_crypto.DecryptionSettings
 import uniffi.matrix_sdk_crypto.LocalTrust
+import uniffi.matrix_sdk_crypto.TrustRequirement
 import java.io.File
 import java.nio.charset.Charset
 import javax.inject.Inject
@@ -450,7 +452,12 @@ internal class OlmMachine @Inject constructor(
                     }
 
                     val serializedEvent = adapter.toJson(event)
-                    val decrypted = inner.decryptRoomEvent(serializedEvent, event.roomId, false, false)
+                    val decrypted = inner.decryptRoomEvent(
+                            serializedEvent, event.roomId,
+                            handleVerificationEvents = false,
+                            strictShields = false,
+                            decryptionSettings = DecryptionSettings(TrustRequirement.UNTRUSTED)
+                    )
 
                     val deserializationAdapter =
                             moshi.adapter<JsonDict>(Map::class.java)

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustCrossSigningService.kt

@@ -58,7 +58,7 @@ internal class RustCrossSigningService @Inject constructor(
      * Checks that my trusted user key has signed the other user UserKey
      */
     override suspend fun checkUserTrust(otherUserId: String): UserTrustResult {
-        val identity = olmMachine.getIdentity(olmMachine.userId())
+        val identity = olmMachine.getIdentity(otherUserId)
 
         // While UserTrustResult has many different states, they are by the callers
         // converted to a boolean value immediately, thus we don't need to support

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt

@@ -85,6 +85,7 @@ internal class OwnUserIdentity(
         private val requestSender: RequestSender,
         private val coroutineDispatchers: MatrixCoroutineDispatchers,
         private val verificationRequestFactory: VerificationRequest.Factory,
+        private val hasVerificationViolation: Boolean
 ) : UserIdentities() {
     /**
      * Our own user id.
@@ -157,8 +158,7 @@ internal class OwnUserIdentity(
         userSigningKey.trustLevel = trustLevel
 
         val crossSigningKeys = listOf(masterKey, selfSigningKey, userSigningKey)
-        // TODO https://github.com/matrix-org/matrix-rust-sdk/issues/1129
+        return MXCrossSigningInfo(userId, crossSigningKeys, hasVerificationViolation)
-        return MXCrossSigningInfo(userId, crossSigningKeys, false)
     }
 }
 
@@ -175,6 +175,7 @@ internal class UserIdentity(
         private val requestSender: RequestSender,
         private val coroutineDispatchers: MatrixCoroutineDispatchers,
         private val verificationRequestFactory: VerificationRequest.Factory,
+        private val hasVerificationViolation: Boolean
 ) : UserIdentities() {
     /**
      * The unique ID of the user that this identity belongs to.
@@ -256,8 +257,7 @@ internal class UserIdentity(
                         masterKey.also { it.trustLevel = trustLevel },
                         selfSigningKey.also { it.trustLevel = trustLevel },
                 ),
-                // TODO https://github.com/matrix-org/matrix-rust-sdk/issues/1129
+                wasTrustedOnce = hasVerificationViolation
-                wasTrustedOnce = false
         )
     }
 }