diff --git a/crypto/verificationhelper/verificationhelper.go b/crypto/verificationhelper/verificationhelper.go index 1fc1fd22..aa38692a 100644 --- a/crypto/verificationhelper/verificationhelper.go +++ b/crypto/verificationhelper/verificationhelper.go @@ -663,13 +663,7 @@ func (vh *VerificationHelper) onVerificationRequest(ctx context.Context, evt *ev } vh.activeTransactionsLock.Lock() - existing, ok := vh.activeTransactions[verificationRequest.TransactionID] - if ok { - vh.activeTransactionsLock.Unlock() - vh.cancelVerificationTxn(ctx, existing, event.VerificationCancelCodeUnexpectedMessage, "received a new verification request for the same transaction ID") - return - } - vh.activeTransactions[verificationRequest.TransactionID] = &verificationTransaction{ + newTxn := &verificationTransaction{ RoomID: evt.RoomID, VerificationState: verificationStateRequested, TransactionID: verificationRequest.TransactionID, @@ -677,6 +671,23 @@ func (vh *VerificationHelper) onVerificationRequest(ctx context.Context, evt *ev TheirUser: evt.Sender, TheirSupportedMethods: verificationRequest.Methods, } + for existingTxnID, existingTxn := range vh.activeTransactions { + if existingTxn.TheirUser == evt.Sender && existingTxn.TheirDevice == verificationRequest.FromDevice { + vh.cancelVerificationTxn(ctx, existingTxn, event.VerificationCancelCodeUnexpectedMessage, "received multiple verification requests from the same device") + vh.cancelVerificationTxn(ctx, newTxn, event.VerificationCancelCodeUnexpectedMessage, "received multiple verification requests from the same device") + delete(vh.activeTransactions, existingTxnID) + vh.activeTransactionsLock.Unlock() + return + } + + if existingTxnID == verificationRequest.TransactionID { + vh.cancelVerificationTxn(ctx, existingTxn, event.VerificationCancelCodeUnexpectedMessage, "received a new verification request for the same transaction ID") + delete(vh.activeTransactions, existingTxnID) + vh.activeTransactionsLock.Unlock() + return + } + } + vh.activeTransactions[verificationRequest.TransactionID] = newTxn vh.activeTransactionsLock.Unlock() vh.verificationRequested(ctx, verificationRequest.TransactionID, evt.Sender) diff --git a/crypto/verificationhelper/verificationhelper_test.go b/crypto/verificationhelper/verificationhelper_test.go index d6ed9b09..e8be5771 100644 --- a/crypto/verificationhelper/verificationhelper_test.go +++ b/crypto/verificationhelper/verificationhelper_test.go @@ -388,5 +388,58 @@ func TestVerification_ErrorOnDoubleAccept(t *testing.T) { err = receivingHelper.AcceptVerification(ctx, txnID) require.NoError(t, err) err = receivingHelper.AcceptVerification(ctx, txnID) - require.ErrorContains(t, err, "transaction is not in the requested state") + assert.ErrorContains(t, err, "transaction is not in the requested state") +} + +// TestVerification_CancelOnDoubleStart ensures that the receiving device +// cancels both transactions if the sending device starts two verifications. +// +// This test ensures that the following bullet point from [Section 10.12.2.2.1 +// of the Spec] is followed: +// +// - When the same device attempts to initiate multiple verification attempts, +// the recipient should cancel all attempts with that device. +// +// [Section 10.12.2.2.1 of the Spec]: https://spec.matrix.org/v1.10/client-server-api/#error-and-exception-handling +func TestVerification_CancelOnDoubleStart(t *testing.T) { + ctx := log.Logger.WithContext(context.TODO()) + ts, sendingClient, receivingClient, _, _, sendingMachine, receivingMachine := initServerAndLoginTwoAlice(t, ctx) + defer ts.Close() + sendingCallbacks, receivingCallbacks, sendingHelper, receivingHelper := initDefaultCallbacks(t, ctx, sendingClient, receivingClient, sendingMachine, receivingMachine) + + _, _, err := sendingMachine.GenerateAndUploadCrossSigningKeys(ctx, nil, "") + require.NoError(t, err) + + // Send and accept the first verification request. + txnID1, err := sendingHelper.StartVerification(ctx, aliceUserID) + require.NoError(t, err) + ts.dispatchToDevice(t, ctx, receivingClient) + err = receivingHelper.AcceptVerification(ctx, txnID1) + require.NoError(t, err) + ts.dispatchToDevice(t, ctx, sendingClient) // Process the m.key.verification.ready event + + // Send a second verification request + txnID2, err := sendingHelper.StartVerification(ctx, aliceUserID) + require.NoError(t, err) + ts.dispatchToDevice(t, ctx, receivingClient) + + // Ensure that the sending device received a cancellation event for both of + // the ongoing transactions. + sendingInbox := ts.DeviceInbox[aliceUserID][sendingDeviceID] + require.Len(t, sendingInbox, 2) + cancelEvt1 := sendingInbox[0].Content.AsVerificationCancel() + cancelEvt2 := sendingInbox[1].Content.AsVerificationCancel() + cancelledTxnIDs := []id.VerificationTransactionID{cancelEvt1.TransactionID, cancelEvt2.TransactionID} + assert.Contains(t, cancelledTxnIDs, txnID1) + assert.Contains(t, cancelledTxnIDs, txnID2) + assert.Equal(t, event.VerificationCancelCodeUnexpectedMessage, cancelEvt1.Code) + assert.Equal(t, event.VerificationCancelCodeUnexpectedMessage, cancelEvt2.Code) + assert.Equal(t, "received multiple verification requests from the same device", cancelEvt1.Reason) + assert.Equal(t, "received multiple verification requests from the same device", cancelEvt2.Reason) + + assert.NotNil(t, receivingCallbacks.GetVerificationCancellation(txnID1)) + assert.NotNil(t, receivingCallbacks.GetVerificationCancellation(txnID2)) + ts.dispatchToDevice(t, ctx, sendingClient) // Process the m.key.verification.cancel events + assert.NotNil(t, sendingCallbacks.GetVerificationCancellation(txnID1)) + assert.NotNil(t, sendingCallbacks.GetVerificationCancellation(txnID2)) }