764645 - SecReview: Firefox Health Report - Security Code Review

Status: NEW

(mozilla.org :: Security Assurance: Review Request, task)

Description

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

SecReview tracking bug
Per the review we need to preform a code review

Comment 1

[Gregory Szorc [:gps]]

I'm on the hook for writing this code now. Will let you know when I have something for you to review.

Comment 2

[Gregory Szorc [:gps]]

The part of Firefox Health Report that will constitute the B2G ADU ping is about to land (disabled). Security review of this code can commence.

Additional code will land before FHR is enabled in Firefox desktop. We can wait and do a comprehensive review or we can do the review in 2 or incremental phases.

Curtis: I think this is your call.

Comment 3

[Gregory Szorc [:gps]]

The part of Firefox Health Report that will constitute the B2G ADU ping is about to land (disabled). Security review of this code can commence.

Additional code will land before FHR is enabled in Firefox desktop. We can wait and do a comprehensive review or we can do the review in 2 or incremental phases.

Curtis: I think this is your call.

Comment 4

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

Actually this bug is owned by dveditz so it's his call

Comment 5

[Daniel Veditz [:dveditz]]

I thought the whole point of FHR was to divorce the additional information from the ADU-counting ping. If those are combined in b2g then we should definitely treat them separately.

Given B2G wants to branch soon let's do them in two phases.

Do we also need to do a privacy review on the information collected or has that already gone through the UDC process?

Comment 6

[Richard Newman [:rnewman]]

(In reply to Daniel Veditz [:dveditz] from comment #5)
> I thought the whole point of FHR was to divorce the additional information
> from the ADU-counting ping. If those are combined in b2g then we should
> definitely treat them separately.

FHR is currently a very minimal set; most of the data providers that will make it distinct from an ADU ping are not yet implemented.

Comment 7

[Gregory Szorc [:gps]]

FHR will be enabled in m-c by the time you likely read this. It will only be enabled in the "browser" app (desktop Firefox) for the time being.

Daniel: it's been a while since I've done a security review. What do you need from me?

Comment 8

[Gregory Szorc [:gps]]

Per IRC discussion, I am to answer the questions to https://wiki.mozilla.org/Security/Reviews/Review_Request_Form#Questions_to_Address_within_Request_Body in this bug.

Comment 9

[Gregory Szorc [:gps]]

Who is/are the point of contact(s) for this review?

  gps

Please provide a short description of the feature / application (e.g. problem solved, use cases, etc.):

  Periodic upload of anonymous product, machine, and usage information to Mozilla.

Please provide links to additional information (e.g. feature page, wiki) if available and not yet included in feature description:

  https://blog.mozilla.org/metrics/2012/09/21/firefox-health-report/

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

  It blocks the release of FHR to the beta channel, IMO.

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

  Review and actions need to be finished before Firefox 20 goes to Beta channel. So let's say by Feb 8.

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

  FHR is a core component of goals for many teams.

Does this feature or code change affect Firefox, Thunderbird or any product or service the Mozilla ships to end users?

  Firefox desktop. Eventually other apps, but those will be separate reviews. We add an always-on, always-collecting, but not always-uploading service to Firefox. We persist data for 180 days in a SQLite database.

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

  3rd party, no. We do interact with Mozilla-owned domains.

Will your application/service collect user data?

  Yes. That is the point of the feature. However, collected data does not personally identify a user and the collected data has been approved by Privacy and Legal.

Desired Date of review:

  Sooner the better, preferably a 1 PM slot. I will be on PTO for many of the upcoming Thursdays and Fridays in February.

Comment 10

[Laura Thomson :laura]

Two things:
1. We also need to review the server side code (I'll provide details in next comment)
2. This is in Firefox 21, shipping Tuesday.

Comment 11

[Laura Thomson :laura]

The following comments refer to the server-side aspect of the feature. about:healthreport loads a page of HTML and JS from a web server via HTTPS. The chrome then injects a JSON payload via postMessage, and the JS interpolates the data into the page and draws a graph. 

- Who is/are the point of contact(s) for this review?
[e-mail]

- Please provide a short description of the feature / application (e.g. problem solved, use cases, etc.)
about:healthreport loads a page of HTML and JS from a web server via HTTPS. The chrome then injects a JSON payload via postMessage, and the JS interpolates the data into the page and draws a graph. 

The page shows the user their performance information as well as providing tips as to how to improve performance.


- Please provide links to additional information (e.g. feature page, wiki) if available and not yet included in feature description:
spec: https://bugzilla.mozilla.org/show_bug.cgi?id=832547#c44 (bug attachment)
code https://github.com/mozilla/fhr-jelly
production https://fhr.cdn.mozilla.net/en-US/
client api docs https://mconnor.etherpad.mozilla.org/aboutHealthReportAPI
server side tracker https://bugzilla.mozilla.org/show_bug.cgi?id=853120

- Does this request block another bug? If so, please indicate the bug number
(as above)

- This review will be scheduled amongst other requested reviews. What is the urgency or needed completion date of this review?
Yesterday - ships in Firefox 21

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

- 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?
Yes

- Are there any portions of the project that interact with 3rd party services?
No, unless you count the CDN

- Will your application/service collect user data? If so, please describe 
Not in the server side part - see gps' comments above

- 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):
n/a

- Desired Date of review (if known from https://mail.mozilla.com/home/[e-mail]/Security%20Review.html) and whom to invite. 
ASAP. Invite mconnor and laura.