1146624 - Secure Element - Verify Developer App GUID in Marketplace upon submission

Status: RESOLVED WONTFIX

(Marketplace Graveyard :: Validation, enhancement, P3)

Description

[Garner Lee]

For FirefoxOS Secure Elements, in Bug 973823, https://bugzilla.mozilla.org/show_bug.cgi?id=973823#c59, the developer is to submit an app with a marketplace given GUID, and a developer cert signed GUID, along with the public developer certificate.

If the application is a Secure Element Application (perhaps known to Marketplace upon submission), some additional information needs to be checked by the marketplace. Since the marketplace is signing the whole package, it must also ensure the marketplace app GUID is correctly tied to the developer so it can be rejected.

There's also the possibly of verifying the signature before packaging the whole application for publicaction, even if the Secure Element's "Access Control Enforcer" will also check this signature upon connection to the target Secure Element.

Additionally, since webcrypto doesn't actually handle certificates directly, the actual public key may also need to be exported by Marketplace, although additional JS library code can potentially extract the key at runtime (TBD).

Comment 1

[David Bialer [:dbialer]]

Is NFC with secure element in scope of 2.2?  It looks like it did not land in 2.2 yet?  
I am reading through to catch up on so I may not have this right.

I am not sure of the exact ask of Marketplace.

It seems like this is more of an app review process issue than a product issue and the ask is:  to ensure the signed app for ACE is tied to the GUID, which, if we validate would then be signed by the Marketplace?

To do this we would need to
a) note that the SE privileged API usage is flagged during the app review process.  We need to enhance our manifest validation tools.  Perhaps if nfc permission is used in the manifest?
b) verify that the GUID used for the ACE signing and in the manifest matches the GUID of the app.  I am not sure I understand who does the signing or how we can verify this.
c) sign it again if everything is OK.

To do this we would need to enhance our validation tools to check to see if the SE API is used and possibly have some way to check the certificate?

Comment 2

[Garner Lee]

a) (In reply to David Bialer [:dbialer] from comment #1)
> Is NFC with secure element in scope of 2.2?  It looks like it did not land
> in 2.2 yet?  
> I am reading through to catch up on so I may not have this right.
> 
> I am not sure of the exact ask of Marketplace.
> 
> It seems like this is more of an app review process issue than a product
> issue and the ask is:  to ensure the signed app for ACE is tied to the GUID,
> which, if we validate would then be signed by the Marketplace?
> 
> To do this we would need to
> a) note that the SE privileged API usage is flagged during the app review
> process.  We need to enhance our manifest validation tools.  Perhaps if nfc
> permission is used in the manifest?

Strictly for SE, but can be associated with NFC access. The permission is currently certified only (targeting privilaged), named:
  "permissions": {
    "secureelement-manage": {}
  },

> b) verify that the GUID used for the ACE signing and in the manifest matches
> the GUID of the app.  I am not sure I understand who does the signing or how
> we can verify this.
From previous discussions, the developer gets the GUID from the marketplace, and signs, with their private key, and submits in the app 1) GUID (should be same as marketplace), 2) Signed signature of GUID, and the public dev certificate in the app zip.

Probably something like the following key names is added by the dev to the manifest:
  "guid": "{1234-5678-901234}",
  "dev_signed_guid": "<hex...string>",

Then the whole package (including the dev public key/cert file) is signed (normally?) by marketplace.

> c) sign it again if everything is OK.
> 
> To do this we would need to enhance our validation tools to check to see if
> the SE API is used and possibly have some way to check the certificate?

Yes, I believe so.

The client side SE code is expected use the public key inside the cert (technical reasons, TBD: https://bugzilla.mozilla.org/show_bug.cgi?id=1132749#c1) to do a basic validation currently, whether or not marketplace additionally checks the dev guid sig.

I do believe marketplace, being earlier in the packaging process at the other end, is well suited to also validate the guid sig, and also the certificate (if possible, self signed, etc.).

Comment 3

[Vincent Chang[:vchang][changyihsin]]

(In reply to David Bialer [:dbialer] from comment #1)
> Is NFC with secure element in scope of 2.2?  It looks like it did not land
> in 2.2 yet?  
> I am reading through to catch up on so I may not have this right.

The NFC secure element APIs are landed in 2.2[1]. But they are default off and are for certified app only. These APIs can be used on "Flame + 2.1 partner built" or "Nexus 5 L" with UICC secure element(provided by operator). 

We are going to fix access control enforcer related bugs[2] in v3.0. You can find more detail design architecture and plan in [3][4]. After ACE bugs are fixed, we plan to turn on secure element APIs and make them open for privileged app. In that case, third party can implement their own app using secure element APIs and complementary applet in UICC.    


[1] Bug 879861 - NFC Secure Element Support
[2] Bug 884594 - Support NFC Access Control for Secure Element Access
[3] https://bug884594.bugzilla.mozilla.org/attachment.cgi?id=8555765
[4] https://docs.google.com/document/d/1EuTFSefQRFTDxF1U2CYvo2IP0jVFl18AsYCdMRu_9hg/edit

Comment 4

[Andrew Williamson [:eviljeff]]

(In reply to Vincent Chang[:vchang] from comment #3)
> (In reply to David Bialer [:dbialer] from comment #1)
> > Is NFC with secure element in scope of 2.2?  It looks like it did not land
> > in 2.2 yet?  
> > I am reading through to catch up on so I may not have this right.
> 
> The NFC secure element APIs are landed in 2.2[1]. But they are default off
> and are for certified app only. These APIs can be used on "Flame + 2.1
> partner built" or "Nexus 5 L" with UICC secure element(provided by
> operator). 

so, just to clarify, for Marketplace (which doesn't support certified apps, only privileged and web) this is a 3.0 feature?

Comment 5

[Andrew Williamson [:eviljeff]]

From what I'm reading here, the possible checks that the validator could make are automated:
a) that the guid manifest field equals the GUID we hold for that app
b) that the dev_signed_guid* is valid based on the public cert in the zip and the guid 
and I'm assuming that both of these would only be done if the secure-element permission is specified.  Manual checks on the validity of manifest fields don't scale particularly well and I'd be worried about them being missed if they are that important.
*(is that manifest field name finalised?  Its a bit wordy)

Some additional possible issues/questions:
1) if we make the validator do the above checks for secure-element apps then the validator will fail on the first submission (where they need to submit to get the guid).  If we make it only a warning (rather than an error) then its likely it'll get ignored/lost amongst the CSP warnings.
2) so to counter 1) - what are the issues/risks with letting a developer just generate their own guid and Marketplace, if finding one in the manifest, using that as the guid in the model, etc?
3) developers have this habit of deleting and resubmitting their apps - especially before approval, and more so while trying to successfully submit an app for approval.  This is fine normally, but each time they get a new guid.
4) Reviewer signed apps, that are installed for testing before an app is approved during the review, have an entirely different, version unique, id in ids.json - **not the GUID of the app used for the approved public version**

Comment 6

[David Bialer [:dbialer]]

[Blocking Requested - why for this release]:

Thanks.  Not much we can do until its privileged and in v3 since Marketplace can't install certified apps.  Marked it for v3 consideration.

Comment 7

[Garner Lee]

(In reply to Andrew Williamson [:eviljeff] from comment #5)

> *(is that manifest field name finalised?  Its a bit wordy)
The WIP code is using "guid_sig", though it's a bit less explicit about who actually signed it. I'll happily go with anything that is clearer.

Comment 8

[Krzysztof Mioduszewski[:tauzen]]

We agreed that Marketplace needs to verify the GUID placed in the manifest file by the developer. Gecko code will trust that the GUID in manifest file is valid.

As suggested by Garner and confirmed by Andrew in comment 5 Marketplace could also verify the dev_signed_guid using the CERT file added by the developer to the app package. Since Gecko code already trusts the GUID value, I don't see any reason why it should not trust that the dev_signed_guid and CERT are valid. If Gecko assumes that CERT is valid it considerably simplifies the code needed on Access Control Enforcer side. In Bug 1132749 comment 4 you can see that, validating the dev_signed_guid using the CERT involves importing PKI libs (rather big), since Web Crypto does not support public key extraction out of the CERT.

If there aren't any problems from security perspective with GECKO trusting the dev_signed_guid and CERT file, we would only need to compute SHA1 hash of CERT file and compare it with the value on the SIM card. Stephanie what do you think about this? Do you think this is possible? If the Marketplace would be validating the GUID and dev_signed_guid using the CERT, and Gecko would trust this we would have a simplified developer signature mechanism similar to Android.

Comment 9

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

FW the needinfo to Paul since Stephanie is on PTO.

Comment 10

[Paul Theriault [:pauljt] (no longer reading bugmail)]

I think this is OK-ish. The difference here is that we can no longer prevent sideloading of SE apps, since an attacker can just copy the required manifest entries from a signed app. 

Just to capture my logic and assumptions here:

Assumptions:
- Marketplace ensures guid can only be used by the developer that requested it (i.e the guid is tied to their Firefox Account)
- Marketplace validates the signature of the guid, which proves to marketplace that the developer owns the private key associated with the public key in the cert

Threats:

1. Attacker copies guid, dev_signed_guid and cert from an approved app. Marketplace will reject this since guid is associated with a Firefox account the attacker doesn't control.

2. Attacker signs with their own guid with their own cert. Gecko ACE will compare the hash of the cert and reject prevent the connection

3. Attacker could side-load an app with the valid data copied from a signed app. This approach will NOT prevent this threat since ACE no longer validates the cert.

Sideloading is difficult to restrict anyways. (If developer has access to a rooted device, they obviously can just remove the ACE entirely). So this is may be acceptable.

Comment 11

[Krzysztof Mioduszewski[:tauzen]]

We already agreed that we cannot secure anything if app is side-loaded. ACE Web-crypto checks are irrelevant, as you wrote in threat#3 valid data can be just copied from signed app. We actually have bug 1156710 to disable ACE checks completely when the phone is rooted (side-loading is possible).

So what do we need to do to move from OK-ish to green light for implementation? ;) Is it safe to assume that signature over guid verification will be done in this bug as part of marketplace submission process for web apps with secure element permission? If yes, this would mean that in Bug 1132749 we won't need to do full signature verification and just compute cert hash and compare it with the value on SIM card.

Comment 12

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Krzysztof Mioduszewski[:tauzen] from comment #11)
> We already agreed that we cannot secure anything if app is side-loaded. ACE
> Web-crypto checks are irrelevant, as you wrote in threat#3 valid data can be
> just copied from signed app. We actually have bug 1156710 to disable ACE
> checks completely when the phone is rooted (side-loading is possible).

Note that side-loading != rooted. You can sideload apps on a production device without rooting. I had assumed that ACE would treat side-loaded apps differently. (For example, not all manifest fields are installed when sideloading). But on refection, we do have a blacklist of non-sideloadable permissions. [1] So I guess we could achieve equivalent control by blacklisting sideloading apps with secure-element permission.  

Whether or not you control side-loading is more up to you I think (as the only consumer of this API currently). What is the ACE enforcing if its just checking for the presence of a certificate that can be added to any sideloaded app? I guess it stops third-parties distributing apps in marketplace that talk to a specific applet. 

Ultimately I don't know what sort of information is available on SIM applets so its hard for me to comment on the security decision here. But maybe its wise to err on the side of caution and prohibit sideloading of secure-element apps on production devices. Then it matters less if you check the certificate in the marketplace, though the ACE seems somewhat pointless to me.

[1] http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#821

> 
> So what do we need to do to move from OK-ish to green light for
> implementation? ;) Is it safe to assume that signature over guid
> verification will be done in this bug as part of marketplace submission
> process for web apps with secure element permission? 

Not unless we get confirmation that marketplace is OK to implement that. (or did you intend to provide a patch for marketplace?) Wil, what do you think of an approach where Marketplace verifies the signature instead of the ACE ?

Comment 13

[Krzysztof Mioduszewski[:tauzen]]

(In reply to Paul Theriault [:pauljt] from comment #12)
> Note that side-loading != rooted. You can sideload apps on a production
> device without rooting. I had assumed that ACE would treat side-loaded apps
> differently. (For example, not all manifest fields are installed when
> sideloading). But on refection, we do have a blacklist of non-sideloadable
> permissions. [1] So I guess we could achieve equivalent control by
> blacklisting sideloading apps with secure-element permission.  

Thanks for clarifying this. I've done a couple of tests and it seems that using the blacklist to prevent sideloading apps with secureelement permission on production phones makes sense. Developers who would want to develop an SE enabled app will need a rooted phone in the first place, so they can override the blacklist themselves (as a gaia user_pref for example).  

> Ultimately I don't know what sort of information is available on SIM applets
> so its hard for me to comment on the security decision here. But maybe its
> wise to err on the side of caution and prohibit sideloading of
> secure-element apps on production devices. Then it matters less if you check
> the certificate in the marketplace, though the ACE seems somewhat pointless
> to me.

So information on SIM applets is really service specific. The Access Rules on SIM card contain a combination of CERT HASH (treated as Application ID), AID (SIM applet identifier), and access modifier (allow/disallow). When an application tries to access a certain SIM applet, ACE should get the app CERT HASH and find a corresponding Access Rule and make the decision. All this is following the GlobalPlatform Device Technology Secure Element Access Control spec which we're implementing in Bug 884594. So ACE just needs the CERT HASH, and the whole signature verification is actually out of ACE scope and it's a workaround for a lack of full developer signature support.  

I hope this gives you more overview. Let's see what Wil can say about comment 12.

Comment 14

[Wil Clouser [:clouserw]]

How, specifically, would the marketplace verify the dev_signed_guid?  Can you give me step by step or pseudocode?

I also think we're missing an important point in comment 5 -- to have a GUID from the Marketplace an app needs to exist there first.  So, you'd upload a blank app, get a GUID, and then build your real app? That's an awkward flow, but the alternatives blow up the scope here.

Comment 15

[Andrew Williamson [:eviljeff]]

Also want to reiterate:

> 4) Reviewer signed apps, that are installed for testing before an app is
> approved during the review, have an entirely different, version unique, id
> in ids.json - **not the GUID of the app used for the approved public
> version**

Does the ID in ids.json in the zip that's installed needs to be 'correct' in regards to the dev_signed_guid?  I.e. does FxOS enforce that check or is it delegated entirely to Marketplace.

If so then reviewers won't be able to install the reviewer download of the app because the secure ID will be incorrect in that zip, so won't be able to actually check the app works.  Working around would need some/possibly-significant reworking of the reviewer tools and the way reviewer signing is done (probably a new bug).

Comment 16

[Krzysztof Mioduszewski[:tauzen]]

(In reply to Wil Clouser [:clouserw] from comment #14)
> How, specifically, would the marketplace verify the dev_signed_guid?  Can
> you give me step by step or pseudocode?

One clarification. We assumed earlier that dev_signed_guid will be a manifest entry which will contain a hexstring of the signed guid, but after doing some test with openssl I think it might be better to put additional file with the signature in the app package instead of putting the hex string in manifest file. 
If we delegate the verification of signed guid to Marketplace, we also don't need to have the guid in manifest file, since it's a known value to the Marketplace.  

So here is an example, the exact commands/file formats can be changed. Manifest could contain paths to the mentioned files.
0. Prerequisites. Developer re-submits the app package which contains:
- developer cert file cert.cer
- guid.sig file, signature over guid (file with guid value from marketplace dashboard) done using developer private key corresponding to cert.cer. 
example command (done by developer): openssl dgst -sha1 -sign private.pem -out guid.signed guid
1. Marketplace sees secureelement permission and starts verification process
2. Converts cert.cer to PEM format:
openssl x509 -inform der -in cert.cer -out cert.pem
3. Extracts public key from PEM cert
openssl x509 -pubkey -noout -in cert.pem > pubkey.pem
4. Gets the guid value of the app from DB and saves into a file
5. Verifies the guid signature
openssl dgst -sha1 -verify pubkey.pem -signature guid.sig guid
If this gives 'Verified OK' result, app can be uploaded to Marketplace. Do you think this would be possible to implement on the Marketplace side?
 
> I also think we're missing an important point in comment 5 -- to have a GUID
> from the Marketplace an app needs to exist there first.  So, you'd upload a
> blank app, get a GUID, and then build your real app? That's an awkward flow,
> but the alternatives blow up the scope here.

So we were thinking about this. We agreed that developer will need to submit the app twice. First time to get the GUID for signature (blank app), second to upload the full working app. During the development if the phone will be rooted, access control checks on the device will be disabled (Bug 1156710). 
Still if the Marketplace will do the signature verification as it proposed here, ACE verification would only involve computing the cert hash and comparing it to value on the SIM.

Comment 17

[Krzysztof Mioduszewski[:tauzen]]

(In reply to Andrew Williamson [:eviljeff] from comment #15) 
> Does the ID in ids.json in the zip that's installed needs to be 'correct' in
> regards to the dev_signed_guid?  I.e. does FxOS enforce that check or is it
> delegated entirely to Marketplace.
> 
> If so then reviewers won't be able to install the reviewer download of the
> app because the secure ID will be incorrect in that zip, so won't be able to
> actually check the app works.  Working around would need
> some/possibly-significant reworking of the reviewer tools and the way
> reviewer signing is done (probably a new bug).

So we actually assumed earlier that GUID is not available on the device and developer will need to add it manually to the manifest file (see here Bug 973823 comment 63). The best situation would be if the Marketplace could do GUID related verification on it's side and the phone would only need to deal with the cert. So I assume we shouldn't have problems with the review process here.

Comment 18

[Wil Clouser [:clouserw]]

Thanks for the steps.  I think that verification looks like something the Marketplace could do.

What is the timeline you had in mind for this?  Still targeting v3?

Comment 19

[Krzysztof Mioduszewski[:tauzen]]

Yes, full ACE support with SE privileged API should be available in v3 (comment 3). Still having this implemented earlier would be nice.

Comment 20

[Krzysztof Mioduszewski[:tauzen]]

Attachment: SE + ACE + Dev sig documentation link

Comment 21

[Krzysztof Mioduszewski[:tauzen]]

Comment on attachment 8614619 [details]
SE + ACE + Dev sig documentation link

Hi Wil, could you take a look at the developer doc and see if you agree with it? Especially sections II.2 Retrieving GUID from Marketplace and II.4 Signature verification on Marketplace side.

Comment 22

[Wil Clouser [:clouserw]]

I think it looks fine, although I'll beat the dead horse again about the flow being difficult for a new user.

Comment 23

[Mahendra Potharaju [:mahe]]

Paul, Can you please check if this should block 2.5? Not sure if it is Market Place specific.

Comment 24

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Mahendra Potharaju [:mahe] from comment #23)
> Paul, Can you please check if this should block 2.5? Not sure if it is
> Market Place specific.

Secure Element is not actively in development, so I dont think this need to block 2.5. (Its basically on hold until there is a partner requirement for this API since it requires a partner telco with the infrastructure required to ship SE apps on a sim card)

Comment 25

[Mahendra Potharaju [:mahe]]

Thanks Paul. 

Removing blocker nomination for 2.5 based on comment 24.