verificationhelper: cancel if multiple requests received from same device

Signed-off-by: Sumner Evans <sumner.evans@automattic.com>
This commit is contained in:
Sumner Evans 2024-05-27 15:24:42 -06:00
commit 3885a6378e
No known key found for this signature in database
2 changed files with 72 additions and 8 deletions

View file

@ -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)

View file

@ -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))
}