From f47e07ddcac214737fa4448de03d6137d98c2165 Mon Sep 17 00:00:00 2001 From: Christian Stewart Date: Wed, 14 Oct 2026 10:48:24 -0700 Subject: [PATCH] Add a test for SAS verification between two users Add RequestDeviceVerification to the client API so a test can verify another user's device, and TestVerificationSASOtherUser, which verifies Bob's device from Alice's, across servers when the client matrix asks for it. The JS client implements it with requestDeviceVerification; the Rust FFI cannot yet, so it skips. RequestOwnUserVerification relied on the request coming back to the sender, which only happens when the sender does not know its other devices yet and sends to '*'. When it already knew them, the request was never tracked and TestVerificationSAS timed out after the receiver became ready. Track the sent request directly and ignore received requests we initiated. Signed-off-by: Christian Stewart --- internal/api/client.go | 5 ++ internal/api/js/js.go | 34 +++++-- internal/api/rust/rust.go | 5 ++ internal/deploy/rpc/client.go | 4 + tests/verification_test.go | 162 +++++++++++++++++++++------------- 5 files changed, 139 insertions(+), 71 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 5f26505f..8abd8704 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -87,6 +87,11 @@ type Client interface { // } // The channel is closed when the verification process reaches a terminal state. RequestOwnUserVerification(t ct.TestLike) chan VerificationStage + // RequestDeviceVerification tries to verify the given device, which may belong to + // another user on another server. The request is sent as a to-device message. + // + // Returns a stream of verification stages. See RequestOwnUserVerification. + RequestDeviceVerification(t ct.TestLike, userID, deviceID string) chan VerificationStage // Log something to stdout and the underlying client log file Logf(t ct.TestLike, format string, args ...interface{}) // The user for this client diff --git a/internal/api/js/js.go b/internal/api/js/js.go index 6a25f8f7..b244c3ee 100644 --- a/internal/api/js/js.go +++ b/internal/api/js/js.go @@ -282,10 +282,12 @@ func (c *JSClient) ensureListeningForVerificationRequests(t ct.TestLike) chan ap // we need to support multiple transition stages firing at once c.verificationChannel = make(chan api.VerificationStage, 4) chrome.MustRunAsyncFn[chrome.Void](t, c.browser.Ctx, ` - window.__client.on(CryptoEvent.VerificationRequestReceived, function(request) { - console.log("CryptoEvent.VerificationRequestReceived fired: request.initiatedByMe " + request.initiatedByMe); + // __trackVerificationRequest emits the stages of a request, whether it was + // received or sent by this client. + window.__trackVerificationRequest = function(request) { + console.log("tracking verification request: request.initiatedByMe " + request.initiatedByMe); request.on("change", () => { - console.log("RequestOwnUserVerification got phase " + request.phase); + console.log("verification request got phase " + request.phase); switch(request.phase) { case VerificationPhase.Unsent: console.log("Unsent"); @@ -350,6 +352,13 @@ func (c *JSClient) ensureListeningForVerificationRequests(t ct.TestLike) chan ap "request.otherDeviceId", "{}", )+` + }; + // A request sent to all of our own devices comes back to us. The sender + // already tracks it, so only track requests from other devices. + window.__client.on(CryptoEvent.VerificationRequestReceived, function(request) { + if (!request.initiatedByMe) { + window.__trackVerificationRequest(request); + } });`) } return c.verificationChannel @@ -463,19 +472,26 @@ func (c *JSClient) ListenForVerificationRequests(t ct.TestLike) chan api.Verific } func (c *JSClient) RequestOwnUserVerification(t ct.TestLike) chan api.VerificationStage { - // When we request key verification, we will /sendToDevice with * devices, which - // rather bizarrely will send the to-device event back to ourselves. This will then - // be picked up as a VerificationRequestReceived. The code that listens for - // VerificationRequestReceived will figure out it's a request from itself and adjust - // the state accordingly. Because of this, we only need to call requestOwnUserVerification - // and do nothing else. + // The request goes to * devices when this client does not know its other devices + // yet, and to the known devices otherwise, so it only sometimes comes back to us. + // Track the request we sent directly; the listener ignores the echo. ch := c.ListenForVerificationRequests(t) chrome.MustRunAsyncFn[chrome.Void](t, c.browser.Ctx, ` const request = await window.__client.getCrypto().requestOwnUserVerification(); + window.__trackVerificationRequest(request); `) return ch } +func (c *JSClient) RequestDeviceVerification(t ct.TestLike, userID, deviceID string) chan api.VerificationStage { + ch := c.ListenForVerificationRequests(t) + chrome.MustRunAsyncFn[chrome.Void](t, c.browser.Ctx, fmt.Sprintf(` + const request = await window.__client.getCrypto().requestDeviceVerification(%q, %q); + window.__trackVerificationRequest(request); + `, userID, deviceID)) + return ch +} + func (c *JSClient) ForceClose(t ct.TestLike) { t.Helper() t.Logf("force closing a JS client is the same as a normal close (closing browser)") diff --git a/internal/api/rust/rust.go b/internal/api/rust/rust.go index eaee2553..da35f578 100644 --- a/internal/api/rust/rust.go +++ b/internal/api/rust/rust.go @@ -326,6 +326,11 @@ func (c *RustClient) RequestOwnUserVerification(t ct.TestLike) chan api.Verifica return ch } +func (c *RustClient) RequestDeviceVerification(t ct.TestLike, userID, deviceID string) chan api.VerificationStage { + t.Skipf("RequestDeviceVerification: the rust FFI cannot request verification of another user's device") + return nil +} + func (c *RustClient) DeletePersistentStorage(t ct.TestLike) { c.FFISpan.Enter() defer c.FFISpan.Exit() diff --git a/internal/deploy/rpc/client.go b/internal/deploy/rpc/client.go index f70c4260..88af40d1 100644 --- a/internal/deploy/rpc/client.go +++ b/internal/deploy/rpc/client.go @@ -217,6 +217,10 @@ func (c *RPCClient) RequestOwnUserVerification(t ct.TestLike) chan api.Verificat panic("unimplemented") } +func (c *RPCClient) RequestDeviceVerification(t ct.TestLike, userID, deviceID string) chan api.VerificationStage { + panic("unimplemented") +} + func (c *RPCClient) ListenForVerificationRequests(t ct.TestLike) chan api.VerificationStage { panic("unimplemented") } diff --git a/tests/verification_test.go b/tests/verification_test.go index e1d6f749..ba5fbfa5 100644 --- a/tests/verification_test.go +++ b/tests/verification_test.go @@ -9,6 +9,7 @@ import ( "github.com/matrix-org/complement-crypto/internal/api" "github.com/matrix-org/complement-crypto/internal/cc" "github.com/matrix-org/complement/ct" + "github.com/matrix-org/gomatrixserverlib/spec" ) var boolTrue = true @@ -74,74 +75,111 @@ func TestVerificationSAS(t *testing.T) { DeviceID: "OTHER_DEVICE", }, }, func(verifiee api.TestClient) { - status := &verificationStatus{ - mu: &sync.Mutex{}, - } verifier.Logf(t, "Verifier (SENDER) %s %s", verifierClientType.Lang, verifier.Opts().DeviceID) verifiee.Logf(t, "Verifiee (RECEIVER) %s %s", verifieeClientType.Lang, verifiee.Opts().DeviceID) verifieeStage := verifiee.ListenForVerificationRequests(t) verifierStage := verifier.RequestOwnUserVerification(t) - for { - select { - case receiverStage := <-verifieeStage: - switch stage := receiverStage.(type) { - case api.VerificationStageRequestedReceiver: - t.Logf("[RECEIVER] VerificationStageRequestedReceiver: %+v", stage.Request()) - stage.Ready() - case api.VerificationStageRequested: - t.Logf("[RECEIVER] VerificationStageRequested: %+v", stage.Request()) - case api.VerificationStageReady: - t.Logf("[RECEIVER] VerificationStageReady") - case api.VerificationStageTransitioned: - t.Logf("[RECEIVER] VerificationStageTransitioned") - status.mu.Lock() - status.ReceiverStage = stage - status.attemptVerification(t) - status.mu.Unlock() - case api.VerificationStageStart: - t.Logf("[RECEIVER] VerificationStageStart") - stage.Transition() - case api.VerificationStageDone: - t.Logf("[RECEIVER] VerificationStageDone") - if status.done(nil, &boolTrue) { - return - } - case api.VerificationStageCancelled: // should not be cancelled - ct.Errorf(t, "[RECEIVER] VerificationStageCancelled") - } - case senderStage := <-verifierStage: - switch stage := senderStage.(type) { - case api.VerificationStageRequestedReceiver: // the verifier should not get a requestee state - ct.Errorf(t, "[SENDER] VerificationStageRequestedReceiver: %+v", stage.Request()) - case api.VerificationStageRequested: - t.Logf("[SENDER] VerificationStageRequested: %+v", stage.Request()) - case api.VerificationStageReady: - t.Logf("[SENDER] VerificationStageReady: starting m.sas.v1") - stage.Start("m.sas.v1") - case api.VerificationStageTransitioned: - t.Logf("[SENDER] VerificationStageTransitioned") - status.mu.Lock() - status.SenderStage = stage - status.attemptVerification(t) - status.mu.Unlock() - case api.VerificationStageStart: - t.Logf("[SENDER] VerificationStageStart") - stage.Transition() - case api.VerificationStageDone: - t.Logf("[SENDER] VerificationStageDone") - if status.done(&boolTrue, nil) { - return - } - case api.VerificationStageCancelled: // should not be cancelled - ct.Errorf(t, "[SENDER] VerificationStageCancelled") - } - case <-time.After(5 * time.Second): - ct.Fatalf(t, "timed out after 5s") - return - } - } + mustVerifySAS(t, verifierStage, verifieeStage) }) }) + }) +} +// happy case test of Alice verifying one of Bob's devices, which may be on another server. +func TestVerificationSASOtherUser(t *testing.T) { + Instance().ClientTypeMatrix(t, func(t *testing.T, verifierClientType, verifieeClientType api.ClientType) { + if verifierClientType.Lang == api.ClientTypeRust || verifieeClientType.Lang == api.ClientTypeRust { + t.Skipf("rust cannot verify another user's device yet") + } + tc := Instance().CreateTestContext(t, verifierClientType, verifieeClientType) + roomID := tc.CreateNewEncryptedRoom( + t, + tc.Alice, + cc.EncRoomOptions.PresetTrustedPrivateChat(), + cc.EncRoomOptions.Invite([]string{tc.Bob.UserID}), + ) + tc.Bob.MustJoinRoom(t, roomID, []spec.ServerName{verifierClientType.HS}) + tc.WithAliceAndBobSyncing(t, func(verifier, verifiee api.TestClient) { + // A client verifies only the devices of users it tracks, so make + // Alice track Bob's devices by messaging him in the shared room. + wantMsgBody := "Hello Bob" + waiter := verifiee.WaitUntilEventInRoom(t, roomID, api.CheckEventHasBody(wantMsgBody)) + verifier.MustSendMessage(t, roomID, wantMsgBody) + waiter.Waitf(t, 5*time.Second, "bob did not see alice's message") + + verifier.Logf(t, "Verifier (SENDER) %s %s", verifierClientType.Lang, verifier.Opts().DeviceID) + verifiee.Logf(t, "Verifiee (RECEIVER) %s %s", verifieeClientType.Lang, verifiee.Opts().DeviceID) + verifieeStage := verifiee.ListenForVerificationRequests(t) + verifierStage := verifier.RequestDeviceVerification(t, verifiee.UserID(), verifiee.Opts().DeviceID) + mustVerifySAS(t, verifierStage, verifieeStage) + }) }) } + +// mustVerifySAS drives an SAS verification between a sender and a receiver +// until both are done, approving it if their emoji match. +func mustVerifySAS(t *testing.T, verifierStage, verifieeStage chan api.VerificationStage) { + t.Helper() + status := &verificationStatus{ + mu: &sync.Mutex{}, + } + for { + select { + case receiverStage := <-verifieeStage: + switch stage := receiverStage.(type) { + case api.VerificationStageRequestedReceiver: + t.Logf("[RECEIVER] VerificationStageRequestedReceiver: %+v", stage.Request()) + stage.Ready() + case api.VerificationStageRequested: + t.Logf("[RECEIVER] VerificationStageRequested: %+v", stage.Request()) + case api.VerificationStageReady: + t.Logf("[RECEIVER] VerificationStageReady") + case api.VerificationStageTransitioned: + t.Logf("[RECEIVER] VerificationStageTransitioned") + status.mu.Lock() + status.ReceiverStage = stage + status.attemptVerification(t) + status.mu.Unlock() + case api.VerificationStageStart: + t.Logf("[RECEIVER] VerificationStageStart") + stage.Transition() + case api.VerificationStageDone: + t.Logf("[RECEIVER] VerificationStageDone") + if status.done(nil, &boolTrue) { + return + } + case api.VerificationStageCancelled: // should not be cancelled + ct.Errorf(t, "[RECEIVER] VerificationStageCancelled") + } + case senderStage := <-verifierStage: + switch stage := senderStage.(type) { + case api.VerificationStageRequestedReceiver: // the verifier should not get a requestee state + ct.Errorf(t, "[SENDER] VerificationStageRequestedReceiver: %+v", stage.Request()) + case api.VerificationStageRequested: + t.Logf("[SENDER] VerificationStageRequested: %+v", stage.Request()) + case api.VerificationStageReady: + t.Logf("[SENDER] VerificationStageReady: starting m.sas.v1") + stage.Start("m.sas.v1") + case api.VerificationStageTransitioned: + t.Logf("[SENDER] VerificationStageTransitioned") + status.mu.Lock() + status.SenderStage = stage + status.attemptVerification(t) + status.mu.Unlock() + case api.VerificationStageStart: + t.Logf("[SENDER] VerificationStageStart") + stage.Transition() + case api.VerificationStageDone: + t.Logf("[SENDER] VerificationStageDone") + if status.done(&boolTrue, nil) { + return + } + case api.VerificationStageCancelled: // should not be cancelled + ct.Errorf(t, "[SENDER] VerificationStageCancelled") + } + case <-time.After(5 * time.Second): + ct.Fatalf(t, "timed out after 5s") + return + } + } +}