Closed Bug 832000 Opened 11 years ago Closed 11 years ago

Security audit of debugger interface for installing apps

Categories

(Core :: General, defect)

Product:
Core
Component:
General
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla26
Project Flags:
blocking-b2g -
Tracking Flags:
Tracking Status
b2g18 + ---

People

(Reporter: cjones, Assigned: ochameau)

References

Details

(Whiteboard: [sg:high])

Attachments

(1 file, 3 obsolete files)

Rebased patch
6.48 KB, patch
ochameau
: review+
Details | Diff | Splinter Review 
This is implemented by having the user explicitly enable debugging support, then we open a server socket for the debugger on which we receive commands.  The user has to adb forward a port to interact with the server, IIRC.

My main question is: can a compromised content process connect to the debugger socket and install arbitrary privileged apps?  If not, how do we defend against that?
If the content process has the ability to make TCP connections to the local machine on a port of its choice, then, yes, it could connect to the debugger server. If chrome debugging is not enabled, it should not be able to execute code with chrome privileges. But it could certainly connect to any content process the debugger could, and make it do whatever it wanted. So it could use other content processes' hardware access privileges, say.
Can we bind the server to a UNIX socket with permissions set to

 rw-rw---- shell.shell

instead?  Otherwise the server enables a rather serious security threat ...
If ADB can forward to such a Unix-domain socket, that would be fine. We'll need a way to create an nsIInputStream and an nsIAsyncOutputStream for Unix-domain sockets.
It looks like ADB can do that: 

adb forward <local> <remote> - forward socket connections
                                 forward specs are one of: 
                                   tcp:<port>
                                   localabstract:<unix domain socket name>
                                   localreserved:<unix domain socket name>
                                   localfilesystem:<unix domain socket name>
                                   dev:<character device name>
                                   jdwp:<process pid> (remote only)
Yes, what fabrice said.
As things stand, we have an sg-high vulnerability that users have to opt-in to create.  So I would soft-block on this, but would really really like a fix.
blocking-b2g: --- → -
tracking-b2g18: --- → +
Whiteboard: [sg:high]
Um, scary thought: the server only binds to localhost, right?
Group: core-security
For context, here are the links to the two security reviews we've had so far:
- https://wiki.mozilla.org/Security/Reviews/Firefox/RemoteDebug
- bug 755385

