935707 - SecReview: Java BrowserID crypto library for Android services projects

Status: VERIFIED FIXED

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

Description

[Mark Goodwin [:mgoodwin]]

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

>2) Please provide a short description of the feature / application (e.g. problem solved, use cases, etc.):
>4) Does this request block another bug? If so, please indicate the bug number
Yes, bug 799734

>5) This review will be scheduled amongst other requested reviews. What is the urgency or needed completion date of this review?
>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?
>7) Please answer the following few questions: (Note: If you are asked to describe anything, 1-2 sentences shall suffice.)
>7a) Does this feature or code change affect Firefox, Thunderbird or any product or service the Mozilla ships to end users?
Yes, Firefox (android)

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

Comment 1

[Mark Goodwin [:mgoodwin]]

Nick, please can you assist with the unanswered questions in comment 0?

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Mark Goodwin [:mgoodwin] from comment #0)
> >1) Who is/are the point of contact(s) for this review?
> Nick Alexander
> 
> >2) Please provide a short description of the feature / application (e.g. problem solved, use cases, etc.):

This is an HTTP client for talking to the Mozilla Services token server.  It is not user exposed, but instead shuffles tokens of one type (Browser ID assertions) for another type (token server tokens).

> >4) Does this request block another bug? If so, please indicate the bug number
> Yes, bug 799734
> 
> >5) This review will be scheduled amongst other requested reviews. What is the urgency or needed completion date of this review?

Early Q1? We're hoping to ship end of Q1.

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

This supports Cloud Services only goal of shipping FxAccount on all major platforms.

> >7) Please answer the following few questions: (Note: If you are asked to describe anything, 1-2 sentences shall suffice.)
> >7a) Does this feature or code change affect Firefox, Thunderbird or any product or service the Mozilla ships to end users?
> Yes, Firefox (android)
> 
> >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):

We'd like eyes, but this is not complicated.  We take tokens and get other tokens.  We could make parsing or formatting errors; we could overlook things; we could use Java APIs insecurely; but we're not introducing complicated new crypto code or flows.

Comment 3

[Nick Alexander :nalexander [he/him]]

I just remembered that the code landed in m-c doesn't include all the tests.  You can see JUnit 4 tests (that don't run on TBPL) at

https://github.com/mozilla-services/android-sync/commits/775bb0f

Comment 4

[Mark Goodwin [:mgoodwin]]

Secreview complete.