785072 - B2G RIL: Expose Integrated Circuit Card Identifier (ICCID) to certified apps through DOM API

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Marcelino Veiga Tuimil [:sonmarce]]

We need to expose the ICCID to content, as it is required for cost control application to uniquely identify a SIM. There are a number of scenarios where the information must be linked to the SIM, and for this reason we need a way to identify the SIM. For example, check balance and top-up functionalities are directly related to the SIM inserted in the phone, so all data we need to store in the phone must be associated to the SIM.

Comment 1

[Philipp von Weitershausen [:philikon]]

Can you explain how this relates to bug 782603? Would the IMSI be sufficient for your purposes or do you need the ICC ID?

Comment 2

[Marcelino Veiga Tuimil [:sonmarce]]

Well, as it seem it is not possible to use IMSI because of privacy problems, we could use ICCID instead. What we need is a way to identify the SIM, so ICCID is ok for us.

Comment 3

[Philipp von Weitershausen [:philikon]]

(In reply to Marcelino Veiga Tuimil from comment #2)
> Well, as it seem it is not possible to use IMSI because of privacy problems,
> we could use ICCID instead. What we need is a way to identify the SIM, so
> ICCID is ok for us.

How does the ICCID solve the privacy problem that the IMSI can't?

Comment 4

[Marcelino Veiga Tuimil [:sonmarce]]

You can see comments in https://bugzilla.mozilla.org/show_bug.cgi?id=782603

Comment 5

[Daniel Coloma:dcoloma]

ICCID identifies the piece of Hardware and hence it is not related at all with the customer subscription-

IMSI is linked to the mobile subscription and having access to it opens a lot of security concerns, such as http://en.wikipedia.org/wiki/IMSI-catcher

Comment 6

[Philipp von Weitershausen [:philikon]]

(In reply to Daniel Coloma from comment #5)
> ICCID identifies the piece of Hardware and hence it is not related at all
> with the customer subscription-
> 
> IMSI is linked to the mobile subscription and having access to it opens a
> lot of security concerns, such as http://en.wikipedia.org/wiki/IMSI-catcher

That makes sense, thanks!

Comment 7

[Marshall Culpepper [:marshall_law]]

(In reply to Daniel Coloma from comment #5)
> ICCID identifies the piece of Hardware and hence it is not related at all
> with the customer subscription-
> 
> IMSI is linked to the mobile subscription and having access to it opens a
> lot of security concerns, such as http://en.wikipedia.org/wiki/IMSI-catcher

FWIW, Apple used to expose a UDID (unique device ID) to apps in it's Objective-C API, but removed it because of privacy concerns. 

http://lifehacker.com/5898282/what-a-udid-is-and-why-apples-rejecting-apps-that-want-yours

This is slightly different, because the APIs will require a higher level of permission (and not be available to just any old web app), but we should keep that in mind. These days, iOS apps will use app-install specific generated UUIDs to track users, but at least.

I have a few follow up questions:
- Other than SIM-specific queries such as cost control, balance, etc.. What (if any) data would be stored on the phone that is tied to the ICCID?
- Would the user swapping the SIM cause any problems with this approach?
- Presumably you plan to store the ICCID in IndexedDB? Would any protections be needed to ensure it is not in the clear and easily pulled from /data/local?

Comment 8

[Marcelino Veiga Tuimil [:sonmarce]]

For a better understanding of the scenario we want to implement, you can see https://wiki.mozilla.org/images/8/87/Credit_and_Data_Usage_App.pdf (pages 47-52).

Comment 9

[Daniel Coloma:dcoloma]

(In reply to Marshall Culpepper [:marshall_law] from comment #7)
> (In reply to Daniel Coloma from comment #5)
> > ICCID identifies the piece of Hardware and hence it is not related at all
> > with the customer subscription-
> > 
> > IMSI is linked to the mobile subscription and having access to it opens a
> > lot of security concerns, such as http://en.wikipedia.org/wiki/IMSI-catcher
> 
> FWIW, Apple used to expose a UDID (unique device ID) to apps in it's
> Objective-C API, but removed it because of privacy concerns. 
> 
> http://lifehacker.com/5898282/what-a-udid-is-and-why-apples-rejecting-apps-
> that-want-yours
> 
> This is slightly different, because the APIs will require a higher level of
> permission (and not be available to just any old web app), but we should
> keep that in mind. These days, iOS apps will use app-install specific
> generated UUIDs to track users, but at least.
> 

It is not only different because of that, but because UDID identifies the device and ICCID identifies the SIM Card. 

> I have a few follow up questions:
> - Other than SIM-specific queries such as cost control, balance, etc.. What
> (if any) data would be stored on the phone that is tied to the ICCID?

Operator variant might also benefit from this parameter and check every time a new SIM Card is inserted if new network configuration parameters (APNs, voice mail numbers..)

> - Would the user swapping the SIM cause any problems with this approach?

No, the other way around, this is to provide a way to handle that scenario

> - Presumably you plan to store the ICCID in IndexedDB? Would any protections
> be needed to ensure it is not in the clear and easily pulled from
> /data/local?

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Marcelino, how are we actually going to use this API? Sorry, I just need a short answer to make a blocking call and the pdf is pretty long. Just a short description about which UI features in which apps would use this new property would be very helpful.

Comment 11

[Lucas Adamski [:ladamski]]

Does this need to be exposed to privileged & certified, or can it be for certified only?

Comment 12

[Marcelino Veiga Tuimil [:sonmarce]]

(In reply to Jonas Sicking (:sicking) from comment #10)
> Marcelino, how are we actually going to use this API? Sorry, I just need a
> short answer to make a blocking call and the pdf is pretty long. Just a
> short description about which UI features in which apps would use this new
> property would be very helpful.

You have to read just a few pages, not the whole document. Anyway I will try to summarize. This is the functionality we want to provide: check balance, top-up, show outgoing call minutes, SMS sent & data usage, set consumption/usage limits, reminders & alerts. As you can see, the whole thing is tied to a SIM card, and if we change the card all the information we have stored is useless, unless we have a way to identify the SIM card, so that we can store information together with SIM id.

Comment 13

[Daniel Coloma:dcoloma]

(In reply to Jonas Sicking (:sicking) from comment #10)
> Marcelino, how are we actually going to use this API? Sorry, I just need a
> short answer to make a blocking call and the pdf is pretty long. Just a
> short description about which UI features in which apps would use this new
> property would be very helpful.

After the F2F session in Sao Paulo, can we remove the [blocked-on-input Marcelino Veiga Tuimil] flag?

Comment 14

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 1: DOM API - WIP.

Comment 15

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 2: RIL impl - WIP.

Comment 16

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 3: Tests - WIP.

Comment 17

[Marcelino Veiga Tuimil [:sonmarce]]

(In reply to Jonas Sicking (:sicking) from comment #10)
> Marcelino, how are we actually going to use this API? Sorry, I just need a
> short answer to make a blocking call and the pdf is pretty long. Just a
> short description about which UI features in which apps would use this new
> property would be very helpful.

I am not sure about the protocol, I guess 'blocked-on-input' means you want I give you feedback. As I already did it, I removed blocked-on-input.

Comment 18

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

Comment on attachment 657829 [details] [diff] [review]
Part 2: RIL impl - WIP.

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

::: dom/system/gonk/ril_worker.js
@@ +1070,5 @@
>      Buf.writeString(aid ? aid : this.aid);
>      Buf.sendParcel();
>    },
>  
> +  /** 

trailing white space,

@@ +1077,5 @@
> +  getICCID: function getICCID() {
> +    function callback() {
> +      let length = Buf.readUint32();
> +      let pairs = length * 2;
> +      this.iccInfo.iccid = GsmPDUHelper.readSwappedNibbleBcdString(pairs);

Here the length is the number of characters, 
for example, 'ab' should be of string of length 2, 
and they should be 1 pair,
so pairs should be length / 2,

@@ +1078,5 @@
> +    function callback() {
> +      let length = Buf.readUint32();
> +      let pairs = length * 2;
> +      this.iccInfo.iccid = GsmPDUHelper.readSwappedNibbleBcdString(pairs);
> +      

trailing white space

@@ +1084,5 @@
> +      if (this.iccInfo.iccid) {
> +        this._handleICCInfoChange();
> +      }
> +    }
> +    

ditto

Comment 19

[Lucas Adamski [:ladamski]]

Seems like these use-cases are certified-only.  If so the bug title is confusing as it should be clear that privileged apps won't have access.

Comment 20

[Philipp von Weitershausen [:philikon]]

(In reply to Lucas Adamski from comment #19)
> Seems like these use-cases are certified-only.

Probably. I used "privileged" as a general concept, not in the specific sense of the security model that's under development. So whatever security level navigator.mozMobileConnection will be exposed at is what we're talking about here. We could even create tighter security for navigator.mozMobileConnection.iccInfo if desired.

Comment 21

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 1: DOM API - V1.

Adds the `iccid` property to nsIDOMMozMobileICCInfo object.

Comment 22

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 2: RIL impl - V2.

RIL implementation. Adds a function in ril_worker in charge of reading that id from the ICC card. Some feedback from Yoshi will be also welcome (since I've change a bit the `_getPathIdForICCRecord` function).

Comment 23

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 657830 [details] [diff] [review]
Part 3: Tests - WIP.

I'm afraid that the emulator lacks of support for reading ICC records. I've not been able to find any information about that. Have you guys some information regarding how to fetch ICC records on the emulator?

Comment 24

[Marshall Culpepper [:marshall_law]]

Comment on attachment 658497 [details] [diff] [review]
Part 2: RIL impl - V2.

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

Punting review to Vicamo, as the SMS filetype changes outside my knowledge area. Just one comment below.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1021,5 @@
>      let oldIcc = this.rilContext.icc;
>      this.rilContext.icc = message;
> +    if (oldIcc &&
> +        (oldIcc.iccid == message.iccid) &&
> +        (oldIcc.mcc == message.mcc || oldIcc.mnc == message.mnc)) {

This logic looks wrong.. shouldn't all of the fields be the same to early escape from this function?

In general, it would probably be easier and more efficient to just check if any field changed rather than if all fields are the same, i.e:

let infoChanged = !oldIcc ||
    oldIcc.iccid != message.iccid ||
    oldIcc.mcc != message.mcc ||
    oldIcc.mnc != message.mnc;


if (infoChanged) {
  ppmm.broadcastAsyncMessage(..)
}

Comment 25

[Marshall Culpepper [:marshall_law]]

Comment on attachment 657830 [details] [diff] [review]
Part 3: Tests - WIP.

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

Is the emulator actually reporting 0 for it's ICCID? According to the emulator's SIM card code, there is a hard coded ICCID stored. The definition looks like:

static const byte_t _const_iccid[10] = {
    0x98, 0x10, 0x14, 0x30, 0x12, 0x11, 0x81, 0x15, 0x70, 0x02
};

See more here:
https://github.com/android/platform_external_qemu/blob/master/telephony/sim_card.c#L299

Comment 26

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ok, so sounds like this is required in order to detect the user switching simcard. If that's the case then this is indeed a blocker.

Comment 27

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

Comment on attachment 657830 [details] [diff] [review]
Part 3: Tests - WIP.

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

As Marshall already pointed out the source,
so I don't give feedback here.

Comment 28

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

Comment on attachment 658497 [details] [diff] [review]
Part 2: RIL impl - V2.

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

::: dom/system/gonk/ril_worker.js
@@ +1091,5 @@
>      Buf.writeString(aid ? aid : this.aid);
>      Buf.sendParcel();
>    },
>  
> +  /** 

trailing ws

@@ +1092,5 @@
>      Buf.sendParcel();
>    },
>  
> +  /** 
> +   * Read the IICD from the ICC card.

typo: should be ICCID.

@@ +1098,5 @@
> +  getICCID: function getICCID() {
> +    function callback() {
> +      let length = Buf.readUint32();
> +      let pairs = length/2;
> +      this.iccInfo.iccid = GsmPDUHelper.readSwappedNibbleBcdString(pairs);

space between operators.
Or maybe you don't have to add another variable 'pairs',
just divide it directly in the function.

@@ +1099,5 @@
> +    function callback() {
> +      let length = Buf.readUint32();
> +      let pairs = length/2;
> +      this.iccInfo.iccid = GsmPDUHelper.readSwappedNibbleBcdString(pairs);
> +

Should have Buf.readStringDelimeter(length) here.
The original format of this response is of type String.

Comment 29

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

Comment on attachment 658497 [details] [diff] [review]
Part 2: RIL impl - V2.

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

::: dom/system/gonk/ril_worker.js
@@ +2327,5 @@
>      if (!app) {
>        return null;
>      }
>  
> +    // Here we handle only file ids that are common to RUIM, SIM, USIM 

trailing ws here.

Comment 30

[Daniel Coloma:dcoloma]

(In reply to Jonas Sicking (:sicking) from comment #26)
> Ok, so sounds like this is required in order to detect the user switching
> simcard. If that's the case then this is indeed a blocker.

Exactly, sorry for not making it clear... and thanks for pointing it out

Comment 31

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 2: RIL impl - V3.

Addressed all comments. Already reviewed by vicamo (r+).

Comment 32

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 3: Tests - V1.

Comment 33

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Marshall Culpepper [:marshall_law] from comment #25)
> Comment on attachment 657830 [details] [diff] [review]
> Part 3: Tests - WIP.
> 
> Review of attachment 657830 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Is the emulator actually reporting 0 for it's ICCID? 

Nope, this patch was only a WIP version. I didn't tested it. 

> See more here:
> https://github.com/android/platform_external_qemu/blob/master/telephony/
> sim_card.c#L299

Thanks for the information Marshall!. Patch updated.

Comment 34

[Andrew Overholt [:overholt]]

Since José is doing the work on this, assigning to him.

Comment 35

[Marshall Culpepper [:marshall_law]]

Comment on attachment 659273 [details] [diff] [review]
Part 3: Tests - V1.

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

Looks good!

::: dom/network/tests/marionette/test_mobile_iccinfo.js
@@ +10,5 @@
>     "connection is instanceof " + connection.constructor);
>  
> +// The emulator's hard coded iccid value.
> +// See it here {B2G_HOME}/external/qemu/telephony/sim_card.c#L299.
> +is(connection.iccInfo.iccid, 89014103211118510720);

Double checking -- this ICCID has been verified?

Comment 36

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Marshall Culpepper [:marshall_law] from comment #35)
> > +is(connection.iccInfo.iccid, 89014103211118510720);
> 
> Double checking -- this ICCID has been verified?

The emulator doesn't like me and I've never been able to run a test on the emulator. This is what I get on the emulator radio log. 

D/AT      (   35): AT> AT+CRSM=176,12258,0,0,10
D/AT      (   35): AT< +CRSM: 144,0,98101430121181157002
D/AT      (   35): AT< OK

Comment 37

[José Antonio Olivera Ortega [:jaoo]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #21)
> Created attachment 658496 [details] [diff] [review]
> Part 1: DOM API - V1.
> 
> Adds the `iccid` property to nsIDOMMozMobileICCInfo object.

Hey Jonas, could you take a look at it please? Thanks.

Comment 38

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 658496 [details] [diff] [review]
Part 1: DOM API - V1.

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

I thought that we had elsewhere opted to not expose the ICCD for privacy reasons. But I don't know these things well enough to have an informed opinion. API-wise this looks good though.

And since it's only exposed on .mobileConnection it's only exposed to certified apps, right?

Comment 39

[Philipp von Weitershausen [:philikon]]

(In reply to Jonas Sicking (:sicking) from comment #38)
> I thought that we had elsewhere opted to not expose the ICCD for privacy
> reasons.

You're probably remembering the IMSI, not the ICCID.

> And since it's only exposed on .mobileConnection it's only exposed to
> certified apps, right?

Right.

Comment 40

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Sounds good. I very well could have been thinking of IMSI.

Comment 41

[José Antonio Olivera Ortega [:jaoo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb6bf17786b0
https://hg.mozilla.org/integration/mozilla-inbound/rev/9935e37117f5
https://hg.mozilla.org/integration/mozilla-inbound/rev/2de0cda36dae

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bb6bf17786b0
https://hg.mozilla.org/mozilla-central/rev/9935e37117f5
https://hg.mozilla.org/mozilla-central/rev/2de0cda36dae