From bb16d77ec6e5f010caa6c4a52c43f9ca2ca16cd2 Mon Sep 17 00:00:00 2001 From: valere Date: Wed, 30 Nov 2022 00:04:13 +0100 Subject: [PATCH] fix QR verification --- .../VerificationListenersHolder.kt | 2 +- .../sdk/internal/crypto/UserIdentities.kt | 4 +- .../verification/RustVerificationService.kt | 37 ++++-- .../verification/VerificationRequest.kt | 112 +++++------------- .../verification/VerificationsProvider.kt | 25 +--- .../verification/qrcode/QrCodeVerification.kt | 54 +++------ 6 files changed, 79 insertions(+), 155 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/VerificationListenersHolder.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/VerificationListenersHolder.kt index e4146fb9da..d44a15b1b7 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/VerificationListenersHolder.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/VerificationListenersHolder.kt @@ -44,7 +44,7 @@ internal class VerificationListenersHolder @Inject constructor( } fun dispatchTxUpdated(tx: VerificationTransaction) { - Timber.v("## SAS dispatchRequestAdded txId:${tx.transactionId} $tx") + Timber.v("## SAS dispatchTxUpdated txId:${tx.transactionId} $tx") scope.launch { eventFlow.emit(VerificationEvent.TransactionUpdated(tx)) } diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt index d0422834db..8d70482ae1 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt @@ -236,8 +236,8 @@ internal class UserIdentity( ): VerificationRequest { val stringMethods = prepareMethods(methods) val content = innerMachine.verificationRequestContent(userId, stringMethods)!! - val eventID = requestSender.sendRoomMessage(EventType.MESSAGE, roomId, content, transactionId).eventId - val innerRequest = innerMachine.requestVerification(userId, roomId, eventID, stringMethods)!! + val eventId = requestSender.sendRoomMessage(EventType.MESSAGE, roomId, content, transactionId).eventId + val innerRequest = innerMachine.requestVerification(userId, roomId, eventId, stringMethods)!! return verificationRequestFactory.create(innerRequest) } diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt index 4368e08a70..31795d3c4d 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt @@ -116,32 +116,45 @@ internal class RustVerificationService @Inject constructor( val sender = event.senderId ?: return val flowId = getFlowId(event) ?: return - olmMachine.getVerificationRequest(sender, flowId)?.dispatchRequestUpdated() + val verificationRequest = olmMachine.getVerificationRequest(sender, flowId) + if (event.getClearType() == EventType.KEY_VERIFICATION_READY) { + verificationRequest?.startQrCode() + } + verificationRequest?.dispatchRequestUpdated() val verification = getExistingTransaction(sender, flowId) ?: return verificationListenersHolder.dispatchTxUpdated(verification) } /** Check if the start event created new verification objects and dispatch updates */ private suspend fun onStart(event: Event) { + if (event.unsignedData?.transactionId != null) return // remote echo Timber.w("VALR onStart $event") val sender = event.senderId ?: return val flowId = getFlowId(event) ?: return - val transaction = getExistingTransaction(sender, flowId) ?: return + // The events have already been processed by the sdk + // The transaction are already created, we are just reacting here + val transaction = getExistingTransaction(sender, flowId) ?: return Unit.also { + Timber.w("onStart for unknown flowId $flowId senderId $sender") + } val request = olmMachine.getVerificationRequest(sender, flowId) + Timber.d("## Verification: matching request $request") - if (request != null && request.isReady()) { + if (request != null) { // 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. If it's a QR code verification, just dispatch an update. - if (transaction is SasVerification) { + if (request.isReady() && transaction is SasVerification) { // accept() will dispatch an update, no need to do it twice. Timber.d("## Verification: Auto accepting SAS verification with $sender") transaction.accept() - } else { - verificationListenersHolder.dispatchTxUpdated(transaction) } + + Timber.d("## Verification: start for $sender") + // update the request as the start updates it's state + request.dispatchRequestUpdated() + verificationListenersHolder.dispatchTxUpdated(transaction) } else { // This didn't originate from a request, so tell our listeners that // this is a new verification. @@ -249,7 +262,7 @@ internal class RustVerificationService @Inject constructor( null -> throw IllegalArgumentException("The user that we wish to verify doesn't support cross signing") } - Timber.w("##VALR requestKeyVerificationInDMs $verification") + Timber.w("##VALR requestKeyVerificationInDMs ${verification.flowId()} > $verification") return verification.toPendingVerificationRequest() } @@ -282,7 +295,7 @@ internal class RustVerificationService @Inject constructor( val request = olmMachine.getVerificationRequest(otherUserId, transactionId) return if (request != null) { request.acceptWithMethods(methods) - + request.startQrCode() request.isReady() } else { false @@ -308,8 +321,12 @@ internal class RustVerificationService @Inject constructor( override suspend fun reciprocateQRVerification(otherUserId: String, requestId: String, scannedData: String): String? { val matchingRequest = olmMachine.getVerificationRequest(otherUserId, requestId) - matchingRequest?.scanQrCode(scannedData) - return matchingRequest?.startQrVerification()?.transactionId + ?: return null + val qrVerification = matchingRequest.scanQrCode(scannedData) + ?: return null + verificationListenersHolder.dispatchRequestUpdated(matchingRequest.toPendingVerificationRequest()) + verificationListenersHolder.dispatchTxAdded(qrVerification) + return qrVerification.transactionId } override suspend fun onPotentiallyInterestingEventRoomFailToDecrypt(event: Event) { diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationRequest.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationRequest.kt index 18acef65d7..34e05f436a 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationRequest.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationRequest.kt @@ -28,6 +28,7 @@ import org.matrix.android.sdk.api.session.crypto.verification.EVerificationState import org.matrix.android.sdk.api.session.crypto.verification.PendingVerificationRequest import org.matrix.android.sdk.api.session.crypto.verification.VerificationMethod import org.matrix.android.sdk.api.session.crypto.verification.safeValueOf +import org.matrix.android.sdk.api.util.fromBase64 import org.matrix.android.sdk.api.util.toBase64NoPadding import org.matrix.android.sdk.internal.crypto.OlmMachine import org.matrix.android.sdk.internal.crypto.model.rest.VERIFICATION_METHOD_QR_CODE_SCAN @@ -37,7 +38,6 @@ import org.matrix.android.sdk.internal.crypto.model.rest.VERIFICATION_METHOD_SAS import org.matrix.android.sdk.internal.crypto.network.RequestSender import org.matrix.android.sdk.internal.crypto.verification.qrcode.QrCodeVerification import org.matrix.android.sdk.internal.util.time.Clock -import org.matrix.rustcomponents.sdk.crypto.QrCode import org.matrix.rustcomponents.sdk.crypto.VerificationRequest as InnerVerificationRequest fun InnerVerificationRequest.dbgString(): String { @@ -78,6 +78,10 @@ internal class VerificationRequest @AssistedInject constructor( fun create(innerVerificationRequest: InnerVerificationRequest): VerificationRequest } + fun startQrCode() { + innerVerificationRequest.startQrVerification() + } + internal fun dispatchRequestUpdated() { val tx = toPendingVerificationRequest() verificationListenersHolder.dispatchRequestUpdated(tx) @@ -156,25 +160,19 @@ internal class VerificationRequest @AssistedInject constructor( val request = innerVerificationRequest.accept(stringMethods) ?: return // should throw here? -// val request = innerOlmMachine.acceptVerificationRequest( -// innerVerificationRequest.otherUserId(), -// innerVerificationRequest.flowId, -// stringMethods -// ) ?: return - try { dispatchRequestUpdated() requestSender.sendVerificationRequest(request) - if (innerVerificationRequest.isReady()) { - activeQRCode = innerVerificationRequest.startQrVerification() - } +// if (innerVerificationRequest.isReady()) { +// activeQRCode = innerVerificationRequest.startQrVerification() +// } } catch (failure: Throwable) { cancel(CancelCode.UserError) } } - var activeQRCode: QrCode? = null +// var activeQRCode: QrCode? = null /** Transition from a ready verification request into emoji verification * @@ -230,36 +228,7 @@ internal class VerificationRequest @AssistedInject constructor( cancel(CancelCode.UserError) return null } - return qrCodeVerificationFactory.create(this, result.qr) - } - - /** Transition into a QR code verification to display a QR code - * - * This method will move the verification forward into QR code verification. - * It will not send out any event out, it should instead be used to display - * a QR code which then can be scanned out of bound by the other side. - * - * A m.key.verification.start event with the method set to m.reciprocate.v1 - * incoming from the other side will only be accepted if this method is called - * and the QR code verification is successfully initiated. - * - * Note: This method will be a noop and return null if the verification request - * isn't considered to be ready, you can check if the request is ready using the - * isReady() method. - * - * @return A freshly created QrCodeVerification object that represents the newly started - * QR code verification, or null if we can't yet transition into QR code verification. - */ - internal fun startQrVerification(): QrCodeVerification? { - activeQRCode = innerVerificationRequest.startQrVerification() -// val qrcode = innerOlmMachine.startQrVerification(innerVerificationRequest.otherUserId, innerVerificationRequest.flowId) - return if (activeQRCode != null) { - TODO("Is this reciprocate or just doing nothing?") -// activeQRCode. -// qrCodeVerificationFactory.create(this, qrcode) - } else { - null - } + return qrCodeVerificationFactory.create(result.qr) } /** Cancel the verification flow @@ -316,7 +285,18 @@ internal class VerificationRequest @AssistedInject constructor( EVerificationState.Started } } - // TODO QR?? + val asQR = started.asQr() + if (asQR != null) { +// Timber.w("VALR: weStarted ${asQR.weStarted()}") +// Timber.w("VALR: reciprocated ${asQR.reciprocated()}") +// Timber.w("VALR: isDone ${asQR.isDone()}") +// Timber.w("VALR: hasBeenScanned ${asQR.hasBeenScanned()}") + if (asQR.reciprocated() || asQR.hasBeenScanned()) { + return if (weStarted()) { + EVerificationState.WeStarted + } else EVerificationState.Started + } + } } if (innerVerificationRequest.isReady()) { return EVerificationState.Ready @@ -351,45 +331,6 @@ internal class VerificationRequest @AssistedInject constructor( val theirMethods = innerVerificationRequest.theirSupportedMethods() val otherDeviceId = innerVerificationRequest.otherDeviceId() -// var requestInfo: ValidVerificationInfoRequest? = null -// var readyInfo: ValidVerificationInfoReady? = null -// -// if (innerVerificationRequest.weStarted && ourMethods != null) { -// requestInfo = -// ValidVerificationInfoRequest( -// transactionId = innerVerificationRequest.flowId, -// fromDevice = innerOlmMachine.deviceId(), -// methods = ourMethods, -// timestamp = null, -// ) -// } else if (!innerVerificationRequest.weStarted && ourMethods != null) { -// readyInfo = -// ValidVerificationInfoReady( -// transactionId = innerVerificationRequest.flowId, -// fromDevice = innerOlmMachine.deviceId(), -// methods = ourMethods, -// ) -// } -// -// if (innerVerificationRequest.weStarted && theirMethods != null && otherDeviceId != null) { -// readyInfo = -// ValidVerificationInfoReady( -// transactionId = innerVerificationRequest.flowId, -// fromDevice = otherDeviceId, -// methods = theirMethods, -// ) -// } else if (!innerVerificationRequest.weStarted && theirMethods != null && otherDeviceId != null) { -// requestInfo = -// ValidVerificationInfoRequest( -// transactionId = innerVerificationRequest.flowId, -// fromDevice = otherDeviceId, -// methods = theirMethods, -// timestamp = clock.epochMillis(), -// ) -// } - - innerVerificationRequest.startQrVerification() - return PendingVerificationRequest( // Creation time ageLocalTs = clock.epochMillis(), @@ -411,14 +352,19 @@ internal class VerificationRequest @AssistedInject constructor( handledByOtherSession = innerVerificationRequest.isPassive(), // devices that should receive the events we send out targetDevices = otherDeviceId?.let { listOf(it) }, - // TODO qr, - qrCodeText = activeQRCode?.generateQrCode(), + qrCodeText = getQrCode(), isSasSupported = ourMethods.canSas() && theirMethods.canSas(), weShouldDisplayQRCode = theirMethods.canScanQR() && ourMethods.canShowQR(), weShouldShowScanOption = ourMethods.canScanQR() && theirMethods.canShowQR() ) } + private fun getQrCode(): String? { + return innerOlmMachine.getVerification(otherUser(), flowId())?.asQr()?.generateQrCode()?.fromBase64()?.let { + String(it, Charsets.ISO_8859_1) + } + } + override fun toString(): String { return super.toString() + "\n${innerVerificationRequest.dbgString()}" } diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationsProvider.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationsProvider.kt index a30c05884e..7544368ef7 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationsProvider.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationsProvider.kt @@ -53,32 +53,9 @@ internal class VerificationsProvider @Inject constructor( return if (verification?.asSas() != null) { sasVerificationFactory.create(verification.asSas()!!) } else if (verification?.asQr() != null) { - // qrVerificationFactory.create(verification, verification.asQr()!!) - // TODO - null + qrVerificationFactory.create(verification.asQr()!!) } else { null } -// return when (val verification = innerMachine.getVerification(userId, flowId)) { -// is org.matrix.rustcomponents.sdk.crypto.Verification. -> { -// val request = getVerificationRequest(userId, flowId) ?: return null -// qrVerificationFactory.create(request, verification.qrcode) -// } -// is org.matrix.rustcomponents.sdk.crypto.Verification.SasV1 -> { -// sasVerificationFactory.create(verification.sas) -// } -// 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 -// // QrCodeVerification object that gets populated once a QR code is scanned. -// val request = getVerificationRequest(userId, flowId) ?: return null -// if (request.canScanQrCodes()) { -// qrVerificationFactory.create(request, null) -// } else { -// null -// } -// } -// } } } diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/qrcode/QrCodeVerification.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/qrcode/QrCodeVerification.kt index fbc87e1b8a..ac79668db9 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/qrcode/QrCodeVerification.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/qrcode/QrCodeVerification.kt @@ -31,13 +31,12 @@ import org.matrix.android.sdk.api.util.fromBase64 import org.matrix.android.sdk.internal.crypto.OlmMachine import org.matrix.android.sdk.internal.crypto.network.RequestSender import org.matrix.android.sdk.internal.crypto.verification.VerificationListenersHolder -import org.matrix.android.sdk.internal.crypto.verification.VerificationRequest import org.matrix.rustcomponents.sdk.crypto.CryptoStoreException import org.matrix.rustcomponents.sdk.crypto.QrCode +import timber.log.Timber /** Class representing a QR code based verification flow */ internal class QrCodeVerification @AssistedInject constructor( - @Assisted private var request: VerificationRequest, @Assisted private var inner: QrCode, private val olmMachine: OlmMachine, private val sender: RequestSender, @@ -47,7 +46,7 @@ internal class QrCodeVerification @AssistedInject constructor( @AssistedFactory interface Factory { - fun create(request: VerificationRequest, inner: QrCode): QrCodeVerification + fun create(inner: QrCode): QrCodeVerification } override val method: VerificationMethod @@ -98,53 +97,38 @@ internal class QrCodeVerification @AssistedInject constructor( } override fun state(): QRCodeVerificationState { + Timber.w("VALR state: weStarted ${inner.weStarted()}") + Timber.w("VALR state: reciprocated ${inner.reciprocated()}") + Timber.w("VALR state: isDone ${inner.isDone()}") + Timber.w("VALR state: hasBeenScanned ${inner.hasBeenScanned()}") + + if (inner.hasBeenScanned()) { + return QRCodeVerificationState.WaitingForScanConfirmation + } + if (inner.isCancelled()) return QRCodeVerificationState.Cancelled + if (inner.isDone()) return QRCodeVerificationState.Done + return QRCodeVerificationState.Reciprocated } -// override var state: VerificationTxState -// get() { -// refreshData() -// val inner = inner -// val cancelInfo = inner?.cancelInfo -// -// return if (inner != null) { -// when { -// cancelInfo != null -> { -// val cancelCode = safeValueOf(cancelInfo.cancelCode) -// val byMe = cancelInfo.cancelledByUs -// VerificationTxState.Cancelled(cancelCode, byMe) -// } -// inner.isDone -> VerificationTxState.Verified -// inner.reciprocated -> VerificationTxState.Started -// inner.hasBeenConfirmed -> VerificationTxState.WaitingOtherReciprocateConfirm -// inner.otherSideScanned -> VerificationTxState.QrScannedByOther -// else -> VerificationTxState.None -// } -// } else { -// VerificationTxState.None -// } -// } -// @Suppress("UNUSED_PARAMETER") -// set(value) { -// } /** Get the unique id of this verification */ override val transactionId: String - get() = request.flowId() + get() = inner.flowId() /** Get the user id of the other user participating in this verification flow */ override val otherUserId: String - get() = request.otherUser() + get() = inner.otherUserId() /** Get the device id of the other user's device participating in this verification flow */ override var otherDeviceId: String? - get() = request.otherDeviceId() + get() = inner.otherDeviceId() @Suppress("UNUSED_PARAMETER") set(value) { } /** Did the other side initiate this verification flow */ override val isIncoming: Boolean - get() = !request.weStarted() + get() = !inner.weStarted() /** Cancel the verification flow * @@ -176,7 +160,7 @@ internal class QrCodeVerification @AssistedInject constructor( /** Is this verification happening over to-device messages */ override fun isToDeviceTransport(): Boolean { - return request.roomId() == null + return inner.roomId() == null } /** Confirm the QR code verification @@ -216,7 +200,7 @@ internal class QrCodeVerification @AssistedInject constructor( /** Fetch fresh data from the Rust side for our verification flow */ private fun refreshData() { - innerMachine.getVerification(request.otherUser(), request.flowId()) + innerMachine.getVerification(inner.otherUserId(), inner.flowId()) ?.asQr()?.let { inner = it }