From dff6bae32a3ec54b9e05dc7af99038dbf6347209 Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Mon, 18 Mar 2024 14:35:40 +0000 Subject: [PATCH 1/3] ForEachClientType now looks at the test matrix for determining which clients to test --- ENVIRONMENT.md | 1 + internal/config/config.go | 15 +++++++++++++++ tests/main_test.go | 3 +++ 3 files changed, 19 insertions(+) diff --git a/ENVIRONMENT.md b/ENVIRONMENT.md index a4c5aee..d4a8769 100644 --- a/ENVIRONMENT.md +++ b/ENVIRONMENT.md @@ -23,6 +23,7 @@ The client test matrix to run. Every test is run for each given permutation. The - `rj,rr`: Run the test twice. Run 1: Alice=rust, Bob=JS. Run 2: Alice=rust, Bob=rust. All on HS1. - `jJ`: Run the test once. Run 1: Alice=JS on HS1, Bob=JS on HS2. Tests federation. ``` + If the matrix only consists of one leter (e.g all j's) then rust-specific tests will not run and vice versa. - Type: `[][]ClientType` diff --git a/internal/config/config.go b/internal/config/config.go index a26c1f5..0034dd6 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -27,8 +27,13 @@ type ComplementCrypto struct { // - `rj,rr`: Run the test twice. Run 1: Alice=rust, Bob=JS. Run 2: Alice=rust, Bob=rust. All on HS1. // - `jJ`: Run the test once. Run 1: Alice=JS on HS1, Bob=JS on HS2. Tests federation. // ``` + // If the matrix only consists of one leter (e.g all j's) then rust-specific tests will not run and vice versa. TestClientMatrix [][2]api.ClientType + // Which languages should be tested in ForEachClientType tests. + // Derived from TestClientMatrix + clientLangs map[api.ClientTypeLang]bool + // Name: COMPLEMENT_CRYPTO_TCPDUMP // Default: 0 // Description: If 1, automatically attempts to run `tcpdump` when the containers are running. Stops dumping when @@ -37,12 +42,17 @@ type ComplementCrypto struct { TCPDump bool } +func (c *ComplementCrypto) ShouldTest(lang api.ClientTypeLang) bool { + return c.clientLangs[lang] +} + func NewComplementCryptoConfigFromEnvVars() *ComplementCrypto { matrix := os.Getenv("COMPLEMENT_CRYPTO_TEST_CLIENT_MATRIX") if matrix == "" { matrix = "jj,jr,rj,rr" } segs := strings.Split(matrix, ",") + clientLangs := make(map[api.ClientTypeLang]bool) var testClientMatrix [][2]api.ClientType for _, val := range segs { // e.g val == 'rj' if len(val) != 2 { @@ -56,21 +66,25 @@ func NewComplementCryptoConfigFromEnvVars() *ComplementCrypto { Lang: api.ClientTypeRust, HS: "hs1", } + clientLangs[api.ClientTypeRust] = true case 'j': testCase[i] = api.ClientType{ Lang: api.ClientTypeJS, HS: "hs1", } + clientLangs[api.ClientTypeJS] = true case 'J': testCase[i] = api.ClientType{ Lang: api.ClientTypeJS, HS: "hs2", } + clientLangs[api.ClientTypeJS] = true case 'R': testCase[i] = api.ClientType{ Lang: api.ClientTypeRust, HS: "hs2", } + clientLangs[api.ClientTypeRust] = true default: panic("COMPLEMENT_CRYPTO_TEST_CLIENT_MATRIX bad value: " + val) } @@ -83,5 +97,6 @@ func NewComplementCryptoConfigFromEnvVars() *ComplementCrypto { return &ComplementCrypto{ TCPDump: os.Getenv("COMPLEMENT_CRYPTO_TCPDUMP") == "1", TestClientMatrix: testClientMatrix, + clientLangs: clientLangs, } } diff --git a/tests/main_test.go b/tests/main_test.go index d9b5278..1c52af3 100644 --- a/tests/main_test.go +++ b/tests/main_test.go @@ -84,6 +84,9 @@ func ClientTypeMatrix(t *testing.T, subTest func(t *testing.T, clientTypeA, clie func ForEachClientType(t *testing.T, subTest func(t *testing.T, clientType api.ClientType)) { for _, tc := range []api.ClientType{{Lang: api.ClientTypeRust, HS: "hs1"}, {Lang: api.ClientTypeJS, HS: "hs1"}} { tc := tc + if !complementCryptoConfig.ShouldTest(tc.Lang) { + continue + } t.Run(string(tc.Lang), func(t *testing.T) { subTest(t, tc) }) From 56141ecfaac0fc5dacf285c716a9e40c824e2d2e Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Mon, 18 Mar 2024 14:44:46 +0000 Subject: [PATCH 2/3] Ensure ALL tests use either ClientTypeMatrix or ForEachClientType This allows us to not build JS bits if we don't run any JS tests, mostly. --- tests/federation_connectivity_test.go | 238 +++++++++++++------------- 1 file changed, 120 insertions(+), 118 deletions(-) diff --git a/tests/federation_connectivity_test.go b/tests/federation_connectivity_test.go index f25ffe3..3917d24 100644 --- a/tests/federation_connectivity_test.go +++ b/tests/federation_connectivity_test.go @@ -16,73 +16,75 @@ import ( // B will be unable to decrypt C's message. TODO: see https://github.com/matrix-org/matrix-rust-sdk/issues/2864 // Ensure sending another message from C is decryptable. func TestNewUserCannotGetKeysForOfflineServer(t *testing.T) { - // normally we would use ForEachClient or ClientTypeMatrix but due to federation we need - // to pin it to certain langs/servers. - tc := CreateTestContext(t, api.ClientType{ - Lang: api.ClientTypeRust, - HS: "hs1", - }, api.ClientType{ - Lang: api.ClientTypeJS, - HS: "hs2", - }, api.ClientType{ - Lang: api.ClientTypeRust, - HS: "hs1", - }) - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.Invite([]string{tc.Bob.UserID})) - t.Logf("%s joining room %s", tc.Bob.UserID, roomID) - tc.Bob.MustJoinRoom(t, roomID, []string{"hs1"}) - - tc.WithAliceAndBobSyncing(t, func(alice, bob api.Client) { - // let clients sync device keys - time.Sleep(time.Second) - - // ensure encrypted messaging works - wantMsgBody := "Hello world" - waiter := bob.WaitUntilEventInRoom(t, roomID, api.CheckEventHasBody(wantMsgBody)) - evID := alice.SendMessage(t, roomID, wantMsgBody) - t.Logf("bob (%s) waiting for event %s", bob.Type(), evID) - waiter.Wait(t, 5*time.Second) - - // now bob's HS becomes unreachable - tc.Deployment.PauseServer(t, "hs2") - - // C now joins the room - tc.Alice.MustInviteRoom(t, roomID, tc.Charlie.UserID) - tc.WithClientSyncing(t, tc.CharlieClientType, tc.Charlie, func(charlie api.Client) { - tc.Charlie.MustJoinRoom(t, roomID, []string{"hs1"}) - - // let charlie sync device keys... and fail to get bob's keys! - time.Sleep(time.Second) - - // send a message: bob won't be able to decrypt this, but alice will. - wantUndecryptableMsgBody := "Bob can't see this because his server is down" - waiter = alice.WaitUntilEventInRoom(t, roomID, api.CheckEventHasBody(wantUndecryptableMsgBody)) - undecryptableEventID := charlie.SendMessage(t, roomID, wantUndecryptableMsgBody) - t.Logf("alice (%s) waiting for event %s", alice.Type(), undecryptableEventID) - waiter.Wait(t, 5*time.Second) + ForEachClientType(t, func(t *testing.T, clientType api.ClientType) { + // normally we would use ForEachClient or ClientTypeMatrix but due to federation we need + // to pin it to certain langs/servers. + tc := CreateTestContext(t, api.ClientType{ + Lang: clientType.Lang, + HS: "hs1", + }, api.ClientType{ + Lang: clientType.Lang, + HS: "hs2", + }, api.ClientType{ + Lang: clientType.Lang, + HS: "hs1", + }) + roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.Invite([]string{tc.Bob.UserID})) + t.Logf("%s joining room %s", tc.Bob.UserID, roomID) + tc.Bob.MustJoinRoom(t, roomID, []string{"hs1"}) - // now bob's server comes back online - tc.Deployment.UnpauseServer(t, "hs2") + tc.WithAliceAndBobSyncing(t, func(alice, bob api.Client) { + // let clients sync device keys + time.Sleep(time.Second) - // now we need to wait a bit for charlie's client to decide to hit /keys/claim again. - // If the client hits too often, there will be constantly send lag so long as bob's HS is offline. - // If the client hits too infrequently, there will be multiple undecryptable messages. - // See https://github.com/matrix-org/matrix-rust-sdk/issues/281 for why we want to backoff. - // See https://github.com/matrix-org/matrix-rust-sdk/issues/2804 for discussions on what the backoff should be. - t.Logf("sleeping until client timeout is ready...") - time.Sleep(20 * time.Second) - - // send another message, bob should be able to decrypt it. - wantMsgBody = "Bob can see this because his server is now back online" - waiter = bob.WaitUntilEventInRoom(t, roomID, api.CheckEventHasBody(wantMsgBody)) - evID = charlie.SendMessage(t, roomID, wantMsgBody) + // ensure encrypted messaging works + wantMsgBody := "Hello world" + waiter := bob.WaitUntilEventInRoom(t, roomID, api.CheckEventHasBody(wantMsgBody)) + evID := alice.SendMessage(t, roomID, wantMsgBody) t.Logf("bob (%s) waiting for event %s", bob.Type(), evID) waiter.Wait(t, 5*time.Second) - // make sure bob cannot decrypt the msg from when his server was offline - // TODO: this isn't ideal, see https://github.com/matrix-org/matrix-rust-sdk/issues/2864 - ev := bob.MustGetEvent(t, roomID, undecryptableEventID) - must.Equal(t, ev.FailedToDecrypt, true, "bob was able to decrypt the undecryptable event") + // now bob's HS becomes unreachable + tc.Deployment.PauseServer(t, "hs2") + + // C now joins the room + tc.Alice.MustInviteRoom(t, roomID, tc.Charlie.UserID) + tc.WithClientSyncing(t, tc.CharlieClientType, tc.Charlie, func(charlie api.Client) { + tc.Charlie.MustJoinRoom(t, roomID, []string{"hs1"}) + + // let charlie sync device keys... and fail to get bob's keys! + time.Sleep(time.Second) + + // send a message: bob won't be able to decrypt this, but alice will. + wantUndecryptableMsgBody := "Bob can't see this because his server is down" + waiter = alice.WaitUntilEventInRoom(t, roomID, api.CheckEventHasBody(wantUndecryptableMsgBody)) + undecryptableEventID := charlie.SendMessage(t, roomID, wantUndecryptableMsgBody) + t.Logf("alice (%s) waiting for event %s", alice.Type(), undecryptableEventID) + waiter.Wait(t, 5*time.Second) + + // now bob's server comes back online + tc.Deployment.UnpauseServer(t, "hs2") + + // now we need to wait a bit for charlie's client to decide to hit /keys/claim again. + // If the client hits too often, there will be constantly send lag so long as bob's HS is offline. + // If the client hits too infrequently, there will be multiple undecryptable messages. + // See https://github.com/matrix-org/matrix-rust-sdk/issues/281 for why we want to backoff. + // See https://github.com/matrix-org/matrix-rust-sdk/issues/2804 for discussions on what the backoff should be. + t.Logf("sleeping until client timeout is ready...") + time.Sleep(20 * time.Second) + + // send another message, bob should be able to decrypt it. + wantMsgBody = "Bob can see this because his server is now back online" + waiter = bob.WaitUntilEventInRoom(t, roomID, api.CheckEventHasBody(wantMsgBody)) + evID = charlie.SendMessage(t, roomID, wantMsgBody) + t.Logf("bob (%s) waiting for event %s", bob.Type(), evID) + waiter.Wait(t, 5*time.Second) + + // make sure bob cannot decrypt the msg from when his server was offline + // TODO: this isn't ideal, see https://github.com/matrix-org/matrix-rust-sdk/issues/2864 + ev := bob.MustGetEvent(t, roomID, undecryptableEventID) + must.Equal(t, ev.FailedToDecrypt, true, "bob was able to decrypt the undecryptable event") + }) }) }) } @@ -96,61 +98,61 @@ func TestNewUserCannotGetKeysForOfflineServer(t *testing.T) { // B will be able to decrypt C's message. // This is ultimately checking that Olm sessions are per-device and not per-room. func TestExistingSessionCannotGetKeysForOfflineServer(t *testing.T) { - // normally we would use ForEachClient or ClientTypeMatrix but due to federation we need - // to pin it to certain langs/servers. - tc := CreateTestContext(t, api.ClientType{ - Lang: api.ClientTypeRust, - HS: "hs1", - }, api.ClientType{ - Lang: api.ClientTypeJS, - HS: "hs2", - }, api.ClientType{ - Lang: api.ClientTypeRust, - HS: "hs1", - }) - roomIDbc := tc.CreateNewEncryptedRoom(t, tc.Charlie, EncRoomOptions.Invite([]string{tc.Bob.UserID})) - roomIDab := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.Invite([]string{tc.Bob.UserID})) - t.Logf("%s joining rooms %s and %s", tc.Bob.UserID, roomIDab, roomIDbc) - tc.Bob.MustJoinRoom(t, roomIDab, []string{"hs1"}) - tc.Bob.MustJoinRoom(t, roomIDbc, []string{"hs1"}) - - tc.WithAliceBobAndCharlieSyncing(t, func(alice, bob, charlie api.Client) { - // let clients sync device keys - time.Sleep(time.Second) - - // ensure encrypted messaging works in rooms ab,bc - wantMsgBody := "Hello world" - waiter := bob.WaitUntilEventInRoom(t, roomIDab, api.CheckEventHasBody(wantMsgBody)) - evID := alice.SendMessage(t, roomIDab, wantMsgBody) - t.Logf("bob (%s) waiting for event %s", bob.Type(), evID) - waiter.Wait(t, 5*time.Second) - waiter = bob.WaitUntilEventInRoom(t, roomIDbc, api.CheckEventHasBody(wantMsgBody)) - evID = charlie.SendMessage(t, roomIDbc, wantMsgBody) - t.Logf("bob (%s) waiting for event %s", bob.Type(), evID) - waiter.Wait(t, 5*time.Second) - - // now bob's HS becomes unreachable - tc.Deployment.PauseServer(t, "hs2") - - // C now joins the room ab - tc.Alice.MustInviteRoom(t, roomIDab, tc.Charlie.UserID) - tc.Charlie.MustJoinRoom(t, roomIDab, []string{"hs1"}) - - // let charlie sync device keys... - time.Sleep(time.Second) - - // send a message as C: everyone should be able to decrypt this because Olm sessions - // are per-device, not per-room. - wantDecryptableMsgBody := "Bob can see this even though his server is down as we had a session already" - waiter = alice.WaitUntilEventInRoom(t, roomIDab, api.CheckEventHasBody(wantDecryptableMsgBody)) - decryptableEventID := charlie.SendMessage(t, roomIDab, wantDecryptableMsgBody) - t.Logf("alice (%s) waiting for event %s", alice.Type(), decryptableEventID) - waiter.Wait(t, 5*time.Second) - - // now bob's server comes back online - tc.Deployment.UnpauseServer(t, "hs2") - - waiter = bob.WaitUntilEventInRoom(t, roomIDab, api.CheckEventHasBody(wantDecryptableMsgBody)) - waiter.Wait(t, 10*time.Second) // longer time to allow for retries + ForEachClientType(t, func(t *testing.T, clientType api.ClientType) { + tc := CreateTestContext(t, api.ClientType{ + Lang: clientType.Lang, + HS: "hs1", + }, api.ClientType{ + Lang: clientType.Lang, + HS: "hs2", + }, api.ClientType{ + Lang: clientType.Lang, + HS: "hs1", + }) + roomIDbc := tc.CreateNewEncryptedRoom(t, tc.Charlie, EncRoomOptions.Invite([]string{tc.Bob.UserID})) + roomIDab := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.Invite([]string{tc.Bob.UserID})) + t.Logf("%s joining rooms %s and %s", tc.Bob.UserID, roomIDab, roomIDbc) + tc.Bob.MustJoinRoom(t, roomIDab, []string{"hs1"}) + tc.Bob.MustJoinRoom(t, roomIDbc, []string{"hs1"}) + + tc.WithAliceBobAndCharlieSyncing(t, func(alice, bob, charlie api.Client) { + // let clients sync device keys + time.Sleep(time.Second) + + // ensure encrypted messaging works in rooms ab,bc + wantMsgBody := "Hello world" + waiter := bob.WaitUntilEventInRoom(t, roomIDab, api.CheckEventHasBody(wantMsgBody)) + evID := alice.SendMessage(t, roomIDab, wantMsgBody) + t.Logf("bob (%s) waiting for event %s", bob.Type(), evID) + waiter.Wait(t, 5*time.Second) + waiter = bob.WaitUntilEventInRoom(t, roomIDbc, api.CheckEventHasBody(wantMsgBody)) + evID = charlie.SendMessage(t, roomIDbc, wantMsgBody) + t.Logf("bob (%s) waiting for event %s", bob.Type(), evID) + waiter.Wait(t, 5*time.Second) + + // now bob's HS becomes unreachable + tc.Deployment.PauseServer(t, "hs2") + + // C now joins the room ab + tc.Alice.MustInviteRoom(t, roomIDab, tc.Charlie.UserID) + tc.Charlie.MustJoinRoom(t, roomIDab, []string{"hs1"}) + + // let charlie sync device keys... + time.Sleep(time.Second) + + // send a message as C: everyone should be able to decrypt this because Olm sessions + // are per-device, not per-room. + wantDecryptableMsgBody := "Bob can see this even though his server is down as we had a session already" + waiter = alice.WaitUntilEventInRoom(t, roomIDab, api.CheckEventHasBody(wantDecryptableMsgBody)) + decryptableEventID := charlie.SendMessage(t, roomIDab, wantDecryptableMsgBody) + t.Logf("alice (%s) waiting for event %s", alice.Type(), decryptableEventID) + waiter.Wait(t, 5*time.Second) + + // now bob's server comes back online + tc.Deployment.UnpauseServer(t, "hs2") + + waiter = bob.WaitUntilEventInRoom(t, roomIDab, api.CheckEventHasBody(wantDecryptableMsgBody)) + waiter.Wait(t, 10*time.Second) // longer time to allow for retries + }) }) } From 1de59e325f205eac7a2e1eb561ba79724f361d51 Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Mon, 18 Mar 2024 17:47:49 +0000 Subject: [PATCH 3/3] Review comments --- ENVIRONMENT.md | 2 +- internal/config/config.go | 2 +- tests/federation_connectivity_test.go | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/ENVIRONMENT.md b/ENVIRONMENT.md index d4a8769..a6f66b6 100644 --- a/ENVIRONMENT.md +++ b/ENVIRONMENT.md @@ -23,7 +23,7 @@ The client test matrix to run. Every test is run for each given permutation. The - `rj,rr`: Run the test twice. Run 1: Alice=rust, Bob=JS. Run 2: Alice=rust, Bob=rust. All on HS1. - `jJ`: Run the test once. Run 1: Alice=JS on HS1, Bob=JS on HS2. Tests federation. ``` - If the matrix only consists of one leter (e.g all j's) then rust-specific tests will not run and vice versa. + If the matrix only consists of one letter (e.g all j's) then rust-specific tests will not run and vice versa. - Type: `[][]ClientType` diff --git a/internal/config/config.go b/internal/config/config.go index 0034dd6..a9a72cf 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -27,7 +27,7 @@ type ComplementCrypto struct { // - `rj,rr`: Run the test twice. Run 1: Alice=rust, Bob=JS. Run 2: Alice=rust, Bob=rust. All on HS1. // - `jJ`: Run the test once. Run 1: Alice=JS on HS1, Bob=JS on HS2. Tests federation. // ``` - // If the matrix only consists of one leter (e.g all j's) then rust-specific tests will not run and vice versa. + // If the matrix only consists of one letter (e.g all j's) then rust-specific tests will not run and vice versa. TestClientMatrix [][2]api.ClientType // Which languages should be tested in ForEachClientType tests. diff --git a/tests/federation_connectivity_test.go b/tests/federation_connectivity_test.go index 3917d24..6991a7a 100644 --- a/tests/federation_connectivity_test.go +++ b/tests/federation_connectivity_test.go @@ -17,8 +17,6 @@ import ( // Ensure sending another message from C is decryptable. func TestNewUserCannotGetKeysForOfflineServer(t *testing.T) { ForEachClientType(t, func(t *testing.T, clientType api.ClientType) { - // normally we would use ForEachClient or ClientTypeMatrix but due to federation we need - // to pin it to certain langs/servers. tc := CreateTestContext(t, api.ClientType{ Lang: clientType.Lang, HS: "hs1",