Closed Bug 935229 Opened 8 years ago Closed 8 years ago

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

Categories

(DevTools Graveyard :: WebIDE, defect, P1)

x86
All
defect

Tracking

(blocking-b2g:koi+, firefox26 fixed, firefox27 fixed, firefox28 fixed, b2g-v1.2 fixed)

RESOLVED FIXED
Firefox 28
blocking-b2g koi+
Tracking Status
firefox26 --- fixed
firefox27 --- fixed
firefox28 --- fixed
b2g-v1.2 --- fixed

People

(Reporter: paul, Assigned: paul)

References

(Blocks 1 open bug)

Details

(Whiteboard: [games:p1][needs-coverage])

Attachments

(2 files)

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

ADB runs as a non-root user.
Connecting results into a timeout.
adb shell tells us that the socket has been created.
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.
Assignee: nobody → paul
Status: NEW → ASSIGNED
Summary: Can't connect to LG d300 → Can't connect app manager to LG d300 (timeout)
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)
Building should be as simple as |./build.sh out/target/product/keon/boot.img|
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.
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.
(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?
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.
Christina, can you give me the exact revision used with the LG?
Hi Paul, I'm out of town, but will get back to you with the revision as soon as I can Monday.  Thanks!
Paul, any update on this one? We will flash LGs today with a newer build and hope that this is fixed.
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.
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.
Are you able to replicate the bug on your end?
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.
Attached file 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...
Attached patch 600 -> 666Splinter Review
As expected, it was a permission issue.

Tested on a non-root Keon.

600 -> connection fails
666 -> success
Attachment #830805 - Flags: review?(past)
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)
Do we want an extra security review?
(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…
Flags: sec-review?
Flags: sec-review? → sec-review?(ptheriault)
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.
Attachment #830805 - Flags: review?(past)
(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.
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.
Flags: sec-review?(ptheriault) → sec-review+
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.
Attachment #830805 - Flags: review+
Keywords: checkin-needed
Paul, can you confirm that we should wait for this patch to land before flashing a new set of LGs for the partner. Thanks!
Blocks: gecko-games
Whiteboard: [games:p1]
(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 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
Attachment #830805 - Flags: approval-mozilla-beta?
Attachment #830805 - Flags: approval-mozilla-aurora?
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
blocking-b2g: --- → koi+
https://hg.mozilla.org/integration/fx-team/rev/f7dd2a78334b
Keywords: checkin-needed
Whiteboard: [games:p1] → [games:p1][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/f7dd2a78334b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [games:p1][fixed-in-fx-team] → [games:p1]
Target Milestone: --- → Firefox 28
Attachment #830805 - Flags: approval-mozilla-beta?
Attachment #830805 - Flags: approval-mozilla-beta+
Attachment #830805 - Flags: approval-mozilla-aurora?
Attachment #830805 - Flags: approval-mozilla-aurora+
Christian or Harald, can you confirm that it works for you now?
Flags: needinfo?(hkirschner)
Flags: needinfo?(Christina.X.Wang.-ND)
Success! Christina and Michael were both able to push with the new version flashed to an LG Fireweb.
Flags: needinfo?(hkirschner)
Flags: needinfo?(Christina.X.Wang.-ND)
Confirmed!  Yes, I am able to push apps to the new LG.  Thanks, all!
Whiteboard: [games:p1] → [games:p1][needs-coverage]
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.