Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

verify signed messages(inband) #1097

Closed
tomholub opened this issue Mar 20, 2021 · 37 comments · Fixed by #1601
Closed

verify signed messages(inband) #1097

tomholub opened this issue Mar 20, 2021 · 37 comments · Fixed by #1601

Comments

@tomholub
Copy link
Collaborator

tomholub commented Mar 20, 2021

When we receive a signed message, currently we only render it in gray border. We should also parse out information from the message about which key signed it, and do signature verification.

Before loading the message, we'd pull public keys from local contacts that are recorded for the email that we received the message from. Then offer these to the verification method as possible public keys it was signed with. If none of them match the signed message, we'd show signature not verified.

See FlowCrypt/flowcrypt-ios#278 (comment) and further discussion below for what to render in what situation (not verified, valid, invalid)

@tomholub
Copy link
Collaborator Author

tomholub commented Oct 5, 2021

Here, before loading the message, we'd pull public keys from local contacts that are recorded for the email that we received the message from. Then offer these to the verification method as possible public keys it was signed with. If none of them match the signed message, we'd show "could not verify message".

This should be done after issues in https://github.com/FlowCrypt/flowcrypt-android/milestone/90 so that we can pull more than one public key per email.

@tomholub
Copy link
Collaborator Author

Here a code snippet from iOS, regarding rendering various signature statuses

  private func renderPgpSignatureCheckResult(
    signature: VerifyRes?, 
    sender: RecipientWithOrderedPublicKeys
  ) -> ProcessedMessage.MessageSignature {
    if signature is nil then return .unsigned // render "not signed" in red
    if signature.error is not nil then return .error(signature.error) // render the "cannot verify signature: \(error)"
    if signature.signer is nil then return .unsigned // render "not signed" in red
    let signingPubKey = sender.pubkeys.find { $0.longids.contains(signature.signer) }
    if signingPubKey is nil or signature.match is nil then return .missingPubkey(longid: signature.signer) // render "cannot verify signature: no Public Key $longid" in red
    if signature.match is false then return .bad // render "bad signature" in red
    return .good
  }

@tomholub
Copy link
Collaborator Author

tomholub commented Nov 13, 2021

Final flow:

  1. user taps a message
  2. message gets downloaded from Gmail API
  3. message gets processed / decrypted / verified with whatever public keys we have locally - for that sender email. Very important: we use the from: email address - the sender email address - for search in local recipientstable, and only attempt verifying with the keys we have saved for that recipient)
  4. result gets rendered including signature verification, and if we are not missing any public keys, then this is the last step
  5. if we are missing a public key, the verification "badge" will show gray 🕒 verifying signature..
  6. at this point we fetch remote public keys by sender email (like explained in step 3), but user is no longer waiting for it
  7. after remote public keys are fetched and local storage is updated, we check if any of the keys from local storage of that recipient/sender now contain the longid that signed the message. If they don't, now we render "missing pub key" badge
  8. if one of the keys now contain the longid of the key that signed the message, we re-process the whole message with that public key - in background. The badge still says 🕒 verifying signature. Once we've done re-processing the message, we only look at the result of signature verification and re-render the badge with the final verification result, without re-rendering the rest of the message.

@tomholub
Copy link
Collaborator Author

Looks like this on iOS

image

@tomholub
Copy link
Collaborator Author

tomholub commented Dec 6, 2021

@DenBond7 I'd like you to start working in this tomorrow - please check if you have everything you need.

@DenBond7
Copy link
Collaborator

DenBond7 commented Dec 7, 2021

Here a code snippet from iOS, regarding rendering various signature statuses
Looks like this on iOS

@tomholub Do we have examples for all cases? I mean UI

@tomholub
Copy link
Collaborator Author

tomholub commented Dec 7, 2021

yup I'll take some screenshots

@DenBond7
Copy link
Collaborator

DenBond7 commented Dec 7, 2021

Need to test the following cases:(in progress)

  • only encrypted

I. Inband signature

can't verify signature (missing pub key)

  • encrypted + signed
  • only signed

