View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0004872 | Taler | wallet (WebExtension) | public | 2017-01-30 11:05 | 2017-06-06 14:18 |
Reporter | Marcello Stanisci | Assigned To | Florian Dold | ||
Priority | normal | Severity | minor | Reproducibility | have not tried |
Status | closed | Resolution | fixed | ||
Product Version | git (master) | ||||
Target Version | 0.3 | Fixed in Version | 0.3 | ||
Summary | 0004872: Nonces in contracts | ||||
Description | In order to avoid illimited contract replication by attckers, the contract should contain a nonce which is invertible by the wallet. Nevertheless, the nonce is optional (likely used for physical goods), so the wallet should obey to some triggering mechanism coming from the merchant: that may be a HTTP header like 'X-Taler-Contract-Nonce: yes' for JS-free payments, or a 'nonce' optional argument for the offerContractFrom(url[, nonce]) function in taler-wallet-lib | ||||
Tags | No tags attached. | ||||
|
Not sure if I understand what this is good for. Can you give an example? Of course for some types of contracts the merchant needs to limit how many they give out, but why does this need any support from the wallet? |
|
Example: Evil merchant E gets a proposal from Good merchant G, and then E just replies that proposal from his shop, as if G signed it. Now your wallet visits E's website and gets a replication of the proposal that E got from G beforehand. How can your wallet detect this situation? By using an invertible nonce, your wallet would easily see that the nounce in the proposal is not right, therefore aborting that malicious business. And the fact that a proposal cannot be replicated also implies a limit in the number of given proposals (as only the legitimate merchant gives them out). |
|
I don't see why this is a problem. What does E gain here? If the wallet signs a deposit permission, only merchant G can deposit it and get the money ... |
|
For example, the proposal replicated by E may refer to some limited good, so G would easily run out of that thing. Furthermore, E may keep replicating that proposal even after G has run out of that thing, therefore making customers unhappy and just messing things around. |
|
IMHO this just adds another complication to an already complex API. All these problems (rate limiting, restricting to whom we give out offers) can be achieved by other, standard means already. Even in your example, the wallet has the same information as the evil merchant, I don't see why the evil merchant couldn't just generate the URL with the nonce on the fly. And again, there are other simpler/standard means to prevent this. |
|
Florian, I don't see your point, especially I don't see simpler/standard means to prevent this. The standard means to prevent a replay attack is to use a nonce, which is what we proposed here. It also doesn't complicate things so much, just one extra arg to give to the request for a contract, and one tiny thing to check (if there is a nonce in the contract, is it mine?) and store (store H^{-1}(nonce)). The attack Marcello proposed OTOH is quite harmful, basically I try to give you a bad reputation by replaying your proposal to other parties, who then try to accept a proposal which you are no longer willing to entertain (but did properly sign!). If the offer-URL is not in the contract, this can even happen without you noticing (until the money is in your bank account, or until customers complain). So I think this is a critical fix. |
|
There must be some fundamental misunderstanding here, either from me or you. Every contract has a unique transaction_id already. If evil-merchant.taler.net replays a contract from good-merchant.taler.net to multiple customers, then good-merchant.taler.net will reject every payment but the first, and the worst thing that could happen is that later customers have to refresh their dirtied coins. I still don't see how the nonce can even prevent the attack you're proposing. Especially since the offerContractFrom is implemented by _merchant JavaScript_. So you'd propose that the wallet always has to fetch the contract? Then why not add a "generated_from":"<contract_generation_url>" to the contract? This would actually prevent the contract relay attack ... With the nonce, the evil-merchant.taler.net can still just go do good-merchant.taler.net and get the nonce and H^{-1}(nonce), this would make the wallet still happy with your proposed API. |
|
Note that you really need the unique transaction_id, otherwise you can't distinguish between replays of a contract that was paid for (on lost cookies) on time and "new" payments that might be too late. We never allow different customers to pay for the same proposal, with good reason! Maybe this is the misunderstanding .... |
|
11671: this assumes the offer URI is in the contract. If evil-merchant points the wallet to the evil's offer URI, the good merchant may not ever see the payment. Similarly, forcing customers to do a refresh is still a really bad user experience (TM), while a wallet that detects that a contract lacks it's nonce can avoid trying to pay in the first place. As for the 'generated from', that's difficult as the merchant may not know the public facing URI (reverse proxies, CDN, etc.). So let's not trust HTTPS here. |
|
And yes, evil-merchant can just fetch a fresh contract, that's fine by me, the point is that the good-merchant can then decide to not have 100 outstanding proposals (with different nonces) if he has only 5 items. 11672: transaction ID helps the merchant make sure it's a unique contract/proposal. But the customer/wallet has currently no way to tell that it got a unique transaction ID. With the nonce, the wallet _also_ has proof that this is a fresh contract, and not a replay. |
|
Ah okay, now it's starting to make sense! From what I read above, I thought that the MERCHANT frontend generates the nonce, and not the wallet, which makes absolutely no sense. But it does mean that we have to remove the possibility to just give a contract to the wallet, no? So we can only give a URL to the wallet to fetch a contract from it. Do we still allow inline contracts? And if we allow inline contracts, couldn't this be used to circumvent the nonce mechanism and do replay anyway? Would the contract have to specify a "allow_inline":true for contracts where replay doesn't matter? Opinions? |
|
Not quite, the merchant can choose *not* to include the nonce in the proposal at all! In this case, this means that the proposal is valid even if it is not unique, i.e. multiple customers may pay for it. This should ONLY be used for goods that are infinite (news paper articles, media), as then the merchant doesn't care about the contract being replayed, cached or redistributed by some evil-merchant --- as long as he gets paid again and again ;-). So there the "bigger" change might be to allow the merchant to accept a second payment for a contract with the same transaction ID. Regardless, my point is that in this case, we can still allow inline contracts. You are right that in general, yes, the NONCE requires a fresh request to the offer/proposal URL. |
|
Okay, +1 from me to the change. Let's figure out the details. Just to recap: When the contract contains the field "nonce" and the contract was NOT fetched via offerContractFrom, then the wallet should reject the contract and show some error/warning to the user. If it was fetched via offerContractFrom, then the nonce generated by offerContractFrom must match the nonce in the contract, otherwise the wallet should reject the contract, again with an error/warning. This requires moving "offerContractFrom(url)" into the wallet, since currently it's just a merchant-side wrapper around "offerContract(contract_json)". Do we agree on this? How should we transfer the nonce though? Options are a &nonce=<..> in the query string, or a X-Taler-Contract-Nonce in the request for the contract. Also, if the request does NOT contain a nonce, then the merchant MUST chose a nonce randomly, otherwise we get one of those freely repeatable contracts, right? |
|
This requires moving "offerContractFrom(url)" into the wallet, since currently it's just a merchant-side wrapper around "offerContract(contract_json)". => Don't understand, could be. I'm for &nonce=NONCE. Also, we discussed that NONCE should probably be a point on an ECC, and the original value an ECC DLOG instead of using a hash function. That way, the wallet can prove _repeatedly_ and _exclusively_ that it generated the nonce, instead of having to disclose this secret in order to prove that it is the one who got the original contract (and henceforth no longer being in the exclusive position to prove this point). Finally, I'd keep it simple and say that _all_ these requests must contain a nonce, otherwise the merchant MUST generate a 400 Bad request. |
|
11678: checking the nonce should happen regardless of the use of offerContractFrom(). In other words, the use of the nonce doesn't care if the purchase is done with or without JavaScript. |
|
Fixed in the wallet with e2738c5. For now it's just a random number, since we don't have any protocols that would use the key pair (yet!). |
|
Well, pretty much for everyone _but_ the wallet it is just a random number. The wallet can use the ZKP of knowledge of DLOG (should already be implemented, either in libbrandt or, if Markus already moved it, in libgnunetutil) to show it is the nonce owner. But that's largely a separate protocol to be implemented _way_ later. Still, not sure why you didn't do the ECC mult right now, shouldn't cost that much, right? |
|
Is there any reason to do this at the level of curve points and DLOG ZKPs when we could just have an EdDSA key pair and prove knowledge of the private key by signing a challenge? I simply didn't implement the version with a key pair yet because we don't have a direct use for it now, and adding it means adding some more boilerplate to the wallet (new object store, new wallet API function, new IPC handler, new IPC wrapper, error handling). |
|
It really makes no difference AFAIK whether we end up using signatures or a ZKP here. My real point was to please already use a point and keep the corresponding scalar in the wallet. |
|
Sure. Also, we should address how this interacts with inline proposals. Right now (if inline contracts were actually implemented), a malicious merchant could circumvent the nonce mechanism by replaying the proposal through headers. Should we have a no_inline=true that merchants can set to prevent this? |
|
Generating nonces and storing their private key in the db is now implemented in 3a074443. |
|
I thought we had discussed about eliminating the inline proposals/contracts altogether... |
|
Ah, okay, just wanted to clarify. |
Date Modified | Username | Field | Change |
---|---|---|---|
2017-01-30 11:05 | Marcello Stanisci | New Issue | |
2017-01-30 11:05 | Marcello Stanisci | Status | new => assigned |
2017-01-30 11:05 | Marcello Stanisci | Assigned To | => Florian Dold |
2017-01-30 16:54 | Florian Dold | Note Added: 0011665 | |
2017-01-30 17:47 | Marcello Stanisci | Note Added: 0011666 | |
2017-01-30 18:00 | Florian Dold | Note Added: 0011667 | |
2017-01-30 18:13 | Marcello Stanisci | Note Added: 0011668 | |
2017-01-30 18:20 | Florian Dold | Note Added: 0011669 | |
2017-01-30 18:28 | Christian Grothoff | Note Added: 0011670 | |
2017-01-30 19:02 | Florian Dold | Note Added: 0011671 | |
2017-01-30 19:07 | Florian Dold | Note Edited: 0011671 | |
2017-01-30 19:12 | Florian Dold | Note Added: 0011672 | |
2017-01-30 19:49 | Christian Grothoff | Note Added: 0011673 | |
2017-01-30 19:51 | Christian Grothoff | Note Added: 0011674 | |
2017-01-30 19:58 | Florian Dold | Note Added: 0011675 | |
2017-01-30 20:03 | Christian Grothoff | Note Added: 0011676 | |
2017-01-30 20:38 | Florian Dold | Note Added: 0011678 | |
2017-01-30 21:13 | Christian Grothoff | Note Added: 0011679 | |
2017-01-31 14:18 | Marcello Stanisci | Note Added: 0011680 | |
2017-02-13 03:31 | Florian Dold | Status | assigned => resolved |
2017-02-13 03:31 | Florian Dold | Resolution | open => fixed |
2017-02-13 03:31 | Florian Dold | Note Added: 0011723 | |
2017-02-13 09:20 | Christian Grothoff | Note Added: 0011724 | |
2017-02-13 09:20 | Christian Grothoff | Status | resolved => feedback |
2017-02-13 09:20 | Christian Grothoff | Resolution | fixed => reopened |
2017-02-13 09:21 | Christian Grothoff | Product Version | => git (master) |
2017-02-13 09:21 | Christian Grothoff | Target Version | => 0.3 |
2017-02-13 09:36 | Florian Dold | Note Added: 0011725 | |
2017-02-13 09:38 | Christian Grothoff | Note Added: 0011726 | |
2017-02-13 09:40 | Florian Dold | Note Added: 0011727 | |
2017-02-13 09:54 | Florian Dold | Note Added: 0011728 | |
2017-02-13 10:03 | Christian Grothoff | Note Added: 0011730 | |
2017-02-13 10:04 | Florian Dold | Note Added: 0011731 | |
2017-02-13 10:04 | Florian Dold | Status | feedback => resolved |
2017-02-13 10:04 | Florian Dold | Resolution | reopened => fixed |
2017-02-13 13:47 | Christian Grothoff | Fixed in Version | => 0.3 |
2017-06-06 14:18 | Christian Grothoff | Status | resolved => closed |
2023-04-13 20:37 | Florian Dold | Category | wallet (WebExtensions) => wallet (WebExtension) |