From bb46dcb262a76467af667a691267a829652bc87a Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 31 Oct 2024 18:17:37 +0100 Subject: [PATCH 1/4] feat(crypto): Add support for verification violation warnings --- matrix-sdk-android/build.gradle | 2 +- .../sdk/internal/crypto/crosssigning/XSigningTest.kt | 4 ---- .../sdk/internal/crypto/ComputeShieldForGroupUseCase.kt | 5 ++++- .../sdk/internal/crypto/GetUserIdentityUseCase.kt | 6 ++++-- .../org/matrix/android/sdk/internal/crypto/OlmMachine.kt | 9 ++++++++- .../sdk/internal/crypto/RustCrossSigningService.kt | 2 +- .../matrix/android/sdk/internal/crypto/UserIdentities.kt | 8 ++++---- 7 files changed, 22 insertions(+), 14 deletions(-) diff --git a/matrix-sdk-android/build.gradle b/matrix-sdk-android/build.gradle index c51228f0c11..d3e50c07f2c 100644 --- a/matrix-sdk-android/build.gradle +++ b/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 diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/crosssigning/XSigningTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/crosssigning/XSigningTest.kt index 12c63edf92d..88005b7b5c0 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/crosssigning/XSigningTest.kt +++ b/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 diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/ComputeShieldForGroupUseCase.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/ComputeShieldForGroupUseCase.kt index 75575b14c34..5b0ad1a1836 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/ComputeShieldForGroupUseCase.kt +++ b/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()) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/GetUserIdentityUseCase.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/GetUserIdentityUseCase.kt index 0725edbc88c..6458ad998b1 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/GetUserIdentityUseCase.kt +++ b/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 diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt index c45f85671e5..a3c68c22300 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt +++ b/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(Map::class.java) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustCrossSigningService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustCrossSigningService.kt index e2def5af8a9..e1c323e01c5 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustCrossSigningService.kt +++ b/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 diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt index 8d70482ae1a..e4c7bcfcc3f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt +++ b/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, false) + return MXCrossSigningInfo(userId, crossSigningKeys, hasVerificationViolation) } } @@ -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 = false + wasTrustedOnce = hasVerificationViolation ) } } From 3df42faf32fbdcfcbd93a52d1681f7c72ff676fa Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 4 Nov 2024 08:40:01 +0100 Subject: [PATCH 2/4] Add changelog --- changelog.d/8933.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8933.bugfix diff --git a/changelog.d/8933.bugfix b/changelog.d/8933.bugfix new file mode 100644 index 00000000000..818adad8623 --- /dev/null +++ b/changelog.d/8933.bugfix @@ -0,0 +1 @@ +Show a notice when a previously verified user is not anymore From 5dd58547cc1d70dd69dbb3aa913dc435c59a727d Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 4 Nov 2024 10:42:59 +0100 Subject: [PATCH 3/4] test: Add compute room shields tests --- .../ComputeShieldForGroupUseCaseTest.kt | 262 ++++++++++++++++++ 1 file changed, 262 insertions(+) create mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/ComputeShieldForGroupUseCaseTest.kt diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/ComputeShieldForGroupUseCaseTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/ComputeShieldForGroupUseCaseTest.kt new file mode 100644 index 00000000000..d7094ee62d9 --- /dev/null +++ b/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 { + coEvery { + getIdentity("@me:localhost") + } returns mockk(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 { + coEvery { + getIdentity("@me:localhost") + } returns mockk(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 { + coEvery { + getIdentity("@me:localhost") + } returns mockk(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 { + coEvery { + getIdentity("@me:localhost") + } returns mockk(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 { + coEvery { + getIdentity("@me:localhost") + } returns mockk(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(relaxed = true) { + every { toCryptoDeviceInfo() } returns CryptoDeviceInfo( + deviceId = deviceId, + userId = userId, + trustLevel = DeviceTrustLevel( + crossSigningVerified = isSecure, locallyVerified = null + ) + ) + } + + internal fun fakeIdentity(isVerified: Boolean, hasVerificationViolation: Boolean) = mockk(relaxed = true) { + coEvery { toMxCrossSigningInfo() } returns mockk { + every { wasTrustedOnce } returns hasVerificationViolation + every { isTrusted() } returns isVerified + } + } + } +} From 90aed72c72bdfbc07778ce7d77f3fbb1389d70b5 Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 4 Nov 2024 17:52:02 +0100 Subject: [PATCH 4/4] CI update setup-matrix-synapse to 1.0.5 --- .github/workflows/post-pr.yml | 2 +- .github/workflows/tests.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/post-pr.yml b/.github/workflows/post-pr.yml index 023945c4ee1..1303b89e6af 100644 --- a/.github/workflows/post-pr.yml +++ b/.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 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ffe703c6062..de4b0b0aa70 100644 --- a/.github/workflows/tests.yml +++ b/.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