w3c / secure-payment-confirmation

Secure Payment Confirmation (SPC)
https://w3c.github.io/secure-payment-confirmation/
Other
111 stars 40 forks source link

Proposal: support data URLs for card art icons #101

Closed stephenmcgruer closed 2 years ago

stephenmcgruer commented 3 years ago

One of the goals of SPC[0] is to minimize the merchant's active reliance on the issuer to perform an authentication ceremony as much as possible. Currently there is one sneaky path where SPC still relies on the issuer - the card art icon in PaymentCredentialInstrument. The icon is likely to be hosted by the issuer (e.g. https://issuer.com/icon.png), and SPC will fail if it cannot be successfully fetched.

To reduce this reliance, we could accept a data URL that encodes an appropriate card-art icon instead. This is probably possible by the spec today, depending on what exactly the fetch algorithm does.

The problem, however, is that data URLs are not small (a test 32x20px image produced a 1202 character string), and currently we sign the URL string as part of clientDataJSON. This might be too long for some authenticators to handle.

Proposed change: instead of signing the URL string, sign a hash (md5sum?) of the actual bytes in the icon.

This would work both for 'normal' URLs and for data URLs.

The hash is not intended to be cryptographically secure, because I doubt that a collision attack on it would be very successful (the colliding image is likely garbage so your malicious caller has... shown the user a garbage card art icon rather than the correct card art icon. I don't see a useful attack there.)

On the server side, the validator would then perform the same hash of the expected image bytes and check for equality as normal.

[0]: Ok, I couldn't technically find a direct quote for this being a goal in the scope or benefits docs, but I consider it aligned to SPC's interests.

rsolomakhin commented 3 years ago

SHA256 hash is common in WebAuthn and does not have md5sum's problems of being able to craft multiple inputs with the same hash (if one tries hard enough).

ianbjacobs commented 3 years ago

Seems like a reasonable proposal. What complexity is introduced by hashing? Are there any normalization requirements?

stephenmcgruer commented 3 years ago

What complexity is introduced by hashing? Are there any normalization requirements?

I'm afraid I don't know what you're asking from either of these questions - can you explain them both further?

ianbjacobs commented 3 years ago

Hi @stephenmcgruer,

I apologize for my incomplete message. Here is what I was thinking (albeit not very deeply):

1) I had imagined that when an RP would validate the assertion by comparing a URL (for an image) that it knows, and what was signed. So I imagined simple URL comparison: https://url.spec.whatwg.org/#url-equivalence

2) If we move to hashes, does that introduce complexity (e.g., because of how URLs are encoded)? I don't know.

One way to avoid the question is to take two parameters:

Then it's just a string and I expect string comparison is easier.

Ian

stephenmcgruer commented 3 years ago

I had imagined that when an RP would validate the assertion by comparing a URL (for an image) that it knows, and what was signed.

Yep, that's definitely what the RP should do today.

If we move to hashes, does that introduce complexity (e.g., because of how URLs are encoded)? I don't know.

I would say yes and no :D:

  1. Computing and comparing hashes is trivial, in my opinion - they are standard concepts which have many libraries to both compute and compare.
  2. However for non-data-URI icons, this would require the RP to actually fetch the image to compute the expected hash for comparison, or otherwise keep a hash of the fetched image somewhere.

So there is a trade-off from point 2 to be considered.

One way to avoid the question is to take two parameters: [...]

Is the suggestion to sign both the hash and the URL? If so, we're back to "data URIs may be big, and some authenticators may not be able to sign them".

(I'm also not clear why the RP providing the hash is useful?)

ianbjacobs commented 3 years ago

I was thinking:

To summarize:

So no need to spend more time on this.

But you said "for non-data-URI icons, this would require the RP to actually fetch the image to compute the expected hash for comparison". I thought you were going to hash the data URL (not the image data).

Ian

stephenmcgruer commented 3 years ago

But you said "for non-data-URI icons, this would require the RP to actually fetch the image to compute the expected hash for comparison". I thought you were going to hash the data URL (not the image data).

That's a really good point, we could do that!

I don't know offhand if that is better or not than hashing image bytes. Opinions @rsolomakhin (or others) ?

rsolomakhin commented 3 years ago

Hashing the URL (be it https:// or data:image/png) could be easier for RP's to verify, but perhaps we can ask some web developers?

Pinging @jcemer-stripe - Any preferences in here?

What other github usernames that we know for the web developers in the current origin trial?

jcemer-stripe commented 3 years ago

It makes sense to me to have the hash of the card art icon in the clientDataJSON. It is the perfect solution to avoid heavy payloads.

I don't think it adds complexity and as a matter of fact, checking this value in the server will be optional.

rsolomakhin commented 3 years ago

@jcemer-stripe - Correct me if I'm wrong, it sounds like you're talking about hashing of the image content, not the image URL. Is that correct?

jcemer-stripe commented 3 years ago

@rsolomakhin for consistency I would hash both. I think that no matter what is included as card art icon, it should be SHA256 hashed and included in clientDataJSON.

What do you think?

rsolomakhin commented 3 years ago

To double-check my understanding:

PaymentRequest([{data: {instrument: {icon: "https://img.com/card-art.png" }}}])   ↳"clientDataJson": {"payment": {"instrument": {"icon": SHA256("https://img.com/card-art.png") }}}

PaymentRequest([{data: {instrument: {icon: "data:image/png,abcdefg=" }}}])   ↳"clientDataJson": {"payment": {"instrument": {"icon": SHA256("data:image/png,abcdefg=") }}}

Is that what you're proposing, @jcemer-stripe ?

jcemer-stripe commented 3 years ago

@rsolomakhin Yes, that's correct.

rsolomakhin commented 3 years ago

Sounds good! Thank you for your feedback, @jcemer-stripe.

@stephenmcgruer - Does that proposal work for us?

stephenmcgruer commented 3 years ago

This was discussed in the SPC task force on August 23rd (minutes). Main points we heard at the task-force:

  1. Doug (Visa) advocated that if there is hashing, it should be the image bytes not the URL that are hashed, because the fetchable contents of a URL can change over time. The RP would probably keep a list of 'acceptable hashes' and compare against that.
  2. @adrianhopebailie advocated for a choice of hashing algorithm, rather than pre-defining e.g. SHA-256. Quoting their later email:

My suggestion would be that we update the PaymentCredentialInstrument https://w3c.github.io/secure-payment-confirmation/#dictdef-paymentcredentialinstrument to have an optional "iconIntegrity" member that gets it's definition from SRI. In the algorithm to check if a payment can be made https://w3c.github.io/secure-payment-confirmation/#sctn-steps-to-check-if-a-payment-can-be-made we check if there is an integrity value provided and if so we follow the algorithm s defined in SRI to parse it and validate it against the content fetched for the image. Would that work?

stephenmcgruer commented 3 years ago

Wrt (2), I want to make sure I understand the full flow. My understanding is:

  1. The Relying Party would (somehow, out of scope of SPC) communicate to the merchant both the icon URL and an integrity string for it (e.g. sha512-Q2bFTOhEALkN...).
  2. The caller of SPC would pass these both into the browser.
  3. The browser would fetch the URL, and verify it against the provided integrity descriptor.
  4. The browser would then sign ???

@adrianhopebailie - In step (4), what are we now signing over, with regards to the icon? I'm trying to understand in my head if the RP providing an integrity value is a net benefit here over other ideas.

Another consideration would be: would we support multiple integrity values as SRI does?

adrianhopebailie commented 3 years ago

We'd sign the integrity string right? The assertion from the browser being: "I displayed an image that passed an integrity check using this hash"

adrianhopebailie commented 3 years ago

Another consideration would be: would we support multiple integrity values as SRI does?

I don't see why not. The behaviour in SRI is to pick the value with the most collision resistance and use that. i.e. We'd still only ever sign over the most collision-resistant

stephenmcgruer commented 3 years ago

Having discussed, we (Chrome) don't think that the complexity of SRI is worth it in this case. SRI is mostly targeted at a site downloading a known resource from a third-party, and wanting to ensure that no tampering has occurred. Here, the resource is first-party (bounced via a merchant) and it is unlikely that tampering could occur in that fashion.

For SPC, the much stronger risk is that the merchant tampers with the inputs before calling SPC, e.g. switching out the card art icon. Since the SRI metadata would also be an input to SPC, it doesn't protect against that - we have to sign something anyway (as you stated) for that protection. As such we feel SRI wouldn't add sufficient benefit for adding another field into SPC.

As always, interested to hear from the rest of the WG :)

adrianhopebailie commented 3 years ago

The first-party vs third-party distinction is irrelevant. The browser is asking the authenticator to sign over something that it attests was shown to the user, the source of the image is irrelevant as long as the RP is happy that the image is one it accepts. But, you can't include the full binary image data in the clientData and you can't sign a URL and expect the resource behind that URL to always be the same. That's the problem we're solving here.

The most common pattern for signing over large data in a constrained environment is to first hash the data and then sign over the hash. This means that you need to give both the data and the hash to the verifier so they can verify the hash is valid AND that the signature over the hash is valid.

The added complexity you get from introducing the hashing step is the need to coordinate accepted hashing functions between the systems that have to generate the hash and also ensure agility in the face of the ever present possibility of vulnerabilities being discovered in hashing functions that make collisions easy to produce.

To come back to the issue we are trying to solve: We need the authenticator to sign over the data that was rendered to the user and in this case the data includes an image which may be a large data blob that is too big to include in clientData.

To solve the issue of images being too large to sign, we sign over a hash of the image instead.

This introduces the question of which hash functions to use and how to ensure agility. We can simply say SHA256 and move on or we can follow best practice and design a system that allows for multiple hash functions and the introduction of new ones over time. Luckily SRI already went through he pain of solving this for us.

If we don't support agility then presumably the browser will generate the SHA256 hash over the binary data of whatever image it uses (irrespective of whether it was a data URL or fetched from a regular URL). That hash will be included in the clientData and we'll need to specify how it is encoded so that the RP is able to decode it deterministically when it is verifying it.

If we do support agility then we must provide the browser with both the hash (including meta data specifying which function was used to create it) and the image. The browser will still generate the hash over the binary data of whatever image it uses but in this case it will verify that the provided hash matches the hash it generated. i.e. It fails fast if there is an issue with the hash. Presumably the provided hash(es) were supplied by the RP.

When the RP verifies the attestation it will extract the hash from the clientData. If the hash was provided by the RP (i.e. the agility-supporting SRI-based solution) then it is presumably encoded exactly as provided so it is easy to verify that it is from a set of supported image hashes. If not then the RP must have a way to decode the hash into some normalised form that it can compare with its internal set of known accepted image hashes, or we have to provide the image itself to the RP to check that the image is a known accepted image and leave the RP to generate a hash and compare it to the one from the client data.

TL;DR: Crypto is hard and deterministic data encoding and normalisation are foot-guns that regularly cause problems in signature or hash based data validation schemes. We can choose to ignore the work already done in SRI to solve for that or take the easy route and make it the RPs problem. By taking the integrity value as input we also have the chance to catch errors early leading to a likely better UX. (A payment isn't sent for authN to the RP just to be declined because there was an error at the merchant in how they rendered the instrument data)

stephenmcgruer commented 3 years ago

I believe that the above comment conflates the ability to select a hash algorithm ('agility') with providing an ahead-of-time integrity value for a given resource. It should be possible to do the former without the latter, and I am arguing that the latter is unnecessary for SPC.

Requiring an SRI input (algorithm + digest) means we cannot have a default value for the field. This 'integrity metadata' then becomes yet-another required field for RPs to provide to the Merchant. This increases overhead for RPs in my opinion, without adding enough value. (I don't consider the early-exit to be much value; see below).

In any case, I think we should seek input on this from expected RPs of SPC. Otherwise we are debating in a vacuum without the important question of what RPs actually want to or are able to do.

A few specific replies to points in your comment:

That hash will be included in the clientData and we'll need to specify how it is encoded so that the RP is able to decode it deterministically when it is verifying it.

It would be reasonable for the encoding in clientData to use the SRI format (given it is standardized), but this does not require using SRI as the input to SPC.

If we do support agility then we must provide the browser with both the hash (including meta data specifying which function was used to create it) and the image.

This may come down to a mismatch in how we view terminology, but to me enabling agility does not require providing the digest, just the hash algorithm(s) one accepts.

By taking the integrity value as input we also have the chance to catch errors early leading to a likely better UX. (A payment isn't sent for authN to the RP just to be declined because there was an error at the merchant in how they rendered the instrument data)

What is the 'error at the merchant' here? The merchant doesn't render the instrument data, they just pass it through to the browser. To me the mostly likely change would come from a malicious merchant, who would also of course replace the SRI digest as part of their attack.

The most that I can imagine here is that the merchant has a coding error where they accidentally mutate either the icon URL or the (in your suggestion) SRI digest value itself. Protecting against that doesn't seem worth adding another required field.

ianbjacobs commented 3 years ago

I have several thoughts:

My expectation is that we will go to FPWD with the current specification, but that it will (rapidly) evolve. I'd love to hear some concrete proposals for, say, optional arguments involve other hashing algorithms.

Ian

adrianhopebailie commented 3 years ago

I believe that the above comment conflates the ability to select a hash algorithm ('agility') with providing an ahead-of-time integrity value for a given resource.

Fair, I hadn't intended to infer this was the only way to get agility rather that it was an easy way to re-use existing work done and avoid doing it all again. Whatever solution you come up with for agility will require the RP to provide, at a minimum, the meta data to the browser that specifies which algorithms it supports although I realise you're attempting to minimize the impact on the RP by defining a default for that metadata.

Have you considered what would happen if the default algorithm needed to be deprecated? How would that play out?

The most that I can imagine here is that the merchant has a coding error where they accidentally mutate either the icon URL or the (in your suggestion) SRI digest value itself. Protecting against that doesn't seem worth adding another required field.

Yes, this is the issue I am trying to protect against. I think we can be confident that a capable RP will protect against malicious merchant behaviour.

There is potentially a lot of work done between the merchant and RP to get set up. Example, the merchant might download logos from the RP and convert to data URLs or fetch during the transaction via proxies and middle-boxes, all of which could result in in accidental mutation the side-effects of which will only manifest when the merchant starts trying to process transactions.

That sounds like a recipe lots of failed transactions that should otherwise have succeeded hence my suggestion that catching this in the browser is preferable as this can be diagnosed during the dev cycle.

stephenmcgruer commented 3 years ago

This was discussed again in the SPC task force on August 30th (minutes). Main points we heard at the task-force:

(Incidentally, we learned that WebAuthn just specifies that ClientDataJSON is hashed with SHA256, with no algorithm selection possible ;)).

stephenmcgruer commented 3 years ago

To clarify, there are two questions left still:

  1. Should we put the image bytes or image URL inside clientDataJSON.payment.instrument.icon
  2. Should we hash the value that we put inside clientDataJSON.payment.instrument.icon, to make the payload smaller for merchants/RPs
ianbjacobs commented 3 years ago

Adding @bdewater, @a-akimov, @tblachowicz, @Eric-Alvarez, @mweksler for additional thoughts. (See @jcemer-stripe comments above).

adrianhopebailie commented 3 years ago
  1. Should we put the image bytes or image URL inside clientDataJSON.payment.instrument.icon

I believe we have to otherwise we are asserting something different to ("I showed the user this image"). We are asserting that we "showed the user the image we fetched from that URL".

Since the RP has no notion of the client environment and what data would be returned to the client when they fetch the URL this seems to be a vulnerability.

  1. Should we hash the value that we put inside clientDataJSON.payment.instrument.icon, to make the payload smaller for merchants/RPs

Why do we need to? Does it save us needing to send the full data to the RP along with the assertion for verification?

stephenmcgruer commented 2 years ago

To give a direction for the upcoming TPAC discussion on this issue: As this doesn't seem to have been an issue for current users of SPC, I propose that we close this issue and leave the implementation as-is

ianbjacobs commented 2 years ago

Based on discussion at the WPWG meeting today [1], there was consensus to close this issue and to support data URLs for icons. In addition, we anticipate a separate pull request to make clearer to readers that compared to http URLs, data URLs provide additional information (via the signature) about what bits were displayed to the user, and RPs may want to take advantage of that information in some contexts.

[1] https://www.w3.org/2021/10/25-wpwg-minutes.html#issue101