diff --git a/.gitignore b/.gitignore index 561f141..6bf3139 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ js-sdk/dist /internal/tests/logs/ __pycache__ /rust-sdk/ +/internal/tests/logs/ /tests/logs/ /tests/chromedp/ /tests/rust_storage/ diff --git a/TEST_HITLIST.md b/TEST_HITLIST.md index 771cfda..49468d1 100644 --- a/TEST_HITLIST.md +++ b/TEST_HITLIST.md @@ -76,7 +76,7 @@ Network connectivity tests are extremely time sensitive as retries are often usi - [x] If a client cannot upload OTKs, it retries. - [x] If a client cannot claim OTKs, it retries. - [x] If a server cannot send device list updates over federation, it retries. https://github.com/matrix-org/complement/pull/695 -- [x] If a client cannot query device keys for a user, it retries. +- [x] If a client cannot query device keys for a user, it retries. (TestFailedDeviceKeyDownloadRetries) - [ ] If a server cannot query device keys on another server, it retries. - [x] If a client cannot send a to-device msg, it retries. - [x] If a server cannot send a to-device msg to another server, it retries. https://github.com/matrix-org/complement/pull/694 @@ -98,7 +98,7 @@ This refers to cases where the client has some state and wishes to synchronise i - [ ] The room key is cycled when one of a user's devices is blacklisted. - [ ] The room key is cycled when history visibility changes to something more restrictive TODO: define precisely. - [ ] The room key is cycled when the encryption algorithm changes. -- [ ] The room key is cycled when `rotation_period_msgs` is met (default: 100). +- [x] The room key is cycled when `rotation_period_msgs` is met (default: 100). (TestRoomKeyIsCycledAfterEnoughMessages) - [ ] The room key is cycled when `rotation_period_ms` is exceeded (default: 1 week). - [x] The room key is not cycled when one of a user's devices logs in. - [x] The room key is not cycled when the client restarts. diff --git a/tests/device_keys_test.go b/tests/device_keys_test.go index 2008acd..caf4ebf 100644 --- a/tests/device_keys_test.go +++ b/tests/device_keys_test.go @@ -42,7 +42,7 @@ func TestFailedDeviceKeyDownloadRetries(t *testing.T) { }, }, func() { // And Alice and Bob are in an encrypted room together - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "private_chat", []string{tc.Bob.UserID}) + roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.Invite([]string{tc.Bob.UserID})) tc.Bob.MustJoinRoom(t, roomID, []string{"hs1"}) tc.WithAliceAndBobSyncing(t, func(alice, bob api.Client) { diff --git a/tests/federation_connectivity_test.go b/tests/federation_connectivity_test.go index c2cbdf3..f25ffe3 100644 --- a/tests/federation_connectivity_test.go +++ b/tests/federation_connectivity_test.go @@ -28,7 +28,7 @@ func TestNewUserCannotGetKeysForOfflineServer(t *testing.T) { Lang: api.ClientTypeRust, HS: "hs1", }) - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "private_chat", []string{tc.Bob.UserID}) + 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"}) @@ -108,8 +108,8 @@ func TestExistingSessionCannotGetKeysForOfflineServer(t *testing.T) { Lang: api.ClientTypeRust, HS: "hs1", }) - roomIDbc := tc.CreateNewEncryptedRoom(t, tc.Charlie, "private_chat", []string{tc.Bob.UserID}) - roomIDab := tc.CreateNewEncryptedRoom(t, tc.Alice, "private_chat", []string{tc.Bob.UserID}) + 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"}) diff --git a/tests/main_test.go b/tests/main_test.go index 9073e5a..d9b5278 100644 --- a/tests/main_test.go +++ b/tests/main_test.go @@ -241,16 +241,27 @@ func (c *TestContext) WithAliceBobAndCharlieSyncing(t *testing.T, callback func( }) } +// An option to customise the behaviour of CreateNewEncryptedRoom +type EncRoomOption = func(reqBody map[string]interface{}) + // CreateNewEncryptedRoom calls creator.MustCreateRoom with the correct m.room.encryption state event. -func (c *TestContext) CreateNewEncryptedRoom(t *testing.T, creator *client.CSAPI, preset string, invite []string) (roomID string) { +// +// options is a set of EncRoomOption that may be provided using methods on +// EncRoomOptions: +// - Preset*: the preset argument passed to createRoom (default: "private_chat") +// - Invite: a list of usernames to invite to the room (default: empty list) +// - RotationPeriodMsgs: value of the rotation_period_msgs param (default: omitted) +func (c *TestContext) CreateNewEncryptedRoom( + t *testing.T, + creator *client.CSAPI, + options ...EncRoomOption, +) (roomID string) { t.Helper() - if invite == nil { - invite = []string{} // else synapse 500s - } - return creator.MustCreateRoom(t, map[string]interface{}{ + + reqBody := map[string]interface{}{ "name": t.Name(), - "preset": preset, - "invite": invite, + "preset": "private_chat", + "invite": []string{}, "initial_state": []map[string]interface{}{ { "type": "m.room.encryption", @@ -260,7 +271,61 @@ func (c *TestContext) CreateNewEncryptedRoom(t *testing.T, creator *client.CSAPI }, }, }, - }) + } + + for _, option := range options { + option(reqBody) + } + + return creator.MustCreateRoom(t, reqBody) +} + +type encRoomOptions int + +// A namespace for the various options that may be passed in to CreateNewEncryptedRoom +const EncRoomOptions encRoomOptions = 0 + +// An option for CreateNewEncryptedRoom that requests the `preset` field to be +// set to `private_chat`. +func (encRoomOptions) PresetPrivateChat() EncRoomOption { + return setPreset("private_chat") +} + +// An option for CreateNewEncryptedRoom that requests the `preset` field to be +// set to `trusted_private_chat`. +func (encRoomOptions) PresetTrustedPrivateChat() EncRoomOption { + return setPreset("trusted_private_chat") +} + +// An option for CreateNewEncryptedRoom that requests the `preset` field to be +// set to `public_chat`. +func (encRoomOptions) PresetPublicChat() EncRoomOption { + return setPreset("public_chat") +} + +func setPreset(preset string) EncRoomOption { + return func(reqBody map[string]interface{}) { + reqBody["preset"] = preset + } +} + +// An option for CreateNewEncryptedRoom that provides a list of Matrix usernames +// to be supplied in the `invite` field. +func (encRoomOptions) Invite(invite []string) EncRoomOption { + return func(reqBody map[string]interface{}) { + reqBody["invite"] = invite + } +} + +// An option for CreateNewEncryptedRoom that adds a `rotation_period_msgs` field +// to the `m.room.encryption` event supplied when the room is created. +func (encRoomOptions) RotationPeriodMsgs(numMsgs int) EncRoomOption { + return func(reqBody map[string]interface{}) { + var initial_state = reqBody["initial_state"].([]map[string]interface{}) + var event = initial_state[0] + var content = event["content"].(map[string]interface{}) + content["rotation_period_msgs"] = numMsgs + } } // OptsFromClient converts a Complement client into a set of options which can be used to create an api.Client. diff --git a/tests/membership_acls_test.go b/tests/membership_acls_test.go index edb5105..3387871 100644 --- a/tests/membership_acls_test.go +++ b/tests/membership_acls_test.go @@ -23,7 +23,12 @@ func TestAliceBobEncryptionWorks(t *testing.T) { ClientTypeMatrix(t, func(t *testing.T, clientTypeA, clientTypeB api.ClientType) { tc := CreateTestContext(t, clientTypeA, clientTypeB) // Alice invites Bob to the encrypted room - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "trusted_private_chat", []string{tc.Bob.UserID}) + roomID := tc.CreateNewEncryptedRoom( + t, + tc.Alice, + EncRoomOptions.PresetTrustedPrivateChat(), + EncRoomOptions.Invite([]string{tc.Bob.UserID}), + ) tc.Bob.MustJoinRoom(t, roomID, []string{clientTypeA.HS}) // SDK testing below @@ -60,7 +65,12 @@ func TestCanDecryptMessagesAfterInviteButBeforeJoin(t *testing.T) { ClientTypeMatrix(t, func(t *testing.T, clientTypeA, clientTypeB api.ClientType) { tc := CreateTestContext(t, clientTypeA, clientTypeB) // Alice invites Bob to the encrypted room - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "trusted_private_chat", []string{tc.Bob.UserID}) + roomID := tc.CreateNewEncryptedRoom( + t, + tc.Alice, + EncRoomOptions.PresetTrustedPrivateChat(), + EncRoomOptions.Invite([]string{tc.Bob.UserID}), + ) // SDK testing below // ----------------- @@ -123,7 +133,7 @@ func TestBobCanSeeButNotDecryptHistoryInPublicRoom(t *testing.T) { ClientTypeMatrix(t, func(t *testing.T, clientTypeA, clientTypeB api.ClientType) { tc := CreateTestContext(t, clientTypeA, clientTypeB) // shared history visibility - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "public_chat", nil) + roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.PresetPublicChat()) // SDK testing below // ----------------- @@ -163,7 +173,7 @@ func TestOnRejoinBobCanSeeButNotDecryptHistoryInPublicRoom(t *testing.T) { } tc := CreateTestContext(t, clientTypeA, clientTypeB) // shared history visibility - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "public_chat", nil) + roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.PresetPublicChat()) tc.Bob.MustJoinRoom(t, roomID, []string{clientTypeA.HS}) // SDK testing below @@ -234,7 +244,7 @@ func TestOnNewDeviceBobCanSeeButNotDecryptHistoryInPublicRoom(t *testing.T) { ClientTypeMatrix(t, func(t *testing.T, clientTypeA, clientTypeB api.ClientType) { tc := CreateTestContext(t, clientTypeA, clientTypeB) // shared history visibility - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "public_chat", nil) + roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.PresetPublicChat()) tc.Bob.MustJoinRoom(t, roomID, []string{clientTypeA.HS}) // SDK testing below @@ -320,7 +330,7 @@ func TestChangingDeviceAfterInviteReEncrypts(t *testing.T) { ClientTypeMatrix(t, func(t *testing.T, clientTypeA, clientTypeB api.ClientType) { tc := CreateTestContext(t, clientTypeA, clientTypeB) // shared history visibility - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "public_chat", nil) + roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.PresetPublicChat()) tc.WithAliceAndBobSyncing(t, func(alice, bob api.Client) { // Alice invites Bob and then she sends an event diff --git a/tests/one_time_keys_test.go b/tests/one_time_keys_test.go index 5fe3011..d4fb709 100644 --- a/tests/one_time_keys_test.go +++ b/tests/one_time_keys_test.go @@ -126,7 +126,12 @@ func TestFallbackKeyIsUsedIfOneTimeKeysRunOut(t *testing.T) { fallbackKeyID, fallbackKey = mustClaimFallbackKey(t, otkGobbler, tc.Alice) // now bob & charlie try to talk to alice, the fallback key should be used - roomID = tc.CreateNewEncryptedRoom(t, tc.Bob, "public_chat", []string{tc.Alice.UserID, tc.Charlie.UserID}) + roomID = tc.CreateNewEncryptedRoom( + t, + tc.Bob, + EncRoomOptions.PresetPublicChat(), + EncRoomOptions.Invite([]string{tc.Alice.UserID, tc.Charlie.UserID}), + ) tc.Charlie.MustJoinRoom(t, roomID, []string{keyConsumerClientType.HS}) tc.Alice.MustJoinRoom(t, roomID, []string{keyConsumerClientType.HS}) charlie.WaitUntilEventInRoom(t, roomID, api.CheckEventHasMembership(alice.UserID(), "join")).Wait(t, 5*time.Second) @@ -246,7 +251,7 @@ func TestFailedKeysClaimRetries(t *testing.T) { defer close() // make a room which will link the 2 users together when - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "public_chat", nil) + roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.PresetPublicChat()) // block /keys/claim and join the room, causing the Olm session to be created tc.Deployment.WithMITMOptions(t, map[string]interface{}{ "statuscode": map[string]interface{}{ diff --git a/tests/room_keys_test.go b/tests/room_keys_test.go index 8a1b318..46362e8 100644 --- a/tests/room_keys_test.go +++ b/tests/room_keys_test.go @@ -29,7 +29,7 @@ func sniffToDeviceEvent(t *testing.T, d complement.Deployment, ch chan deploy.Ca return callbackURL, close } -// This test ensure we change the m.room_key when a device leaves an E2EE room. +// This test ensures we change the m.room_key when a device leaves an E2EE room. // If the key is not changed, the left device could potentially decrypt the encrypted // event if they could get access to it. func TestRoomKeyIsCycledOnDeviceLogout(t *testing.T) { @@ -40,7 +40,12 @@ func TestRoomKeyIsCycledOnDeviceLogout(t *testing.T) { csapiAlice2 := tc.MustRegisterNewDevice(t, tc.Alice, clientTypeA.HS, "OTHER_DEVICE") tc.WithAliceAndBobSyncing(t, func(alice, bob api.Client) { tc.WithClientSyncing(t, clientTypeA, csapiAlice2, func(alice2 api.Client) { - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "trusted_private_chat", []string{tc.Bob.UserID}) + roomID := tc.CreateNewEncryptedRoom( + t, + tc.Alice, + EncRoomOptions.PresetTrustedPrivateChat(), + EncRoomOptions.Invite([]string{tc.Bob.UserID}), + ) tc.Bob.MustJoinRoom(t, roomID, []string{clientTypeA.HS}) alice.WaitUntilEventInRoom(t, roomID, api.CheckEventHasMembership(tc.Bob.UserID, "join")).Wait(t, 5*time.Second) // check the room works @@ -92,6 +97,78 @@ func TestRoomKeyIsCycledOnDeviceLogout(t *testing.T) { }) } +// The room key is cycled when `rotation_period_msgs` is met (default: 100). +// +// This test ensures we change the m.room_key when we have sent enough messages, +// where "enough" means the value set in the `m.room.encryption` event under the +// `rotation_period_msgs` property. +// +// If the key were not changed, someone who stole the key would have access to +// future messages. +// +// See https://gitlab.matrix.org/matrix-org/olm/blob/master/docs/megolm.md#lack-of-backward-secrecy +func TestRoomKeyIsCycledAfterEnoughMessages(t *testing.T) { + ClientTypeMatrix(t, func(t *testing.T, clientTypeA, clientTypeB api.ClientType) { + // Given a room containing Alice and Bob + tc := CreateTestContext(t, clientTypeA, clientTypeB) + roomID := tc.CreateNewEncryptedRoom( + t, + tc.Alice, + EncRoomOptions.PresetTrustedPrivateChat(), + EncRoomOptions.Invite([]string{tc.Bob.UserID}), + EncRoomOptions.RotationPeriodMsgs(5), + ) + tc.Bob.MustJoinRoom(t, roomID, []string{clientTypeA.HS}) + + tc.WithAliceAndBobSyncing(t, func(alice, bob api.Client) { + // And some messages were sent, but not enough to trigger resending + for i := 0; i < 4; i++ { + wantMsgBody := "Before we hit the threshold" + waiter := bob.WaitUntilEventInRoom(t, roomID, api.CheckEventHasBody(wantMsgBody)) + alice.SendMessage(t, roomID, wantMsgBody) + waiter.Wait(t, 5*time.Second) + } + + // Sniff calls to /sendToDevice to ensure we see the new room key being sent. + ch := make(chan deploy.CallbackData, 10) + callbackURL, close := sniffToDeviceEvent(t, tc.Deployment, ch) + defer close() + tc.Deployment.WithMITMOptions(t, map[string]interface{}{ + "callback": map[string]interface{}{ + "callback_url": callbackURL, + "filter": "~u .*\\/sendToDevice.*", + }, + }, func() { + wantMsgBody := "This one hits the threshold" + // When we send two messages (one to hit the threshold and one to pass it) + // + // Note that we deliberately cover two possible valid behaviours + // of the client here. It's valid for the client to cycle the key: + // - eagerly as soon as the threshold is reached, or + // - lazily on the next message that would take the count above the threshold + // By sending two messages, we ensure that clients using either + // of these approaches will pass the test. + waiter := bob.WaitUntilEventInRoom(t, roomID, api.CheckEventHasBody(wantMsgBody)) + alice.SendMessage(t, roomID, wantMsgBody) + waiter.Wait(t, 5*time.Second) + + wantMsgBody = "After the threshold" + waiter = bob.WaitUntilEventInRoom(t, roomID, api.CheckEventHasBody(wantMsgBody)) + alice.SendMessage(t, roomID, wantMsgBody) + waiter.Wait(t, 5*time.Second) + }) + + // Then we did send out new keys + select { + case <-ch: + // Success - keys were sent + default: + ct.Fatalf(t, "did not see /sendToDevice after sending rotation_period_msgs messages") + } + }) + }) +} + func TestRoomKeyIsCycledOnMemberLeaving(t *testing.T) { ClientTypeMatrix(t, func(t *testing.T, clientTypeA, clientTypeB api.ClientType) { tc := CreateTestContext(t, clientTypeA, clientTypeB, clientTypeB) @@ -99,7 +176,12 @@ func TestRoomKeyIsCycledOnMemberLeaving(t *testing.T) { tc.WithAliceBobAndCharlieSyncing(t, func(alice, bob, charlie api.Client) { // do setup code after all clients are syncing to ensure that if Alice asks for Charlie's keys on receipt of the // join event, then Charlie has already uploaded keys. - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "trusted_private_chat", []string{tc.Bob.UserID, tc.Charlie.UserID}) + roomID := tc.CreateNewEncryptedRoom( + t, + tc.Alice, + EncRoomOptions.PresetTrustedPrivateChat(), + EncRoomOptions.Invite([]string{tc.Bob.UserID, tc.Charlie.UserID}), + ) tc.Bob.MustJoinRoom(t, roomID, []string{clientTypeA.HS}) tc.Charlie.MustJoinRoom(t, roomID, []string{clientTypeA.HS}) alice.WaitUntilEventInRoom(t, roomID, api.CheckEventHasMembership(tc.Charlie.UserID, "join")).Wait(t, 5*time.Second) @@ -153,7 +235,12 @@ func TestRoomKeyIsCycledOnMemberLeaving(t *testing.T) { func TestRoomKeyIsNotCycled(t *testing.T) { ClientTypeMatrix(t, func(t *testing.T, clientTypeA, clientTypeB api.ClientType) { tc := CreateTestContext(t, clientTypeA, clientTypeB) - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "trusted_private_chat", []string{tc.Bob.UserID}) + roomID := tc.CreateNewEncryptedRoom( + t, + tc.Alice, + EncRoomOptions.PresetTrustedPrivateChat(), + EncRoomOptions.Invite([]string{tc.Bob.UserID}), + ) tc.Bob.MustJoinRoom(t, roomID, []string{clientTypeA.HS}) // Alice, Bob are in a room. @@ -292,7 +379,12 @@ func TestRoomKeyIsNotCycledOnClientRestart(t *testing.T) { func testRoomKeyIsNotCycledOnClientRestartRust(t *testing.T, clientType api.ClientType) { tc := CreateTestContext(t, clientType, clientType) - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "trusted_private_chat", []string{tc.Bob.UserID}) + roomID := tc.CreateNewEncryptedRoom( + t, + tc.Alice, + EncRoomOptions.PresetTrustedPrivateChat(), + EncRoomOptions.Invite([]string{tc.Bob.UserID}), + ) tc.Bob.MustJoinRoom(t, roomID, []string{clientType.HS}) tc.WithClientSyncing(t, clientType, tc.Bob, func(bob api.Client) { @@ -371,7 +463,12 @@ func testRoomKeyIsNotCycledOnClientRestartRust(t *testing.T, clientType api.Clie func testRoomKeyIsNotCycledOnClientRestartJS(t *testing.T, clientType api.ClientType) { tc := CreateTestContext(t, clientType, clientType) - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "trusted_private_chat", []string{tc.Bob.UserID}) + roomID := tc.CreateNewEncryptedRoom( + t, + tc.Alice, + EncRoomOptions.PresetTrustedPrivateChat(), + EncRoomOptions.Invite([]string{tc.Bob.UserID}), + ) tc.Bob.MustJoinRoom(t, roomID, []string{clientType.HS}) // Alice and Bob are in a room. diff --git a/tests/to_device_test.go b/tests/to_device_test.go index 0061285..498c8ad 100644 --- a/tests/to_device_test.go +++ b/tests/to_device_test.go @@ -19,7 +19,7 @@ import ( func TestClientRetriesSendToDevice(t *testing.T) { ClientTypeMatrix(t, func(t *testing.T, clientTypeA, clientTypeB api.ClientType) { tc := CreateTestContext(t, clientTypeA, clientTypeB) - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "public_chat", nil) + roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.PresetPublicChat()) tc.Bob.MustJoinRoom(t, roomID, []string{clientTypeA.HS}) tc.WithAliceAndBobSyncing(t, func(alice, bob api.Client) { // lets device keys be exchanged @@ -73,7 +73,7 @@ func TestUnprocessedToDeviceMessagesArentLostOnRestart(t *testing.T) { ForEachClientType(t, func(t *testing.T, clientType api.ClientType) { // prepare for the test: register all 3 clients and create the room tc := CreateTestContext(t, clientType, clientType) - roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, "private_chat", []string{tc.Bob.UserID}) + roomID := tc.CreateNewEncryptedRoom(t, tc.Alice, EncRoomOptions.Invite([]string{tc.Bob.UserID})) tc.Bob.MustJoinRoom(t, roomID, []string{clientType.HS}) alice2 := tc.Deployment.Login(t, clientType.HS, tc.Alice, helpers.LoginOpts{ DeviceID: "ALICE_TWO",