971467 - Treeherder Security Review

Status: RESOLVED FIXED

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

Description

[Jonathan Eads ( :jeads )]

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

Jonathan Eads ( jeads <at> mozilla.com, jeads)

You can find the developers working on this project in the treeherder channel:

IRC: #treeherder

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

Treeherder is a replacement of an existing application, Tinderbox Pushlog (https://tbpl.mozilla.org/), used for the display of automated test results for several core mozilla products. Treeherder addresses a variety of limitations in tbpl's data model, web service, and user interface. A summary of those limitations can be found here https://wiki.mozilla.org/Auto-tools/Projects/TBPL2. 

The primary server side use cases for treeherder are to provide a web service interface that allows easy submission of test data. This test data is generated by internal test automation systems (buildbot, jenkins, or any job scheduler) associated with pushing code to a particular repository, for any of mozilla's products. These systems need to scale appropriately and also isolate data from different repositories without the need for co-localization within a database.

In addition to server side requirements, an extensive list of UI requirements, can be found here https://wiki.mozilla.org/Auto-tools/Projects/Treeherder/Current_TBPL_Feature_List. 

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

Related source code repositories:
https://github.com/mozilla/treeherder-service
https://github.com/mozilla/treeherder-ui
https://github.com/mozilla/treeherder-client

Deployed test systems:
https://treeherder.allizom.org
http://treeherder-dev.allizom.org

Background:
https://wiki.mozilla.org/Auto-tools/Projects/TBPL2
https://wiki.mozilla.org/Auto-tools/Projects/Treeherder/Current_TBPL_Feature_List

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

Not at the moment. We will add a production release bug when it's available.

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

The review needs to be completed by the first week of April at the beginning of Q2.

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?

Our goal is to complete a production release of treeherder in early/mid Q2
https://wiki.mozilla.org/Auto-tools/Goals/2014Q1#Treeherder_.28jeads.29

We are targeting full feature parity in Q1, it's a top level goal to complete a production release in early/mid Q2
https://wiki.mozilla.org/Auto-tools/Goals/2014Q1

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

No, not directly. It does affect our ability to release bug free products to users.

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

We ingest data generated by buildbot and published to the publicly available buildapi, this includes parsing test log output. We also ingest data from bugzilla and try to associate it with test failures to look for recurrent/known test failure behavior and to assist in managing bug fixing. We interact with the same services that https://tbpl.mozilla.org/ interacts with. 

Will your application/service collect user data? If so, please describe:

The application does not collect user data. It collects test data and push meta data (list of source code authors, times, patch descriptions) associated with source code repository pushes/pull requests.

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):

In addition to test data we will be implementing user profiles and providing some admin/settings functionality through the UI. So we will require a login feature.

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

Tuesday April 1st or Wednesday April 2nd. If available, a PST morning time would work best for the proposed attendees. 

Please invite: Jonathan Eads (:jeads), Cameron Dawson (:camd), and Mauro Doglio (:mdoglio)

Comment 1

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

We can do April 2 at 1pm PST or April 3 or 4 at 10am PST which works best for you all?

Comment 2

[Jonathan Eads ( :jeads )]

April 3rd at 10am PST would work best for us.

Comment 3

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

Review Scheduled
https://mail.mozilla.com/home/[e-mail]/Security%20Review.html?view=month&date=20140413&action=view&invId=400560-400559&pstat=AC&exInvId=400560-419404&useInstance=1&instStartTime=1396544400000&instDuration=3600000

Pre-meeting information we need from you all
http://goo.gl/p5YGRs

Comment 4

[Jonathan Eads ( :jeads )]

Pre-meeting information requested:

Goal of Feature, what is trying to be achieved (problem solved, use cases, etc / please include links to relevant bugs, wiki, github what have you)

This is well covered above but for more on the top level goals of the project please see https://wiki.mozilla.org/Auto-tools/Projects/Treeherder/Design_Draft. This covers some of the design goals we're trying to achieve that came out of limitations in the data model and web services provided in https://tbpl.mozilla.org/.