valid signature (good)

  • encrypted + signed
  • only signed

valid signature (partially)

  • encrypted + signed
  • only signed

valid signature (good+mixed)

  • encrypted + signed
  • only signed

II. Cleartext signature

can't verify signature (missing pub key)

  • only signed

valid signature (good)

  • only signed

valid signature (partially)

  • only signed

valid signature (good+mixed)(for example two different cleartext in sequence)

  • only signed

II. Detached signature

valid signature (good)

  • encrypted + signed
  • only signed

@tomholub
Copy link
Collaborator Author

tomholub commented Dec 7, 2021

IMG_1521
IMG_1520
IMG_1519
IMG_1518
IMG_1517

@tomholub
Copy link
Collaborator Author

tomholub commented Dec 7, 2021

This one is a bit broken - the badge is supposed to wrap around

IMG_C8829161E870-1

@DenBond7
Copy link
Collaborator

DenBond7 commented Dec 7, 2021

Thank you!

@tomholub
Copy link
Collaborator Author

tomholub commented Dec 7, 2021

I'll send you credentials for that account. You can test

@tomholub
Copy link
Collaborator Author

tomholub commented Dec 7, 2021

Here is updated code from iOS, that also takes into account partial and mixed signatures. Maybe, that would be for another issue later.

I think more interesting for you could be the TypeScript code that produces VerifyRes ?

    private func evaluateSignatureVerificationResult(
        signature: MsgBlock.VerifyRes?
    ) async -> ProcessedMessage.MessageSignature {
        guard let signature = signature else { return .unsigned }
        if let error = signature.error { return .error(error) }
        guard let signer = signature.signer else { return .unsigned }
        guard signature.match != nil else { return .missingPubkey(signer) }
        guard signature.match == true else { return .bad }
        guard signature.partial != true else { return .partial }
        guard signature.mixed != true else { return .goodMixed }
        return .good
    }
}

@tomholub
Copy link
Collaborator Author

tomholub commented Dec 8, 2021

Were you able to log into the e2e test account successfully?

@DenBond7
Copy link
Collaborator

DenBond7 commented Dec 8, 2021

Were you able to log into the e2e test account successfully?

Yup, all works perfectly.

@DenBond7
Copy link
Collaborator

DenBond7 commented Dec 8, 2021

@tomholub honestly, I'm a little confused with the current code. Currently, we support various types of different cases. I mean we support a lot of time MIME messages: with inner MIME, with PGP (encryption, signing), even PGP in PGP. We split all parsed msg blocks into 2 groups: content and other(like atts). After that, we transform all blocks from content to formated HTML. Now I'm thinking about how to deal with signatures.

Please loot at this example

Return-Path: <[email protected]>
Delivered-To: [email protected]
Received: from mail.flowcrypt.test
	by mail.flowcrypt.test with LMTP
	id kgZDL3j0sGGSBQAAc/RpdQ
	(envelope-from <[email protected]>)
	for <[email protected]>; Wed, 08 Dec 2021 18:07:52 +0000
Received: from localhost (localhost [127.0.0.1])
	by mail.flowcrypt.test (Postfix) with ESMTP id BB9526C0A07
	for <[email protected]>; Wed,  8 Dec 2021 18:07:52 +0000 (UTC)
Date: Wed, 8 Dec 2021 20:07:49 +0200 (GMT+02:00)
From: [email protected]
To: [email protected]
Message-ID: <[email protected]>
Subject: Encrypted + signed
Mime-Version: 1.0
Content-Type: multipart/mixed; 
	boundary="----=_Part_8_111556158.1638986869241"

------=_Part_8_111556158.1638986869241
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

-----BEGIN PGP MESSAGE-----
Version: PGPainless

