928775 - [WAP Push] Support WAP Provisioning Bootstrap Authentication for USERNETWPIN

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: RIL, defect)

Description

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Follow up bug for supporting WAP Provisioning Bootstrap Authentication for USERNETWPIN [1].

[1] http://technical.openmobilealliance.org/tech/affiliates/wap/wap-184-provboot-20010314-a.pdf, Clause 6.2.1

Comment 1

[Kai-Chih Hu [:khu]]

Should it be koi+ as well? Does it block 917312 for sure?

Comment 2

[Bruce Huang [:bhuang] (inactive)]

This will currently be out of scope for 1.2 since the platform is not able to support it yet.  My understanding is there are some security concerns with exposing what's needed for this authentication?

Comment 3

[Beatriz Rodríguez [:brg]]

(In reply to Bruce Huang (:bhuang) from comment #2)
> This will currently be out of scope for 1.2 since the platform is not able
> to support it yet.  
Thanks Bruce. Removing the blocker dependency accordingly.

Comment 4

[Ken Chang[:kenkai]]

I wonder if we could create a api to get the user input for this bug. Vincent, what do you think?

Comment 5

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Ken Chang from comment #4)
> I wonder if we could create a api to get the user input for this bug.
> Vincent, what do you think?

FYI we were already discussing about the possible solutions in comments:

https://bugzilla.mozilla.org/show_bug.cgi?id=914685#c5
https://bugzilla.mozilla.org/show_bug.cgi?id=914685#c6
https://bugzilla.mozilla.org/show_bug.cgi?id=914685#c8
https://bugzilla.mozilla.org/show_bug.cgi?id=914685#c9

Comment 6

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

I have another possible solution: add a permission for getting imsi, and provide an API in telephony.

Comment 7

[Vincent Chang[:vchang][changyihsin]]

Maybe we should confirm with vyang ...

Comment 8

[Ken Chang[:kenkai]]

Yoshi, we also need your comments.

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

As jaoo raised in Bug 782603 before,
I don't see any reason we need to expose IMSI to Web.

Comment 10

[Ken Chang[:kenkai]]

Vicamo, is it fine to add a API in Mobile Connection for this bug?

Comment 11

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0001. Add object wrapper for WAP Push system message.

Comment 12

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0002. Provide CP authentication from object function.

Comment 13

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8351316 [details] [diff] [review]
WIP - 0002. Provide CP authentication from object function.

Review of attachment 8351316 [details] [diff] [review]:
-----------------------------------------------------------------

I think all the lines in RadioInterfaceLayer.js can be easily moved into WapPushManager.js.

::: dom/wappush/src/WapPushAuth.manifest
@@ +1,3 @@
>  component {1664b58d-9466-4d15-8682-3efd60bee65f} WapPushMessage.js
>  contract @mozilla.org/dom/system-messages/wrapper/wappush-received;1 {1664b58d-9466-4d15-8682-3efd60bee65f}
> +

nit: redundant empty line

Comment 14

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. Add object wrapper for WAP Push system message.

Comment 15

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. Provide CP authentication from object function.

Address comment 13.

Comment 16

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8355998 [details] [diff] [review]
0002. Provide CP authentication from object function.

Review of attachment 8355998 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, thank you :)

::: dom/wappush/src/gonk/WapPushManager.js
@@ +46,3 @@
>  
> +const WAP_PUSH_CHECK_MSG              = "WapPushMessage:Check";
> +const NS_XPCOM_SHUTDOWN_OBSERVER_ID   = "xpcom-shutdown";

nit: empty line after 'const NS_XPCOM...'

@@ +72,5 @@
> +      if (DEBUG) {
> +        debug("WAP Push message " + msg.name +
> +              " from a content process with no 'wappush' privileges.");
> +      }
> +      return false;

nit: we don't have synced messages here.  Returning null or void is okay.

