updates related to #1509

[rrrooommmaaa]

Trying to fix#5373 (to be merged after PR #5372)

---

Tests(delete all except exactly one):

• Tests added or updated

---

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

• addresses the issue it closes (if any)
• code is readable and understandable
• is accompanied with tests, or tests are not needed
• is free of vulnerabilities
• is documented clearly and usefully, or doesn't need documentation

[rrrooommmaaa]

@tomholub

These issues are encountered:

1. broadcast messages sent with chrome.runtime.sendMessage don't reach content script's handler
2. in Firefox, broadcast messages sent with chrome.runtime.sendMessage from the content script don't reach any frames (only background)

[tomholub]

deleted my comment. what if we used the insecure messaging everywhere, does it easily reach where we need it?

then we encrypt & sign, decrypt & verify.

[rrrooommmaaa]

Somewhat easier, I think.
You said to take the symmetric key for encryption from the local storage.
There might be problems addressing local storage from the content script, not sure.
We have this comment:

flowcrypt-browser/extension/js/common/platform/store/acct-store.ts

Lines 88 to 90 in 6d6a340

	if(Env.isContentScript()){
	// extension storage can be disallowed in rare cases for content scripts throwing 'Error: Access to extension API denied.'
	// go through bg script to avoid such errors

Also, how should we keep session passphrases? Keeping them in the content script would mean that opening several gmail tabs would require entering passphrases into each of them independently, right?

[tomholub]

Also, how should we keep session passphrases? Keeping them in the content script would mean that opening several gmail tabs would require entering passphrases into each of them independently, right?

I'm ok with that limitation. We could then reintroduce a background worker later, if it turns out a problem.

// extension storage can be disallowed in rare cases for content scripts throwing 'Error: Access to extension API denied.'
// go through bg script to avoid such errors

Could have been fixed on recent browser versions? That's probably from 6 years ago. We could just try it and see what happens. Alternatively, we could try window.localStorage instead, in case it can be adequately accessed from everywhere. Worst case, each content script would make a worker, and access the extension storage through its worker.

[rrrooommmaaa]

then we encrypt & sign, decrypt & verify.

You were previously talking about using a symmetric key. Can you elaborate on sign/verify part?

[tomholub]

I don't have anything that specific in mind, but surely window.crypto has some way to encrypt things in a manner that can be verified to be authentic when decrypting.

Roughly we could send something like {destination: [list of receiving frames], content: encryptedData}

Where the encryptedData also contains the destination. That way, receiving frames don't have to decrypt it to decide if they should drop the message or not, while at the same time they can still verify that the message and destination is authentic and matching once they do decrypt it.

Then the content would just be an encrypted/serialised json of whatever today is in these messages.

Just a rough idea.

[rrrooommmaaa]

some API is not available for the content script. e.g. chrome.tabs, so some features aren't possible without the background page, e.g. to open a settings page from this notification

What should we do about it?

[tomholub]

some API is not available for the content script. e.g. chrome.tabs, so some features aren't possible without the background page, e.g. to open a settings page from this notification

What should we do about it?

We could just make it a regular a href target=_blank ? I forgot why we do this through background page.

[rrrooommmaaa]

I forgot why we do this through background page.

To disallow opening settings tab multiple times?

[tomholub]

I forgot why we do this through background page.

To disallow opening settings tab multiple times?

Sounds not so important, ok to not worry about that

[rrrooommmaaa]

We also use chrome.windows.create to create Oauth popup.
What should we do? Embed it in a frame?

[rrrooommmaaa]

this mainly involves reconnectAcctAuthPopup process initiated by the content script

[tomholub]

We also use chrome.windows.create to create Oauth popup.
What should we do? Embed it in a frame?

this mainly involves reconnectAcctAuthPopup process initiated by the content script

This will not be easy, imo it warrants a separate PR. So I'd leave that as is for now, from background script.

[rrrooommmaaa]

There is a problem using fetch() from the content script in Firefox: The page’s settings blocked the loading of a resource at inline (“script-src”) Any ideas of how to fix this?
Should I make fetch() calls from the background page in this PR? For Firefox only or both?
Switching to direct fetch() would simplify upload/download progress implementation, but will break Firefox.
In Chrome, fetch is allowed in the content script though with some peculiarities: after logging in to a mock google account, the browser submits google's access token to some other endpoints, e.g. ekm's /keys/private, meaning:
the mock endpoint expects an idtoken like Authorization: Bearer fakeheader... and passing this in headers of the fetch request is overridden by the browser, if it has already remembered google's accessToken. The mock endpoint thus receives Authorization: Bearer mock-access-token-... instead. Changing the url of mock ekm to https://ekm.localhost doesn't help.
In production there isn't such a problem as domains don't look similar there.
I wonder how we can wire mock endpoints and oauth in a better way. Or would it be acceptable to update the mock endpoint to accept idtoken or accesstoken?
@tomholub@sosnovsky

[sosnovsky]

Maybe then it'll be better to implement migration to native fetch in another PR?
So in this PR you'll implement fix for original issue #5329 and then we can find solution for this fetch migration (or just leave it for later).

It's possible to allow inline scripts using nonce or hashes - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#unsafe_inline_script, but not sure if it's possible in our case.

[sosnovsky]

@sosnovsky Is this PR still relevant?

I think it can be closed, as initial issue is not reproducible anymore.