View Issue Details

IDProjectCategoryView StatusLast Update
0004872Talerwallet (WebExtensions)public2017-06-06 14:18
ReporterMarcello StanisciAssigned ToFlorian Dold 
PrioritynormalSeverityminorReproducibilityhave not tried
Status closedResolutionfixed 
Product VersionSVN HEAD 
Target Version0.3Fixed in Version0.3 
Summary0004872: Nonces in contracts
DescriptionIn 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
TagsNo tags attached.

Activities

Florian Dold

2017-01-30 16:54

manager   ~0011665

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?

Marcello Stanisci

2017-01-30 17:47

manager   ~0011666

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).

Florian Dold

2017-01-30 18:00

manager   ~0011667

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 ...

Marcello Stanisci

2017-01-30 18:13

manager   ~0011668

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.

Florian Dold

2017-01-30 18:20

manager   ~0011669

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.

Christian Grothoff

2017-01-30 18:28

manager   ~0011670

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.

Florian Dold

2017-01-30 19:02

manager   ~0011671

Last edited: 2017-01-30 19:07

View 2 revisions

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.

Florian Dold

2017-01-30 19:12

manager   ~0011672

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 ....

Christian Grothoff

2017-01-30 19:49

manager   ~0011673

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.

Christian Grothoff

2017-01-30 19:51

manager   ~0011674

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.

Florian Dold

2017-01-30 19:58

manager   ~0011675

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?

Christian Grothoff

2017-01-30 20:03

manager   ~0011676

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.

Florian Dold

2017-01-30 20:38

manager   ~0011678

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?

Christian Grothoff

2017-01-30 21:13

manager   ~0011679

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.

Marcello Stanisci

2017-01-31 14:18

manager   ~0011680

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.

Florian Dold

2017-02-13 03:31

manager   ~0011723

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!).

Christian Grothoff

2017-02-13 09:20

manager   ~0011724

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?

Florian Dold

2017-02-13 09:36

manager   ~0011725

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).

Christian Grothoff

2017-02-13 09:38

manager   ~0011726

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.

Florian Dold

2017-02-13 09:40

manager   ~0011727

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?

Florian Dold

2017-02-13 09:54

manager   ~0011728

Generating nonces and storing their private key in the db is now implemented in 3a074443.

Christian Grothoff

2017-02-13 10:03

manager   ~0011730

I thought we had discussed about eliminating the inline proposals/contracts altogether...

Florian Dold

2017-02-13 10:04

manager   ~0011731

Ah, okay, just wanted to clarify.

Issue History

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 View Revisions
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 => SVN HEAD
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