Sometimes we need a system message broadcast to launch apps conditionally. Current internal API SystemMessageInternal::broadcastMessage has no support for filtering. If we want such a filter function, we have to write another web API and do very similar things for every single filter. Therefore, it's good to have a message-defined filter for each system message by extending the existing configurator.

What are the use cases? Can you give some examples?

(In reply to Fabrice Desré [:fabrice] from comment #2) > What are the use cases? Can you give some examples? (The draft patch is for sync filter case. I should rewrite it to async version.) NFC has an asynchronous filtering use case, which requires some application manifest verification and certain access control. Needinfo :vchang for further information.

Ming will help to provide the use case here.

(In reply to Fabrice Desré [:fabrice] from comment #2) > What are the use cases? Can you give some examples? please find the related use cases at https://bugzilla.mozilla.org/show_bug.cgi?id=979152

In case of NFC payment, there is a working mode called "fastpay". Under this mode, the user can tap the phone against PoS and make payment, even without running Wallet app before, or even when the screen is off. After the payment, a HCI Event is fired to trigger some UI notification on the device.

Before passing the HCI Event to applications, we need to check AID and security element type. The format of the filter may look like a new protocol (nfc://security element type/aid). Prior to do the filter, we also have to use access control mechanism defined in http://opoto.github.io/secure-element/. It seems too complicate if we intent to implement access control in system filter. Maybe it should be implemented in NFC component.

We are facing now an unconfortable situation. Based on the ongoing HCI-event discussion, its dependency on Bug 879861 and 884594 in terms of security has been identified. However, according to the roadmap plan, the feature "HCI-event handling" is going to be landed in 2.1, ahead of the landing of Secure Element related features, including SE access control. How could we solve this problem? Would it be acceptable to land firstly the HCI-Event handling without access control based check in 2.1?

> We are facing now an unconfortable situation. > > Based on the ongoing HCI-event discussion, its dependency on Bug 879861 and > 884594 in terms of security has been identified. However, according to the > roadmap plan, the feature "HCI-event handling" is going to be landed in 2.1, > ahead of the landing of Secure Element related features, including SE access > control. How could we solve this problem? > > Would it be acceptable to land firstly the HCI-Event handling without access > control based check in 2.1? There's current discussion on how to route the message to a white listed webapp. Can apps manipulate/read existing items in the notification bar? To put it another way, is the notification bar "trusted"?. If not, can some kind of system dialog work while more complete SE support is added? We'll still push the message up to gecko, just not directed to a webapp until more support arrives.

(In reply to Garner Lee from comment #9) > > We are facing now an unconfortable situation. > > > > Based on the ongoing HCI-event discussion, its dependency on Bug 879861 and > > 884594 in terms of security has been identified. However, according to the > > roadmap plan, the feature "HCI-event handling" is going to be landed in 2.1, > > ahead of the landing of Secure Element related features, including SE access > > control. How could we solve this problem? > > > > Would it be acceptable to land firstly the HCI-Event handling without access > > control based check in 2.1? > > There's current discussion on how to route the message to a white listed > webapp. Can apps manipulate/read existing items in the notification bar? To > put it another way, is the notification bar "trusted"?. If not, can some > kind of system dialog work while more complete SE support is added? We'll > still push the message up to gecko, just not directed to a webapp until more > support arrives. Can we directly to have a very simple SE component(may just have 1 ow 2 webapis) for this HCI-event feature in 2.1, and enlarge this SE component functions in 2.2? Since I think we need to support HCI-event for SE component in 2.2 as well.

(In reply to Ming Yin from comment #8) > Would it be acceptable to land firstly the HCI-Event handling without access > control based check in 2.1? Hi, I am a little confused. Who is requesting that we need to handle HCI-Event with access control check? If HCI-Event in 2.1 is only for status update, I don't think we need to have access control for HCI-Event.

Hi, Given the current roadmap plan, I would propose 2-steps implementation plan for HCI-event handling. Step-1 for FFOS 2.1 - without SE-access-control integration and the security will use what FFOS can offer; Step-2 for FFOS 2.2 - with SE-access-control integration, which is based on GlobalPlatform Standard and DT's HCI-Event handling requirements; What do you think?

Ken, in addition to the plan outlined by Ming, I'd like to note that there is an ongoing discussion on how do we want to implement this. It's started on the dev.webapi mailing list [1]. We'd like to push this discission to be a bit higher level, and current status is that we're compiling requirements from specifications (GSMA, GP), operators, and requirements of payment scheme operators like Mastercard. As soon as we have it in place - which should be this week - we'll share it. The purpose of this is to have a common understanding on what the exact requirements are, so we can then decide on a solution which fits FxOS best. Hope it will help to clear some of your questions, we'll share it asap.

[1] I forgot to attach a link to the mailing list thread: https://groups.google.com/forum/#!topic/mozilla.dev.webapi/oER2OLg40pg

(In reply to Ming Yin from comment #12) > > What do you think? It is okay for me. (In reply to Kamil Leszczuk [:kamituel] from comment #13) Thanks for this information. Indeed, we have to discuss this first before starting 2.2.

Ken, please find the overview, requirements and implementation plan we've prepared here: https://docs.google.com/document/d/1tguuqvVUZHTLDfOtiwot5g3VrEsxfQoW0Twb-qUsaGo/edit?usp=sharing. Your comments and questions are welcome! Also, note that there is a discussion happening on webapps mailing list regarding EVT_TRANSACTION and ways to deliver it to Gaia apps.

(In reply to Kamil Leszczuk [:kamituel] from comment #16) > Ken, please find the overview, requirements and implementation plan we've > prepared here: > https://docs.google.com/document/d/1tguuqvVUZHTLDfOtiwot5g3VrEsxfQoW0Twb- > qUsaGo/edit?usp=sharing. > > Your comments and questions are welcome! > > Also, note that there is a discussion happening on webapps mailing list > regarding EVT_TRANSACTION and ways to deliver it to Gaia apps. Reading the doc, I have one question regarding the requirements 1&2. Requirement 1&2 requires the EVT_TRANSACTION to be dispatched to apps which are granted by ACE module even the apps are not running. My question is, if an app is not running, how does ACE module decide if the app is allowed to receive EVT_TRANSACTION? Does ACE module only need the static information (such as app name, certain attribute defined in manifest.webapp) to authorize an app? Thanks.

Hi Henry! ACE module is an Access Control Enforcer module, which basically uses rules stored on a SIM card (or embedded secure element, if that's the case) to determine whether an app has rights to receive EVT_TRANSACTION. In order to make such a decision, ACE needs two values: - AID of the SIM card applet that originated the event. This AID is stored in the event itself. - Hash of the certificate using which the Gaia application (i.e. Wallet app) has been signed. This can be gotten from the app package itself. Those two values will be matched against the access rules stored on the SIM card - those rules are basically "AID <-> cert hash" pairs. So you see that the application doesn't need to be running in order to make the access control check. Note: one piece missing in this picure is the certificate part - currently we don't support developers signing their apps (Marketplace does the signing). This part has been discussed with Stephanie, Paul from sec-team and Jonas and it looks like this is what we'll do. Hopefully we'll have bugs for those soon.

(In reply to Kamil Leszczuk [:kamituel] from comment #18) > Hi Henry! > > ACE module is an Access Control Enforcer module, which basically uses rules > stored on a SIM card (or embedded secure element, if that's the case) to > determine whether an app has rights to receive EVT_TRANSACTION. > > In order to make such a decision, ACE needs two values: > > - AID of the SIM card applet that originated the event. This AID is stored > in the event itself. > - Hash of the certificate using which the Gaia application (i.e. Wallet > app) has been signed. > This can be gotten from the app package itself. > > Those two values will be matched against the access rules stored on the SIM > card - those rules are basically "AID <-> cert hash" pairs. So you see that > the application doesn't need to be running in order to make the access > control check. > > Note: one piece missing in this picure is the certificate part - currently > we don't support developers signing their apps (Marketplace does the > signing). This part has been discussed with Stephanie, Paul from sec-team > and Jonas and it looks like this is what we'll do. Hopefully we'll have bugs > for those soon. Hi Kamil, According to the information needed by ACE module, a simple solution along with the filter addressed by this bug might be feasible: We can create a configurator for the payment system message, say 'nfc-payment', then implement the filter function as checking the AID reported by gonk and the cert hash, which is supposed to be able to obtain from mainfest url (could you confirm?). The only thing needed to modify is the filter function should be async version since fetching manifest information seems to be a async operation.

> In order to make such a decision, ACE needs two values: > > - AID of the SIM card applet that originated the event. This AID is stored > in the event itself. > - Hash of the certificate using which the Gaia application (i.e. Wallet > app) has been signed. > This can be gotten from the app package itself. > > Those two values will be matched against the access rules stored on the SIM > card - those rules are basically "AID <-> cert hash" pairs. So you see that > the application doesn't need to be running in order to make the access > control check. I am wondering where do the AID and cert hash come from? Are these two values written to SIM card when installing the gaia app? I am worry about if someone steals the signed hash code from a certificated wallet app, and appends that hash code to the fake app.

> We can create a configurator for the payment system message, > say 'nfc-payment', then implement the filter function as checking > the AID reported by gonk and the cert hash, which is supposed > to be able to obtain from mainfest url (could you confirm?). > I am wondering where do the AID and cert hash come from? Are these two values written to SIM card when installing the gaia app? Answering both questions: developer would have a certificate. Using it (a private key) he/she would sign the application (*). Signature and the public part of certificate would be included in the application package. When user installs the application, system would verify the package integrity using both Marketplace signature, as well as developer signature. If the developer signature is correct, hash of the public part of certificate would be computed. Now, when EVT_TRANSACTION is about to be delivered to the application, system would get the AID from the event, and hash from the application, and invoke "isAllowed(aid, hash)" on the access control enforcer. ACE would then check whether this AID-hash pair exists on the SIM card. If so, access is granted. Rules on secure element (SIM or embedded SE) are provisioned by the provider of the secure element (operator in case of SIM card, OEM in case of embedded SE). @Vincent, so you see that even if someone steals the public part of certificate, he/she wouldn't be able to generate the valid signature for malicious app. This would require stealing the private key as well. @Henry, can you tell us more about system message filters - what are those? I've simplified this description a bit, obviously. I'll share details on access control on the mailing list. (*) This would work the same way as current Marketplace signatures work - instead of signing the whole ZIP file, we sign every file inside it separately.

Also, there is a discussion happening on webapps mailing list: https://groups.google.com/forum/#!topic/mozilla.dev.webapi/oER2OLg40pg for the very same topic as we're discussing here.

> @Henry, can you tell us more about system message filters - what are those? Please refer to my WIP V2. Take 'dummy-system-message' for example. I implement a xpcom 'DummySystemMessageConfigurator' with contract id '@mozilla.org/dom/system-messages/configurator/dummy-system-message;1'. Its shouldDispatch function will return true/false/Promise according to the payload of the message. So, you may add a 'nfc-payment' system message and implement its configurator. Its |shouldDispatch| can be implemented as the following: 1) Asynchronously fetch the hash from the manifest URL given by the arguments of shouldDispatch. 2) When the hash is obtained, call the |isAllow| function with AID carried by the message. 3) When all the check is done, call resolve(true/false). Does this filtering function meet your requirement?

I think using system message filters as you described, it'd be possible to add an access control step when sending a message to the Gaia application. Ming, what do you think?

I am ok for Henry's proposal.

Henry, what would be the next steps regarding this? I.e. do you expect to ask for a code review, or does it require some more work? If I can help you some way, just let me know!

(In reply to Kamil Leszczuk [:kamituel] from comment #27) > Henry, what would be the next steps regarding this? I.e. do you expect to > ask for a code review, or does it require some more work? > > If I can help you some way, just let me know! Hi Kamil, I am going to ask for feedback from :fabrice to know if he has any concern about the broadcast filter. Hi Fabrice, Please refer to the attachment which extends SystemMessageConfigurator to introduce a per-message broadcast filter. The use case for NFC is described in comment 24. For any other message which would like to have a sync/async broadcast filter could implement its configurator. Do you have any concern regarding my proposal? Thanks!

Comment on attachment 8462416 [details] [diff] [review] WIP V2 (supports async filtering) Review of attachment 8462416 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/installer/package-manifest.in @@ +554,5 @@ > @BINPATH@/components/SystemMessageInternal.js > @BINPATH@/components/SystemMessageManager.js > @BINPATH@/components/SystemMessageManager.manifest > +@BINPATH@/components/DummySystemMessage.manifest > +@BINPATH@/components/DummySystemMessageConfigurator.js I guess this is test code? We rather want to update the default configurator at https://mxr.mozilla.org/mozilla-central/source/dom/messages/SystemMessageInternal.js?force=1#66 ::: dom/messages/SystemMessageInternal.js @@ +245,5 @@ > + return; > + } > + > + // Take the advantage of arrow function's lexical 'this' > + // so that we have no worries about 'this'. nit: no need for this comment. @@ +284,5 @@ > + .then(shouldDispatch => { > + if (shouldDispatch) { > + doDispatch(); > + } > + }); why do you need the Promise.resolve() here? Why not just shouldDispatchFunc(...).then(shouldDispatch => { ... }) ? ::: dom/messages/interfaces/nsISystemMessagesInternal.idl @@ +68,5 @@ > + /* > + * A broadcast filter for a specific message type. > + * > + * @return |true| or |false| for synchronous filtering to indicate wheterh this > + * message should be dispatched to manifest/page; or a promise which nit: trailing whitespace @@ +69,5 @@ > + * A broadcast filter for a specific message type. > + * > + * @return |true| or |false| for synchronous filtering to indicate wheterh this > + * message should be dispatched to manifest/page; or a promise which > + * return |true| or |false| on resolve. I'm not a big fan of returning either a boolean or a promise. How does the caller know what to expect? I would go with always returning a promise. It's always possible to have a sync-like implementation as: shoudlDispatch: function(...) { let deferred = Promise.defer(); deferred.resolve(true); return deferred; }

Thanks for the review! Please see my reply below. Besides, other than the implementation itself, do you have any generality or flexibility concern for this design? Thanks! (In reply to Fabrice Desré [:fabrice] from comment #29) > Comment on attachment 8462416 [details] [diff] [review] > WIP V2 (supports async filtering) > > Review of attachment 8462416 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: b2g/installer/package-manifest.in > @@ +554,5 @@ > > @BINPATH@/components/SystemMessageInternal.js > > @BINPATH@/components/SystemMessageManager.js > > @BINPATH@/components/SystemMessageManager.manifest > > +@BINPATH@/components/DummySystemMessage.manifest > > +@BINPATH@/components/DummySystemMessageConfigurator.js > > I guess this is test code? We rather want to update the default configurator > at > https://mxr.mozilla.org/mozilla-central/source/dom/messages/ > SystemMessageInternal.js?force=1#66 > > ::: dom/messages/SystemMessageInternal.js I don't catch this... Could you please detail more? > > why do you need the Promise.resolve() here? Why not just > shouldDispatchFunc(...).then(shouldDispatch => { ... }) ? > Because the filter function can simply return true/false if it's a fast and synchronous operation to simplify the use of this filter. 'shouldDispatchFunc(...).then' won't work in this case. > > @@ +69,5 @@ > > + * A broadcast filter for a specific message type. > > + * > > + * @return |true| or |false| for synchronous filtering to indicate wheterh this > > + * message should be dispatched to manifest/page; or a promise which > > + * return |true| or |false| on resolve. > > I'm not a big fan of returning either a boolean or a promise. How does the > caller know what to expect? I would go with always returning a promise. It's > always possible to have a sync-like implementation as: > shoudlDispatch: function(...) { > let deferred = Promise.defer(); > deferred.resolve(true); > return deferred; > } Since the caller would use Promise.resolve() to concatenate the return value so it doesn't matter that the callee returns a boolean or a Promise. However, this is not a technical issue but a design issue. I am neutral to either of these two designs. One is to make the API easy to use but a little ambiguous and the other makes it a clear API but always rely on Promise even if it is a synchronous call.

Hi Henri, (In reply to Henry Chang [:henry] from comment #30) > Thanks for the review! Please see my reply below. > > Besides, other than the implementation itself, > do you have any generality or flexibility concern > for this design? I don't have objections on the general design, no. > > ::: b2g/installer/package-manifest.in > > @@ +554,5 @@ > > > @BINPATH@/components/SystemMessageInternal.js > > > @BINPATH@/components/SystemMessageManager.js > > > @BINPATH@/components/SystemMessageManager.manifest > > > +@BINPATH@/components/DummySystemMessage.manifest > > > +@BINPATH@/components/DummySystemMessageConfigurator.js > > > > I guess this is test code? We rather want to update the default configurator > > at > > https://mxr.mozilla.org/mozilla-central/source/dom/messages/ > > SystemMessageInternal.js?force=1#66 > > > > ::: dom/messages/SystemMessageInternal.js > > I don't catch this... Could you please detail more? What I mean is that I think that we don't want to ship this DummySystemMessageConfigurator component. It looks like it's used as fallback, but we already have a fallback configurator object that only needs to get the new method. > Since the caller would use Promise.resolve() to concatenate > the return value so it doesn't matter that the callee returns > a boolean or a Promise. However, this is not a technical issue > but a design issue. I am neutral to either of these two designs. > One is to make the API easy to use but a little ambiguous and > the other makes it a clear API but always rely on Promise even > if it is a synchronous call. Yep, I really don't want a polymorphic return type. Please just return a Promise, I doubt that this will cost us a lot in the "pseudo sync" case since you are anyway resolving a promise with the result.

(In reply to Fabrice Desré [:fabrice] from comment #31) > Hi Henri, > > (In reply to Henry Chang [:henry] from comment #30) > > Thanks for the review! Please see my reply below. > > > > Besides, other than the implementation itself, > > do you have any generality or flexibility concern > > for this design? > > I don't have objections on the general design, no. > Thanks! > > > > ::: b2g/installer/package-manifest.in > > > @@ +554,5 @@ > > > > @BINPATH@/components/SystemMessageInternal.js > > > > @BINPATH@/components/SystemMessageManager.js > > > > @BINPATH@/components/SystemMessageManager.manifest > > > > +@BINPATH@/components/DummySystemMessage.manifest > > > > +@BINPATH@/components/DummySystemMessageConfigurator.js > > > > > > I guess this is test code? We rather want to update the default configurator > > > at > > > https://mxr.mozilla.org/mozilla-central/source/dom/messages/ > > > SystemMessageInternal.js?force=1#66 > > > > > > ::: dom/messages/SystemMessageInternal.js > > > > I don't catch this... Could you please detail more? > > What I mean is that I think that we don't want to ship this > DummySystemMessageConfigurator component. It looks like it's used as > fallback, but we already have a fallback configurator object that only needs > to get the new method. > It's not a fallback but for testing. I hope to have a test case for 'dummy-system-message'. It's a real XPCOM configurator to test the custom configurator but not a fallback one. If we cannot have such a component, I could only add a test case whenever the first configurator which implementing 'shouldDispatch' is created. > > Since the caller would use Promise.resolve() to concatenate > > the return value so it doesn't matter that the callee returns > > a boolean or a Promise. However, this is not a technical issue > > but a design issue. I am neutral to either of these two designs. > > One is to make the API easy to use but a little ambiguous and > > the other makes it a clear API but always rely on Promise even > > if it is a synchronous call. > > Yep, I really don't want a polymorphic return type. Please just return a > Promise, I doubt that this will cost us a lot in the "pseudo sync" case > since you are anyway resolving a promise with the result. No problem! I will address all the comments and ask for review for the next revision. Thanks!

Full try run: https://tbpl.mozilla.org/?tree=Try&rev=7db54e154aeb

Comment on attachment 8468185 [details] [diff] [review] SysMsgFilter.patch Review of attachment 8468185 [details] [diff] [review]: ----------------------------------------------------------------- Nice, much simpler! ::: dom/messages/SystemMessageInternal.js @@ +275,3 @@ > } > + > + shouldDispatchFunc(aPage.pageURL, aPage.manifestURL, aType, aMessage, aExtra) and remember to swap them here too. @@ +275,4 @@ > } > + > + shouldDispatchFunc(aPage.pageURL, aPage.manifestURL, aType, aMessage, aExtra) > + .then(shouldDispatch => { nit: then(aShouldDispatch => {... ::: dom/messages/interfaces/nsISystemMessagesInternal.idl @@ +70,5 @@ > + * > + * @return Promise which resolves with |true| or |false| to indicate if > + * we want to dispatch this message. > + */ > + jsval shouldDispatch(in DOMString pageURL, in DOMString manifestURL, nit: can you swap the order of pageURL and manifestURL arguments?

Thanks Fabrice! I'll address all the comments. Thanks for your review! (In reply to Fabrice Desré [:fabrice] from comment #36) > Comment on attachment 8468185 [details] [diff] [review] > SysMsgFilter.patch > > Review of attachment 8468185 [details] [diff] [review]: > ----------------------------------------------------------------- > > Nice, much simpler! > > ::: dom/messages/SystemMessageInternal.js > @@ +275,3 @@ > > } > > + > > + shouldDispatchFunc(aPage.pageURL, aPage.manifestURL, aType, aMessage, aExtra) > > and remember to swap them here too. > > @@ +275,4 @@ > > } > > + > > + shouldDispatchFunc(aPage.pageURL, aPage.manifestURL, aType, aMessage, aExtra) > > + .then(shouldDispatch => { > > nit: then(aShouldDispatch => {... > > ::: dom/messages/interfaces/nsISystemMessagesInternal.idl > @@ +70,5 @@ > > + * > > + * @return Promise which resolves with |true| or |false| to indicate if > > + * we want to dispatch this message. > > + */ > > + jsval shouldDispatch(in DOMString pageURL, in DOMString manifestURL, > > nit: can you swap the order of pageURL and manifestURL arguments?

The tree Try is closed so I cannot get a final full try run before flagging checkin-needed....

Full try run: https://tbpl.mozilla.org/?tree=Try&rev=ed726e5fe750