Harden UUID-handling code paths

js/background.js

@@ -244,9 +244,7 @@
           regionCode: window.storage.get('regionCode'),
           ourNumber,
           ourUuid,
-          ourConversationId: ConversationController.getConversationId(
+          ourConversationId: ConversationController.getOurConversationId(),
-            ourNumber || ourUuid
-          ),
         };
         Whisper.events.trigger('userChanged', user);
 
@@ -616,9 +614,7 @@
     );
     const ourNumber = textsecure.storage.user.getNumber();
     const ourUuid = textsecure.storage.user.getUuid();
-    const ourConversationId = ConversationController.getConversationId(
+    const ourConversationId = ConversationController.getOurConversationId();
-      ourNumber || ourUuid
-    );
     const initialState = {
       conversations: {
         conversationLookup: Signal.Util.makeLookup(conversations, 'id'),
@@ -1803,10 +1799,9 @@
         Whisper.events.trigger('contactsync');
       });
 
-      const ourUuid = textsecure.storage.user.getUuid();
+      const ourId = ConversationController.getOurConversationId();
-      const ourNumber = textsecure.storage.user.getNumber();
       const { wrap, sendOptions } = ConversationController.prepareForSend(
-        ourNumber || ourUuid,
+        ourId,
         {
           syncMessage: true,
         }
@@ -1953,18 +1948,16 @@
       return;
     }
 
-    const conversation = ConversationController.get(
+    const senderId = ConversationController.ensureContactIds({
-      groupId || sender || senderUuid
+      e164: sender,
-    );
+      uuid: senderUuid,
-    const ourUuid = textsecure.storage.user.getUuid();
+    });
-    const ourNumber = textsecure.storage.user.getNumber();
+    const conversation = ConversationController.get(groupId || senderId);
+    const ourId = ConversationController.getOurConversationId();
 
     if (conversation) {
       // We drop typing notifications in groups we're not a part of
-      if (
+      if (!conversation.isPrivate() && !conversation.hasMember(ourId)) {
-        !conversation.isPrivate() &&
-        !conversation.hasMember(ourNumber || ourUuid)
-      ) {
         window.log.warn(
           `Received typing indicator for group ${conversation.idForLogging()}, which we're not a part of. Dropping.`
         );
@@ -1973,8 +1966,10 @@
 
       conversation.notifyTyping({
         isTyping: started,
+        isMe: ourId === senderId,
         sender,
         senderUuid,
+        senderId,
         senderDevice,
       });
     }
@@ -2046,10 +2041,11 @@
     }
 
     try {
-      const conversation = await ConversationController.getOrCreateAndWait(
+      const detailsId = ConversationController.ensureContactIds({
-        details.number || details.uuid,
+        e164: details.number,
-        'private'
+        uuid: details.uuid,
-      );
+      });
+      const conversation = ConversationController.get(detailsId);
       let activeAt = conversation.get('active_at');
 
       // The idea is to make any new contact show up in the left pane. If
@@ -2116,15 +2112,16 @@
       const { expireTimer } = details;
       const isValidExpireTimer = typeof expireTimer === 'number';
       if (isValidExpireTimer) {
-        const sourceE164 = textsecure.storage.user.getNumber();
+        const ourId = ConversationController.getOurConversationId();
-        const sourceUuid = textsecure.storage.user.getUuid();
         const receivedAt = Date.now();
 
         await conversation.updateExpirationTimer(
           expireTimer,
-          sourceE164 || sourceUuid,
+          ourId,
           receivedAt,
-          { fromSync: true }
+          {
+            fromSync: true,
+          }
         );
       }
 
@@ -2265,7 +2262,13 @@
   const getDescriptorForReceived = ({ message, source, sourceUuid }) =>
     message.group
       ? getGroupDescriptor(message.group)
-      : { type: Message.PRIVATE, id: source || sourceUuid };
+      : {
+          type: Message.PRIVATE,
+          id: ConversationController.ensureContactIds({
+            e164: source,
+            uuid: sourceUuid,
+          }),
+        };
 
   // Received:
   async function handleMessageReceivedProfileUpdate({
@@ -2304,22 +2307,7 @@
       });
     }
 
-    const message = initIncomingMessage(data);
+    const message = initIncomingMessage(data, messageDescriptor);
-
-    const result = ConversationController.getOrCreate(
-      messageDescriptor.id,
-      messageDescriptor.type,
-      messageDescriptor.type === 'group'
-        ? { addedBy: message.getContact().get('id') }
-        : undefined
-    );
-
-    if (messageDescriptor.type === 'private') {
-      result.updateE164(data.source);
-      if (data.sourceUuid) {
-        result.updateUuid(data.sourceUuid);
-      }
-    }
 
     if (data.message.reaction) {
       const { reaction } = data.message;
@@ -2331,7 +2319,10 @@
         targetAuthorUuid: reaction.targetAuthorUuid,
         targetTimestamp: reaction.targetTimestamp.toNumber(),
         timestamp: Date.now(),
-        fromId: data.source || data.sourceUuid,
+        fromId: ConversationController.ensureContactIds({
+          e164: data.source,
+          uuid: data.sourceUuid,
+        }),
       });
       // Note: We do not wait for completion here
       Whisper.Reactions.onReaction(reactionModel);
