Video app is killed while we put onpeerready callback. The logcat shows: I/Gecko ( 1514): Security problem: Content process does not have `nfc-write'. It will be killed. I/Gecko ( 1514): I/Gecko ( 1514): ###!!! [Parent][MessageChannel] Error: Channel error: cannot send/recv I/Gecko ( 1514): V/AudioFlinger( 172): 1922 died, releasing its sessions I/Gecko:ProcessPriorityManager( 1514): [Homescreen, child-id=1, pid=1584] Changing priority from BACKGROUND_HOMESCREEN:CPU_NORMAL to FOREGROUND:CPU_NORMAL. I/Gonk ( 1514): Setting nice for pid 1584 to 1 I/Gonk ( 1514): Changed nice for pid 1584 from 18 to 1. I/Gecko:ProcessPriorityManager( 1514): [Video, child-id=4, pid=1922] Scheduling reset timer to fire in 1000ms. I/Gecko:ProcessPriorityManager( 1514): [Video, child-id=4, pid=1922] ScheduleResetPriority bailing; the timer is already running. I/Gecko:ProcessPriorityManager( 1514): [child-id=4, pid=-1] Destroying ParticularProcessPriorityManager. I/Gecko ( 1514): [Parent 1514] WARNING: waitpid failed pid:1922 errno:10: file ../../../ipc/chromium/src/base/process_util_posix.cc, line 254 I/Gecko ( 1514): [Parent 1514] WARNING: waitpid failed pid:1922 errno:10: file ../../../ipc/chromium/src/base/process_util_posix.cc, line 254 I/Gecko ( 1514): [Parent 1514] WARNING: Failed to deliver SIGKILL to 1922!(3).: file ../../../ipc/chromium/src/chrome/common/process_watcher_posix_sigchld.cc, line 118

According to MDN, https://wiki.mozilla.org/WebAPI/WebNFC, no clue about what's going on.

Garner, it is a critical issue, since more Gaia developers are going to join this NFC development.

I got what's going on. If we don't put permission to manifest, apps will be killed. That shouldn't be that. The correct permission is: "nfc": { "access": "readwrite" }.

Chrome process explicitly checks/asserts for 'nfc-write' perms before registering 'onpeerready' for that particular application. Do you suggest that application should not be killed, should it not have valid permission (nfc-write) ?

Essentially what Sid said. Some suggestions for discussion: 1) The DOM can hide the offending API function call on the DOM entirely (like navigator.MozNfc itself can be hidden if nfc permissions are missing). 2) We kill the App like Windows may do when an app erronously tries to write/modify files in a secured file location. Since that permission is the app developer's job to add to the manifest, I prefer bombing out instead of quietly failing. 2.1) In another instance of this scenario, I proposed having an error bubble up to a system modal UI dialog to say "[Force Kill] Application [%s] was force killed due to accessing an API that requried [%s] permissions." I like this idea in principal, because apps can be web connected, and can access external JS code? I think the goal is to make the problem obvious.

Killing with improper webAPI usage doesn't make any sense IMHO. Web != Windows.

(In reply to Alive Kuo [:alive][NEEDINFO] from comment #6) > Killing with improper webAPI usage doesn't make any sense IMHO. Web != > Windows. We do that to prevent a hacked content process to get access to features in the parent process. That's part of the security model. Note that we don't kill on improper API usage, we kill when you have no permissions to use the API.

Comment on attachment 8347146 [details] [diff] [review] Patch Review of attachment 8347146 [details] [diff] [review]: ----------------------------------------------------------------- Since I will update a new version of this patch, so change r? to feedback? Kyle, in the Chrome process, Nfc.js will also check nfc permission, given that I'd use WebIDL to do the permission check, do you think we still have to check permission in Chrome process, for example, in http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/Nfc.js#289 ::: dom/webidl/MozNfc.webidl @@ +20,5 @@ > * > * Users of this API should have valid permissions 'nfc-manager' > * and 'nfc-write' > */ > + [Func="Navigator::HasNfcPeerSupport"] This line is wrong. checkP2PRegistration is used by nfc-manager. I'll remove this in next version.

Comment on attachment 8359110 [details] [diff] [review] Patch. v2. Review of attachment 8359110 [details] [diff] [review]: ----------------------------------------------------------------- Hi, Kyle Can you check my previous comment 9 if it's neccesary? "Do you think we still have to check permission in Chrome process, for example, in http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/Nfc.js#294 "

Comment on attachment 8359110 [details] [diff] [review] Patch. v2. Review of attachment 8359110 [details] [diff] [review]: ----------------------------------------------------------------- Yes, we still need to check permissions in the parent process. If the child process is compromised by a security exploit, the parent is responsible for enforcing that the permissions restrictions still apply.

Comment on attachment 8359110 [details] [diff] [review] Patch. v2. Thanks Kyle, sr? to smaug for the WebIDL change.

Comment on attachment 8359110 [details] [diff] [review] Patch. v2. This doesn't need sr, just land it.

khuey is a DOM peer, so it doesn't need anything beyond that for the WebIDL stuff.

Thanks, Kyle and Andrew. I'll push to try server and once it's green I'll push the patch to inbound.