Skip to content

Commit 5abb4e3

Browse files
AnderasAnderas
committed
Return only keys matching the requested roomId
1 parent c7cf75c commit 5abb4e3

File tree

5 files changed

+87
-28
lines changed

5 files changed

+87
-28
lines changed

‎MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -533,12 +533,13 @@ - (void)requestKeysForEvent:(MXEvent*)event
533533

534534
#pragma mark - MXSharedHistoryKeyStore
535535

536-
537-
- (BOOL)hasSharedHistoryForSessionId:(NSString *)sessionId senderKey:(NSString *)senderKey
536+
- (BOOL)hasSharedHistoryForRoomId:(NSString *)roomId
537+
sessionId:(NSString *)sessionId
538+
senderKey:(NSString *)senderKey
538539
{
539540
MXOlmInboundGroupSession *session = [crypto.store inboundGroupSessionWithId:sessionId
540541
andSenderKey:senderKey];
541-
return session.sharedHistory;
542+
return session.sharedHistory && [session.roomId isEqualToString:roomId];
542543
}
543544

544545
- (void)shareKeysForRequest:(MXSharedHistoryKeyRequest *)request

‎MatrixSDK/Crypto/KeySharing/MXSharedHistoryKeyManager.swift‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,16 @@ import Foundation
2727
@objc
2828
public class MXSharedHistoryKeyManager: NSObject {
2929
struct SessionInfo: Hashable {
30-
let roomId: String
3130
let sessionId: String
3231
let senderKey: String
3332
}
3433

34+
private let roomId: String
3535
private let crypto: MXCrypto
3636
private let service: MXSharedHistoryKeyService
3737

38-
@objc public init(crypto: MXCrypto, service: MXSharedHistoryKeyService) {
38+
@objc public init(roomId: String, crypto: MXCrypto, service: MXSharedHistoryKeyService) {
39+
self.roomId = roomId
3940
self.crypto = crypto
4041
self.service = service
4142
}
@@ -67,7 +68,7 @@ public class MXSharedHistoryKeyManager: NSObject {
6768
let request = MXSharedHistoryKeyRequest(
6869
userId: userId,
6970
devices: devices,
70-
roomId: session.roomId,
71+
roomId: roomId,
7172
sessionId: session.sessionId,
7273
senderKey: session.senderKey
7374
)
@@ -94,21 +95,19 @@ public class MXSharedHistoryKeyManager: NSObject {
9495
private func sessionInfo(for message: MXEvent) -> SessionInfo? {
9596
let content = message.wireContent
9697
guard
97-
let roomId = message.roomId,
9898
let sessionId = content?["session_id"] as? String,
9999
let senderKey = content?["sender_key"] as? String
100100
else {
101101
MXLog.debug("[MXSharedHistoryRoomKeyRequestManager] Cannot create key request")
102102
return nil
103103
}
104-
105-
guard service.hasSharedHistory(forSessionId: sessionId, senderKey: senderKey) else {
106-
MXLog.debug("[MXSharedHistoryRoomKeyRequestManager] Skipping keys for message without shared history")
104+
105+
guard service.hasSharedHistory(forRoomId: roomId, sessionId: sessionId, senderKey: senderKey) else {
106+
MXLog.debug("[MXSharedHistoryRoomKeyRequestManager] Skipping keys for message without shared history or mismatched room identifier")
107107
return nil
108108
}
109109

110110
return .init(
111-
roomId: roomId,
112111
sessionId: sessionId,
113112
senderKey: senderKey
114113
)

‎MatrixSDK/Crypto/KeySharing/MXSharedHistoryKeyService.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ FOUNDATION_EXPORT NSString *const kMXSharedHistoryKeyName;
3232
/**
3333
Check whether key for a given session (sessionId + senderKey) exists
3434
*/
35-
- (BOOL)hasSharedHistoryForSessionId:(NSString *)sessionId
36-
senderKey:(NSString *)senderKey;
35+
- (BOOL)hasSharedHistoryForRoomId:(NSString *)roomId
36+
sessionId:(NSString *)sessionId
37+
senderKey:(NSString *)senderKey;
3738

3839
/**
3940
Share keys for a given request, containing userId, list of devices and session to share

‎MatrixSDK/Data/MXRoom.m‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,9 @@ - (id)initWithRoomId:(NSString *)roomId matrixSession:(MXSession *)mxSession2 an
125125
if (mxSession.crypto)
126126
{
127127
MXMegolmDecryption *decryption = [[MXMegolmDecryption alloc] initWithCrypto:mxSession.crypto];
128-
sharedHistoryKeyManager = [[MXSharedHistoryKeyManager alloc] initWithCrypto:mxSession.crypto service:decryption];
128+
sharedHistoryKeyManager = [[MXSharedHistoryKeyManager alloc] initWithRoomId:roomId
129+
crypto:mxSession.crypto
130+
service:decryption];
129131
}
130132

131133
if (store)

‎MatrixSDKTests/Crypto/KeySharing/MXSharedHistoryKeyManagerUnitTests.swift‎

Lines changed: 70 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,20 @@ class MXSharedHistoryKeyManagerUnitTests: XCTestCase {
2929
}
3030

3131
class SpyService: NSObject, MXSharedHistoryKeyService {
32-
var sharedHistory: Set<String>?
33-
func hasSharedHistory(forSessionId sessionId: String!, senderKey: String!) -> Bool {
32+
struct SessionStub: Hashable {
33+
let roomId: String
34+
let sessionId: String
35+
let senderKey: String
36+
}
37+
38+
var sharedHistory: Set<SessionStub>?
39+
func hasSharedHistory(forRoomId roomId: String!, sessionId: String!, senderKey: String!) -> Bool {
3440
guard let sharedHistory = sharedHistory else {
3541
return true
3642
}
37-
return sharedHistory.contains(sessionId)
43+
44+
let session = SessionStub(roomId: roomId, sessionId: sessionId, senderKey: senderKey)
45+
return sharedHistory.contains(session)
3846
}
3947

4048
var requests = [MXSharedHistoryKeyRequest]()
@@ -76,15 +84,14 @@ class MXSharedHistoryKeyManagerUnitTests: XCTestCase {
7684
crypto.devices.setObject(MXDeviceInfo(deviceId: "1"), forUser: "user1", andDevice: "1")
7785

7886
service = SpyService()
79-
manager = MXSharedHistoryKeyManager(crypto: crypto, service: service)
8087
}
8188

8289
private func makeEvent(
8390
sessionId: String = "123",
8491
senderKey: String = "456"
8592
) -> MXEvent {
8693
MXEvent(fromJSON: [
87-
"room_id": "123",
94+
"room_id": "ABC",
8895
"type": kMXEventTypeStringRoomEncrypted,
8996
"content": [
9097
"session_id": sessionId,
@@ -93,13 +100,35 @@ class MXSharedHistoryKeyManagerUnitTests: XCTestCase {
93100
])
94101
}
95102

103+
private func makeInboundSession(
104+
roomId: String = "ABC",
105+
sessionId: String = "123",
106+
senderKey: String = "456"
107+
) -> SpyService.SessionStub {
108+
return .init(roomId: roomId, sessionId: sessionId, senderKey: senderKey)
109+
}
110+
111+
private func shareKeys(
112+
userId: String = "user1",
113+
roomId: String = "ABC",
114+
enumerator: MXEventsEnumerator? = nil,
115+
limit: Int = .max
116+
) {
117+
manager = MXSharedHistoryKeyManager(roomId: roomId, crypto: crypto, service: service)
118+
manager.shareMessageKeys(
119+
withUserId: userId,
120+
messageEnumerator: enumerator ?? self.enumerator,
121+
limit: limit
122+
)
123+
}
124+
96125
func testDoesNotCreateRequestIfNoKnownDevices() {
97126
enumerator.messages = [
98127
makeEvent(sessionId: "A", senderKey: "B")
99128
]
100129
crypto.devices = MXUsersDevicesMap<MXDeviceInfo>()
101130

102-
manager.shareMessageKeys(withUserId: "user1", messageEnumerator: enumerator, limit: .max)
131+
shareKeys()
103132

104133
XCTAssertEqual(service.requests.count, 0)
105134
}
@@ -112,7 +141,7 @@ class MXSharedHistoryKeyManagerUnitTests: XCTestCase {
112141
crypto.devices.setObject(MXDeviceInfo(deviceId: "2"), forUser: "user1", andDevice: "2")
113142
crypto.devices.setObject(MXDeviceInfo(deviceId: "3"), forUser: "user2", andDevice: "3")
114143

115-
manager.shareMessageKeys(withUserId: "user1", messageEnumerator: enumerator, limit: .max)
144+
shareKeys()
116145

117146
XCTAssertEqual(service.requests.count, 1)
118147
XCTAssertEqual(
@@ -123,7 +152,7 @@ class MXSharedHistoryKeyManagerUnitTests: XCTestCase {
123152
MXDeviceInfo(deviceId: "1"),
124153
MXDeviceInfo(deviceId: "2")
125154
],
126-
roomId: "123",
155+
roomId: "ABC",
127156
sessionId: "A",
128157
senderKey: "B"
129158
)
@@ -141,7 +170,7 @@ class MXSharedHistoryKeyManagerUnitTests: XCTestCase {
141170
makeEvent(sessionId: "3", senderKey: "B"),
142171
]
143172

144-
manager.shareMessageKeys(withUserId: "user1", messageEnumerator: enumerator, limit: .max)
173+
shareKeys()
145174

146175
let identifiers = service.requests.map { [$0.sessionId, $0.senderKey] }
147176
XCTAssertEqual(service.requests.count, 5)
@@ -161,7 +190,7 @@ class MXSharedHistoryKeyManagerUnitTests: XCTestCase {
161190
makeEvent(sessionId: "1"),
162191
]
163192

164-
manager.shareMessageKeys(withUserId: "user1", messageEnumerator: enumerator, limit: 3)
193+
shareKeys(limit: 3)
165194

166195
let identifiers = service.requests.map { $0.sessionId }
167196
XCTAssertEqual(service.requests.count, 3)
@@ -177,17 +206,44 @@ class MXSharedHistoryKeyManagerUnitTests: XCTestCase {
177206
makeEvent(sessionId: "5"),
178207
]
179208
service.sharedHistory = [
180-
"1",
181-
"2",
182-
"4",
209+
makeInboundSession(sessionId: "1"),
210+
makeInboundSession(sessionId: "2"),
211+
makeInboundSession(sessionId: "4"),
183212
]
184213

185-
manager.shareMessageKeys(withUserId: "user1", messageEnumerator: enumerator, limit: .max)
214+
shareKeys()
186215

187216
let identifiers = service.requests.map { $0.sessionId }
188217
XCTAssertEqual(service.requests.count, 3)
189218
XCTAssertEqual(Set(identifiers), ["1", "2", "4"])
190219
}
220+
221+
func testIgnoresEventsWithMismatchedRoomId() {
222+
enumerator.messages = [
223+
makeEvent(sessionId: "1"),
224+
makeEvent(sessionId: "2"),
225+
makeEvent(sessionId: "3"),
226+
]
227+
service.sharedHistory = [
228+
makeInboundSession(
229+
roomId: "XYZ",
230+
sessionId: "1"
231+
),
232+
makeInboundSession(
233+
roomId: "ABC",
234+
sessionId: "2"
235+
),
236+
makeInboundSession(
237+
roomId: "XYZ",
238+
sessionId: "3"
239+
),
240+
]
241+
242+
shareKeys(roomId: "ABC")
243+
244+
XCTAssertEqual(service.requests.count, 1)
245+
XCTAssertEqual(service.requests.first?.sessionId, "2")
246+
}
191247
}
192248

193249
extension MXSharedHistoryKeyRequest {

0 commit comments

Comments
 (0)