From 93f36db43c7527532ab426c6f040fe83e19ca3d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 20 Jul 2021 16:35:50 +0200 Subject: [PATCH] crypto: Add proper scopes to our verification methods --- .../sdk/internal/crypto/QrCodeVerification.kt | 18 +-- .../internal/crypto/RustSasVerification.kt | 80 ++++++------- .../internal/crypto/VerificationRequest.kt | 111 ++++++++---------- .../verification/RustVerificationService.kt | 39 +++--- 4 files changed, 117 insertions(+), 131 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt index 209f060e0a..00d00bed5e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt @@ -156,7 +156,7 @@ internal class QrCodeVerification( * This confirms that the other side has scanned our QR code. */ @Throws(CryptoStoreErrorException::class) - suspend fun confirm() { + private suspend fun confirm() { val request = withContext(Dispatchers.IO) { machine.confirmVerification(request.otherUser(), request.flowId()) @@ -167,14 +167,6 @@ internal class QrCodeVerification( } } - /** Send out a verification request in a blocking manner*/ - private fun sendRequest(request: OutgoingVerificationRequest) { - runBlocking { sender.sendVerificationRequest(request) } - - refreshData() - dispatchTxUpdated() - } - private fun cancelHelper(code: CancelCode) { val request = this.machine.cancelVerification(this.request.otherUser(), this.request.flowId(), code.value) @@ -183,6 +175,14 @@ internal class QrCodeVerification( } } + /** Send out a verification request in a blocking manner*/ + private fun sendRequest(request: OutgoingVerificationRequest) { + runBlocking { sender.sendVerificationRequest(request) } + + refreshData() + dispatchTxUpdated() + } + /** Fetch fetch data from the Rust side for our verification flow */ private fun refreshData() { when (val verification = this.machine.getVerification(this.request.otherUser(), this.request.flowId())) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustSasVerification.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustSasVerification.kt index 839bdba5f8..4080a3355e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustSasVerification.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustSasVerification.kt @@ -56,18 +56,6 @@ internal class SasVerification( } } - private fun refreshData() { - when (val verification = this.machine.getVerification(this.inner.otherUserId, this.inner.flowId)) { - is Verification.SasV1 -> { - this.inner = verification.sas - } - else -> { - } - } - - return - } - override val isIncoming: Boolean get() = !this.inner.weStarted @@ -84,7 +72,7 @@ internal class SasVerification( refreshData() val cancelInfo = this.inner.cancelInfo return when { - cancelInfo != null -> { + cancelInfo != null -> { val cancelCode = safeValueOf(cancelInfo.cancelCode) VerificationTxState.Cancelled(cancelCode, cancelInfo.cancelledByUs) } @@ -118,9 +106,8 @@ internal class SasVerification( override fun supportsDecimal(): Boolean { // This is ignored anyways, throw it away? - // The spec also mandates that devices support - // at least decimal and the rust-sdk cancels if - // devices don't support it + // The spec also mandates that devices support at least decimal and + // the rust-sdk cancels if devices don't support it return true } @@ -130,15 +117,26 @@ internal class SasVerification( } override fun userHasVerifiedShortCode() { - val request = runBlocking { confirm() } ?: return - sendRequest(request) + runBlocking { confirm() } } override fun acceptVerification() { runBlocking { accept() } } - suspend fun accept() { + override fun getDecimalCodeRepresentation(): String { + val decimals = this.machine.getDecimals(this.inner.otherUserId, this.inner.flowId) + + return decimals?.joinToString(" ") ?: "" + } + + override fun getEmojiCodeRepresentation(): List { + val emojiIndex = this.machine.getEmojiIndex(this.inner.otherUserId, this.inner.flowId) + + return emojiIndex?.map { getEmojiForCode(it) } ?: listOf() + } + + internal suspend fun accept() { val request = this.machine.acceptSasVerification(this.inner.otherUserId, inner.flowId) if (request != null) { @@ -149,12 +147,16 @@ internal class SasVerification( } @Throws(CryptoStoreErrorException::class) - suspend fun confirm(): OutgoingVerificationRequest? = - withContext(Dispatchers.IO) { - machine.confirmVerification(inner.otherUserId, inner.flowId) - } + private suspend fun confirm() { + val request = withContext(Dispatchers.IO) { + machine.confirmVerification(inner.otherUserId, inner.flowId) + } + if (request != null) { + this.sender.sendVerificationRequest(request) + } + } - fun cancelHelper(code: CancelCode) { + private fun cancelHelper(code: CancelCode) { val request = this.machine.cancelVerification(this.inner.otherUserId, inner.flowId, code.value) if (request != null) { @@ -162,24 +164,22 @@ internal class SasVerification( } } - override fun getEmojiCodeRepresentation(): List { - val emojiIndex = this.machine.getEmojiIndex(this.inner.otherUserId, this.inner.flowId) - - return emojiIndex?.map { getEmojiForCode(it) } ?: listOf() - } - - override fun getDecimalCodeRepresentation(): String { - val decimals = this.machine.getDecimals(this.inner.otherUserId, this.inner.flowId) - - return decimals?.joinToString(" ") ?: "" - } - - fun sendRequest(request: OutgoingVerificationRequest) { - runBlocking { - sender.sendVerificationRequest(request) - } + private fun sendRequest(request: OutgoingVerificationRequest) { + runBlocking { sender.sendVerificationRequest(request) } refreshData() dispatchTxUpdated() } + + private fun refreshData() { + when (val verification = this.machine.getVerification(this.inner.otherUserId, this.inner.flowId)) { + is Verification.SasV1 -> { + this.inner = verification.sas + } + else -> { + } + } + + return + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/VerificationRequest.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/VerificationRequest.kt index 1adc1e8d0f..5e531ec428 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/VerificationRequest.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/VerificationRequest.kt @@ -34,6 +34,7 @@ import org.matrix.android.sdk.internal.crypto.model.rest.VERIFICATION_METHOD_QR_ import org.matrix.android.sdk.internal.crypto.model.rest.VERIFICATION_METHOD_RECIPROCATE import org.matrix.android.sdk.internal.crypto.model.rest.VERIFICATION_METHOD_SAS import org.matrix.android.sdk.internal.crypto.model.rest.toValue +import org.matrix.android.sdk.internal.crypto.verification.prepareMethods import timber.log.Timber import uniffi.olm.OlmMachine import uniffi.olm.VerificationRequest @@ -46,16 +47,6 @@ internal class VerificationRequest( ) { private val uiHandler = Handler(Looper.getMainLooper()) - private fun refreshData() { - val request = this.machine.getVerificationRequest(this.inner.otherUserId, this.inner.flowId) - - if (request != null) { - this.inner = request - } - - return - } - internal fun dispatchRequestUpdated() { uiHandler.post { listeners.forEach { @@ -68,42 +59,65 @@ internal class VerificationRequest( } } - fun isCanceled(): Boolean { - refreshData() - return this.inner.isCancelled - } - - fun isDone(): Boolean { - refreshData() - return this.inner.isDone - } - - fun flowId(): String { + internal fun flowId(): String { return this.inner.flowId } - fun otherUser(): String { + internal fun otherUser(): String { return this.inner.otherUserId } - fun otherDeviceId(): String? { + internal fun otherDeviceId(): String? { refreshData() return this.inner.otherDeviceId } - fun weStarted(): Boolean { + internal fun weStarted(): Boolean { return this.inner.weStarted } - fun roomId(): String? { + internal fun roomId(): String? { return this.inner.roomId } - fun isReady(): Boolean { + internal fun isReady(): Boolean { refreshData() return this.inner.isReady } + internal fun canScanQrCodes(): Boolean { + refreshData() + return this.inner.ourMethods?.contains(VERIFICATION_METHOD_QR_CODE_SCAN) ?: false + } + + suspend fun acceptWithMethods(methods: List) { + val stringMethods = prepareMethods(methods) + + val request = this.machine.acceptVerificationRequest( + this.inner.otherUserId, + this.inner.flowId, + stringMethods + ) + + if (request != null) { + this.sender.sendVerificationRequest(request) + this.dispatchRequestUpdated() + } + } + + internal suspend fun startSasVerification(): SasVerification? { + return withContext(Dispatchers.IO) { + val result = machine.startSasVerification(inner.otherUserId, inner.flowId) + + if (result != null) { + sender.sendVerificationRequest(result.request) + SasVerification(machine, result.sas, sender, listeners) + } else { + null + } + } + } + internal suspend fun scanQrCode(data: String): QrCodeVerification? { // TODO again, what's the deal with ISO_8859_1? val byteArray = data.toByteArray(Charsets.ISO_8859_1) @@ -130,18 +144,11 @@ internal class VerificationRequest( } } - suspend fun acceptWithMethods(methods: List) { - val stringMethods: MutableList = methods.map { it.toValue() }.toMutableList() - - if (stringMethods.contains(VERIFICATION_METHOD_QR_CODE_SHOW) || - stringMethods.contains(VERIFICATION_METHOD_QR_CODE_SCAN)) { - stringMethods.add(VERIFICATION_METHOD_RECIPROCATE) - } - - val request = this.machine.acceptVerificationRequest( - this.inner.otherUserId, - this.inner.flowId, - stringMethods + internal suspend fun cancel() { + val request = this.machine.cancelVerification( + this.inner.otherUserId, + this.inner.flowId, + CancelCode.User.value ) if (request != null) { @@ -150,35 +157,15 @@ internal class VerificationRequest( } } - suspend fun cancel() { - val request = this.machine.cancelVerification(this.inner.otherUserId, this.inner.flowId, CancelCode.User.value) + private fun refreshData() { + val request = this.machine.getVerificationRequest(this.inner.otherUserId, this.inner.flowId) if (request != null) { - this.sender.sendVerificationRequest(request) - this.dispatchRequestUpdated() + this.inner = request } } - fun canScanQrCodes(): Boolean { - refreshData() - return this.inner.ourMethods?.contains(VERIFICATION_METHOD_QR_CODE_SCAN) ?: false - } - - suspend fun startSasVerification(): SasVerification? { - refreshData() - - return withContext(Dispatchers.IO) { - val result = machine.startSasVerification(inner.otherUserId, inner.flowId) - if (result != null) { - sender.sendVerificationRequest(result.request) - SasVerification(machine, result.sas, sender, listeners) - } else { - null - } - } - } - - fun toPendingVerificationRequest(): PendingVerificationRequest { + internal fun toPendingVerificationRequest(): PendingVerificationRequest { refreshData() val cancelInfo = this.inner.cancelInfo val cancelCode = diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt index ed1363c6e1..2a11b6f540 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt @@ -46,7 +46,7 @@ import timber.log.Timber import uniffi.olm.Verification @JsonClass(generateAdapter = true) -data class ToDeviceVerificationEvent( +internal data class ToDeviceVerificationEvent( @Json(name = "sender") val sender: String?, @Json(name = "transaction_id") val transactionId: String, ) @@ -61,6 +61,17 @@ private fun getFlowId(event: Event): String? { } } +internal fun prepareMethods(methods: List): List { + val stringMethods: MutableList = methods.map { it.toValue() }.toMutableList() + + if (stringMethods.contains(VERIFICATION_METHOD_QR_CODE_SHOW) || + stringMethods.contains(VERIFICATION_METHOD_QR_CODE_SCAN)) { + stringMethods.add(VERIFICATION_METHOD_RECIPROCATE) + } + + return stringMethods +} + internal class RustVerificationService( private val olmMachine: OlmMachine, private val requestSender: RequestSender, @@ -117,7 +128,7 @@ internal class RustVerificationService( } } - suspend fun onEvent(event: Event) = when (event.getClearType()) { + internal suspend fun onEvent(event: Event) = when (event.getClearType()) { MessageType.MSGTYPE_VERIFICATION_REQUEST -> onRequest(event) EventType.KEY_VERIFICATION_START -> onStart(event) EventType.KEY_VERIFICATION_READY, @@ -148,11 +159,11 @@ internal class RustVerificationService( if (request != null && request.isReady()) { // If this is a SAS verification originating from a `m.key.verification.request` - // event we auto-accept here considering that we either initiated the request or - // accepted the request, otherwise it's a QR code verification, just dispatch an update. + // event, we auto-accept here considering that we either initiated the request or + // accepted the request. If it's a QR code verification, just dispatch an update. if (verification is SasVerification) { // Accept dispatches an update, no need to do it twice. - Timber.d("## Verification: Auto accepting SAS verification with $sender") + Timber.d("## Verification: Auto accepting SAS verification with $sender") verification.accept() } else { dispatchTxUpdated(verification) @@ -227,7 +238,7 @@ internal class RustVerificationService( is Verification.SasV1 -> { SasVerification(this.olmMachine.inner(), verification.sas, this.requestSender, this.listeners) } - null -> { + null -> { // This branch exists because scanning a QR code is tied to the QrCodeVerification, // i.e. instead of branching into a scanned QR code verification from the verification request, // like it's done for SAS verifications, the public API expects us to create an empty dummy @@ -296,12 +307,7 @@ internal class RustVerificationService( otherUserId: String, otherDevices: List? ): PendingVerificationRequest { - - val stringMethods: MutableList = methods.map { it.toValue() }.toMutableList() - if (stringMethods.contains(VERIFICATION_METHOD_QR_CODE_SHOW) || - stringMethods.contains(VERIFICATION_METHOD_QR_CODE_SCAN)) { - stringMethods.add(VERIFICATION_METHOD_RECIPROCATE) - } + val stringMethods = prepareMethods(methods) val result = this.olmMachine.inner().requestSelfVerification(stringMethods) runBlocking { @@ -318,13 +324,7 @@ internal class RustVerificationService( localId: String? ): PendingVerificationRequest { Timber.i("## SAS Requesting verification to user: $otherUserId in room $roomId") - val stringMethods: MutableList = methods.map { it.toValue() }.toMutableList() - - if (stringMethods.contains(VERIFICATION_METHOD_QR_CODE_SHOW) || - stringMethods.contains(VERIFICATION_METHOD_QR_CODE_SCAN)) { - stringMethods.add(VERIFICATION_METHOD_RECIPROCATE) - } - + val stringMethods = prepareMethods(methods) val content = this.olmMachine.inner().verificationRequestContent(otherUserId, stringMethods)!! val eventID = runBlocking { @@ -376,7 +376,6 @@ internal class RustVerificationService( otherDeviceId: String, transactionId: String? ): String? { - // should check if already one (and cancel it) return if (method == VerificationMethod.SAS) { if (transactionId != null) { val request = this.getVerificationRequest(otherUserId, transactionId)