935229 - Can't connect app manager to LG d300 (timeout)

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect, P1)

Description

[Paul Rouget [:paul]]

LG d300:
Boot2Gecko 1.2.0.0-prerelease
26.0a2
20131022004000
leo/1.2.0/nightly

ADB runs as a non-root user.

Comment 1

[Paul Rouget [:paul]]

Connecting results into a timeout.
adb shell tells us that the socket has been created.

Comment 2

[Paul Rouget [:paul]]

We're not 100% sure that the app manager works correctly with non rooted phones (as in: adb runs as a non-root user). The first step here is to confirm that this works.

Comment 3

[Paul Rouget [:paul]]

To build B2G for Keon and setup a non-rooted environment:
> just use a Keon build tree, |export VARIANT=user| in your
> .userconfig and rebuild a boot.img

(from Alexandre)

Comment 4

[:gerard-majax]

Building should be as simple as |./build.sh out/target/product/keon/boot.img|

Comment 5

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

If there is indeed a permissions issue with the credentials adb runs with and the /data/local/debugger-socket file system path, one idea is to try using the abstract namespace like this:

adb forward tcp:6000 localabstract:debugger-socket

This would hopefully only need to use "localabstract:debugger-socket" as the value for the "devtools.debugger.unix-domain-socket" pref. I plan on trying that for Fennec in bug 916777, but I got sidetracked.

Another solution of course would be to continue using sockets in the file system namespace, but in a more permissive path.

Comment 6

[Paul Rouget [:paul]]

Might be relevant: http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/main.js#429

Comment 7

[Christina.X.Wang.-ND]

My results in App Manager are consistent with yours.

Additionally, I tried the old Firefox OS Simulator, which times out when trying to push to the LG even though it detects the device as connected.

Comment 8

[Paul Rouget [:paul]]

