From 3f956130c668d9f3dc00f4fbfe8a5cebeaca3f94 Mon Sep 17 00:00:00 2001 From: Michelle Linington Date: Tue, 21 Sep 2021 18:09:08 -0700 Subject: [PATCH] Ignore call messages that won't trigger CallKit ring --- .../NSECallMessageHandler.swift | 86 ++++++++++--------- .../Signaling/WebRTCCallMessageHandler.swift | 4 + .../environment/NoopCallMessageHandler.swift | 4 + .../src/Messages/OWSMessageManager.m | 28 ++++-- .../src/Protocols/OWSCallMessageHandler.h | 13 +++ .../src/TestUtils/OWSFakeCallMessageHandler.m | 5 ++ 6 files changed, 91 insertions(+), 49 deletions(-) diff --git a/NotificationServiceExtension/NSECallMessageHandler.swift b/NotificationServiceExtension/NSECallMessageHandler.swift index c147443c46..d1b50690cc 100644 --- a/NotificationServiceExtension/NSECallMessageHandler.swift +++ b/NotificationServiceExtension/NSECallMessageHandler.swift @@ -20,6 +20,22 @@ public class NSECallMessageHandler: NSObject, OWSCallMessageHandler { // MARK: - Call Handlers + public func action(for envelope: SSKProtoEnvelope, callMessage: SSKProtoCallMessage) -> OWSCallMessageAction { + // We should only handoff messages that are likely to cause a ring. + guard Date.ows_millisecondTimestamp() - envelope.timestamp < 5 * kMinuteInMs else { + Logger.info("Discarding very old call message. No longer relevant.") + } + + if callMessage.offer != nil { + return .handoff + } else if let opaqueMessage = callMessage.opaque, opaqueMessage.urgency == .handleImmediately { + return .handoff + } else { + Logger.info("Ignoring call message. Not an offer or urgent opaque message.") + return .ignore + } + } + public func externallyHandleCallMessage( envelope: SSKProtoEnvelope, plaintextData: Data, @@ -27,48 +43,37 @@ public class NSECallMessageHandler: NSObject, OWSCallMessageHandler { serverDeliveryTimestamp: UInt64, transaction: SDSAnyWriteTransaction ) -> Bool { - do { - // There used to be a way to hand off decrypted messages to the main - // app for processing, but it was prone to races. We have ideas to fix - // this but in the meantime, we'll be more aggressive about blocking - // and handoff. - // - // If we successfully wake the main app, we should kill the NSE mid- - // transaction without acking to the service. This will roll back our - // sessions to their prior state and the service will re-deliver - // the encrypted message to the main app. - // - // We have to block on a semaphore because we don't want to continue - // processing other messages and risk this transaction committing until - // we know that the main app isn't going to be launched. -// let payload = try CallMessageRelay.enqueueCallMessageForMainApp( -// envelope: envelope, -// plaintextData: plaintextData, -// wasReceivedByUD: wasReceivedByUD, -// serverDeliveryTimestamp: serverDeliveryTimestamp, -// transaction: transaction -// ) + // There used to be a way to hand off decrypted messages to the main + // app for processing, but it was prone to races. We have ideas to fix + // this but in the meantime, we'll be more aggressive about blocking + // and handoff. + // + // If we successfully wake the main app, we should kill the NSE mid- + // transaction without acking to the service. This will roll back our + // sessions to their prior state and the service will re-deliver + // the encrypted message to the main app. + // + // We have to block on a semaphore because we don't want to continue + // processing other messages and risk this transaction committing until + // we know that the main app isn't going to be launched. +// let payload = try CallMessageRelay.enqueueCallMessageForMainApp( + let payload = CallMessageRelay.wakeMainAppPayload() - let payload = CallMessageRelay.wakeMainAppPayload() - - // This semaphore should only be signalled if we failed to wake the main app. - // If we successfully wake the main app, we should exit. - let sema = DispatchSemaphore(value: 0) - - CXProvider.reportNewIncomingVoIPPushPayload(payload) { error in - if let error = error { - owsFailDebug("Failed to notify main app of call message: \(error)") - sema.signal() - } else { - Logger.info("Successfully notified main app of call message. Quitting...") - exit(0) - } - } - if sema.wait(timeout: .now() + .seconds(30)) == .timedOut { - owsFail("Timed out waiting for response from main app") + // This semaphore should only be signalled if we failed to wake the main app. + // If we successfully wake the main app, we should exit. + let sema = DispatchSemaphore(value: 0) + CXProvider.reportNewIncomingVoIPPushPayload(payload) { error in + if let error = error { + owsFailDebug("Failed to notify main app of call message: \(error)") + sema.signal() + } else { + Logger.info("Successfully notified main app of call message. Quitting...") + exit(0) } } - + if sema.wait(timeout: .now() + .seconds(30)) == .timedOut { + owsFail("Timed out waiting for response from main app") + } return true } @@ -114,6 +119,7 @@ public class NSECallMessageHandler: NSObject, OWSCallMessageHandler { _ update: SSKProtoDataMessageGroupCallUpdate, for groupThread: TSGroupThread, serverReceivedTimestamp: UInt64) { - owsFailDebug("This should never be called, calls are handled externally") + // TODO: Let's fix this soon, but for now let's just ignore to unblock iOS 15 + Logger.warn("Ignoring group call update message delivered to NSE") } } diff --git a/Signal/src/Calls/Signaling/WebRTCCallMessageHandler.swift b/Signal/src/Calls/Signaling/WebRTCCallMessageHandler.swift index e9edbab20f..7e4ff5bf31 100644 --- a/Signal/src/Calls/Signaling/WebRTCCallMessageHandler.swift +++ b/Signal/src/Calls/Signaling/WebRTCCallMessageHandler.swift @@ -20,6 +20,10 @@ public class WebRTCCallMessageHandler: NSObject, OWSCallMessageHandler { // MARK: - Call Handlers + public func action(for envelope: SSKProtoEnvelope, callMessage: SSKProtoCallMessage) -> OWSCallMessageAction { + .process + } + public func receivedOffer( _ offer: SSKProtoCallMessageOffer, from caller: SignalServiceAddress, diff --git a/SignalMessaging/environment/NoopCallMessageHandler.swift b/SignalMessaging/environment/NoopCallMessageHandler.swift index 3764120aef..435634a0bc 100644 --- a/SignalMessaging/environment/NoopCallMessageHandler.swift +++ b/SignalMessaging/environment/NoopCallMessageHandler.swift @@ -7,6 +7,10 @@ import SignalServiceKit @objc public class NoopCallMessageHandler: NSObject, OWSCallMessageHandler { + public func action(for envelope: SSKProtoEnvelope, callMessage: SSKProtoCallMessage) -> OWSCallMessageAction { + .process + } + public func receivedOffer(_ offer: SSKProtoCallMessageOffer, from caller: SignalServiceAddress, sourceDevice device: UInt32, sentAtTimestamp: UInt64, serverReceivedTimestamp: UInt64, serverDeliveryTimestamp: UInt64, supportsMultiRing: Bool) { owsFailDebug("") } diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 508aa7098c..f0815c162b 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -428,15 +428,25 @@ NS_ASSUME_NONNULL_BEGIN OWSLogInfo(@"Discarding message with timestamp: %llu", envelope.timestamp); return; } - if (![self.callMessageHandler externallyHandleCallMessageWithEnvelope:envelope - plaintextData:plaintextData - wasReceivedByUD:wasReceivedByUD - serverDeliveryTimestamp:serverDeliveryTimestamp - transaction:transaction]) { - [self handleIncomingEnvelope:envelope - withCallMessage:contentProto.callMessage - serverDeliveryTimestamp:serverDeliveryTimestamp - transaction:transaction]; + OWSCallMessageAction action = [self.callMessageHandler actionForEnvelope:envelope + callMessage:contentProto.callMessage]; + switch (action) { + case OWSCallMessageActionIgnore: + OWSLogInfo(@"Ignoring call message with timestamp: %llu", envelope.timestamp); + break; + case OWSCallMessageActionHandoff: + [self.callMessageHandler externallyHandleCallMessageWithEnvelope:envelope + plaintextData:plaintextData + wasReceivedByUD:wasReceivedByUD + serverDeliveryTimestamp:serverDeliveryTimestamp + transaction:transaction]; + break; + case OWSCallMessageActionProcess: + [self handleIncomingEnvelope:envelope + withCallMessage:contentProto.callMessage + serverDeliveryTimestamp:serverDeliveryTimestamp + transaction:transaction]; + break; } } else if (contentProto.typingMessage) { [self handleIncomingEnvelope:envelope diff --git a/SignalServiceKit/src/Protocols/OWSCallMessageHandler.h b/SignalServiceKit/src/Protocols/OWSCallMessageHandler.h index daa1138656..d8f9b4e50c 100644 --- a/SignalServiceKit/src/Protocols/OWSCallMessageHandler.h +++ b/SignalServiceKit/src/Protocols/OWSCallMessageHandler.h @@ -5,6 +5,7 @@ NS_ASSUME_NONNULL_BEGIN @class SDSAnyWriteTransaction; +@class SSKProtoCallMessage; @class SSKProtoCallMessageAnswer; @class SSKProtoCallMessageBusy; @class SSKProtoCallMessageHangup; @@ -16,8 +17,20 @@ NS_ASSUME_NONNULL_BEGIN @class SignalServiceAddress; @class TSGroupThread; +typedef NS_ENUM(NSUInteger, OWSCallMessageAction) { + // This message should not be processed + OWSCallMessageActionIgnore, + // Process the message by deferring to -externallyHandleCallMessage... + OWSCallMessageActionHandoff, + // Process the message normally + OWSCallMessageActionProcess, +}; + @protocol OWSCallMessageHandler +/// Informs caller of how the handler would like to handle this message +- (OWSCallMessageAction)actionForEnvelope:(SSKProtoEnvelope *)envelope callMessage:(SSKProtoCallMessage *)message; + - (void)receivedOffer:(SSKProtoCallMessageOffer *)offer fromCaller:(SignalServiceAddress *)caller sourceDevice:(uint32_t)device diff --git a/SignalServiceKit/src/TestUtils/OWSFakeCallMessageHandler.m b/SignalServiceKit/src/TestUtils/OWSFakeCallMessageHandler.m index aa5fc7da28..eabdde4cfb 100644 --- a/SignalServiceKit/src/TestUtils/OWSFakeCallMessageHandler.m +++ b/SignalServiceKit/src/TestUtils/OWSFakeCallMessageHandler.m @@ -10,6 +10,11 @@ NS_ASSUME_NONNULL_BEGIN @implementation OWSFakeCallMessageHandler +- (OWSCallMessageAction)actionForEnvelope:(SSKProtoEnvelope *)envelope callMessage:(SSKProtoCallMessage *)message; +{ + return OWSCallMessageActionProcess; +} + - (void)receivedOffer:(SSKProtoCallMessageOffer *)offer fromCaller:(SignalServiceAddress *)caller sourceDevice:(uint32_t)device