@@ +236,5 @@
> +   *        }
> +   }
> +   */
> +  check: function WapPushManager_check(aAuthInfo) {
> +    let result = false;

nit: `result` is never referenced.

@@ +237,5 @@
> +   }
> +   */
> +  check: function WapPushManager_check(aAuthInfo) {
> +    let result = false;
> +    let key = "";

nit: assigned a value which is never used.

@@ +238,5 @@
> +   */
> +  check: function WapPushManager_check(aAuthInfo) {
> +    let result = false;
> +    let key = "";
> +    let imsi = CP.Authenticator.formatImsi(gRIL.getRadioInterface(0).rilContext.imsi);

Please file a follow-up bug for WAP Push handling under DSDS and place a TODO comment above.

Comment 17

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Vicamo Yang [:vicamo][:vyang] (PTO Dec. 21 ~ Jan. 5) from comment #16)
> Comment on attachment 8355998 [details] [diff] [review]
> 0002. Provide CP authentication from object function.
> 
> Review of attachment 8355998 [details] [diff] [review]:
> -----------------------------------------------------------------
> @@ +238,5 @@
> > +   */
> > +  check: function WapPushManager_check(aAuthInfo) {
> > +    let result = false;
> > +    let key = "";
> > +    let imsi = CP.Authenticator.formatImsi(gRIL.getRadioInterface(0).rilContext.imsi);
> 
> Please file a follow-up bug for WAP Push handling under DSDS and place a
> TODO comment above.

Thanks for reviewing~

WAP Push support for DSDS is in bug 951999, I'll update the patch based on bug 951999 after it's landed.

Comment 18

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. Add object wrapper for WAP Push system message. V2

Support serviceId introduced by DSDS.

Comment 19

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. Provide CP authentication from object function. V2

Support serviceId introduced by DSDS.

Comment 20

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8356462 [details] [diff] [review]
0002. Provide CP authentication from object function. V2

Review of attachment 8356462 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/wappush/src/gonk/WapPushManager.js
@@ +73,5 @@
> +      if (DEBUG) {
> +        debug("WAP Push message " + msg.name +
> +              " from a content process with no 'wappush' privileges.");
> +      }
> +      return null;

Sorry that I didn't make myself clear.  Please see http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsIMessageManager.idl#152 .  You only need a return value when it's called with a synced message type.  Otherwise, just `return` will do.  So all the return statement in this function don't really need a return value now.

@@ +86,5 @@
> +        requestID: authInfo.requestID,
> +        result: result
> +      });
> +
> +      return result;

ditto, just `return;`.

@@ +89,5 @@
> +
> +      return result;
> +    }
> +
> +    return false;

ditto, simply omit this return statement.

Comment 21

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. Provide CP authentication from object function. V3

Address comment 20.

Comment 22

[Gene Lian (I already quit Mozilla)]

Just for the records, we'd better avoid exposing an object which is able to call some internal functions of IPC helper. This would casue security issues.

Comment 23

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8356461 [details] [diff] [review]
0001. Add object wrapper for WAP Push system message. V2

Review of attachment 8356461 [details] [diff] [review]:
-----------------------------------------------------------------

Clear the review? for now due to comment #22.

::: dom/wappush/src/WapPushAuth.manifest
@@ +1,2 @@
> +component {1664b58d-9466-4d15-8682-3efd60bee65f} WapPushMessage.js
> +contract @mozilla.org/dom/system-messages/wrapper/wappush-received;1 {1664b58d-9466-4d15-8682-3efd60bee65f}

Can we make the file name WapPushAuth.manifest so something more generic? So that we can reasonably keep adding stuff in this manifest in the future. I'd suggest WapPush.manifest.

Comment 24

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. Add object wrapper for WAP Push system message. V3

1. Define interface for wap push message object, per comment 22.
2. Change filename per comment 23.

Comment 25

[Ken Chang[:kenkai]]

Put this bug into backlog.

Comment 26

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8362386 [details] [diff] [review]
0001. Add object wrapper for WAP Push system message. V3

Don't review at this moment until the feature is really needed.

Comment 27

[Vincent Chang[:vchang][changyihsin]]

I will take over to this bug. Free feel to take it back if you are still working on it.

Comment 28

[BMO Automation]

Firefox OS is not being worked on