Prevent session corruption by using same queue for encrypt vs. decrypt

// FREEBIE

src/Messages/OWSMessageSender.m

@@ -1,5 +1,6 @@
-//  Created by Michael Kirk on 10/7/16.
-//  Copyright © 2016 Open Whisper Systems. All rights reserved.
+//
+//  Copyright (c) 2017 Open Whisper Systems. All rights reserved.
+//
 
 #import "OWSMessageSender.h"
 #import "ContactsUpdater.h"
@@ -628,11 +629,17 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
             // DEPRECATED - Remove after all clients have been upgraded.
             BOOL isLegacyMessage = ![message isKindOfClass:[OWSOutgoingSyncMessage class]];
 
-            NSDictionary *messageDict = [self encryptedMessageWithPlaintext:plainText
-                                                                toRecipient:recipient.uniqueId
-                                                                   deviceId:deviceNumber
-                                                              keyingStorage:[TSStorageManager sharedManager]
-                                                                     legacy:isLegacyMessage];
+            __block NSDictionary *messageDict;
+            // Mutating session state is not thread safe, so we operate on a serial queue, shared with decryption
+            // operations.
+            dispatch_sync([OWSDispatch sessionCipher], ^{
+                messageDict = [self encryptedMessageWithPlaintext:plainText
+                                                      toRecipient:recipient.uniqueId
+                                                         deviceId:deviceNumber
+                                                    keyingStorage:[TSStorageManager sharedManager]
+                                                           legacy:isLegacyMessage];
+            });
+
             if (messageDict) {
                 [messagesArray addObject:messageDict];
             } else {
@@ -724,11 +731,9 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
                                                             recipientId:identifier
                                                                deviceId:[deviceNumber intValue]];
 
-    // Mutating session state is not thread safe.
-    id<CipherMessage> encryptedMessage;
-    @synchronized (self) {
-        encryptedMessage = [cipher encryptMessage:[plainText paddedMessageBody]];
-    }
+    id<CipherMessage> encryptedMessage = [cipher encryptMessage:[plainText paddedMessageBody]];
+
+
     NSData *serializedMessage = encryptedMessage.serialized;
     TSWhisperMessageType messageType = [self messageTypeForCipherMessage:encryptedMessage];
 

src/Network/WebSockets/TSSocketManager.m

@@ -5,6 +5,7 @@
 #import "SubProtocol.pb.h"
 
 #import "Cryptography.h"
+#import "OWSDispatch.h"
 #import "OWSSignalService.h"
 #import "OWSWebsocketSecurityPolicy.h"
 #import "TSAccountManager.h"
@@ -218,17 +219,21 @@ NSString *const SocketConnectingNotification = @"SocketConnectingNotification";
     [self keepAliveBackground];
 
     if ([message.path isEqualToString:@"/api/v1/message"] && [message.verb isEqualToString:@"PUT"]) {
-        NSData *decryptedPayload =
-            [Cryptography decryptAppleMessagePayload:message.body withSignalingKey:TSStorageManager.signalingKey];
-
-        if (!decryptedPayload) {
-            DDLogWarn(@"Failed to decrypt incoming payload or bad HMAC");
-            return;
-        }
+        // SessionCipher state is mutated in a couple places, during decryption, in handleReceivedEnvelope.
+        // To ensure consistent state, that decryption mutation must occur on the same queue as encryption.
+        dispatch_async([OWSDispatch sessionCipher], ^{
+            NSData *decryptedPayload =
+                [Cryptography decryptAppleMessagePayload:message.body withSignalingKey:TSStorageManager.signalingKey];
+
+            if (!decryptedPayload) {
+                DDLogWarn(@"Failed to decrypt incoming payload or bad HMAC");
+                return;
+            }
 
-        OWSSignalServiceProtosEnvelope *envelope = [OWSSignalServiceProtosEnvelope parseFromData:decryptedPayload];
+            OWSSignalServiceProtosEnvelope *envelope = [OWSSignalServiceProtosEnvelope parseFromData:decryptedPayload];
 
-        [[TSMessagesManager sharedManager] handleReceivedEnvelope:envelope];
+            [[TSMessagesManager sharedManager] handleReceivedEnvelope:envelope];
+        });
     } else {
         DDLogWarn(@"Unsupported WebSocket Request");
     }

src/Util/OWSDispatch.h

@@ -1,10 +1,25 @@
+//
+//  Copyright (c) 2017 Open Whisper Systems. All rights reserved.
+//
 
 NS_ASSUME_NONNULL_BEGIN
 
 @interface OWSDispatch : NSObject
 
+/**
+ * Attachment downloading
+ */
 + (dispatch_queue_t)attachmentsQueue;
 
+/**
+ * Signal protocol session state must be coordinated on a serial queue. This is sometimes used synchronously,
+ * so it's recommended that you avoid dispatching sync *from* this queue to avoid deadlock.
+ */
++ (dispatch_queue_t)sessionCipher;
+
+/**
+ * Serial message sending queue
+ */
 + (dispatch_queue_t)sendingQueue;
 
 @end

src/Util/OWSDispatch.m

@@ -1,5 +1,6 @@
-//  Created by Michael Kirk on 10/18/16.
-//  Copyright © 2016 Open Whisper Systems. All rights reserved.
+//
+//  Copyright (c) 2017 Open Whisper Systems. All rights reserved.
+//
 
 #import "OWSDispatch.h"
 
@@ -17,6 +18,16 @@ NS_ASSUME_NONNULL_BEGIN
     return queue;
 }
 
++ (dispatch_queue_t)sessionCipher
+{
+    static dispatch_once_t onceToken;
+    static dispatch_queue_t queue;
+    dispatch_once(&onceToken, ^{
+        queue = dispatch_queue_create("org.whispersystems.signal.sessionCipher", NULL);
+    });
+    return queue;
+}
+
 + (dispatch_queue_t)sendingQueue
 {
     static dispatch_once_t onceToken;