1231331 - Adb access with USB debugging disabled

Status: RESOLVED INCOMPLETE

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[Clément Lefèvre]

On nightlies from end of October at least (previous foxfooding build), even with USB debugging turned off, it is possible to access to the device through adb, and especially to open a shell on it.

Reproduced on both Flame with recent build [1] and Foxfooding Aries [2].

Flame is using userdebug usual build, so in case it would be a normal behavior of userdebug build but not intended to happen in user builds, jlorenzo did the test: it does reproduce on user builds too.

The Flame especially was totally reflashed and reset, so no profile to have strange values and so on.

[1] Nightly Flame:

uild ID               20151207150206
Build Type             userdebug
Gaia Revision          24ed003a53a81f63367e265fa7117cbe7d23d4c8
Gaia Date              2015-12-07 03:36:55
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/59bc3c7a83de7ffb611203912a7da6ad84535a5a
Gecko Version          45.0a1
Build Name             flame-userdebug 4.4.2 KOT49H eng.cltbld.20150527.043015 test-keys
Device ID              flame
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150527.043015
Firmware Date          Wed May 27 04:30:24 EDT 2015
Bootloader             L1TC000118D0

[2] Aries, foxfooding build from end of October:

Build ID               20151019104907
Build Type             userdebug
Gaia Revision          f75bd584aca0a751a5bed115800250faa8412927
Gaia Date              2015-10-19 06:39:58
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/d3e87bb40753327550143ba8ac8ee27b300cd4a9
Gecko Version          44.0a1
Build Name             aries-userdebug 4.4.2 KOT49H eng.worker.20151019.100643 test-keys
Device ID              aries
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151019.100643
Firmware Date          Mon Oct 19 10:06:51 UTC 2015
Bootloader             s1

Comment 1

[Clément Lefèvre]

[Blocking Requested - why for this release]:

Comment 2

[Stephanie Ouillon [:arroway] (needinfo me)]

Thanks for reporting this Clément, I'll investigate what's happening in the code soon.

Comment 3

[Stephanie Ouillon [:arroway] (needinfo me)]

I think the reason adb is enabled is because marionette is enabled in the build (on my foxfooding device, I've checked the marionette pref is turned off and I can't access adb).
To be consistent I guess, we should show adb is on and have the debug icon on the homescreen when marionette is enabled.

I was able to reproduce the bug on the flame Kit Kat base build from the end of last May.

Comment 4

[Clément Lefèvre]

Okay, so after Stéphanie pointed out that marionette.defaultPrefs.enabled could be responsible for this, I checked too, and by disabling it with WebIDE, it was not possible anymore to access to the phone via adb.

The only way to access to this preference is through WebIDE, it's not exposed at all from the phone itself in the developer menu.

Should it be possible to change this preference from the phone in the developer menu? If not, maybe is it possible to associate marionette preference state to the USB debugging preference, and when USB debugging is disabled, save/store marionette state and restore it when re-enabling USB debugging.

Then, in which cases this preference should be enabled by default? I guess that it never should on user builds, but should it on userdebug ones or only on engineering/debug, considering for example that foxfooding builds are userdebug ones.

Anyway, whichever the choice would be, the "bug" icon in the status bar must indeed reflect if it is possible or not to access the phone and hence consider marionette preference state.

To not have some "bad surprises" at least for foxfooders, I guess that when USB debugging is set to disabled, it really shouldn't be possible to acess the phone, so either by ignoring marionette pref state, either by changing it to match the debugging state.

Comment 5

[Paul Theriault [:pauljt] (no longer reading bugmail)]

See also https://bugzilla.mozilla.org/show_bug.cgi?id=1182952 :/ Maybe the same thing. Dave can you help?

Comment 6

[Dave Hylands [:dhylands]]

eng and userdebug builds essentially have adb enabled all the time (and marionette is enabled for both of these as well).

flame foxfooding builds used to be user builds.
aries foxfooding builds are userdebug I believe.

If marionette is disabled for userdebug builds, then the adb stuff in the UI would work.

Comment 7

[Dave Hylands [:dhylands]]

I meant to say if marionette is disabled for foxfooding builds then the adb stuff in the UI would work. 

In general marionette depends on adb, so it needs so we can't disable marionetter for user debug in general, but we might be able to disable it for foxfooding (not sure what implications that would have).

Comment 8

[Clément Lefèvre]

(In reply to Dave Hylands [:dhylands] from comment #6)
> eng and userdebug builds essentially have adb enabled all the time (and
> marionette is enabled for both of these as well).
> 
> flame foxfooding builds used to be user builds.
> aries foxfooding builds are userdebug I believe.
> 
> If marionette is disabled for userdebug builds, then the adb stuff in the UI
> would work.

Still, couple of days ago when doing some tests, jlorenzo was able to reproduce the issue on a freshly flashed Flame with a user build. We haven't checked the marionette preference at that time though as we didn't knew yet this was probably the cause of the problem.

I guess this isn't normal at all for user builds?
And what about having the marionette preference synced with the USB debugging preference as for what proposed earlier?

Comment 9

[Dave Hylands [:dhylands]]

With user builds, then adb access should follow the USB Debugging setting modulo a couple of caveats.
It turns out that USB Mass Storage, RNDIS, and adb are all controlled using the same sys.usb.config property, so if USB Mass Storage or RNDIS happen to be active, then disabling ADB will defer the actual disable until USB Mass Storage/RNDIS are also turned off (or when the USB cable is unplugged).

I'm not sure what the implications of having the marionette preference synced with the USB debugging preference would be. It certainly sounds reasonable.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Is this a relevant issue anymore with the move away from phones?

Comment 11

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Al Billings [:abillings] from comment #10)
> Is this a relevant issue anymore with the move away from phones?

It's still relevant to anyone working on flame. But I can't confirm it even still exists as I don't have a working build environment and in any case this will likely to be up to the community to fix. I really don't think its a sec-high though if thats what's bugging you - foxfooding build have many security controls weakened or disabled, and foxfooders are warned about this. 

I wonder if we should open this bug up so the community is aware of this issue (if it even still is an issue.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

FirefoxOS is no longer under active development.