From 160a4ef52caf601107e401d8352a6d5b34f50911 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Sat, 28 Sep 2019 20:53:44 +0200 Subject: [PATCH] add test for emergencies, and associated bugfixes to auditor and auditor report --- contrib/auditor-report.tex.j2 | 57 ++++++++++----- src/auditor/taler-auditor.c | 131 ++++++++++++++++++++++++++-------- src/auditor/test-auditor.sh | 54 +++++++++++++- 3 files changed, 194 insertions(+), 48 deletions(-) diff --git a/contrib/auditor-report.tex.j2 b/contrib/auditor-report.tex.j2 index 8230314ac..a5c2bec27 100644 --- a/contrib/auditor-report.tex.j2 +++ b/contrib/auditor-report.tex.j2 @@ -224,15 +224,25 @@ exposure} is the amount of coins in circulation for a particular denomination and the maximum loss for the exchange from this type of compromise. -{% if (data.emergencies|length() != 0) - or (data.emergencies_by_count|length() != 0) %} -The total risk from emergencies is -{\bf {{ data.emergencies_risk_total }} } +{% if (data.emergencies|length() != 0) %} +The total risk from emergencies detected by amount is +{\bf {{ data.emergencies_risk_by_amount }} }. +The total loss from emergencies detected by amount is +{\bf {{ data.emergencies_loss }} }. +{% endif %} + +{% if (data.emergencies_by_count|length() != 0) %} +The total risk from emergencies detected by counting coins is +{\bf {{ data.emergencies_risk_by_count }} } +The total loss from emergencies detected by counting coins could be up to +{\bf {{ data.emergencies_loss_by_count }} }. {% endif %} \subsubsection{Emergencies by counting coins} +% Table generation tested by testcase #18 in test-auditor.sh + {% if data.emergencies_by_count|length() == 0 %} {\bf No emergencies detected by counting coins.} {% else %} @@ -251,7 +261,7 @@ The total risk from emergencies is \label{table:emergencies_coin_counting} \endlastfoot {% for item in data.emergencies_by_count %} - \multicolumn{4}{l}{ {\tt \truncate{\textwidth}{ {{ item.denompub_hash }} } } } \\ + \multicolumn{4}{l}{ {\tt \truncate{0.95\textwidth}{ {{ item.denompub_hash }} } } } \\ \nopagebreak {{ item.value }} & {\tiny \begin{tabular}{c} @@ -264,8 +274,11 @@ The total risk from emergencies is \end{longtable} {% endif %} + \subsubsection{Emergencies by value deposited} +% Table generation tested by testcase #18 in test-auditor.sh + {% if data.emergencies|length() == 0 %} {\bf No emergencies by value detected.} {% else %} @@ -284,7 +297,7 @@ The total risk from emergencies is \label{table:emergencies} \endlastfoot {% for item in data.emergencies %} - \multicolumn{4}{l}{ {\tt \truncate{\textwidth}{ {{ item.denompub_hash }} } } } \\ + \multicolumn{4}{l}{ {\tt \truncate{0.95\textwidth}{ {{ item.denompub_hash }} } } } \\ \nopagebreak {{ item.value }} & {\tiny \begin{tabular}{c} @@ -306,32 +319,40 @@ Disagreements imply that either the exchange made a loss (sending out too much money), or screwed a customer (and thus at least needs to fix the financial damage done to the customer). +% Table generation tested by testcase #18 in test-auditor.sh + {% if data.amount_arithmetic_inconsistencies|length() == 0 %} {\bf No arithmetic problems detected.} {% else %} - \begin{longtable}{p{3.5cm}|l|r|r} - {\bf Operation} & {\bf Table row} & \multicolumn{2}{|c|}{ {\bf Exchange}} & \multicolumn{2}{|c}{ {\bf Auditor}} \\ + \begin{longtable}{p{3.5cm}|r|r|r|c} + {\bf Operation} & {\bf Row} & {\bf Exchange} & {\bf Auditor} & {\bf P} \\ \hline \hline \endfirsthead - {\bf Operation} & {\bf Table row} & \multicolumn{2}{|c|}{ {\bf Exchange}} & \multicolumn{2}{|c}{ {\bf Auditor}} \\ \hline \hline + {\bf Operation} & {\bf Row} & {\bf Exchange} & {\bf Auditor} & {\bf P} \\ \hline \hline \endhead \hline \hline - {\bf Operation} & {\bf Table row} & \multicolumn{2}{|c|}{ {\bf Exchange}} & \multicolumn{2}{|c}{ {\bf Auditor}} \\ + {\bf Operation} & {\bf Row} & {\bf Exchange} & {\bf Auditor} & {\bf P} \\ \endfoot \hline \hline \multicolumn{2}{l|}{ {\bf $\sum$ Deltas (Auditor-Exchange)} } & + {{ data.total_arithmetic_delta_plus }} & - - {{ data.total_arithmetic_delta_minus }} \\ + - {{ data.total_arithmetic_delta_minus }} & \\ \caption{Arithmetic inconsistencies.} \label{table:amount:arithmetic:inconsistencies} \endlastfoot {% for item in data.amount_arithmetic_inconsistencies %} - {{ item.operation }} & + \truncate{3.3cm}{ {\tiny {{ item.operation }} } } & {{ item.rowid }} & {{ item.exchange }} & - {{ item.auditor }} \\ \hline + {{ item.auditor }} & + {{ item.profitable }} \\ \hline {% endfor %} \end{longtable} + +The {\bf P} colum is set to "1" if the arithmetic problem was be determined to be +profitable for the exchange, "-1" if the problem resulted in a net loss for +the exchange, and "0" if this is unclear or at least the gain/loss is not +easily determined from the amounts and thus not included in the totals. {% endif %} @@ -397,7 +418,7 @@ would be reported separately in Section~\ref{sec:wire_check_out}. {\bf Row} & {\bf Expected} & {\bf Claimed} \\ \endfoot \hline - {\bf Total deltas} & + {\bf Total deltas} & {{ data.total_wire_out_delta_plus}} & - {{ data.total_wire_out_delta_minus}} \\ \caption{Claimed wire out aggregate totals not matching up.} @@ -484,7 +505,7 @@ public key for ``payback-master'' operations. {\bf Operation type} & Database row & {\bf Loss amount} \\ \endfoot \hline - \multicolumn{2}{l}{ {\bf Total losses} } & + \multicolumn{2}{l}{ {\bf Total losses} } & {\bf {{ data.total_bad_sig_loss}} } \\ \caption{Losses from operations performed on coins without proper signatures.} \label{table:bad_signature_losses} @@ -625,7 +646,7 @@ with respect to outgoing wire transfers. {\bf Diagnostic} & {\bf Row} & {\bf Timestamp} \\ \endfoot \hline - {\bf Total deltas} & + {\bf Total deltas} & {{ wire.total_wire_out_delta_plus }} & - {{ wire.total_wire_out_delta_minus }} \\ \caption{Outgoing wire transfer amounts not matching up.} @@ -634,7 +655,7 @@ with respect to outgoing wire transfers. {% for item in wire.wire_out_amount_inconsistencies %} {\tt \small \truncate{5.2cm}{ {{ item.wtid }} } } & {{ item.amount_wired }} & - {{ item.amount_justified }} \\ + {{ item.amount_justified }} \\ \nopagebreak {{ item.diagnostic }} & {{ item.row }} & @@ -667,7 +688,7 @@ translate into a financial loss (yet). {\bf Reserve} & {\bf Auditor} & {\bf Exchange} \endfoot \hline - {\bf Total deltas} & + {\bf Total deltas} & {{ data.total_balance_summary_delta_plus}} & - {{ data.total_balance_summary_delta_minus}} \\ \caption{Reserves balances not matching up.} diff --git a/src/auditor/taler-auditor.c b/src/auditor/taler-auditor.c index 9d4dff83f..6ab9bb344 100644 --- a/src/auditor/taler-auditor.c +++ b/src/auditor/taler-auditor.c @@ -268,10 +268,25 @@ static json_int_t number_missed_deposit_confirmations; */ static struct TALER_Amount total_missed_deposit_confirmations; +/** + * Total amount reported in all calls to #report_emergency_by_count(). + */ +static struct TALER_Amount reported_emergency_risk_by_count; + /** * Total amount reported in all calls to #report_emergency(). */ -static struct TALER_Amount reported_emergency_sum; +static struct TALER_Amount reported_emergency_risk_by_amount; + +/** + * Total amount in losses reported in all calls to #report_emergency(). + */ +static struct TALER_Amount reported_emergency_loss; + +/** + * Total amount in losses reported in all calls to #report_emergency_by_count(). + */ +static struct TALER_Amount reported_emergency_loss_by_count; /** * Expected balance in the escrow account. @@ -419,9 +434,13 @@ report_emergency_by_amount (const struct "value", TALER_JSON_from_amount_nbo (&dki->properties.value))); GNUNET_assert (GNUNET_OK == - TALER_amount_add (&reported_emergency_sum, - &reported_emergency_sum, + TALER_amount_add (&reported_emergency_risk_by_amount, + &reported_emergency_risk_by_amount, risk)); + GNUNET_assert (GNUNET_OK == + TALER_amount_add (&reported_emergency_loss, + &reported_emergency_loss, + loss)); } @@ -446,6 +465,8 @@ report_emergency_by_count (const struct uint64_t num_known, const struct TALER_Amount *risk) { + struct TALER_Amount denom_value; + report (report_emergencies_by_count, json_pack ("{s:o, s:I, s:I, s:o, s:o, s:o, s:o}", "denompub_hash", @@ -463,9 +484,17 @@ report_emergency_by_count (const struct "value", TALER_JSON_from_amount_nbo (&dki->properties.value))); GNUNET_assert (GNUNET_OK == - TALER_amount_add (&reported_emergency_sum, - &reported_emergency_sum, + TALER_amount_add (&reported_emergency_risk_by_count, + &reported_emergency_risk_by_count, risk)); + TALER_amount_ntoh (&denom_value, + &dki->properties.value); + for (uint64_t i = num_issued; iqs = qs; } - cleanup: +cleanup: GNUNET_assert (GNUNET_YES == GNUNET_CONTAINER_multihashmap_remove (rc->reserves, key, @@ -3859,16 +3888,28 @@ refresh_session_cb (void *cls, { dso->denom_balance = tmp; } - if (GNUNET_SYSERR == - TALER_amount_subtract (&total_escrow_balance, - &total_escrow_balance, - amount_with_fee)) + if (-1 == TALER_amount_cmp (&total_escrow_balance, + amount_with_fee)) { - /* This should not be possible, unless the AUDITOR - has a bug in tracking total balance. */ - GNUNET_break (0); - cc->qs = GNUNET_DB_STATUS_HARD_ERROR; - return GNUNET_SYSERR; + /* This can theoretically happen if for example the exchange + never issued any coins (i.e. escrow balance is zero), but + accepted a forged coin (i.e. emergency situation after + private key compromise). In that case, we cannot even + subtract the profit we make from the fee from the escrow + balance. Tested as part of test-auditor.sh, case #18 */ + report_amount_arithmetic_inconsistency ( + "subtracting refresh fee from escrow balance", + rowid, + &total_escrow_balance, + amount_with_fee, + 0); + } + else + { + GNUNET_assert (GNUNET_SYSERR != + TALER_amount_subtract (&total_escrow_balance, + &total_escrow_balance, + amount_with_fee)); } GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, @@ -4030,16 +4071,29 @@ deposit_cb (void *cls, { ds->denom_balance = tmp; } - if (GNUNET_SYSERR == - TALER_amount_subtract (&total_escrow_balance, - &total_escrow_balance, - amount_with_fee)) + + if (-1 == TALER_amount_cmp (&total_escrow_balance, + amount_with_fee)) { - /* This should not be possible, unless the AUDITOR - has a bug in tracking total balance. */ - GNUNET_break (0); - cc->qs = GNUNET_DB_STATUS_HARD_ERROR; - return GNUNET_SYSERR; + /* This can theoretically happen if for example the exchange + never issued any coins (i.e. escrow balance is zero), but + accepted a forged coin (i.e. emergency situation after + private key compromise). In that case, we cannot even + subtract the profit we make from the fee from the escrow + balance. Tested as part of test-auditor.sh, case #18 */ + report_amount_arithmetic_inconsistency ( + "subtracting deposit fee from escrow balance", + rowid, + &total_escrow_balance, + amount_with_fee, + 0); + } + else + { + GNUNET_assert (GNUNET_SYSERR != + TALER_amount_subtract (&total_escrow_balance, + &total_escrow_balance, + amount_with_fee)); } GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, @@ -5094,7 +5148,16 @@ run (void *cls, "Starting audit\n"); GNUNET_assert (GNUNET_OK == TALER_amount_get_zero (currency, - &reported_emergency_sum)); + &reported_emergency_loss)); + GNUNET_assert (GNUNET_OK == + TALER_amount_get_zero (currency, + &reported_emergency_risk_by_amount)); + GNUNET_assert (GNUNET_OK == + TALER_amount_get_zero (currency, + &reported_emergency_risk_by_count)); + GNUNET_assert (GNUNET_OK == + TALER_amount_get_zero (currency, + &reported_emergency_loss_by_count)); GNUNET_assert (GNUNET_OK == TALER_amount_get_zero (currency, &total_escrow_balance)); @@ -5213,7 +5276,8 @@ run (void *cls, " s:o, s:o, s:o, s:o, s:o," " s:o, s:o, s:o, s:o, s:o," " s:o, s:o, s:o, s:o, s:I," - " s:o, s:o, s:o }", + " s:o, s:o, s:o, s:o, s:o," + " s:o }", /* blocks of 5 for easier counting/matching to format string */ /* block */ "reserve_balance_insufficient_inconsistencies", @@ -5248,8 +5312,9 @@ run (void *cls, TALER_JSON_from_amount (&income_fee_total), "emergencies", report_emergencies, - "emergencies_risk_total", - TALER_JSON_from_amount (&reported_emergency_sum), + "emergencies_risk_by_amount", + TALER_JSON_from_amount ( + &reported_emergency_risk_by_amount), "reserve_not_closed_inconsistencies", report_reserve_not_closed_inconsistencies, /* block */ @@ -5309,7 +5374,15 @@ run (void *cls, "total_payback_loss", TALER_JSON_from_amount (&total_payback_loss), "emergencies_by_count", - report_emergencies_by_count + report_emergencies_by_count, + "emergencies_risk_by_count", + TALER_JSON_from_amount ( + &reported_emergency_risk_by_count), + "emergencies_loss", + TALER_JSON_from_amount (&reported_emergency_loss), + /* block */ + "emergencies_loss_by_count", + TALER_JSON_from_amount (&reported_emergency_loss_by_count) ); GNUNET_break (NULL != report); json_dumpf (report, diff --git a/src/auditor/test-auditor.sh b/src/auditor/test-auditor.sh index 83de1f6c0..d56497a9c 100755 --- a/src/auditor/test-auditor.sh +++ b/src/auditor/test-auditor.sh @@ -83,7 +83,7 @@ function audit_only () { # Cleanup to run after the auditor function post_audit () { - kill -9 `jobs -p` >/dev/null 2>/dev/null || true + kill -TERM `jobs -p` >/dev/null 2>/dev/null || true echo -n "TeXing ." ../../contrib/render.py test-audit.json test-wire-audit.json < ../../contrib/auditor-report.tex.j2 > test-report.tex || exit_fail "Renderer failed" @@ -980,6 +980,58 @@ echo "UPDATE app_banktransaction SET date='${OLD_DATE}' WHERE id='${OLD_ID}';" | +# Test where we trigger an emergency. +function test_18() { +echo "===========18: emergency=================" + +echo "DELETE FROM reserves_out;" | psql -Aqt $DB + +run_audit + +echo -n "Testing emergency detection... " + +jq -e .reserve_balance_summary_wrong_inconsistencies[0] < test-audit.json > /dev/null || exit_fail "Reserve balance inconsistency not detected" + +jq -e .emergencies[0] < test-audit.json > /dev/null || exit_fail "Emergency not detected" +jq -e .emergencies_by_count[0] < test-audit.json > /dev/null || exit_fail "Emergency by count not detected" +jq -e .amount_arithmetic_inconsistencies[0] < test-audit.json > /dev/null || exit_fail "Escrow balance calculation impossibility not detected" + +echo PASS + +echo -n "Testing risk/loss calculation... " + +AMOUNT=`jq -r .emergencies_risk_by_amount < test-audit.json` +if test "x$AMOUNT" == "xTESTKUDOS:0" +then + exit_fail "Reported amount wrong: $AMOUNT" +fi +AMOUNT=`jq -r .emergencies_risk_by_count < test-audit.json` +if test "x$AMOUNT" == "xTESTKUDOS:0" +then + exit_fail "Reported amount wrong: $AMOUNT" +fi +AMOUNT=`jq -r .emergencies_loss < test-audit.json` +if test "x$AMOUNT" == "xTESTKUDOS:0" +then + exit_fail "Reported amount wrong: $AMOUNT" +fi +AMOUNT=`jq -r .emergencies_loss_by_count < test-audit.json` +if test "x$AMOUNT" == "xTESTKUDOS:0" +then + exit_fail "Reported amount wrong: $AMOUNT" +fi + +echo PASS + + +# cannot easily undo broad DELETE operation, hence full reload +echo -n "Reloading database ..." +full_reload +echo "DONE" +} + + + # ************************************************** # FIXME: Add more tests here! :-) # Specifically: