diff options
| -rw-r--r-- | contrib/auditor-report.tex.j2 | 34 | ||||
| -rw-r--r-- | src/auditor/taler-auditor.c | 59 | ||||
| -rw-r--r-- | src/auditor/taler-wire-auditor.c | 6 | ||||
| -rwxr-xr-x | src/auditor/test-auditor.sh | 153 |
4 files changed, 194 insertions, 58 deletions
diff --git a/contrib/auditor-report.tex.j2 b/contrib/auditor-report.tex.j2 index e61541279..cbdf6c34b 100644 --- a/contrib/auditor-report.tex.j2 +++ b/contrib/auditor-report.tex.j2 @@ -361,29 +361,33 @@ violations do not imply that the wire transfer was actually made (as that is a separate check). Note that not making the wire transfer would be reported separately in Section~\ref{sec:wire_check_out}. +% Table generation tested by testcase #XX in test-auditor.sh {% if data.wire_out_inconsistencies|length() == 0 %} {\bf All aggregations matched up.} {% else %} - \begin{longtable}{p{1.5cm}|l|r|r} - {\bf Destination account} & {\bf Database row} & {\bf Expected} & {\bf Claimed} \\ \hline \hline + \begin{longtable}{r|r|r} + \multicolumn{3}{c}{ {\bf Destination account} } \\ + {\bf Row} & {\bf Expected} & {\bf Claimed} \\ \hline \hline \endfirsthead - {\bf Destination account} & {\bf Database row} & {\bf Expected} & {\bf Claimed} \\ \hline \hline + \multicolumn{3}{c}{ {\bf Destination account} } \\ + {\bf Row} & {\bf Expected} & {\bf Claimed} \\ \hline \hline \endhead \hline \hline - {\bf Destination account} & {\bf Database row} & {\bf Expected} & {\bf Claimed} \\ + \multicolumn{3}{c}{ {\bf Destination account} } \\ + {\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.} \label{table:reserve:wire_out_balance_inconsistencies} \endlastfoot {% for item in data.wire_out_inconsistencies %} - \multicolumn{6}{l}{ {\tt {{ item.destination_account }} } } \\ + \multicolumn{3}{l}{ {\tt \truncate{0.95\textwidth}{ {{ item.destination_account.url }} } } } \\ \nopagebreak - & {{ item.rowid }} & + {{ item.rowid }} & {{ item.expected }} & {{ item.claimed }} \\ \hline {% endfor %} @@ -443,7 +447,7 @@ the (hash of the) denomination public key for ``payback-verify'' and ``deposit-verify'' operations, and the master public key for ``payback-master'' operations. -% Table generation tested by testcase #4/#5 in test-auditor.sh +% Table generation tested by testcase #4/#5/#6/#7/#13 in test-auditor.sh {% if data.bad_sig_losses|length() == 0 %} {\bf All signatures were valid.} @@ -583,6 +587,8 @@ account. This section highlights cases where the exchange missbehaved with respect to outgoing wire transfers. +% Table generation tested by testcase #11 in test-auditor.sh + {% if wire.wire_out_amount_inconsistencies|length() == 0 %} {\bf All outgoing wire transfers matched up.} {% else %} @@ -683,6 +689,8 @@ have created follow-up errors in this table. This section describes issues found by the wire auditor that do not have a clear financial impact. +% Table generation tested by testcase #17 in test-auditor.sh + {% if wire.row_inconsistencies|length() == 0 %} {\bf No wire row inconsistencies found.} {% else %} @@ -757,10 +765,12 @@ with duplicate or malformed wire transfer subjects. This section lists cases where the exchange's database may be ambiguous with respect to what wire fee it charges at what time. +% Table generation tested by testcase #14 in test-auditor.sh + {% if data.wire_fee_time_inconsistencies|length() == 0 %} {\bf No wire fee timing issues detected.} {% else %} - \begin{longtable}{p{1.5cm}|r|p{5.5}} + \begin{longtable}{p{1.5cm}|r|p{6}} {\bf Wire format} & {\bf Timestamp} & {\bf Diagnostic} \\ \hline \hline \endfirsthead @@ -788,10 +798,12 @@ with respect to what wire fee it charges at what time. This section describes issues found that do not have a clear financial impact. +% Table generation tested by testcase #13/#15 in test-auditor.sh + {% if data.row_inconsistencies|length() == 0 %} {\bf No row inconsistencies found.} {% else %} - \begin{longtable}{p{1.5cm}|l|p{6cm}} + \begin{longtable}{p{2.5cm}|l|p{7cm}} {\bf Table} & {\bf Row} & {\bf Diagnostic} \\ \hline \hline \endfirsthead @@ -933,7 +945,7 @@ implications. This section lists issues with wire transfers related to timestamps. -% Table generation tested by testcase #10 in test-auditor.sh +% Table generation tested by testcase #10/#17 in test-auditor.sh {% if wire.row_minor_inconsistencies|length() == 0 %} {\bf No timestamp issues detected.} diff --git a/src/auditor/taler-auditor.c b/src/auditor/taler-auditor.c index f2bcad6b8..5a621992e 100644 --- a/src/auditor/taler-auditor.c +++ b/src/auditor/taler-auditor.c @@ -2088,7 +2088,7 @@ check_transaction_history_for_deposit (const struct { report_row_inconsistency ("deposits", tl->serial_id, - "h_wire does not match wire"); + "h(wire) does not match wire"); } } amount_with_fee = &tl->details.deposit->amount_with_fee; @@ -2368,21 +2368,17 @@ wire_transfer_information_cb (void *cls, TALER_JSON_merchant_wire_signature_hash (account_details, &hw)) { - wcc->qs = GNUNET_DB_STATUS_HARD_ERROR; report_row_inconsistency ("aggregation", rowid, "failed to compute hash of given wire data"); - return; } - if (0 != - GNUNET_memcmp (&hw, - h_wire)) + else if (0 != + GNUNET_memcmp (&hw, + h_wire)) { - wcc->qs = GNUNET_DB_STATUS_HARD_ERROR; report_row_inconsistency ("aggregation", rowid, "database contains wrong hash code for wire details"); - return; } /* Obtain coin's transaction history */ @@ -2514,16 +2510,7 @@ wire_transfer_information_cb (void *cls, wcc->qs = GNUNET_DB_STATUS_HARD_ERROR; report_row_inconsistency ("aggregation", rowid, - "wire method of aggregate do not match wire transfer"); - } - if (0 != GNUNET_memcmp (h_wire, - &wcc->h_wire)) - { - wcc->qs = GNUNET_DB_STATUS_HARD_ERROR; - report_row_inconsistency ("aggregation", - rowid, - "account details of aggregate do not match account details of wire transfer"); - return; + "target of outgoing wire transfer do not match hash of wire from deposit"); } if (exec_time.abs_value_us != wcc->date.abs_value_us) { @@ -2533,18 +2520,22 @@ wire_transfer_information_cb (void *cls, report_row_inconsistency ("aggregation", rowid, "date given in aggregate does not match wire transfer date"); - return; } /* Add coin's contribution to total aggregate value */ - if (GNUNET_OK != - TALER_amount_add (&wcc->total_deposits, - &wcc->total_deposits, - &coin_value_without_fee)) { - GNUNET_break (0); - wcc->qs = GNUNET_DB_STATUS_HARD_ERROR; - return; + struct TALER_Amount res; + + if (GNUNET_OK != + TALER_amount_add (&res, + &wcc->total_deposits, + &coin_value_without_fee)) + { + GNUNET_break (0); + wcc->qs = GNUNET_DB_STATUS_HARD_ERROR; + return; + } + wcc->total_deposits = res; } } @@ -2732,17 +2723,18 @@ check_wire_out_cb GNUNET_free (method); return GNUNET_SYSERR; } - if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != wcc.qs) { - /* FIXME: can we provide a more detailed error report? */ - report_row_inconsistency ("wire_out", - rowid, - "audit of associated transactions failed"); - GNUNET_free (method); - return GNUNET_OK; + /* Note: detailed information was already logged + in #wire_transfer_information_cb, so here we + only log for debugging */ + GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, + "Inconsitency for wire_out %llu (WTID %s) detected\n", + (unsigned long long) rowid, + TALER_B2S (wtid)); } + /* Subtract aggregation fee from total (if possible) */ { const struct TALER_Amount *wire_fee; @@ -5276,6 +5268,7 @@ run (void *cls, /* Tested in test-auditor.sh #4/#5/#6/#7/#13 */ "total_bad_sig_loss", TALER_JSON_from_amount (&total_bad_sig_loss), + /* Tested in test-auditor.sh #14/#15 */ "row_inconsistencies", report_row_inconsistencies, "denomination_key_validity_withdraw_inconsistencies", diff --git a/src/auditor/taler-wire-auditor.c b/src/auditor/taler-wire-auditor.c index c53527f16..e1af48883 100644 --- a/src/auditor/taler-wire-auditor.c +++ b/src/auditor/taler-wire-auditor.c @@ -404,12 +404,12 @@ do_shutdown (void *cls) " s:o, s:o, s:o, s:o, s:o," " s:o, s:o, s:o, s:o }", /* blocks of 5 */ - /* Tested in test-auditor.sh #11 */ + /* Tested in test-auditor.sh #11, #15 */ "wire_out_amount_inconsistencies", report_wire_out_inconsistencies, "total_wire_out_delta_plus", TALER_JSON_from_amount (&total_bad_amount_out_plus), - /* Tested in test-auditor.sh #11 */ + /* Tested in test-auditor.sh #11, #15 */ "total_wire_out_delta_minus", TALER_JSON_from_amount (&total_bad_amount_out_minus), /* Tested in test-auditor.sh #2 */ @@ -430,7 +430,7 @@ do_shutdown (void *cls) TALER_JSON_from_amount (&total_missattribution_in), "row_inconsistencies", report_row_inconsistencies, - /* Tested in test-auditor.sh #10 */ + /* Tested in test-auditor.sh #10/#17 */ "row_minor_inconsistencies", report_row_minor_inconsistencies, /* block */ diff --git a/src/auditor/test-auditor.sh b/src/auditor/test-auditor.sh index c038910c4..5058fea92 100755 --- a/src/auditor/test-auditor.sh +++ b/src/auditor/test-auditor.sh @@ -9,7 +9,7 @@ set -eu # Set of numbers for all the testcases. # When adding new tests, increase the last number: -ALL_TESTS=`seq 0 13` +ALL_TESTS=`seq 0 17` # $TESTS determines which tests we should run. # This construction is used to make it easy to @@ -788,29 +788,31 @@ echo PASS # cannot easily undo aggregator, hence full reload echo -n "Reloading database ..." -# full_reload +full_reload echo "DONE" } -# FIXME: Test where h_wire in the deposit table is wrong -test_99() { -echo "===========99: deposit wire hash wrong=================" +# Test where h_wire in the deposit table is wrong +function test_15() { +echo "===========15: deposit wire hash wrong=================" # Modify h_wire hash, so it is inconsistent with 'wire' echo "UPDATE deposits SET h_wire='\x973e52d193a357940be9ef2939c19b0575ee1101f52188c3c01d9005b7d755c397e92624f09cfa709104b3b65605fe5130c90d7e1b7ee30f8fc570f39c16b853' WHERE deposit_serial_id=1" | psql -Aqt $DB # The auditor checks h_wire consistency only for # coins where the wire transfer has happened, hence # run aggregator first to get this test to work. -# -# FIXME: current test database has transfers still -# in the *distant* future, test cannot yet work. -# patch up once DB was re-generated! run_audit aggregator -# FIXME: check for the respective inconsistency in the report! +echo -n "Testing inconsistency detection... " +TABLE=`jq -r .row_inconsistencies[0].table < test-audit.json` +if test "x$TABLE" != "xaggregation" -a "x$TABLE" != "xdeposits" +then + exit_fail "Reported table wrong: $TABLE" +fi +echo PASS # cannot easily undo aggregator, hence full reload echo -n "Reloading database ..." @@ -819,13 +821,142 @@ echo "DONE" } +# Test where wired amount (wire out) is wrong +function test_16() { +echo "===========16: incorrect wire_out amount=================" + +# First, we need to run the aggregator so we even +# have a wire_out to modify. +pre_audit aggregator + +# Modify wire amount, such that it is inconsistent with 'aggregation' +# (exchange account is #2, so the logic below should select the outgoing +# wire transfer): +OLD_ID=`echo "SELECT id FROM app_banktransaction WHERE debit_account_id=2 ORDER BY id LIMIT 1;" | psql $DB -Aqt` +OLD_AMOUNT=`echo "SELECT amount FROM app_banktransaction WHERE id='${OLD_ID}';" | psql $DB -Aqt` +NEW_AMOUNT="TESTKUDOS:50" +echo "UPDATE app_banktransaction SET amount='${NEW_AMOUNT}' WHERE id='${OLD_ID}';" | psql -Aqt $DB + +audit_only + +echo -n "Testing inconsistency detection... " + +AMOUNT=`jq -r .wire_out_amount_inconsistencies[0].amount_justified < test-wire-audit.json` +if test "x$AMOUNT" != "x$OLD_AMOUNT" +then + exit_fail "Reported justified amount wrong: $AMOUNT" +fi +AMOUNT=`jq -r .wire_out_amount_inconsistencies[0].amount_wired < test-wire-audit.json` +if test "x$AMOUNT" != "x$NEW_AMOUNT" +then + exit_fail "Reported wired amount wrong: $AMOUNT" +fi +TOTAL_AMOUNT=`jq -r .total_wire_out_delta_minus < test-wire-audit.json` +if test "x$TOTAL_AMOUNT" != "xTESTKUDOS:0" +then + exit_fail "Reported total wired amount minus wrong: $TOTAL_AMOUNT" +fi +TOTAL_AMOUNT=`jq -r .total_wire_out_delta_plus < test-wire-audit.json` +if test "x$TOTAL_AMOUNT" = "xTESTKUDOS:0" +then + exit_fail "Reported total wired amount plus wrong: $TOTAL_AMOUNT" +fi +echo PASS + +echo "Second modification: wire nothing" +NEW_AMOUNT="TESTKUDOS:0" +echo "UPDATE app_banktransaction SET amount='${NEW_AMOUNT}' WHERE id='${OLD_ID}';" | psql -Aqt $DB + +audit_only + +echo -n "Testing inconsistency detection... " + +AMOUNT=`jq -r .wire_out_amount_inconsistencies[0].amount_justified < test-wire-audit.json` +if test "x$AMOUNT" != "x$OLD_AMOUNT" +then + exit_fail "Reported justified amount wrong: $AMOUNT" +fi +AMOUNT=`jq -r .wire_out_amount_inconsistencies[0].amount_wired < test-wire-audit.json` +if test "x$AMOUNT" != "x$NEW_AMOUNT" +then + exit_fail "Reported wired amount wrong: $AMOUNT" +fi +TOTAL_AMOUNT=`jq -r .total_wire_out_delta_minus < test-wire-audit.json` +if test "x$TOTAL_AMOUNT" != "x$OLD_AMOUNT" +then + exit_fail "Reported total wired amount minus wrong: $TOTAL_AMOUNT (wanted $OLD_AMOUNT)" +fi +TOTAL_AMOUNT=`jq -r .total_wire_out_delta_plus < test-wire-audit.json` +if test "x$TOTAL_AMOUNT" != "xTESTKUDOS:0" +then + exit_fail "Reported total wired amount plus wrong: $TOTAL_AMOUNT" +fi +echo PASS + +post_audit + + +# Undo +echo "UPDATE app_banktransaction SET amount='${OLD_AMOUNT}' WHERE id='${OLD_ID}';" | psql -Aqt $DB +} + + + + +# Test where wire-out timestamp is wrong +function test_17() { +echo "===========17: incorrect wire_out timestamp=================" + +# First, we need to run the aggregator so we even +# have a wire_out to modify. +pre_audit aggregator + +# Modify wire amount, such that it is inconsistent with 'aggregation' +# (exchange account is #2, so the logic below should select the outgoing +# wire transfer): +OLD_ID=`echo "SELECT id FROM app_banktransaction WHERE debit_account_id=2 ORDER BY id LIMIT 1;" | psql $DB -Aqt` +OLD_DATE=`echo "SELECT date FROM app_banktransaction WHERE id='${OLD_ID}';" | psql $DB -Aqt` +# Note: need - interval '1h' as "NOW()" may otherwise be exactly what is already in the DB +# (due to rounding, if this machine is fast...) +echo "UPDATE app_banktransaction SET date=NOW()- interval '1 hour' WHERE id='${OLD_ID}';" | psql -Aqt $DB + +audit_only +post_audit + +echo -n "Testing inconsistency detection... " +TABLE=`jq -r .row_minor_inconsistencies[0].table < test-wire-audit.json` +if test "x$TABLE" != "xwire_out" +then + exit_fail "Reported table wrong: $TABLE" +fi +DIAG=`jq -r .row_minor_inconsistencies[0].diagnostic < test-wire-audit.json` +if test "x$DIAG" != "xexecution date missmatch" +then + exit_fail "Reported diagnostic wrong: $DIAG" +fi +echo PASS + +# Undo +echo "UPDATE app_banktransaction SET date='${OLD_DATE}' WHERE id='${OLD_ID}';" | psql -Aqt $DB +} + + # ************************************************** -# Add more tests here! :-) +# FIXME: Add more tests here! :-) +# Specifically: +# - emergencies (detection) +# - revocation (payback, accepting +# of coins despite denomination revocation) +# - refunds +# - reserve closure (or lack thereof) +# - arithmetic problems +# - wire.row_inconsistencies (i.e. duplicate wire offset) # ************************************************** + # *************** Main logic starts here ************** # ####### Setup globals ###### |