code cleanup (comments, scoping, naming, indentation)

1 files changed, 157 insertions, 97 deletions

diff --git a/src/exchangedb/exchangedb_denomkeys.c b/src/exchangedb/exchangedb_denomkeys.c
index 01b04198f..03daab300 100644
--- a/src/exchangedb/exchangedb_denomkeys.c
+++ b/src/exchangedb/exchangedb_denomkeys.c
@@ -58,39 +58,42 @@ GNUNET_NETWORK_STRUCT_END
  * @return #GNUNET_OK upon success; #GNUNET_SYSERR upon failure.
  */
 int
-TALER_EXCHANGEDB_denomination_key_revoke (const char *revocation_dir,
-                                          const struct
-                                          GNUNET_HashCode *denom_hash,
-                                          const struct
-                                          TALER_MasterPrivateKeyP *mpriv)
+TALER_EXCHANGEDB_denomination_key_revoke (
+  const char *revocation_dir,
+  const struct GNUNET_HashCode *denom_hash,
+  const struct TALER_MasterPrivateKeyP *mpriv)
 {
-  struct TALER_MasterDenominationKeyRevocationPS rm;
   char *fn;
   int ret;
   struct RevocationFileP rd;
 
+  {
+    struct TALER_MasterDenominationKeyRevocationPS rm = {
+      .purpose.purpose = htonl (
+        TALER_SIGNATURE_MASTER_DENOMINATION_KEY_REVOKED),
+      .purpose.size = htonl (sizeof (rm)),
+      .h_denom_pub = *denom_hash
+    };
+
+    GNUNET_assert (GNUNET_OK ==
+                   GNUNET_CRYPTO_eddsa_sign (&mpriv->eddsa_priv,
+                                             &rm.purpose,
+                                             &rd.msig.eddsa_signature));
+  }
   GNUNET_asprintf (&fn,
                    "%s" DIR_SEPARATOR_STR
                    "%s.rev",
                    revocation_dir,
                    GNUNET_h2s_full (denom_hash));
-  rm.purpose.purpose = htonl (TALER_SIGNATURE_MASTER_DENOMINATION_KEY_REVOKED);
-  rm.purpose.size = htonl (sizeof (rm));
-  rm.h_denom_pub = *denom_hash;
-  GNUNET_assert (GNUNET_OK ==
-                 GNUNET_CRYPTO_eddsa_sign (&mpriv->eddsa_priv,
-                                           &rm.purpose,
-                                           &rd.msig.eddsa_signature));
   rd.denom_hash = *denom_hash;
-  if (sizeof (rd) !=
-      GNUNET_DISK_fn_write (fn,
-                            &rd,
-                            sizeof (rd),
-                            GNUNET_DISK_PERM_USER_READ
-                            | GNUNET_DISK_PERM_USER_WRITE))
-    ret = GNUNET_SYSERR;
-  else
-    ret = GNUNET_OK;
+  ret = (sizeof (rd) !=
+         GNUNET_DISK_fn_write (fn,
+                               &rd,
+                               sizeof (rd),
+                               GNUNET_DISK_PERM_USER_READ
+                               | GNUNET_DISK_PERM_USER_WRITE))
+        ? GNUNET_SYSERR
+        : GNUNET_OK;
   GNUNET_free (fn);
   return ret;
 }
@@ -105,10 +108,9 @@ TALER_EXCHANGEDB_denomination_key_revoke (const char *revocation_dir,
  *         #GNUNET_SYSERR upon failure
  */
 int
-TALER_EXCHANGEDB_denomination_key_read (const char *filename,
-                                        struct
-                                        TALER_EXCHANGEDB_DenominationKey
-                                        *dki)
+TALER_EXCHANGEDB_denomination_key_read (
+  const char *filename,
+  struct TALER_EXCHANGEDB_DenominationKey *dki)
 {
   uint64_t size;
   size_t offset;
@@ -129,9 +131,20 @@ TALER_EXCHANGEDB_denomination_key_read (const char *filename,
   offset = sizeof (struct TALER_EXCHANGEDB_DenominationKeyInformationP);
   if (size <= offset)
   {
-    GNUNET_break (0);
+    GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                "File size (%llu bytes) too small for file `%s' to contain denomination key data. Skipping it.\n",
+                (unsigned long long) size,
+                filename);
     return GNUNET_SYSERR;
   }
+  if (size >= GNUNET_MAX_MALLOC_CHECKED)
+  {
+    GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                "File size (%llu bytes) too large for file `%s' to contain denomination key data. Skipping it.\n",
+                (unsigned long long) size,
+                filename);
+    return GNUNET_OK;
+  }
   data = GNUNET_malloc (size);
   if (((ssize_t) size) !=
       GNUNET_DISK_fn_read (filename,
@@ -160,16 +173,19 @@ TALER_EXCHANGEDB_denomination_key_read (const char *filename,
           offset);
   GNUNET_free (data);
   if (0 == GNUNET_TIME_absolute_get_remaining
-        (GNUNET_TIME_absolute_ntoh (
-          dki->issue.properties.expire_withdraw)).rel_value_us)
+        (GNUNET_TIME_absolute_ntoh
+          (dki->issue.properties.expire_withdraw)).rel_value_us)
   {
+    /* key expired for withdrawal, remove private key to
+       minimize chance of compromise */
     if (0 != unlink (filename))
     {
       GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR,
                                 "unlink",
                                 filename);
-      return GNUNET_OK; /* yes, we had an error, but the file content
-                           was fine and is being returned */
+      /* yes, we had an error, but the file content
+         was fine and is being returned */
+      return GNUNET_OK;
     }
   }
   return GNUNET_OK;
@@ -184,26 +200,25 @@ TALER_EXCHANGEDB_denomination_key_read (const char *filename,
  * @return #GNUNET_OK upon success; #GNUNET_SYSERR upon failure.
  */
 int
-TALER_EXCHANGEDB_denomination_key_write (const char *filename,
-                                         const struct
-                                         TALER_EXCHANGEDB_DenominationKey
-                                         *dki)
+TALER_EXCHANGEDB_denomination_key_write (
+  const char *filename,
+  const struct TALER_EXCHANGEDB_DenominationKey *dki)
 {
-  char *priv_enc;
-  size_t priv_enc_size;
   struct GNUNET_DISK_FileHandle *fh;
   ssize_t wrote;
   size_t wsize;
-  int ret;
+  int eno;
 
-  fh = NULL;
-  priv_enc_size
-    = GNUNET_CRYPTO_rsa_private_key_encode (dki->denom_priv.rsa_private_key,
-                                            &priv_enc);
-  ret = GNUNET_SYSERR;
   if (GNUNET_OK !=
       GNUNET_DISK_directory_create_for_file (filename))
+  {
+    eno = errno;
+    GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR,
+                              "mkdir (for file)",
+                              filename);
+    errno = eno;
     return GNUNET_SYSERR;
+  }
   if (NULL == (fh = GNUNET_DISK_file_open
                       (filename,
                       GNUNET_DISK_OPEN_WRITE | GNUNET_DISK_OPEN_CREATE
@@ -211,27 +226,53 @@ TALER_EXCHANGEDB_denomination_key_write (const char *filename,
                       | GNUNET_DISK_OPEN_FAILIFEXISTS,
                       GNUNET_DISK_PERM_USER_READ
                       | GNUNET_DISK_PERM_USER_WRITE)))
-    goto cleanup;
+  {
+    eno = errno;
+    GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR,
+                              "open",
+                              filename);
+    errno = eno;
+    return GNUNET_SYSERR;
+  }
   wsize = sizeof (struct TALER_EXCHANGEDB_DenominationKeyInformationP);
-  if (GNUNET_SYSERR == (wrote = GNUNET_DISK_file_write (fh,
-                                                        &dki->issue,
-                                                        wsize)))
-    goto cleanup;
-  if (wrote != (ssize_t) wsize)
-    goto cleanup;
-  if (GNUNET_SYSERR ==
-      (wrote = GNUNET_DISK_file_write (fh,
-                                       priv_enc,
-                                       priv_enc_size)))
+  if ( (GNUNET_SYSERR == (wrote = GNUNET_DISK_file_write (fh,
+                                                          &dki->issue,
+                                                          wsize))) ||
+       (wrote != (ssize_t) wsize) )
     goto cleanup;
-  if (wrote != (ssize_t) priv_enc_size)
-    goto cleanup;
-  ret = GNUNET_OK;
+  {
+    char *priv_enc;
+    size_t priv_enc_size;
+
+    priv_enc_size
+      = GNUNET_CRYPTO_rsa_private_key_encode (dki->denom_priv.rsa_private_key,
+                                              &priv_enc);
+    wrote = GNUNET_DISK_file_write (fh,
+                                    priv_enc,
+                                    priv_enc_size);
+    GNUNET_free (priv_enc);
+    if ( (GNUNET_SYSERR == wrote) ||
+         (wrote != (ssize_t) priv_enc_size) )
+      goto cleanup;
+  }
+  GNUNET_assert (GNUNET_OK ==
+                 GNUNET_DISK_file_close (fh));
+  return GNUNET_OK;
+
 cleanup:
-  GNUNET_free_non_null (priv_enc);
-  if (NULL != fh)
-    (void) GNUNET_DISK_file_close (fh);
-  return ret;
+  eno = errno;
+  GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR,
+                            "write",
+                            filename);
+  GNUNET_break (GNUNET_OK ==
+                GNUNET_DISK_file_close (fh));
+  /* try to remove the file, as it must be malformed */
+  if (0 != unlink (filename))
+    GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR,
+                              "unlink",
+                              filename);
+  errno = eno;
+  return GNUNET_SYSERR;
 }
 
 
@@ -337,21 +378,22 @@ denomkeys_iterate_topdir_iter (void *cls,
  *         as maybe none of the files were well-formed)
  */
 int
-TALER_EXCHANGEDB_denomination_keys_iterate (const char *exchange_base_dir,
-                                            TALER_EXCHANGEDB_DenominationKeyIterator
-                                            it,
-                                            void *it_cls)
+TALER_EXCHANGEDB_denomination_keys_iterate (
+  const char *exchange_base_dir,
+  TALER_EXCHANGEDB_DenominationKeyIterator it,
+  void *it_cls)
 {
+  struct DenomkeysIterateContext dic = {
+    .it = it,
+    .it_cls = it_cls
+  };
   char *dir;
-  struct DenomkeysIterateContext dic;
   int ret;
 
   GNUNET_asprintf (&dir,
                    "%s" DIR_SEPARATOR_STR
                    TALER_EXCHANGEDB_DIR_DENOMINATION_KEYS,
                    exchange_base_dir);
-  dic.it = it;
-  dic.it_cls = it_cls;
   ret = GNUNET_DISK_directory_scan (dir,
                                     &denomkeys_iterate_topdir_iter,
                                     &dic);
@@ -401,42 +443,57 @@ revocations_iterate_cb (void *cls,
 {
   struct RevocationsIterateContext *ric = cls;
   struct RevocationFileP rf;
-  struct TALER_MasterDenominationKeyRevocationPS rm;
+  ssize_t rd;
 
   /* Check if revocation is valid... */
-  if (sizeof (rf) !=
-      GNUNET_DISK_fn_read (filename,
-                           &rf,
-                           sizeof (rf)))
+  rd = GNUNET_DISK_fn_read (filename,
+                            &rf,
+                            sizeof (rf));
+  if (GNUNET_SYSERR == rd)
   {
-    GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
-                _ (
-                  "Invalid revocation file `%s' found and ignored (bad size)\n"),
-                filename);
+    GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
+                              "read",
+                              filename);
     return GNUNET_OK;
   }
-  rm.purpose.purpose = htonl (TALER_SIGNATURE_MASTER_DENOMINATION_KEY_REVOKED);
-  rm.purpose.size = htonl (sizeof (rm));
-  rm.h_denom_pub = rf.denom_hash;
-  if (GNUNET_OK !=
-      GNUNET_CRYPTO_eddsa_verify (
-        TALER_SIGNATURE_MASTER_DENOMINATION_KEY_REVOKED,
-        &rm.purpose,
-        &rf.msig.eddsa_signature,
-        &ric->master_pub->eddsa_pub))
+  if (sizeof (rf) != (size_t) rd)
   {
     GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
-                _ (
-                  "Invalid revocation file `%s' found and ignored (bad signature)\n"),
-                filename);
+                "Invalid revocation file `%s' found and ignored (bad size: %llu)\n",
+                filename,
+                (unsigned long long) rd);
     return GNUNET_OK;
   }
-  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "Denomination key `%s' was revoked!\n",
-              GNUNET_h2s (&rm.h_denom_pub));
-  return ric->it (ric->it_cls,
-                  &rm.h_denom_pub,
-                  &rf.msig);
+
+  {
+    struct TALER_MasterDenominationKeyRevocationPS rm = {
+      .purpose.purpose = htonl (
+        TALER_SIGNATURE_MASTER_DENOMINATION_KEY_REVOKED),
+      .purpose.size = htonl (sizeof (rm)),
+      .h_denom_pub = rf.denom_hash
+    };
+
+    if (GNUNET_OK !=
+        GNUNET_CRYPTO_eddsa_verify (
+          TALER_SIGNATURE_MASTER_DENOMINATION_KEY_REVOKED,
+          &rm.purpose,
+          &rf.msig.eddsa_signature,
+          &ric->master_pub->eddsa_pub))
+    {
+      GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+                  _ (
+                    "Invalid revocation file `%s' found and ignored (bad signature)\n"),
+                  filename);
+      return GNUNET_OK;
+    }
+
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "Denomination key `%s' was revoked!\n",
+                GNUNET_h2s (&rm.h_denom_pub));
+    return ric->it (ric->it_cls,
+                    &rm.h_denom_pub,
+                    &rf.msig);
+  }
 }
 
 
@@ -467,8 +524,11 @@ TALER_EXCHANGEDB_revocations_iterate (const char *revocation_dir,
 
   if (GNUNET_OK !=
       GNUNET_DISK_directory_create (revocation_dir))
-    return 0; /* directory doesn't exist and we couldn't even create it,
-     clearly means there are no revocations there */
+  {
+    /* directory doesn't exist and we couldn't even create it,
+       clearly means there are no revocations there */
+    return 0;
+  }
   return GNUNET_DISK_directory_scan (revocation_dir,
                                      &revocations_iterate_cb,
                                      &ric);