SecReview: Script Debugger: prefs

RESOLVED FIXED in Firefox 15

Status

()

Firefox
Developer Tools: Debugger
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: curtisk, Assigned: past)

Tracking

Trunk
Firefox 16
Points:
---

Firefox Tracking Flags

(firefox15 fixed)

Details

(Whiteboard: [action item], URL)

Attachments

(1 attachment, 1 obsolete attachment)

SecReview tracking bug
2 prefs - allow debugging via localhost / allow debugging via anywhere. Both disabled by default
(In reply to Curtis Koenig [:curtisk] from comment #0)
> SecReview tracking bug
> 2 prefs - allow debugging via localhost / allow debugging via anywhere. Both
> disabled by default

The way I remember this requirement is that we wanted one additional pref to the one added (or rather enforced) in bug 758696: devtools.debugger.remote-enabled, that allows connections through the remote protocol, over the loopback or other interfaces. The new pref, which I will suggest we name devtools.debugger.local-only, will force the debugger server to bind to the loopback interface if true, instead of all the interfaces, as it currently does.
Status: NEW → ASSIGNED
(In reply to Panos Astithas [:past] from comment #1)
> The way I remember this requirement is that we wanted one additional pref to
> the one added (or rather enforced) in bug 758696:
> devtools.debugger.remote-enabled, that allows connections through the remote
> protocol, over the loopback or other interfaces.

This is my recollection also.
Created attachment 633100 [details] [diff] [review]
Working patch

This patch works as advertised in my tests on a B2G phone. I have used a conservative default of having the listener bound to the loopback interface only. This is in accordance with the default we picked in bug 758696 of 'Cancel', for the users who don't know any better. If you feel that the usability gains from binding to all interfaces by default (debugging over WiFi), trump any security concerns, let me know.
Attachment #633100 - Flags: review?(rcampbell)
Comment on attachment 633100 [details] [diff] [review]
Working patch

Passing the review over to Dave, since Rob is on PTO.
Attachment #633100 - Flags: review?(rcampbell) → review?(dcamp)
Created attachment 634463 [details] [diff] [review]
Patch v2

Dave made a good point that we don't need to provide callers with the ability to limit binding on the loopback interface, since nobody seems to want that.
Attachment #633100 - Attachment is obsolete: true
Attachment #633100 - Flags: review?(dcamp)
Attachment #634463 - Flags: review?(dcamp)

Updated

5 years ago
Attachment #634463 - Flags: review?(dcamp) → review+
Comment on attachment 634463 [details] [diff] [review]
Patch v2

Fennec, B2G and Marionette patches are one-liner cleanups, but let's keep this by the book.
Attachment #634463 - Flags: review?(mark.finkle)
Attachment #634463 - Flags: review?(jgriffin)
Attachment #634463 - Flags: review?(21)
Attachment #634463 - Flags: review?(jgriffin) → review+
Attachment #634463 - Flags: review?(mark.finkle) → review+
Attachment #634463 - Flags: review?(21) → review+
Whiteboard: [action item] → [action item][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/576e10abf824
Whiteboard: [action item][land-in-fx-team] → [action item][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/576e10abf824

There is no Target Milestone field in this bug, but this was fixed in Firefox 16.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [action item][fixed-in-fx-team] → [action item]
Curtis, can I change the product/component of this bug to Developer Tools: Debugger? I can't seem to be able to nominate the patch for aurora uplift as it is.
Panos - sure, as long as we can keep blocking the sec review bug as well I am fine with that, I use these action items as blockers of the review bug so I can track progress and know when things are done.
OK, thanks.
Component: Security Assurance → Developer Tools: Debugger
Product: mozilla.org → Firefox
QA Contact: security-assurance → developer.tools.debugger
Target Milestone: --- → Firefox 16
Version: other → Trunk
Comment on attachment 634463 [details] [diff] [review]
Patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New feature
User impact if declined: No visible user impact, but security review has deemed this an important protection against potential abuse
Testing completed (on m-c, etc.): On m-c and fx-team
Risk to taking this patch (and alternatives if risky): Pretty trivial patch on a developer-only feature
String or UUID changes made by this patch: none
Attachment #634463 - Flags: approval-mozilla-aurora?
Comment on attachment 634463 [details] [diff] [review]
Patch v2

[Triage Comment]
Trivial patch, the security team considers this important, approved for Aurora 15.
Attachment #634463 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/e6b0d54297e6
status-firefox15: --- → fixed
You need to log in before you can comment on or make changes to this bug.