-
Notifications
You must be signed in to change notification settings - Fork 47
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
#5668 Add Thunderbird attachment decryption #5828
base: master
Are you sure you want to change the base?
Conversation
…derbird-attachments-support
…derbird-attachments-support
There is some limitation I've encountered for attachment's from which we tried to decrypt and render in case they match any of encrypteMsgNames here: flowcrypt-browser/extension/js/common/core/attachment.ts Lines 43 to 51 in a6c8f59
Problem is that, the Thunderbird client tries to decrypt the attachment named of those before the extension resulting to conflict and irretrievable attachments so we cant proceed to do decryption for ourselves. cc: @sosnovsky I'll check if this is the same case for detached signature. |
So there is no way to get original encrypted data from these attachments? |
…derbird-attachments-support
Greetings @sosnovsky I have explored the issue further. The attachment object gets only accessible if the attachment were actually decrypted by thunderbird, that's the only chance the extension can access the attachment object if they we're already decrypted by Thunderbird itself. I had accessed the attachment after I imported my private key into my thunderbird's account settings. Everything works well after I added my private key into Thunderbird and the message were rendered into the messageDisplay UI very well. We can somehow add this as one of the know limitation of the FlowCrypt for Thunderbird extension - what do you think?
Unfortunately, that's the problem. We wont be able to get any information about attachment without the thunderbird decrypt the message.asc first. |
Thanks for exploration, let's use your current solution with private key import and maybe later we'll find some way to make it work directly in FlowCrypt extension. |
Thank you, I'll keep the current changes. I'll let you know when its ready to be tested. I'll be here to assist you in that case when needed. Regarding my progress, I'm in the last phase of implementing detached signature verification. |
…derbird-attachments-support
Hi @sosnovsky - I'd like to report my progress in this PR. For testing, I've prepared every test cases in flowcrypt.compatibility account that we'll be using to test the implemented features in Thunderbird build. Below are the test cases that needs to be tested. The thunderbird tests are all compiled in the "thunderbird_client_test" Needs to be manually tested are (for reviewer):
|
As of now, the only one I have problems with in-general is the signature verification for detached signatures. This one fails on FlowCrypt thunderbird version but not on web browser version -- verifying the same signed message on openpgp.js raw code works as it passes the verification: Meanwhile, this works on thunderbird but not on web -- the |
extension/js/content_scripts/webmail/thunderbird/thunderbird-element-replacer.ts
Outdated
Show resolved
Hide resolved
extension/js/content_scripts/webmail/thunderbird/thunderbird-element-replacer.ts
Outdated
Show resolved
Hide resolved
extension/js/content_scripts/webmail/thunderbird/thunderbird-element-replacer.ts
Outdated
Show resolved
Hide resolved
let from = ''; | ||
if (tab.id && message?.id) { | ||
from = Str.parseEmail(message.author).email || ''; | ||
const mimeMsg = await messenger.messages.getFull(message.id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recently Thunderbird introduced new param decrypt
which enables/disables message decryption for full message fetch - https://webextension-api.thunderbird.net/en/128-esr-mv2/changes/128.html#id12.
Probably setting it to false
should help to handle case described in #5828 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sosnovsky - After trying to add the decrypt
option set to false
, it doesn't allow me to access the encrypted "message.asc" though I was really hoping that to work.
Upon reviewing, the main issue I think relies on listAttachments() as in that part the error raises when I did try another debugging. This leads me to wrote a support help via Thunderbird forum - https://support.mozilla.org/en-US/questions/1474480.
I'll further check if there's any workaround I can share with you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about using messages.getRaw for this case?
I tried to run getRaw
with decrypt: false
param and it returns full content of message.asc
attachment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sosnovsky - you are right. the .getRaw()
seemed to be the way to go. I tried to use "mailparser" but I can't get it to work in background script. I mean, I have settled the issue in typescript types through content script tsconfig.json but the usage for the actual js does not work. I think the 'mailparser' js in not injected in content script so I'm getting undefined when calling simpleParser
: https://nodemailer.com/extras/mailparser/#simpleparser
Sorry, do you know how I could check if mailparser's simpleParser gets injected into TB?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please kindly ignore my last question regarding "mailparser", im doing it incorrectly. I'm now trying different approach with PostalMime which said to be usable for webpack compiled project: https://www.npmjs.com/package/postal-mime
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @martgil, in mentioned PR Ioan made 2 separate builds for web-stream-tools
lib -streams_web.js
is used in browser, while streams_common.js
is used in tests, because of some import issues, as I remember.
Thunderbird errors can be caused by similar issue, let's try to use streams_common.js
in Thunderbird build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sosnovsky I've tried several things including using streams_common.js
in Thunderbird build (manifest.json) and removes streams_web.js
and it doesn't work and I'm still getting the generic error uncaught exception: Object
from the Thunderbird email client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sosnovsky sorry for pinging you so often. I still really need help for this one. Also, I'm now started to wonder that this issue in Thunderbird build will experience the same issue more often if it still in the same branch like in the main.
I'm thinking of ways to solve that as well so that it won't be affected in case of any unexpected change from the content script. Do you think it will be solve by having it a separate branch of its own?
I'm a bit stuck with this PR, I can't fix it on my own. Sorry about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @martgil, sorry for the late reply!
On this week we've had a high priority task - to add password-protected messages compliance for all FlowCrypt apps (#5879), so I didn't have enough time to deeply check this issue with Thunderbird content scripts.
I'm thinking of ways to solve that as well so that it won't be affected in case of any unexpected change from the content script. Do you think it will be solve by having it a separate branch of its own?
I think it’s possible to fix this and maintain a single main
branch, as managing a separate branch for Thunderbird builds could become more complicated in the future. I’ll give it a try next week and let you know if I find a way to resolve the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Roma, no worries - I understand completely.
On this week we've had a high priority task - to add password-protected messages compliance for all FlowCrypt apps (#5879), so I didn't have enough time to deeply check this issue with Thunderbird content scripts.
I understand. I believe in your insights as always. If there is anything I can do to try and fix this, please feel free to provide instructions, and I'll follow them.
I think it’s possible to fix this and maintain a single main branch, as managing a separate branch for Thunderbird builds could become more complicated in the future. I’ll give it a try next week and let you know if I find a way to resolve the issue.
…derbird-attachments-support
…derbird-attachments-support
…derbird-attachments-support
This PR adds Thunderbird port the ability to render, recognize attachment and download decrypted attachment (when possible).
#5668
Tests (delete all except exactly one):
To be filled by reviewers
I have reviewed that this PR... (tick whichever items you personally focused on during this review):