major clean up of /keys logic, validate offline signatures before /management/keys transaction logic starts

3 files changed, 188 insertions, 231 deletions

diff --git a/src/exchange/taler-exchange-httpd_keys.c b/src/exchange/taler-exchange-httpd_keys.c
index fd6b18b3b..9448ce36f 100644
--- a/src/exchange/taler-exchange-httpd_keys.c
+++ b/src/exchange/taler-exchange-httpd_keys.c
@@ -432,10 +432,6 @@ struct WireFeeSet
  */
 struct WireStateHandle
 {
-  /**
-   * Cached reply for /wire response.
-   */
-  struct MHD_Response *wire_reply;
 
   /**
    * JSON reply for /wire response.
@@ -474,9 +470,9 @@ struct WireStateHandle
   uint64_t wire_generation;
 
   /**
-   * HTTP status to return with this response.
+   * Is the wire data ready?
    */
-  unsigned int http_status;
+  bool ready;
 
 };
 
@@ -594,7 +590,6 @@ destroy_wire_state (struct WireStateHandle *wsh)
     GNUNET_free (wfs->method);
     GNUNET_free (wfs);
   }
-  MHD_destroy_response (wsh->wire_reply);
   json_decref (wsh->json_reply);
   GNUNET_free (wsh->etag);
   GNUNET_free (wsh);
@@ -849,10 +844,7 @@ build_wire_state (void)
   {
     GNUNET_break (0);
     json_decref (wire_accounts_array);
-    wsh->http_status = MHD_HTTP_INTERNAL_SERVER_ERROR;
-    wsh->wire_reply
-      = TALER_MHD_make_error (TALER_EC_GENERIC_DB_FETCH_FAILED,
-                              "get_wire_accounts");
+    wsh->ready = false;
     return wsh;
   }
   GNUNET_log (GNUNET_ERROR_TYPE_INFO,
@@ -878,11 +870,7 @@ build_wire_state (void)
         GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
                     "No wire method in `%s'\n",
                     payto_uri);
-        wsh->http_status = MHD_HTTP_INTERNAL_SERVER_ERROR;
-        wsh->wire_reply
-          = TALER_MHD_make_error (
-              TALER_EC_EXCHANGE_WIRE_INVALID_PAYTO_CONFIGURED,
-              payto_uri);
+        wsh->ready = false;
         json_decref (wire_accounts_array);
         json_decref (wire_fee_object);
         GNUNET_CRYPTO_hash_context_abort (hc);
@@ -910,10 +898,7 @@ build_wire_state (void)
           json_decref (wire_fee_object);
           json_decref (wire_accounts_array);
           GNUNET_free (wire_method);
-          wsh->http_status = MHD_HTTP_INTERNAL_SERVER_ERROR;
-          wsh->wire_reply
-            = TALER_MHD_make_error (TALER_EC_GENERIC_DB_FETCH_FAILED,
-                                    "get_wire_fees");
+          wsh->ready = false;
           GNUNET_CRYPTO_hash_context_abort (hc);
           return wsh;
         }
@@ -945,59 +930,7 @@ build_wire_state (void)
                                    wads),
     GNUNET_JSON_pack_object_incref ("fees",
                                     wire_fee_object));
-  wsh->wire_reply = TALER_MHD_MAKE_JSON_PACK (
-    GNUNET_JSON_pack_array_steal ("accounts",
-                                  wire_accounts_array),
-    GNUNET_JSON_pack_array_steal ("wads",
-                                  wads),
-    GNUNET_JSON_pack_object_steal ("fees",
-                                   wire_fee_object),
-    GNUNET_JSON_pack_data_auto ("master_public_key",
-                                &TEH_master_public_key));
-  {
-    struct GNUNET_TIME_Timestamp m;
-
-    m = GNUNET_TIME_absolute_to_timestamp (wsh->cache_expiration);
-    TALER_MHD_get_date_string (m.abs_time,
-                               wsh->dat);
-    GNUNET_log (GNUNET_ERROR_TYPE_INFO,
-                "Setting 'Expires' header for '/wire' to '%s'\n",
-                wsh->dat);
-    GNUNET_break (MHD_YES ==
-                  MHD_add_response_header (wsh->wire_reply,
-                                           MHD_HTTP_HEADER_EXPIRES,
-                                           wsh->dat));
-  }
-  /* Set cache control headers: our response varies depending on these headers */
-  GNUNET_break (MHD_YES ==
-                MHD_add_response_header (wsh->wire_reply,
-                                         MHD_HTTP_HEADER_VARY,
-                                         MHD_HTTP_HEADER_ACCEPT_ENCODING));
-  /* Information is always public, revalidate after 1 day */
-  GNUNET_break (MHD_YES ==
-                MHD_add_response_header (wsh->wire_reply,
-                                         MHD_HTTP_HEADER_CACHE_CONTROL,
-                                         "public,max-age=86400"));
-
-  {
-    struct GNUNET_HashCode h;
-    char etag[sizeof (h) * 2];
-    char *end;
-
-    GNUNET_CRYPTO_hash_context_finish (hc,
-                                       &h);
-    end = GNUNET_STRINGS_data_to_string (&h,
-                                         sizeof (h),
-                                         etag,
-                                         sizeof (etag));
-    *end = '\0';
-    wsh->etag = GNUNET_strdup (etag);
-    GNUNET_break (MHD_YES ==
-                  MHD_add_response_header (wsh->wire_reply,
-                                           MHD_HTTP_HEADER_ETAG,
-                                           etag));
-  }
-  wsh->http_status = MHD_HTTP_OK;
+  wsh->ready = true;
   return wsh;
 }
 
@@ -2437,7 +2370,7 @@ create_krd (struct TEH_KeyStateHandle *ksh,
   json_t *keys;
 
   wsh = get_wire_state ();
-  if (MHD_HTTP_OK != wsh->http_status)
+  if (! wsh->ready)
   {
     GNUNET_break (0);
     return GNUNET_SYSERR;
@@ -2686,12 +2619,12 @@ create_krd (struct TEH_KeyStateHandle *ksh,
                                     krd.response_compressed);
     /* Set cache control headers: our response varies depending on these headers */
     GNUNET_break (MHD_YES ==
-                  MHD_add_response_header (wsh->wire_reply,
+                  MHD_add_response_header (krd.response_compressed,
                                            MHD_HTTP_HEADER_VARY,
                                            MHD_HTTP_HEADER_ACCEPT_ENCODING));
     /* Information is always public, revalidate after 1 day */
     GNUNET_break (MHD_YES ==
-                  MHD_add_response_header (wsh->wire_reply,
+                  MHD_add_response_header (krd.response_compressed,
                                            MHD_HTTP_HEADER_CACHE_CONTROL,
                                            "public,max-age=86400"));
     GNUNET_break (MHD_YES ==
@@ -2910,22 +2843,48 @@ finish_keys_response (struct TEH_KeyStateHandle *ksh)
 {
   enum GNUNET_GenericReturnValue ret = GNUNET_SYSERR;
   json_t *recoup;
-  struct SignKeyCtx sctx;
+  struct SignKeyCtx sctx = {
+    .min_sk_frequency = GNUNET_TIME_UNIT_FOREVER_REL
+  };
   json_t *grouped_denominations = NULL;
   struct GNUNET_TIME_Timestamp last_cherry_pick_date;
   struct GNUNET_CONTAINER_Heap *heap;
   struct SignatureContext sig_ctx = { 0 };
   /* Remember if we have any denomination with age restriction */
   bool has_age_restricted_denomination = false;
+  struct WireStateHandle *wsh;
 
+  wsh = get_wire_state ();
+  if (! wsh->ready)
+  {
+    GNUNET_break (0);
+    return GNUNET_SYSERR;
+  }
+  if (0 ==
+      json_array_size (json_object_get (wsh->json_reply,
+                                        "accounts")) )
+  {
+    GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                "No wire accounts available. Refusing to generate /keys response.\n");
+    return GNUNET_NO;
+  }
   sctx.signkeys = json_array ();
   GNUNET_assert (NULL != sctx.signkeys);
-  sctx.min_sk_frequency = GNUNET_TIME_UNIT_FOREVER_REL;
+  recoup = json_array ();
+  GNUNET_assert (NULL != recoup);
+  grouped_denominations = json_array ();
+  GNUNET_assert (NULL != grouped_denominations);
+
   GNUNET_CONTAINER_multipeermap_iterate (ksh->signkey_map,
                                          &add_sign_key_cb,
                                          &sctx);
-  recoup = json_array ();
-  GNUNET_assert (NULL != recoup);
+  if (0 == json_array_size (sctx.signkeys))
+  {
+    GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                "No online signing keys available. Refusing to generate /keys response.\n");
+    ret = GNUNET_NO;
+    goto CLEANUP;
+  }
   heap = GNUNET_CONTAINER_heap_create (GNUNET_CONTAINER_HEAP_ORDER_MAX);
   {
     struct DenomKeyCtx dkc = {
@@ -2942,9 +2901,6 @@ finish_keys_response (struct TEH_KeyStateHandle *ksh)
                                   sctx.min_sk_frequency);
   }
 
-  grouped_denominations = json_array ();
-  GNUNET_assert (NULL != grouped_denominations);
-
   last_cherry_pick_date = GNUNET_TIME_UNIT_ZERO_TS;
 
   {
@@ -3147,8 +3103,8 @@ finish_keys_response (struct TEH_KeyStateHandle *ksh)
                                            NULL);
     GNUNET_CONTAINER_multihashmap_destroy (denominations_by_group);
   }
-
   GNUNET_CONTAINER_heap_destroy (heap);
+
   if (! GNUNET_TIME_absolute_is_zero (last_cherry_pick_date.abs_time))
   {
     struct GNUNET_HashCode hc;
@@ -3197,7 +3153,8 @@ CLEANUP:
                      sig_ctx.elements_size,
                      0);
   json_decref (grouped_denominations);
-  json_decref (sctx.signkeys);
+  if (NULL != sctx.signkeys)
+    json_decref (sctx.signkeys);
   json_decref (recoup);
   return ret;
 }
@@ -3395,7 +3352,7 @@ build_key_state (struct HelperState *hs,
       finish_keys_response (ksh))
   {
     GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                "Could not finish /keys response (likely no signing keys available yet)\n");
+                "Could not finish /keys response (required data not configured yet)\n");
     destroy_key_state (ksh,
                        true);
     return NULL;
diff --git a/src/exchange/taler-exchange-httpd_management_post_keys.c b/src/exchange/taler-exchange-httpd_management_post_keys.c
index 0ddc46916..0cf8ab727 100644
--- a/src/exchange/taler-exchange-httpd_management_post_keys.c
+++ b/src/exchange/taler-exchange-httpd_management_post_keys.c
@@ -1,6 +1,6 @@
 /*
   This file is part of TALER
-  Copyright (C) 2020, 2021 Taler Systems SA
+  Copyright (C) 2020-2023 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 published by the Free Software
@@ -47,6 +47,16 @@ struct DenomSig
    */
   struct TALER_MasterSignatureP master_sig;
 
+  /**
+   * Fee structure for this key, as per our configuration.
+   */
+  struct TALER_EXCHANGEDB_DenominationKeyMetaData meta;
+
+  /**
+   * The full public key.
+   */
+  struct TALER_DenominationPublicKey denom_pub;
+
 };
 
 
@@ -65,6 +75,11 @@ struct SigningSig
    */
   struct TALER_MasterSignatureP master_sig;
 
+  /**
+   * Our meta data on this key.
+   */
+  struct TALER_EXCHANGEDB_SignkeyMetaData meta;
+
 };
 
 
@@ -128,14 +143,9 @@ add_keys (void *cls,
   {
     struct DenomSig *d = &akc->d_sigs[i];
     enum GNUNET_DB_QueryStatus qs;
-    bool is_active = false;
     struct TALER_EXCHANGEDB_DenominationKeyMetaData meta;
-    struct TALER_DenominationPublicKey denom_pub;
 
     /* For idempotency, check if the key is already active */
-    memset (&denom_pub,
-            0,
-            sizeof (denom_pub));
     qs = TEH_plugin->lookup_denomination_key (
       TEH_plugin->cls,
       &d->h_denom_pub,
@@ -151,66 +161,9 @@ add_keys (void *cls,
                                              "lookup denomination key");
       return qs;
     }
-    if (0 == qs)
-    {
-      enum GNUNET_GenericReturnValue rv;
-
-      rv = TEH_keys_load_fees (akc->ksh,
-                               &d->h_denom_pub,
-                               &denom_pub,
-                               &meta);
-      switch (rv)
-      {
-      case GNUNET_SYSERR:
-        *mhd_ret = TALER_MHD_reply_with_error (
-          connection,
-          MHD_HTTP_INTERNAL_SERVER_ERROR,
-          TALER_EC_EXCHANGE_GENERIC_BAD_CONFIGURATION,
-          GNUNET_h2s (&d->h_denom_pub.hash));
-        return GNUNET_DB_STATUS_HARD_ERROR;
-      case GNUNET_NO:
-        *mhd_ret = TALER_MHD_reply_with_error (
-          connection,
-          MHD_HTTP_NOT_FOUND,
-          TALER_EC_EXCHANGE_GENERIC_DENOMINATION_KEY_UNKNOWN,
-          GNUNET_h2s (&d->h_denom_pub.hash));
-        return GNUNET_DB_STATUS_HARD_ERROR;
-      case GNUNET_OK:
-        break;
-      }
-    }
-    else
-    {
-      is_active = true;
-    }
-
-    /* check signature is valid */
-    TEH_METRICS_num_verifications[TEH_MT_SIGNATURE_EDDSA]++;
-    if (GNUNET_OK !=
-        TALER_exchange_offline_denom_validity_verify (
-          &d->h_denom_pub,
-          meta.start,
-          meta.expire_withdraw,
-          meta.expire_deposit,
-          meta.expire_legal,
-          &meta.value,
-          &meta.fees,
-          &TEH_master_public_key,
-          &d->master_sig))
-    {
-      GNUNET_break_op (0);
-      *mhd_ret = TALER_MHD_reply_with_error (
-        connection,
-        MHD_HTTP_FORBIDDEN,
-        TALER_EC_EXCHANGE_MANAGEMENT_KEYS_DENOMKEY_ADD_SIGNATURE_INVALID,
-        GNUNET_h2s (&d->h_denom_pub.hash));
-      if (! is_active)
-        TALER_denom_pub_free (&denom_pub);
-      return GNUNET_DB_STATUS_HARD_ERROR;
-    }
-
-    if (is_active)
+    if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT == qs)
     {
+      /* FIXME: assert meta === d->meta might be good */
       GNUNET_log (GNUNET_ERROR_TYPE_INFO,
                   "Denomination key %s already active, skipping\n",
                   GNUNET_h2s (&d->h_denom_pub.hash));
@@ -220,10 +173,9 @@ add_keys (void *cls,
     qs = TEH_plugin->add_denomination_key (
       TEH_plugin->cls,
       &d->h_denom_pub,
-      &denom_pub,
-      &meta,
+      &d->denom_pub,
+      &d->meta,
       &d->master_sig);
-    TALER_denom_pub_free (&denom_pub);
     if (qs < 0)
     {
       if (GNUNET_DB_STATUS_SOFT_ERROR == qs)
@@ -245,7 +197,6 @@ add_keys (void *cls,
   {
     struct SigningSig *s = &akc->s_sigs[i];
     enum GNUNET_DB_QueryStatus qs;
-    bool is_active = false;
     struct TALER_EXCHANGEDB_SignkeyMetaData meta;
 
     qs = TEH_plugin->lookup_signing_key (
@@ -263,47 +214,9 @@ add_keys (void *cls,
                                              "lookup signing key");
       return qs;
     }
-    if (0 == qs)
-    {
-      if (GNUNET_OK !=
-          TEH_keys_get_timing (&s->exchange_pub,
-                               &meta))
-      {
-        /* For idempotency, check if the key is already active */
-        *mhd_ret = TALER_MHD_reply_with_error (
-          connection,
-          MHD_HTTP_NOT_FOUND,
-          TALER_EC_EXCHANGE_MANAGEMENT_KEYS_SIGNKEY_UNKNOWN,
-          TALER_B2S (&s->exchange_pub));
-        return GNUNET_DB_STATUS_HARD_ERROR;
-      }
-    }
-    else
-    {
-      is_active = true; /* if we pass, it's active! */
-    }
-
-    /* check signature is valid */
-    TEH_METRICS_num_verifications[TEH_MT_SIGNATURE_EDDSA]++;
-    if (GNUNET_OK !=
-        TALER_exchange_offline_signkey_validity_verify (
-          &s->exchange_pub,
-          meta.start,
-          meta.expire_sign,
-          meta.expire_legal,
-          &TEH_master_public_key,
-          &s->master_sig))
-    {
-      GNUNET_break_op (0);
-      *mhd_ret = TALER_MHD_reply_with_error (
-        connection,
-        MHD_HTTP_FORBIDDEN,
-        TALER_EC_EXCHANGE_MANAGEMENT_KEYS_SIGNKEY_ADD_SIGNATURE_INVALID,
-        TALER_B2S (&s->exchange_pub));
-      return GNUNET_DB_STATUS_HARD_ERROR;
-    }
-    if (is_active)
+    if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT == qs)
     {
+      /* FIXME: assert meta === d->meta might be good */
       GNUNET_log (GNUNET_ERROR_TYPE_INFO,
                   "Signing key %s already active, skipping\n",
                   TALER_B2S (&s->exchange_pub));
@@ -312,7 +225,7 @@ add_keys (void *cls,
     qs = TEH_plugin->activate_signing_key (
       TEH_plugin->cls,
       &s->exchange_pub,
-      &meta,
+      &s->meta,
       &s->master_sig);
     if (qs < 0)
     {
@@ -334,6 +247,25 @@ add_keys (void *cls,
 }
 
 
+/**
+ * Clean up state in @a akc, but do not free @a akc itself
+ *
+ * @param[in,out] akc state to clean up
+ */
+static void
+cleanup_akc (struct AddKeysContext *akc)
+{
+  for (unsigned int i = 0; i<akc->nd_sigs; i++)
+  {
+    struct DenomSig *d = &akc->d_sigs[i];
+
+    TALER_denom_pub_free (&d->denom_pub);
+  }
+  GNUNET_free (akc->d_sigs);
+  GNUNET_free (akc->s_sigs);
+}
+
+
 MHD_RESULT
 TEH_handler_management_post_keys (
   struct MHD_Connection *connection,
@@ -349,7 +281,6 @@ TEH_handler_management_post_keys (
                                   &signkey_sigs),
     GNUNET_JSON_spec_end ()
   };
-  bool ok;
   MHD_RESULT ret;
 
   {
@@ -364,7 +295,8 @@ TEH_handler_management_post_keys (
       return MHD_YES; /* failure */
   }
   GNUNET_log (GNUNET_ERROR_TYPE_INFO,
-              "Received /management/keys\n");
+              "Received POST /management/keys request\n");
+
   akc.ksh = TEH_keys_get_state_for_management_only (); /* may start its own transaction, thus must be done here, before we run ours! */
   if (NULL == akc.ksh)
   {
@@ -375,10 +307,10 @@ TEH_handler_management_post_keys (
       TALER_EC_EXCHANGE_GENERIC_KEYS_MISSING,
       "no key state (not even for management)");
   }
+
   akc.nd_sigs = json_array_size (denom_sigs);
   akc.d_sigs = GNUNET_new_array (akc.nd_sigs,
                                  struct DenomSig);
-  ok = true;
   for (unsigned int i = 0; i<akc.nd_sigs; i++)
   {
     struct DenomSig *d = &akc.d_sigs[i];
@@ -395,26 +327,64 @@ TEH_handler_management_post_keys (
                                      json_array_get (denom_sigs,
                                                      i),
                                      ispec);
-    if (GNUNET_SYSERR == res)
+    if (GNUNET_OK != res)
     {
-      ret = MHD_NO; /* hard failure */
-      ok = false;
-      break;
+      GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+                  "Failure to handle /management/keys\n");
+      cleanup_akc (&akc);
+      return (GNUNET_NO == res) ? MHD_YES : MHD_NO;
     }
-    if (GNUNET_NO == res)
+
+    res = TEH_keys_load_fees (akc.ksh,
+                              &d->h_denom_pub,
+                              &d->denom_pub,
+                              &d->meta);
+    switch (res)
     {
-      ret = MHD_YES;
-      ok = false;
+    case GNUNET_SYSERR:
+      ret = TALER_MHD_reply_with_error (
+        connection,
+        MHD_HTTP_INTERNAL_SERVER_ERROR,
+        TALER_EC_EXCHANGE_GENERIC_BAD_CONFIGURATION,
+        GNUNET_h2s (&d->h_denom_pub.hash));
+      cleanup_akc (&akc);
+      return ret;
+    case GNUNET_NO:
+      ret = TALER_MHD_reply_with_error (
+        connection,
+        MHD_HTTP_NOT_FOUND,
+        TALER_EC_EXCHANGE_GENERIC_DENOMINATION_KEY_UNKNOWN,
+        GNUNET_h2s (&d->h_denom_pub.hash));
+      cleanup_akc (&akc);
+      return ret;
+    case GNUNET_OK:
       break;
     }
+    /* check signature is valid */
+    TEH_METRICS_num_verifications[TEH_MT_SIGNATURE_EDDSA]++;
+    if (GNUNET_OK !=
+        TALER_exchange_offline_denom_validity_verify (
+          &d->h_denom_pub,
+          d->meta.start,
+          d->meta.expire_withdraw,
+          d->meta.expire_deposit,
+          d->meta.expire_legal,
+          &d->meta.value,
+          &d->meta.fees,
+          &TEH_master_public_key,
+          &d->master_sig))
+    {
+      GNUNET_break_op (0);
+      ret = TALER_MHD_reply_with_error (
+        connection,
+        MHD_HTTP_FORBIDDEN,
+        TALER_EC_EXCHANGE_MANAGEMENT_KEYS_DENOMKEY_ADD_SIGNATURE_INVALID,
+        GNUNET_h2s (&d->h_denom_pub.hash));
+      cleanup_akc (&akc);
+      return ret;
+    }
   }
-  if (! ok)
-  {
-    GNUNET_free (akc.d_sigs);
-    GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
-                "Failure to handle /management/keys\n");
-    return ret;
-  }
+
   akc.ns_sigs = json_array_size (signkey_sigs);
   akc.s_sigs = GNUNET_new_array (akc.ns_sigs,
                                  struct SigningSig);
@@ -434,26 +404,58 @@ TEH_handler_management_post_keys (
                                      json_array_get (signkey_sigs,
                                                      i),
                                      ispec);
-    if (GNUNET_SYSERR == res)
+    if (GNUNET_OK != res)
     {
-      ret = MHD_NO; /* hard failure */
-      ok = false;
-      break;
+      GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+                  "Failure to handle /management/keys\n");
+      cleanup_akc (&akc);
+      return (GNUNET_NO == res) ? MHD_YES : MHD_NO;
     }
-    if (GNUNET_NO == res)
+    res = TEH_keys_get_timing (&s->exchange_pub,
+                               &s->meta);
+    switch (res)
     {
-      ret = MHD_YES;
-      ok = false;
+    case GNUNET_SYSERR:
+      ret = TALER_MHD_reply_with_error (
+        connection,
+        MHD_HTTP_INTERNAL_SERVER_ERROR,
+        TALER_EC_EXCHANGE_GENERIC_BAD_CONFIGURATION,
+        TALER_B2S (&s->exchange_pub));
+      cleanup_akc (&akc);
+      return ret;
+    case GNUNET_NO:
+      /* For idempotency, check if the key is already active */
+      ret = TALER_MHD_reply_with_error (
+        connection,
+        MHD_HTTP_NOT_FOUND,
+        TALER_EC_EXCHANGE_MANAGEMENT_KEYS_SIGNKEY_UNKNOWN,
+        TALER_B2S (&s->exchange_pub));
+      cleanup_akc (&akc);
+      return ret;
+    case GNUNET_OK:
       break;
     }
-  }
-  if (! ok)
-  {
-    GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
-                "Failure to handle /management/keys\n");
-    GNUNET_free (akc.d_sigs);
-    GNUNET_free (akc.s_sigs);
-    return ret;
+
+    /* check signature is valid */
+    TEH_METRICS_num_verifications[TEH_MT_SIGNATURE_EDDSA]++;
+    if (GNUNET_OK !=
+        TALER_exchange_offline_signkey_validity_verify (
+          &s->exchange_pub,
+          s->meta.start,
+          s->meta.expire_sign,
+          s->meta.expire_legal,
+          &TEH_master_public_key,
+          &s->master_sig))
+    {
+      GNUNET_break_op (0);
+      ret = TALER_MHD_reply_with_error (
+        connection,
+        MHD_HTTP_FORBIDDEN,
+        TALER_EC_EXCHANGE_MANAGEMENT_KEYS_SIGNKEY_ADD_SIGNATURE_INVALID,
+        TALER_B2S (&s->exchange_pub));
+      cleanup_akc (&akc);
+      return ret;
+    }
   }
   GNUNET_log (GNUNET_ERROR_TYPE_INFO,
               "Received %u denomination and %u signing key signatures\n",
@@ -468,8 +470,7 @@ TEH_handler_management_post_keys (
                                   &ret,
                                   &add_keys,
                                   &akc);
-    GNUNET_free (akc.d_sigs);
-    GNUNET_free (akc.s_sigs);
+    cleanup_akc (&akc);
     if (GNUNET_SYSERR == res)
       return ret;
   }
diff --git a/src/exchangedb/pg_lookup_denomination_key.c b/src/exchangedb/pg_lookup_denomination_key.c
index 20eb703dd..a358528ad 100644
--- a/src/exchangedb/pg_lookup_denomination_key.c
+++ b/src/exchangedb/pg_lookup_denomination_key.c
@@ -60,7 +60,6 @@ TEH_PG_lookup_denomination_key (
     GNUNET_PQ_result_spec_end
   };
 
-  /* used in #postgres_lookup_denomination_key() */
   PREPARE (pg,
            "lookup_denomination_key",
            "SELECT"