POST /private/instances logic cleanup

2 files changed, 132 insertions, 110 deletions

diff --git a/src/backend/taler-merchant-httpd.c b/src/backend/taler-merchant-httpd.c
index be52ba27..bfc0337e 100644
--- a/src/backend/taler-merchant-httpd.c
+++ b/src/backend/taler-merchant-httpd.c
@@ -334,6 +334,8 @@ compute_pay_key (const char *order_id,
   size_t olen = strlen (order_id);
   char buf[sizeof (*mpub) + olen];
 
+  /* sanity check for arithmetic overflow */
+  GNUNET_assert (olen < 1024 * 1024);
   memcpy (buf,
           mpub,
           sizeof (*mpub));
@@ -344,7 +346,7 @@ compute_pay_key (const char *order_id,
                       sizeof (buf),
                       key);
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "Pay key for %s is %s\n",
+              "Pay key for `%s' is %s\n",
               order_id,
               GNUNET_h2s (key));
 }
@@ -951,7 +953,10 @@ TMH_add_instance (struct TMH_MerchantInstance *mi)
                                            mi,
                                            GNUNET_CONTAINER_MULTIHASHMAPOPTION_UNIQUE_ONLY);
   if (GNUNET_OK == ret)
+  {
+    GNUNET_assert (mi->rc < UINT_MAX);
     mi->rc++;
+  }
   return ret;
 }
 
@@ -1693,7 +1698,10 @@ url_handler (void *cls,
       }
     }
     if (NULL != hc->instance)
+    {
+      GNUNET_assert (hc->instance->rc < UINT_MAX);
       hc->instance->rc++;
+    }
   }
 
   {
diff --git a/src/backend/taler-merchant-httpd_private-post-instances.c b/src/backend/taler-merchant-httpd_private-post-instances.c
index f47ae7ed..7550beb4 100644
--- a/src/backend/taler-merchant-httpd_private-post-instances.c
+++ b/src/backend/taler-merchant-httpd_private-post-instances.c
@@ -1,6 +1,6 @@
 /*
   This file is part of TALER
-  (C) 2020 Taler Systems SA
+  (C) 2020, 2021 Taler Systems SA
 
   TALER is free software; you can redistribute it and/or modify
   it under the terms of the GNU Affero General Public License as
@@ -67,8 +67,8 @@ accounts_equal (const struct TMH_MerchantInstance *mi,
       {
         const char *str = json_string_value (json_array_get (payto_uris,
                                                              i));
-        if (NULL == str)
-          return false;
+
+        GNUNET_assert (NULL != str);
         if (0 == strcasecmp (uri,
                              str))
         {
@@ -150,24 +150,24 @@ TMH_private_post_instances (const struct TMH_RequestHandler *rh,
   struct TMH_WireMethod *wm_tail = NULL;
   json_t *jauth;
   struct GNUNET_JSON_Specification spec[] = {
-    GNUNET_JSON_spec_json ("auth",
-                           &jauth),
     GNUNET_JSON_spec_json ("payto_uris",
                            &payto_uris),
     GNUNET_JSON_spec_string ("id",
                              (const char **) &is.id),
     GNUNET_JSON_spec_string ("name",
                              (const char **) &is.name),
+    GNUNET_JSON_spec_json ("auth",
+                           &jauth),
     GNUNET_JSON_spec_json ("address",
                            &is.address),
     GNUNET_JSON_spec_json ("jurisdiction",
                            &is.jurisdiction),
-    TALER_JSON_spec_amount ("default_max_deposit_fee",
-                            &is.default_max_deposit_fee),
     TALER_JSON_spec_amount ("default_max_wire_fee",
                             &is.default_max_wire_fee),
     GNUNET_JSON_spec_uint32 ("default_wire_fee_amortization",
                              &is.default_wire_fee_amortization),
+    TALER_JSON_spec_amount ("default_max_deposit_fee",
+                            &is.default_max_deposit_fee),
     TALER_JSON_spec_relative_time ("default_wire_transfer_delay",
                                    &is.default_wire_transfer_delay),
     TALER_JSON_spec_relative_time ("default_pay_delay",
@@ -187,36 +187,42 @@ TMH_private_post_instances (const struct TMH_RequestHandler *rh,
              : MHD_NO;
   }
 
-
   {
-    bool auth_ok = false;
+    bool auth_wellformed = false;
     const char *auth_method = json_string_value (json_object_get (jauth,
                                                                   "method"));
 
     if (NULL == auth_method)
+    {
       GNUNET_break_op (0);
-    else if (0 == strcmp (auth_method, "external"))
+    }
+    else if (0 == strcmp (auth_method,
+                          "external"))
     {
       auth_token = NULL;
-      auth_ok = true;
+      auth_wellformed = true;
     }
-    else if (0 == strcmp (auth_method, "token"))
+    else if (0 == strcmp (auth_method,
+                          "token"))
     {
-      auth_token = json_string_value (json_object_get (jauth, "token"));
-      if (NULL != auth_token)
+      auth_token = json_string_value (json_object_get (jauth,
+                                                       "token"));
+      if (NULL == auth_token)
+      {
+        GNUNET_break_op (0);
+      }
+      else
       {
         if (0 != strncasecmp (RFC_8959_PREFIX,
                               auth_token,
                               strlen (RFC_8959_PREFIX)))
           GNUNET_break_op (0);
         else
-          auth_ok = true;
+          auth_wellformed = true;
       }
-      else
-        GNUNET_break_op (0);
     }
 
-    if (! auth_ok)
+    if (! auth_wellformed)
     {
       GNUNET_break_op (0);
       GNUNET_JSON_parse_free (spec);
@@ -227,11 +233,77 @@ TMH_private_post_instances (const struct TMH_RequestHandler *rh,
     }
   }
 
+  /* check payto_uris for well-formedness */
+  {
+    bool payto_ok = true;
+
+    if (! json_is_array (payto_uris))
+    {
+      GNUNET_break_op (0);
+      payto_ok = false;
+    }
+    else
+    {
+      unsigned int len = json_array_size (payto_uris);
+
+      for (unsigned int i = 0; i<len; i++)
+      {
+        json_t *payto_uri = json_array_get (payto_uris,
+                                            i);
+        const char *uri;
+
+        if (! json_is_string (payto_uri))
+          payto_ok = false;
+        uri = json_string_value (payto_uri);
+        /* Test for the same payto:// URI being given twice */
+        for (unsigned int j = 0; j<i; j++)
+        {
+          json_t *old_uri = json_array_get (payto_uris,
+                                            j);
+          if (json_equal (payto_uri,
+                          old_uri))
+          {
+            GNUNET_break_op (0);
+            payto_ok = false;
+            break;
+          }
+        }
+        if (! payto_ok)
+          break;
+        if (GNUNET_SYSERR ==
+            TALER_JSON_validate_payto (uri))
+        {
+          payto_ok = false;
+          break;
+        }
+      }
+    }
+    if (! payto_ok)
+      return TALER_MHD_reply_with_error (connection,
+                                         MHD_HTTP_BAD_REQUEST,
+                                         TALER_EC_GENERIC_PAYTO_URI_MALFORMED,
+                                         NULL);
+  }
 
-  if ((0 != strcasecmp (is.default_max_deposit_fee.currency,
-                        TMH_currency)) ||
-      (0 != strcasecmp (is.default_max_wire_fee.currency,
-                        TMH_currency)))
+  /* check 'id' well-formed */
+  {
+    bool id_wellformed = true;
+
+    if (NULL != strchr (is.id, '/'))
+      id_wellformed = false;
+    // FIXME: implement regex [A-Za-z0-9_.@-]
+    // FIXME: document charset in docs, add restriction to SPA
+    if (! id_wellformed)
+      return TALER_MHD_reply_with_error (connection,
+                                         MHD_HTTP_BAD_REQUEST,
+                                         TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                                         "id");
+  }
+
+  if ( (0 != strcasecmp (is.default_max_deposit_fee.currency,
+                         TMH_currency)) ||
+       (0 != strcasecmp (is.default_max_wire_fee.currency,
+                         TMH_currency)) )
   {
     GNUNET_break_op (0);
     GNUNET_JSON_parse_free (spec);
@@ -310,103 +382,40 @@ TMH_private_post_instances (const struct TMH_RequestHandler *rh,
     }
   }
 
+  /* convert provided payto URIs into internal data structure with salts */
   {
-    bool payto_ok = true;
-    unsigned int len;
+    unsigned int len = json_array_size (payto_uris);
 
-    if (! json_is_array (payto_uris))
-    {
-      GNUNET_break_op (0);
-      payto_ok = false;
-      len = 0;
-    }
-    else
-    {
-      len = json_array_size (payto_uris);
-    }
     for (unsigned int i = 0; i<len; i++)
     {
       json_t *payto_uri = json_array_get (payto_uris,
                                           i);
-
-      if (! json_is_string (payto_uri))
-      {
-        GNUNET_break_op (0);
-        payto_ok = false;
-        break;
-      }
-      /* Test for the same payto:// URI being given twice */
-      for (unsigned int j = 0; j<i; j++)
-      {
-        json_t *old_uri = json_array_get (payto_uris,
-                                          j);
-        if (json_equal (payto_uri,
-                        old_uri))
-        {
-          GNUNET_break_op (0);
-          payto_ok = false;
-          break;
-        }
-      }
-      if (! payto_ok)
-        break;
-
-      {
-        struct TMH_WireMethod *wm;
-        struct GNUNET_HashCode salt;
-
-        GNUNET_CRYPTO_random_block (GNUNET_CRYPTO_QUALITY_NONCE,
-                                    &salt,
-                                    sizeof (salt));
-        wm = GNUNET_new (struct TMH_WireMethod);
-        wm->j_wire = json_pack ("{s:O, s:o}",
-                                "payto_uri", payto_uri,
-                                "salt", GNUNET_JSON_from_data_auto (&salt));
-        GNUNET_assert (NULL != wm->j_wire);
-        /* This also tests for things like the IBAN being malformed */
-        if (GNUNET_OK !=
-            TALER_JSON_merchant_wire_signature_hash (wm->j_wire,
-                                                     &wm->h_wire))
-        {
-          GNUNET_break_op (0);
-          payto_ok = false;
-          GNUNET_free (wm);
-          break;
-        }
-        wm->wire_method
-          = TALER_payto_get_method (json_string_value (payto_uri));
-        if (NULL == wm->wire_method)
-        {
-          GNUNET_break_op (0);
-          payto_ok = false;
-          GNUNET_free (wm);
-          break;
-        }
-        wm->active = true;
-        GNUNET_CONTAINER_DLL_insert (wm_head,
-                                     wm_tail,
-                                     wm);
-      }
-    }
-    if (! payto_ok)
-    {
       struct TMH_WireMethod *wm;
-
-      while (NULL != (wm = wm_head))
-      {
-        GNUNET_CONTAINER_DLL_remove (wm_head,
-                                     wm_tail,
-                                     wm);
-        free_wm (wm);
-      }
-      GNUNET_JSON_parse_free (spec);
-      return TALER_MHD_reply_with_error (connection,
-                                         MHD_HTTP_BAD_REQUEST,
-                                         TALER_EC_GENERIC_PAYTO_URI_MALFORMED,
-                                         NULL);
+      struct GNUNET_HashCode salt;
+
+      GNUNET_CRYPTO_random_block (GNUNET_CRYPTO_QUALITY_NONCE,
+                                  &salt,
+                                  sizeof (salt));
+      wm = GNUNET_new (struct TMH_WireMethod);
+      wm->j_wire = json_pack ("{s:O, s:o}",
+                              "payto_uri", payto_uri,
+                              "salt", GNUNET_JSON_from_data_auto (&salt));
+      GNUNET_assert (NULL != wm->j_wire);
+      /* This also tests for things like the IBAN being malformed */
+      GNUNET_assert (GNUNET_OK ==
+                     TALER_JSON_merchant_wire_signature_hash (wm->j_wire,
+                                                              &wm->h_wire));
+      wm->wire_method
+        = TALER_payto_get_method (json_string_value (payto_uri));
+      GNUNET_assert (NULL != wm->wire_method); /* checked earlier */
+      wm->active = true;
+      GNUNET_CONTAINER_DLL_insert (wm_head,
+                                   wm_tail,
+                                   wm);
     }
   }
 
+  /* handle authentication token setup */
   if (NULL == auth_token)
   {
     memset (&ias.auth_salt,
@@ -418,10 +427,13 @@ TMH_private_post_instances (const struct TMH_RequestHandler *rh,
   }
   else
   {
+    /* Sets 'auth_salt' and 'auth_hash' */
     TMH_compute_auth (auth_token,
                       &ias.auth_salt,
                       &ias.auth_hash);
   }
+
+  /* create in-memory data structure */
   {
     struct TMH_MerchantInstance *mi;
     enum GNUNET_DB_QueryStatus qs;
@@ -501,6 +513,8 @@ TMH_private_post_instances (const struct TMH_RequestHandler *rh,
       {
         GNUNET_break (GNUNET_DB_STATUS_SOFT_ERROR == qs);
         TMH_db->rollback (TMH_db->cls);
+        if (GNUNET_DB_STATUS_HARD_ERROR == qs)
+          break;
         goto retry;
       }
       qs = TMH_db->commit (TMH_db->cls);