Refine sealed sender logic

js/background.js

@@ -1121,9 +1121,11 @@
 
   // Sent:
   async function handleMessageSentProfileUpdate({
+    data,
     confirm,
     messageDescriptor,
   }) {
+    // First set profileSharing = true for the conversation we sent to
     const { id, type } = messageDescriptor;
     const conversation = await ConversationController.getOrCreateAndWait(
       id,
@@ -1135,6 +1137,14 @@
       Conversation: Whisper.Conversation,
     });
 
+    // Then we update our own profileKey if it's different from what we have
+    const ourNumber = textsecure.storage.user.getNumber();
+    const profileKey = data.message.profileKey.toString('base64');
+    const me = await ConversationController.getOrCreate(ourNumber, 'private');
+
+    // Will do the save for us if needed
+    await me.setProfileKey(profileKey);
+
     return confirm();
   }
 

js/models/conversations.js

@@ -16,6 +16,13 @@
 
   window.Whisper = window.Whisper || {};
 
+  const SEALED_SENDER = {
+    UNKNOWN: 0,
+    ENABLED: 1,
+    DISABLED: 2,
+    UNRESTRICTED: 3,
+  };
+
   const { Util } = window.Signal;
   const {
     Conversation,
@@ -113,6 +120,14 @@
       this.on('read', this.updateAndMerge);
       this.on('expiration-change', this.updateAndMerge);
       this.on('expired', this.onExpired);
+
+      const sealedSender = this.get('sealedSender');
+      if (sealedSender === undefined) {
+        this.set({ sealedSender: SEALED_SENDER.UNKNOWN });
+      }
+      this.unset('unidentifiedDelivery');
+      this.unset('unidentifiedDeliveryUnrestricted');
+      this.unset('hasFetchedProfile');
     },
 
     isMe() {
@@ -783,38 +798,65 @@
       return promise.then(
         async result => {
           // success
-          if (
+          if (result) {
-            result &&
+            await this.handleMessageSendResult(
-            result.failoverNumbers &&
+              result.failoverNumbers,
-            result.failoverNumbers.length
+              result.unidentifiedDeliveries
-          ) {
+            );
-            await this.handleFailover(result.failoverNumbers);
           }
           return result;
         },
         async result => {
           // failure
-          if (
+          if (result) {
-            result &&
+            await this.handleMessageSendResult(
-            result.failoverNumbers &&
+              result.failoverNumbers,
-            result.failoverNumbers.length
+              result.unidentifiedDeliveries
-          ) {
+            );
-            await this.handleFailover(result.failoverNumbers);
           }
           throw result;
         }
       );
     },
 
-    handleFailover(numberArray) {
+    async handleMessageSendResult(failoverNumbers, unidentifiedDeliveries) {
-      return Promise.all(
+      await Promise.all(
-        (numberArray || []).map(async number => {
+        (failoverNumbers || []).map(async number => {
           const conversation = ConversationController.get(number);
-          if (conversation && conversation.get('unidentifiedDelivery')) {
+
-            window.log.info(
+          if (
-              `Marking unidentifiedDelivery false for conversation ${conversation.idForLogging()}`
+            conversation &&
+            conversation.get('sealedSender') !== SEALED_SENDER.DISABLED
+          ) {
+            conversation.set({
+              sealedSender: SEALED_SENDER.DISABLED,
+            });
+            await window.Signal.Data.updateConversation(
+              conversation.id,
+              conversation.attributes,
+              { Conversation: Whisper.Conversation }
             );
-            conversation.set({ unidentifiedDelivery: false });
+          }
+        })
+      );
+
+      await Promise.all(
+        (unidentifiedDeliveries || []).map(async number => {
+          const conversation = ConversationController.get(number);
+
+          if (
+            conversation &&
+            conversation.get('sealedSender') === SEALED_SENDER.UNKNOWN
+          ) {
+            if (conversation.get('accessKey')) {
+              conversation.set({
+                sealedSender: SEALED_SENDER.ENABLED,
+              });
+            } else {
+              conversation.set({
+                sealedSender: SEALED_SENDER.UNRESTRICTED,
+              });
+            }
             await window.Signal.Data.updateConversation(
               conversation.id,
               conversation.attributes,
@@ -835,42 +877,54 @@
       };
     },
 
-    getNumberInfo() {
+    getNumberInfo({ disableMeCheck } = {}) {
-      const UD = 'unidentifiedDelivery';
-      const UNRESTRICTED_UD = 'unidentifiedDeliveryUnrestricted';
-
       // We don't want to enable unidentified delivery for send unless it is
       //   also enabled for our own account.
       const me = ConversationController.getOrCreate(this.ourNumber, 'private');
-      if (!me.get(UD) && !me.get(UNRESTRICTED_UD)) {
+      if (
+        !disableMeCheck &&
+        me.get('sealedSender') === SEALED_SENDER.DISABLED
+      ) {
         return null;
       }
 
-      if (this.isPrivate()) {
+      if (!this.isPrivate()) {
-        const accessKey = this.get('accessKey');
+        const infoArray = this.contactCollection.map(conversation =>
-        const unidentifiedDelivery = this.get(UD);
+          conversation.getNumberInfo({ disableMeCheck })
-        const unrestricted = this.get(UNRESTRICTED_UD);
+        );
+        return Object.assign({}, ...infoArray);
+      }
 
-        if (!unidentifiedDelivery && !unrestricted) {
+      const accessKey = this.get('accessKey');
-          return null;
+      const sealedSender = this.get('sealedSender');
-        }
 
+      // If we've never fetched user's profile, we default to what we have
+      if (sealedSender === SEALED_SENDER.UNKNOWN) {
         return {
           [this.id]: {
             accessKey:
-              accessKey && !unrestricted
+              accessKey ||
-                ? accessKey
+              window.Signal.Crypto.arrayBufferToBase64(
-                : window.Signal.Crypto.arrayBufferToBase64(
+                window.Signal.Crypto.getRandomBytes(16)
-                    window.Signal.Crypto.getRandomBytes(16)
+              ),
-                  ),
           },
         };
       }
 
-      const infoArray = this.contactCollection.map(conversation =>
+      if (sealedSender === SEALED_SENDER.DISABLED) {
-        conversation.getNumberInfo()
+        return null;
-      );
+      }
-      return Object.assign({}, ...infoArray);
+
+      return {
+        [this.id]: {
+          accessKey:
+            accessKey && sealedSender === SEALED_SENDER.ENABLED
+              ? accessKey
+              : window.Signal.Crypto.arrayBufferToBase64(
+                  window.Signal.Crypto.getRandomBytes(16)
+                ),
+        },
+      };
     },
 
     async updateLastMessage() {
@@ -1237,37 +1291,19 @@
       c.changed = {};
 
       try {
-        if (c.get('profileKey') && !c.get('accessKey')) {
+        await c.deriveAccessKeyIfNeeded();
-          const profileKeyBuffer = window.Signal.Crypto.base64ToArrayBuffer(
+        const numberInfo = c.getNumberInfo({ disableMeCheck: true }) || {};
-            c.get('profileKey')
+        const getInfo = numberInfo[c.id] || {};
-          );
-          const buffer = await window.Signal.Crypto.deriveAccessKey(
-            profileKeyBuffer
-          );
-          c.set({
-            accessKey: window.Signal.Crypto.arrayBufferToBase64(buffer),
-          });
-        }
-
-        const firstProfileFetch = !c.get('hasFetchedProfile');
-        const accessKey = c.get('accessKey');
 
         let profile;
-        if (c.get('unidentifiedDelivery') || firstProfileFetch) {
+        if (getInfo.accessKey) {
           try {
             profile = await textsecure.messaging.getProfile(id, {
-              accessKey:
+              accessKey: getInfo.accessKey,
-                accessKey ||
-                window.Signal.Crypto.arrayBufferToBase64(
-                  window.window.Signal.Crypto.getRandomBytes(16)
-                ),
             });
           } catch (error) {
             if (error.code === 401 || error.code === 403) {
-              c.set({
+              c.set({ sealedSender: SEALED_SENDER.DISABLED });
-                unidentifiedDelivery: false,
-                unidentifiedDeliveryUnrestricted: false,
-              });
               profile = await textsecure.messaging.getProfile(id);
             } else {
               throw error;
@@ -1297,17 +1333,13 @@
           await sessionCipher.closeOpenSessionForDevice();
         }
 
-        c.set({
+        const accessKey = c.get('accessKey');
-          hasFetchedProfile: true,
-        });
-
         if (
           profile.unrestrictedUnidentifiedAccess &&
           profile.unidentifiedAccess
         ) {
           c.set({
-            unidentifiedDelivery: true,
+            sealedSender: SEALED_SENDER.UNRESTRICTED,
-            unidentifiedDeliveryUnrestricted: true,
           });
         } else if (accessKey && profile.unidentifiedAccess) {
           const haveCorrectKey = await window.Signal.Crypto.verifyAccessKey(
@@ -1315,17 +1347,18 @@
             window.Signal.Crypto.base64ToArrayBuffer(profile.unidentifiedAccess)
           );
 
-          window.log.info(
+          if (haveCorrectKey) {
-            `Setting unidentifiedDelivery to ${haveCorrectKey} for conversation ${c.idForLogging()}`
+            c.set({
-          );
+              sealedSender: SEALED_SENDER.ENABLED,
-          c.set({
+            });
-            unidentifiedDelivery: haveCorrectKey,
+          } else {
-            unidentifiedDeliveryUnrestricted: false,
+            c.set({
-          });
+              sealedSender: SEALED_SENDER.DISABLED,
+            });
+          }
         } else {
           c.set({
-            unidentifiedDelivery: false,
+            sealedSender: SEALED_SENDER.DISABLED,
-            unidentifiedDeliveryUnrestricted: false,
           });
         }
 
@@ -1406,17 +1439,13 @@
     async setProfileKey(profileKey) {
       // profileKey is a string so we can compare it directly
       if (this.get('profileKey') !== profileKey) {
-        const profileKeyBuffer = window.Signal.Crypto.base64ToArrayBuffer(
+        this.set({
-          profileKey
+          profileKey,
-        );
+          accessKey: null,
-        const accessKeyBuffer = await window.Signal.Crypto.deriveAccessKey(
+          sealedSender: SEALED_SENDER.UNKNOWN,
-          profileKeyBuffer
+        });
-        );
-        const accessKey = window.Signal.Crypto.arrayBufferToBase64(
-          accessKeyBuffer
-        );
 
-        this.set({ profileKey, accessKey });
+        await this.deriveAccessKeyIfNeeded();
 
         await window.Signal.Data.updateConversation(this.id, this.attributes, {
           Conversation: Whisper.Conversation,
@@ -1424,6 +1453,27 @@
       }
     },
 
+    async deriveAccessKeyIfNeeded() {
+      const profileKey = this.get('profileKey');
+      if (!profileKey) {
+        return;
+      }
+      if (this.get('accessKey')) {
+        return;
+      }
+
+      const profileKeyBuffer = window.Signal.Crypto.base64ToArrayBuffer(
+        profileKey
+      );
+      const accessKeyBuffer = await window.Signal.Crypto.deriveAccessKey(
+        profileKeyBuffer
+      );
+      const accessKey = window.Signal.Crypto.arrayBufferToBase64(
+        accessKeyBuffer
+      );
+      this.set({ accessKey });
+    },
+
     async upgradeMessages(messages) {
       for (let max = messages.length, i = 0; i < max; i += 1) {
         const message = messages.at(i);

libtextsecure/outgoing_message.js

@@ -66,7 +66,7 @@ OutgoingMessage.prototype = {
     this.errors[this.errors.length] = error;
     this.numberCompleted();
   },
-  reloadDevicesAndSend(number, recurse, failover) {
+  reloadDevicesAndSend(number, recurse) {
     return () =>
       textsecure.storage.protocol.getDeviceIds(number).then(deviceIds => {
         if (deviceIds.length === 0) {
@@ -76,7 +76,7 @@ OutgoingMessage.prototype = {
             null
           );
         }
-        return this.doSendMessage(number, deviceIds, recurse, failover);
+        return this.doSendMessage(number, deviceIds, recurse);
       });
   },
 
@@ -245,7 +245,7 @@ OutgoingMessage.prototype = {
     return this.plaintext;
   },
 
-  doSendMessage(number, deviceIds, recurse, failover) {
+  doSendMessage(number, deviceIds, recurse) {
     const ciphers = {};
     const plaintext = this.getPlaintext();
 
@@ -261,8 +261,7 @@ OutgoingMessage.prototype = {
       );
     }
 
-    // If failover is true, we don't send an unidentified sender message
+    const sealedSender = Boolean(accessKey && senderCertificate);
-    const sealedSender = Boolean(!failover && accessKey && senderCertificate);
 
     // We don't send to ourselves if unless sealedSender is enabled
     const ourNumber = textsecure.storage.user.getNumber();
@@ -288,7 +287,6 @@ OutgoingMessage.prototype = {
           options.messageKeysLimit = false;
         }
 
-        // If failover is true, we don't send an unidentified sender message
         if (sealedSender) {
           const secretSessionCipher = new window.Signal.Metadata.SecretSessionCipher(
             textsecure.storage.protocol
@@ -397,9 +395,9 @@ OutgoingMessage.prototype = {
                 ? error.response.staleDevices
                 : error.response.missingDevices;
             return this.getKeysForNumber(number, resetDevices).then(
-              // For now, we we won't retry unidentified delivery if we get here; new
+              // We continue to retry as long as the error code was 409; the assumption is
-              //   devices could have been added which don't support it.
+              //   that we'll request new device info and the next request will succeed.
-              this.reloadDevicesAndSend(number, error.code === 409, true)
+              this.reloadDevicesAndSend(number, error.code === 409)
             );
           });
         } else if (error.message === 'Identity key changed') {