Closed Bug 783716 Opened 12 years ago Closed 6 years ago

rename systemXHR and TCP sockets permissions to networktcp

Categories

(Core :: DOM: Device Interfaces, defect)

Product:
Core
Component:
DOM: Device Interfaces
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED INCOMPLETE
Project Flags:
blocking-basecamp -

People

(Reporter: gwagner, Assigned: gwagner)

Details

      No description provided.
As I read this, you are saying that any application that needs the ability to use mozAnon to not send cookie/authentication headers or mozSystem to avoid origin checks will have the ability to open TCP sockets.  Is this the intent?

If it is, I'd appreciate additional rationale.  The ability to open TCP connections seems much more dangerous than the ability to use a relaxed variant of XHR, and so seems like it should be its own privilege.
I'd have to agree with Andrew here. What is the rationale?

The ability to do XHR without domain restriction and the ability to perform arbitrary client socket io are vastly different security profiles. I think the permission should be different.
They are different from a technical perspective, but that's hard to convey in simple terms to the average non-technical user.
(In reply to Fabrice Desré [:fabrice] from comment #3)
> They are different from a technical perspective, but that's hard to convey
> in simple terms to the average non-technical user.

But the TCP socket privilege is only for reviewed apps on app-stores because it is so dangerous.  I think it's reasonable to assume that the reviewers be able to know the difference, even if the distinction is not surfaced in the user-friendly description of privileges required by the application.

One could make the argument that since the app will be reviewed, it doesn't matter, but I think it's helpful to the reviewers if they can rely on a solid technical backstop to know that an app can't open TCP connections.
Another +1 for keeping these separate. My understanding was that System XHR was not going to be a permission at all and it was just going to be available to privileged apps (hence the title of bug 692677). 

For consistency, I think it would be better to actually have a permission for systemXHR, so that an application that wants to take advantage of this feature has to enumerate it. This is what we have done with other powerful APIs as a risk mitigation control. This reduces the impact of a vulnerability in a privileged app (exploit code only gets the permissions enumerated in the manifest) and aids the review process (manifest clearly shows what powers need to be audited).

But as per the comments above, the risk associated with TCPSocket is much greater than SystemXHR (especially since apps run in a sandbox which means they have their own cookies only). SystemXHR is mainly a threat against internal websites which don't require auth. TCPSocket is a threat against ANY network resource which doesn't require auth or has network exploitable weaknesses (file servers, smtp servers, network devices etc)

Final 2c: Neither TCPSocket nor the ability of privileged apps to perform systemXHR should ever be communicated to the user. This information is for technical reviewers only.
Both systemXHR and tcp-sockets are only exposed to "trusted apps" as per discussion on the mailing list (sorry, I forget which one) and documented at [1].

The "only" additional security sensitive capabilities that systemXHR and the TCP-socket API provides is that they can connect to servers behind firewalls. Other than that, a web developer can do the exact same thing by simply using server-to-server connections.

I don't think we need to worry about non-technical users. We aren't planning on exposing this in any normal user-facing UI. Per [1] neither of these APIs prompt the user.

So the question is, when reviewing app code for approval to use systemXHR/tcp-socket, does the reviewer need to look for different types of exploitable and/or malicious code?

I would say that generally the types of things you'd look for in both is about the same: Connecting to servers behind firewalls to read data and send that data to a home-server. Or launching DDoS attacks against target servers.

Granted, the TCP-socket API could also be used to connect to SMTP servers and sending spam. I'm not sure how much of a problem open SMTP servers is these days?

Another reason to keep them together is that companies or users might want to protect firewalled networks by disabling these two features. In that case disabling just one doesn't make much sense. However if we add such UI we could definitely make sure to have one UI-switch to flip both permissions.

So I guess I could live with using separate permissions for both these APIs, but I'm not entirely convinced that it's needed.

[1] https://docs.google.com/a/sicking.cc/spreadsheet/ccc?key=0Akyz_Bqjgf5pdENVekxYRjBTX0dCXzItMnRyUU1RQ0E#gid=0
Raw TCP sockets can additionally be used to poke things like UPnP devices, right?

I don't have a strong opinion on separate permission names, but the set of capabilities enabled by system XHR certainly is a strict subset of that of raw TCP sockets.  Having a blanket setting/pref for disabling cross-origin network traffic definitely seems reasonable.
UPnP uses UDP + HTTP.

Another reason having the two be the same permission is that even if someone manually disables systemXHR permission, that won't do anything security-wise if that app has tcp-access. In other words, it creates a more complex interdependency between permissions which is something I've wanted to avoid in order to keep things simple.
Open STMP is still extremely common on internal enterprise networks. As are a range of compatible services which are very useful to an attacker - all the common things you find in an internal penetration test: DNS, ldap servers, network devices etc.

My concern was that the use case for system XHR would be pretty common, where as full blown TCP would be rare - not sure if this is a valid assumption or not. What about having the same permission, but having an additional qualifier, like 'xhr-only' for a system that only wants to use system XHR? (or even better, a qualifier required for 'full-tcp' so it is more secure by default) That way we can still use the same permission, but get the benefits of risk mitigation by not granting this permission to far. Not sure if that helps or confuses the situation though.

Another suggestion from the security review of TCPSocket was to grant the permission on a port range or IP basis, but the suggestion there was to support the "connect" directive of CSP for TCPSocket instead of putting it in the permission if recall correctly - (hence bug 763960).
> My concern was that the use case for system XHR would be pretty common,
> where as full blown TCP would be rare - not sure if this is a valid
> assumption or not.

I don't think that it is. I would think that most of the time reason people design HTTP based APIs is so that they can be used from webpages, in which case there's no need for systemXHR.

I think that the majority of sensitive things that you can access behind firewalls, you can access using simply HTTP, and so systemXHR seems like it introduces most of the risks that TCP-socket does.

Though obviously TCP-socket is a strict superset of systemXHR and so TCP-socket so it does strictly introduce more risks. But the part that I'm worried about is that people will be less worried about systemXHR and I think that to a large extent that is not true.

> What about having the same permission, but having an
> additional qualifier, like 'xhr-only' for a system that only wants to use
> system XHR? (or even better, a qualifier required for 'full-tcp' so it is
> more secure by default) That way we can still use the same permission, but
> get the benefits of risk mitigation by not granting this permission to far.
> Not sure if that helps or confuses the situation though.

Yeah, I this is just a syntactical shuffle, so I don't think this makes a big difference.

> Another suggestion from the security review of TCPSocket was to grant the
> permission on a port range or IP basis, but the suggestion there was to
> support the "connect" directive of CSP for TCPSocket instead of putting it
> in the permission if recall correctly - (hence bug 763960).

This sounds like a good idea, but applies equally much to systemXHR as to TCPSocket. In fact, it sounds like an argument to treat them more equally than more different.
blocking-basecamp: --- → ?
While this would be great to have, it was discussed at triage and we decided not to block on it.
blocking-basecamp: ? → -
Sorry to bikeshed, but in https://bug778326.bugzilla.mozilla.org/attachment.cgi?id=658698 the name of the proposed permission is 'network-tcp'  (as well as network-http) . Just noting that, so one or the other gets updated accordingly.

Also PermissionsTable.jsm from bug 758269 will need to be updated.

For now, I am leaving the old permission name in the basecamp permissions spreadsheet, and in permission testing code since this isn't a blocking bug.

For reference: 
https://docs.google.com/spreadsheet/ccc?key=0Akyz_Bqjgf5pdHNlbDBDUGMzUzJSdFYyNEZjcngtUWc#gid=0
As it stands now, PermissionsInstaller.jsm [1] only know about network-tcp, while Gaia installer [2] only knows about systemXHR (which is also the name still use in the actual check in [3]). 

The only reason apps are working right now is because the PermissionsInstaller code isn't being used by the apps. As the code is now, there's no way for a new app to request the systemXHR permission, by any name (since if it requests network-tcp it will be granted but it won't get access to the systemXHR, and if it requests systemXHR it will fail).

So this might not be blocking... but one way or the other the names have to be synchronized, or we're going to ship something that doesn't work for third party apps (and depending on how ours are finally installed or updated maybe not even for those either). Either add systemXHR to PermissionsInstaller, or rename it to network-tcp on the check at [2].

Oh, and I think that network-http is only used in the Permissions.txt file.

FWIW I also think that the permissions should be kept separate, since although one is a subset of the other that way we favor the minimum privilege approach.



[1] https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/PermissionsInstaller.jsm#79
[2] https://github.com/mozilla-b2g/gaia/blob/master/build/permissions.js#L10
[3] https://mxr.mozilla.org/mozilla-central/source/dom/workers/WorkerPrivate.cpp#3042
Antonia, FYI, I have patches in bug 787439 that make sure that gaia, PermissionsInstaller.jsm and the APIs implementation actually use the same permission names.
I don't think we should ever combine raw sockets with XHR.  They are not the same, and allowing raw socket access for apps that only need XHR is introducing a lot of additional risk.  There are a lot of neat attacks you can do with raw sockets that you can't do with XHR; its not just about network topography.
More mass-incompleting of FxOS bugs in the Device Interfaces component.

Please update and let me know if any of these are still valid.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.