Security/Reviews/Gaia/InterAppCommunicationAPI: Difference between revisions

< Security‎ | Reviews‎ | Gaia
(Created page with "== Review Details == * Topic: Inter-App Communication API * Review Date: January, 2014 * Review Lead: Rob Fletcher <rfletcher@mozilla.com> (:omerta) * Repo: * Connections: Ge...")
 
 
(26 intermediate revisions by the same user not shown)
Line 2: Line 2:
* Topic: Inter-App Communication API
* Topic: Inter-App Communication API
* Review Date: January, 2014
* Review Date: January, 2014
* Status: Ongoing/Incomplete
* Review Lead: Rob Fletcher <rfletcher@mozilla.com> (:omerta)
* Review Lead: Rob Fletcher <rfletcher@mozilla.com> (:omerta)
* Repo:  
* Repo:  
Line 17: Line 18:
== Source Code ==
== Source Code ==
=== Gaia ===
=== Gaia ===
* shared/js/iac_handler.js - handles IAC messages
* [https://github.com/mozilla-b2g/gaia/blob/master/shared/js/iac_handler.js shared/js/iac_handler.js] - handles IAC messages
* shared/js/fxa_iac_client.js - Firefox Accounts IAC client
* [https://github.com/mozilla-b2g/gaia/blob/master/shared/js/fxa_iac_client.js shared/js/fxa_iac_client.js] - Firefox Accounts IAC client


=== Gecko ===
=== Gecko ===
* dom/apps/src/Webapps.js - defines connect() and getConnections()
* [http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.js dom/apps/src/Webapps.js] - cpmm("Webapps:Connect"...), cpmm("Webapps:GetConnections"...)
* dom/apps/src/Webapps.jsm - process manifest file for new ‘connections’
* [http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm dom/apps/src/Webapps.jsm] - process manifest file for new ‘connections’
* dom/apps/src/InterAppComm.cpp
* [http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/InterAppComm.cpp dom/apps/src/InterAppComm.cpp]
* '''dom/apps/src/InterAppCommService.js'''
* [http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/InterAppCommService.js dom/apps/src/InterAppCommService.js]
** parent process, does checking of installOrigin, manifestURLs, and minimumAcccessLevel, main file for API
** parent process, does checking of installOrigin, manifestURLs, and minimumAcccessLevel, main file for API
* dom/apps/src/InterAppConnection.js - child process, InterAppConnection object
* [http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/InterAppConnection.js dom/apps/src/InterAppConnection.js] - child process, InterAppConnection object
* dom/apps/src/InterAppMessagePort.js - child process, InterAppMessagePort object
* [http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/InterAppMessagePort.js dom/apps/src/InterAppMessagePort.js] - child process, InterAppMessagePort object


=== WebIDL ===
=== WebIDL ===
Line 35: Line 36:
* dom/webidl/InterAppMessagePort.webidl - MozInterAppMessagePort
* dom/webidl/InterAppMessagePort.webidl - MozInterAppMessagePort


==== IDL ===
=== IDL ===
* dom/interfaces/apps/nsIDOMApplicationRegistry.idl - registers connect() and getConnections()
* dom/interfaces/apps/nsIDOMApplicationRegistry.idl - registers connect() and getConnections()
* dom/interfaces/apps/nsIInterAppCommService.idl -  nsIInterAppCommService
* dom/interfaces/apps/nsIInterAppCommService.idl -  nsIInterAppCommService
Line 45: Line 46:


==== installOrigins ====
==== installOrigins ====
A ist 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.  
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 ====
==== manifestURLs ====
Can be used to set specific subscribers by a list of manifestURLs.  
Can be used to set specific subscribers by a list of manifestURLs.  


== Concerns ==
== 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) ==
* http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#748
* http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#748
** I think we can control ‘keyword’ and this looks like its chrome code, is this one of those chrome/content bypasses?
** I think we can control ‘keyword’ and this looks like its chrome code
* I think a lot of this just needs to be put through manual testing.
* I think a lot of this just needs to be put through manual testing.
* http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/InterAppCommService.js#349
* http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/InterAppCommService.js#349
** does checking for ‘security’ things. It uses 2 fields each time. ex. aSubAppManifestURL and aPubAppManifestURL. Can i set one of those on my app and ‘bypass’ these tests
** does checking for ‘security’ things. It uses 2 fields each time. ex. aSubAppManifestURL and aPubAppManifestURL. Can i set one of those on my app and ‘bypass’ these tests
* So this uses postMessage, is there any opportunity for other apps just listening for 'message' will be able to intercept sensitivei comms?


=== manifest ===
=== manifest ===
* The installOrigins field inside manifest file limits communications origins. This needs to be tested
* 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?
** also, them seem to just be a domain name, are we not doing port, domain, protocol along with app id?

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?