Closed Bug 1012652 Opened 10 years ago Closed 6 years ago

[Email] Remove inline style for CSP compliance

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: gerard-majax, Unassigned)

References

Details

Confere bug 968907 and bug 858787. We need to remove all CSS inline usage in certified apps.

https://github.com/mozilla-b2g/gaia/blob/master/apps/email/index.html#L43
Depends on: 817674
If I understand the implications of those bugs (and I may not!), then enabling inline-style enforcement for the email app may do very bad things to the email app for HTML message display.

Specifically, we show HTML mails in an <iframe sandbox="allow-same-origin">.  We use the same origin so we can mess with the DOM and be able to show confirmation prompts for clicking links, etc.  It's my impression that this will subject the iframe to rules at least as strict as the email app itself.  This will cause problems since HTML mails must use inline styles.  (Well, they could theoretically embed them as a separate "cid:" part, but that ship has sailed/dog won't hunt/etc.)

Am I correct in my understanding?


Note: I'm not saying we should never flip the switch, I just want to make sure we understand the impact on the email app since if I do understand correctly, we may need to do a fairly extensive overhaul of how we display HTML mails.  I think that there was some conversation about our situation as it relates to this a while back and we ended up concluding we'd deal with this when it becomes a problem.  I can probably dig out some links to that if desired.
Flags: needinfo?(grobinson)
> Specifically, we show HTML mails in an <iframe sandbox="allow-same-origin">
[...]
> This will cause problems since HTML mails must use inline styles.

Yes, the default blocking of inline styles sounds like it will cause problems for you.

One way to fix this could be to make the iframe a mozbrowser, since that overrides the default CSP application. Would that work in your case? This sort of fits with the original goal of mozbrowser (which was to enable implementing a web browser app on B2G).

Another way might be to set a fake src="" attribute on the iframe. I haven't tried this, but that might break the "inherit parent principal's" behavior that you're seeing now. I'm not sure if that would cause other problems.

If this is a really difficult problem, we may want to reconsider including inline style blocking in the default CSP for certified apps.
Flags: needinfo?(grobinson)
First, I should note that email doesn't particularly need to be a certified app, so that might be the easiest solution that doesn't diminish the security of apps that really do need to be certified.  There may be some mozContacts-related wrinkles, but in general as documented at https://wiki.mozilla.org/Gaia/AppPermissions#email we currently only need it because our send sound is globally controlled.


I think mozbrowser is a lot more powerful than we want.  If we can lose the CSP by putting the iframe in another origin via src/srcdoc, then I think there are things we can do to maintain the status quo.  If we continue our current weird ways, I think we'd end up with:

- outer iframe (new origin), JS enabled.  Runs our control code and talks to us via postMessage.

- inner iframe, JS disabled, this is the iframe we actually put the mail content in.  (Although our sanitizer and its whitelist ideally don't let anything through, I sleep better at night if JS is disabled in the iframe.)


We could also just revisit things from first principles.  The main things we need to do with the iframe right now that complicate things are:

- Enable external resource fetches.  By default our sanitizer produces HTML that is inert (<img src="x"> becomes <img class="moz-external-image" ext-src="x">) and we manipulate it to turn those back on.  We technically don't need to manipulate the existing DOM to do this; we could instead just create a new iframe/document that has the external fetches re-enabled.

- Display embedded images.  HTML mails may include attachments and reference them via the "cid" protocol.  We store those in IndexedDB as Blobs and use URL.createObjectURL to show them in the document.  The spec's bit about the URLs not working cross origin at http://dev.w3.org/2006/webapi/FileAPI/#security-discussion seems to be the most explicit guidance on this.  I'm not too sure how we can efficiently get around this other than passing the Blob in via postMessage and using createObjectURL/createObjectFor inside the iframe.  Using data URIs is arguably way too inefficient for us to entertain the notion.  We might be okay if we address this by using a ServiceWorker as long as our implementation prevents other apps or emails from nefariously inferring information about the presence of messages by generating URLs via this mechanism.

- Prompting for navigation.  Because of how the rest of the FxOS UI works, it's important for the email app to explicitly show the user the URL of what link they're clicking in order to have a fighting chance of not getting phished.  Right now, like images, links are rendered inert (moved to "ext-href") and we explicitly handle the click event.  We could instead rewrite all links to use a trampoline that we control or otherwise cause a notification to internally notify our app by abusing the web activity subsystem, etc.   Note that we're currently discussing potentially changing this UI in bug 1007820.  (I have a pending comment about phishing concerns that I need to finish and send, so please CC/wait for my response there before chiming in if you're interested in that issue.)
I'd be happy if we could make the email app prvileged (instead of certified). Other apps are going that route and I think it might be worthwhile to follow :)

I also agree that mozbrowser is a bit too heavy.
I like the idea with srcdocs. This might allow having the sandboxed frame in a different origin, which would provide another security layer.
The double-frame postMessage thing might work as well but it feels a bit quirky and not so intuitive to me.
(In reply to Frederik Braun [:freddyb] from comment #4)
> I'd be happy if we could make the email app prvileged (instead of
> certified). Other apps are going that route and I think it might be
> worthwhile to follow :)

+1 for not certified.

> 
> I also agree that mozbrowser is a bit too heavy.
> I like the idea with srcdocs. This might allow having the sandboxed frame in
> a different origin, which would provide another security layer.
> The double-frame postMessage thing might work as well but it feels a bit
> quirky and not so intuitive to me.

Doesn't it have to be same-origin so that the email app can reflow the email as it wants etc?
Bug 1018534 converted the email app to a privileged app. Marking that bug as a dependency for this bug.
Depends on: 1018534
Bug 1027185 will be switching the manifest back to "certified" (details on why in that bug). Bug 1018534 is no longer a dependency, and email as "privileged" will not be a tool in solving this issue.
No longer depends on: 1018534
A workaround for this bug may be to just to allow certified apps to override their CSP, not just tighten it.
PS in general I would like to avoid modifying a certified app's CSP, but I dont see any other solution here, and I dont want this to block changing the default certified CSP back to blocking inline style.
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.