Merge pull request #1603 from Bilb/fix-attachment-padding

pad Message buffer for all outgoing messages (even opengroupv2)

app/tray_icon.js

@@ -9,7 +9,13 @@ let tray = null;
 function createTrayIcon(getMainWindow, messages) {
   // A smaller icon is needed on macOS
   const iconSize = process.platform === 'darwin' ? '16' : '256';
-  const iconNoNewMessages = path.join(__dirname, '..', 'images', 'session', `session_icon_${iconSize}.png`);
+  const iconNoNewMessages = path.join(
+    __dirname,
+    '..',
+    'images',
+    'session',
+    `session_icon_${iconSize}.png`
+  );
 
   tray = new Tray(iconNoNewMessages);
 

ts/components/session/SessionPasswordModal.tsx

@@ -208,9 +208,7 @@ class SessionPasswordModalInner extends React.Component<Props, State> {
       return;
     }
 
-    const isValidWithStoredInDB = Boolean(
+    const isValidWithStoredInDB = Boolean(await this.validatePasswordHash(oldPassword));
-      await this.validatePasswordHash(oldPassword)
-    );
     if (!isValidWithStoredInDB) {
       this.setState({
         error: window.i18n('changePasswordInvalid'),

ts/receiver/attachments.ts

@@ -11,6 +11,7 @@ import {
 } from '../opengroup/opengroupV2/OpenGroupAPIV2';
 import { OpenGroupRequestCommonType } from '../opengroup/opengroupV2/ApiUtil';
 import { FSv2 } from '../fileserver';
+import { getUnpaddedAttachment } from '../session/crypto/BufferPadding';
 
 export async function downloadAttachment(attachment: any) {
   const serverUrl = new URL(attachment.url).origin;
@@ -69,14 +70,13 @@ export async function downloadAttachment(attachment: any) {
     if (!size || size !== data.byteLength) {
       // we might have padding, check that all the remaining bytes are padding bytes
       // otherwise we have an error.
-      if (AttachmentUtils.isLeftOfBufferPaddingOnly(data, size)) {
+      const unpaddedData = getUnpaddedAttachment(data, size);
-        // we can safely remove the padding
+      if (!unpaddedData) {
-        data = data.slice(0, size);
-      } else {
         throw new Error(
           `downloadAttachment: Size ${size} did not match downloaded attachment size ${data.byteLength}`
         );
       }
+      data = unpaddedData;
     }
   }
 
@@ -120,17 +120,17 @@ export async function downloadAttachmentOpenGroupV2(
   if (attachment.size !== dataUint.length) {
     // we might have padding, check that all the remaining bytes are padding bytes
     // otherwise we have an error.
-    if (AttachmentUtils.isLeftOfBufferPaddingOnly(dataUint.buffer, attachment.size)) {
+    const unpaddedData = getUnpaddedAttachment(dataUint.buffer, attachment.size);
-      // we can safely remove the padding
+    if (!unpaddedData) {
-      data = data.slice(0, attachment.size);
-    } else {
       throw new Error(
         `downloadAttachment: Size ${attachment.size} did not match downloaded attachment size ${data.byteLength}`
       );
     }
+    data = new Uint8Array(unpaddedData);
   } else {
-    // nothing to do, the attachment has already the correct size. There is just no padding included, which is bas
+    // nothing to do, the attachment has already the correct size.
-    window.log.warn('Received opengroupv2 unpadded attachment');
+    // There is just no padding included, which is what we agreed on
+    window.log.info('Received opengroupv2 unpadded attachment');
   }
 
   return {

ts/receiver/contentMessage.ts

@@ -17,6 +17,7 @@ import { KeyPairRequestManager } from './keyPairRequestManager';
 import { requestEncryptionKeyPair } from '../session/group';
 import { handleConfigurationMessage } from './configMessage';
 import { ConversationTypeEnum } from '../models/conversation';
+import { removeMessagePadding } from '../session/crypto/BufferPadding';
 
 export async function handleContentMessage(envelope: EnvelopePlus) {
   try {
@@ -94,7 +95,7 @@ async function decryptForClosedGroup(envelope: EnvelopePlus, ciphertext: ArrayBu
     }
     window.log.info('ClosedGroup Message decrypted successfully with keyIndex:', keyIndex);
 
-    return unpad(decryptedContent);
+    return removeMessagePadding(decryptedContent);
   } catch (e) {
     /**
      * If an error happened during the decoding,
@@ -130,7 +131,7 @@ async function decryptForClosedGroup(envelope: EnvelopePlus, ciphertext: ArrayBu
  * or a message sent to a closed group.
  *
  * We do not unpad the result here, as in the case of the keypair wrapper, there is not padding.
- * Instead, it is the called who needs to unpad() the content.
+ * Instead, it is the called who needs to removeMessagePadding() the content.
  */
 export async function decryptWithSessionProtocol(
   envelope: EnvelopePlus,
@@ -191,22 +192,6 @@ export async function decryptWithSessionProtocol(
   return plaintext;
 }
 
-export function unpad(paddedData: ArrayBuffer): ArrayBuffer {
-  const paddedPlaintext = new Uint8Array(paddedData);
-
-  for (let i = paddedPlaintext.length - 1; i >= 0; i -= 1) {
-    if (paddedPlaintext[i] === 0x80) {
-      const plaintext = new Uint8Array(i);
-      plaintext.set(paddedPlaintext.subarray(0, i));
-      return plaintext.buffer;
-    } else if (paddedPlaintext[i] !== 0x00) {
-      throw new Error('Invalid padding');
-    }
-  }
-
-  throw new Error('Invalid padding');
-}
-
 export async function isBlocked(number: string) {
   return BlockedNumberController.isBlockedAsync(number);
 }
@@ -227,7 +212,7 @@ async function decryptUnidentifiedSender(
     );
     // keep the await so the try catch works as expected
     const retSessionProtocol = await decryptWithSessionProtocol(envelope, ciphertext, ecKeyPair);
-    return unpad(retSessionProtocol);
+    return removeMessagePadding(retSessionProtocol);
   } catch (e) {
     window.log.warn('decryptWithSessionProtocol for unidentified message throw:', e);
     return null;

ts/receiver/receiver.ts

@@ -9,7 +9,7 @@ import { processMessage } from '../session/snode_api/swarmPolling';
 import { onError } from './errors';
 
 // innerHandleContentMessage is only needed because of code duplication in handleDecryptedEnvelope...
-import { handleContentMessage, innerHandleContentMessage, unpad } from './contentMessage';
+import { handleContentMessage, innerHandleContentMessage } from './contentMessage';
 import _, { noop } from 'lodash';
 
 export { processMessage };
@@ -36,6 +36,7 @@ import { OpenGroupMessageV2 } from '../opengroup/opengroupV2/OpenGroupMessageV2'
 import { OpenGroupRequestCommonType } from '../opengroup/opengroupV2/ApiUtil';
 import { handleMessageJob } from './queuedJob';
 import { fromBase64ToArray } from '../session/utils/String';
+import { removeMessagePadding } from '../session/crypto/BufferPadding';
 
 // TODO: check if some of these exports no longer needed
 
@@ -315,7 +316,8 @@ export async function handleOpenGroupV2Message(
     return;
   }
 
-  const dataUint = new Uint8Array(unpad(fromBase64ToArray(base64EncodedData)));
+  // Note: opengroup messages are not padded
+  const dataUint = new Uint8Array(removeMessagePadding(fromBase64ToArray(base64EncodedData)));
 
   const decoded = SignalService.Content.decode(dataUint);
 

ts/session/crypto/BufferPadding.ts

@@ -0,0 +1,92 @@
+/**
+ * This file is used to pad message buffer and attachments
+ */
+const PADDING_BYTE = 0x00;
+
+/**
+ * Unpad the buffer from its padding.
+ * An error is thrown if there is no padding.
+ * A padded buffer is
+ *  * whatever at start
+ *  * ends with 0x80 and any number of 0x00 until the end
+ */
+export function removeMessagePadding(paddedData: ArrayBuffer): ArrayBuffer {
+  const paddedPlaintext = new Uint8Array(paddedData);
+  window.log.info('Removing message padding...');
+  for (let i = paddedPlaintext.length - 1; i >= 0; i -= 1) {
+    if (paddedPlaintext[i] === 0x80) {
+      const plaintext = new Uint8Array(i);
+      plaintext.set(paddedPlaintext.subarray(0, i));
+      return plaintext.buffer;
+    } else if (paddedPlaintext[i] !== PADDING_BYTE) {
+      throw new Error('Invalid padding');
+    }
+  }
+
+  throw new Error('Invalid padding');
+}
+
+/**
+ * Add padding to a message buffer
+ * @param messageBuffer The buffer to add padding to.
+ */
+export function addMessagePadding(messageBuffer: Uint8Array): Uint8Array {
+  window.log?.info('Adding message padding...');
+
+  const plaintext = new Uint8Array(getPaddedMessageLength(messageBuffer.byteLength + 1) - 1);
+  plaintext.set(new Uint8Array(messageBuffer));
+  plaintext[messageBuffer.byteLength] = 0x80;
+
+  return plaintext;
+}
+
+function getPaddedMessageLength(originalLength: number): number {
+  const messageLengthWithTerminator = originalLength + 1;
+  let messagePartCount = Math.floor(messageLengthWithTerminator / 160);
+
+  if (messageLengthWithTerminator % 160 !== 0) {
+    messagePartCount += 1;
+  }
+
+  return messagePartCount * 160;
+}
+
+/*
+ * If the attachment has padding, remove the padding and return the unpad attachment
+ */
+export function getUnpaddedAttachment(
+  data: ArrayBuffer,
+  unpaddedExpectedSize: number
+): ArrayBuffer | null {
+  window.log?.info('Removing attachment padding...');
+
+  // to have a padding we must have a strictly longer length expected
+  if (data.byteLength <= unpaddedExpectedSize) {
+    return null;
+  }
+  const dataUint = new Uint8Array(data);
+  for (let i = unpaddedExpectedSize; i < data.byteLength; i++) {
+    if (dataUint[i] !== PADDING_BYTE) {
+      return null;
+    }
+  }
+
+  return data.slice(0, unpaddedExpectedSize);
+}
+
+export function addAttachmentPadding(data: ArrayBuffer): ArrayBuffer {
+  const originalUInt = new Uint8Array(data);
+  window.log.info('Adding attchment padding...');
+
+  const paddedSize = Math.max(
+    541,
+    Math.floor(Math.pow(1.05, Math.ceil(Math.log(originalUInt.length) / Math.log(1.05))))
+  );
+  const paddedData = new ArrayBuffer(paddedSize);
+  const paddedUInt = new Uint8Array(paddedData);
+
+  paddedUInt.fill(PADDING_BYTE, originalUInt.length);
+  paddedUInt.set(originalUInt);
+
+  return paddedUInt.buffer;
+}

ts/session/crypto/MessageEncrypter.ts

@@ -6,29 +6,7 @@ import { fromHexToArray } from '../utils/String';
 export { concatUInt8Array, getSodium };
 import { getLatestClosedGroupEncryptionKeyPair } from '../../../ts/data/data';
 import { UserUtils } from '../utils';
-
+import { addMessagePadding } from './BufferPadding';
-/**
- * Add padding to a message buffer
- * @param messageBuffer The buffer to add padding to.
- */
-export function padPlainTextBuffer(messageBuffer: Uint8Array): Uint8Array {
-  const plaintext = new Uint8Array(getPaddedMessageLength(messageBuffer.byteLength + 1) - 1);
-  plaintext.set(new Uint8Array(messageBuffer));
-  plaintext[messageBuffer.byteLength] = 0x80;
-
-  return plaintext;
-}
-
-function getPaddedMessageLength(originalLength: number): number {
-  const messageLengthWithTerminator = originalLength + 1;
-  let messagePartCount = Math.floor(messageLengthWithTerminator / 160);
-
-  if (messageLengthWithTerminator % 160 !== 0) {
-    messagePartCount += 1;
-  }
-
-  return messagePartCount * 160;
-}
 
 type EncryptResult = {
   envelopeType: SignalService.Envelope.Type;
@@ -53,7 +31,7 @@ export async function encrypt(
     throw new Error(`Invalid encryption type:${encryptionType}`);
   }
   const encryptForClosedGroup = encryptionType === EncryptionType.ClosedGroup;
-  const plainText = padPlainTextBuffer(plainTextBuffer);
+  const plainText = addMessagePadding(plainTextBuffer);
 
   if (encryptForClosedGroup) {
     window?.log?.info(

ts/session/sending/MessageSender.ts

@@ -10,10 +10,10 @@ import { UserUtils } from '../utils';
 import { OpenGroupRequestCommonType } from '../../opengroup/opengroupV2/ApiUtil';
 import { postMessage } from '../../opengroup/opengroupV2/OpenGroupAPIV2';
 import { OpenGroupMessageV2 } from '../../opengroup/opengroupV2/OpenGroupMessageV2';
-import { padPlainTextBuffer } from '../crypto/MessageEncrypter';
 import { fromUInt8ArrayToBase64 } from '../utils/String';
 import { OpenGroupVisibleMessage } from '../messages/outgoing/visibleMessage/OpenGroupVisibleMessage';
 import * as LokiMessageApi from './LokiMessageApi';
+import { addMessagePadding } from '../crypto/BufferPadding';
 
 // ================ Regular ================
 
@@ -136,7 +136,8 @@ export async function sendToOpenGroupV2(
   rawMessage: OpenGroupVisibleMessage,
   roomInfos: OpenGroupRequestCommonType
 ): Promise<OpenGroupMessageV2> {
-  const paddedBody = padPlainTextBuffer(rawMessage.plainTextBuffer());
+  // we agreed to pad message for opengroupv2
+  const paddedBody = addMessagePadding(rawMessage.plainTextBuffer());
   const v2Message = new OpenGroupMessageV2({
     sentTimestamp: Date.now(),
     sender: UserUtils.getOurPubKeyStrFromCache(),

ts/session/utils/Attachments.ts

@@ -9,6 +9,7 @@ import {
 } from '../messages/outgoing/visibleMessage/VisibleMessage';
 import { OpenGroup } from '../../opengroup/opengroupV1/OpenGroup';
 import { FSv2 } from '../../fileserver';
+import { addAttachmentPadding } from '../crypto/BufferPadding';
 
 interface UploadParams {
   attachment: Attachment;
@@ -39,8 +40,6 @@ interface RawQuote {
 
 // tslint:disable-next-line: no-unnecessary-class
 export class AttachmentUtils {
-  public static readonly PADDING_BYTE = 0;
-
   private constructor() {}
 
   public static async uploadV1(params: UploadParams): Promise<AttachmentPointer> {
@@ -74,6 +73,7 @@ export class AttachmentUtils {
 
     let attachmentData: ArrayBuffer;
 
+    // We don't pad attachments for opengroup as they are unencrypted
     if (isRaw || openGroup) {
       attachmentData = attachment.data;
     } else {
@@ -84,7 +84,7 @@ export class AttachmentUtils {
       const dataToEncrypt =
         !shouldPad || !window.lokiFeatureFlags.padOutgoingAttachments
           ? attachment.data
-          : AttachmentUtils.addAttachmentPadding(attachment.data);
+          : addAttachmentPadding(attachment.data);
       const data = await window.textsecure.crypto.encryptAttachment(
         dataToEncrypt,
         pointer.key.buffer,
@@ -195,38 +195,4 @@ export class AttachmentUtils {
       attachments,
     };
   }
-
-  public static isLeftOfBufferPaddingOnly(
-    data: ArrayBuffer,
-    unpaddedExpectedSize: number
-  ): boolean {
-    // to have a padding we must have a strictly longer length expected
-    if (data.byteLength <= unpaddedExpectedSize) {
-      return false;
-    }
-    const dataUint = new Uint8Array(data);
-    for (let i = unpaddedExpectedSize; i < data.byteLength; i++) {
-      if (dataUint[i] !== this.PADDING_BYTE) {
-        return false;
-      }
-    }
-
-    return true;
-  }
-
-  public static addAttachmentPadding(data: ArrayBuffer): ArrayBuffer {
-    const originalUInt = new Uint8Array(data);
-
-    const paddedSize = Math.max(
-      541,
-      Math.floor(Math.pow(1.05, Math.ceil(Math.log(originalUInt.length) / Math.log(1.05))))
-    );
-    const paddedData = new ArrayBuffer(paddedSize);
-    const paddedUInt = new Uint8Array(paddedData);
-
-    paddedUInt.fill(AttachmentUtils.PADDING_BYTE, originalUInt.length);
-    paddedUInt.set(originalUInt);
-
-    return paddedUInt.buffer;
-  }
 }

ts/session/utils/AttachmentsV2.ts

@@ -10,6 +10,7 @@ import {
 } from '../messages/outgoing/visibleMessage/VisibleMessage';
 import { AttachmentUtils } from './Attachments';
 import { uploadFileOpenGroupV2 } from '../../opengroup/opengroupV2/OpenGroupAPIV2';
+import { addAttachmentPadding } from '../crypto/BufferPadding';
 
 interface UploadParamsV2 {
   attachment: Attachment;
@@ -35,8 +36,6 @@ interface RawQuote {
   attachments?: Array<RawQuoteAttachment>;
 }
 
-const PADDING_BYTE = 0;
-
 export async function uploadV2(params: UploadParamsV2): Promise<AttachmentPointer> {
   const { attachment, openGroup } = params;
   if (typeof attachment !== 'object' || attachment == null) {
@@ -57,9 +56,10 @@ export async function uploadV2(params: UploadParamsV2): Promise<AttachmentPointe
     caption: attachment.caption,
   };
 
-  const paddedAttachment: ArrayBuffer = window.lokiFeatureFlags.padOutgoingAttachments
+  const paddedAttachment: ArrayBuffer =
-    ? AttachmentUtils.addAttachmentPadding(attachment.data)
+    window.lokiFeatureFlags.padOutgoingAttachments && !openGroup
-    : attachment.data;
+      ? addAttachmentPadding(attachment.data)
+      : attachment.data;
 
   const fileDetails = await uploadFileOpenGroupV2(new Uint8Array(paddedAttachment), openGroup);
 

ts/test/session/unit/crypto/MessageEncrypter_test.ts

@@ -11,6 +11,7 @@ import { StringUtils, UserUtils } from '../../../../session/utils';
 import chaiBytes from 'chai-bytes';
 import { PubKey } from '../../../../session/types';
 import { fromHex, toHex } from '../../../../session/utils/String';
+import { addMessagePadding } from '../../../../session/crypto/BufferPadding';
 
 chai.use(chaiBytes);
 
@@ -187,7 +188,7 @@ describe('MessageEncrypter', () => {
       const spy = sinon.spy(MessageEncrypter, 'encryptUsingSessionProtocol');
       await MessageEncrypter.encrypt(TestUtils.generateFakePubKey(), data, EncryptionType.Fallback);
       chai.expect(spy.callCount).to.be.equal(1);
-      const paddedData = MessageEncrypter.padPlainTextBuffer(data);
+      const paddedData = addMessagePadding(data);
       const firstArgument = new Uint8Array(spy.args[0][1]);
       chai.expect(firstArgument).to.deep.equal(paddedData);
       spy.restore();