[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor/master] Stop failing when key files are zero-length
commit fd7e9e9030cee9d8e863cea3f3f90226ae66fdfe
Author: teor <teor2345@xxxxxxxxx>
Date: Sun Oct 19 17:48:07 2014 +1100
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.
---
changes/bug13111-generate-keys-on-empty-file | 10 +++++++
src/common/compat.c | 1 +
src/common/util.c | 30 ++++++++++++++-------
src/common/util.h | 2 +-
src/or/config.c | 22 +++++++++++-----
src/or/router.c | 36 +++++++++++++++++++-------
src/or/routerlist.c | 1 +
src/or/statefile.c | 3 +++
8 files changed, 79 insertions(+), 26 deletions(-)
diff --git a/changes/bug13111-generate-keys-on-empty-file b/changes/bug13111-generate-keys-on-empty-file
index f6a56ef..345e87a 100644
--- 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 e4758aa..b28790f 100644
--- 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 e850b14..1c35338 100644
--- 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 d7feb6e..c5471ff 100644
--- 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 3f31e87..3e7a73f 100644
--- 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 01838b4..94ae2e7 100644
--- 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 0099486..5c08f59 100644
--- 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 2ce53fd..6640aed 100644
--- 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:
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits