Merge remote-tracking branch 'tor-gitlab/mr/88'

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.

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;

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,16 +870,12 @@ 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)) {
+  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");
     goto done;
   }
-  }
 
   log_notice(LD_GENERAL, "Your Tor %s identity key %s fingerprint is '%s %s'",
              hashed ? "bridge's hashed" : "server's",
@@ -888,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;

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;
   }

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 <b>fname</b>. If the file's contents is
+ * equal to the string <b>str</b>, return 0. Otherwise, attempt to
+ * overwrite the file with the contents of <b>str</b> 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

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. */

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)
 
@@ -355,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
@@ -5786,6 +5837,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 +5868,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));
@@ -6545,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),