View Issue Details

IDProjectCategoryView StatusLast Update
0006416Talerauditorpublic2021-08-24 16:23
Reporteroec Assigned ToChristian Grothoff  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version0.7.0 
Target Version0.8Fixed in Version0.8 
Summary0006416: Same coin_pub with multiple denom_sigs - a problem?
DescriptionTaler uses a cache for fast lookups of coin_pub -> (denom_pub, denom_sig), the table known_coins. The table is populated via TEH_DB_know_coin_transaction before deposit, melt and recoup operations, i.e. independent of the outcome of those operations.

Consider the scenario where the same coin_pub is signed with different denomination keys. The first usage of one of those coins would lock the denomination value in the known_coins table. However, it is not clear (to me) what would happen if the same coin_pub than is used later with a _different_ (but also validly signed) denomination for any of the operations.

I have not come up with a particular attack to the advantage of a customer (i.e. gain profit). But maybe leaving the exchange in a confused state that the auditor might notice and complain about could lead to DoS?

Would it make sense to have (coin_pub, denom_pub) as an index for the known_coins and allow multiple entries with the same coin_pub in it?
TagsNo tags attached.
Attached Files

Activities

Christian Grothoff

2020-06-25 11:48

manager   ~0016366

Great find. Discussed with Florian, we will change the index to span coin_pub and denom_h and make sure all SELECT()s also select on both columns.

Christian Grothoff

2020-07-07 00:28

manager   ~0016391

Proposed DB change attached. May be incomplete (see FIXMEs), also requires modest changes to the rest of the exchange code.
db.patch (85,066 bytes)   

Christian Grothoff

2020-07-08 12:49

manager   ~0016396

After some internal discussion, we decided that it would be too messy (and brittle) to really allow coin_priv/pub re-use. Reasons include that having 'coin_pub' serve as a primary key is nice and simple, and that tolerating wallets that non-sensicaly re-use coin keys should really not be encouraged by allowing it per spec. So instead we will change the code/protocol to
a) detect that the coin_pub has been used before, and
b) return an error proving this.

This requires the following changes:
1) Various request signatures (deposit, melt, recoup) must include the h_denom_pub so we can prove that the same coin key was involved in different denominations, and it's not the exchange's fault.
2) The transactions must be changed to only commit to known_coins table if the rest of the transaction that includes the above signature also ends up in the DB. So we merge the two transactions (add known coin + actual work) into one transaction.
3) The transactions must be changed to detect the coin_pub re-use with a different denom_pub and then return the coin's transaction history, proving that it was previously used with the same denom_pub_h.
4) We _may_ need to return h_denom_pub with some other replies to enable verification of the signatures with the changes from (1). (not sure).

Christian Grothoff

2020-07-08 15:36

manager   ~0016398

95f5de1..8236b1e updates the docs.git specification to add the h_denom_pub where needed (deposit and melt, already present for recoup).

Christian Grothoff

2020-07-08 17:22

manager   ~0016399

1ca062fc..97dfbec0 adds the h_denom_pub to the deposit request.

Christian Grothoff

2020-07-08 18:05

manager   ~0016400

97dfbec0..8e03498a adds the h_denom_pub to the melt request.

Christian Grothoff

2020-07-08 18:27

manager   ~0016401

8e03498a..c93f6471 combines the known_coin transactions with the respective deposit/melt/recoup transactions, so that there will never be a known coin entry if the 'rest' of the transaction failed.

Christian Grothoff

2020-07-08 19:45

manager   ~0016402

c93f6471..8a1402a5 implements generation of proper errors for conflicting denomination keys on the server-side.

Christian Grothoff

2020-07-08 20:05

manager   ~0016403

8236b1e..31e66d9 documents the API change in docs.git

Christian Grothoff

2020-07-08 21:09

manager   ~0016404

31e66d9..2f705cc further updates the specs to update the coin history response to include the coin's signatures on the RECOUP request and not merely exchange signatures on the confirmation(s) where applicable.

Christian Grothoff

2020-07-08 21:30

manager   ~0016405

8a1402a5..92ac6dd1 should implement the new logic client-side as well. However, I noticed the client did previously _NOT_ handle some of the corner cases properly! Needs more testing!

Christian Grothoff

2020-07-10 23:16

manager   ~0016441

ddf95c49..7085cfef adds test cases specifically creating coins with private key re-use and checking that deposit/melt fail with this.
*recoup* handling is NOT yet tested.

Christian Grothoff

2020-07-10 23:24

manager   ~0016442

7085cfef..8ea4e50a adds a test with recoup as well. Declaring victory.

Issue History

Date Modified Username Field Change
2020-06-25 11:15 oec New Issue
2020-06-25 11:15 oec Status new => assigned
2020-06-25 11:15 oec Assigned To => Christian Grothoff
2020-06-25 11:48 Christian Grothoff Note Added: 0016366
2020-06-25 23:47 Christian Grothoff Target Version => 0.8
2020-06-25 23:48 Christian Grothoff Product Version => 0.7.0
2020-07-07 00:28 Christian Grothoff File Added: db.patch
2020-07-07 00:28 Christian Grothoff Note Added: 0016391
2020-07-08 12:49 Christian Grothoff Note Added: 0016396
2020-07-08 15:36 Christian Grothoff Note Added: 0016398
2020-07-08 17:22 Christian Grothoff Note Added: 0016399
2020-07-08 18:05 Christian Grothoff Note Added: 0016400
2020-07-08 18:27 Christian Grothoff Note Added: 0016401
2020-07-08 19:45 Christian Grothoff Note Added: 0016402
2020-07-08 20:05 Christian Grothoff Note Added: 0016403
2020-07-08 21:09 Christian Grothoff Note Added: 0016404
2020-07-08 21:30 Christian Grothoff Note Added: 0016405
2020-07-10 23:16 Christian Grothoff Note Added: 0016441
2020-07-10 23:24 Christian Grothoff Note Added: 0016442
2020-07-10 23:24 Christian Grothoff Status assigned => resolved
2020-07-10 23:24 Christian Grothoff Resolution open => fixed
2020-07-10 23:24 Christian Grothoff Fixed in Version => 0.8
2021-08-24 16:23 Christian Grothoff Status resolved => closed