View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0004743 | Taler | wallet (WebExtension) | public | 2016-10-19 13:46 | 2016-10-20 17:03 |
| Reporter | Marcello Stanisci | Assigned To | Florian Dold | ||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | closed | Resolution | no change required | ||
| Summary | 0004743: Merchant signature needed by fulfillment URL | ||||
| Description | The fulfillment URL gets a set of parameters needed to reconstruct a contract, but there is no link to the contract that was generated in the first place. That way, a wallet can tune contract fields (the timestamp, for example), get it reconstructed, and pay for it. The solution is to include the merchant signature along the fulfillment URL parameters, so that the frontend can then match this signature with the one it gets from the reconstruction. Note that: 1 is not possible to include the merchant signature into the fulfillment URL because it is inside the contract. 2 the backend never stores contracts it generates the first time; it does so only when handling a /pay. | ||||
| Tags | No tags attached. | ||||
|
|
Right now the fulfillment URL can contain the ${H_contract} parameter, which is expanded by the wallet to be the contract hash. Would a ${merchant_pay_sig} with similar semantics be sufficient for your purpose? Could you elaborate what we need this for? |
|
|
Rigth now, the payment is accomplished by: 1 Get a contract 2 Visit fulfillment URL (which contains ${H_contract} and parameter to reconstruct the contract) 3 Pay for ${H_contract} which is only checked against the contract reconstructed in 2. Now, step 3 totally ignores the contract generated in 1. So a malicious wallet can forge/modify a contract, hash it, set ${H_contract} to this hash, and start the payment from step 2. That can be prevented by including the merchant signature of the original contract (the one gotten in step 1) along the fulfillment URL's parameters. So to answer to your question, a ${merchant_sig} placeholder would fix it. |
|
|
Your analysis is not correct. In step (2) we set a cookie (signed, can't be forged by the client). In step (3), we only mark an article as read when the right cookie was set in step (2), see [1]. You're right that we shouldn't accept the payment before we check the cookie, but that is a much simpler change. I've just fixed that. I'm against including the merchant's signature in the fulfillment URL by default, since then sharing the URL will also share the payment. For non-technical users, it's much less trivial to share cookies than it is to share a URL. [1] https://www.git.taler.net/?p=merchant-frontends.git;a=blob;f=talerfrontends/blog/blog.py;h=3402f29ccada042441e5f714fce186caa67f879a;hb=816cccea794195bdd1bcc559ca5eeab4f0d38fd4#l224 |
|
|
That is the point: we set the cookie in step (2) to the hash of the _restored_ contract. And the restored contract is derived 100% from the parameters given in fulfillment URL, which don't contain any merchant's autorship right now. Thus the wallet can forge them and continue the payment from step (2) with a restored contract which differs from the contract of step (1). |
|
|
That's okay though. When restoring contracts, the merchant only allows contracts that are acceptable to the merchant. If the merchant wants to make sure that it's really a contract from step (1), they'll have to include more information in the fulfillment URL or even store something about the transaction ID in a database. That's not necessary for the blog though. But it's a good point to raise anyway, we have to make sure that we document this properly! |
|
|
Ok. But as long as the fulfillment URL is used for the restoration, it must contain some proof of merchant authorship, which ${H_contract} cannot provide. Leaving it open in case you want to reply. |
|
|
Yeah, the point is that the merchant can look up "authorship" in their database if there's really no other way to confirm that the restored contract is "acceptable" to the merchant. I'm not completely opposed to having a ${merchant_pay_sig} in the fulfillment URL (that will be filled out with some dummy value when the contract isn't payed for yet). But it's not necessary for any of the merchants we have so far! |
|
|
I thought the ${merchant_pay_sig} would just be the merchant's signature over the contract from step (1) (so the wallet can't forge stuff), which is never dummy ... |
|
|
Oops, you're right, of course. We could also introduce a variable for the signature we get from /pay for cookie-less payments, but that's another issue I suppose! |
|
|
Erm ... I just re-checked what we send in /pay, and we do actually send the merchant's signature there. So I don't really see why it would also be required in the fulfillment URL ... |
|
|
It's true, deposit permission contains the signature, but contains H_contract as well. At this point, replacing ${H_contract} with ${merchant_pay_sig} in the fulfillment URL can be considered an optimization: we don't wait an extra round trip until /pay gets called in order to detect a fraudulent contract reconstruction. |
|
|
It costs two actions more: (1) /fulfillment handler needs to store merchant_sig in the state and (2) the /pay handler in frontend needs to check if the signature it got in deposit permission equals the one stored in the state. |
|
|
That's wrong. The wallet sends the merchants signature when posting coins to /pay... Seems like we really need to document this better ;) |
|
|
Yes, but the signature sent by the wallet to /pay must be compared with the one we got from the backend at reconstruction time (= when the wallet visits fulfillment URL). And we have store that signature in the state at reconstruction time, otherwise we lose it (the /pay handler does not reconstruct the contract anymore, and the backend just checks if H_contract is well signed, without reconstructing H_contract -- it cannot detect if H_contract is malicious). Just to make clear: the goal is to verify that the wallet doesn't gives malicious parameters at fulfillment URL. |
|
|
Nope, the signature is checked by the backend on /pay. The contract is already restored/validated when we go to the fulfillment URL, which sets session state that /pay will check (namely the contract hash) Could you please provide a concrete example how you could tamper with the contract in any way for the blog merchant? |
|
|
Sorry, I was ignoring that h_contract in deposit permission is used to access the state. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2016-10-19 13:46 | Marcello Stanisci | New Issue | |
| 2016-10-19 13:46 | Marcello Stanisci | Status | new => assigned |
| 2016-10-19 13:46 | Marcello Stanisci | Assigned To | => Florian Dold |
| 2016-10-19 13:55 | Florian Dold | Note Added: 0011348 | |
| 2016-10-19 14:15 | Marcello Stanisci | Note Added: 0011349 | |
| 2016-10-19 14:44 | Florian Dold | Note Added: 0011350 | |
| 2016-10-19 14:48 | Florian Dold | Note Edited: 0011350 | |
| 2016-10-19 15:50 | Marcello Stanisci | Note Added: 0011351 | |
| 2016-10-19 15:54 | Florian Dold | Note Added: 0011352 | |
| 2016-10-19 16:04 | Marcello Stanisci | Note Added: 0011353 | |
| 2016-10-19 16:06 | Florian Dold | Note Added: 0011354 | |
| 2016-10-19 16:10 | Marcello Stanisci | Note Added: 0011355 | |
| 2016-10-19 16:12 | Florian Dold | Note Added: 0011356 | |
| 2016-10-19 16:22 | Florian Dold | Note Added: 0011357 | |
| 2016-10-19 16:23 | Florian Dold | Status | assigned => closed |
| 2016-10-19 16:23 | Florian Dold | Resolution | open => no change required |
| 2016-10-19 18:48 | Marcello Stanisci | Note Added: 0011358 | |
| 2016-10-20 14:30 | Marcello Stanisci | Note Added: 0011363 | |
| 2016-10-20 15:47 | Florian Dold | Note Added: 0011366 | |
| 2016-10-20 16:28 | Marcello Stanisci | Note Added: 0011367 | |
| 2016-10-20 16:31 | Marcello Stanisci | Status | closed => feedback |
| 2016-10-20 16:39 | Florian Dold | Note Added: 0011368 | |
| 2016-10-20 17:03 | Marcello Stanisci | Note Added: 0011369 | |
| 2016-10-20 17:03 | Marcello Stanisci | Status | feedback => assigned |
| 2016-10-20 17:03 | Marcello Stanisci | Status | assigned => closed |
| 2023-04-13 20:37 | Florian Dold | Category | wallet (WebExtensions) => wallet (WebExtension) |