hF4D16Pe22XLHvsSAQdA0EYKe+cg7EFOsmSGIUz55HKj9mAQ0l5CD/SnNvdEyH8w
DVGPGDO3+wYyR6vjWdqzLl68JGwfpYMY5H8avp+xJXojaOYNU8jIeUWM2HaRDRfM
hF4DTxRYvSK3u1MSAQdAvCdXhN2UyZHo9SNIhkc3QbnJUpytM9ImItOjadJQi10w
wDYNT+8ymf/nQk0RpUUkqVEcah2go9R3gjbvlbr2tlfYAJa2b2s7FMuMv8OqO8V0
0rcB1aUqzdIKfWHPY6HfuSTAR9mlD5ucPp3unLYzo857h4v16sAUxVCxo3Y4RlXT
qL2MttauH3ZbUgjRmSZU3V+5zGEBZykjF+0aBcxIaB4jEFQT0AvZ2TconoNMRChf
HZNifsrjFVc8JT1OaXdceH5v09BtPNfzvudBMU0GzQ7qymHEdXCpro4D1M1D+eJ6
M70+vltwnnY2eezULhsNsB9nXBZjO6eGUiADeCIHA0E0s+4sEXCChCM=
=iI3Z
-----END PGP MESSAGE-----

-----BEGIN PGP MESSAGE-----
Version: PGPainless

hF4D16Pe22XLHvsSAQdA0EYKe+cg7EFOsmSGIUz55HKj9mAQ0l5CD/SnNvdEyH8w
DVGPGDO3+wYyR6vjWdqzLl68JGwfpYMY5H8avp+xJXojaOYNU8jIeUWM2HaRDRfM
hF4DTxRYvSK3u1MSAQdAvCdXhN2UyZHo9SNIhkc3QbnJUpytM9ImItOjadJQi10w
wDYNT+8ymf/nQk0RpUUkqVEcah2go9R3gjbvlbr2tlfYAJa2b2s7FMuMv8OqO8V0
0rcB1aUqzdIKfWHPY6HfuSTAR9mlD5ucPp3unLYzo857h4v16sAUxVCxo3Y4RlXT
qL2MttauH3ZbUgjRmSZU3V+5zGEBZykjF+0aBcxIaB4jEFQT0AvZ2TconoNMRChf
HZNifsrjFVc8JT1OaXdceH5v09BtPNfzvudBMU0GzQ7qymHEdXCpro4D1M1D+eJ6
M70+vltwnnY2eezULhsNsB9nXBZjO6eGUiADeCIHA0E0s+4sEXCChCM=
=iI3Z
-----END PGP MESSAGE-----

------=_Part_8_111556158.1638986869241--

Both of these PGP blocks are encrypted and signed. I understand that it is an irregular case. But how should I handle such a case? The Android app can parse, decrypt and show this content. But what about inband signatures? Here we can have a verified signature for the first PGP block and an unverified for the second.

@tomholub
Copy link
Collaborator Author

tomholub commented Dec 8, 2021

Good question. I'll share what we've done on iOS.

@DenBond7
Copy link
Collaborator

DenBond7 commented Dec 8, 2021

Also, It would be nice to have the latest version of the iOS app, maybe @Kharchevskyi can update it

@tomholub
Copy link
Collaborator Author

tomholub commented Dec 8, 2021

I can make a build if you can tell me the uuid of your iOS device.

@tomholub
Copy link
Collaborator Author

tomholub commented Dec 8, 2021

export type VerifyRes = {
  signer?: string;
  match: boolean | null;
  error?: string;
  mixed?: boolean;
  partial?: boolean;
};

