Security/Reviews/Gaia/InterAppCommunicationAPI: Difference between revisions

From MozillaWiki
< Security‎ | Reviews‎ | Gaia
Jump to navigation Jump to search
 
(5 intermediate revisions by the same user not shown)
Line 102: Line 102:
=== Gaia ===
=== Gaia ===
==== XSS & HTML Injection Attacks ====
==== XSS & HTML Injection Attacks ====
User controlled values are pretty much limited to filename. The filename is displayed in the notifications pull-down as well as the Settings Downloads list. [https://bugzilla.mozilla.org/show_bug.cgi?id=960749 960749] prevented us from being able to completely check for HTML injections. (See Future Work below)
TBD
 
Based on source code inspection, there are no dangerous coding practices (like misuse of innerHTML) that will result in HTML/JS injections.
 
Characters &#39;,&quot;,&gt;, \, & and &lt; were tested in filenames. We could not directly test &gt; or &lt; because the filesystem disallowed those characters in filenames, however we did use App Manager to break into the JS and insert those characters to see if filenames were rendered safely in the Notifications pull down as well as the Settings->Downloads menu.


==== Secure Communications ====
==== Secure Communications ====
Not relevant.
TBD


==== Secure Data Storage ====
==== Secure Data Storage ====
Downloads are stored on the SDcard, which is appropriate for user content.
TBD


==== Denial of Service ====
==== Denial of Service ====
See [https://bugzilla.mozilla.org/show_bug.cgi?id=960739 960739]
TBD


==== Interfaces with other Apps/Content====
==== Interfaces with other Apps/Content====
===== gaia/apps/system/js/download/download_notification.js =====
TBD
Used to launch Settings->Download list
  183        var activity = new MozActivity({
  184          name: 'configure',
  185          data: {
  186            target: 'device',
  187            section: 'downloads'
  188          }
  189        });
 
===== gaia/gaia/shared/js/download/download_helper.js =====
Used to open file after download
  176    var activity = new MozActivity({
  177      name: 'open',
  178      data: {
  179        url: download.path,
  180        type: contentType,
  181        blob: blob
  182      }


=== Gecko ===
=== Gecko ===
==== 1. Content/Chrome Segregation ====
==== 1. Content/Chrome Segregation ====
DownloadsAPI is implemented using WebIDL. There was a lot of discussion around what to expose in the case when a page does not have the permission present - see [https://bugzilla.mozilla.org/show_bug.cgi?id=957592 bug 957592] for details.
TBD


==== 2. Process Segregation ====
==== 2. Process Segregation ====
Inter-process communication is performed through DownloadsIPC.jsm & DownloadsAPI.jsm. We are mainly interested in the message which the parent listens for:
The message which the parent listens for:
* Downloads:GetList
* Webapps:Connect
* Downloads:ClearAllDone
* Webapps:GetConnections
* Downloads:Remove
* InterAppConnection:Cancel
* Downloads:Pause
* InterAppMessagePort:PostMessage
* Downloads:Resume
* InterAppMessagePort:Register
 
* InterAppMessagePort:Unregister
Permissions are checked in the parent before processing any messages, using the standard approach:
* child-process-shutdown


  144  receiveMessage: function(aMessage) {
There is no permission associated with Inter App Communications, so we do not have the assertPermission() check in the parent.
  145    if (!aMessage.target.assertPermission("downloads")) {
  146      debug("No 'downloads' permission!");
  147      return;
  148    }


One issue was identified in the way the message was processed however - see bug [https://bugzilla.mozilla.org/show_bug.cgi?id=966141 966141] for details.
The parent process prevents a compromised child process from sending messages to the parent by verifying the manifestURL sent in the message matches the manifest URL of the publishing app.


==== 3. Data validation & Sanitization ====
==== 3. Data validation & Sanitization ====
The API accepts only minimal data from content, and as such the attack surface is very small, and no issues were found.
TBD


====4. Denial of Service ====
====4. Denial of Service ====
[https://bugzilla.mozilla.org/show_bug.cgi?id=960739 960739] was identified as a potential DoS scenario.
TBD


== Concerns (To-Delete) ==
== Concerns (To-Delete) ==

Latest revision as of 20:59, 3 February 2014

Review Details

Overview

The Inter-App Communication API will allow apps to communicate in a publisher/subscriber model.

Apps will register for communication in their manifest file, defining specific restrictions and details relating to the communications desired. An application can setup to send communications and/or handle communications.

Currently, only certified apps are allowed to do connections, but there are plans to open them up in the future.

Source Code

Gaia

Gecko

WebIDL

  • dom/webidl/InterAppConnection.webidl - MozInterAppConnection
  • dom/webidl/InterAppConnectionRequest.webidl - MozInterAppConnectionRequest
  • dom/webidl/MozInterAppMessageEvent.webidl - MozInterAppMessageEvent
  • dom/webidl/InterAppMessagePort.webidl - MozInterAppMessagePort

IDL

  • dom/interfaces/apps/nsIDOMApplicationRegistry.idl - registers connect() and getConnections()
  • dom/interfaces/apps/nsIInterAppCommService.idl - nsIInterAppCommService

Security Features

manifest ‘rules’

minimumAccessLevel

Defines a ‘minimum’ application type level: web, privileged, or certified. Defaults to ‘web’.

installOrigins

A list of install origins from where subscriber apps should have been installed. Since certified apps has not a valid install origin, these constraint does not apply to them.

manifestURLs

Can be used to set specific subscribers by a list of manifestURLs.

Current Usage

connect()

  • apps/bluetooth/js/transfer.js:216: app.connect('bluetoothTransfercomms').then(function(ports) {
  • apps/communications/dialer/js/calls_handler.js:114: app.connect('dialercomms').then(function(ports) {
  • apps/communications/ftu/js/tutorial.js:123: app.connect('ftucomms').then(function onConnAccepted(ports) {
  • apps/homescreen/everything.me/js/search/control.js:12: app.connect('search-results').then(
  • apps/search/js/search.js:37: app.connect('search-results').then(
  • apps/system/js/rocketbar.js:249: app.connect('search').then(
  • apps/system/test/marionette/fakemusic/js/comms.js:34: app.connect('mediacomms').then(function(ports) {
  • shared/js/media/remote_controls.js:184: app.connect('mediacomms').then(function(ports) {

apps/search/manifest.webapp

 28     "search": {
 29       "handler_path": "index.html",
 30       "description": "Proxies search to copied search app. Should be moved to the search app manifest if we split the app up.",
 31       "rules": {}
 apps/system/js/rocketbar.js:249: app.connect('search')...
 Used by System app, in rocketbar.js, to insert '...the search app iframe into the dom'

apps/system/manifest.webapp

 83     "mediacomms": {
 84       "description": "Communication with media apps for now playing info",
 85       "rules": {}
 87     "search-results": {
 88       "description": "Communicate between search results and search app",
 89       "rules": {}
 91     "ftucomms": {
 92       "description": "Communicate between communications/ftu and System",
 93       "rules": {}
 95     "bluetoothTransfercomms": {
 96       "description": "Communication with bluetooth apps for sending files info",
 97       "rules": {}
 99     "dialercomms": {
100       "description": "Communication with dialer app for sleep message",
101       "rules": {}
103     "fxa-mgmt": {
104       "description": "Firefox Accounts management API",
105       "rules": {
106         "minimumAccessLevel": "certified"
107       }

Review Notes

Gaia

XSS & HTML Injection Attacks

TBD

Secure Communications

TBD

Secure Data Storage

TBD

Denial of Service

TBD

Interfaces with other Apps/Content

TBD

Gecko

1. Content/Chrome Segregation

TBD

2. Process Segregation

The message which the parent listens for:

  • Webapps:Connect
  • Webapps:GetConnections
  • InterAppConnection:Cancel
  • InterAppMessagePort:PostMessage
  • InterAppMessagePort:Register
  • InterAppMessagePort:Unregister
  • child-process-shutdown

There is no permission associated with Inter App Communications, so we do not have the assertPermission() check in the parent.

The parent process prevents a compromised child process from sending messages to the parent by verifying the manifestURL sent in the message matches the manifest URL of the publishing app.

3. Data validation & Sanitization

TBD

4. Denial of Service

TBD

Concerns (To-Delete)

manifest

  • The installOrigins field inside manifest file limits communications origins. This needs to be tested
    • also, them seem to just be a domain name, are we not doing port, domain, protocol along with app id?