implement duplicate reserve_pub detection in fakebank, add test (fails in pybank), for #6863

contrib/gana

@@ -1 +1 @@
-Subproject commit 3d2fde0d1f6bd4eee7087ef061755619eaae0dc2
+Subproject commit c70de07db9e32b5cf419c9c4d245f6f73f03b2d2

src/bank-lib/fakebank.c

@@ -81,7 +81,12 @@ struct Transaction
     /**
      * Transfer FROM the exchange.
      */
-    T_DEBIT
+    T_DEBIT,
+
+    /**
+     * Exchange-to-exchange WAD transfer.
+     */
+    T_WAD,
   } type;
 
   /**
@@ -121,6 +126,24 @@ struct Transaction
 
     } credit;
 
+    /**
+     * Used if @e type is T_WAD.
+     */
+    struct
+    {
+
+      /**
+       * Subject of the transfer.
+       */
+      struct TALER_WadIdentifierP wad;
+
+      /**
+       * Base URL of the originating exchange.
+       */
+      char *origin_base_url;
+
+    } wad;
+
   } subject;
 
   /**
@@ -168,6 +191,13 @@ struct TALER_FAKEBANK_Handle
    */
   struct GNUNET_SCHEDULER_Task *mhd_task;
 
+  /**
+   * Hashmap of reserve public keys to
+   * `struct Transaction` with that reserve public
+   * key. Used to prevent public-key re-use.
+   */
+  struct GNUNET_CONTAINER_MultiPeerMap *rpubs;
+
   /**
    * Number of transactions.
    */
@@ -210,19 +240,43 @@ struct TALER_FAKEBANK_Handle
 static void
 check_log (struct TALER_FAKEBANK_Handle *h)
 {
-  for (struct Transaction *t = h->transactions_head; NULL != t; t = t->next)
+  for (struct Transaction *t = h->transactions_head;
+       NULL != t;
+       t = t->next)
   {
     if (GNUNET_YES == t->checked)
       continue;
-    GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+    switch (t->type)
-                "%s -> %s (%s) %s (%s)\n",
+    {
-                t->debit_account,
+    case T_DEBIT:
-                t->credit_account,
+      GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
-                TALER_amount2s (&t->amount),
+                  "%s -> %s (%s) %s (%s)\n",
-                (T_DEBIT == t->type)
+                  t->debit_account,
-                ? t->subject.debit.exchange_base_url
+                  t->credit_account,
-                : TALER_B2S (&t->subject.credit.reserve_pub),
+                  TALER_amount2s (&t->amount),
-                (T_DEBIT == t->type) ? "DEBIT" : "CREDIT");
+                  t->subject.debit.exchange_base_url,
+                  "DEBIT");
+      break;
+    case T_CREDIT:
+      GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+                  "%s -> %s (%s) %s (%s)\n",
+                  t->debit_account,
+                  t->credit_account,
+                  TALER_amount2s (&t->amount),
+                  TALER_B2S (&t->subject.credit.reserve_pub),
+                  "CREDIT");
+      break;
+    case T_WAD:
+      GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+                  "%s -> %s (%s) %s[%s] (%s)\n",
+                  t->debit_account,
+                  t->credit_account,
+                  TALER_amount2s (&t->amount),
+                  t->subject.wad.origin_base_url,
+                  TALER_B2S (&t->subject.wad),
+                  "WAD");
+      break;
+    }
   }
 }
 
@@ -252,7 +306,9 @@ TALER_FAKEBANK_check_debit (struct TALER_FAKEBANK_Handle *h,
 {
   GNUNET_assert (0 == strcasecmp (want_amount->currency,
                                   h->currency));
-  for (struct Transaction *t = h->transactions_head; NULL != t; t = t->next)
+  for (struct Transaction *t = h->transactions_head;
+       NULL != t;
+       t = t->next)
   {
     if ( (0 == strcasecmp (want_debit,
                            t->debit_account)) &&
@@ -304,7 +360,9 @@ TALER_FAKEBANK_check_credit (struct TALER_FAKEBANK_Handle *h,
 {
   GNUNET_assert (0 == strcasecmp (want_amount->currency,
                                   h->currency));
-  for (struct Transaction *t = h->transactions_head; NULL != t; t = t->next)
+  for (struct Transaction *t = h->transactions_head;
+       NULL != t;
+       t = t->next)
   {
     if ( (0 == strcasecmp (want_debit,
                            t->debit_account)) &&
@@ -371,9 +429,12 @@ TALER_FAKEBANK_make_transfer (
                                   strlen ("payto://")));
   if (NULL != request_uid)
   {
-    for (struct Transaction *t = h->transactions_head; NULL != t; t = t->next)
+    for (struct Transaction *t = h->transactions_head;
+         NULL != t;
+         t = t->next)
     {
-      if (0 != GNUNET_memcmp (request_uid, &t->request_uid))
+      if (0 != GNUNET_memcmp (request_uid,
+                              &t->request_uid))
         continue;
       if ( (0 != strcasecmp (debit_account,
                              t->debit_account)) ||
@@ -442,7 +503,18 @@ TALER_FAKEBANK_make_admin_transfer (
   const struct TALER_ReservePublicKeyP *reserve_pub)
 {
   struct Transaction *t;
+  const struct GNUNET_PeerIdentity *pid;
 
+  GNUNET_assert (sizeof (*pid) ==
+                 sizeof (*reserve_pub));
+  pid = (const struct GNUNET_PeerIdentity *) reserve_pub;
+  t = GNUNET_CONTAINER_multipeermap_get (h->rpubs,
+                                         pid);
+  if (NULL != t)
+  {
+    GNUNET_break (0);
+    return 0;
+  }
   GNUNET_assert (0 == strcasecmp (amount->currency,
                                   h->currency));
   GNUNET_assert (NULL != debit_account);
@@ -465,6 +537,12 @@ TALER_FAKEBANK_make_admin_transfer (
   GNUNET_CONTAINER_DLL_insert_tail (h->transactions_head,
                                     h->transactions_tail,
                                     t);
+  GNUNET_assert (GNUNET_OK ==
+                 GNUNET_CONTAINER_multipeermap_put (
+                   h->rpubs,
+                   pid,
+                   t,
+                   GNUNET_CONTAINER_MULTIHASHMAPOPTION_UNIQUE_ONLY));
   GNUNET_log (GNUNET_ERROR_TYPE_INFO,
               "Making transfer from %s to %s over %s and subject %s at row %llu\n",
               debit_account,
@@ -523,8 +601,18 @@ TALER_FAKEBANK_stop (struct TALER_FAKEBANK_Handle *h)
                                  t);
     GNUNET_free (t->debit_account);
     GNUNET_free (t->credit_account);
-    if (T_DEBIT == t->type)
+    switch (t->type)
+    {
+    case T_CREDIT:
+      /* nothing to free */
+      break;
+    case T_DEBIT:
       GNUNET_free (t->subject.debit.exchange_base_url);
+      break;
+    case T_WAD:
+      GNUNET_free (t->subject.wad.origin_base_url);
+      break;
+    }
     GNUNET_free (t);
   }
   if (NULL != h->mhd_task)
@@ -541,6 +629,7 @@ TALER_FAKEBANK_stop (struct TALER_FAKEBANK_Handle *h)
     h->mhd_bank = NULL;
   }
   GNUNET_free (h->my_baseurl);
+  GNUNET_CONTAINER_multipeermap_destroy (h->rpubs);
   GNUNET_free (h->currency);
   GNUNET_free (h);
 }
@@ -626,9 +715,12 @@ handle_admin_add_incoming (struct TALER_FAKEBANK_Handle *h,
     struct TALER_ReservePublicKeyP reserve_pub;
     char *debit;
     struct GNUNET_JSON_Specification spec[] = {
-      GNUNET_JSON_spec_fixed_auto ("reserve_pub", &reserve_pub),
+      GNUNET_JSON_spec_fixed_auto ("reserve_pub",
-      GNUNET_JSON_spec_string ("debit_account", &debit_account),
+                                   &reserve_pub),
-      TALER_JSON_spec_amount ("amount", &amount),
+      GNUNET_JSON_spec_string ("debit_account",
+                               &debit_account),
+      TALER_JSON_spec_amount ("amount",
+                              &amount),
       GNUNET_JSON_spec_end ()
     };
 
@@ -642,6 +734,15 @@ handle_admin_add_incoming (struct TALER_FAKEBANK_Handle *h,
       /* We're fakebank, no need for nice error handling */
       return MHD_NO;
     }
+    if (0 != strcasecmp (amount.currency,
+                         h->currency))
+    {
+      return TALER_MHD_reply_with_error (
+        connection,
+        MHD_HTTP_CONFLICT,
+        TALER_EC_GENERIC_CURRENCY_MISMATCH,
+        NULL);
+    }
     debit = TALER_xtalerbank_account_from_payto (debit_account);
     GNUNET_log (GNUNET_ERROR_TYPE_INFO,
                 "Receiving incoming wire transfer: %s->%s, subject: %s, amount: %s\n",
@@ -655,6 +756,14 @@ handle_admin_add_incoming (struct TALER_FAKEBANK_Handle *h,
                                                  &amount,
                                                  &reserve_pub);
     GNUNET_free (debit);
+    if (0 == row_id)
+    {
+      return TALER_MHD_reply_with_error (
+        connection,
+        MHD_HTTP_CONFLICT,
+        TALER_EC_BANK_DUPLICATE_RESERVE_PUB_SUBJECT,
+        NULL);
+    }
   }
   json_decref (json);
 
@@ -821,11 +930,9 @@ handle_home_page (struct TALER_FAKEBANK_Handle *h,
            (strlen (HELLOMSG),
            HELLOMSG,
            MHD_RESPMEM_MUST_COPY);
-
   ret = MHD_queue_response (connection,
                             MHD_HTTP_OK,
                             resp);
-
   MHD_destroy_response (resp);
   return ret;
 }
@@ -1389,12 +1496,17 @@ schedule_httpd (struct TALER_FAKEBANK_Handle *h)
   FD_ZERO (&ws);
   FD_ZERO (&es);
   max = -1;
-  if (MHD_YES != MHD_get_fdset (h->mhd_bank, &rs, &ws, &es, &max))
+  if (MHD_YES != MHD_get_fdset (h->mhd_bank,
+                                &rs,
+                                &ws,
+                                &es,
+                                &max))
   {
     GNUNET_assert (0);
     return;
   }
-  haveto = MHD_get_timeout (h->mhd_bank, &timeout);
+  haveto = MHD_get_timeout (h->mhd_bank,
+                            &timeout);
   if (MHD_YES == haveto)
     tv.rel_value_us = (uint64_t) timeout * 1000LL;
   else
@@ -1403,8 +1515,12 @@ schedule_httpd (struct TALER_FAKEBANK_Handle *h)
   {
     wrs = GNUNET_NETWORK_fdset_create ();
     wws = GNUNET_NETWORK_fdset_create ();
-    GNUNET_NETWORK_fdset_copy_native (wrs, &rs, max + 1);
+    GNUNET_NETWORK_fdset_copy_native (wrs,
-    GNUNET_NETWORK_fdset_copy_native (wws, &ws, max + 1);
+                                      &rs,
+                                      max + 1);
+    GNUNET_NETWORK_fdset_copy_native (wws,
+                                      &ws,
+                                      max + 1);
   }
   else
   {
@@ -1418,7 +1534,8 @@ schedule_httpd (struct TALER_FAKEBANK_Handle *h)
                                  tv,
                                  wrs,
                                  wws,
-                                 &run_mhd, h);
+                                 &run_mhd,
+                                 h);
   if (NULL != wrs)
     GNUNET_NETWORK_fdset_destroy (wrs);
   if (NULL != wws)
@@ -1468,6 +1585,8 @@ TALER_FAKEBANK_start (uint16_t port,
   GNUNET_assert (strlen (currency) < TALER_CURRENCY_LEN);
   h = GNUNET_new (struct TALER_FAKEBANK_Handle);
   h->port = port;
+  h->rpubs = GNUNET_CONTAINER_multipeermap_create (128,
+                                                   GNUNET_NO);
   h->currency = GNUNET_strdup (currency);
   GNUNET_asprintf (&h->my_baseurl,
                    "http://localhost:%u/",

src/include/taler_crypto_lib.h

@@ -591,6 +591,19 @@ struct TALER_WireTransferIdentifierRawP
 };
 
 
+/**
+ * Raw value of a wire transfer subject for a wad.
+ */
+struct TALER_WadIdentifierP
+{
+
+  /**
+   * Wad identifier, in binary encoding.
+   */
+  uint8_t raw[24];
+};
+
+
 /**
  * Binary information encoded in Crockford's Base32 in wire transfer
  * subjects of transfers from Taler to a merchant.  The actual value

src/include/taler_fakebank_lib.h

@@ -99,7 +99,7 @@ TALER_FAKEBANK_make_transfer (
  * @param credit_account account to credit
  * @param amount amount to transfer
  * @param reserve_pub reserve public key to use in subject
- * @return serial_id of the transfer
+ * @return serial_id of the transfer, 0 on error
  */
 uint64_t
 TALER_FAKEBANK_make_admin_transfer (

src/testing/test_bank_api.c

@@ -87,6 +87,11 @@ run (void *cls,
                                             "KUDOS:5.01",
                                             &bc.exchange_auth,
                                             bc.user42_payto),
+      TALER_TESTING_cmd_admin_add_incoming_with_ref ("credit-1-fail",
+                                                     "KUDOS:2.01",
+                                                     &bc.exchange_auth,
+                                                     bc.user42_payto,
+                                                     "credit-1"),
       TALER_TESTING_cmd_sleep ("Waiting 4s for 'credit-1' to settle",
                                4),
       TALER_TESTING_cmd_bank_credits ("history-1c",

src/testing/test_exchange_api.c

@@ -121,9 +121,9 @@ run (void *cls,
      * Move money to the exchange's bank account.
      */
     CMD_TRANSFER_TO_EXCHANGE ("create-reserve-1",
-                              "EUR:4.01"),
+                              "EUR:6.02"),
     TALER_TESTING_cmd_check_bank_admin_transfer ("check-create-reserve-1",
-                                                 "EUR:4.01",
+                                                 "EUR:6.02",
                                                  bc.user42_payto,
                                                  bc.exchange_payto,
                                                  "create-reserve-1"),
@@ -132,20 +132,6 @@ run (void *cls,
      * transfer.
      */
     CMD_EXEC_WIREWATCH ("wirewatch-1"),
-    /**
-     * Do another transfer to the same reserve
-     */
-    TALER_TESTING_cmd_admin_add_incoming_with_ref ("create-reserve-1.2",
-                                                   "EUR:2.01",
-                                                   &bc.exchange_auth,
-                                                   bc.user42_payto,
-                                                   "create-reserve-1"),
-    TALER_TESTING_cmd_check_bank_admin_transfer ("check-create-reserve-1.2",
-                                                 "EUR:2.01",
-                                                 bc.user42_payto,
-                                                 bc.exchange_payto,
-                                                 "create-reserve-1.2"),
-    CMD_EXEC_WIREWATCH ("wirewatch-1.2"),
     /**
      * Withdraw EUR:5.
      */

src/testing/testing_api_cmd_bank_admin_add_incoming.c

@@ -148,6 +148,11 @@ struct AdminAddIncomingState
    * enable retries? If so, how often should we still retry?
    */
   unsigned int do_retry;
+
+  /**
+   * Expected HTTP status code.
+   */
+  unsigned int expected_http_status;
 };
 
 
@@ -215,6 +220,13 @@ confirmation_cb (void *cls,
   switch (http_status)
   {
   case MHD_HTTP_OK:
+    if (fts->expected_http_status !=
+        MHD_HTTP_OK)
+    {
+      GNUNET_break (0);
+      TALER_TESTING_interpreter_fail (is);
+      return;
+    }
     fts->serial_id = serial_id;
     fts->timestamp = timestamp;
     TALER_TESTING_interpreter_next (is);
@@ -233,6 +245,16 @@ confirmation_cb (void *cls,
       break;
     }
     break;
+  case MHD_HTTP_CONFLICT:
+    if (fts->expected_http_status !=
+        MHD_HTTP_CONFLICT)
+    {
+      GNUNET_break (0);
+      TALER_TESTING_interpreter_fail (is);
+      return;
+    }
+    TALER_TESTING_interpreter_next (is);
+    return;
   default:
     if (0 != fts->do_retry)
     {
@@ -405,6 +427,10 @@ admin_add_incoming_traits (void *cls,
                            unsigned int index)
 {
   struct AdminAddIncomingState *fts = cls;
+
+  if (MHD_HTTP_OK !=
+      fts->expected_http_status)
+    return GNUNET_NO; /* requests that failed generate no history */
   if (fts->reserve_priv_known)
   {
     struct TALER_TESTING_Trait traits[] = {
@@ -479,6 +505,7 @@ make_fts (const char *amount,
   fts->exchange_credit_url = auth->wire_gateway_url;
   fts->payto_debit_account = payto_debit_account;
   fts->auth = *auth;
+  fts->expected_http_status = MHD_HTTP_OK;
   if (GNUNET_OK !=
       TALER_string_to_amount (amount,
                               &fts->amount))
@@ -515,15 +542,6 @@ make_command (const char *label,
 }
 
 
-/**
- * Create admin/add-incoming command.
- *
- * @param label command label.
- * @param amount amount to transfer.
- * @param payto_debit_account which account sends money.
- * @param auth authentication data
- * @return the command.
- */
 struct TALER_TESTING_Command
 TALER_TESTING_cmd_admin_add_incoming (const char *label,
                                       const char *amount,
@@ -538,20 +556,6 @@ TALER_TESTING_cmd_admin_add_incoming (const char *label,
 }
 
 
-/**
- * Create "/admin/add-incoming" CMD, letting the caller specify
- * a reference to a command that can offer a reserve private key.
- * This private key will then be used to construct the subject line
- * of the wire transfer.
- *
- * @param label command label.
- * @param amount the amount to transfer.
- * @param payto_debit_account which account sends money
- * @param auth authentication data
- * @param ref reference to a command that can offer a reserve
- *        private key or public key.
- * @return the command.
- */
 struct TALER_TESTING_Command
 TALER_TESTING_cmd_admin_add_incoming_with_ref
   (const char *label,
@@ -566,6 +570,7 @@ TALER_TESTING_cmd_admin_add_incoming_with_ref
                   auth,
                   payto_debit_account);
   fts->reserve_reference = ref;
+  fts->expected_http_status = MHD_HTTP_CONFLICT;
   return make_command (label,
                        fts);
 }