Closed
Bug 1100964
Opened 10 years ago
Closed 10 years ago
Expose a way to enable system app debugging
Categories
(Firefox OS Graveyard :: Developer Tools, defect)
Firefox OS Graveyard
Developer Tools
Tracking
(b2g-v2.2 fixed)
RESOLVED
FIXED
2.2 S2 (19dec)
Tracking | Status | |
---|---|---|
b2g-v2.2 | --- | fixed |
People
(Reporter: ochameau, Assigned: ochameau)
References
Details
(Keywords: dev-doc-complete)
Attachments
(2 files, 1 obsolete file)
To followup the various discussions on the ML:
Old first discussion about devtools and security limitations:
https://groups.google.com/d/msg/mozilla.dev.b2g/TgKygGrmsVo/bwofq8KhtqwJ
The recent one, justifying this bug:
https://groups.google.com/d/msg/mozilla.dev.b2g/skYzCcgdMVk/mpQaihO_2UsJ
We would start by adding an item in the developer menu, in order to reset the phone with the unrestricted devtools mode, where you are not limited to debug just the app you pushed, but also certified and pre-installed apps as well as chrome of apps and parent process. This mode allows hacking on Gaia instead of just single apps.
Then, we would try to introduce an item into FTU, in order to enable this mode while suggesting the developer to keep a pincode on his phone.
Assignee | ||
Comment 1•10 years ago
|
||
I started looking at how to wipe the phone.
navigator.mozPower.factoryReset has two modes "normal" and "wipe",
but, unfortunately, both of them do not show the FTU after the reset.
If we want to just introduce a "Wipe and enable unrestricted mode" button,
we would most likely have to add a new mode to factoryReset,
but I'm not sure it is reasonable?
Otherwise, we can fix wipe or normal, in order to ensure that we show the FTU on reset and then somehow expose the unrestricted toggle in some pages of the FTU.
Assignee | ||
Comment 2•10 years ago
|
||
Paul, Did you had any discussion with FTU owners and reached an agreement about introducing new UI in the FTU??
Flags: needinfo?(ptheriault)
Comment 3•10 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #1)
> Created attachment 8524635 [details] [review]
> Introduce a wipe button in settings
There is already a wipe but under More Information -> Reset Phone. I assume the point of this patch is that _this_ developer wipe button would also toggle the preference unrestricted devtools access?
>
> I started looking at how to wipe the phone.
> navigator.mozPower.factoryReset has two modes "normal" and "wipe",
> but, unfortunately, both of them do not show the FTU after the reset.
I've talked about this approach with Fabrice before, and the challenge is getting something to persist after factory reset, since that it wipes everything. But then I don't understand why FTU isn't shown (how is this state persisted?
>
> If we want to just introduce a "Wipe and enable unrestricted mode" button,
> we would most likely have to add a new mode to factoryReset,
> but I'm not sure it is reasonable?
>
> Otherwise, we can fix wipe or normal, in order to ensure that we show the
> FTU on reset and then somehow expose the unrestricted toggle in some pages
> of the FTU.
Either approach is fine with me.
(In reply to Alexandre Poirot [:ochameau] from comment #2)
> Paul, Did you had any discussion with FTU owners and reached an agreement
> about introducing new UI in the FTU??
No, not yet - adding FTU module owner to this bug. I sent an email to UX too in case they need to be involved.
Flags: needinfo?(ptheriault)
Comment 4•10 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #1)
> Created attachment 8524635 [details] [review]
> Introduce a wipe button in settings
>
> I started looking at how to wipe the phone.
> navigator.mozPower.factoryReset has two modes "normal" and "wipe",
> but, unfortunately, both of them do not show the FTU after the reset.
Are you sure? I just tested this on 2.1,using the existing button in the settings app, and I was shown FTU after factory reset?
Flags: needinfo?(poirot.alex)
Assignee | ||
Comment 5•10 years ago
|
||
I'm all but sure it is a reasonable way to implement
a "reset and activate unrestricted devtools" feature on gecko side.
As I wasn't sure it was possible to introduce a new mode to mozPower.factoryReset,
I worked around that by using a setting to pass additional commands
to execute after reset reboot.
But that works...
Also here, I'm using "wipe" feature, that also deletes sdcard.
I'm not sure we want to wipe, but just do the "regular reset".
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Paul Theriault [:pauljt] from comment #4)
> (In reply to Alexandre Poirot [:ochameau] from comment #1)
> > Created attachment 8524635 [details] [review]
> > Introduce a wipe button in settings
> >
> > I started looking at how to wipe the phone.
> > navigator.mozPower.factoryReset has two modes "normal" and "wipe",
> > but, unfortunately, both of them do not show the FTU after the reset.
>
> Are you sure? I just tested this on 2.1,using the existing button in the
> settings app, and I was shown FTU after factory reset?
That may be because of engineering build or something... I don't know why I don't get the FTU again.
Flags: needinfo?(poirot.alex)
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8525388 [details] [diff] [review]
gecko patch v1
Fabrice, any feeling to share about this patch?
I'm trying to implement a "reset and activate devtools unrestricted mode" button in the settings apps.
This patch is a workaround to prevent introducing a new mode in mozPower.factoryReset. I can also just introduce mozPower.factoryReset("root"), or something, if you think that's better option.
Attachment #8525388 -
Flags: feedback?(fabrice)
Comment 8•10 years ago
|
||
Comment on attachment 8525388 [details] [diff] [review]
gecko patch v1
Review of attachment 8525388 [details] [diff] [review]:
-----------------------------------------------------------------
When does the preference change happen ProcessGlobal.js? Before or after the wipe - after would be better I think? Keep in mind that an attacker can sideload an app with the power permission, and call factoryReset too. This is fine, as long as the wipe happens first (otherwise the attacker could initiate the wipe, then pull the battery to flip the pref without doing it.)
::: b2g/components/RecoveryService.js
@@ +98,5 @@
> + try {
> + let cmds = Services.prefs.getCharPref("b2g.wipe.additional-commands");
> + log("Pass additional commands: " + cmds);
> + text += cmds;
> + } catch(e) {}
Just be careful here - hijacking recovery service is pretty common way to root a device. This looks ok at a glance though (the commands are coded as a preference, and without root you can't change it right?)
Comment 9•10 years ago
|
||
Comment on attachment 8525388 [details] [diff] [review]
gecko patch v1
Review of attachment 8525388 [details] [diff] [review]:
-----------------------------------------------------------------
I don't think you should piggyback on 'wipe', because it's erasing the sd-card and you will make many people unhappy... I don't see any special issue with using a new mode instead. I'll let you bikeshed the naming though ;)
Attachment #8525388 -
Flags: feedback?(fabrice) → feedback-
Assignee | ||
Comment 10•10 years ago
|
||
Ok, so what about this?
navigator.mozPower.factoryReset('root');
I'm no longer hacking the 'wipe' command, but I'm still using the same post-reset file helper.
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0bc0270c6855
Attachment #8525388 -
Attachment is obsolete: true
Attachment #8529080 -
Flags: review?(fabrice)
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to Paul Theriault [:pauljt] from comment #8)
> Comment on attachment 8525388 [details] [diff] [review]
> gecko patch v1
>
> Review of attachment 8525388 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> When does the preference change happen ProcessGlobal.js? Before or after
> the wipe - after would be better I think? Keep in mind that an attacker can
> sideload an app with the power permission, and call factoryReset too. This
> is fine, as long as the wipe happens first (otherwise the attacker could
> initiate the wipe, then pull the battery to flip the pref without doing it.)
Reset happen first, we read the file on the first launch after reset.
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8524635 [details] [review]
Introduce a wipe button in settings
I've updated the gaia patch accordingly.
The existing button in Settings > More information isn't enough, as we need a dedicated one, just for devtools, that would reset AND root the phone.
Any idea who should review this?
Updated•10 years ago
|
Attachment #8529080 -
Flags: review?(fabrice) → review+
Comment 14•10 years ago
|
||
needs dom peer review for MozPowerManager.webidl
Flags: needinfo?(poirot.alex)
Keywords: checkin-needed
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8529080 [details] [diff] [review]
gecko patch v2
Hey Jonas, could you review the API change made to mozPower.
Flags: needinfo?(poirot.alex)
Attachment #8529080 -
Flags: review?(jonas)
Assignee | ||
Comment 16•10 years ago
|
||
Comment on attachment 8524635 [details] [review]
Introduce a wipe button in settings
Hi Arthur,
I'm adding a new item in the developer menu, a button to reset AND unlock devtools privileges. It is different from the one to reset the phone in "More information" menu.
Attachment #8524635 -
Flags: review?(arthur.chen)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → poirot.alex
Comment on attachment 8529080 [details] [diff] [review]
gecko patch v2
Review of attachment 8529080 [details] [diff] [review]:
-----------------------------------------------------------------
I don't know the code well enough to really mark r+. But the approach seems great so thumbs up from me.
Attachment #8529080 -
Flags: review?(jonas) → feedback+
Assignee | ||
Comment 18•10 years ago
|
||
I think we are good to go for the gecko patch.
Keywords: checkin-needed
Comment 19•10 years ago
|
||
Keywords: checkin-needed → leave-open
Comment 20•10 years ago
|
||
Comment 21•10 years ago
|
||
Comment on attachment 8524635 [details] [review]
Introduce a wipe button in settings
Overall it looks good, f=me. Suggest to display the dialog using the DialogService module.
Attachment #8524635 -
Flags: review?(arthur.chen) → feedback+
Assignee | ||
Comment 22•10 years ago
|
||
Comment on attachment 8524635 [details] [review]
Introduce a wipe button in settings
Arthur, I rebased, fixed the locales and switched to DialogService
Attachment #8524635 -
Flags: review?(arthur.chen)
Comment 23•10 years ago
|
||
Comment on attachment 8524635 [details] [review]
Introduce a wipe button in settings
Great work! r=me with the comments addressed, thanks.
Attachment #8524635 -
Flags: review?(arthur.chen) → review+
Assignee | ||
Comment 24•10 years ago
|
||
Oh I understand now why we have all these tv/tablet specific strings! (I was really wondering why while writing the patch...)
Patch updated, waiting for try results.
Assignee | ||
Comment 25•10 years ago
|
||
Checking needed for the gaia patch, there is just a calendar intermittent.
Keywords: leave-open → checkin-needed
Comment 26•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-b2g-v2.2:
--- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S2 (19dec)
Updated•10 years ago
|
Keywords: dev-doc-needed
Updated•10 years ago
|
Blocks: fxos-dev-papercuts
This option has been documented[1] on the Developer Settings page, but there may be other places that it should be included as well.
[1]: https://developer.mozilla.org/en-US/Firefox_OS/Debugging/Developer_settings#Reset_and_enable_full_DevTools
Comment 28•10 years ago
|
||
Thanks jryans!
Cmills, is there anywhere else this should be documented?
Flags: needinfo?(cmills)
Comment 29•10 years ago
|
||
I think jryans has got the settings page entry perfect, although I think the linked section needs some updates:
https://developer.mozilla.org/en-US/docs/Tools/WebIDE#Unrestricted_app_debugging_%28including_certified_apps.2C_main_process.2C_etc.%29
When you select "Reset and enable full DevTools", how does this relate to the linked section? Does it do the same thing as the WebIDE "request higher privileges" setting?
Updated•10 years ago
|
Flags: needinfo?(cmills)
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #29)
> I think jryans has got the settings page entry perfect, although I think the
> linked section needs some updates:
>
> https://developer.mozilla.org/en-US/docs/Tools/
> WebIDE#Unrestricted_app_debugging_%28including_certified_apps.
> 2C_main_process.2C_etc.%29
>
> When you select "Reset and enable full DevTools", how does this relate to
> the linked section? Does it do the same thing as the WebIDE "request higher
> privileges" setting?
They both flip the same pref to enable full DevTools access. WebIDE does not reset the device (delete all user data) because the device must already be rooted to use the WebIDE button successfully (so it can't be used by most with production devices, since they are not rooted).
The Settings app button "Reset and enable full DevTools" will work for any user, whether they have a rooted device or not, but it also wipes user data to ensure it can't be accessed by attacker trivially flipping a toggle switch.
Comment 31•10 years ago
|
||
Cool, thanks for the explanation. I've added a note to the WebIDE page to explain this and link back to the developer setting.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•