If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Use chrome:// urls for Activity Stream content files

RESOLVED INVALID

Status

()

Firefox
Activity Streams: General
RESOLVED INVALID
4 months ago
4 months ago

People

(Reporter: k88hudson, Assigned: k88hudson)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

4 months ago
We should update Activity Stream to use chrome:// URLs for content files so they cannot be loaded in regular web pages (resource:// URLs can).

This needs to be done in MC first because it requires a change to aboutNewTabService.js, we can port back to Github after that lands.
(Assignee)

Updated

4 months ago
Assignee: nobody → khudson
(Assignee)

Updated

4 months ago
Summary: Update Activity Stream to use chrome:// → Use chrome:// urls for Activity Stream content files
Comment hidden (mozreview-request)

Comment 2

4 months ago
Can you unpack the thinking a bit here?  In particular, have a look at <https://dxr.mozilla.org/mozilla-central/source/browser/components/about/AboutRedirector.cpp#32>.  Currently, we don't supply the URI_SAFE_FOR_UNTRUSTED_CONTENT flag, meaning that our stuff is running with chrome privs.  However, I suspect we don't want or need those privs, if we can use smaller privilege blocks (eg the way about:home does today).  If I'm right, and we add URI_SAFE_FOR_UNTRUSTED_CONTENT, then that content makes them viewable by web content anyway, although more secure than they are today.

Why is having them be viewable by web content a problem that is important to solve?

Comment 3

4 months ago
There's other system-addons with resource URIs:

https://dxr.mozilla.org/mozilla-central/search?q=resource%20path%3Ajar.mn%20path%3Aextensions
(Assignee)

Comment 4

4 months ago
The original reason we wanted to do this because ursula was working on trying to switch our code over to use file path references for screenshots, and resource:// urls don't have access to the filesystem, whereas chrome:// urls do. The eventual solution for this will be by fixing the page-thumbs protocol to work in content (which ursula is going to work on in https://bugzilla.mozilla.org/show_bug.cgi?id=1184701), however mconley mentioned that it would be a good idea to use a chrome url anyway for security reasons since they can't be included as sub resources inside a resource:// or a regular html page. It would prevent people from doing something weird like embedding the new tab page in an iframe etc.
(Assignee)

Comment 5

4 months ago
What do you think about this bholley? Are there any reasons why we should use either resource:// or chrome:// URLs for Activity Stream on the new tab page (note that it's unprivileged and will be in a separate content process)
Flags: needinfo?(bobbyholley)
In general it's a good goal to prevent web content from loading or linking to internal pages. That, an even more important goal is ensuring that all of the content we write runs with minimal (i.e. non-system-principal) privileges. If we can achieve both that's worth doing, but the latter is more important.

There were some efforts at some point to load browser content in <iframe sandbox> to minimize their privileges and apply a CSP, but I don't recall exactly what happened there. I've been too busy with stylo stuff in recent times to keep up with how the frontend architecture is evolving, so I'm not a great source for full guidance on the specifics unfortunately.
Flags: needinfo?(bobbyholley)
I'm a little confused about the problem we are trying to solve by switching to chrome:// URLs here.  Is it just that resource:// has no concept of contentaccessible as defined at <https://developer.mozilla.org/en/docs/Chrome_Registration#contentaccessible>?  If so, the patches in bug 863246 are adding such a concept, I believe.

Comment 8

4 months ago
Comment on attachment 8868224 [details]
Bug 1365332 - Use chrome:// urls for Activity Stream content files

Sounds like this won't be necessary anymore? Could you provide some details and resolve if so?
Flags: needinfo?(khudson)
Attachment #8868224 - Flags: review?(edilee)
(Assignee)

Comment 9

4 months ago
After some discussion we decided this isn't necessary (given that we Activity-Stream will be unprivileged and can handle other security issues with a CSP)
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Flags: needinfo?(khudson)
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.