[RFC] Add a test for SAS verification between two users

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

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

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

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

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
+		}
+	}
+}