755385 - [Security Review] for Developer tools: Debugger

Status: VERIFIED FIXED

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

Description

[Rob Campbell [:rc] (:robcee)]

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

   Panos Astithas, Rob Campbell

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

   The Debugger allows a user to "Debug" executing scripts in a webpage. This means they can set/clear breakpoints within source code and cause the execution to pause. They may observe and modify property values.

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

    https://wiki.mozilla.org/DevTools/Features/Debugger

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

    https://bugzilla.mozilla.org/show_bug.cgi?id=minotaur

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

    Fairly urgent. Would like review completed prior to June 6th.

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?

    yes. see https://wiki.mozilla.org/Firefox/Goals/2012Q2#Script_Tools
    "Turn on Debugger for Firefox 15"

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

        Yes. We're providing a new JavaScript Debugging engine. The server lives in Toolkit and will be part of any gecko application.

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

        no.

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

        no.

7.4  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're landing the UI for the JS Debugger. We've previously completed review of the protocol but should probably do a comprehensive review now that it's nearing completion.

Other items of interest may be the Remote Debugger (https://bugzilla.mozilla.org/show_bug.cgi?id=747429) and Chrome Debugger.

8.   Desired Date of review (if known from ) and whom to invite.

     Thursday, May 24th, 2012. Panos Astithas, Rob Campbell, Jim Blandy

Comment 1

[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=110484-110483&pstat=AC&exInvId=110484-179672&useInstance=1&instStartTime=1337878800000&instDuration=3600000

Comment 2

[Jim Blandy :jimb]

(In reply to Rob Campbell [:rc] (:robcee) from comment #0)
> 2. Please provide a short description of the feature / application (e.g.
> problem solved, use cases, etc.):
> 
>    The Debugger allows a user to "Debug" executing scripts in a webpage.
> This means they can set/clear breakpoints within source code and cause the
> execution to pause. They may observe and modify property values.

Is it worth pointing out that the user can also cause content code to run; so they have all the power content code does.

Comment 3

[Tanvi Vyas[:tanvi]]

What version of ff did remote debugging landed in?  Are we okay with releasing remote debugging without authentication (bug 755513)?  What does the UI to turn it on remote debugging look like?

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Tanvi Vyas from comment #3)
> What version of ff did remote debugging landed in?  Are we okay with
> releasing remote debugging without authentication (bug 755513)?  What does
> the UI to turn it on remote debugging look like?

Remote debugging landed during this cycle, so it will ship with Firefox 15, assuming we enable it by default. You need to flip an additional preference to enable the "Remote Debugger" menu item, devtools.debugger.remote-enabled. Longer-term we are planning to get rid of the menu item in favor of a debugger toolbar button, labeled "Connect" (or something similar) that prompts for the remote address and displays the remote debugger window.

Regarding the authentication issue, my vote is not blocking the remote debugger on it. Other browsers don't have it either, and it's overengineering for the most common use case of localhost connections (as is common in the Android world with debugging over USB). We could even think about disabling the, admittedly very convenient, debugging-over-WiFi feature, if we consider it critically important.

Comment 5

[Tanvi Vyas[:tanvi]]

Okay, makes sense.  We can discuss whether or not we should disable debugging-over-WiFi feature (or add an extra about:config pref for that until authentication is added) in tomorrow's security review.  I guess it also depends on how long we will have debugging-over-WiFi without authentication (i.e. what ff version will the authentication piece land in).

Comment 6

[Rob Campbell [:rc] (:robcee)]

Is there any action left to on this? We would like to close our debugger meta bug.

Comment 7

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

The review page shows this
* Who :: What :: By when
* panos:: Dialog for remote connections::before Aurora (ff 15)
* panos:: Add encryption for a future release (ZTRP - bug 755513 ).  :?? (https://wiki.mozilla.org/DOMCryptInternalAPI)? We need to figure this out separately.
* panos::2 prefs - allow debugging via localhost / allow debugging via anywhere. Both disabled by default.::before Aurora (ff 15)

I am in the process of catching up on backed up bug filings for sec reviews today. If we have bugs on these already they should be blocking this bug so we know they are done and can resolve this bug. Other wise I will file bugs, blocking this bug for these items.

Comment 8

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

since I see no dependent bugs being added I will file the necessary blocking bugs for this

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Curtis Koenig [:curtisk] from comment #8)
> since I see no dependent bugs being added I will file the necessary blocking
> bugs for this

Sorry, I forgot to respond to this yesterday. I'll followup on the blocking bugs.

Comment 10

[Mark Goodwin [:mgoodwin]]

I'm happy for this to be closed; we have separate bugs for everything that still needs to be tracked. Curtis, do you agree?

Comment 11

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

Our standard flow is for any bugs we brought up to block this bug and this not to be closed until we resolve those bugs. I can add blockers for the duplicates that are identified. The blocking bugs are considered action items from the review and the review is not closed until they are resolved.

Comment 12

[Panos Astithas (he/him) [:past] (please ni?)]

I think we agreed that bug 755513 (authentication and encryption) will not be a short-term goal (i.e. we'll ship without it and add it later).

Comment 13

[Tanvi Vyas[:tanvi]]

(In reply to Panos Astithas [:past] from comment #12)
> I think we agreed that bug 755513 (authentication and encryption) will not
> be a short-term goal (i.e. we'll ship without it and add it later).

Yes, we did agree to that.  But how should we track 755513?  Through bug https://bugzilla.mozilla.org/show_bug.cgi?id=750736?

Comment 14

[Mark Goodwin [:mgoodwin]]

(In reply to Tanvi Vyas from comment #13)
> Yes, we did agree to that.  But how should we track 755513?  Through bug
> https://bugzilla.mozilla.org/show_bug.cgi?id=750736?

That works for me.