(In reply to Chris Jones [:cjones] [:warhammer] from comment #7)
> Um, scary thought: the server only binds to localhost, right?

The server only binds to localhost by default, but can be configured to bind to all IP addresses if the pref devtools.debugger.force-local is set to false. There is no UI for toggling this pref, but it is being used by Marionette and it has been mentioned in a couple of blog posts.

(In reply to Jim Blandy :jimb from comment #3)
> If ADB can forward to such a Unix-domain socket, that would be fine. We'll
> need a way to create an nsIInputStream and an nsIAsyncOutputStream for
> Unix-domain sockets.

We need to implement this in the platform, right? I can't see any way to do it with the APIs we have right now.
OK, thanks.
Group: core-security
Do we allow privileged apps with tcpsocket permissions to connect to localhost?
(In reply to Chris Jones [:cjones] [:warhammer] from comment #10)
> Do we allow privileged apps with tcpsocket permissions to connect to
> localhost?

Yes.
So that's a pretty serious privilege escalation then.  Let's please use a UNIX socket! :)
(In reply to Chris Jones [:cjones] [:warhammer] from comment #0)
> My main question is: can a compromised content process connect to the
> debugger socket and install arbitrary privileged apps?  If not, how do we
> defend against that?

[...]

> So that's a pretty serious privilege escalation then.
> Let's please use a UNIX socket! :)

I am not sure I understand the issue, but that won't stop me from commenting:

The debugger must be listening on an network interface exposed to the outside world so that a desktop computer can connect to it. So, how would a UNIX socket even work?

Also, what does it matter if a compromised content process is doing this, vs. any computer on the internet connecting to the debugger socket and installing arbitrary privileged apps?

I thought that we were going to have one security prompt per network connection, that requires the user to consent to the use of the debugger before we execute (ideally, before we parse) the first debugging command. Did the prompt get implemented? And, if so, is cjones concerned that the prompt is not sufficient protection against a compromised child process, because the child process may be able to do a clickjacking or similar attack?
(In reply to Brian Smith (:bsmith) from comment #13)
> So, how would a UNIX socket even work?

It seems I missed a chunk of bugmail. Never mind explaining that this would be done with adb port forwarding.
(In reply to Brian Smith (:bsmith) from comment #13)
> The debugger must be listening on an network interface exposed to the
> outside world so that a desktop computer can connect to it. So, how would a
> UNIX socket even work?

adb let us forward a unix socket on the device to a tcp port on the desktop end.

> Also, what does it matter if a compromised content process is doing this,
> vs. any computer on the internet connecting to the debugger socket and
> installing arbitrary privileged apps?

Not sure what you mean by "any computer on the internet connecting to the debugger socket".

> I thought that we were going to have one security prompt per network
> connection, that requires the user to consent to the use of the debugger
> before we execute (ideally, before we parse) the first debugging command.
> Did the prompt get implemented? And, if so, is cjones concerned that the
> prompt is not sufficient protection against a compromised child process,
> because the child process may be able to do a clickjacking or similar attack?

We have the prompt. I'll let cjones comment about the clickjacking risk.
(In reply to Fabrice Desré [:fabrice] from comment #15)
> (In reply to Brian Smith (:bsmith) from comment #13)
> > Also, what does it matter if a compromised content process is doing this,
> > vs. any computer on the internet connecting to the debugger socket and
> > installing arbitrary privileged apps?
> 
> Not sure what you mean by "any computer on the internet connecting to the
> debugger socket".

I know that the original goal of remote debugging support was to enable debugging over wifi. Perhaps that was reduced to debugging over USB to avoid the "any computer on the internet" issue because we don't think that the per-connection prompt is enough security-wise?

> We have the prompt. I'll let cjones comment about the clickjacking risk.

Given we have the prompt, I would say this is not that serious of an issue. The prompt isn't a perfect design security-wise. But, given that we want to eventually support wifi debugging, I'm not sure it is worthwhile to do this, especially if we can do bug 832436.
(In reply to Brian Smith (:bsmith) from comment #16)
> 
> I know that the original goal of remote debugging support was to enable
> debugging over wifi. Perhaps that was reduced to debugging over USB to avoid
> the "any computer on the internet" issue because we don't think that the
> per-connection prompt is enough security-wise?

Ha, I never heard about wifi access to the debugger. I always had the usb+adb model in mind, but I may just be wrong!
Indeed, remote debugging over WiFi was a feature that was specifically designed for and discussed with various product and security folks. Bug 755513 is what is blocking us from enabling it by default and promoting it broadly.

I'll note that Opera Mobile supports debugging over WiFi for a long time, and they recently(?) added UPnP discoverability, which is something we decided to stay away from, at least for v1. Chrome, on the other hand, only allows localhost connections AFAICT, and uses adb forwarding for mobile.
What makes this bug so serious is that it *doesn't* require a compromised process to exploit it.  Any app with tcpsocket permissions can escalate privileges to install arbitrary unsigned privileged applications.

I would be happy ~resolving this through bug 832436.  I don't feel particularly comfortable relying on the "Accept connection?" prompt since a malicious app could spam the user into submission, and there's no indication at all that an app client is using the debugger interface rather than an adb forward client.  Maybe I'm being too paranoid about this though.
Depends on: 832436
(In reply to Brian Smith (:bsmith) from comment #16)
> I know that the original goal of remote debugging support was to enable
> debugging over wifi. Perhaps that was reduced to debugging over USB to avoid
> the "any computer on the internet" issue because we don't think that the
> per-connection prompt is enough security-wise?

I'll readily take the blame for mistakes I actually do make, but I am unhappy to have rumors started that make me look careless. In the future, could you please ask before you guess? The story might not be as bad as your first guess; the difference might really matter to someone.

The original goal of the remote debugging protocol was to support e10s out-of-process content, web workers (by making it possible to restrict cross-thread interaction to simply exchanging strings), and devices with limited UI facilities like phones and tablets.

We do want to support debugging over wifi eventually, but that's always been in the context of first choosing some secure transport, authenticated with something lightweight and exposed to public review, perhaps like ZRTP:
http://en.wikipedia.org/wiki/ZRTP
This was discussed with Tanvi (Security Engineer) and Johnathan Nightingale (who has security background) at the first London devtools meeting. But we don't have an implementation of ZRTP available right now, and writing cryptographic code requires special expertise if you want it to actually be secure, so that's in the "future plan" category.

Our assumption has always been that, once a client is talking to the remote protocol server, in can be trusted, just as ADB trusts its client.

Our technical error here was that we did not know that some B2G apps would be able to make arbitrary TCP connections (the above mention of "tcpsocket permissions" is the first I've heard of it), since that's not a privilege extended to web pages. We assumed that accepting TCP connections only from localhost for the ADB/Gecko connection was adequate. Now we know this to be untrue.

Our procedural error here was not having a B2G architect and devtools architect discuss this issue before enabling the socket listener on B2G.
Looking at the descriptions of the B2G permissions model here:
https://wiki.mozilla.org/Apps/Security#Definitions

and looking at the table of which permissions are granted to which kinds of apps:
https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/PermissionsTable.jsm#51

my understanding is that the tcpsocket permission will be available to "installed privileged applications", which an app store has reviewed and signed, but neither "normal web content" nor "unauthenticated web applications".

So this seems serious and important to fix, but we do have the stores' review processes between us and the attackers.
Depends on: 834002
> We do want to support debugging over wifi eventually, but that's always been
> in the context of first choosing some secure transport, authenticated with
> something lightweight and exposed to public review, perhaps like ZRTP:
> http://en.wikipedia.org/wiki/ZRTP
> This was discussed with Tanvi (Security Engineer) and Johnathan Nightingale
> (who has security background) at the first London devtools meeting. But we
> don't have an implementation of ZRTP available right now, and writing
> cryptographic code requires special expertise if you want it to actually be
> secure, so that's in the "future plan" category.

There's a lot of discussion about that, and in particular about using our already-existing authentication mechanism (J-PAKE) in bug 832436, which I was helping Tanvi with before. (That's how I know about the desire to eventually support wifi debugging in the first place.)

My main point is that if we know that we're going to support Wifi debugging in the future, then we know that any switch to domain sockets would be a temporary hack. And, I'd rather we spend time on bug 832436 and on improving the security prompt's security/usability than have you guys spend time on switching to domain sockets. (Especially, I don't think anybody wants to hold up your work on debugging content processes more than necessary.)

(In reply to Jim Blandy :jimb from comment #21)
> my understanding is that the tcpsocket permission will be available to
> "installed privileged applications", which an app store has reviewed and
> signed, but neither "normal web content" nor "unauthenticated web
> applications".

Right. Sorry I didn't mention this before, but I agree that it offers some level of protection. On the other hand, I think we should be careful about relying on the app store reviews as a security mechanism right now, as we're unproven (at best) at stopping such attacks through such review.

The problem is that we don't have a plan for improving the security prompt and we don't have anybody working on bug 832436. So, even though I don't like the idea, switching to domain sockets might be the easiest, safest, and most expedient solution to the concerns here, even if it means writing throwaway code. But, that is based on a very uninformed understanding of how the debugger works at all.
(In reply to Brian Smith (:bsmith) from comment #22)
> The problem is that we don't have a plan for improving the security prompt
> and we don't have anybody working on bug 832436. So, even though I don't
> like the idea, switching to domain sockets might be the easiest, safest, and
> most expedient solution to the concerns here, even if it means writing
> throwaway code. But, that is based on a very uninformed understanding of how
> the debugger works at all.

I agree with this summation. I would also add that domain sockets could also be used for bug 797627.
(In reply to Brian Smith (:bsmith) from comment #22)
> My main point is that if we know that we're going to support Wifi debugging
> in the future, then we know that any switch to domain sockets would be a
> temporary hack. And, I'd rather we spend time on bug 832436 and on improving
> the security prompt's security/usability than have you guys spend time on
> switching to domain sockets. (Especially, I don't think anybody wants to
> hold up your work on debugging content processes more than necessary.)

It seems plausible to me that we might want to permit debugger connections over USB connections without requiring the user to interactively authenticate the connection, ZRTP or J-PAKE style. If so, that would continue to be a use case for the unix-domain sockets.

> (In reply to Jim Blandy :jimb from comment #21)
> > my understanding is that the tcpsocket permission will be available to
> > "installed privileged applications", which an app store has reviewed and
> > signed, but neither "normal web content" nor "unauthenticated web
> > applications".
> 
> Right. Sorry I didn't mention this before, but I agree that it offers some
> level of protection. On the other hand, I think we should be careful about
> relying on the app store reviews as a security mechanism right now, as we're
> unproven (at best) at stopping such attacks through such review.

I agree.

> The problem is that we don't have a plan for improving the security prompt
> and we don't have anybody working on bug 832436. So, even though I don't
> like the idea, switching to domain sockets might be the easiest, safest, and
> most expedient solution to the concerns here, even if it means writing
> throwaway code. But, that is based on a very uninformed understanding of how
> the debugger works at all.

I think the implementation wouldn't be too involved.
With this patch, you should be able to connect to the device after having called:
$ adb forward tcp:6000 localfilesystem:/data/local/debugger-socket

And then using localhost:6000 on the connect screen.

        Attachment #774875 -
        Attachment is obsolete: true

    
  
Depends on: 892114

    
    

    
  
Comment on attachment 796740 [details] [diff] [review]
Use unix domain socket file for devtools remote connection.

jim, here is the final patch that switch to domain socket only on b2g

paul, can we have you blessing on this move?
To summarize:
So far, devtool protocol was listening on tcp local port on the device.
As (privileged) webapps could request TCP permission, some would be able to connect to the devtool protocol and gain more privileges.
So we preffed off various devtools features, like being able to connect to an app, list/open/close/uninstall apps, ...
Thanks to this patch, and according to the discussion that happened in this bug, we fill confident about enabling all devtools features again as unix domain socket file can't be accessed by any app, even certified.
This patch only switch from TCP to unix domain socket. I'll attached another patch to pref on all devtools features:
  http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#990
  http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/webapps.js#650

        Attachment #796740 -
        Flags: review?(jimb)

        Attachment #796740 -
        Flags: feedback?(ptheriault)

    
    

    
  
Comment on attachment 796740 [details] [diff] [review]
Use unix domain socket file for devtools remote connection.

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

This looks good to be, but I'm not sure I can review changes to b2g.js and shell.js.

::: toolkit/devtools/server/main.js
@@ +336,5 @@
>     *
>     * @param aPort int
>     *        The port to listen on.
> +   * @param aUnixSocketPath string
> +   *        The path to the unix socket domain file.

It's weird to have two parameters, only one of which may be used in any given call. I think I would rather see this take a single parameter, aPortOrPath, and listen on the given TCP port if it's a number, or open a Unix domain socket at the given path if it's a string. But that can be a follow-up bug.

        Attachment #796740 -
        Flags: review?(jimb) → review+

    
    

    
  


  

        Attachment #796740 -
        Attachment is obsolete: true

        Attachment #796740 -
        Flags: feedback?(ptheriault)

    
    

    
  
Comment on attachment 798927 [details] [diff] [review]
Use single aPortOrPath argument for DebuggerServer.openListener

I ended up using your suggestion and use only one argument for openListener,
can you give it another look?

Vivien, Can you bless this change made to b2g/?

        Attachment #798927 -
        Flags: review?(jimb)

        Attachment #798927 -
        Flags: review?(21)

    
  
Assignee: nobody → poirot.alex

    
  

        Attachment #798927 -
        Flags: feedback?(ptheriault)

    
    

    
  
Comment on attachment 798927 [details] [diff] [review]
Use single aPortOrPath argument for DebuggerServer.openListener

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

r+ for the b2g/ part.

        Attachment #798927 -
        Flags: review?(21) → review+

    
  

        Attachment #798927 -
        Flags: review?(jimb) → review+

    
  
Blocks: appmgr_v1

    
    

    
  
What are the implications of this change landing - will users now be able to remotely debug any app, including the system app? Sounds like the risk of apps accessing debugger service is mitigated by above, but what about people using the debugger to access sensitive data usually not accessible on the phone? For example, if if I find someone's phone it is non-trivial to read their email password. However if I can just debug the email app, I can probably read their password. Any thoughts on how to mitigate this type of risk - do we perhaps limit which apps you are allowed to connect a debugger to?

    
    

    
  
(In reply to Paul Theriault [:pauljt] from comment #33)
> What are the implications of this change landing - will users now be able to
> remotely debug any app, including the system app?

Yes, that's the plan. And we need to keep a way for debugging system app. In order to finally offer great tools for gecko and gaia developers. Having said that we can mitigate that behind a new setting. For now, we suffer from a regression (bug 912213) that disallow accessing apps from parent process (system, keyboard,...). But certified apps like email can be debugged right now. Also non-certified apps can contain sensitive data. An unofficial email app can store passwords and won't be a certified app. So it won't be much more safe to blacklist apps by their priviledges.

> Sounds like the risk of
> apps accessing debugger service is mitigated by above, but what about people
> using the debugger to access sensitive data usually not accessible on the
> phone? For example, if if I find someone's phone it is non-trivial to read
> their email password. However if I can just debug the email app, I can
> probably read their password. Any thoughts on how to mitigate this type of
> risk - do we perhaps limit which apps you are allowed to connect a debugger
> to?

Two things already mitigate this:
  * remote debugging has to be toggle manually in the (hidden) settings app menu
  * when the phone is locked, remote debugging is shut down

If you think that's not enough I see two possible ways to lock that down:
  * introduce yet another setting to explicitely allow certified app debugging, otherwise you will only be able to debug hosted and privileged apps. (easy)
  * flag apps that have been installed via remote debugging and only allow debugging them by default. Then add a setting to allow debugging all other apps. That's non-trivial and may be challenging to implement by the time of the next merge. (we do want to enable apps debugging in FF26)

I'm going to land this patch after some communication about necessary "adb forward" changes.
This patch actually doesn't enable any new priviledges, it only switch from tcp to unix socket.
All new devtools features are still hidden behind devtools.debugger.enable-content-actors pref.

    
    

    
  
(In reply to Alexandre Poirot (:ochameau) from comment #34)
> (In reply to Paul Theriault [:pauljt] from comment #33)
> > What are the implications of this change landing - will users now be able to
> > remotely debug any app, including the system app?
> 
> Yes, that's the plan. And we need to keep a way for debugging system app. In
> order to finally offer great tools for gecko and gaia developers. Having
> said that we can mitigate that behind a new setting. For now, we suffer from
> a regression (bug 912213) that disallow accessing apps from parent process
> (system, keyboard,...). But certified apps like email can be debugged right
> now. Also non-certified apps can contain sensitive data. An unofficial email
> app can store passwords and won't be a certified app. So it won't be much
> more safe to blacklist apps by their priviledges.
> 
> > Sounds like the risk of
> > apps accessing debugger service is mitigated by above, but what about people
> > using the debugger to access sensitive data usually not accessible on the
> > phone? For example, if if I find someone's phone it is non-trivial to read
> > their email password. However if I can just debug the email app, I can
> > probably read their password. Any thoughts on how to mitigate this type of
> > risk - do we perhaps limit which apps you are allowed to connect a debugger
> > to?
> 
> Two things already mitigate this:
>   * remote debugging has to be toggle manually in the (hidden) settings app
> menu
>   * when the phone is locked, remote debugging is shut down

I'm hearing the second one will probably change (and thats fine with me, since we already have the 'after 12 hours turn off remote debugging feature). But neither of these protect the average user who doesn't set a passcode.

Don't get me wrong - that value and power of remote debugging for developers may well outweigh the security risks introduced by this change. I am just concerned that basically we are introducing root like privileges to the devices, and I want to make sure we go into this considering the risks involved. Maybe we just want to make the call that if you don't set a passcode, then you don't care about security but that seems like a harsh call to make.


> 
> If you think that's not enough I see two possible ways to lock that down:
>   * introduce yet another setting to explicitely allow certified app
> debugging, otherwise you will only be able to debug hosted and privileged
> apps. (easy)

A setting on its own doesn't help (again I am thinking of the 'lost device without a passcode use case'). But maybe the ability to set a password on developer features.

>   * flag apps that have been installed via remote debugging and only allow
> debugging them by default. Then add a setting to allow debugging all other
> apps. That's non-trivial and may be challenging to implement by the time of
> the next merge. (we do want to enable apps debugging in FF26)
> 
> I'm going to land this patch after some communication about necessary "adb
> forward" changes.
> This patch actually doesn't enable any new priviledges, it only switch from
> tcp to unix socket.
> All new devtools features are still hidden behind
> devtools.debugger.enable-content-actors pref.

Sounds fine to me - I have other thoughts, questions and comments, but I created a separate secreview bug to continue this discussion since it isn't related to this change (bug 914543).

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Rebased patch
        — 
          — Splinter Review
      

    


  

        Attachment #798927 -
        Attachment is obsolete: true

        Attachment #798927 -
        Flags: feedback?(ptheriault)

        Attachment #802156 -
        Flags: review+

    
  
Keywords: checkin-needed

    
    

    
  
(In reply to Paul Theriault [:pauljt] from comment #36)
> Don't get me wrong - that value and power of remote debugging for developers
> may well outweigh the security risks introduced by this change. I am just
> concerned that basically we are introducing root like privileges to the
> devices, and I want to make sure we go into this considering the risks
> involved. Maybe we just want to make the call that if you don't set a
> passcode, then you don't care about security but that seems like a harsh
> call to make.

Access to Unix domain sockets is checked using filesystem permissions, so isn't it the case that a process with sufficient privilege to connect to the Unix domain socket could just as well apply ptrace to the main process, or any number of other operations, and do whatever it pleases?

I don't feel like "introduces root like privileges to the devices" is a proportional description of what running the server really does.

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/6a8d3277faaf
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: