[tor-commits] [tor/master] Move unparseable descriptor dumps into subdirectory of DataDir
nickm at torproject.org
nickm at torproject.org
Thu Jun 30 15:18:32 UTC 2016
commit 38cced90ef62aff04477d1b814ab2ee3b7776638
Author: Andrea Shepard <andrea at 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 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;
More information about the tor-commits
mailing list