View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0006413 | Taler | auditor | public | 2020-06-24 11:39 | 2021-08-24 14:23 |
| Reporter | oec | Assigned To | Christian Grothoff | Resolution deadline | |
| Implementer | Reviewer | ||||
| Release signed by | Deployed by | ||||
| Verifier | Communicator | ||||
| Data protection authority notification required | No | ||||
| CSIRT notification required | No | ||||
| Estimated effort (hours) | Actual effort (hours) | 0 | |||
| Fast-track | No | ||||
| Work package | none | ||||
| Priority | normal | Severity | tweak | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Product Version | git (master) | ||||
| Target Version | 0.8 | Fixed in Version | 0.8 | ||
| Summary | 0006413: Not enough checks for deposits | ||||
| Description | taler-helper-auditor-deposits.c implements only one check for the deposits: "deposit confirmation missing". I suggest to add at least the following checks: - compare `refund_deadline` and `wire_deadline` to timestamp - `coin_pub` and `coin_sig` are sound - `amount_with_fee_val` and `amount_with_fee_frac` are sound - `merchant_pub` points to an existing merchant In comparison, for aggregation, coins, wire and reserves there is an average of 20 distict checks implemented. | ||||
| Tags | No tags attached. | ||||
| - refund_deadline and wire_deadline and timestamp are (almost) orthogonal. You can legitimately set them far in the future, and I think the only constraint that applies is that refund_deadline <= wire_deadline here. What exactly would you want to check that certainly holds? - coin_pub and coin_sig: these are checked in taler-helper-auditor-coins. It seems strictly wasteful to check the same signature multiple times, and would also result in duplicate reports (which doesn't make them easier to understand). - amount_with_fee*-soundness: not sure which check you want to see, but again I wonder if this is not checked elsewhere. - merchant_pub: what do you mean by "existing merchant"? We don't have a merchant registry. So very unclear to me what you would want to check here. The merchant_pub primarily is of interest because that is the key used to sign _refunds_. So we don't really care if it 'exists', the merchant could use a different merchant_pub for every deposit. | |
| - As long as the tests for coin_pub/con_sig is implemented _somewhere_, that's fine. However, I suggest to put a description and reference to the location in taler-helper-auditor-deposits.c - amount_with_fee*, same: If there are checks implemented somewhere, a note in the taler-*-deposits.c would be helpful - merchant_pub: I see. For some reason I thought that merchants are registered. Would it make sense to check the refunds to be signed with a merchant_pub that is listed in a deposit? | |
| Merchants are not registered, we want the system to be open. Forcing registration on merchants is not desirable, as that would allow an exchange to refuse to do business with some merchants, and we do not believe that this is socially desirable. HOWEVER, it _may_ be that in the future _SOME_ form of registration is required from a regulatory perspective (KYC). *IF* this registration _WERE_ tied to a merchant_pub, then it may make sense to add some check here. But, that is only if regulatory requirements are articulated in such a way, and that remains very much unclear. So for the currently specified and implemented protocol, such a check makes no sense. | |
| A check that the refund deadline should not be after the wire deadline was added to the taler-helper-auditor-coins in Git 2570b21d..62d5aae1. | |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2020-06-24 11:39 | oec | New Issue | |
| 2020-06-24 11:39 | oec | Status | new => assigned |
| 2020-06-24 11:39 | oec | Assigned To | => Christian Grothoff |
| 2020-06-24 11:45 | Christian Grothoff | Note Added: 0016351 | |
| 2020-06-24 11:46 | Christian Grothoff | Status | assigned => feedback |
| 2020-06-24 11:57 | Christian Grothoff | Severity | major => text |
| 2020-06-24 12:00 | Christian Grothoff | Severity | text => tweak |
| 2020-06-25 08:55 | oec | Note Added: 0016362 | |
| 2020-06-25 08:55 | oec | Status | feedback => assigned |
| 2020-06-25 09:05 | Christian Grothoff | Note Added: 0016365 | |
| 2020-06-25 21:48 | Christian Grothoff | Product Version | => git (master) |
| 2020-06-25 21:48 | Christian Grothoff | Target Version | => 0.8 |
| 2020-07-14 19:15 | Christian Grothoff | Note Added: 0016448 | |
| 2020-07-14 19:16 | Christian Grothoff | Status | assigned => informing |
| 2020-07-14 19:16 | Christian Grothoff | Resolution | open => fixed |
| 2020-07-14 19:16 | Christian Grothoff | Fixed in Version | => 0.8 |
| 2021-08-24 14:23 | Christian Grothoff | Status | informing => closed |