@@ -2345,7 +2336,10 @@
       const deleteModel = Whisper.Deletes.add({
         targetSentTimestamp: del.targetSentTimestamp,
         serverTimestamp: data.serverTimestamp,
-        fromId: data.source || data.sourceUuid,
+        fromId: ConversationController.ensureContactIds({
+          e164: data.source,
+          uuid: data.sourceUuid,
+        }),
       });
       // Note: We do not wait for completion here
       Whisper.Deletes.onDelete(deleteModel);
@@ -2410,13 +2404,9 @@
     window.Signal.Data.updateConversation(conversation.attributes);
 
     // Then we update our own profileKey if it's different from what we have
-    const ourNumber = textsecure.storage.user.getNumber();
+    const ourId = ConversationController.getOurConversationId();
-    const ourUuid = textsecure.storage.user.getUuid();
+    const me = ConversationController.get(ourId);
     const profileKey = data.message.profileKey.toString('base64');
-    const me = await ConversationController.getOrCreate(
-      ourNumber || ourUuid,
-      'private'
-    );
 
     // Will do the save for us if needed
     await me.setProfileKey(profileKey);
@@ -2477,9 +2467,6 @@
 
     const message = createSentMessage(data);
 
-    const ourNumber = textsecure.storage.user.getNumber();
-    const ourUuid = textsecure.storage.user.getUuid();
-
     if (data.message.reaction) {
       const { reaction } = data.message;
       const reactionModel = Whisper.Reactions.add({
@@ -2489,7 +2476,7 @@
         targetAuthorUuid: reaction.targetAuthorUuid,
         targetTimestamp: reaction.targetTimestamp.toNumber(),
         timestamp: Date.now(),
-        fromId: ourNumber || ourUuid,
+        fromId: ConversationController.getOurConversationId(),
         fromSync: true,
       });
       // Note: We do not wait for completion here
@@ -2504,7 +2491,7 @@
       const deleteModel = Whisper.Deletes.add({
         targetSentTimestamp: del.targetSentTimestamp,
         serverTimestamp: del.serverTimestamp,
-        fromId: ourNumber || ourUuid,
+        fromId: ConversationController.getOurConversationId(),
       });
       // Note: We do not wait for completion here
       Whisper.Deletes.onDelete(deleteModel);
@@ -2525,10 +2512,22 @@
     return Promise.resolve();
   }
 
-  function initIncomingMessage(data) {
+  function initIncomingMessage(data, descriptor) {
-    const targetId = data.source || data.sourceUuid;
+    // Ensure that we have an accurate record for who this message is from
-    const conversation = ConversationController.get(targetId);
+    const fromContactId = ConversationController.ensureContactIds({
-    const conversationId = conversation ? conversation.id : targetId;
+      e164: data.source,
+      uuid: data.sourceUuid,
+    });
+
+    // Determine if this message is in a group
+    const isGroup = descriptor.type === Message.GROUP;
+
+    // Determine the conversationId this message belongs to
+    const conversationId = isGroup
+      ? ConversationController.ensureGroup(descriptor.id, {
+          addedBy: fromContactId,
+        })
+      : fromContactId;
 
     return new Whisper.Message({
       source: data.source,
@@ -2640,7 +2639,13 @@
         return Promise.resolve();
       }
       const envelope = ev.proto;
-      const message = initIncomingMessage(envelope);
+      const message = initIncomingMessage(envelope, {
+        type: Message.PRIVATE,
+        id: ConversationController.ensureContactIds({
+          e164: envelope.source,
+          uuid: envelope.sourceUuid,
+        }),
+      });
 
       const conversationId = message.get('conversationId');
       const conversation = ConversationController.getOrCreate(
@@ -2832,10 +2837,8 @@
       ev.viaContactSync ? 'via contact sync' : ''
     );
 
-    const contact = await ConversationController.getOrCreateAndWait(
+    const verifiedId = ConversationController.ensureContactIds({ e164, uuid });
-      e164 || uuid,
+    const contact = await ConversationController.get(verifiedId, 'private');
-      'private'
-    );
     const options = {
       viaSyncMessage: true,
       viaContactSync: ev.viaContactSync,

js/conversation_controller.js

@@ -67,7 +67,7 @@
     dangerouslyCreateAndAdd(attributes) {
       return conversations.add(attributes);
     },
-    getOrCreate(identifier, type) {
+    getOrCreate(identifier, type, additionalInitialProps = {}) {
       if (typeof identifier !== 'string') {
         throw new TypeError("'id' must be a string");
       }
@@ -99,6 +99,7 @@
           groupId: identifier,
           type,
           version: 2,
+          ...additionalInitialProps,
         });
       } else if (window.isValidGuid(identifier)) {
         conversation = conversations.add({
@@ -108,6 +109,7 @@
           groupId: null,
           type,
           version: 2,
+          ...additionalInitialProps,
         });
       } else {
         conversation = conversations.add({
@@ -117,6 +119,7 @@
           groupId: null,
           type,
           version: 2,
+          ...additionalInitialProps,
         });
       }
 
@@ -154,9 +157,9 @@
 
       return conversation;
     },
-    getOrCreateAndWait(id, type) {
+    getOrCreateAndWait(id, type, additionalInitialProps = {}) {
       return this._initialPromise.then(() => {
-        const conversation = this.getOrCreate(id, type);
+        const conversation = this.getOrCreate(id, type, additionalInitialProps);
 
         if (conversation) {
           return conversation.initialPromise.then(() => conversation);
@@ -184,7 +187,58 @@
     getOurConversationId() {
       const e164 = textsecure.storage.user.getNumber();
       const uuid = textsecure.storage.user.getUuid();
-      return this.getConversationId(e164 || uuid);
+      return this.ensureContactIds({ e164, uuid });
+    },
+    /**
+     * Given a UUID and/or an E164, resolves to a string representing the local
+     * database of the given contact. If a conversation is found it is updated
+     * to have the given UUID and E164. If a conversation is not found, this
+     * function creates a conversation with the given UUID and E164. If the
+     * conversation * is found in the local database it is updated.
+     *
+     * This function also additionally checks for mismatched e164/uuid pairs out
+     * of abundance of caution.
+     */
+    ensureContactIds({ e164, uuid }) {
+      // Check for at least one parameter being provided. This is necessary
+      // because this path can be called on startup to resolve our own ID before
+      // our phone number or UUID are known. The existing behavior in these
+      // cases can handle a returned `undefined` id, so we do that.
+      if (!e164 && !uuid) {
+        return undefined;
+      }
+
+      const lowerUuid = uuid ? uuid.toLowerCase() : undefined;
+
+      const convoE164 = this.get(e164);
+      const convoUuid = this.get(lowerUuid);
+
+      // Check for mismatched UUID and E164
+      if (
+        convoE164 &&
+        convoUuid &&
+        convoE164.get('id') !== convoUuid.get('id')
+      ) {
+        window.log.warn('Received a message with a mismatched UUID and E164.');
+      }
+
+      const convo = convoUuid || convoE164;
+
+      const idOrIdentifier = convo ? convo.get('id') : e164 || lowerUuid;
+
+      const finalConversation = this.getOrCreate(idOrIdentifier, 'private');
+      finalConversation.updateE164(e164);
+      finalConversation.updateUuid(lowerUuid);
+
+      return finalConversation.get('id');
+    },
+    /**
+     * Given a groupId and optional additional initialization properties,
+     * ensures the existence of a group conversation and returns a string
+     * representing the local database ID of the group conversation.
+     */
+    ensureGroup(groupId, additionalInitProps = {}) {
+      return this.getOrCreate(groupId, 'group', additionalInitProps).get('id');
     },
     /**
      * Given certain metadata about a message (an identifier of who wrote the

js/models/conversations.js

@@ -435,7 +435,7 @@
       const typingValues = _.values(this.contactTypingTimers || {});
       const typingMostRecent = _.first(_.sortBy(typingValues, 'timestamp'));
       const typingContact = typingMostRecent
-        ? ConversationController.getOrCreate(typingMostRecent.sender, 'private')
+        ? ConversationController.get(typingMostRecent.senderId)
         : null;
 
       const timestamp = this.get('timestamp');
@@ -491,7 +491,7 @@
 
     updateE164(e164) {
       const oldValue = this.get('e164');
-      if (e164 !== oldValue) {
+      if (e164 && e164 !== oldValue) {
         this.set('e164', e164);
         window.Signal.Data.updateConversation(this.attributes);
         this.trigger('idUpdated', this, 'e164', oldValue);
@@ -499,15 +499,15 @@
     },
     updateUuid(uuid) {
       const oldValue = this.get('uuid');
-      if (uuid !== oldValue) {
+      if (uuid && uuid !== oldValue) {
-        this.set('uuid', uuid);
+        this.set('uuid', uuid.toLowerCase());
         window.Signal.Data.updateConversation(this.attributes);
         this.trigger('idUpdated', this, 'uuid', oldValue);
       }
     },
     updateGroupId(groupId) {
       const oldValue = this.get('groupId');
-      if (groupId !== oldValue) {
+      if (groupId && groupId !== oldValue) {
         this.set('groupId', groupId);
         window.Signal.Data.updateConversation(this.attributes);
         this.trigger('idUpdated', this, 'groupId', oldValue);
@@ -1716,10 +1716,11 @@
       // START: this code has an Expiration date of ~2018/11/21
       // We don't want to enable unidentified delivery for send unless it is
       //   also enabled for our own account.
-      const me = ConversationController.getOrCreate(
+      const myId = ConversationController.ensureContactIds({
-        this.ourNumber || this.ourUuid,
+        e164: this.ourNumber,
-        'private'
+        uuid: this.ourUuid,
-      );
+      });
+      const me = ConversationController.get(myId);
       if (
         !disableMeCheck &&
         me.get('sealedSender') === SEALED_SENDER.DISABLED
@@ -2262,7 +2263,7 @@
         );
       }
 
-      const c = await ConversationController.getOrCreateAndWait(id, 'private');
+      const c = ConversationController.getOrCreate(id, 'private');
       const {
         generateProfileKeyCredentialRequest,
         getClientZkProfileOperations,
@@ -2613,8 +2614,8 @@
         return Promise.resolve();
       }
       const members = this.get('members') || [];
-      const promises = members.map(number =>
+      const promises = members.map(identifier =>
-        ConversationController.getOrCreateAndWait(number, 'private')
+        ConversationController.getOrCreateAndWait(identifier, 'private')
       );
 
       return Promise.all(promises).then(contacts => {
@@ -2807,10 +2808,17 @@
     },
 
     notifyTyping(options = {}) {
-      const { isTyping, sender, senderUuid, senderDevice } = options;
+      const {
+        isTyping,
+        sender,
+        senderUuid,
+        senderId,
+        isMe,
+        senderDevice,
+      } = options;
 
       // We don't do anything with typing messages from our other devices
-      if (sender === this.ourNumber || senderUuid === this.ourUuid) {
+      if (isMe) {
         return;
       }
 
@@ -2829,6 +2837,8 @@
         ] || {
           timestamp: Date.now(),
           sender,
+          senderId,
+          senderUuid,
           senderDevice,
         };
 

js/models/messages.js

@@ -1228,10 +1228,11 @@
         return null;
       }
 
-      return ConversationController.getOrCreate(
+      const contactId = ConversationController.ensureContactIds({
-        source || sourceUuid,
+        e164: source,
-        'private'
+        uuid: sourceUuid,
-      );
+      });
+      return ConversationController.get(contactId, 'private');
     },
     isOutgoing() {
       return this.get('type') === 'outgoing';
@@ -2562,12 +2563,13 @@
               } else if (conversation.isPrivate()) {
                 conversation.setProfileKey(profileKey);
               } else {
-                ConversationController.getOrCreateAndWait(
+                const localId = ConversationController.ensureContactIds({
-                  source || sourceUuid,
+                  e164: source,
-                  'private'
+                  uuid: sourceUuid,
-                ).then(sender => {
-                  sender.setProfileKey(profileKey);
                 });
+                ConversationController.get(localId, 'private').setProfileKey(
+                  profileKey
+                );
               }
             }
 

js/modules/refresh_sender_certificate.js

@@ -17,12 +17,11 @@ function refreshOurProfile() {
   window.log.info('refreshOurProfile');
   const ourNumber = textsecure.storage.user.getNumber();
   const ourUuid = textsecure.storage.user.getUuid();
-  const conversation = ConversationController.getOrCreate(
+  const ourId = ConversationController.ensureContactIds({
-    // This is explicitly ourNumber first in order to avoid creating new
+    e164: ourNumber,
-    // conversations when an old one exists
+    uuid: ourUuid,
-    ourNumber || ourUuid,
+  });
-    'private'
+  const conversation = ConversationController.get(ourId, 'private');
-  );
   conversation.updateUuid(ourUuid);
   conversation.updateE164(ourNumber);
   conversation.getProfiles();

js/signal_protocol_store.js

@@ -152,10 +152,7 @@
       encodedAddress
     );
     try {
-      const conv = await ConversationController.getOrCreateAndWait(
+      const conv = ConversationController.getOrCreate(identifier, 'private');
-        identifier,
-        'private'
-      );
       return `${conv.get('id')}.${deviceId}`;
     } catch (e) {
       window.log.error(
@@ -585,7 +582,9 @@
 
       const identifier = textsecure.utils.unencodeNumber(encodedAddress)[0];
       const identityRecord = this.getIdentityRecord(identifier);
-      const id = ConversationController.getConversationId(identifier);
+      const id = ConversationController.getOrCreate(identifier, 'private').get(
+        'id'
+      );
 
       if (!identityRecord || !identityRecord.publicKey) {
         // Lookup failed, or the current key was removed, so save this one.
@@ -661,10 +660,7 @@
 
       const identifier = textsecure.utils.unencodeNumber(encodedAddress)[0];
       const identityRecord = this.getIdentityRecord(identifier);
-      const conv = await ConversationController.getOrCreateAndWait(
+      const conv = ConversationController.getOrCreate(identifier, 'private');
-        identifier,
-        'private'
-      );
       const id = conv.get('id');
 
       const updates = {

ts/textsecure/AccountManager.ts

@@ -693,19 +693,14 @@ export default class AccountManager extends EventTarget {
   async registrationDone({ uuid, number }: { uuid?: string; number?: string }) {
     window.log.info('registration done');
 
-    const identifier = number || uuid;
+    const conversationId = window.ConversationController.ensureContactIds({
-    if (!identifier) {
+      e164: number,
-      throw new Error('registrationDone: no identifier!');
+      uuid,
+    });
+    if (!conversationId) {
+      throw new Error('registrationDone: no conversationId!');
     }
 
-    // Ensure that we always have a conversation for ourself
-    const conversation = await window.ConversationController.getOrCreateAndWait(
-      identifier,
-      'private'
-    );
-    conversation.updateE164(number);
-    conversation.updateUuid(uuid);
-
     window.log.info('dispatching registration event');
 
     this.dispatchEvent(new Event('registration'));

ts/textsecure/MessageReceiver.ts

@@ -1535,6 +1535,11 @@ class MessageReceiverInner extends EventTarget {
       while (contactDetails !== undefined) {
         const contactEvent = new Event('contact');
         contactEvent.contactDetails = contactDetails;
+        window.normalizeUuids(
+          contactEvent,
+          ['contactDetails.verified.destinationUuid'],
+          'message_receiver::handleContacts::handleAttachment'
+        );
         results.push(this.dispatchAndWait(contactEvent));
 
         contactDetails = contactBuffer.next();

ts/util/lint/exceptions.json

@@ -164,7 +164,7 @@
     "rule": "jQuery-load(",
     "path": "js/conversation_controller.js",
     "line": "    async load() {",
-    "lineNumber": 252,
+    "lineNumber": 306,
     "reasonCategory": "falseMatch",
     "updated": "2020-06-19T18:29:40.067Z"
   },
@@ -172,7 +172,7 @@
     "rule": "jQuery-load(",
     "path": "js/conversation_controller.js",
     "line": "      this._initialPromise = load();",
-    "lineNumber": 294,
+    "lineNumber": 348,
     "reasonCategory": "falseMatch",
     "updated": "2020-06-19T18:29:40.067Z"
   },
@@ -225,7 +225,7 @@
     "line": "      await wrap(",
     "lineNumber": 641,
     "reasonCategory": "falseMatch",
-    "updated": "2020-05-27T21:15:43.044Z"
+    "updated": "2020-06-09T20:26:46.515Z"
   },
   {
     "rule": "jQuery-append(",
@@ -325,9 +325,9 @@
     "rule": "jQuery-load(",
     "path": "js/signal_protocol_store.js",
     "line": "      await ConversationController.load();",
-    "lineNumber": 985,
+    "lineNumber": 981,
     "reasonCategory": "falseMatch",
-    "updated": "2020-04-27T20:02:20.424Z"
+    "updated": "2020-06-12T14:20:09.936Z"
   },
   {
     "rule": "DOM-innerHTML",
@@ -11878,4 +11878,4 @@
     "reasonCategory": "falseMatch",
     "updated": "2020-04-05T23:45:16.746Z"
   }
-]
+]

ts/window.d.ts

@@ -96,7 +96,12 @@ export type ConversationControllerType = {
     identifier: string,
     type: 'private' | 'group'
   ) => Promise<ConversationType>;
+  getOrCreate: (
+    identifier: string,
+    type: 'private' | 'group'
+  ) => ConversationType;
   getConversationId: (identifier: string) => string | null;
+  ensureContactIds: (o: { e164?: string; uuid?: string }) => string;
   prepareForSend: (
     id: string,
     options: Object