Closed
Bug 836103
Opened 11 years ago
Closed 11 years ago
Enable/disable ADB when changing the "remote debugging" setting in the Settings App
Categories
(Firefox OS Graveyard :: General, defect)
Firefox OS Graveyard
General
Tracking
(blocking-b2g:tef+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed, b2g18-v1.0.0 fixed, b2g18-v1.0.1 fixed)
People
(Reporter: pauljt, Assigned: fabrice)
References
Details
(Keywords: late-l10n)
Attachments
(1 file, 1 obsolete file)
3.86 KB,
patch
|
dhylands
:
review+
|
Details | Diff | Splinter Review |
ADB should be disabled by default in productions as it represents significant security risk, even if we run adb with lower risks as discussed in 810092. However currently there is no setting in the UI to enable adb, so if we disable it by default, there is no way to ever turn it back on (afaik). As discussed on IRC, we could link the existing "remote debugging" option under developer settings to enabled adb, in addition to enabling the Remote Debugger Service. This could be enabled by having this setting change the android system property assocaited with adb.
Reporter | ||
Updated•11 years ago
|
Priority: P1 → --
Summary: Provide a setting in the Settings App to enable/disable ADB → Enable/disable ADB when changing the "remote debugging" setting in the Settings App
Comment 1•11 years ago
|
||
What is the expected settings for the system props? I think that the settings we want are ro.secure=1 ro.debuggable=0|1 depending on "remote debugging setting" persist.sys.usb.config=mass_storage,adb Looking at shell.js, the settings can be manipulated with libcutils.property_set(opt, value) and put into the RemoteDebugger handleEvent http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#888
Assignee | ||
Comment 2•11 years ago
|
||
Doesn't work unfortunately... I always get adb access.
Comment 3•11 years ago
|
||
So as far as I'm aware, you can't change ro.xxx settings. 2018 >adb shell root@android:/ # getprop ro.secure 0 root@android:/ # setprop ro.secure 1 root@android:/ # getprop ro.secure 0 To disable adb, you need to remove the ,adb from the persist.sys.usb.config The correct way would be to read the current setting, remove ,adb and write it out again. To enable, read the setting, add ,adb if it isn't already there and write it back out. Manipulating the sys.usb.config can be tricky, general, as the string HAS to match one of the strings from init.qcom.rc. Fortunately, for adb, it's always at the end of the string so we can always append ,adb or remove it
Assignee | ||
Comment 4•11 years ago
|
||
Thanks for the info Dave! that seems to work fine now.
Assignee: nobody → fabrice
Attachment #707933 -
Attachment is obsolete: true
Attachment #707997 -
Flags: review?(dhylands)
Comment 5•11 years ago
|
||
Comment on attachment 707997 [details] [diff] [review] patch Review of attachment 707997 [details] [diff] [review]: ----------------------------------------------------------------- r=me with dumps removed and no trailing comma on the value stored in persist.sys.usb.config ::: b2g/chrome/content/settings.js @@ +199,5 @@ > +#ifdef MOZ_WIDGET_GONK > + // Configure adb. > + try { > + let current = libcutils.property_get("persist.sys.usb.config"); > + dump("Current value: " + current); Everybody else complains when I leave dump statements in... @@ +200,5 @@ > + // Configure adb. > + try { > + let current = libcutils.property_get("persist.sys.usb.config"); > + dump("Current value: " + current); > + let prefix = current.split(",adb"); So when I tried this in firefox scratchpad "mass_storage,adb".split(",adb"); it leaves a trailing comma on the string, which I think is bad (it may be disabling adb, but I think you'll find its not enabling mass_storage). Wouldn't it be better to use: let prefix = current.replace(/,adb/, "");
Attachment #707997 -
Flags: review?(dhylands) → review+
Assignee | ||
Comment 6•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7cf999a95467
blocking-b2g: --- → tef?
Comment 7•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7cf999a95467
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 8•11 years ago
|
||
You probably need to deal with the startup condition as well (i.e. enabled/disable adb based on the setting at startup). Or maybe the settings stuff generates a startup event? I can't remember.
Updated•11 years ago
|
blocking-b2g: tef? → tef+
Assignee | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/625a955ec8a8
Comment 10•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_0/rev/e989bd06ca2d
status-b2g18:
--- → fixed
status-b2g18-v1.0.0:
--- → fixed
status-firefox19:
--- → wontfix
status-firefox20:
--- → wontfix
status-firefox21:
--- → fixed
Target Milestone: --- → B2G C4 (2jan on)
Comment 12•11 years ago
|
||
Verified fixed in 2013-02-06-11-05-09 pvt nightly b2g18 build
Status: RESOLVED → VERIFIED
Updated•11 years ago
|
status-b2g18-v1.0.1:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•