1006025 - Add test to fxos-certsuite to verify permissions model

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Certification Suite, defect)

Description

[Dan Minor [:dminor]]

The fxos certification suite requires tests that permissions model is implemented properly, with the following requirements:
* Branded devices must implement the permission model and trust levels documented here [1] [2].
* Branded devices must gate access to Web APIs by appropriate permission checks.  Current permissions checks are documented here [3].
* Note:  system updates untestable

Bug 976706 implemented static WebAPI tests that overlap with the requirements here. The initial activity will focus on investigating what additional work is required to verify the permissions matrix described at [3].

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Security/Security_model
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Security/System_security
[3] https://developer.mozilla.org/en-US/docs/Web/Apps/App_permissions

Comment 1

[Dan Minor [:dminor]]

Some of the WebAPIs fail when called without the appropriate app permission, so additional tests beyond the ones in Bug 976706 are needed. The pull request for permissions listed in [1] is at [2]. 

The next step will be to look at the full set of permissions listed in [3].

[1] https://developer.mozilla.org/en-US/docs/Web/Apps/App_permissions
[2] https://github.com/mozilla-b2g/fxos-certsuite/pull/79
[3] https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/PermissionsTable.jsm#36

Comment 2

[Dan Minor [:dminor]]

The audio-capture and video-capture permissions tests causes some phones (e.g. the buri) to hang when using mozGetUserMedia so we'll disable them for now.

Comment 3

[(not currently active) Ted Mielczarek]

To clarify, getUserMedia crashes the content process on my buri, which causes the Python harness to hang waiting for the results. This appears to be bug 988105.

Comment 4

[Dan Minor [:dminor]]

This next set of permissions are added in: https://github.com/mozilla-b2g/fxos-certsuite/pull/91

Work to do this sprint:
- network permissions testing once I have a SIM card
- follow up on nfc permissions checking using the flame
- follow up on injecting wappush-received messages to test wappush permission

Comment 5

[Dan Minor [:dminor]]

Ehsan, I was hoping you had some time to do a quick review of these tests [1]. I'm particularly interested if any of the items marked TODO are actually testable, but if you see any problems with the other tests, that would be nice too. We're just starting a new sprint, so feedback anytime in the next two weeks would be great.

In case you want some additional context, the python portion of the harness is here [2]. It is generating hosted, privileged and certified apps with either all or no permissions granted, and then running the tests at [1].

Thank you!

[1] https://github.com/mozilla-b2g/fxos-certsuite/blob/master/certsuite/static/permissions-test-app/main.js
[2] https://github.com/mozilla-b2g/fxos-certsuite/blob/master/certsuite/cert.py

Comment 6

[(no longer active)]

I am not a huge fan of this approach...  Can we instead run some chrome privileged JS, import PermissionsTable.jsm and inspect it directly?

Comment 7

[Dan Minor [:dminor]]

Thanks for taking a look at this!

I don't think introspecting PermissionsTable.jsm would allow us the same kind of test coverage that we get with these tests.

For example, as far as I can tell, the only way to see if the 'mozbrowser' permission is working as expected is to create an iframe and check to see if the extra methods are available. Similarly, a bad SystemXHR permission seems to require you actually try to send() it before it will fail.

That said, the majority of these tests are just checking for the existence or value of a property off of navigator, which is redundant with other tests we are running.

If you don't like this approach primarily due to the fact I've hard-coded a lot of tests that could be generated from PermissionsTable.jsm, maybe I should only test the permissions that require special handling here, and let our other tests cover the rest. That would be easier to maintain.

Comment 8

[(no longer active)]

(In reply to Dan Minor [:dminor] from comment #7)
> Thanks for taking a look at this!
> 
> I don't think introspecting PermissionsTable.jsm would allow us the same
> kind of test coverage that we get with these tests.
> 
> For example, as far as I can tell, the only way to see if the 'mozbrowser'
> permission is working as expected is to create an iframe and check to see if
> the extra methods are available. Similarly, a bad SystemXHR permission seems
> to require you actually try to send() it before it will fail.

Right.  Note that we'll be trying to hide the systemXHR property from non-privileged contexts soon (see the patch in bug 1009645)

> That said, the majority of these tests are just checking for the existence
> or value of a property off of navigator, which is redundant with other tests
> we are running.

Yes.  :-)  Also, they don't really check which permission has caused the name to appear if it's there either.

> If you don't like this approach primarily due to the fact I've hard-coded a
> lot of tests that could be generated from PermissionsTable.jsm, maybe I
> should only test the permissions that require special handling here, and let
> our other tests cover the rest. That would be easier to maintain.

Yes, that's mostly what I meant.  :-)  The other thing which I would like us to test is to see if there are permissions in the table which we don't expect to see, as they might be used to expose APIs hidden behind them and we won't be able to detect those cases in any other way.

Comment 9

[Dan Minor [:dminor]]

Ok, so it sounds like what I should do is use PermissionsTable.jsm as suggested and for each permission, toggle it to set and then walk the navigator object, recording the properties to see if anything has changed.

Once this is done, I can run the "special case" tests for things like the mozbrowser permission that need an object to be instantiated to test if the permission is working as expected.

Comment 10

[(no longer active)]

That sounds like a great plan!

Comment 11

[Dan Minor [:dminor]]

Attachment: Run tests based upon PermissionsTable

Comment 12

[Jonathan Griffin (:jgriffin)]

Can this be marked as fixed?

Comment 13

[Dan Minor [:dminor]]

I think we can mark this as fixed, we can always track any problems that show up in separate bugs.