export const fmtContentBlock = (allContentBlocks: MsgBlock[]): { contentBlock: MsgBlock, text: string } => {
  let msgContentAsHtml = '';
  let msgContentAsText = '';
  const contentBlocks = allContentBlocks.filter(b => !Mime.isPlainImgAtt(b))
  const imgsAtTheBottom: MsgBlock[] = [];
  const inlineImgsByCid: { [cid: string]: MsgBlock } = {};
  for (let plainImgBlock of allContentBlocks.filter(b => Mime.isPlainImgAtt(b))) {
    if (plainImgBlock.attMeta!.cid) {
      inlineImgsByCid[plainImgBlock.attMeta!.cid.replace(/>$/, '').replace(/^</, '')] = plainImgBlock;
    } else {
      imgsAtTheBottom.push(plainImgBlock);
    }
  }

  var verifyRes: (VerifyRes | undefined) = undefined;
  var mixedSignatures = false;
  var signedBlockCount = 0;
  for (const block of contentBlocks) {
    if (block.verifyRes) {
      ++signedBlockCount;
      if (!verifyRes) {
        verifyRes = block.verifyRes;
      } else if (!block.verifyRes.match) {
        if (verifyRes.match) {
          verifyRes = block.verifyRes;
        }
      } else if (verifyRes.match && block.verifyRes.signer !== verifyRes.signer) {
        mixedSignatures = true;
      }
    }
    if (block.type === 'decryptedText') {
      msgContentAsHtml += fmtMsgContentBlockAsHtml(Str.asEscapedHtml(block.content.toString()), 'green');
      msgContentAsText += block.content.toString() + '\n';
    } else if (block.type === 'decryptedHtml') {
      // todo - add support for inline imgs? when included using cid
      msgContentAsHtml += fmtMsgContentBlockAsHtml(stripHtmlRootTags(block.content.toString()), 'green');
      msgContentAsText += Xss.htmlUnescape(Xss.htmlSanitizeAndStripAllTags(block.content.toString(), '\n') + '\n');
    } else if (block.type === 'plainText') {
      msgContentAsHtml += fmtMsgContentBlockAsHtml(Str.asEscapedHtml(block.content.toString()), 'plain');
      msgContentAsText += block.content.toString() + '\n';
    } else if (block.type === 'plainHtml') {
      const dirtyHtmlWithImgs = fillInlineHtmlImgs(stripHtmlRootTags(block.content.toString()), inlineImgsByCid);
      msgContentAsHtml += fmtMsgContentBlockAsHtml(dirtyHtmlWithImgs, 'plain');
      msgContentAsText += Xss.htmlUnescape(Xss.htmlSanitizeAndStripAllTags(dirtyHtmlWithImgs, '\n') + '\n');
    } else if (block.type === 'verifiedMsg') {
      msgContentAsHtml += fmtMsgContentBlockAsHtml(block.content.toString(), 'gray');
      msgContentAsText += Xss.htmlSanitizeAndStripAllTags(block.content.toString(), '\n') + '\n';
    } else {
      msgContentAsHtml += fmtMsgContentBlockAsHtml(block.content.toString(), 'plain');
      msgContentAsText += block.content.toString() + '\n';
    }
  }

  if (verifyRes && verifyRes.match) {
    if (mixedSignatures) {
      verifyRes.mixed = true;
    }
    if (signedBlockCount > 0 && signedBlockCount != contentBlocks.length) {
      verifyRes.partial = true;
    }
  }

  for (const inlineImg of imgsAtTheBottom.concat(Object.values(inlineImgsByCid))) { // render any images we did not insert into content, at the bottom
    let alt = `${inlineImg.attMeta!.name || '(unnamed image)'} - ${inlineImg.attMeta!.length! / 1024}kb`;
    // in current usage, as used by `endpoints.ts`: `block.attMeta!.data` actually contains base64 encoded data, not Uint8Array as the type claims
    let inlineImgTag = `<img src="data:${inlineImg.attMeta!.type};base64,${inlineImg.attMeta!.data}" alt="${Xss.escape(alt)} " />`;
    msgContentAsHtml += fmtMsgContentBlockAsHtml(inlineImgTag, 'plain');
    msgContentAsText += `[image: ${alt}]\n`;
  }

  msgContentAsHtml = `
  <!DOCTYPE html><html>
    <head>
      <meta name="viewport" content="width=device-width" />
      <style>
        body { word-wrap: break-word; word-break: break-word; hyphens: auto; margin-left: 0px; padding-left: 0px; }
        body img { display: inline !important; height: auto !important; max-width: 95% !important; }
        body pre { white-space: pre-wrap !important; }
        body > div.MsgBlock > table { zoom: 75% } /* table layouts tend to overflow - eg emails from fb */
      </style>
    </head>
    <body>${msgContentAsHtml}</body>
  </html>`;
  const contentBlock = MsgBlock.fromContent('plainHtml', msgContentAsHtml);
  contentBlock.verifyRes = verifyRes;
  return { contentBlock: contentBlock, text: msgContentAsText.trim() };
}

