fix(crypto): fix in-room verification encryption, echo filtering, and commitment calculation - #555
fix(crypto): fix in-room verification encryption, echo filtering, and commitment calculation#555underhax wants to merge 1 commit into
Conversation
|
Independent confirmation of the transaction ID routing problem, plus one dead branch in the same handler that this PR doesn't touch yet. We ran into this with a small bot (mautrix-go v0.30.0, goolm build tag, Synapse behind MAS) in an encrypted two-person room, verifying from Element Desktop via "Verify User". The flow always stopped after our ready. We measured both variants: when the bot started SAS, Element answered with m.key.verification.accept and the bot never reacted. When Element sent m.key.verification.start itself, same silence. A separate handler registered on the same event types confirmed the events do reach the process. Reading the code we arrived at the same conclusion as point 2 of this PR: wrapHandler uses evt.ID as the transaction ID for any event that has one, so incoming in-room accept/start/key/mac are looked up under their own event ID instead of the request's event ID and get dropped as unknown transactions. The log line that gave it away was failed to get verification transaction error="unknown transaction ID" with transaction_id equal to event_id. One thing this PR does not seem to change: the cancellation path for unknown transactions is unreachable. In Init's wrapHandler: txn, err := vh.store.GetVerificationTransaction(ctx, transactionID)
if err != nil && errors.Is(err, ErrUnknownVerificationTransaction) {
log.Err(err).Msg("failed to get verification transaction")
vh.activeTransactionsLock.Unlock()
return
} else if errors.Is(err, ErrUnknownVerificationTransaction) {
// builds a fake transaction and sends m.unknown_transaction — never reachedThe first condition already matches every unknown-transaction error, so the helper never sends m.unknown_transaction to the other side. The peer only sees its own timeout; Element then shows the misleading "request timed out / was denied / verification mismatch" message. Might be worth folding into this PR, since the branch only becomes reachable once the routing fix lands. We can test this branch against Element Desktop on our homeserver if that helps the review. |
Summary
Fixes in-room SAS verification failing with unexpected message cancellations or
m.key_mismatchcommitment errors when communicating with Matrix clients like Element / matrix-rust-sdk.Problem Description
StartInRoomVerificationandsendVerificationEventsent in-room verification events in plaintext instead of encrypting them asm.room.encryptedin encrypted rooms.m.relates_to.event_idand did not ignore echoed events from our own device/user, leading to state machine collisions and premature cancellations (m.unexpected_message).m.key_mismatch): SAS commitment verification failed becauseevent.RelatesTo.IsFallingBackserialized"is_falling_back": falseinto the canonical JSON object ofm.relates_to.Solution
Crypto.Encryptwhen encryption is enabled.wrapHandlerto extracttransactionIDfromm.relates_to.event_idand ignore echoed events from our own device/user.onVerificationKey.omitzerotag toRelatesTo.IsFallingBackto prevent serializing zero-valued booleans in canonical JSON.canonicalMarshalandmatchCommitmentfor strict canonical JSON hashing and resilient SAS commitment verification.MockServer.DispatchToDeviceto dispatch to-device encryption events toOlmMachine.HandleToDeviceEvent.TestInRoomVerification_EventEncryptionAndTypetoverificationhelper_test.go.Checklist