issue #1144 fix decrypt error rendering (#1219)

* Draft unit test for integrity check * updated tests * fix rendering blockParseErr * fix no-mdc error * fix attachment with mismatch key * simplify * rename test * fix * PR comments Co-authored-by: tomholub <[email protected]> Co-authored-by: Roma Sosnovsky <[email protected]>
FlowCrypt · Dec 21, 2021 · 6a9a87e · 6a9a87e
1 parent 4087b48
commit 6a9a87e
Show file tree

Hide file tree

Showing 19 changed files with 11,039 additions and 425 deletions.
diff --git a/Core/package-lock.json b/Core/package-lock.json
diff --git a/Core/package.json b/Core/package.json
@@ -37,8 +37,8 @@
     "_updateCoreLib": "mkdir source/lib && cd source/lib && LIBD='../../../flowcrypt-browser/extension/lib' && cp -r $LIBD/emailjs . && cp $LIBD/openpgp.js . && cp $LIBD/iso-*.js . && cp $LIBD/zxcvbn.js .",
     "donateCore": "rm -rf ../flowcrypt-browser/extension/js/common/core && cp -r source/core ../flowcrypt-browser/extension/js/common/core"
   },
-  "author": "FlowCrypt a.s.",
+  "author": "FlowCrypt a. s.",
   "license": "SEE LICENSE IN <LICENSE>",
   "private": true,
   "homepage": "https://flowcrypt.com"
