Closed Bug 1365332 Opened 7 years ago Closed 7 years ago

Use chrome:// urls for Activity Stream content files

Categories

(Firefox Graveyard :: Activity Streams: General, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: k88hudson, Assigned: k88hudson)

Details

Attachments

(1 file)

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: nobody → khudson
Summary: Update Activity Stream to use chrome:// → Use chrome:// urls for Activity Stream content files
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?
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.
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 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)
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
Closed: 7 years ago
Flags: needinfo?(khudson)
Resolution: --- → INVALID
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: