Fingerprint and version metadata (in Web App Manifest spec)

mgiuca commented 7 years ago

Re: Zach Koch's Payment Manifest Proposal V2

(Pasting a rambled thought train from a blink-dev thread.)

Firstly, I'm not sure why Web Payments is going to mandate things specific to Play (i.e., the Android platform). How will this work on other platforms? Will you need to specify a native app for each platform? Will we need to specify additional metadata in other platforms for related_applications?

This proposes adding two new fields to the related_applications struct. This is already kind of a dumping ground for proprietary metadata (since the id field is at the discretion of the platform, and specific platforms are not part of the standard). But the manifest spec currently doesn't allow for arbitrary extra fields to be added by platform, so we should work out a way to do that in a future-proof way. Maybe we just say "any platform can add new proprietary fields", or maybe we want to add a standard-named field meta, which is a dict that can contain arbitrary JSON content.

Lastly, it doesn't say what those two fields are used for. I assume sha256_cert_fingerprints is to verify that the Android app matches that fingerprint (so you can't just have a random side-loaded app with the same app ID). Would we be incorporating this checking into all the things that use related_applications? Or just web payments?

And what is version for? Minimum? Maximum? Is it supposed to match the Android version code? What if I want to define rules for a range of allowed versions? Is this going to break if a new version of the APK rolls out?

Matt

mgiuca commented 7 years ago

@zkoch @marcoscaceres

marcoscaceres commented 7 years ago

So yeah, we need see if other stores can make use of fingerprints. The spec allows proprietary fields by vendor-prefixing them. I'm also not clear on version.

domenic commented 7 years ago

Firstly, I'm not sure why Web Payments is going to mandate things specific to Play (i.e., the Android platform). How will this work on other platforms? Will you need to specify a native app for each platform? Will we need to specify additional metadata in other platforms for related_applications?

The formal spec is still to be written, but we certainly won't include anything vendor-specific in it. It'll instead say something like "refer to each platform's documentation for additional fields that may be required".

The spec allows proprietary fields by vendor-prefixing them.

Oh, I missed that. So the preferred extension mechanism would be

{
  "related_applications": [
      {
        "platform": "play",
        "url": "https://play.google.com/store/apps/details?id=com.example.app1",
        "id": "com.example.app1",
        "play_version": "...",
        "play_sha256_cert_fingerprints": "..."
      }]
 }

? I guess that's not so bad.

marcoscaceres commented 7 years ago

yeah, that would be fine.

mgiuca commented 7 years ago

Some thoughts:

The spec allows proprietary fields by vendor-prefixing them.

That isn't quite the same as prefixing with the platform (e.g., "play"). It would suggest that we have to add chrome_sha256_cert_fingerprints, webkit_sha256_cert_fingerprints, moz_sha256_cert_fingerprints, etc. At some level, I think it should be possible to have a standard way to refer to an Android app (though probably not in a web standard, maybe in the Android documentation), and that should have a standard prefix like "play" -- NOT a vendor prefix.

Chrome's policy for several years (since the Blink fork) has been no vendor prefixes, so we certainly wouldn't add "chrome", "blink" or "webkit_" attributes to the manifest.

If the prefix is "play_" then it calls into question why have a prefix at all... since we are scoped to the "relatedapplications" dict, and "platform" is "play", there isn't going to be a naming clash between proprietary fields of different platforms within this dict. ALL proprietary fields will belong to "play" by definition. The only chance for a clash is if we (at the web standard level) wanted to add a new field, that might clash with a proprietary field that's been added. My suggestion of a "meta" field would eliminate that. Otherwise, we could introduce a standard prefix for proprietary metadata, like "meta" or "x_" (to borrow from HTTP). So, for example:

{
  "related_applications": [
      {
        "platform": "play",
        "url": "https://play.google.com/store/apps/details?id=com.example.app1",
        "id": "com.example.app1",
        "meta": {
          "version": "...",
          "sha256_cert_fingerprints": "..."
        }
      }]
 }

or

{
  "related_applications": [
      {
        "platform": "play",
        "url": "https://play.google.com/store/apps/details?id=com.example.app1",
        "id": "com.example.app1",
        "x_version": "...",
        "x_sha256_cert_fingerprints": "..."
      }]
 }

marcoscaceres commented 7 years ago

Meta could actually be nicer. We definitely don't won't to end up with "x_" situation as happened with HTTP (where x stuff then becomes standard). Also good points about platform: scoping things to "play".

zkoch commented 7 years ago

I think meta is a nice compromise.

As for version, agreed it's ambiguous. @rsolomakhin how does this behave? min version?

rsolomakhin commented 7 years ago

version is the minimum version of the Android application.

