From eed1ffe1076d089ed8d26dcb786f67dcc39d47ef Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Mon, 10 Mar 2025 22:09:46 -0600 Subject: [PATCH] verificationhelper/sas: fix error when both sides send StartSAS Signed-off-by: Sumner Evans --- crypto/verificationhelper/sas.go | 37 ++++----- .../verificationhelper/verificationhelper.go | 8 +- .../verificationhelper_sas_test.go | 77 +++++++++++++++++++ 3 files changed, 101 insertions(+), 21 deletions(-) diff --git a/crypto/verificationhelper/sas.go b/crypto/verificationhelper/sas.go index 70b77759..2906e3a2 100644 --- a/crypto/verificationhelper/sas.go +++ b/crypto/verificationhelper/sas.go @@ -224,28 +224,29 @@ func (vh *VerificationHelper) onVerificationStartSAS(ctx context.Context, txn Ve } txn.MACMethod = macMethod txn.EphemeralKey = &ECDHPrivateKey{ephemeralKey} - txn.StartEventContent = startEvt - commitment, err := calculateCommitment(ephemeralKey.PublicKey(), startEvt) - if err != nil { - return fmt.Errorf("failed to calculate commitment: %w", err) - } + if !txn.StartedByUs { + commitment, err := calculateCommitment(ephemeralKey.PublicKey(), txn) + if err != nil { + return fmt.Errorf("failed to calculate commitment: %w", err) + } - err = vh.sendVerificationEvent(ctx, txn, event.InRoomVerificationAccept, &event.VerificationAcceptEventContent{ - Commitment: commitment, - Hash: hashAlgorithm, - KeyAgreementProtocol: keyAggreementProtocol, - MessageAuthenticationCode: macMethod, - ShortAuthenticationString: sasMethods, - }) - if err != nil { - return fmt.Errorf("failed to send accept event: %w", err) + err = vh.sendVerificationEvent(ctx, txn, event.InRoomVerificationAccept, &event.VerificationAcceptEventContent{ + Commitment: commitment, + Hash: hashAlgorithm, + KeyAgreementProtocol: keyAggreementProtocol, + MessageAuthenticationCode: macMethod, + ShortAuthenticationString: sasMethods, + }) + if err != nil { + return fmt.Errorf("failed to send accept event: %w", err) + } + txn.VerificationState = VerificationStateSASAccepted } - txn.VerificationState = VerificationStateSASAccepted return vh.store.SaveVerificationTransaction(ctx, txn) } -func calculateCommitment(ephemeralPubKey *ecdh.PublicKey, startEvt *event.VerificationStartEventContent) ([]byte, error) { +func calculateCommitment(ephemeralPubKey *ecdh.PublicKey, txn VerificationTransaction) ([]byte, error) { // The commitmentHashInput is the hash (encoded as unpadded base64) of the // concatenation of the device's ephemeral public key (encoded as // unpadded base64) and the canonical JSON representation of the @@ -255,7 +256,7 @@ func calculateCommitment(ephemeralPubKey *ecdh.PublicKey, startEvt *event.Verifi // hashing it, but we are just stuck on that. commitmentHashInput := sha256.New() commitmentHashInput.Write([]byte(base64.RawStdEncoding.EncodeToString(ephemeralPubKey.Bytes()))) - encodedStartEvt, err := json.Marshal(startEvt) + encodedStartEvt, err := json.Marshal(txn.StartEventContent) if err != nil { return nil, err } @@ -339,7 +340,7 @@ func (vh *VerificationHelper) onVerificationKey(ctx context.Context, txn Verific if txn.EphemeralPublicKeyShared { // Verify that the commitment hash is correct - commitment, err := calculateCommitment(publicKey, txn.StartEventContent) + commitment, err := calculateCommitment(publicKey, txn) if err != nil { log.Err(err).Msg("Failed to calculate commitment") return diff --git a/crypto/verificationhelper/verificationhelper.go b/crypto/verificationhelper/verificationhelper.go index c47eea71..550df942 100644 --- a/crypto/verificationhelper/verificationhelper.go +++ b/crypto/verificationhelper/verificationhelper.go @@ -818,6 +818,8 @@ func (vh *VerificationHelper) onVerificationStart(ctx context.Context, txn Verif } else if txn.VerificationState != VerificationStateReady { vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeUnexpectedMessage, "got start event for transaction that is not in ready state") return + } else { + txn.StartEventContent = startEvt } switch startEvt.Method { @@ -829,7 +831,7 @@ func (vh *VerificationHelper) onVerificationStart(ctx context.Context, txn Verif } case event.VerificationMethodReciprocate: log.Info().Msg("Received reciprocate start event") - if !bytes.Equal(txn.QRCodeSharedSecret, startEvt.Secret) { + if !bytes.Equal(txn.QRCodeSharedSecret, txn.StartEventContent.Secret) { vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeKeyMismatch, "reciprocated shared secret does not match") return } @@ -842,8 +844,8 @@ func (vh *VerificationHelper) onVerificationStart(ctx context.Context, txn Verif // Note that we should never get m.qr_code.show.v1 or m.qr_code.scan.v1 // here, since the start command for scanning and showing QR codes // should be of type m.reciprocate.v1. - log.Error().Str("method", string(startEvt.Method)).Msg("Unsupported verification method in start event") - vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeUnknownMethod, fmt.Sprintf("unknown method %s", startEvt.Method)) + log.Error().Str("method", string(txn.StartEventContent.Method)).Msg("Unsupported verification method in start event") + vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeUnknownMethod, fmt.Sprintf("unknown method %s", txn.StartEventContent.Method)) } } diff --git a/crypto/verificationhelper/verificationhelper_sas_test.go b/crypto/verificationhelper/verificationhelper_sas_test.go index 22b1563c..5747ac34 100644 --- a/crypto/verificationhelper/verificationhelper_sas_test.go +++ b/crypto/verificationhelper/verificationhelper_sas_test.go @@ -283,3 +283,80 @@ func TestVerification_SAS(t *testing.T) { }) } } + +func TestVerification_SAS_BothCallStart(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) + var err error + + var sendingRecoveryKey string + var sendingCrossSigningKeysCache *crypto.CrossSigningKeysCache + + sendingRecoveryKey, sendingCrossSigningKeysCache, err = sendingMachine.GenerateAndUploadCrossSigningKeys(ctx, nil, "") + require.NoError(t, err) + assert.NotEmpty(t, sendingRecoveryKey) + assert.NotNil(t, sendingCrossSigningKeysCache) + + // Send the verification request from the sender device and accept + // it on the receiving device and receive the verification ready + // event on the sending device. + txnID, err := sendingHelper.StartVerification(ctx, aliceUserID) + require.NoError(t, err) + ts.dispatchToDevice(t, ctx, receivingClient) + err = receivingHelper.AcceptVerification(ctx, txnID) + require.NoError(t, err) + ts.dispatchToDevice(t, ctx, sendingClient) + + err = sendingHelper.StartSAS(ctx, txnID) + require.NoError(t, err) + + err = receivingHelper.StartSAS(ctx, txnID) + require.NoError(t, err) + + // Ensure that both devices have received the verification start event. + receivingInbox := ts.DeviceInbox[aliceUserID][receivingDeviceID] + assert.Len(t, receivingInbox, 1) + assert.Equal(t, txnID, receivingInbox[0].Content.AsVerificationStart().TransactionID) + sendingInbox := ts.DeviceInbox[aliceUserID][sendingDeviceID] + assert.Len(t, sendingInbox, 1) + assert.Equal(t, txnID, sendingInbox[0].Content.AsVerificationStart().TransactionID) + + // Process the start event from the receiving client to the sending client. + ts.dispatchToDevice(t, ctx, sendingClient) + receivingInbox = ts.DeviceInbox[aliceUserID][receivingDeviceID] + assert.Len(t, receivingInbox, 2) + assert.Equal(t, txnID, receivingInbox[0].Content.AsVerificationStart().TransactionID) + assert.Equal(t, txnID, receivingInbox[1].Content.AsVerificationAccept().TransactionID) + + // Process the rest of the events until we need to confirm the SAS. + for len(ts.DeviceInbox[aliceUserID][sendingDeviceID]) > 0 || len(ts.DeviceInbox[aliceUserID][receivingDeviceID]) > 0 { + ts.dispatchToDevice(t, ctx, receivingClient) + ts.dispatchToDevice(t, ctx, sendingClient) + } + + // Confirm the SAS only the receiving device. + receivingHelper.ConfirmSAS(ctx, txnID) + ts.dispatchToDevice(t, ctx, sendingClient) + + // Verification is not done until both devices confirm the SAS. + assert.False(t, sendingCallbacks.IsVerificationDone(txnID)) + assert.False(t, receivingCallbacks.IsVerificationDone(txnID)) + + // Now, confirm it on the sending device. + sendingHelper.ConfirmSAS(ctx, txnID) + + // Dispatching the events to the receiving device should get us to the done + // state on the receiving device. + ts.dispatchToDevice(t, ctx, receivingClient) + assert.False(t, sendingCallbacks.IsVerificationDone(txnID)) + assert.True(t, receivingCallbacks.IsVerificationDone(txnID)) + + // Dispatching the events to the sending client should get us to the done + // state on the sending device. + ts.dispatchToDevice(t, ctx, sendingClient) + assert.True(t, sendingCallbacks.IsVerificationDone(txnID)) + assert.True(t, receivingCallbacks.IsVerificationDone(txnID)) +}