Stop failing when key files are zero-length

Instead, generate new keys, and overwrite the empty key files.
Adds FN_EMPTY to file_status_t and file_status.
Fixes bug 13111.

Related changes due to review of FN_FILE usage:
Stop generating a fresh .old RSA key file when the .old file is missing.
Avoid overwriting .old key files with empty key files.
Skip loading zero-length extra info store, router store, stats, state,
and key files.

diff --git a/changes/bug13111-generate-keys-on-empty-file b/changes/bug13111-generate-keys-on-empty-file
index f6a56effbb1970e8f7e217cf50a01a4806a73678..345e87ab756930795b72f4d2629724dd7294be97 100644 (file)
--- a/changes/bug13111-generate-keys-on-empty-file
+++ b/changes/bug13111-generate-keys-on-empty-file
@@ -1,7 +1,17 @@
   o Minor bugfixes
+    - Stop failing when key files are zero-length. Instead, generate new
+      keys, and overwrite the empty key files.
+      Fixes bug 13111.
+    - Stop generating a fresh .old RSA key file when the .old file is missing.
+      Fixed as part of bug 13111.
+    - Avoid overwriting .old key files with empty key files.
+      Fixed as part of bug 13111.
     - Stop crashing when a NULL filename is passed to file_status().
       Fixed as part of bug 13111.
 
   o Minor enhancements:
+    - Skip loading zero-length extra info store, router store, stats, state,
+      and key files.
+      Implemented with bug 13111.
     - Return FN_ERROR when a zero-length filename is passed to file_status().
       Fixed as part of bug 13111.

diff --git a/src/common/compat.c b/src/common/compat.c
index e4758aaf8833204e99e90d3825f203555b32a525..b28790f0e42803abe5b5bc42db7fd29169f25069 100644 (file)
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -823,6 +823,7 @@ replace_file(const char *from, const char *to)
     case FN_NOENT:
       break;
     case FN_FILE:
+    case FN_EMPTY:
       if (unlink(to)) return -1;
       break;
     case FN_ERROR:

diff --git a/src/common/util.c b/src/common/util.c
index e850b14a16e6fdbe21c199651f1c98433ef9473a..1c35338eef6ebc1c5d8f7ce8f6389c7adce4e7e9 100644 (file)
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1888,10 +1888,15 @@ clean_name_for_stat(char *name)
 #endif
 }
 
-/** Return FN_ERROR if filename can't be read, or is NULL or zero-length,
- * FN_NOENT if it doesn't
- * exist, FN_FILE if it is a regular file, or FN_DIR if it's a
- * directory.  On FN_ERROR, sets errno. */
+/** Return:
+ * FN_ERROR if filename can't be read, is NULL, or is zero-length,
+ * FN_NOENT if it doesn't exist,
+ * FN_FILE if it is a non-empty regular file, or a FIFO on unix-like systems,
+ * FN_EMPTY for zero-byte regular files,
+ * FN_DIR if it's a directory, and
+ * FN_ERROR for any other file type.
+ * On FN_ERROR and FN_NOENT, sets errno.  (errno is not set when FN_ERROR
+ * is returned due to an unhandled file type.) */
 file_status_t
 file_status(const char *fname)
 {
@@ -1912,16 +1917,23 @@ file_status(const char *fname)
     }
     return FN_ERROR;
   }
-  if (st.st_mode & S_IFDIR)
+  if (st.st_mode & S_IFDIR) {
     return FN_DIR;
-  else if (st.st_mode & S_IFREG)
-    return FN_FILE;
+  } else if (st.st_mode & S_IFREG) {
+    if (st.st_size > 0) {
+      return FN_FILE;
+    } else if (st.st_size == 0) {
+      return FN_EMPTY;
+    } else {
+      return FN_ERROR;
+    }
 #ifndef _WIN32
-  else if (st.st_mode & S_IFIFO)
+  } else if (st.st_mode & S_IFIFO) {
     return FN_FILE;
 #endif
-  else
+  } else {
     return FN_ERROR;
+  }
 }
 
 /** Check whether <b>dirname</b> exists and is private.  If yes return 0.  If

diff --git a/src/common/util.h b/src/common/util.h
index d7feb6e925a399a3d76825f871b9c6bbb2d4cd8f..c5471ff9f23129f6019ae51da419ad7ce83c4a34 100644 (file)
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -334,7 +334,7 @@ enum stream_status get_string_from_pipe(FILE *stream, char *buf, size_t count);
 
 /** Return values from file_status(); see that function's documentation
  * for details. */