rsolomakhin commented 7 years ago

I've updated the design doc according to the explainer and will work on churning out a web platform spec together with @domenic.

Personally, I'm not fully convinced that we need the "meta" section or "meta_" prefix. Can anyone elaborate on why that's better than adding "version" and "sha256_cert_fingerprints" directly into the dictionaries of "related_applications"?

mgiuca commented 7 years ago

version is the minimum version of the Android application.

Then could I suggest calling it min_version?

Can anyone elaborate on why that's better than adding "version" and "sha256_cert_fingerprints" directly into the dictionaries of "related_applications"?

It's an issue of how it's defined in the specs and how it interops across platforms and future versions of the Manifest spec.

Since these two fields are proprietary to Android, you can't just get them added to the Manifest spec as fields of related_applications, you have to specify them "somewhere else" (neither in the Manifest nor the Web Payments spec, but in some Android-specific documentation). Each platform could potentially define different fields here. The key is that a platform-specific field should not block the addition of a new field later down the track to the official spec. For example, if the Manifest spec wants to add a "version" field with a specific meaning, we don't want to have an objection "we can't add that because [PLATFORM-X] already has a field called version with its own proprietary meaning". So you have to name it in a way that won't conflict with any future features.

On the other hand, we could go down the route of actually specifying these as non-proprietary fields. If we did go down this route, we'd have to wriggle them around to make them not specific to Android. Here's a quick brainstorm of how we could change it to cover all platforms:

Make min_version a string, and specify it as "the minimum version of the app; how the version strings are interpreted and ordered is at the discretion of the platform". It's a string because on most systems, version numbers can be arbitrarily complex, not simple ints like they are on Android. For example, Debian defines a strict, but complex, ordering on version strings.
Make the hash a bit more generic. Call it fingerprints, and spec it as a list of dicts with keys: {format, value}. format would be "sha256_cert" for Android, with space for future fingerprint types, and value would be the fingerprint in an Android-proprietary format.
The idea is that both min_version and fingerprints (along with id) are meaningless in general; their meaning is imposed by the platform. But the spec tells us the general format that these fields should take.

The new version of Zach's example becomes:

"related_applications": [
    {
      "platform": "play",
      "url": "https://play.google.com/store/apps/details?id=com.bobpay",
      "fingerprints": [  //new
        {"format": "sha256_cert", "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8..."}
      ],
      "min_version": "1",  // new
      "id": "com.example.app1"
    }, {
      "platform": "itunes",
      "url": "https://itunes.apple.com/app/example-app1/id123456789"
    }
  ]

Now I'm actually leaning towards specifying this in the Manifest as opposed to adding a "meta" proprietary value bag. Thoughts?

cyberphone commented 7 years ago

IMO, this proposal seems largely redundant. Payment credentials (keys) in Android are bound to the app that enrolled/generated them. I.e. there's no way for AlicePay using BobPay's keys unless BobPay have explicitly declared its keys "global" which nobody (in their right mind...) would ever consider. If there is any problem to fix it is in the enrollment end. In addition, practically all native payment applications (including Android Pay) are also usable in non-Web payment scenarios.

mgiuca commented 7 years ago

@cyberphone Are you suggesting that Web Payments shouldn't need to verify the payment app because additional verification would take place while completing the payment flow?

True, but I still think you want some guarantee that you are using the app you think you are (e.g., to avoid the user putting their credentials into the wrong app).

cyberphone commented 7 years ago

@mgiuca

Are you suggesting that Web Payments shouldn't need to verify the payment app because additional verification would take place while completing the payment flow?

What I'm saying is that the potential problem is (or should be at least) addressed during payment credential enrollment which is out of scope for this WG as far as I can tell. That is, banks do not want to deploy their precious keys in arbitrary apps which is why Android provides isolation features like: https://developer.android.com/training/articles/keystore.html

mgiuca commented 7 years ago

Auth should definitely be taken care of in a way that is out of scope here. (i.e., you should not rely on Android package signing for payment auth). However, I still think there is merit to being able to verify that the Android app you are talking to is the one that was signed by the right person.

cyberphone commented 7 years ago

However, I still think there is merit to being able to verify that the Android app you are talking to is the one that was signed by the right person.

It is a free world so please go ahead :-) IMO, it doesn't matter if the payment app is "wrong" because it shouldn't have the "right" keys and the payment process would not go through. There are already tons of related Apps out there building on this security concept which is independent of Web-browser versus Standalone usage.

zkoch commented 7 years ago

@mgiuca Ah, that proposal feels better to me than a generic catchall of "meta".

+1 to changing "version" to "min_version". I discussed the same thing with @domenic and @rsolomakhin yesterday evening.