What solutions/approaches were considered other than the proposed solution?
Upgrading tbpl (https://tbpl.mozilla.org/) directly by a comprehensive refactoring. However, after examining the application's architecture this approach was declared a dead end.

Why was this solution chosen?
It satisfied all of our requirements and facilitated long term scalable development.

Any security threats already considered in the design and why?

There are web service endpoints for posting data to. These are an obvious exploit, we implemented 2 legged oauth to secure these (https://github.com/mozilla/treeherder-service/blob/master/treeherder/webapp/api/views.py#L23). Oauth credentials are created for each source code repository that receives test data. Worker applications posting data must authenticate individually with each repository. The client side of the oauth implementation is found here https://github.com/mozilla/treeherder-client/blob/master/thclient/client.py#L663.

There are several operations in the user interface that require data modifications that need to be stored. We used persona to provide login functionality in django and the ui for user authentication.

Another possible exploit is data mangling. Data ingested from the incoming json in some cases is displayed directly in the user interface. We're using angularjs, which provides a built in directive for html sanitization, http://docs.angularjs.org/api/ngSanitize.

Comment 5

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I think this project is important enough to also justify a more intensive pentest/review on the actual site and code. I am interested in helping with that.

Comment 6

[Jonathan Eads ( :jeads )]

This is an updated link to the server side oauth implementation https://github.com/mozilla/treeherder-service/blob/master/treeherder/webapp/api/utils.py#L97

It's implemented as a python decorator called oauth_required. Web service methods requiring oauth authentication require this decorator. Here's an example https://github.com/mozilla/treeherder-service/blob/master/treeherder/webapp/api/objectstore.py#L19

Comment 7

[Adam Muntner [:adamm] (use NEEDINFO)]

Following are the Threat Brainstorming notes from the secreview call:

=== Threat Brainstorming ===
* Web Service Endpoints
** There   are web service endpoints for posting data to. These are an obvious   exploit, we implemented 2 legged oauth to secure these (https://github.com/mozilla/treeherder-service/blob/master/treeherder/webapp/api/views.py#L23).   Oauth credentials are created for each source code repository that   receives test data. Worker applications posting data must authenticate   individually with each repository. The client side of the oauth   implementation is found here https://github.com/mozilla/treeherder-client/blob/master/thclient/client.py#L663.

* (note: 2-legged oauth skips authorization but that doesn't look like it will be an issue for this usage)
*** To test: oauth authentication implementation for correctness
        Test case: can test data be submitted with no or broken authentication?

* There   are several operations in the user interface that require data   modifications that need to be stored. We used persona to provide login   functionality in django and the ui for user authentication.
** (note: this isn't completed yet, or at least isn't on the dev instances)
API UI: 
http://treeherder-dev.allizom.org/docs/
** Another   possible exploit is data mangling. Data ingested from the incoming  json  in some cases is displayed directly in the user interface. We're  using  angularjs, which provides a built in directive for html  sanitization, http://docs.angularjs.org/api/ngSanitize.

* CSP

* SQL Quoting - does it happen on sql placeholders?
https://github.com/mozilla/treeherder-service/blob/master/treeherder/model/derived/jobs.py#L156

Comment 8

[Ed Morley [:emorley]]

Treeherder is already running in production (https://treeherder.mozilla.org/) and is intended for sheriff use by the end of this quarter, and all other dev use either then or shortly after (TBPL won't be EOLed straight away, so uptake depends on devs preference initially).

Is this review complete (in which case we can close out this bug), or is there work remaining here? :-)

Comment 9

[Yvan Boily [:ygjb][:yvan]]

Hey Adam,

If there are no actions needed here can you close out this bug?

Comment 10

[Adam Muntner [:adamm] (use NEEDINFO)]

Unless we are going to do a pentest/code review, yes. And if we are, there should probably be a new bug.