[tor-commits] [tor/master] Make the DataDirectory option immutable.

[nickm at torproject.org]

commit 192be006919c0bef04b7fce9e53c88ec1fcf4219
Author: Nick Mathewson <nickm at torproject.org>
Date:   Tue Nov 14 15:54:36 2017 -0500

    Make the DataDirectory option immutable.
    
    By convention, the torrc options that the user sets are
    unchangeable.  If we need to change them, we should be using a copy
    that's stored in another field
    
    To avoid trouble, I'm keeping DataDirectory as the name for the
    field that the rest of Tor uses, and using DataDirectory_option for
    the confparse-controlled field.
    
    This commit also modernizes some older string handling code in the
    DataDirectory normalization function.
---
 src/or/config.c | 41 ++++++++++++++++++++---------------------
 src/or/or.h     |  4 +++-
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/src/or/config.c b/src/or/config.c
index b0a140d23..ad5c3143f 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -286,7 +286,7 @@ static config_var_t option_vars_[] = {
   V(CookieAuthFileGroupReadable, BOOL,     "0"),
   V(CookieAuthFile,              STRING,   NULL),
   V(CountPrivateBandwidth,       BOOL,     "0"),
-  V(DataDirectory,               FILENAME, NULL),
+  VAR("DataDirectory",           FILENAME, DataDirectory_option, NULL),
   V(DataDirectoryGroupReadable,  BOOL,     "0"),
   V(DisableOOSCheck,             BOOL,     "1"),
   V(DisableNetwork,              BOOL,     "0"),
@@ -941,6 +941,7 @@ or_options_free(or_options_t *options)
     SMARTLIST_FOREACH(options->FilesOpenedByIncludes, char *, f, tor_free(f));
     smartlist_free(options->FilesOpenedByIncludes);
   }
+  tor_free(options->DataDirectory);
   tor_free(options->BridgePassword_AuthDigest_);
   tor_free(options->command_arg);
   tor_free(options->master_key_fname);
@@ -7682,31 +7683,29 @@ port_exists_by_type_addr32h_port(int listener_type, uint32_t addr_ipv4h,
                                        check_wildcard);
 }
 
-/** Adjust the value of options->DataDirectory, or fill it in if it's
- * absent. Return 0 on success, -1 on failure. */
-static int
-normalize_data_directory(or_options_t *options)
+/** Allocate and return a good value for the DataDirectory based on
+ *  <b>val</b>, which may be NULL.  Return NULL on failure. */
+static char *
+get_data_directory(const char *val)
 {
 #ifdef _WIN32
-  char *p;
-  if (options->DataDirectory)
-    return 0; /* all set */
-  p = tor_malloc(MAX_PATH);
-  strlcpy(p,get_windows_conf_root(),MAX_PATH);
-  options->DataDirectory = p;
-  return 0;
+  if (val) {
+    return tor_strdup(val);
+  } else {
+    return tor_strdup(get_windows_conf_root());
+  }
 #else /* !(defined(_WIN32)) */
-  const char *d = options->DataDirectory;
+  const char *d = val;
   if (!d)
     d = "~/.tor";
 
- if (strncmp(d,"~/",2) == 0) {
+  if (!strcmpstart(d, "~/")) {
    char *fn = expand_filename(d);
    if (!fn) {
      log_warn(LD_CONFIG,"Failed to expand filename \"%s\".", d);
-     return -1;
+     return NULL;
    }
-   if (!options->DataDirectory && !strcmp(fn,"/.tor")) {
+   if (!val && !strcmp(fn,"/.tor")) {
      /* If our homedir is /, we probably don't want to use it. */
      /* Default to LOCALSTATEDIR/tor which is probably closer to what we
       * want. */
@@ -7717,10 +7716,9 @@ normalize_data_directory(or_options_t *options)
      tor_free(fn);
      fn = tor_strdup(LOCALSTATEDIR PATH_SEPARATOR "tor");
    }
-   tor_free(options->DataDirectory);
-   options->DataDirectory = fn;
+   return fn;
  }
- return 0;
+ return tor_strdup(d);
 #endif /* defined(_WIN32) */
 }
 
@@ -7729,9 +7727,10 @@ normalize_data_directory(or_options_t *options)
 static int
 validate_data_directory(or_options_t *options)
 {
-  if (normalize_data_directory(options) < 0)
+  tor_free(options->DataDirectory);
+  options->DataDirectory = get_data_directory(options->DataDirectory_option);
+  if (!options->DataDirectory)
     return -1;
-  tor_assert(options->DataDirectory);
   if (strlen(options->DataDirectory) > (512-128)) {
     log_warn(LD_CONFIG, "DataDirectory is too long.");
     return -1;
diff --git a/src/or/or.h b/src/or/or.h
index fa5268ac5..61e8d0b44 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -3641,7 +3641,9 @@ typedef struct {
   char *SyslogIdentityTag; /**< Identity tag to add for syslog logging. */
 
   char *DebugLogFile; /**< Where to send verbose log messages. */
-  char *DataDirectory; /**< OR only: where to store long-term data. */
+  char *DataDirectory_option; /**< Where to store long-term data, as
+                               * configured by the user. */
+  char *DataDirectory; /**< Where to store long-term data, as modified. */
   int DataDirectoryGroupReadable; /**< Boolean: Is the DataDirectory g+r? */
   char *Nickname; /**< OR only: nickname of this onion router. */
   char *Address; /**< OR only: configured address for this onion router. */