-}
+}
diff --git a/Core/source/assets/compat/direct-encrypted-key-mismatch-file.txt b/Core/source/assets/compat/direct-encrypted-key-mismatch-file.txt
diff --git a/Core/source/assets/compat/mime-email-not-integrity-protected.txt b/Core/source/assets/compat/mime-email-not-integrity-protected.txt
@@ -0,0 +1,54 @@
+Received: from 717284730244
+	named unknown
+	by gmailapi.google.com
+	with HTTPREST;
+	Fri, 8 Jun 2018 04:15:40 -0700
+Content-Type: multipart/mixed;
+ boundary="----sinikael-?=_1-15284565398080.6365395020974245"
+From: [email protected]
+To: [email protected]
+Subject: not integrity protected - should show a warning and not decrypt
+ automatically
+Date: Fri, 8 Jun 2018 04:15:40 -0700
+Message-Id: <CAKbuLTreh7ZN-TP=8-2=ED=AKkgriEi0RLB51i16AcgcC5driQ@mail.gmail.com>
+MIME-Version: 1.0
+
+------sinikael-?=_1-15284565398080.6365395020974245
+Content-Type: text/plain
+Content-Transfer-Encoding: quoted-printable
+
+-----BEGIN PGP MESSAGE-----
+Version: FlowCrypt 5.5.9 Gmail Encryption flowcrypt.com
+Comment: Seamlessly send, receive and search encrypted email
+
+wcFMA0taL/zmLZUBAQ/7Bwida5vvhXv5Zi+qJbG/QPst11jWfljDQlw1VLzF
+ou8ofoIEHpvoFgXegZUnoQXBmlHGD+XLs9jG/TV1mtE2RWq4hDtqiTQ6rEIa
+brN3Nx77Yr+4EN1aKI20aTLEPTIjVU2GH2i9DAmjHteBU3nkL9Z3yecB8Pn8
+EdhpCRY6cj2yrhJ5MPwmXrus9OFv39wA2DqYpqW5Be+KD8mipZ2CtJo5xtin
+aeEhpWSDsdg26rjx1nz4dA0NcFzZK2p/BPfPIFzRvmoXoWFigpUnwryEoCqX
+/tgmcrv7PqiYT5oziPmMuBc1lb7icI/Aq69uXz2z6+4MJHOlcTEFygV36J+1
+1opcjoX+JKJNn1nvHovBxuemcMwriJdmDj4Hmfo4zkd6ryUtGVrMVn8DbRp6
+TWB/0MSE8cmfuiA5DgzdGbrevdL6RxnQDmalTHJ5oxurFQVoLwpmbgd36C4Q
+xMfG1xEqFn5zvrCTGHg2OfS2cynal8CQDG0ZQCoWwdb0kT5D6bx7QKcuyy1/
+1TXKnp1NamD5Uhu1+XuxD7EbvDYUWYh3bkqgslsoX+OUl+ONdtMD5PswArd5
+KisD9UJuddJShL4clBUPoXeNrRxrU6HqjP5T4fapK684MeizicHIRpAww7fu
+Z8YtaySZ/hoOAKWsx0rV4grgJV7pryj4ARBRa1pLL9rBwUwDS1ov/OYtlQEB
+D/47fyD/6BvepqWmZXj7VLl2y63eE0b/6hf5K+Izv5A/+5l/EnjFx0rq+qeX
+6hftYZBUAbbBvKfxq9D5xsWg3tnhFv2sYIE3YpkCSzZpWJmahHwQOVNT0ASw
+gbO25OiTPlYPqfSkGYe0palbL+4T5dLOwVilmrZ2bQf/rLePwA4RQpWDPYio
+NDU0Xfi7TQcHQrZTpwFbVzNPXgCHnQkqF+s0v8RDJHnt9vVs2KEpi49V/YgN
++gZnZOeADL0rbre/PrIck1YSjZLbrWtQVk4+sCf0TjvixJ7MNjA4NgdZPo0M
+Hke/9XBFie3NiZaW/cEIVZ7WnjB3IbhkmOMJd4LgdHKgmswJwCYm+XvpOI19
+FzU1vzZmfOA1nEJSuuCDNVUoKYIQA5UEYJrVJeGnVN5sU5jkdlX9xPtYceww
+YFmLisuf9Ev0HC7v27KwYQRDPNYRA8GeK/jY6aZdg+VccsnzEigdYL5Tm4JI
+Zrxp/G807bZvt0yZwWh0gpWOFgbVgrm4Hpji5ilDyulZSW+8nJxB5tDoPzL4
+j4w9malje0c60GWNtiyCPLURyN63C2q144UpQjSU5r66oP1yF2A97aXKbf4p
+qO7cSNWEOTpqJkJrNFVKQdWvXZ+mvW1PQFmkkwish2HiQIXmWb04uV1pI8hR
+6YWk2ox9aZiJ664MpncgyJ5uIMlzVfYrX+AZRtBW36RgCTprIv6l1M5NcHMy
+zEscTaSY/e+pM5HzQKSzX+zHLa5kk5L7veX+1G33saiqSJ/fK13+k7qDNZQD
+nbtaebfh2JS0Pdbub6FUFjPHR5PydU9ltuppGEeYrOe1SxwiZ6BZfIXO2/8M
+hA=3D=3D
+=3DB/NE
+-----END PGP MESSAGE-----
+
+------sinikael-?=_1-15284565398080.6365395020974245--
diff --git a/Core/source/core/pgp-msg.ts b/Core/source/core/pgp-msg.ts
@@ -211,7 +211,7 @@ export class PgpMsg {
       const content = new Buf(await openpgp.stream.readToEnd(decrypted.getLiteralData()!)); // read content second to prevent stream hang
       const signature = verifyResults ? await PgpMsg.verify(verifyResults, []) : undefined; // evaluate verify results third to prevent stream hang
       if (!prepared.isCleartext && (prepared.message as OpenPGP.message.Message).packets.filterByTag(openpgp.enums.packet.symmetricallyEncrypted).length) {
-        const noMdc = 'Security threat!\n\nMessage is missing integrity checks (MDC). The sender should update their outdated software.\n\nDisplay the message at your own risk.';
+        const noMdc = 'Security threat!\n\nMessage is missing integrity checks (MDC). The sender should update their outdated software and resend.';
         return { success: false, content, error: { type: DecryptErrTypes.noMdc, message: noMdc }, message: prepared.message, longids, isEncrypted };
       }
       return { success: true, content, isEncrypted, filename: decrypted.getFilename() || undefined, signature };

diff --git a/Core/source/mobile-interface/endpoints.ts b/Core/source/mobile-interface/endpoints.ts
@@ -118,7 +118,7 @@ export class Endpoints {
           decryptRes.message = undefined;
           sequentialProcessedBlocks.push({
             type: 'decryptErr',
-            content: decryptRes.error.type === DecryptErrTypes.noMdc ? decryptRes.content! : rawBlock.content,
+            content: decryptRes.error.type === DecryptErrTypes.noMdc ? decryptRes.content!.toUtfStr() : rawBlock.content.toString(),
             decryptErr: decryptRes,
             complete: true
           });
@@ -150,6 +150,10 @@ export class Endpoints {
         // no longer used below, only gets passed to be serialized as JSON - later consumed by iOS or Android app
         block.attMeta.data = Buf.fromUint8(block.attMeta.data).toBase64Str() as any as Uint8Array;
       }
+      if (block.decryptErr?.content instanceof Buf) {
+        // cannot pass a Buf using a json, converting to String before it gets serialized
+        block.decryptErr.content = block.decryptErr.content.toUtfStr() as any as Buf;
+      }
       if (block.type === 'decryptedHtml' || block.type === 'decryptedText' || block.type === 'decryptedAtt') {
         replyType = 'encrypted';
       }
@@ -167,7 +171,7 @@ export class Endpoints {
               complete: true,
               decryptErr: {
                 success: false,
-                error: { type: 'format' as DecryptErrTypes, message: 'Badly formatted public key' },
+                error: { type: DecryptErrTypes.format, message: 'Badly formatted public key' },
                 longids: { message: [], matching: [], chosen: [], needPassphrase: [] }
               }
             });
@@ -191,20 +195,21 @@ export class Endpoints {
 
   public decryptFile = async (uncheckedReq: any, data: Buffers, verificationPubkeys?: string[]): Promise<EndpointRes> => {
     const { keys: kisWithPp, msgPwd } = ValidateInput.decryptFile(uncheckedReq);
-    const decryptedMeta = await PgpMsg.decrypt({ kisWithPp, encryptedData: Buf.concat(data), msgPwd, verificationPubkeys });
-    if (!decryptedMeta.success) {
-      decryptedMeta.message = undefined;
-      return fmtRes(decryptedMeta);
+    const decryptRes = await PgpMsg.decrypt({ kisWithPp, encryptedData: Buf.concat(data), msgPwd, verificationPubkeys });
+    if (!decryptRes.success) {
+      decryptRes.message = undefined;
+      decryptRes.content = undefined;
+      return fmtRes({ decryptErr: decryptRes });
     }
-    return fmtRes({ success: true, name: decryptedMeta.filename || '' }, decryptedMeta.content);
+    return fmtRes({ decryptSuccess: { name: decryptRes.filename || '' } }, decryptRes.content);
   }
 
   public parseDateStr = async (uncheckedReq: any): Promise<EndpointRes> => {
     const { dateStr } = ValidateInput.parseDateStr(uncheckedReq);
     return fmtRes({ timestamp: String(Date.parse(dateStr) || -1) });
   }
 
-  public zxcvbnStrengthBar = async (uncheckedReq: any) : Promise<EndpointRes> => {
+  public zxcvbnStrengthBar = async (uncheckedReq: any): Promise<EndpointRes> => {
     const r = ValidateInput.zxcvbnStrengthBar(uncheckedReq);
     if (r.purpose === 'passphrase') {
       if (typeof r.guesses === 'number') { // the host has a port of zxcvbn and already knows amount of guesses per password
@@ -224,7 +229,7 @@ export class Endpoints {
     }
   }
 
-  public gmailBackupSearch = async (uncheckedReq: any) : Promise<EndpointRes> => {
+  public gmailBackupSearch = async (uncheckedReq: any): Promise<EndpointRes> => {
     const { acctEmail } = ValidateInput.gmailBackupSearch(uncheckedReq);
     return fmtRes({ query: gmailBackupSearchQuery(acctEmail) });
   }
@@ -289,8 +294,7 @@ export class Endpoints {
 }
 
 export const getSigningPrv = async (req: NodeRequest.composeEmailEncrypted): Promise<OpenPGP.key.Key | undefined> => {
-  if (!req.signingPrv)
-  {
+  if (!req.signingPrv) {
     return undefined;
   }
   const key = await readArmoredKeyOrThrow(req.signingPrv.private);

diff --git a/Core/source/test.ts b/Core/source/test.ts
@@ -103,7 +103,7 @@ orig message
 
 ava.default('composeEmail format:plain with attachment', async t => {
   const content = 'hello\nwrld';
-  const req = { format: 'plain', text: content, to: ['[email protected]'], cc: ['[email protected]'], bcc: [], from: '[email protected]', subject: 'a subj', atts: [{name: 'sometext.txt', type: 'text/plain', base64: Buffer.from('hello, world!!!').toString('base64')}] };
+  const req = { format: 'plain', text: content, to: ['[email protected]'], cc: ['[email protected]'], bcc: [], from: '[email protected]', subject: 'a subj', atts: [{ name: 'sometext.txt', type: 'text/plain', base64: Buffer.from('hello, world!!!').toString('base64') }] };
   const { data: plainMimeMsg, json: composeEmailJson } = parseResponse(await endpoints.composeEmail(req));
   expectEmptyJson(composeEmailJson);
   const plainMimeStr = plainMimeMsg.toString();
@@ -267,7 +267,7 @@ ava.default('composeEmail format:encrypt-inline -> parseDecryptMsg', async t =>
 ava.default('composeEmail format:encrypt-inline with attachment', async t => {
   const content = 'hello\nwrld';
   const { pubKeys } = getKeypairs('rsa1');
-  const req = { pubKeys, format: 'encrypt-inline', text: content, to: ['[email protected]'], cc: [], bcc: [], from: '[email protected]', subject: 'encr subj', atts: [{name: 'topsecret.txt', type: 'text/plain', base64: Buffer.from('hello, world!!!').toString('base64') }] };
+  const req = { pubKeys, format: 'encrypt-inline', text: content, to: ['[email protected]'], cc: [], bcc: [], from: '[email protected]', subject: 'encr subj', atts: [{ name: 'topsecret.txt', type: 'text/plain', base64: Buffer.from('hello, world!!!').toString('base64') }] };
   const { data: encryptedMimeMsg, json: encryptJson } = parseResponse(await endpoints.composeEmail(req));
   expectEmptyJson(encryptJson);
   const encryptedMimeStr = encryptedMimeMsg.toString();
@@ -287,7 +287,7 @@ for (const keypairName of allKeypairNames.filter(name => name != 'expired' && na
     expectEmptyJson(encryptJson);
     expectData(encryptedFile);
     const { data: decryptedContent, json: decryptJson } = parseResponse(await endpoints.decryptFile({ keys }, [encryptedFile]));
-    expect(decryptJson).to.deep.equal({ success: true, name });
+    expect(decryptJson).to.deep.equal({ decryptSuccess: { name } });
     expectData(decryptedContent, 'binary', content);
     t.pass();
   });
@@ -538,7 +538,7 @@ ava.default('parseDecryptMsg compat mime-email-encrypted-inline-text-2 Mime-Text
   t.pass();
 });
 
-ava.default('parseDecryptMsg - decryptErr', async t => {
+ava.default('parseDecryptMsg - decryptErr wrong key when dencrypting content', async t => {
   const { keys } = getKeypairs('rsa2'); // intentional key mismatch
   const { data: blocks, json: decryptJson } = parseResponse(await endpoints.parseDecryptMsg({ keys }, [await getCompatAsset('direct-encrypted-text')]));
   expectData(blocks, 'msgBlocks', [{
@@ -564,6 +564,28 @@ ava.default('parseDecryptMsg - decryptErr', async t => {
   t.pass();
 });
 
+ava.default('decryptFile - decryptErr wrong key when decrypting attachment', async t => {
+  const jsonReq = { keys: getKeypairs('rsa2').keys }; // intentional key mismatch
+  const { json: decryptJson } = parseResponse(await endpoints.decryptFile(jsonReq, [await getCompatAsset('direct-encrypted-key-mismatch-file')]));
+  expect(decryptJson).to.deep.equal({
+    "decryptErr": {
+      "success": false,
+      "error": {
+        "type": "key_mismatch",
+        "message": "Missing appropriate key"
+      },
+      "longids": {
+        "message": ["305F81A9AED12035"],
+        "matching": [],
+        "chosen": [],
+        "needPassphrase": []
+      },
+      "isEncrypted": true
+    }
+  });
+  t.pass();
+});
+
 ava.default('parseDecryptMsg compat mime-email-plain-html', async t => {
   const { keys } = getKeypairs('rsa1');
   const { data: blocks, json: decryptJson } = parseResponse(await endpoints.parseDecryptMsg({ keys, isEmail: true }, [await getCompatAsset('mime-email-plain-html')]));
@@ -730,3 +752,13 @@ ava.default('verify signed message with detached signature by providing it corre
   expect(parsedDecryptData.verifyRes.match).equals(true);
   t.pass();
 });
+
+ava.default('decryptErr for not integrity protected message', async t => {
+  const { keys, pubKeys } = getKeypairs('flowcrypt.compatibility');
+  const { json: decryptJson, data: decryptData } = parseResponse(await endpoints.parseDecryptMsg({ keys, isEmail: true, verificationPubkeys: pubKeys }, [await getCompatAsset('mime-email-not-integrity-protected')]));
+  expect(decryptJson.replyType).equals('plain');
+  expect(decryptJson.subject).equals('not integrity protected - should show a warning and not decrypt automatically');
+  const blocks = decryptData.toString().split('\n').map(block => JSON.parse(block));
+  expect(blocks[1].decryptErr.error.type).equals('no_mdc');
+  t.pass();
+});