From c118daa061083be2c41632e36575b55f06e935f2 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Mon, 31 Aug 2020 13:49:16 +1000 Subject: [PATCH 1/7] Fix log --- .../src/Loki/API/Onion Requests/OnionRequestAPI.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift b/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift index 45a7c0e41..60540fd13 100644 --- a/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift +++ b/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift @@ -290,8 +290,7 @@ public enum OnionRequestAPI { let url = "\(guardSnode.address):\(guardSnode.port)/onion_req" let finalEncryptionResult = intermediate.finalEncryptionResult let onion = finalEncryptionResult.ciphertext - let requestSizeLimit = Double(FileServerAPI.maxFileSize) / FileServerAPI.fileSizeORMultiplier - if case Destination.server = destination, Double(onion.count) > 0.75 * requestSizeLimit { + if case Destination.server = destination, Double(onion.count) > 0.75 * Double(FileServerAPI.maxFileSize) { print("[Loki] Approaching request size limit: ~\(onion.count) bytes.") } let parameters: JSON = [ From 677b8e3348c2fc85d33c09ef49153209888d7247 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Mon, 31 Aug 2020 13:54:12 +1000 Subject: [PATCH 2/7] Fix SSK based closed group size inconsistency --- Signal/src/Loki/View Controllers/NewClosedGroupVC.swift | 2 +- .../src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Signal/src/Loki/View Controllers/NewClosedGroupVC.swift b/Signal/src/Loki/View Controllers/NewClosedGroupVC.swift index 3e31f2eff..dac0b5f99 100644 --- a/Signal/src/Loki/View Controllers/NewClosedGroupVC.swift +++ b/Signal/src/Loki/View Controllers/NewClosedGroupVC.swift @@ -174,7 +174,7 @@ final class NewClosedGroupVC : BaseVC, UITableViewDataSource, UITableViewDelegat guard selectedContacts.count >= 1 else { return showError(title: "Please pick at least 1 group member") } - guard selectedContacts.count < 50 else { // Minus one because we're going to include self later + guard selectedContacts.count < ClosedGroupsProtocol.groupSizeLimit else { // Minus one because we're going to include self later return showError(title: NSLocalizedString("vc_create_closed_group_too_many_group_members_error", comment: "")) } let selectedContacts = self.selectedContacts diff --git a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift index 8c233519a..3ce59d291 100644 --- a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift @@ -13,6 +13,7 @@ import PromiseKit @objc(LKClosedGroupsProtocol) public final class ClosedGroupsProtocol : NSObject { public static let isSharedSenderKeysEnabled = false + public static let groupSizeLimit = 10 // MARK: - Sending From e83205a3024d39c059192b26b3ff979b06e254b1 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Mon, 31 Aug 2020 15:51:12 +1000 Subject: [PATCH 3/7] Debug --- .../Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift index 3ce59d291..5622fc89a 100644 --- a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift @@ -183,6 +183,7 @@ public final class ClosedGroupsProtocol : NSObject { let userRatchet = SharedSenderKeysImplementation.shared.generateRatchet(for: groupPublicKey, senderPublicKey: userPublicKey, using: transaction) let userSenderKey = ClosedGroupSenderKey(chainKey: Data(hex: userRatchet.chainKey), keyIndex: userRatchet.keyIndex, publicKey: Data(hex: userPublicKey)) for member in members { // This internally takes care of multi device + guard member != userPublicKey else { continue } let thread = TSContactThread.getOrCreateThread(withContactId: member, transaction: transaction) thread.save(with: transaction) let closedGroupUpdateMessageKind = ClosedGroupUpdateMessage.Kind.senderKey(groupPublicKey: Data(hex: groupPublicKey), senderKey: userSenderKey) @@ -287,7 +288,7 @@ public final class ClosedGroupsProtocol : NSObject { } let group = thread.groupModel // Check that the sender is a member of the group (before the update) - var membersAndLinkedDevices: Set = Set(group.groupMemberIds) + var membersAndLinkedDevices: Set = Set(members) for member in group.groupMemberIds { let deviceLinks = OWSPrimaryStorage.shared().getDeviceLinks(for: member, in: transaction) membersAndLinkedDevices.formUnion(deviceLinks.flatMap { [ $0.master.publicKey, $0.slave.publicKey ] }) @@ -316,6 +317,7 @@ public final class ClosedGroupsProtocol : NSObject { let userRatchet = SharedSenderKeysImplementation.shared.generateRatchet(for: groupPublicKey, senderPublicKey: userPublicKey, using: transaction) let userSenderKey = ClosedGroupSenderKey(chainKey: Data(hex: userRatchet.chainKey), keyIndex: userRatchet.keyIndex, publicKey: Data(hex: userPublicKey)) for member in members { + guard member != userPublicKey else { continue } let thread = TSContactThread.getOrCreateThread(withContactId: member, transaction: transaction) thread.save(with: transaction) let closedGroupUpdateMessageKind = ClosedGroupUpdateMessage.Kind.senderKey(groupPublicKey: Data(hex: groupPublicKey), senderKey: userSenderKey) From 385dca2a81084d482f8e92c20e3b4ea55ba825fe Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Mon, 31 Aug 2020 16:04:10 +1000 Subject: [PATCH 4/7] Clean --- .../src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift index 5622fc89a..40aed41e1 100644 --- a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift @@ -55,6 +55,7 @@ public final class ClosedGroupsProtocol : NSObject { // Send a closed group update message to all members (and their linked devices) using established channels var promises: [Promise] = [] for member in members { // Not `membersAndLinkedDevices` as this internally takes care of multi device already + guard member != userPublicKey else { continue } let thread = TSContactThread.getOrCreateThread(withContactId: member, transaction: transaction) thread.save(with: transaction) let closedGroupUpdateMessageKind = ClosedGroupUpdateMessage.Kind.new(groupPublicKey: Data(hex: groupPublicKey), name: name, From cd7efff6f84cc25db34a851bf86f6e55900f1fa7 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 1 Sep 2020 10:36:47 +1000 Subject: [PATCH 5/7] Also request sender keys on failed encryption --- .../SharedSenderKeysImplementation.swift | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Closed Groups/SharedSenderKeysImplementation.swift b/SignalServiceKit/src/Loki/Protocol/Closed Groups/SharedSenderKeysImplementation.swift index 9f988f976..cc0da29b5 100644 --- a/SignalServiceKit/src/Loki/Protocol/Closed Groups/SharedSenderKeysImplementation.swift +++ b/SignalServiceKit/src/Loki/Protocol/Closed Groups/SharedSenderKeysImplementation.swift @@ -133,7 +133,17 @@ public final class SharedSenderKeysImplementation : NSObject, SharedSenderKeysPr } public func encrypt(_ plaintext: Data, for groupPublicKey: String, senderPublicKey: String, using transaction: YapDatabaseReadWriteTransaction) throws -> (ivAndCiphertext: Data, keyIndex: UInt) { - let ratchet = try stepRatchetOnce(for: groupPublicKey, senderPublicKey: senderPublicKey, using: transaction) + let ratchet: ClosedGroupRatchet + do { + ratchet = try stepRatchetOnce(for: groupPublicKey, senderPublicKey: senderPublicKey, using: transaction) + } catch { + // FIXME: It'd be cleaner to handle this in OWSMessageDecrypter (where all the other decryption errors are handled), but this was a lot more + // convenient because there's an easy way to get the sender public key from here. + if case RatchetingError.loadingFailed(_, _) = error { + ClosedGroupsProtocol.requestSenderKey(for: groupPublicKey, senderPublicKey: senderPublicKey, using: transaction) + } + throw error + } let iv = Data.getSecureRandomData(ofSize: SharedSenderKeysImplementation.ivSize)! let gcm = GCM(iv: iv.bytes, tagLength: Int(SharedSenderKeysImplementation.gcmTagSize), mode: .combined) let messageKey = ratchet.messageKeys.last! From aa14e9b7adbffbd9268903d0ba81d880218f3b18 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 1 Sep 2020 10:37:28 +1000 Subject: [PATCH 6/7] Don't show leave group option if user has already left --- .../OWSConversationSettingsViewController.m | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/Signal/src/ViewControllers/ThreadSettings/OWSConversationSettingsViewController.m b/Signal/src/ViewControllers/ThreadSettings/OWSConversationSettingsViewController.m index 3a0da4db5..5e7eb4304 100644 --- a/Signal/src/ViewControllers/ThreadSettings/OWSConversationSettingsViewController.m +++ b/Signal/src/ViewControllers/ThreadSettings/OWSConversationSettingsViewController.m @@ -683,22 +683,25 @@ const CGFloat kIconViewLength = 24; [weakSelf showGroupMembersView]; }] ]; - [mainSection addItem:[OWSTableItem - itemWithCustomCellBlock:^{ - UITableViewCell *cell = - [weakSelf disclosureCellWithName:NSLocalizedString(@"LEAVE_GROUP_ACTION", - @"table cell label in conversation settings") - iconName:@"table_ic_group_leave" - accessibilityIdentifier:ACCESSIBILITY_IDENTIFIER_WITH_NAME( - OWSConversationSettingsViewController, @"leave_group")]; - cell.userInteractionEnabled = !weakSelf.hasLeftGroup; - - return cell; - } - actionBlock:^{ - [weakSelf didTapLeaveGroup]; - }] - ]; + NSString *userPublicKey = OWSIdentityManager.sharedManager.identityKeyPair.hexEncodedPublicKey; + if ([((TSGroupThread *)self.thread).groupModel.groupMemberIds containsObject:userPublicKey]) { + [mainSection addItem:[OWSTableItem + itemWithCustomCellBlock:^{ + UITableViewCell *cell = + [weakSelf disclosureCellWithName:NSLocalizedString(@"LEAVE_GROUP_ACTION", + @"table cell label in conversation settings") + iconName:@"table_ic_group_leave" + accessibilityIdentifier:ACCESSIBILITY_IDENTIFIER_WITH_NAME( + OWSConversationSettingsViewController, @"leave_group")]; + cell.userInteractionEnabled = !weakSelf.hasLeftGroup; + + return cell; + } + actionBlock:^{ + [weakSelf didTapLeaveGroup]; + }] + ]; + } } From 2725e2096d6082f4e3cd5aecb08c01da2b02ee3a Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 1 Sep 2020 10:37:44 +1000 Subject: [PATCH 7/7] Add logs --- .../src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift index 40aed41e1..2fb9aa316 100644 --- a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift @@ -204,6 +204,7 @@ public final class ClosedGroupsProtocol : NSObject { } public static func requestSenderKey(for groupPublicKey: String, senderPublicKey: String, using transaction: YapDatabaseReadWriteTransaction) { + print("[Loki] Requesting sender key for group public key: \(groupPublicKey), sender public key: \(senderPublicKey).") // Establish session if needed SessionManagementProtocol.sendSessionRequestIfNeeded(to: senderPublicKey, using: transaction) // Send the request @@ -358,6 +359,7 @@ public final class ClosedGroupsProtocol : NSObject { return print("[Loki] Ignoring closed group sender key request from non-member.") } // Respond to the request + print("[Loki] Responding to sender key request from: \(senderPublicKey).") SessionManagementProtocol.sendSessionRequestIfNeeded(to: senderPublicKey, using: transaction) // This internally takes care of multi device let userRatchet = SharedSenderKeysImplementation.shared.generateRatchet(for: groupPublicKey, senderPublicKey: userPublicKey, using: transaction) let userSenderKey = ClosedGroupSenderKey(chainKey: Data(hex: userRatchet.chainKey), keyIndex: userRatchet.keyIndex, publicKey: Data(hex: userPublicKey)) @@ -393,6 +395,7 @@ public final class ClosedGroupsProtocol : NSObject { return print("[Loki] Ignoring invalid closed group sender key.") } // Store the sender key + print("[Loki] Received a sender key from: \(senderPublicKey).") let ratchet = ClosedGroupRatchet(chainKey: senderKey.chainKey.toHexString(), keyIndex: UInt(senderKey.keyIndex), messageKeys: []) Storage.setClosedGroupRatchet(for: groupPublicKey, senderPublicKey: senderPublicKey, ratchet: ratchet, using: transaction) }