commit 726dc9acf599567bf2ed1786fc2af785f15cac25
Author: Andrea Shepard <andrea(a)torproject.org>
Date: Sat Jun 25 05:20:46 2016 +0000
Remove old unparseable descriptor logging mechanism, add bump-to-head-of-queue for repeated unparseable descriptors, rename config variable
---
src/or/config.c | 2 +-
src/or/or.h | 6 +-
src/or/routerparse.c | 231 ++++++++++++++++++++++++++-------------------------
3 files changed, 123 insertions(+), 116 deletions(-)
diff --git …
[View More]a/src/or/config.c b/src/or/config.c
index 275be28..b00f8a9 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -212,7 +212,6 @@ static config_var_t option_vars_[] = {
V(CountPrivateBandwidth, BOOL, "0"),
V(DataDirectory, FILENAME, NULL),
V(DataDirectoryGroupReadable, BOOL, "0"),
- V(DetailedLogForUnparseableDescriptors, MEMUNIT, "0 bytes"),
V(DisableNetwork, BOOL, "0"),
V(DirAllowPrivateAddresses, BOOL, "0"),
V(TestingAuthDirTimeToLearnReachability, INTERVAL, "30 minutes"),
@@ -326,6 +325,7 @@ static config_var_t option_vars_[] = {
VAR("MaxMemInQueues", MEMUNIT, MaxMemInQueues_raw, "0"),
OBSOLETE("MaxOnionsPending"),
V(MaxOnionQueueDelay, MSEC_INTERVAL, "1750 msec"),
+ V(MaxUnparseableDescSizeToLog, MEMUNIT, "10 MB"),
V(MinMeasuredBWsForAuthToIgnoreAdvertised, INT, "500"),
V(MyFamily, STRING, NULL),
V(NewCircuitPeriod, INTERVAL, "30 seconds"),
diff --git a/src/or/or.h b/src/or/or.h
index 364699f..a1a0810 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4499,10 +4499,10 @@ typedef struct {
/** Autobool: Do we try to retain capabilities if we can? */
int KeepBindCapabilities;
- /** If > 0, maximum total size of unparseable descriptors to log
- * during the lifetime of this Tor process.
+ /** Maximum total size of unparseable descriptors to log during the
+ * lifetime of this Tor process.
*/
- uint64_t DetailedLogForUnparseableDescriptors;
+ uint64_t MaxUnparseableDescSizeToLog;
} or_options_t;
/** Persistent state for an onion router, as saved to disk. */
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 5ad6549..1d227fd 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -585,28 +585,40 @@ static int check_signature_token(const char *digest,
#define DUMP_AREA(a,name) STMT_NIL
#endif
-/** Last time we dumped a descriptor to disk. */
-static time_t last_desc_dumped = 0;
+/* Dump mechanism for unparseable descriptors */
+
/** List of dumped descriptors for FIFO cleanup purposes */
static smartlist_t *descs_dumped = NULL;
/** Total size of dumped descriptors for FIFO cleanup */
static size_t len_descs_dumped = 0;
+/*
+ * One entry in the list of dumped descriptors; filename dumped to, length
+ * and SHA-256.
+ */
+
typedef struct {
char *filename;
size_t len;
- time_t timestamp;
+ uint8_t digest_sha256[DIGEST256_LEN];
} dumped_desc_t;
/** Dump desc FIFO/cleanup; take ownership of the given filename, add it to
* the FIFO, and clean up the oldest entries to the extent they exceed the
- * configured cap. */
+ * configured cap. If any old entries with a matching hash existed, they
+ * just got overwritten right before this was called and we should adjust
+ * the total size counter without deleting them.
+ */
static void
-dump_desc_fifo_add_and_clean(char *filename, size_t len, time_t now)
+dump_desc_fifo_add_and_clean(char *filename, const uint8_t *digest_sha256,
+ size_t len)
{
dumped_desc_t *ent = NULL, *tmp;
size_t max_len;
+ tor_assert(filename != NULL);
+ tor_assert(digest_sha256 != NULL);
+
if (descs_dumped == NULL) {
/* We better have no length, then */
tor_assert(len_descs_dumped == 0);
@@ -618,43 +630,45 @@ dump_desc_fifo_add_and_clean(char *filename, size_t len, time_t now)
ent = tor_malloc_zero(sizeof(*ent));
ent->filename = filename;
ent->len = len;
- ent->timestamp = now;
+ memcpy(ent->digest_sha256, digest_sha256, DIGEST256_LEN);
/* Do we need to do some cleanup? */
- if (get_options()->DetailedLogForUnparseableDescriptors > 0) {
- max_len = get_options()->DetailedLogForUnparseableDescriptors;
- /* Iterate over the list until we've freed enough space */
- while (len_descs_dumped + len > max_len &&
- smartlist_len(descs_dumped) > 0) {
- /* Get the oldest thing on the list */
- tmp = (dumped_desc_t *)(smartlist_get(descs_dumped, 0));
+ max_len = get_options()->MaxUnparseableDescSizeToLog;
+ /* Iterate over the list until we've freed enough space */
+ while (len_descs_dumped + len > max_len &&
+ smartlist_len(descs_dumped) > 0) {
+ /* Get the oldest thing on the list */
+ tmp = (dumped_desc_t *)(smartlist_get(descs_dumped, 0));
+
+ /*
+ * Check if it matches the filename we just added, so we don't delete
+ * something we just emitted if we get repeated identical descriptors.
+ */
+ if (strcmp(tmp->filename, filename) != 0) {
+ /* Delete it and adjust the length counter */
+ unlink(tmp->filename);
+ tor_assert(len_descs_dumped >= tmp->len);
+ len_descs_dumped -= tmp->len;
+ log_info(LD_DIR,
+ "Deleting old unparseable descriptor dump %s due to "
+ "space limits",
+ tmp->filename);
+ } else {
/*
- * Check if it matches the filename we just added, so we don't
- * delete something we just emitted if we get repeated identical
- * descriptors.
+ * Don't delete, but do adjust the counter since we will bump it
+ * later
*/
- if (strcmp(tmp->filename, filename) != 0) {
- /* Delete it and adjust the length counter */
- unlink(tmp->filename);
- tor_assert(len_descs_dumped >= tmp->len);
- len_descs_dumped -= tmp->len;
- log_info(LD_DIR, "Deleting old unparseable descriptor dump %s due to "
- "space limits", tmp->filename);
- } else {
- /*
- * Don't delete, but do adjust the counter since we will bump it
- * later
- */
- tor_assert(len_descs_dumped >= tmp->len);
- len_descs_dumped -= tmp->len;
- log_info(LD_DIR, "Replacing old descriptor dump %s with new identical"
- " one", tmp->filename);
- }
- /* Free it and remove it from the list */
- smartlist_del_keeporder(descs_dumped, 0);
- tor_free(tmp->filename);
- tor_free(tmp);
+ tor_assert(len_descs_dumped >= tmp->len);
+ len_descs_dumped -= tmp->len;
+ log_info(LD_DIR,
+ "Replacing old descriptor dump %s with new identical one",
+ tmp->filename);
}
+
+ /* Free it and remove it from the list */
+ smartlist_del_keeporder(descs_dumped, 0);
+ tor_free(tmp->filename);
+ tor_free(tmp);
}
/* Append our entry to the end of the list and bump the counter */
@@ -662,6 +676,43 @@ dump_desc_fifo_add_and_clean(char *filename, size_t len, time_t now)
len_descs_dumped += len;
}
+/** Check if we already have a descriptor for this hash and move it to the
+ * head of the queue if so. Return 1 if one existed and 0 otherwise.
+ */
+static int
+dump_desc_fifo_bump_hash(const uint8_t *digest_sha256)
+{
+ dumped_desc_t *match = NULL;
+
+ tor_assert(digest_sha256);
+
+ if (descs_dumped) {
+ /* Find a match if one exists */
+ SMARTLIST_FOREACH_BEGIN(descs_dumped, dumped_desc_t *, ent) {
+ if (ent &&
+ memcmp(ent->digest_sha256, digest_sha256, DIGEST256_LEN) == 0) {
+ /*
+ * Save a pointer to the match and remove it from its current
+ * position.
+ */
+ match = ent;
+ SMARTLIST_DEL_CURRENT_KEEPORDER(descs_dumped, ent);
+ break;
+ }
+ } SMARTLIST_FOREACH_END(ent);
+
+ if (match) {
+ /* Add it back at the end of the list */
+ smartlist_add(descs_dumped, match);
+
+ /* Indicate we found one */
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
/** Clean up on exit; just memory, leave the dumps behind
*/
static void
@@ -688,28 +739,14 @@ dump_desc_fifo_cleanup(void)
static void
dump_desc(const char *desc, const char *type)
{
- time_t now = time(NULL);
tor_assert(desc);
tor_assert(type);
- /* Are we dumping this one to a file? */
- int dumping_this_one;
- /*
- * Are we including the hash in the filename and using the
- * FIFO mechanism?
- */
- int dumping_by_hash;
- /* Emit an "Unable to parse descriptor..." prefix? */
- int emit_prefix;
size_t len;
/* The SHA256 of the string */
uint8_t digest_sha256[DIGEST256_LEN];
char digest_sha256_hex[HEX_DIGEST256_LEN+1];
/* Filename to log it to */
char *debugfile, *debugfile_base;
- /* Expected length of file */
- size_t filelen;
- /* File content */
- char *content;
/* Get the hash for logging purposes anyway */
len = strlen(desc);
@@ -724,76 +761,46 @@ dump_desc(const char *desc, const char *type)
base16_encode(digest_sha256_hex, sizeof(digest_sha256_hex),
(const char *)digest_sha256, sizeof(digest_sha256));
- if (get_options()->DetailedLogForUnparseableDescriptors > 0) {
- /* Okay, we're logging to a fresh file named by hash for each one */
- dumping_by_hash = 1;
- dumping_this_one = 1;
- /*
- * Detailed logging mechanism will mention type and hash in the main log;
- * don't clutter up the files with anything but the exact dump.
- */
- emit_prefix = 0;
- tor_asprintf(&debugfile_base, "unparseable-desc.%s", digest_sha256_hex);
- debugfile = get_datadir_fname(debugfile_base);
- } else if (!last_desc_dumped || last_desc_dumped + 60 < now) {
- /* Simple mechanism, check so we don't dump too often */
- dumping_by_hash = 0;
- dumping_this_one = 1;
- emit_prefix = 1;
- debugfile_base = tor_strdup("unparseable-desc");
- debugfile = get_datadir_fname(debugfile_base);
- } else {
- /* No logging of the full descriptor this time */
- dumping_by_hash = 0;
- dumping_this_one = 0;
- emit_prefix = 1;
- debugfile_base = NULL;
- debugfile = NULL;
- }
-
- if (dumping_this_one) {
- if (emit_prefix) filelen = 50 + strlen(type) + len;
- else filelen = len;
-
- /* Get content pointing at what we're writing */
- if (emit_prefix) {
- content = tor_malloc_zero(filelen);
- tor_snprintf(content, filelen, "Unable to parse descriptor of type "
- "%s:\n%s", type, desc);
- } else {
- content = tor_strdup(desc);
- }
-
- /* Write it, and tell the main log about it */
- write_str_to_file(debugfile, content, 1);
- log_info(LD_DIR,
- "Unable to parse descriptor of type %s with hash %s and length "
- "%lu. See file %s in data directory for details.",
- type, digest_sha256_hex, (unsigned long)len, debugfile_base);
-
- /* Free our in-memory copy if necessary */
- tor_free(content);
-
- last_desc_dumped = now;
-
- /* Give it to the FIFO and clean up as needed, if we're using one */
- if (dumping_by_hash) {
- dump_desc_fifo_add_and_clean(debugfile, filelen, now);
+ /*
+ * We mention type and hash in the main log; don't clutter up the files
+ * with anything but the exact dump.
+ */
+ tor_asprintf(&debugfile_base, "unparseable-desc.%s", digest_sha256_hex);
+ debugfile = get_datadir_fname(debugfile_base);
+
+ if (len <= get_options()->MaxUnparseableDescSizeToLog) {
+ if (!dump_desc_fifo_bump_hash(digest_sha256)) {
+ /* Write it, and tell the main log about it */
+ write_str_to_file(debugfile, desc, 1);
+ log_info(LD_DIR,
+ "Unable to parse descriptor of type %s with hash %s and "
+ "length %lu. See file %s in data directory for details.",
+ type, digest_sha256_hex, (unsigned long)len, debugfile_base);
+
+ dump_desc_fifo_add_and_clean(debugfile, digest_sha256, len);
/* Since we handed ownership over, don't free debugfile later */
debugfile = NULL;
+ } else {
+ /* We already had one with this hash dumped */
+ log_info(LD_DIR,
+ "Unable to parse descriptor of type %s with hash %s and "
+ "length %lu. Descriptor not dumped because one with that hash "
+ "has already been dumped.",
+ type, digest_sha256_hex, (unsigned long)len);
+ /* We do have to free debugfile in this case */
}
} else {
/* Just log that it happened without dumping */
log_info(LD_DIR,
"Unable to parse descriptor of type %s with hash %s and length "
- "%lu. Descriptor not dumped since we just dumped one %u seconds "
- "ago.",
- type, digest_sha256_hex, (unsigned long)len,
- (unsigned int)(now - last_desc_dumped));
+ "%lu. Descriptor not dumped because it exceeds maximum log size "
+ "all by itself.",
+ type, digest_sha256_hex, (unsigned long)len);
+ /* We do have to free debugfile in this case */
}
- if (debugfile_base != NULL) tor_free(debugfile_base);
- if (debugfile != NULL) tor_free(debugfile);
+ tor_free(debugfile_base);
+ tor_free(debugfile);
err:
return;
[View Less]
commit 603f483092778786e29944acf71a608bfa21650b
Author: Andrea Shepard <andrea(a)torproject.org>
Date: Wed Jun 29 22:40:28 2016 +0000
Use uint64_t for total length of dumped descriptors, nad be careful about overflows in the loop in dump_desc_fifo_add_and_clean()
---
src/or/routerparse.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 5f1dde4..afdfcbd 100644
--- a/src/or/routerparse.c
+++ b/src/or/…
[View More]routerparse.c
@@ -590,7 +590,7 @@ static int check_signature_token(const char *digest,
/** List of dumped descriptors for FIFO cleanup purposes */
STATIC smartlist_t *descs_dumped = NULL;
/** Total size of dumped descriptors for FIFO cleanup */
-STATIC size_t len_descs_dumped = 0;
+STATIC uint64_t len_descs_dumped = 0;
/*
* One entry in the list of dumped descriptors; filename dumped to, length
@@ -614,7 +614,7 @@ dump_desc_fifo_add_and_clean(char *filename, const uint8_t *digest_sha256,
size_t len)
{
dumped_desc_t *ent = NULL, *tmp;
- size_t max_len;
+ uint64_t max_len;
tor_assert(filename != NULL);
tor_assert(digest_sha256 != NULL);
@@ -635,7 +635,7 @@ dump_desc_fifo_add_and_clean(char *filename, const uint8_t *digest_sha256,
/* Do we need to do some cleanup? */
max_len = get_options()->MaxUnparseableDescSizeToLog;
/* Iterate over the list until we've freed enough space */
- while (len_descs_dumped + len > max_len &&
+ while (len > max_len - len_descs_dumped &&
smartlist_len(descs_dumped) > 0) {
/* Get the oldest thing on the list */
tmp = (dumped_desc_t *)(smartlist_get(descs_dumped, 0));
[View Less]
commit dc37546cff2f025613ef142e74ad4db1c7d99ade
Author: Andrea Shepard <andrea(a)torproject.org>
Date: Wed Jun 29 22:47:41 2016 +0000
Add sandbox_is_active() check to dump_desc()
---
src/or/routerparse.c | 55 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 35 insertions(+), 20 deletions(-)
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index afdfcbd..93b90cc 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -28,6 +28,7 @@
#include "…
[View More]routerparse.h"
#include "entrynodes.h"
#include "torcert.h"
+#include "sandbox.h"
#undef log
#include <math.h>
@@ -768,35 +769,49 @@ dump_desc(const char *desc, const char *type)
tor_asprintf(&debugfile_base, "unparseable-desc.%s", digest_sha256_hex);
debugfile = get_datadir_fname(debugfile_base);
- if (len <= get_options()->MaxUnparseableDescSizeToLog) {
- if (!dump_desc_fifo_bump_hash(digest_sha256)) {
- /* Write it, and tell the main log about it */
- write_str_to_file(debugfile, desc, 1);
- log_info(LD_DIR,
- "Unable to parse descriptor of type %s with hash %s and "
- "length %lu. See file %s in data directory for details.",
- type, digest_sha256_hex, (unsigned long)len, debugfile_base);
-
- dump_desc_fifo_add_and_clean(debugfile, digest_sha256, len);
- /* Since we handed ownership over, don't free debugfile later */
- debugfile = NULL;
+ if (!sandbox_is_active()) {
+ if (len <= get_options()->MaxUnparseableDescSizeToLog) {
+ if (!dump_desc_fifo_bump_hash(digest_sha256)) {
+ /* Write it, and tell the main log about it */
+ write_str_to_file(debugfile, desc, 1);
+ log_info(LD_DIR,
+ "Unable to parse descriptor of type %s with hash %s and "
+ "length %lu. See file %s in data directory for details.",
+ type, digest_sha256_hex, (unsigned long)len,
+ debugfile_base);
+ dump_desc_fifo_add_and_clean(debugfile, digest_sha256, len);
+ /* Since we handed ownership over, don't free debugfile later */
+ debugfile = NULL;
+ } else {
+ /* We already had one with this hash dumped */
+ log_info(LD_DIR,
+ "Unable to parse descriptor of type %s with hash %s and "
+ "length %lu. Descriptor not dumped because one with that "
+ "hash has already been dumped.",
+ type, digest_sha256_hex, (unsigned long)len);
+ /* We do have to free debugfile in this case */
+ }
} else {
- /* We already had one with this hash dumped */
+ /* Just log that it happened without dumping */
log_info(LD_DIR,
"Unable to parse descriptor of type %s with hash %s and "
- "length %lu. Descriptor not dumped because one with that hash "
- "has already been dumped.",
+ "length %lu. Descriptor not dumped because it exceeds maximum"
+ " log size all by itself.",
type, digest_sha256_hex, (unsigned long)len);
/* We do have to free debugfile in this case */
}
} else {
- /* Just log that it happened without dumping */
+ /*
+ * Not logging because the sandbox is active and seccomp2 apparently
+ * doesn't have a sensible way to allow filenames according to a pattern
+ * match. (If we ever figure out how to say "allow writes to /regex/",
+ * remove this checK).
+ */
log_info(LD_DIR,
- "Unable to parse descriptor of type %s with hash %s and length "
- "%lu. Descriptor not dumped because it exceeds maximum log size "
- "all by itself.",
+ "Unable to parse descriptor of type %s with hash %s and "
+ "length %lu. Descriptor not dumped because the sandbox is "
+ "active",
type, digest_sha256_hex, (unsigned long)len);
- /* We do have to free debugfile in this case */
}
tor_free(debugfile_base);
[View Less]
commit 38cced90ef62aff04477d1b814ab2ee3b7776638
Author: Andrea Shepard <andrea(a)torproject.org>
Date: Thu Jun 30 00:39:29 2016 +0000
Move unparseable descriptor dumps into subdirectory of DataDir
---
src/common/util.c | 6 +--
src/common/util.h | 5 +-
src/or/main.c | 3 ++
src/or/routerparse.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++-----
src/or/routerparse.h | 1 +
src/test/test_dir.c | 34 +++++++++++--
6 files changed, 162 insertions(+), 20 …
[View More]deletions(-)
diff --git a/src/common/util.c b/src/common/util.c
index d4553c3..d60380c 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -2086,9 +2086,9 @@ file_status(const char *fname)
* When effective_user is not NULL, check permissions against the given user
* and its primary group.
*/
-int
-check_private_dir(const char *dirname, cpd_check_t check,
- const char *effective_user)
+MOCK_IMPL(int,
+check_private_dir,(const char *dirname, cpd_check_t check,
+ const char *effective_user))
{
int r;
struct stat st;
diff --git a/src/common/util.h b/src/common/util.h
index fd9b983..70483bb 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -326,8 +326,9 @@ typedef unsigned int cpd_check_t;
#define CPD_GROUP_READ (1u << 3)
#define CPD_CHECK_MODE_ONLY (1u << 4)
#define CPD_RELAX_DIRMODE_CHECK (1u << 5)
-int check_private_dir(const char *dirname, cpd_check_t check,
- const char *effective_user);
+MOCK_DECL(int, check_private_dir,
+ (const char *dirname, cpd_check_t check,
+ const char *effective_user));
#define OPEN_FLAGS_REPLACE (O_WRONLY|O_CREAT|O_TRUNC)
#define OPEN_FLAGS_APPEND (O_WRONLY|O_CREAT|O_APPEND)
diff --git a/src/or/main.c b/src/or/main.c
index 3291570..65a67a9 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -3045,6 +3045,9 @@ tor_init(int argc, char *argv[])
log_warn(LD_NET, "Problem initializing libevent RNG.");
}
+ /* Scan/clean unparseable descroptors; after reading config */
+ routerparse_init();
+
return 0;
}
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 93b90cc..75c09d2 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -592,6 +592,11 @@ static int check_signature_token(const char *digest,
STATIC smartlist_t *descs_dumped = NULL;
/** Total size of dumped descriptors for FIFO cleanup */
STATIC uint64_t len_descs_dumped = 0;
+/** Directory to stash dumps in */
+static int have_dump_desc_dir = 0;
+static int problem_with_dump_desc_dir = 0;
+
+#define DESC_DUMP_DATADIR_SUBDIR "unparseable-descs"
/*
* One entry in the list of dumped descriptors; filename dumped to, length
@@ -604,6 +609,88 @@ typedef struct {
uint8_t digest_sha256[DIGEST256_LEN];
} dumped_desc_t;
+/** Find the dump directory and check if we'll be able to create it */
+static void
+dump_desc_init(void)
+{
+ char *dump_desc_dir;
+
+ dump_desc_dir = get_datadir_fname(DESC_DUMP_DATADIR_SUBDIR);
+
+ /*
+ * We just check for it, don't create it at this point; we'll
+ * create it when we need it if it isn't already there.
+ */
+ if (check_private_dir(dump_desc_dir, CPD_CHECK, get_options()->User) < 0) {
+ /* Error, log and flag it as having a problem */
+ log_notice(LD_DIR,
+ "Doesn't look like we'll be able to create descriptor dump "
+ "directory %s; dumps will be disabled.",
+ dump_desc_dir);
+ problem_with_dump_desc_dir = 1;
+ tor_free(dump_desc_dir);
+ return;
+ }
+
+ /* Check if it exists */
+ switch (file_status(dump_desc_dir)) {
+ case FN_DIR:
+ /* We already have a directory */
+ have_dump_desc_dir = 1;
+ break;
+ case FN_NOENT:
+ /* Nothing, we'll need to create it later */
+ have_dump_desc_dir = 0;
+ break;
+ case FN_ERROR:
+ /* Log and flag having a problem */
+ log_notice(LD_DIR,
+ "Couldn't check whether descriptor dump directory %s already"
+ " exists: %s",
+ dump_desc_dir, strerror(errno));
+ problem_with_dump_desc_dir = 1;
+ case FN_FILE:
+ case FN_EMPTY:
+ default:
+ /* Something else was here! */
+ log_notice(LD_DIR,
+ "Descriptor dump directory %s already exists and isn't a "
+ "directory",
+ dump_desc_dir);
+ problem_with_dump_desc_dir = 1;
+ }
+
+ tor_free(dump_desc_dir);
+}
+
+/** Create the dump directory if needed and possible */
+static void
+dump_desc_create_dir(void)
+{
+ char *dump_desc_dir;
+
+ /* If the problem flag is set, skip it */
+ if (problem_with_dump_desc_dir) return;
+
+ /* Do we need it? */
+ if (!have_dump_desc_dir) {
+ dump_desc_dir = get_datadir_fname(DESC_DUMP_DATADIR_SUBDIR);
+
+ if (check_private_dir(dump_desc_dir, CPD_CREATE,
+ get_options()->User) < 0) {
+ log_notice(LD_DIR,
+ "Failed to create descriptor dump directory %s",
+ dump_desc_dir);
+ problem_with_dump_desc_dir = 1;
+ }
+
+ /* Okay, we created it */
+ have_dump_desc_dir = 1;
+
+ tor_free(dump_desc_dir);
+ }
+}
+
/** Dump desc FIFO/cleanup; take ownership of the given filename, add it to
* the FIFO, and clean up the oldest entries to the extent they exceed the
* configured cap. If any old entries with a matching hash existed, they
@@ -767,21 +854,35 @@ dump_desc(const char *desc, const char *type)
* with anything but the exact dump.
*/
tor_asprintf(&debugfile_base, "unparseable-desc.%s", digest_sha256_hex);
- debugfile = get_datadir_fname(debugfile_base);
+ debugfile = get_datadir_fname2(DESC_DUMP_DATADIR_SUBDIR, debugfile_base);
if (!sandbox_is_active()) {
if (len <= get_options()->MaxUnparseableDescSizeToLog) {
if (!dump_desc_fifo_bump_hash(digest_sha256)) {
- /* Write it, and tell the main log about it */
- write_str_to_file(debugfile, desc, 1);
- log_info(LD_DIR,
- "Unable to parse descriptor of type %s with hash %s and "
- "length %lu. See file %s in data directory for details.",
- type, digest_sha256_hex, (unsigned long)len,
- debugfile_base);
- dump_desc_fifo_add_and_clean(debugfile, digest_sha256, len);
- /* Since we handed ownership over, don't free debugfile later */
- debugfile = NULL;
+ /* Create the directory if needed */
+ dump_desc_create_dir();
+ /* Make sure we've got it */
+ if (have_dump_desc_dir && !problem_with_dump_desc_dir) {
+ /* Write it, and tell the main log about it */
+ write_str_to_file(debugfile, desc, 1);
+ log_info(LD_DIR,
+ "Unable to parse descriptor of type %s with hash %s and "
+ "length %lu. See file %s in data directory for details.",
+ type, digest_sha256_hex, (unsigned long)len,
+ debugfile_base);
+ dump_desc_fifo_add_and_clean(debugfile, digest_sha256, len);
+ /* Since we handed ownership over, don't free debugfile later */
+ debugfile = NULL;
+ } else {
+ /* Problem with the subdirectory */
+ log_info(LD_DIR,
+ "Unable to parse descriptor of type %s with hash %s and "
+ "length %lu. Descriptor not dumped because we had a "
+ "problem creating the " DESC_DUMP_DATADIR_SUBDIR
+ " subdirectory",
+ type, digest_sha256_hex, (unsigned long)len);
+ /* We do have to free debugfile in this case */
+ }
} else {
/* We already had one with this hash dumped */
log_info(LD_DIR,
@@ -5676,6 +5777,16 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr)
return result;
}
+/** Called on startup; right now we just handle scanning the unparseable
+ * descriptor dumps, but hang anything else we might need to do in the
+ * future here as well.
+ */
+void
+routerparse_init(void)
+{
+ dump_desc_init();
+}
+
/** Clean up all data structures used by routerparse.c at exit */
void
routerparse_free_all(void)
diff --git a/src/or/routerparse.h b/src/or/routerparse.h
index 2b18143..a96146a 100644
--- a/src/or/routerparse.h
+++ b/src/or/routerparse.h
@@ -85,6 +85,7 @@ int rend_parse_introduction_points(rend_service_descriptor_t *parsed,
size_t intro_points_encoded_size);
int rend_parse_client_keys(strmap_t *parsed_clients, const char *str);
+void routerparse_init(void);
void routerparse_free_all(void);
#ifdef ROUTERPARSE_PRIVATE
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index e18ed42..b4fc615 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -4103,6 +4103,22 @@ test_dir_choose_compression_level(void* data)
}
/*
+ * Mock check_private_dir(), and always succeed - no need to actually
+ * look at or create anything on the filesystem.
+ */
+
+static int
+mock_check_private_dir(const char *dirname, cpd_check_t check,
+ const char *effective_user)
+{
+ (void)dirname;
+ (void)check;
+ (void)effective_user;
+
+ return 0;
+}
+
+/*
* This really mocks options_get_datadir_fname2_suffix(), but for testing
* dump_desc(), we only care about get_datadir_fname(sub1), which is defined
* in config.h as:
@@ -4118,16 +4134,24 @@ mock_get_datadir_fname(const or_options_t *options,
char *rv = NULL;
/*
- * Assert we were called like get_datadir_fname(), since it's all
- * we implement here.
+ * Assert we were called like get_datadir_fname2() or get_datadir_fname(),
+ * since that's all we implement here.
*/
tt_assert(options != NULL);
tt_assert(sub1 != NULL);
- tt_assert(sub2 == NULL);
+ /*
+ * No particular assertions about sub2, since we could be in the
+ * get_datadir_fname() or get_datadir_fname2() case.
+ */
tt_assert(suffix == NULL);
/* Just duplicate the basename and return it for this mock */
- rv = strdup(sub1);
+ if (sub2) {
+ /* If we have sub2, it's the basename, otherwise sub1 */
+ rv = strdup(sub2);
+ } else {
+ rv = strdup(sub1);
+ }
done:
return rv;
@@ -4262,6 +4286,7 @@ test_dir_dump_unparseable_descriptors(void *data)
reset_options(mock_options, &mock_get_options_calls);
mock_options->MaxUnparseableDescSizeToLog = 1536;
MOCK(get_options, mock_get_options);
+ MOCK(check_private_dir, mock_check_private_dir);
MOCK(options_get_datadir_fname2_suffix,
mock_get_datadir_fname);
@@ -4768,6 +4793,7 @@ test_dir_dump_unparseable_descriptors(void *data)
UNMOCK(write_str_to_file);
mock_write_str_to_file_reset();
UNMOCK(options_get_datadir_fname2_suffix);
+ UNMOCK(check_private_dir);
UNMOCK(get_options);
free(mock_options);
mock_options = NULL;
[View Less]
commit 13a16e001164581b687dae2d3377f77eedb701ff
Author: Andrea Shepard <andrea(a)torproject.org>
Date: Thu Jun 30 09:37:23 2016 +0000
Also check if the sandbox is configured as well as if it's active; sandbox_init() runs rather late in the startup process
---
src/or/routerparse.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index def6373..875bb60 100644
--- a/src/or/routerparse.c
+++ b/src/or/…
[View More]routerparse.c
@@ -1073,7 +1073,11 @@ dump_desc(const char *desc, const char *type)
DESC_DUMP_BASE_FILENAME ".%s", digest_sha256_hex);
debugfile = get_datadir_fname2(DESC_DUMP_DATADIR_SUBDIR, debugfile_base);
- if (!sandbox_is_active()) {
+ /*
+ * Check if the sandbox is active or will become active; see comment
+ * below at the log message for why.
+ */
+ if (!(sandbox_is_active() || get_options()->Sandbox)) {
if (len <= get_options()->MaxUnparseableDescSizeToLog) {
if (!dump_desc_fifo_bump_hash(digest_sha256)) {
/* Create the directory if needed */
@@ -1128,7 +1132,7 @@ dump_desc(const char *desc, const char *type)
log_info(LD_DIR,
"Unable to parse descriptor of type %s with hash %s and "
"length %lu. Descriptor not dumped because the sandbox is "
- "active",
+ "configured",
type, digest_sha256_hex, (unsigned long)len);
}
@@ -6001,7 +6005,12 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr)
void
routerparse_init(void)
{
- if (!(sandbox_is_active())) {
+ /*
+ * Check both if the sandbox is active and whether it's configured; no
+ * point in loading all that if we won't be able to use it after the
+ * sandbox becomes active.
+ */
+ if (!(sandbox_is_active() || get_options()->Sandbox)) {
dump_desc_init();
}
}
[View Less]