mgiuca commented 7 years ago

I don't have a strong opinion about whether these additional checks (sha256 fingerprint and version) are needed from a payments perspective. I'll let the Web Payments team make that call. I'm just here on behalf of the Web Manifest side, to figure out if this is wanted, how to best incorporate it into the manifest spec.

rsolomakhin commented 7 years ago

Re new format proposed by @mgiuca

"related_applications": [
    {
      "platform": "play",
      "url": "https://play.google.com/store/apps/details?id=com.bobpay",
      "fingerprints": [ 
        {"format": "sha256_cert", "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8..."}
      ],
      "min_version": "1",
      "id": "com.example.app1"
    }, {
      "platform": "itunes",
      "url": "https://itunes.apple.com/app/example-app1/id123456789"
    }
  ]

I'm OK with using this.

There's one point of interest, however. The "sha256_cert" is not a "format" per se. The format is 256 bits. The string "sha256_cert" indicates that the fingerprint is calculated by taking a SHA256 hash of the certificate that was used to sign the application binary. I'm OK with using "format": "sha256_cert", but I wanted to let you know ahead of time that naming is a bit confusing.

dlongley commented 7 years ago

Would "algorithm" or "digestAlgorithm" be better than "format"?

dlongley commented 7 years ago

It's also unclear that colon-delimited hex is what is to be expected, no? Perhaps nix the colons as well -- aren't those really just for display purposes?

mgiuca commented 7 years ago

[Now bikeshedding] Re the discussion about the name "format" and the syntax of the "value" field: I chose "format" (as opposed to "bits", "algorithm" or equivalent) precisely because it captures all of these aspects:

Which algorithm is used.
How many bits it takes.
How it is represented (e.g., hexadecimal, base64, colon-separated octets, decoded as Latin-1 and escaped, etc).

This allows each platform to define a set of "formats" which they define all of the above aspects. For example, a platform could specify (strawman: don't do this) a format "checksum" which is just an integer sum of all the bytes of the package, expressed as a decimal integer (as a string). That doesn't fit cleanly into a scheme that says "you must express your fingerprint as a series of hexadecimal-encoded octets".

The format is 256 bits.

I disagree that the word "format" implies just the number of bits.

Would "algorithm" or "digestAlgorithm" be better than "format"? ... It's also unclear that colon-delimited hex is what is to be expected, no?

This is going down a different route which is being more descriptive in the spec (which I am OK with also). In this route, we would call it "algorithm" and it would be "sha256" (no "cert"). The syntax would be specified as a run of hexadecimal-encoded octets (probably with no colons). The format would be the same across all platforms, but different platforms could define their own hashing algorithms, and also define what exactly is the object that's being hashed (e.g., on Android it's the APK).

adrianhopebailie commented 7 years ago

Suggest re-using 'integrity' attribute from https://www.w3.org/TR/SRI/

"related_applications": [
    {
      "platform": "play",
      "url": "https://play.google.com/store/apps/details?id=com.bobpay",
      "integrity": "sha384-Li9vy3DqF8tnTXuiaAJuML3ky+er10rcgNR/VqsVpcw+ThHmYcwiB1pbOxEbzJr7",
      "min_version": "1",
      "id": "com.example.app1"
    }, {
      "platform": "itunes",
      "url": "https://itunes.apple.com/app/example-app1/id123456789"
    }
  ]

mgiuca commented 7 years ago

@adrianhopebailie Good! That fits nicely and is pre-bikeshedded so we can just point to that spec. I'm happy with that suggestion.

@marcoscaceres Would you be OK with adding "integrity" (as defined in https://www.w3.org/TR/SRI/) and "min_version" (semantics left at the discretion of the platform) to the Manifest spec? I can do it as I'm planning to work on the Manifest spec on unrelated business shortly.

Web Payments people, is this something you want to get into the Manifest spec ASAP, or do you want to wait and deliberate in Web Payments working groups first?

adrianhopebailie commented 7 years ago

We should probably hear from @rsolomakhin or any other platform person on whether or not the 'integrity' approach works for them.

zkoch commented 7 years ago

Web Payments people, is this something you want to get into the Manifest spec ASAP, or do you want to wait and deliberate in Web Payments working groups first?

More in the ASAP category, but we also want to make sure we have buy-in. If @rsolomakhin and @domenic and @marcoscaceres are okay with "integrity", that works for me. I'll update explainer and we'll start on the other changes.

domenic commented 7 years ago

I'm a little worried people will think that the same considerations apply here as they do for SRI. That is, that adding this and basing it on SRI (even by just using the name "integrity") implies that Android supports all the same hash functions that SRI does; that it supports the fallback semantics of using multiple hash functions and the UA choosing the strongest; etc. Are we planning to actually reuse the SRI codepaths in our implementation, @rsolomakhin? My impression was that this was more of an Android-specific thing that we'd hand off to them.

In general I am more comfortable letting vendor-specific use cases be covered by vendor-specific properties, instead of trying to broaden the scope and make these generally applicable.

mgiuca commented 7 years ago

@domenic Those are valid concerns.

In general I am more comfortable letting vendor-specific use cases be covered by vendor-specific properties, instead of trying to broaden the scope and make these generally applicable.

I would prefer to have standard properties, but let vendors choose what they support. For instance, we could say that the vendor can choose which hashing algorithms are required or supported, and Android only supports sha256.

We could call it "fingerprint" instead of "integrity" if you want to distance it from the implications of SRI, and merely refer to their spec so we don't duplicate the definition.

The implementation can be handled by Android but the spec can still point to the SRI definition.

marcoscaceres commented 7 years ago

@mgiuca, @zkoch you have my support to add this to the web manifest spec. Happy to review or help in any way I can.

rsolomakhin commented 7 years ago

I'm OK using SRI:

"integrity": "sha256-QTY6N0I6MjA6MDQ6RDE6MDM6MDc6OTI6RTI6QTU6MzE6Njc6NjYK
              sha256-RTk6RTM6ODc6MTQ6RUM6NkQ6QTI6MDQ6MjE6RTA6RkQ6M0I6RDEK"

However, we cannot use the SRI algorithm as-is. A list of hashes in SRI means "verify the strongest fingerprint," whereas a list of fingerprints on Android means "verify each fingerprint." If that's OK with @mgiuca, then let's roll with it.

FYI, the original proposal was loosely based on Digital Asset Links.

zkoch commented 7 years ago

Chatted with @mgiuca and @domenic off thread and we've agreed to circle back to one of Matt's original ideas but with a slight modification:

"related_applications": [
    {
      "platform": "play",
      "url": "https://play.google.com/store/apps/details?id=com.bobpay",
      "fingerprints": [ 
        {"type": "sha256_cert", "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8..."}
      ],
      "min_version": "1",
      "id": "com.example.app1"
    }, {
      "platform": "itunes",
      "url": "https://itunes.apple.com/app/example-app1/id123456789"
    }
  ]

Note that we've replaced format with type. I think @mgiuca will fill in on more of the reasoning behind this.

rsolomakhin commented 7 years ago

I'm happy with this proposal.

mgiuca commented 7 years ago

Thanks for updating, Zach.

Some rationale from the discussion:

We didn't want to precisely use "integrity" from SRI, because it has some specific semantics that we don't intend to match here. Specifically, the algorithm for choosing which fingerprint is used as pointed out in rsolomakhin's second-most-recent post. We want each platform to use its own verification semantics.
We could have stuck with the SRI syntax but that would maybe come with the implication that the SRI algorithm is being used. We also want the platforms to be able to specify their own hashing algorithms.
Since the verification is entirely specific to the platform, it didn't make much sense to constrain it to a specific syntax (it just means more work if the implementation uses one format and we prescribe another).
We want to specify at least the JSON types so the format is coherent, so it makes sense to not give the platform total control over the "fingerprints" value. Therefore, we decided to go back to the earlier proposal of specifying it as {"format": <platform-specific-format-string>, "value": <platform-and-format-specific-fingerprint-syntax>}.
Due to the previous complaint (confusion / overloading) of the word "format", we thought "type" might be a good alternative and still sufficiently generic. (It might collide with the JSON/JavaScript notion of "type", e.g., string, number, array; but it's pretty clear from examples that it means something different.)

adrianhopebailie commented 7 years ago

Agree with the rationale. Makes sense for this to be platform specific.

Just as an FYI, for those interested, here are 4 different specs that define different ways to encode a hash. There are probably more. (Insert mandatory link to XKCD comic about standards here)

Naming things with hashes - RFC6920
Sub-resource Integirty
Google's Digital Asset Links
Multi-hash
Crypto-conditions (Uses RFC6920 for text encoding but has its own binary encoding)

mgiuca commented 7 years ago

I love that one of those specs (RFC6920) defines a hash naming scheme called "NIH" (and is self-aware about it too).

RByers commented 7 years ago

There's now a spec. @mgicua are all your concerns in this issue addressed now? If not please file issues in the repo to track.

mgiuca commented 7 years ago

Oh yes, this has been resolved separately in the Web App Manifest spec (see https://github.com/w3c/manifest/pull/566). We now have min_version and fingerprints in the standard for related_applications objects.

w3c / webpayments

Fingerprint and version metadata (in Web App Manifest spec) #225