Closed Bug 840780 Opened 7 years ago Closed 7 years ago

(webicc) WebICC - Secure Elements

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: dgarnerlee, Assigned: psiddh)

References

()

Details

(Keywords: dev-doc-needed)

Attachments

(4 files, 3 obsolete files)

Track WebICC feature/bug to add Secure Elements functionality to the DOM tree. This is separate from Bug 674741 (webnfc).
Assignee: nobody → psiddh
Component: DOM → DOM: Device Interfaces
Attachment #713634 - Flags: review?(vyang)
Attachment #713636 - Flags: review?(vyang)
Attachment #713637 - Flags: review?(vyang)
Attachment #713638 - Flags: review?(vyang)
Comment on attachment 713634 [details] [diff] [review]
New RIL interface changes

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

Do we really have to modify data structure definitions here?  R- for two reasons:

1) You can always add new proprietary parcel types without revealing their native data structure definitions to public.  Struct RIL_SIM_IO_v5 is for internal reference between libril & libril-reference.

2) There is no ICC secure channel support in AOSP.  I don't know what will it looks like in the end and touching native interfaces here & now doesn't seem to me a good idea.
Attachment #713634 - Flags: review?(vyang) → review-
Comment on attachment 713638 [details] [diff] [review]
DOM changes to support managing of Logical channels to UICC

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

Should be reviewed by DOM peers.

::: dom/icc/src/IccManager.cpp
@@ +151,5 @@
>  
> +NS_IMETHODIMP
> +IccManager::IccOpenChannel(const nsAString& aAid, nsIDOMDOMRequest** aRequest)
> +{
> +  *aRequest = nullptr;

You don't need this.

@@ +163,5 @@
> +  return NS_OK;
> +}
> +
> +NS_IMETHODIMP
> +IccManager::IccExchangeAPDU(PRInt32 aChannel, const jsval& aApdu, nsIDOMDOMRequest** aRequest)

No more PR* types, see bug 579517.
Attachment #713638 - Flags: review?(vyang) → review?(mounir)
Comment on attachment 713638 [details] [diff] [review]
DOM changes to support managing of Logical channels to UICC

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

As Vicamo pointed, no need to set *aRequest to nullptr at the beginning of the methods and you should no longer use PRInt32 but int32_t instead.

r=me with all the mentionned changes applied.

::: dom/icc/src/IccManager.cpp
@@ +159,5 @@
> +  }
> +
> +  nsresult rv = mProvider->IccOpenChannel(GetOwner(), aAid, aRequest);
> +  NS_ENSURE_SUCCESS(rv, rv);
> +  return NS_OK;

Do simply:
return mProvider->IccOpenChannel(...);

@@ +173,5 @@
> +  }
> +
> +  nsresult rv = mProvider->IccExchangeAPDU(GetOwner(), aChannel, aApdu, aRequest);
> +  NS_ENSURE_SUCCESS(rv, rv);
> +  return NS_OK;

ditto

@@ +187,5 @@
> +  }
> +
> +  nsresult rv = mProvider->IccCloseChannel(GetOwner(), aChannel, aRequest);
> +  NS_ENSURE_SUCCESS(rv, rv);
> +  return NS_OK;

ditto
Attachment #713638 - Flags: review?(mounir) → review+
Thanks to both for the comments on attachment #713638 [details] [diff] [review].
I will work on these changes to remove PR* types.