(In reply to Christina.X.Wang.-ND from comment #7)
> My results in App Manager are consistent with yours.
> 
> Additionally, I tried the old Firefox OS Simulator, which times out when
> trying to push to the LG even though it detects the device as connected.

Christina, which version of Firefox OS do you use?

Comment 9

[Christina.X.Wang.-ND]

I'm on the latest downloadable versions of nightly and Firefox OS Sim.  I can push apps to other devices, the ZTE and Allcatel, so I think it's a device specific issue.

Comment 10

[Paul Rouget [:paul]]

Christina, can you give me the exact revision used with the LG?

Comment 11

[Christina.X.Wang.-ND]

Hi Paul, I'm out of town, but will get back to you with the revision as soon as I can Monday.  Thanks!

Comment 12

[:Harald Kirschner :digitarald]

Paul, any update on this one? We will flash LGs today with a newer build and hope that this is fixed.

Comment 13

[Christina.X.Wang.-ND]

At this point, should I flash the LG with the older version of Firefox OS that worked with the Nexus?  Or a newer version you guys have?  Flashing would reset all the defaults, so I could get more info.

Comment 14

[:Harald Kirschner :digitarald]

As the cause of this bug is still unclear we are testing if flashing a new gacko/gaia version fixes this; but so far the best option seems to be to ship new replacement phones.

Comment 15

[Christina.X.Wang.-ND]

Are you able to replicate the bug on your end?

Comment 16

[Paul Rouget [:paul]]

Still working on this. This will take some time to figure out what's going on (I'm waiting for a phone and I'm trying to replicate the ADB running context on a flashable device).

Once fixed, we will require a 1.2 uplift.

Comment 17

[Paul Rouget [:paul]]

Attachment: non-root boot.img

I can reproduce the issue with a locked geekphones.

I rebuild a boot.img file that doesn't run adb as root.

It's not specific to the LG phone, but a broader issue.

Looking at how to fix that...

Comment 18

[Paul Rouget [:paul]]

Attachment: 600 -> 666

As expected, it was a permission issue.

Tested on a non-root Keon.

600 -> connection fails
666 -> success

Comment 19

[Paul Rouget [:paul]]

Adb permissions:

uid=2000(shell) gid=2000(shell) groups=1003(graphics),1004(input),1007(log),1009(mount),1011(adb),1015(sdcard_rw),3001(net_bt_admin),3002(net_bt),3003(inet),3006(net_bw_stats)

Comment 20

[Paul Rouget [:paul]]

Do we want an extra security review?

Comment 21

[Paul Rouget [:paul]]

(In reply to Paul Rouget [:paul] from comment #20)
> Do we want an extra security review?

I guess if I ask that mean we might need one…

Comment 22

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

Comment on attachment 830805 [details] [diff] [review]
600 -> 666

Review of attachment 830805 [details] [diff] [review]:
-----------------------------------------------------------------

This doesn't seem right, we are now allowing any process on the device (or computer for desktop debugging) to access the remote protocol socket. This is not strictly worse than what we had before with the TCP socket (any process could connect to it), but it is less secure from what we have now. Wouldn't a different path either on the file system or in the abstract namespace work better?

This is primarily a security issue, so if our security team decides that this is OK to do, then I'll r+ it.

Comment 23

[Paul Rouget [:paul]]

(In reply to Panos Astithas [:past] from comment #22)
> Comment on attachment 830805 [details] [diff] [review]
> 600 -> 666
> 
> Review of attachment 830805 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This doesn't seem right, we are now allowing any process on the device (or
> computer for desktop debugging) to access the remote protocol socket. This
> is not strictly worse than what we had before with the TCP socket (any
> process could connect to it), but it is less secure from what we have now.
> Wouldn't a different path either on the file system or in the abstract
> namespace work better?
> 
> This is primarily a security issue, so if our security team decides that
> this is OK to do, then I'll r+ it.

I haven't followed the TCP vs. unixsocket thing… but I thought that the reason of not using a TCP socket is that apps can access the TCP socket. The unix socket is not accessible not because it's 600, but because it's a file, and afaik, apps can't access files.

Comment 24

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

This seems ok to me for Firefox OS. As per the previous comment, there is no direct access to files on Firefox OS, so changing the permission doesn't change the threat model much. A user could install an untrusted binary on the phone to try to attack the debugger service directly, but they could already do this via ADB forward, which afaik just forwards all traffic anyways. 

My only thought is can we just keep the code the same for desktop though (is this code even used on desktop) ? Seems like there is no point reducing desktop security to fix a Firefox OS bug. I can't really think of an attack scenario though, so not really a show stopper if this is too hard to implement.

Comment 25

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

Comment on attachment 830805 [details] [diff] [review]
600 -> 666

Review of attachment 830805 [details] [diff] [review]:
-----------------------------------------------------------------

r=me then, if we are OK security-wise. This code is shared by all embeddings (desktop, Fennec, B2G), but until we convert the others to use domain sockets, this change will only affect Firefox OS. I'm planning to do the same for Fennec in bug 916777, but we haven't considered doing this for desktop yet, and it is unclear if it would be a good tradeoff to use it on Linux and OS X only, compared to keeping all desktop platforms consistent.

Comment 26

[:Harald Kirschner :digitarald]

Paul, can you confirm that we should wait for this patch to land before flashing a new set of LGs for the partner. Thanks!

Comment 27

[Paul Rouget [:paul]]

(In reply to Harald Kirschner :digitarald from comment #26)
> Paul, can you confirm that we should wait for this patch to land before
> flashing a new set of LGs for the partner. Thanks!

Yes - and please wait until this had been uplifted to FxOS 1.2.

Comment 28

[Paul Rouget [:paul]]

Comment on attachment 830805 [details] [diff] [review]
600 -> 666

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new feature (app manager)
User impact if declined: non-rooted phones are not debuggable
Testing completed (on m-c, etc.): local
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 29

[Lukas Blakk [:lsblakk] use ?needinfo]

Waiting for this to land on central before taking it up to branches and marking koi+ so we ensure the landing (or merge from beta) happens on b2g26 branch for 1.2

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/f7dd2a78334b

Comment 31

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/f7dd2a78334b

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/42014e107777
https://hg.mozilla.org/releases/mozilla-beta/rev/2a1f51ff962b

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/2a1f51ff962b

Comment 34

[Paul Rouget [:paul]]

Christian or Harald, can you confirm that it works for you now?

Comment 35

[:Harald Kirschner :digitarald]

Success! Christina and Michael were both able to push with the new version flashed to an LG Fireweb.

Comment 36

[Christina.X.Wang.-ND]

Confirmed!  Yes, I am able to push apps to the new LG.  Thanks, all!