From c212578bf05138801597dcce23f52780c9b7dbb3 Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Fri, 24 Jul 2020 18:14:15 -0700 Subject: [PATCH 1/4] Introduce write_str_if_not_equal() --- src/feature/relay/router.c | 14 +++++--------- src/lib/fs/files.c | 20 ++++++++++++++++++++ src/lib/fs/files.h | 2 ++ 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 48f53a263d..25263468c8 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -870,15 +870,11 @@ router_write_fingerprint(int hashed, int ed25519_identity) tor_asprintf(&fingerprint_line, "%s %s\n", options->Nickname, fingerprint); /* Check whether we need to write the (hashed-)fingerprint file. */ - - cp = read_file_to_str(keydir, RFTS_IGNORE_MISSING, NULL); - if (!cp || strcmp(cp, fingerprint_line)) { - if (write_str_to_file(keydir, fingerprint_line, 0)) { - log_err(LD_FS, "Error writing %s%s line to file", - hashed ? "hashed " : "", - ed25519_identity ? "ed25519 identity" : "fingerprint"); - goto done; - } + if (write_str_if_not_equal(keydir, fingerprint_line)) { + log_err(LD_FS, "Error writing %s%s line to file", + hashed ? "hashed " : "", + ed25519_identity ? "ed25519 identity" : "fingerprint"); + goto done; } log_notice(LD_GENERAL, "Your Tor %s identity key %s fingerprint is '%s %s'", diff --git a/src/lib/fs/files.c b/src/lib/fs/files.c index a0b5a40aac..189d2cb646 100644 --- a/src/lib/fs/files.c +++ b/src/lib/fs/files.c @@ -718,6 +718,26 @@ read_file_to_str, (const char *filename, int flags, struct stat *stat_out)) return string; } +/** Attempt to read a file fname. If the file's contents is + * equal to the string str, return 0. Otherwise, attempt to + * overwrite the file with the contents of str and return + * the value of write_str_to_file(). + */ +int +write_str_to_file_if_not_equal(const char *fname, const char *str) +{ + char *fstr = read_file_to_str(fname, RFTS_IGNORE_MISSING, NULL); + int rv; + + if (!fstr || strcmp(str, fstr)) { + rv = write_str_to_file(fname, str, 0); + } else { + rv = 0; + } + tor_free(fstr); + return rv; +} + #if !defined(HAVE_GETDELIM) || defined(TOR_UNIT_TESTS) #include "ext/getdelim.c" #endif diff --git a/src/lib/fs/files.h b/src/lib/fs/files.h index a109cd6248..62b79c4cd8 100644 --- a/src/lib/fs/files.h +++ b/src/lib/fs/files.h @@ -91,6 +91,8 @@ int append_bytes_to_file(const char *fname, const char *str, size_t len, int write_bytes_to_new_file(const char *fname, const char *str, size_t len, int bin); +int write_str_to_file_if_not_equal(const char *fname, const char *str); + /** Flag for read_file_to_str: open the file in binary mode. */ #define RFTS_BIN 1 /** Flag for read_file_to_str: it's okay if the file doesn't exist. */ From 67a62ccf51f46bde75e0675b1ee19c024152f088 Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Fri, 24 Jul 2020 18:23:50 -0700 Subject: [PATCH 2/4] Use write_str_if_not_equal() for onion services --- changes/bug40062 | 6 ++++++ src/feature/hs/hs_service.c | 2 +- src/feature/relay/router.c | 5 ++--- src/feature/rend/rendservice.c | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 changes/bug40062 diff --git a/changes/bug40062 b/changes/bug40062 new file mode 100644 index 0000000000..9f18685a94 --- /dev/null +++ b/changes/bug40062 @@ -0,0 +1,6 @@ + o Minor features (onion services): + - When writing an onion service hostname file, first read it to make + sure it contains what we want before attempting to write it. Now + onion services can set their existing onion service directories to + read-only and Tor will still work. Resolves ticket 40062. Patch by + Neel Chauhan. diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index b56b7f4368..3e264b4686 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -990,7 +990,7 @@ write_address_to_file(const hs_service_t *service, const char *fname_) tor_asprintf(&addr_buf, "%s.%s\n", service->onion_address, address_tld); /* Notice here that we use the given "fname_". */ fname = hs_path_from_filename(service->config.directory_path, fname_); - if (write_str_to_file(fname, addr_buf, 0) < 0) { + if (write_str_to_file_if_not_equal(fname, addr_buf)) { log_warn(LD_REND, "Could not write onion address to hostname file %s", escaped(fname)); goto end; diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 25263468c8..675b977ade 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -835,7 +835,7 @@ router_initialize_tls_context(void) STATIC int router_write_fingerprint(int hashed, int ed25519_identity) { - char *keydir = NULL, *cp = NULL; + char *keydir = NULL; const char *fname = hashed ? "hashed-fingerprint" : (ed25519_identity ? "fingerprint-ed25519" : "fingerprint"); @@ -870,7 +870,7 @@ router_write_fingerprint(int hashed, int ed25519_identity) tor_asprintf(&fingerprint_line, "%s %s\n", options->Nickname, fingerprint); /* Check whether we need to write the (hashed-)fingerprint file. */ - if (write_str_if_not_equal(keydir, fingerprint_line)) { + if (write_str_to_file_if_not_equal(keydir, fingerprint_line)) { log_err(LD_FS, "Error writing %s%s line to file", hashed ? "hashed " : "", ed25519_identity ? "ed25519 identity" : "fingerprint"); @@ -884,7 +884,6 @@ router_write_fingerprint(int hashed, int ed25519_identity) result = 0; done: - tor_free(cp); tor_free(keydir); tor_free(fingerprint_line); return result; diff --git a/src/feature/rend/rendservice.c b/src/feature/rend/rendservice.c index 1ac88d0eb7..8e1a22fb39 100644 --- a/src/feature/rend/rendservice.c +++ b/src/feature/rend/rendservice.c @@ -1554,7 +1554,7 @@ rend_service_load_keys(rend_service_t *s) fname = rend_service_path(s, hostname_fname); tor_snprintf(buf, sizeof(buf),"%s.onion\n", s->service_id); - if (write_str_to_file(fname,buf,0)<0) { + if (write_str_to_file_if_not_equal(fname, buf)) { log_warn(LD_CONFIG, "Could not write onion address to hostname file."); goto err; } From f3e5b283ad0c589acf4eda31d7ca478553c28376 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 30 Jul 2020 10:37:45 -0400 Subject: [PATCH 3/4] test_util.c: Extract utime() function. We need this to manipulate mtimes, but only in this file. --- src/test/test_util.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/test/test_util.c b/src/test/test_util.c index 2aee07a26a..5fe9617f89 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -77,6 +77,8 @@ #define DISABLE_PWDB_TESTS #endif +static void set_file_mtime(const char *fname, time_t when); + #define INFINITY_DBL ((double)INFINITY) #define NAN_DBL ((double)NAN) @@ -5786,6 +5788,20 @@ test_util_get_avail_disk_space(void *arg) ; } +/** Helper: Change the atime and mtime of a file. */ +static void +set_file_mtime(const char *fname, time_t when) +{ + struct utimbuf u = { when, when }; + struct stat st; + tt_int_op(0, OP_EQ, utime(fname, &u)); + tt_int_op(0, OP_EQ, stat(fname, &st)); + /* Let's hope that utime/stat give the same second as a round-trip? */ + tt_i64_op(st.st_mtime, OP_EQ, when); +done: + ; +} + static void test_util_touch_file(void *arg) { @@ -5803,11 +5819,7 @@ test_util_touch_file(void *arg) tt_i64_op(st.st_mtime, OP_GE, now - 1); const time_t five_sec_ago = now - 5; - struct utimbuf u = { five_sec_ago, five_sec_ago }; - tt_int_op(0, OP_EQ, utime(fname, &u)); - tt_int_op(0, OP_EQ, stat(fname, &st)); - /* Let's hope that utime/stat give the same second as a round-trip? */ - tt_i64_op(st.st_mtime, OP_EQ, five_sec_ago); + set_file_mtime(fname, five_sec_ago); /* Finally we can touch the file */ tt_int_op(0, OP_EQ, touch_file(fname)); From fcf4954cc83570818d6be15f2117e31cc3eda34e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 30 Jul 2020 10:40:18 -0400 Subject: [PATCH 4/4] Add a unit test for write_str_to_file_if_not_equal() --- src/test/test_util.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/test/test_util.c b/src/test/test_util.c index 5fe9617f89..1bbe747ec1 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -357,6 +357,55 @@ test_util_write_chunks_to_file(void *arg) tor_free(temp_str); } +/* Test write_str_to_file_if_not_equal(). */ +static void +test_util_write_str_if_changed(void *arg) +{ + (void)arg; + char *fname = tor_strdup(get_fname("write_if_changed")); + char *s = NULL; + int rv; + const char str1[] = "The wombat lives across the seas"; + const char str2[] = "Among the far Antipodes"; /* -- Ogden Nash */ + + /* We can create files. */ + rv = write_str_to_file_if_not_equal(fname, str1); + tt_int_op(rv, OP_EQ, 0); + s = read_file_to_str(fname, 0, NULL); + tt_str_op(s, OP_EQ, str1); + tor_free(s); + + /* We can replace files. */ + rv = write_str_to_file_if_not_equal(fname, str2); + tt_int_op(rv, OP_EQ, 0); + s = read_file_to_str(fname, 0, NULL); + tt_str_op(s, OP_EQ, str2); + tor_free(s); + + /* Make sure we don't replace files when they're equal. (That's the whole + * point of the function we're testing. */ + /* First, change the mtime of the file so that we can tell whether we + * replaced it. */ + const time_t now = time(NULL); + const time_t five_sec_ago = now - 5; + set_file_mtime(fname, five_sec_ago); + rv = write_str_to_file_if_not_equal(fname, str2); + tt_int_op(rv, OP_EQ, 0); + /* Make sure that the file's mtime is unchanged... */ + struct stat st; + rv = stat(fname, &st); + tt_int_op(rv, OP_EQ, 0); + tt_i64_op(st.st_mtime, OP_EQ, five_sec_ago); + /* And make sure its contents are unchanged. */ + s = read_file_to_str(fname, 0, NULL); + tt_str_op(s, OP_EQ, str2); + tor_free(s); + + done: + tor_free(fname); + tor_free(s); +} + #ifndef COCCI #define _TFE(a, b, f) tt_int_op((a).f, OP_EQ, (b).f) /** test the minimum set of struct tm fields needed for a unique epoch value @@ -6557,6 +6606,7 @@ struct testcase_t util_tests[] = { UTIL_TEST(read_file_eof_zero_bytes, 0), UTIL_TEST(read_file_endlines, 0), UTIL_TEST(write_chunks_to_file, 0), + UTIL_TEST(write_str_if_changed, 0), UTIL_TEST(mathlog, 0), UTIL_TEST(fraction, 0), UTIL_TEST(weak_random, 0),