Vicamo,
Regarding RIL patch (attachment #713634 [details] [diff] [review]), I agree it may not be needed to modify RIL to support Secure transactions. This is only one of many ways of accessing Secure element. 
(SIMalliance Open Mobile API , Seek project for Android has similar proposals). 
Note that we are currently working on alternate way of achieving the secure access. This also means that we need not have to modifyril.h, ril.cpp, ril_consts.js, ril_worker.js, RadioInterfaceLayer.js etc. 

However mozIccManager and other parts of Gecko may still require a change. We will keep you posted on the happenings / new security design and upload another set of patches whenever they are ready.

There is no standardized way of accessing secure element yet. Post Jelly Bean ,4.2 update, there may be a support for secure element access in Android implementation.
Comment on attachment 713637 [details] [diff] [review]
IDL changes to support managing of Logical channels to UICC

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

::: dom/icc/interfaces/nsIDOMIccManager.idl
@@ +267,5 @@
> +   * @param aid
> +   *        The Application Identifier of the Applet to be selected on this channel
> +   * return value : An instance of Channel (channelID) if available or null.
> +   */
> +  nsIDOMDOMRequest iccOpenChannel(in DOMString aid);

How do you get AID here?
I don't know there's a web-api for that.
Just to give a brief overview..

The RIL interfaces could be mapped this way to vendor side

RIL_REQUEST_SIM_OPEN_CHANNEL - AT+CCHO
RIL_REQUEST_SIM_CLOSE_CHANNEL - AT+CCHC
RIL_REQUEST_SIM_SEND_PDU       - AT+CGLA

For AT commands implementation in venodr RIL, refer to 3GPP TS 27.007,8.45, 8.46, 8.43 respectively.

RIL library -->Vendor RIL sends AT command to baseban modem
  AT+CCHO= <<AID of applet to be selected>>
    Note that AID is a hardcoded bytes sent by the Gaia Apps (only Certified apps can know of the AID)
    AID: This is a pre-understood hex byte (keys) array that will be shared by SIM manufacturers  to OEMs say who wish to add secure payment through UICC

>> AT+CCHO = "xxxxxxxx"
<< +CCHO: 12345<CR> OK   //12345, say is the session id


Similarly, while closing
>> AT+CCHC=12345
<< OK

For Send PDU,

AT+CGLA=12345,<<size of APDU>>,<<APDU> // APDU is hex string

>> AT+CGLA=12345,10,"ABCDEF1234"
<< +CGLA: <<APDU response from baseband>>
Attachment #713637 - Flags: review?(vyang) → review+
Comment on attachment 713636 [details] [diff] [review]
B2G-RIL changes to support secure element acces

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

We won't land attachment #713634 [details] [diff] [review], right?

::: dom/system/gonk/ril_worker.js
@@ +5941,5 @@
> +
> +  options.sw1 = Buf.readUint32();
> +  options.sw2 = Buf.readUint32();
> +  options.simResponse = Buf.readString();
> +  if (DEBUG) debug("Setting return values for RIL[RIL_REQUEST_SIM_SIM_ACCESS_CHANNEL]: [" + options.sw1 + "," + options.sw2 + ", " + options.simResponse + "]");

Line wrap please :)
Attachment #713636 - Flags: review?(vyang) → review+
Apparently this breaks B2G Marionette & xpcshell test cases :(
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Hi Vicamo, do you know how can I run these test cases in our setup, so that I can fix it in my next patches. (or) would we able to make anything out from the failed logs otherwise. Pls suggest
(In reply to Siddartha P from comment #14)
> Hi Vicamo, do you know how can I run these test cases in our setup, so that
> I can fix it in my next patches. (or) would we able to make anything out
> from the failed logs otherwise. Pls suggest

https://wiki.mozilla.org/ReleaseEngineering/TryServer
Comment on attachment 713636 [details] [diff] [review]
B2G-RIL changes to support secure element acces

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

::: dom/system/gonk/ril_consts.js
@@ +151,5 @@
>  
> +// UICC Secure Access
> +this.REQUEST_SIM_OPEN_CHANNEL = 121;
> +this.REQUEST_SIM_CLOSE_CHANNEL = 122;
> +this.RIL_REQUEST_SIM_ACCESS_CHANNEL = 123;

REQUEST_SIM_ACCESS_CHANNEL

::: dom/system/gonk/ril_worker.js
@@ +5932,5 @@
> +
> +  // No return value
> +  this.sendDOMMessage(options);
> +};
> +RIL[RIL_REQUEST_SIM_SIM_ACCESS_CHANNEL] = function RIL_REQUEST_SIM_SIM_ACCESS_CHANNEL(length, options) {

REQUEST_SIM_ACCESS_CHANNEL

@@ +5941,5 @@
> +
> +  options.sw1 = Buf.readUint32();
> +  options.sw2 = Buf.readUint32();
> +  options.simResponse = Buf.readString();
> +  if (DEBUG) debug("Setting return values for RIL[RIL_REQUEST_SIM_SIM_ACCESS_CHANNEL]: [" + options.sw1 + "," + options.sw2 + ", " + options.simResponse + "]");

REQUEST_SIM_ACCESS_CHANNEL
Update hg patches used for try.
Attachment #713636 - Attachment is obsolete: true
Attachment #717743 - Flags: review+
Depends on: 853049
Duplicate of this bug: 892427
Hello,

Just to know if the implementation for secure element access is based on SEEK4Android API, SIM Alliance API or GP Secure Access standard?

Thanks and Regards,
JDs
(In reply to Jesus Domingues from comment #25)
> Hello,
> 
> Just to know if the implementation for secure element access is based on
> SEEK4Android API, SIM Alliance API or GP Secure Access standard?
> 
> Thanks and Regards,
> JDs

I would say no. If you want to look at this on the terms of the SIM Alliance API standard, what has been implemented here so far is just:

Session.openLogicalChannel == nsIDOMMozIccManager.iccOpenChannel
Channel.transmit           == nsIDOMMozIccManager.iccExchangeAPDU
Channel.close              == nsIDOMDOMRequest iccCloseChannel

The rest of the API isn't implemented, as such. Also, currently there's no specific permission for this, it all goes on the mobileconnection permission.
Hi, Yes the complete secure element feature is not implemented as part of this change list. 
Please refer to this bug : https://bugzilla.mozilla.org/show_bug.cgi?id=879861 
for further updates on this topic.

Brief code snippet of Secure Elements after adding APIs to FxOS would look like below:

// Requires special Secure element permissions
var secElemMgr = navigator.mozSecureElement;

if (secElemMgr) {
  seProvider = secElemMgr.getSecureElemProvider(secElemMgr.SE_TYPE_UICC, function()) {
	if (seProvider ) {
  	  var session = seProvider.connect(function()) {
    	  if(session.status == OK)
       	     // open the session with UICC
       	     var sessionID = session.openChannel('AID');
       	  else 
              throw exception
       	     .
       	     .
  	   }
        }
}
Flags: sec-review?
Flags: sec-review? → sec-review?(ptheriault)
Keywords: dev-doc-needed
Created a 947787 to track the secreview of this and other bugs related to Secure Element implementation.
Flags: sec-review?(ptheriault)
You need to log in before you can comment on or make changes to this bug.