@DenBond7 DenBond7 changed the title verify signed messages verify signed messages(inband) Dec 13, 2021
@DenBond7
Copy link
Collaborator

@tomholub Please let me clarify some questions: what does "bad signature" mean in detail?

@tomholub
Copy link
Collaborator Author

@tomholub Please let me clarify some questions: what does "bad signature" mean in detail?

Signature could not be successfully verified for other reason than not having the right public key.

For example, the message was changed in transit.

@DenBond7
Copy link
Collaborator

What is already done?

  1. offline verification of inband signature that comes as PGP message
-----BEGIN PGP MESSAGE-----
Version: PGPainless

owGbwMvMwCXWaL/vvdiErcGMpxWSGEAgODM9LzVFoTyzJEOhJCNVoTg1OT8vRSE7
tbKjlIVBjIuBjZUpcevzDwyKnAIwzWKKLHfMHwmFMAmaprieNocJp/Uy/A/MjTzK
N9UkpXLTg0dXFFasM5wQxT93ufHj+o9hhzuO97YxMlwwsG+6lNHn4xkilcwbmmhx
/ernRzJH7rFaPLM7d/zlHj4A
=Yz2p
-----END PGP MESSAGE-----
  1. offline verification of cleartext signature
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

[...]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v0.9.7 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iEYEARECAAYFAjdYCQoACgkQJ9S6ULt1dqz6IwCfQ7wP6i/i8HhbcOSKF4ELyQB1
oCoAoOuqpRqEzr4kOkQqHRLE/b8/Rw2k
=y6kj
-----END PGP SIGNATURE-----
  1. Added examples to the local email server for the following cases

I. Inband signature

can't verify signature (missing pub key)

  • encrypted + signed
  • only signed

valid signature (good)

  • encrypted + signed
  • only signed

valid signature (partially)

  • encrypted + signed
  • only signed

valid signature (good+mixed)

  • encrypted + signed
  • only signed

What is left?

  1. Add tests for Inband signature
  2. Add tests for cleartext signature
  3. Add support of detached signature
  4. Add online verification(lookup)

For simplicity, I propose to do 1. in the current issue, move 2 for soon, and do 3 and 4 in separate issues.

@tomholub What do you think?

@tomholub
Copy link
Collaborator Author

What is Inband signature?

@tomholub
Copy link
Collaborator Author

Agree that 2 can be moved to soon milestone. Actually, 3 and 4 could also be done in a different milestone. I'll file them.

@DenBond7
Copy link
Collaborator

What is Inband signature?

It's PGPainless terminology :), means not detached.

@tomholub
Copy link
Collaborator Author

Got it. Yes - agree to finish 1 here, and then move onto the next task in current milestone. (forward attachments)

DenBond7 added a commit that referenced this issue Dec 13, 2021
DenBond7 added a commit that referenced this issue Dec 14, 2021
DenBond7 added a commit that referenced this issue Dec 15, 2021
DenBond7 added a commit that referenced this issue Dec 15, 2021
* Added a workable version of signature verification(Not all cases).| #1097

* Renamed PgpDecrypt to PgpDecryptAndOrVerify.| #1097

* Renamed PgpEncrypt to PgpEncryptAndOrSign.| #1097

* Refactored code.| #1097

* Added signature verification for cleartext.| #1097

* Modifed docker-mailserver source.| #1097

* Added handling 'bad signature' case.| #1097

* Fixed Junit tests.| #1097

* Fixed some UI tests.| #1097

* MessageDetailsActivityTest. Restored "retry" rule usage.| #1097

* Added tests for signature verification(inband).| #1097

* Refactored code.| #1097
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants