753703 - SecReview: Network Monitor

Status: RESOLVED FIXED

(mozilla.org :: Security Assurance, task)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

1) Who is/are the point of contact(s) for this review?
Jan Odvarko, Joe Walker, Mike Ratcliffe

2) Please provide a short description of the feature / application (e.g. problem solved, use cases, etc.):
Monitoring of HTTP activity of a selected tab in the browser
Feature page: https://wiki.mozilla.org/DevTools/Features/NetworkView

3) Please provide links to additional information (e.g. feature page, wiki) if available and not yet included in feature description:
Introduction blog post: http://www.softwareishard.com/blog/planet-mozilla/http-monitor/
Wiki page: http://getfirebug.com/wiki/index.php/HTTP_Monitor
Source: https://github.com/firebug/httpmonitor

4) Does this request block another bug? If so, please indicate the bug number
bug 749411

5) This review will be scheduled amongst other requested reviews. What is the urgency or needed completion date of this review?
Not urgent AFAIK

6) To help prioritize this work request, does this project support a goal specifically listed on this quarter's goal list? If so, which goal?
No 

7) Please answer the following few questions: (Note: If you are asked to describe anything, 1-2 sentences shall suffice.)
- Does this feature or code change affect Firefox, Thunderbird or any product or service the Mozilla ships to end users?
There is new menu item under "Web Developer" menu called "HTTP Monitor". This menu opens HTTP Monitor console.

- Are there any portions of the project that interact with 3rd party services?
No

- Will your application/service collect user data? If so, please describe 
Yes, the monitor collects data about HTTP activity (timing, posted data, response bodies, headers, urls, etc.)

8) If you feel something is missing here or you would like to provide other kind of feedback, feel free to do so here (no limits on size):
No

9) Desired Date of review (if known from https://mail.mozilla.com/home/[e-mail]/Security%20Review.html) and whom to invite. 
21st May, Mark Goodwin

Honza

Comment 1

[Joe Walker [:jwalker]]

This feature had several confusing names (HttpMonitor/NetPanel/...) we've settled on Network Monitor (netmonitor for short).

Comment 2

[Mark Goodwin [:mgoodwin]]

Curtis, can we have the 21st secreview session for this please?

Comment 3

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

SecReview Scheduled:
https://mail.mozilla.com/home/[e-mail]/Security%20Review.html?view=month&action=view&invId=130654-130653&pstat=AC&exInvId=130654-179151&useInstance=1&instStartTime=1337630400000&instDuration=3600000

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Any chance, the meeting could be a bit (2-3 hours) sooner? (I am GMT+2)
Honza

Comment 5

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

Sorry Jan, but no. that would conflict with other Mozilla wide meetings. And SecReviews are set for fixed slots M/W 13:00 PST and Th/F 10:00 PST so that all the various schedules of regular attendees can be reserved to attend.

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Sure, I understand, no problem
Honza

Comment 7

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

Item to be reiviewed: netmonitor
Link to calendar entry: https://mail.mozilla.com/home/[e-mail]/Security%20Review.html?view=month&action=view&invId=39f7eb29-6e7d-4e4d-97b6-550567eab25c%3a130654-130653&pstat=AC&exInvId=39f7eb29-6e7d-4e4d-97b6-550567eab25c%3a130654-179151&useInstance=1&instStartTime=1337630400000&instDuration=3600000
SecReview Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=753703 
Security Lead: mgoodwin
Required Reading List:
Introduction blog post: http://www.softwareishard.com/blog/planet-mozilla/http-monitor/
Wiki page: http://getfirebug.com/wiki/index.php/HTTP_Monitor
Optional reading:
Source: https://github.com/firebug/httpmonitor
(If possible prefill this area for copying to the notes section of the review)
Introduce Feature (5-10 minutes) [can be answered ahead of time to save meeting time]
- Goal of Feature, what is trying to be achieved (problem solved, use cases, etc)
The Firebug net panel provides a useful way of looking at the HTTP Activity of a tab; work is underway to extract the net panel logic from firebug and expose it within firefox (currently as an extension, eventually in-browser) in 2 ways:
1) a firebug-free version of the netpanel UI
2) as an HTTP tracking component that can be used by the above and any other tools that want to intercept and analyse HTTP traffic.
NetMonitor makes use of the debugger protocol built for JSD2 - currently (in the extension) it creates its own listener which communicates via this protocol; eventually, the plan is to share listeners when both remote debugging and remote monitoring are enabled.  Note: This review is not intended to cover the debugger protocol itself; hopefully we can cover that on the 24th in the debugger review session - obviously, any net monitor specific implications are in-scope.
 
- What solutions/approaches were considered other than the proposed solution?
- Why was this solution chosen?
- Any security threats already considered in the design and why?
* Threat Brainstorming (30-40 minutes)
* Are the File objects persisted on disk? Also are these files different per-tab / private-browsing mode?
** When are the files deleted if persisted?
* Conclusions / Action Items (10-20 minutes)

Comment 8

[Mark Goodwin [:mgoodwin]]

Some notes:
 * Currently doesn't support anything other than HTTP.
 * Uses existing necko APIs - if the monitor should intercept other types, we'd need hooks to be available for other events, etc.
** so far, websockets is most likely to happen soon.

== Threat Brainstorming ==
* Are the File objects persisted on disk? Also are these files different per-tab / private-browsing mode?
** no
** When are the files deleted if persisted?
*** N/a - see above
* Is this enabled all the time or per user request?
 ** by default it's per tab (clear cache when changed)
 ** there's a persist button to enable "persist mode" which allows previous requests to be kept in the console (for analysing redirects or page flow).
* When the response body is displayed to the user, sometimes it's useful for it to be formatted somehow. (e.g. JSON / XML you might want an expandable tree)
** in these cases - it's important to mention that if we're parsing that content, that could have security implications.
** There are a potential number of flaws to worry about in the UI part of the output (output filtering, etc. is important).
* Response body is displayed as text in the UI.  And also limited by size - to avoid crashes.  There is a limit on the display and a separate limit on the cache.
* Everything displayed to the user is in an iFrame (origin: chrome) - recommended as a defense in depth thing to remove privileges where possible. The whole UI is injected into the iFrame... looks like it might need scripting (maybe use the sandbox feature when we've got that done).
** find the docshell that's associated with the iframe, "fiddle with that" to control this.
** Is there a reason this would be a better solution to CSP? Because CSP needs an origin. - the docshell features don't care where things are loaded from as they're features of the container and not the document. If you know it can't navigate then you could inject a CSP on that (otherwise an attack could inject a link in there, navigation could result in CSP becoming non-effectual).
* What if the image requests are not idempotent? Or if the resource is switched post-original request.
** What about URLs changing to a JS url
** What about images being lots larger than they were originally?

== Conclusions / Action Items ==
* Who :: What :: By when (Keep in mind all these things will be bugs that block the reivew bug, that blocks the feature bug)
* JoeW/honza :: Look at restricting privileges of the iframe. CSP possible if navigation is not allowed, otherwise look at docshell tricks / iframe sandbox::before Aurora
* Jo/Honza :: Put in a check that URL is http / https::before Aurora