-typedef enum { FN_ERROR, FN_NOENT, FN_FILE, FN_DIR } file_status_t;
+typedef enum { FN_ERROR, FN_NOENT, FN_FILE, FN_DIR, FN_EMPTY } file_status_t;
 file_status_t file_status(const char *filename);
 
 /** Possible behaviors for check_private_dir() on encountering a nonexistent

diff --git a/src/or/config.c b/src/or/config.c
index 3f31e876dd89189f9ddc8c6d890ee0774b178bbd..3e7a73f6127b116cd917897cbedc3bd508d06ea6 100644 (file)
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -4017,17 +4017,24 @@ find_torrc_filename(config_line_t *cmd_arg,
   if (*using_default_fname) {
     /* didn't find one, try CONFDIR */
     const char *dflt = get_default_conf_file(defaults_file);
-    if (dflt && file_status(dflt) == FN_FILE) {
+    file_status_t st = file_status(dflt);
+    if (dflt && (st == FN_FILE || st == FN_EMPTY)) {
       fname = tor_strdup(dflt);
     } else {
 #ifndef _WIN32
       char *fn = NULL;
-      if (!defaults_file)
+      if (!defaults_file) {
         fn = expand_filename("~/.torrc");
-      if (fn && file_status(fn) == FN_FILE) {
-        fname = fn;
+      }
+      if (fn) {
+        file_status_t hmst = file_status(fn);
+        if (hmst == FN_FILE || hmst == FN_EMPTY) {
+          fname = fn;
+        } else {
+          tor_free(fn);
+          fname = tor_strdup(dflt);
+        }
       } else {
-        tor_free(fn);
         fname = tor_strdup(dflt);
       }
 #else
@@ -4063,7 +4070,8 @@ load_torrc_from_disk(config_line_t *cmd_arg, int defaults_file)
   *fname_var = fname;
 
   /* Open config file */
-  if (file_status(fname) != FN_FILE ||
+  file_status_t st = file_status(fname);
+  if (!(st == FN_FILE || st == FN_EMPTY) ||
       !(cf = read_file_to_str(fname,0,NULL))) {
     if (using_default_torrc == 1 || ignore_missing_torrc) {
       if (!defaults_file)
@@ -6413,7 +6421,9 @@ write_configuration_file(const char *fname, const or_options_t *options)
   tor_assert(fname);
 
   switch (file_status(fname)) {
+    /* create backups of old config files, even if they're empty */
     case FN_FILE:
+    case FN_EMPTY:
       old_val = read_file_to_str(fname, 0, NULL);
       if (!old_val || strcmpstart(old_val, GENERATED_FILE_PREFIX)) {
         rename_old = 1;

diff --git a/src/or/router.c b/src/or/router.c
index 01838b4b3ecefce5b7979a47f98bc741819dede6..94ae2e78c4dc511fa6d68a4647c13e0b24e9d19c 100644 (file)
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -313,6 +313,7 @@ rotate_onion_key(void)
   time_t now;
   fname = get_datadir_fname2("keys", "secret_onion_key");
   fname_prev = get_datadir_fname2("keys", "secret_onion_key.old");
+  /* There isn't much point replacing an old key with an empty file */
   if (file_status(fname) == FN_FILE) {
     if (replace_file(fname, fname_prev))
       goto error;
@@ -335,6 +336,7 @@ rotate_onion_key(void)
   fname_prev = get_datadir_fname2("keys", "secret_onion_key_ntor.old");
   if (curve25519_keypair_generate(&new_curve25519_keypair, 1) < 0)
     goto error;
+  /* There isn't much point replacing an old key with an empty file */
   if (file_status(fname) == FN_FILE) {
     if (replace_file(fname, fname_prev))
       goto error;
@@ -389,9 +391,9 @@ log_new_relay_greeting(void)
   already_logged = 1;
 }
 
-/** Try to read an RSA key from <b>fname</b>.  If <b>fname</b> doesn't exist
- * and <b>generate</b> is true, create a new RSA key and save it in
- * <b>fname</b>.  Return the read/created key, or NULL on error.  Log all
+/** Try to read an RSA key from <b>fname</b>.  If <b>fname</b> doesn't exist,
+ * or is empty, and <b>generate</b> is true, create a new RSA key and save it
+ * in <b>fname</b>.  Return the read/created key, or NULL on error.  Log all
  * errors at level <b>severity</b>.
  */
 crypto_pk_t *
@@ -409,7 +411,11 @@ init_key_from_file(const char *fname, int generate, int severity)
     case FN_ERROR:
       tor_log(severity, LD_FS,"Can't read key from \"%s\"", fname);
       goto error;
+    /* treat empty key files as if the file doesn't exist, and,
+     * if generate is set, replace the empty file in
+     * crypto_pk_write_private_key_to_filename() */
     case FN_NOENT:
+    case FN_EMPTY:
       if (generate) {
         if (!have_lockfile()) {
           if (try_locking(get_options(), 0)<0) {
@@ -460,10 +466,10 @@ init_key_from_file(const char *fname, int generate, int severity)
 }
 
 /** Load a curve25519 keypair from the file <b>fname</b>, writing it into
- * <b>keys_out</b>.  If the file isn't found and <b>generate</b> is true,
- * create a new keypair and write it into the file.  If there are errors, log
- * them at level <b>severity</b>. Generate files using <b>tag</b> in their
- * ASCII wrapper. */
+ * <b>keys_out</b>.  If the file isn't found, or is empty, and <b>generate</b>
+ * is true, create a new keypair and write it into the file.  If there are
+ * errors, log them at level <b>severity</b>. Generate files using <b>tag</b>
+ * in their ASCII wrapper. */
 static int
 init_curve25519_keypair_from_file(curve25519_keypair_t *keys_out,
                                   const char *fname,
@@ -476,7 +482,10 @@ init_curve25519_keypair_from_file(curve25519_keypair_t *keys_out,
     case FN_ERROR:
       tor_log(severity, LD_FS,"Can't read key from \"%s\"", fname);
       goto error;
+    /* treat empty key files as if the file doesn't exist, and, if generate
+     * is set, replace the empty file in curve25519_keypair_write_to_file() */
     case FN_NOENT:
+    case FN_EMPTY:
       if (generate) {
         if (!have_lockfile()) {
           if (try_locking(get_options(), 0)<0) {
@@ -876,7 +885,9 @@ init_keys(void)
 
   keydir = get_datadir_fname2("keys", "secret_onion_key.old");
   if (!lastonionkey && file_status(keydir) == FN_FILE) {
-    prkey = init_key_from_file(keydir, 1, LOG_ERR); /* XXXX Why 1? */
+    /* Load keys from non-empty files only.
+     * Missing old keys won't be replaced with freshly generated keys. */
+    prkey = init_key_from_file(keydir, 0, LOG_ERR);
     if (prkey)
       lastonionkey = prkey;
   }
@@ -897,6 +908,8 @@ init_keys(void)
                            last_curve25519_onion_key.pubkey.public_key,
                         CURVE25519_PUBKEY_LEN) &&
         file_status(keydir) == FN_FILE) {
+      /* Load keys from non-empty files only.
+       * Missing old keys won't be replaced with freshly generated keys. */
       init_curve25519_keypair_from_file(&last_curve25519_onion_key,
                                         keydir, 0, LOG_ERR, "onion");
     }
@@ -2562,8 +2575,9 @@ router_has_orport(const routerinfo_t *router, const tor_addr_port_t *orport)
  * <b>end_line</b>, ensure that its timestamp is not more than 25 hours in
  * the past or more than 1 hour in the future with respect to <b>now</b>,
  * and write the file contents starting with that line to *<b>out</b>.
- * Return 1 for success, 0 if the file does not exist, or -1 if the file
- * does not contain a line matching these criteria or other failure. */
+ * Return 1 for success, 0 if the file does not exist or is empty, or -1
+ * if the file does not contain a line matching these criteria or other
+ * failure. */
 static int
 load_stats_file(const char *filename, const char *end_line, time_t now,
                 char **out)
@@ -2597,7 +2611,9 @@ load_stats_file(const char *filename, const char *end_line, time_t now,
      notfound:
       tor_free(contents);
       break;
+    /* treat empty stats files as if the file doesn't exist */
     case FN_NOENT:
+    case FN_EMPTY:
       r = 0;
       break;
     case FN_ERROR:

diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 00994863f0efadf36118cd8173f7ebc863a56546..5c08f5990fcebe955c26f71166eb64265843c4e8 100644 (file)
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1204,6 +1204,7 @@ router_reload_router_list_impl(desc_store_t *store)
 
   tor_free(fname);
   fname = get_datadir_fname_suffix(store->fname_base, ".new");
+  /* don't load empty files - we wouldn't get any data, even if we tried */
   if (file_status(fname) == FN_FILE)
     contents = read_file_to_str(fname, RFTS_BIN|RFTS_IGNORE_MISSING, &st);
   if (contents) {

diff --git a/src/or/statefile.c b/src/or/statefile.c
index 2ce53fdfcac81126d19a05f2d57239fd1b05a563..6640aed7d0f7f933584fcad9a520a8b6e0b64fb9 100644 (file)
--- a/src/or/statefile.c
+++ b/src/or/statefile.c
@@ -323,7 +323,10 @@ or_state_load(void)
         goto done;
       }
       break;
+    /* treat empty state files as if the file doesn't exist, and generate
+     * a new state file, overwriting the empty file in or_state_save() */
     case FN_NOENT:
+    case FN_EMPTY:
       break;
     case FN_ERROR:
     case FN_DIR: