Merge branch 'more_directories_squashed'

18 files changed, 465 insertions, 277 deletions

diff --git a/src/or/config.c b/src/or/config.c
index dabebdc226..d589876785 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -253,6 +253,8 @@ static config_var_t option_vars_[] = {
   V(BridgeRecordUsageByCountry,  BOOL,     "1"),
   V(BridgeRelay,                 BOOL,     "0"),
   V(BridgeDistribution,          STRING,   NULL),
+  VAR("CacheDirectory",          FILENAME, CacheDirectory_option, NULL),
+  V(CacheDirectoryGroupReadable, BOOL,     "0"),
   V(CellStatistics,              BOOL,     "0"),
   V(PaddingStatistics,           BOOL,     "1"),
   V(LearnCircuitBuildTimeout,    BOOL,     "1"),
@@ -286,7 +288,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"),
@@ -392,6 +394,8 @@ static config_var_t option_vars_[] = {
   V(Socks5Proxy,                 STRING,   NULL),
   V(Socks5ProxyUsername,         STRING,   NULL),
   V(Socks5ProxyPassword,         STRING,   NULL),
+  VAR("KeyDirectory",            FILENAME, KeyDirectory_option, NULL),
+  V(KeyDirectoryGroupReadable,   BOOL,     "0"),
   V(KeepalivePeriod,             INTERVAL, "5 minutes"),
   V(KeepBindCapabilities,            AUTOBOOL, "auto"),
   VAR("Log",                     LINELIST, Logs,             NULL),
@@ -733,7 +737,7 @@ static int parse_ports(or_options_t *options, int validate_only,
 static int check_server_ports(const smartlist_t *ports,
                               const or_options_t *options,
                               int *num_low_ports_out);
-static int validate_data_directory(or_options_t *options);
+static int validate_data_directories(or_options_t *options);
 static int write_configuration_file(const char *fname,
                                     const or_options_t *options);
 static int options_init_logs(const or_options_t *old_options,
@@ -941,6 +945,9 @@ 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->CacheDirectory);
+  tor_free(options->KeyDirectory);
   tor_free(options->BridgePassword_AuthDigest_);
   tor_free(options->command_arg);
   tor_free(options->master_key_fname);
@@ -1250,6 +1257,69 @@ consider_adding_dir_servers(const or_options_t *options,
   return 0;
 }
 
+/**
+ * Make sure that <b>directory</b> exists, with appropriate ownership and
+ * permissions (as modified by <b>group_readable</b>). If <b>create</b>,
+ * create the directory if it is missing. Return 0 on success.
+ * On failure, return -1 and set *<b>msg_out</b>.
+ */
+static int
+check_and_create_data_directory(int create,
+                                const char *directory,
+                                int group_readable,
+                                const char *owner,
+                                char **msg_out)
+{
+  cpd_check_t cpd_opts = create ? CPD_CREATE : CPD_CHECK;
+  if (group_readable)
+      cpd_opts |= CPD_GROUP_READ;
+  if (check_private_dir(directory,
+                        cpd_opts,
+                        owner) < 0) {
+    tor_asprintf(msg_out,
+                 "Couldn't %s private data directory \"%s\"",
+                 create ? "create" : "access",
+                 directory);
+    return -1;
+  }
+
+#ifndef _WIN32
+  if (group_readable) {
+    /* Only new dirs created get new opts, also enforce group read. */
+    if (chmod(directory, 0750)) {
+      log_warn(LD_FS,"Unable to make %s group-readable: %s",
+               directory, strerror(errno));
+    }
+  }
+#endif /* !defined(_WIN32) */
+
+  return 0;
+}
+
+/**
+ * Ensure that our keys directory exists, with appropriate permissions.
+ * Return 0 on success, -1 on failure.
+ */
+int
+create_keys_directory(const or_options_t *options)
+{
+  /* Make sure DataDirectory exists, and is private. */
+  cpd_check_t cpd_opts = CPD_CREATE;
+  if (options->DataDirectoryGroupReadable)
+    cpd_opts |= CPD_GROUP_READ;
+  if (check_private_dir(options->DataDirectory, cpd_opts, options->User)) {
+    log_err(LD_OR, "Can't create/check datadirectory %s",
+            options->DataDirectory);
+    return -1;
+  }
+
+  /* Check the key directory. */
+  if (check_private_dir(options->KeyDirectory, CPD_CREATE, options->User)) {
+    return -1;
+  }
+  return 0;
+}
+
 /* Helps determine flags to pass to switch_id. */
 static int have_low_ports = -1;
 
@@ -1404,29 +1474,30 @@ options_act_reversible(const or_options_t *old_options, char **msg)
   }
 
   /* Ensure data directory is private; create if possible. */
-  cpd_check_t cpd_opts = running_tor ? CPD_CREATE : CPD_CHECK;
-  if (options->DataDirectoryGroupReadable)
-      cpd_opts |= CPD_GROUP_READ;
-  if (check_private_dir(options->DataDirectory,
-                        cpd_opts,
-                        options->User)<0) {
-    tor_asprintf(msg,
-              "Couldn't access/create private data directory \"%s\"",
-              options->DataDirectory);
-
+  /* It's okay to do this in "options_act_reversible()" even though it isn't
+   * actually reversible, since you can't change the DataDirectory while
+   * Tor is running. */
+  if (check_and_create_data_directory(running_tor /* create */,
+                                      options->DataDirectory,
+                                      options->DataDirectoryGroupReadable,
+                                      options->User,
+                                      msg) < 0) {
     goto done;
-    /* No need to roll back, since you can't change the value. */
   }
-
-#ifndef _WIN32
-  if (options->DataDirectoryGroupReadable) {
-    /* Only new dirs created get new opts, also enforce group read. */
-    if (chmod(options->DataDirectory, 0750)) {
-      log_warn(LD_FS,"Unable to make %s group-readable: %s",
-               options->DataDirectory, strerror(errno));
-    }
+  if (check_and_create_data_directory(running_tor /* create */,
+                                      options->KeyDirectory,
+                                      options->KeyDirectoryGroupReadable,
+                                      options->User,
+                                      msg) < 0) {
+    goto done;
+  }
+  if (check_and_create_data_directory(running_tor /* create */,
+                                      options->CacheDirectory,
+                                      options->CacheDirectoryGroupReadable,
+                                      options->User,
+                                      msg) < 0) {
+    goto done;
   }
-#endif /* !defined(_WIN32) */
 
   /* Bail out at this point if we're not going to be a client or server:
    * we don't run Tor itself. */
@@ -3188,7 +3259,7 @@ options_validate(or_options_t *old_options, or_options_t *options,
   if (parse_outbound_addresses(options, 1, msg) < 0)
     return -1;
 
-  if (validate_data_directory(options)<0)
+  if (validate_data_directories(options)<0)
     REJECT("Invalid DataDirectory");
 
   if (options->Nickname == NULL) {
@@ -4586,6 +4657,22 @@ options_transition_allowed(const or_options_t *old,
     return -1;
   }
 
+  if (!opt_streq(old->KeyDirectory, new_val->KeyDirectory)) {
+    tor_asprintf(msg,
+               "While Tor is running, changing KeyDirectory "
+               "(\"%s\"->\"%s\") is not allowed.",
+               old->KeyDirectory, new_val->KeyDirectory);
+    return -1;
+  }
+
+  if (!opt_streq(old->CacheDirectory, new_val->CacheDirectory)) {
+    tor_asprintf(msg,
+               "While Tor is running, changing CacheDirectory "
+               "(\"%s\"->\"%s\") is not allowed.",
+               old->CacheDirectory, new_val->CacheDirectory);
+    return -1;
+  }
+
   if (!opt_streq(old->User, new_val->User)) {
     *msg = tor_strdup("While Tor is running, changing User is not allowed.");
     return -1;
@@ -7682,60 +7769,81 @@ 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) {
-   char *fn = expand_filename(d);
-   if (!fn) {
-     log_warn(LD_CONFIG,"Failed to expand filename \"%s\".", d);
-     return -1;
-   }
-   if (!options->DataDirectory && !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. */
-     log_warn(LD_CONFIG,
-              "Default DataDirectory is \"~/.tor\".  This expands to "
-              "\"%s\", which is probably not what you want.  Using "
-              "\"%s"PATH_SEPARATOR"tor\" instead", fn, LOCALSTATEDIR);
-     tor_free(fn);
-     fn = tor_strdup(LOCALSTATEDIR PATH_SEPARATOR "tor");
-   }
-   tor_free(options->DataDirectory);
-   options->DataDirectory = fn;
- }
- return 0;
+  if (!strcmpstart(d, "~/")) {
+    char *fn = expand_filename(d);
+    if (!fn) {
+      log_warn(LD_CONFIG,"Failed to expand filename \"%s\".", d);
+      return NULL;
+    }
+    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. */
+      log_warn(LD_CONFIG,
+               "Default DataDirectory is \"~/.tor\".  This expands to "
+               "\"%s\", which is probably not what you want.  Using "
+               "\"%s"PATH_SEPARATOR"tor\" instead", fn, LOCALSTATEDIR);
+      tor_free(fn);
+      fn = tor_strdup(LOCALSTATEDIR PATH_SEPARATOR "tor");
+    }
+    return fn;
+  }
+  return tor_strdup(d);
 #endif /* defined(_WIN32) */
 }
 
-/** Check and normalize the value of options->DataDirectory; return 0 if it
- * is sane, -1 otherwise. */
+/** Check and normalize the values of options->{Key,Data,Cache}Directory;
+ * return 0 if it is sane, -1 otherwise. */
 static int
-validate_data_directory(or_options_t *options)
+validate_data_directories(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;
   }
+
+  tor_free(options->KeyDirectory);
+  if (options->KeyDirectory_option) {
+    options->KeyDirectory = get_data_directory(options->KeyDirectory_option);
+    if (!options->KeyDirectory)
+      return -1;
+  } else {
+    /* Default to the data directory's keys subdir */
+    tor_asprintf(&options->KeyDirectory, "%s"PATH_SEPARATOR"keys",
+                 options->DataDirectory);
+  }
+
+  tor_free(options->CacheDirectory);
+  if (options->CacheDirectory_option) {
+    options->CacheDirectory = get_data_directory(
+                                             options->CacheDirectory_option);
+    if (!options->CacheDirectory)
+      return -1;
+  } else {
+    /* Default to the data directory. */
+    options->CacheDirectory = tor_strdup(options->DataDirectory);
+  }
+
   return 0;
 }
 
@@ -7876,53 +7984,56 @@ init_libevent(const or_options_t *options)
   suppress_libevent_log_msg(NULL);
 }
 
-/** Return a newly allocated string holding a filename relative to the data
- * directory.  If <b>sub1</b> is present, it is the first path component after
+/** Return a newly allocated string holding a filename relative to the
+ * directory in <b>options</b> specified by <b>roottype</b>.
+ * If <b>sub1</b> is present, it is the first path component after
  * the data directory.  If <b>sub2</b> is also present, it is the second path
  * component after the data directory.  If <b>suffix</b> is present, it
  * is appended to the filename.
  *
- * Examples:
- *    get_datadir_fname2_suffix("a", NULL, NULL) -> $DATADIR/a
- *    get_datadir_fname2_suffix("a", NULL, ".tmp") -> $DATADIR/a.tmp
- *    get_datadir_fname2_suffix("a", "b", ".tmp") -> $DATADIR/a/b/.tmp
- *    get_datadir_fname2_suffix("a", "b", NULL) -> $DATADIR/a/b
- *
- * Note: Consider using the get_datadir_fname* macros in or.h.
+ * Note: Consider using macros in config.h that wrap this function;
+ * you should probably never need to call it as-is.
  */
 MOCK_IMPL(char *,
-options_get_datadir_fname2_suffix,(const or_options_t *options,
-                                   const char *sub1, const char *sub2,
-                                   const char *suffix))
+options_get_dir_fname2_suffix,(const or_options_t *options,
+                               directory_root_t roottype,
+                               const char *sub1, const char *sub2,
+                               const char *suffix))
 {
-  char *fname = NULL;
-  size_t len;
   tor_assert(options);
-  tor_assert(options->DataDirectory);
-  tor_assert(sub1 || !sub2); /* If sub2 is present, sub1 must be present. */
-  len = strlen(options->DataDirectory);
-  if (sub1) {
-    len += strlen(sub1)+1;
-    if (sub2)
-      len += strlen(sub2)+1;
-  }
-  if (suffix)
-    len += strlen(suffix);
-  len++;
-  fname = tor_malloc(len);
-  if (sub1) {
-    if (sub2) {
-      tor_snprintf(fname, len, "%s"PATH_SEPARATOR"%s"PATH_SEPARATOR"%s",
-                   options->DataDirectory, sub1, sub2);
-    } else {
-      tor_snprintf(fname, len, "%s"PATH_SEPARATOR"%s",
-                   options->DataDirectory, sub1);
-    }
+
+  const char *rootdir = NULL;
+  switch (roottype) {
+    case DIRROOT_DATADIR:
+      rootdir = options->DataDirectory;
+      break;
+    case DIRROOT_CACHEDIR:
+      rootdir = options->CacheDirectory;
+      break;
+    case DIRROOT_KEYDIR:
+      rootdir = options->KeyDirectory;
+      break;
+    default:
+      tor_assert_unreached();
+      break;
+  }
+  tor_assert(rootdir);
+
+  if (!suffix)
+    suffix = "";
+
+  char *fname = NULL;
+
+  if (sub1 == NULL) {
+    tor_asprintf(&fname, "%s%s", rootdir, suffix);
+    tor_assert(!sub2); /* If sub2 is present, sub1 must be present. */
+  } else if (sub2 == NULL) {
+    tor_asprintf(&fname, "%s"PATH_SEPARATOR"%s%s", rootdir, sub1, suffix);
   } else {
-    strlcpy(fname, options->DataDirectory, len);
+    tor_asprintf(&fname, "%s"PATH_SEPARATOR"%s"PATH_SEPARATOR"%s%s",
+                 rootdir, sub1, sub2, suffix);
   }
-  if (suffix)
-    strlcat(fname, suffix, len);
+
   return fname;
 }
 
diff --git a/src/or/config.h b/src/or/config.h
index efdd8c59b0..1e4239b6d7 100644
--- a/src/or/config.h
+++ b/src/or/config.h
@@ -58,31 +58,77 @@ config_line_t *option_get_assignment(const or_options_t *options,
                                      const char *key);
 int options_save_current(void);
 const char *get_torrc_fname(int defaults_fname);
+typedef enum {
+  DIRROOT_DATADIR,
+  DIRROOT_CACHEDIR,
+  DIRROOT_KEYDIR
+} directory_root_t;
+
 MOCK_DECL(char *,
-          options_get_datadir_fname2_suffix,
+          options_get_dir_fname2_suffix,
           (const or_options_t *options,
+           directory_root_t roottype,
            const char *sub1, const char *sub2,
            const char *suffix));
+
+/* These macros wrap options_get_dir_fname2_suffix to provide a more
+ * convenient API for finding filenames that Tor uses inside its storage
+ * They are named according to a pattern:
+ *    (options_)?get_(cache|key|data)dir_fname(2)?(_suffix)?
+ *
+ * Macros that begin with options_ take an options argument; the others
+ * work with respect to the global options.
+ *
+ * Each macro works relative to the data directory, the key directory,
+ * or the cache directory, as determined by which one is mentioned.
+ *
+ * Macro variants with "2" in their name take two path components; others
+ * take one.
+ *
+ * Macro variants with "_suffix" at the end take an additional suffix
+ * that gets appended to the end of the file
+ */
+#define options_get_datadir_fname2_suffix(options, sub1, sub2, suffix) \
+  options_get_dir_fname2_suffix((options), DIRROOT_DATADIR, \
+                                (sub1), (sub2), (suffix))
+#define options_get_cachedir_fname2_suffix(options, sub1, sub2, suffix) \
+  options_get_dir_fname2_suffix((options), DIRROOT_CACHEDIR, \
+                                (sub1), (sub2), (suffix))
+#define options_get_keydir_fname2_suffix(options, sub1, sub2, suffix) \
+  options_get_dir_fname2_suffix((options), DIRROOT_KEYDIR, \
+                                (sub1), (sub2), (suffix))
+
+#define options_get_datadir_fname(opts,sub1)                    \
+  options_get_datadir_fname2_suffix((opts),(sub1), NULL, NULL)
+#define options_get_datadir_fname2(opts,sub1,sub2)                      \
+  options_get_datadir_fname2_suffix((opts),(sub1), (sub2), NULL)
+
 #define get_datadir_fname2_suffix(sub1, sub2, suffix) \
   options_get_datadir_fname2_suffix(get_options(), (sub1), (sub2), (suffix))
-/** Return a newly allocated string containing datadir/sub1.  See
- * get_datadir_fname2_suffix.  */
-#define get_datadir_fname(sub1) get_datadir_fname2_suffix((sub1), NULL, NULL)
-/** Return a newly allocated string containing datadir/sub1/sub2.  See
- * get_datadir_fname2_suffix.  */
+#define get_datadir_fname(sub1)                 \
+  get_datadir_fname2_suffix((sub1), NULL, NULL)
 #define get_datadir_fname2(sub1,sub2) \
   get_datadir_fname2_suffix((sub1), (sub2), NULL)
-/** Return a newly allocated string containing datadir/sub1/sub2 relative to
- * opts.  See get_datadir_fname2_suffix.  */
-#define options_get_datadir_fname2(opts,sub1,sub2)                      \
-  options_get_datadir_fname2_suffix((opts),(sub1), (sub2), NULL)
-/** Return a newly allocated string containing datadir/sub1suffix.  See
- * get_datadir_fname2_suffix. */
 #define get_datadir_fname_suffix(sub1, suffix) \
   get_datadir_fname2_suffix((sub1), NULL, (suffix))
 
+/** DOCDOC */
+#define options_get_keydir_fname(options, sub1)  \
+  options_get_keydir_fname2_suffix((options), (sub1), NULL, NULL)
+#define get_keydir_fname_suffix(sub1, suffix)   \
+  options_get_keydir_fname2_suffix(get_options(), (sub1), NULL, suffix)
+#define get_keydir_fname(sub1)                  \
+  options_get_keydir_fname2_suffix(get_options(), (sub1), NULL, NULL)
+
+#define get_cachedir_fname(sub1) \
+  options_get_cachedir_fname2_suffix(get_options(), (sub1), NULL, NULL)
+#define get_cachedir_fname_suffix(sub1, suffix) \
+  options_get_cachedir_fname2_suffix(get_options(), (sub1), NULL, (suffix))
+
 int using_default_dir_authorities(const or_options_t *options);
 
+int create_keys_directory(const or_options_t *options);
+
 int check_or_create_data_subdir(const char *subdir);
 int write_to_data_subdir(const char* subdir, const char* fname,
                          const char* str, const char* descr);
diff --git a/src/or/conscache.c b/src/or/conscache.c
index 1a15126f97..9a90ff4dd2 100644
--- a/src/or/conscache.c
+++ b/src/or/conscache.c
@@ -79,7 +79,7 @@ consensus_cache_open(const char *subdir, int max_entries)
 {
   int storagedir_max_entries;
   consensus_cache_t *cache = tor_malloc_zero(sizeof(consensus_cache_t));
-  char *directory = get_datadir_fname(subdir);
+  char *directory = get_cachedir_fname(subdir);
   cache->max_entries = max_entries;
 
 #ifdef MUST_UNMAP_TO_UNLINK
diff --git a/src/or/control.c b/src/or/control.c
index f1960b8849..de08b2ee7d 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -2170,7 +2170,7 @@ getinfo_helper_dir(control_connection_t *control_conn,
         *answer = tor_strdup(consensus->dir);
     }
     if (!*answer) { /* try loading it from disk */
-      char *filename = get_datadir_fname("cached-consensus");
+      char *filename = get_cachedir_fname("cached-consensus");
       *answer = read_file_to_str(filename, RFTS_IGNORE_MISSING, NULL);
       tor_free(filename);
       if (!*answer) { /* generate an error */
diff --git a/src/or/main.c b/src/or/main.c
index dd7b0134ee..aae98dd8ab 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -3274,7 +3274,7 @@ try_locking(const or_options_t *options, int err_if_locked)
   if (lockfile)
     return 0;
   else {
-    char *fname = options_get_datadir_fname2_suffix(options, "lock",NULL,NULL);
+    char *fname = options_get_datadir_fname(options, "lock");
     int already_locked = 0;
     tor_lockfile_t *lf = tor_lockfile_lock(fname, 0, &already_locked);
     tor_free(fname);
@@ -3550,7 +3550,7 @@ sandbox_init_filter(void)
   int i;
 
   sandbox_cfg_allow_openat_filename(&cfg,
-      get_datadir_fname("cached-status"));
+      get_cachedir_fname("cached-status"));
 
 #define OPEN(name)                              \
   sandbox_cfg_allow_open_filename(&cfg, tor_strdup(name))
@@ -3571,21 +3571,38 @@ sandbox_init_filter(void)
     OPEN_DATADIR2(name, name2 suffix);                  \
   } while (0)
 
+#define OPEN_KEY_DIRECTORY() \
+  sandbox_cfg_allow_open_filename(&cfg, tor_strdup(options->KeyDirectory))
+#define OPEN_CACHEDIR(name)                      \
+  sandbox_cfg_allow_open_filename(&cfg, get_cachedir_fname(name))
+#define OPEN_CACHEDIR_SUFFIX(name, suffix) do {  \
+    OPEN_CACHEDIR(name);                         \
+    OPEN_CACHEDIR(name suffix);                  \
+  } while (0)
+#define OPEN_KEYDIR(name)                      \
+  sandbox_cfg_allow_open_filename(&cfg, get_keydir_fname(name))
+#define OPEN_KEYDIR_SUFFIX(name, suffix) do {    \
+    OPEN_KEYDIR(name);                           \
+    OPEN_KEYDIR(name suffix);                    \
+  } while (0)
+
   OPEN(options->DataDirectory);
-  OPEN_DATADIR("keys");
-  OPEN_DATADIR_SUFFIX("cached-certs", ".tmp");
-  OPEN_DATADIR_SUFFIX("cached-consensus", ".tmp");
-  OPEN_DATADIR_SUFFIX("unverified-consensus", ".tmp");
-  OPEN_DATADIR_SUFFIX("unverified-microdesc-consensus", ".tmp");
-  OPEN_DATADIR_SUFFIX("cached-microdesc-consensus", ".tmp");
-  OPEN_DATADIR_SUFFIX("cached-microdescs", ".tmp");
-  OPEN_DATADIR_SUFFIX("cached-microdescs.new", ".tmp");
-  OPEN_DATADIR_SUFFIX("cached-descriptors", ".tmp");
-  OPEN_DATADIR_SUFFIX("cached-descriptors.new", ".tmp");
-  OPEN_DATADIR("cached-descriptors.tmp.tmp");
-  OPEN_DATADIR_SUFFIX("cached-extrainfo", ".tmp");
-  OPEN_DATADIR_SUFFIX("cached-extrainfo.new", ".tmp");
-  OPEN_DATADIR("cached-extrainfo.tmp.tmp");
+  OPEN_KEY_DIRECTORY();
+
+  OPEN_CACHEDIR_SUFFIX("cached-certs", ".tmp");
+  OPEN_CACHEDIR_SUFFIX("cached-consensus", ".tmp");
+  OPEN_CACHEDIR_SUFFIX("unverified-consensus", ".tmp");
+  OPEN_CACHEDIR_SUFFIX("unverified-microdesc-consensus", ".tmp");
+  OPEN_CACHEDIR_SUFFIX("cached-microdesc-consensus", ".tmp");
+  OPEN_CACHEDIR_SUFFIX("cached-microdescs", ".tmp");
+  OPEN_CACHEDIR_SUFFIX("cached-microdescs.new", ".tmp");
+  OPEN_CACHEDIR_SUFFIX("cached-descriptors", ".tmp");
+  OPEN_CACHEDIR_SUFFIX("cached-descriptors.new", ".tmp");
+  OPEN_CACHEDIR("cached-descriptors.tmp.tmp");
+  OPEN_CACHEDIR_SUFFIX("cached-extrainfo", ".tmp");
+  OPEN_CACHEDIR_SUFFIX("cached-extrainfo.new", ".tmp");
+  OPEN_CACHEDIR("cached-extrainfo.tmp.tmp");
+
   OPEN_DATADIR_SUFFIX("state", ".tmp");
   OPEN_DATADIR_SUFFIX("sr-state", ".tmp");
   OPEN_DATADIR_SUFFIX("unparseable-desc", ".tmp");
@@ -3629,20 +3646,31 @@ sandbox_init_filter(void)
                            get_datadir_fname2(prefix, name suffix),     \
                            get_datadir_fname2(prefix, name))
 
-  RENAME_SUFFIX("cached-certs", ".tmp");
-  RENAME_SUFFIX("cached-consensus", ".tmp");
-  RENAME_SUFFIX("unverified-consensus", ".tmp");
-  RENAME_SUFFIX("unverified-microdesc-consensus", ".tmp");
-  RENAME_SUFFIX("cached-microdesc-consensus", ".tmp");
-  RENAME_SUFFIX("cached-microdescs", ".tmp");
-  RENAME_SUFFIX("cached-microdescs", ".new");
-  RENAME_SUFFIX("cached-microdescs.new", ".tmp");
-  RENAME_SUFFIX("cached-descriptors", ".tmp");
-  RENAME_SUFFIX("cached-descriptors", ".new");
-  RENAME_SUFFIX("cached-descriptors.new", ".tmp");
-  RENAME_SUFFIX("cached-extrainfo", ".tmp");
-  RENAME_SUFFIX("cached-extrainfo", ".new");
-  RENAME_SUFFIX("cached-extrainfo.new", ".tmp");
+#define RENAME_CACHEDIR_SUFFIX(name, suffix)        \
+  sandbox_cfg_allow_rename(&cfg,           \
+      get_cachedir_fname(name suffix),      \
+      get_cachedir_fname(name))
+
+#define RENAME_KEYDIR_SUFFIX(name, suffix)    \
+  sandbox_cfg_allow_rename(&cfg,           \
+      get_keydir_fname(name suffix),      \
+      get_keydir_fname(name))
+
+  RENAME_CACHEDIR_SUFFIX("cached-certs", ".tmp");
+  RENAME_CACHEDIR_SUFFIX("cached-consensus", ".tmp");
+  RENAME_CACHEDIR_SUFFIX("unverified-consensus", ".tmp");
+  RENAME_CACHEDIR_SUFFIX("unverified-microdesc-consensus", ".tmp");
+  RENAME_CACHEDIR_SUFFIX("cached-microdesc-consensus", ".tmp");
+  RENAME_CACHEDIR_SUFFIX("cached-microdescs", ".tmp");
+  RENAME_CACHEDIR_SUFFIX("cached-microdescs", ".new");
+  RENAME_CACHEDIR_SUFFIX("cached-microdescs.new", ".tmp");
+  RENAME_CACHEDIR_SUFFIX("cached-descriptors", ".tmp");
+  RENAME_CACHEDIR_SUFFIX("cached-descriptors", ".new");
+  RENAME_CACHEDIR_SUFFIX("cached-descriptors.new", ".tmp");
+  RENAME_CACHEDIR_SUFFIX("cached-extrainfo", ".tmp");
+  RENAME_CACHEDIR_SUFFIX("cached-extrainfo", ".new");
+  RENAME_CACHEDIR_SUFFIX("cached-extrainfo.new", ".tmp");
+
   RENAME_SUFFIX("state", ".tmp");
   RENAME_SUFFIX("sr-state", ".tmp");
   RENAME_SUFFIX("unparseable-desc", ".tmp");
@@ -3654,14 +3682,21 @@ sandbox_init_filter(void)
 #define STAT_DATADIR(name)                      \
   sandbox_cfg_allow_stat_filename(&cfg, get_datadir_fname(name))
 
+#define STAT_CACHEDIR(name)                                             \
+  sandbox_cfg_allow_stat_filename(&cfg, get_cachedir_fname(name))
+
 #define STAT_DATADIR2(name, name2)                                      \
   sandbox_cfg_allow_stat_filename(&cfg, get_datadir_fname2((name), (name2)))
 
+#define STAT_KEY_DIRECTORY() \
+  sandbox_cfg_allow_stat_filename(&cfg, tor_strdup(options->KeyDirectory))
+
   STAT_DATADIR(NULL);
   STAT_DATADIR("lock");
   STAT_DATADIR("state");
   STAT_DATADIR("router-stability");
-  STAT_DATADIR("cached-extrainfo.new");
+
+  STAT_CACHEDIR("cached-extrainfo.new");
 
   {
     smartlist_t *files = smartlist_new();
@@ -3726,22 +3761,20 @@ sandbox_init_filter(void)
   // orport
   if (server_mode(get_options())) {
 
-    OPEN_DATADIR2_SUFFIX("keys", "secret_id_key", ".tmp");
-    OPEN_DATADIR2_SUFFIX("keys", "secret_onion_key", ".tmp");
-    OPEN_DATADIR2_SUFFIX("keys", "secret_onion_key_ntor", ".tmp");
-    OPEN_DATADIR2("keys", "secret_id_key.old");
-    OPEN_DATADIR2("keys", "secret_onion_key.old");
-    OPEN_DATADIR2("keys", "secret_onion_key_ntor.old");
-
-    OPEN_DATADIR2_SUFFIX("keys", "ed25519_master_id_secret_key", ".tmp");
-    OPEN_DATADIR2_SUFFIX("keys", "ed25519_master_id_secret_key_encrypted",
-                         ".tmp");
-    OPEN_DATADIR2_SUFFIX("keys", "ed25519_master_id_public_key", ".tmp");
-    OPEN_DATADIR2_SUFFIX("keys", "ed25519_signing_secret_key", ".tmp");
-    OPEN_DATADIR2_SUFFIX("keys", "ed25519_signing_secret_key_encrypted",
-                         ".tmp");
-    OPEN_DATADIR2_SUFFIX("keys", "ed25519_signing_public_key", ".tmp");
-    OPEN_DATADIR2_SUFFIX("keys", "ed25519_signing_cert", ".tmp");
+    OPEN_KEYDIR_SUFFIX("secret_id_key", ".tmp");
+    OPEN_KEYDIR_SUFFIX("secret_onion_key", ".tmp");
+    OPEN_KEYDIR_SUFFIX("secret_onion_key_ntor", ".tmp");
+    OPEN_KEYDIR("secret_id_key.old");
+    OPEN_KEYDIR("secret_onion_key.old");
+    OPEN_KEYDIR("secret_onion_key_ntor.old");
+
+    OPEN_KEYDIR_SUFFIX("ed25519_master_id_secret_key", ".tmp");
+    OPEN_KEYDIR_SUFFIX("ed25519_master_id_secret_key_encrypted", ".tmp");
+    OPEN_KEYDIR_SUFFIX("ed25519_master_id_public_key", ".tmp");
+    OPEN_KEYDIR_SUFFIX("ed25519_signing_secret_key", ".tmp");
+    OPEN_KEYDIR_SUFFIX("ed25519_signing_secret_key_encrypted", ".tmp");
+    OPEN_KEYDIR_SUFFIX("ed25519_signing_public_key", ".tmp");
+    OPEN_KEYDIR_SUFFIX("ed25519_signing_cert", ".tmp");
 
     OPEN_DATADIR2_SUFFIX("stats", "bridge-stats", ".tmp");
     OPEN_DATADIR2_SUFFIX("stats", "dirreq-stats", ".tmp");
@@ -3760,11 +3793,13 @@ sandbox_init_filter(void)
     OPEN("/etc/resolv.conf");
 
     RENAME_SUFFIX("fingerprint", ".tmp");
-    RENAME_SUFFIX2("keys", "secret_onion_key_ntor", ".tmp");
-    RENAME_SUFFIX2("keys", "secret_id_key", ".tmp");
-    RENAME_SUFFIX2("keys", "secret_id_key.old", ".tmp");
-    RENAME_SUFFIX2("keys", "secret_onion_key", ".tmp");
-    RENAME_SUFFIX2("keys", "secret_onion_key.old", ".tmp");
+    RENAME_KEYDIR_SUFFIX("secret_onion_key_ntor", ".tmp");
+
+    RENAME_KEYDIR_SUFFIX("secret_id_key", ".tmp");
+    RENAME_KEYDIR_SUFFIX("secret_id_key.old", ".tmp");
+    RENAME_KEYDIR_SUFFIX("secret_onion_key", ".tmp");
+    RENAME_KEYDIR_SUFFIX("secret_onion_key.old", ".tmp");
+
     RENAME_SUFFIX2("stats", "bridge-stats", ".tmp");
     RENAME_SUFFIX2("stats", "dirreq-stats", ".tmp");
     RENAME_SUFFIX2("stats", "entry-stats", ".tmp");
@@ -3775,20 +3810,20 @@ sandbox_init_filter(void)
     RENAME_SUFFIX("hashed-fingerprint", ".tmp");
     RENAME_SUFFIX("router-stability", ".tmp");
 
-    RENAME_SUFFIX2("keys", "ed25519_master_id_secret_key", ".tmp");
-    RENAME_SUFFIX2("keys", "ed25519_master_id_secret_key_encrypted", ".tmp");
-    RENAME_SUFFIX2("keys", "ed25519_master_id_public_key", ".tmp");
-    RENAME_SUFFIX2("keys", "ed25519_signing_secret_key", ".tmp");
-    RENAME_SUFFIX2("keys", "ed25519_signing_cert", ".tmp");
+    RENAME_KEYDIR_SUFFIX("ed25519_master_id_secret_key", ".tmp");
+    RENAME_KEYDIR_SUFFIX("ed25519_master_id_secret_key_encrypted", ".tmp");
+    RENAME_KEYDIR_SUFFIX("ed25519_master_id_public_key", ".tmp");
+    RENAME_KEYDIR_SUFFIX("ed25519_signing_secret_key", ".tmp");
+    RENAME_KEYDIR_SUFFIX("ed25519_signing_cert", ".tmp");
 
     sandbox_cfg_allow_rename(&cfg,
-             get_datadir_fname2("keys", "secret_onion_key"),
-             get_datadir_fname2("keys", "secret_onion_key.old"));
+             get_keydir_fname("secret_onion_key"),
+             get_keydir_fname("secret_onion_key.old"));
     sandbox_cfg_allow_rename(&cfg,
-             get_datadir_fname2("keys", "secret_onion_key_ntor"),
-             get_datadir_fname2("keys", "secret_onion_key_ntor.old"));
+             get_keydir_fname("secret_onion_key_ntor"),
+             get_keydir_fname("secret_onion_key_ntor.old"));
 
-    STAT_DATADIR("keys");
+    STAT_KEY_DIRECTORY();
     OPEN_DATADIR("stats");
     STAT_DATADIR("stats");
     STAT_DATADIR2("stats", "dirreq-stats");
diff --git a/src/or/microdesc.c b/src/or/microdesc.c
index fe327c6c82..d8a4660af1 100644
--- a/src/or/microdesc.c
+++ b/src/or/microdesc.c
@@ -238,8 +238,8 @@ get_microdesc_cache_noload(void)
   if (PREDICT_UNLIKELY(the_microdesc_cache==NULL)) {
     microdesc_cache_t *cache = tor_malloc_zero(sizeof(*cache));
     HT_INIT(microdesc_map, &cache->map);
-    cache->cache_fname = get_datadir_fname("cached-microdescs");
-    cache->journal_fname = get_datadir_fname("cached-microdescs.new");
+    cache->cache_fname = get_cachedir_fname("cached-microdescs");
+    cache->journal_fname = get_cachedir_fname("cached-microdescs.new");
     the_microdesc_cache = cache;
   }
   return the_microdesc_cache;
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index caf771193c..d426d4858e 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -197,7 +197,7 @@ networkstatus_read_cached_consensus_impl(int flav,
     tor_snprintf(buf, sizeof(buf), "%s-%s-consensus", prefix, flavorname);
   }
 
-  char *filename = get_datadir_fname(buf);
+  char *filename = get_cachedir_fname(buf);
   char *result = read_file_to_str(filename, RFTS_IGNORE_MISSING, NULL);
   tor_free(filename);
   return result;
@@ -1794,15 +1794,15 @@ networkstatus_set_current_consensus(const char *consensus,
   }
 
   if (!strcmp(flavor, "ns")) {
-    consensus_fname = get_datadir_fname("cached-consensus");
-    unverified_fname = get_datadir_fname("unverified-consensus");
+    consensus_fname = get_cachedir_fname("cached-consensus");
+    unverified_fname = get_cachedir_fname("unverified-consensus");
     if (current_ns_consensus) {
       current_digests = &current_ns_consensus->digests;
       current_valid_after = current_ns_consensus->valid_after;
     }
   } else if (!strcmp(flavor, "microdesc")) {
-    consensus_fname = get_datadir_fname("cached-microdesc-consensus");
-    unverified_fname = get_datadir_fname("unverified-microdesc-consensus");
+    consensus_fname = get_cachedir_fname("cached-microdesc-consensus");
+    unverified_fname = get_cachedir_fname("unverified-microdesc-consensus");
     if (current_md_consensus) {
       current_digests = &current_md_consensus->digests;
       current_valid_after = current_md_consensus->valid_after;
@@ -1811,9 +1811,9 @@ networkstatus_set_current_consensus(const char *consensus,
     cached_dir_t *cur;
     char buf[128];
     tor_snprintf(buf, sizeof(buf), "cached-%s-consensus", flavor);
-    consensus_fname = get_datadir_fname(buf);
+    consensus_fname = get_cachedir_fname(buf);
     tor_snprintf(buf, sizeof(buf), "unverified-%s-consensus", flavor);
-    unverified_fname = get_datadir_fname(buf);
+    unverified_fname = get_cachedir_fname(buf);
     cur = dirserv_get_consensus(flavor);
     if (cur) {
       current_digests = &cur->digests;
@@ -2265,7 +2265,6 @@ void
 networkstatus_dump_bridge_status_to_file(time_t now)
 {
   char *status = networkstatus_getinfo_by_purpose("bridge", now);
-  const or_options_t *options = get_options();
   char *fname = NULL;
   char *thresholds = NULL;
   char *published_thresholds_and_status = NULL;
@@ -2287,8 +2286,7 @@ networkstatus_dump_bridge_status_to_file(time_t now)
                "published %s\nflag-thresholds %s\n%s%s",
                published, thresholds, fingerprint_line ? fingerprint_line : "",
                status);
-  tor_asprintf(&fname, "%s"PATH_SEPARATOR"networkstatus-bridges",
-               options->DataDirectory);
+  fname = get_datadir_fname("networkstatus-bridges");
   write_str_to_file(fname,published_thresholds_and_status,0);
   tor_free(thresholds);
   tor_free(published_thresholds_and_status);
diff --git a/src/or/or.h b/src/or/or.h
index fa5268ac59..e10decf734 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -3641,8 +3641,21 @@ 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 *KeyDirectory_option; /**< Where to store keys, as
+                               * configured by the user. */
+  char *KeyDirectory; /**< Where to store keys data, as modified. */
+  int KeyDirectoryGroupReadable; /**< Boolean: Is the KeyDirectory g+r? */
+
+  char *CacheDirectory_option; /**< Where to store cached data, as
+                               * configured by the user. */
+  char *CacheDirectory; /**< Where to store cached data, as modified. */
+  int CacheDirectoryGroupReadable; /**< Boolean: Is the CacheDirectory g+r? */
+
   char *Nickname; /**< OR only: nickname of this onion router. */
   char *Address; /**< OR only: configured address for this onion router. */
   char *PidFile; /**< Where to store PID of Tor process. */
diff --git a/src/or/router.c b/src/or/router.c
index 010ee339a0..cd1848ee7b 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -174,7 +174,7 @@ expire_old_onion_keys(void)
 
   tor_mutex_release(key_lock);
 
-  fname = get_datadir_fname2("keys", "secret_onion_key.old");
+  fname = get_keydir_fname("secret_onion_key.old");
   if (file_status(fname) == FN_FILE) {
     if (tor_unlink(fname) != 0) {
       log_warn(LD_FS, "Couldn't unlink old onion key file %s: %s",
@@ -183,7 +183,7 @@ expire_old_onion_keys(void)
   }
   tor_free(fname);
 
-  fname = get_datadir_fname2("keys", "secret_onion_key_ntor.old");
+  fname = get_keydir_fname("secret_onion_key_ntor.old");
   if (file_status(fname) == FN_FILE) {
     if (tor_unlink(fname) != 0) {
       log_warn(LD_FS, "Couldn't unlink old ntor onion key file %s: %s",
@@ -378,8 +378,8 @@ rotate_onion_key(void)
   or_state_t *state = get_or_state();
   curve25519_keypair_t new_curve25519_keypair;
   time_t now;
-  fname = get_datadir_fname2("keys", "secret_onion_key");
-  fname_prev = get_datadir_fname2("keys", "secret_onion_key.old");
+  fname = get_keydir_fname("secret_onion_key");
+  fname_prev = get_keydir_fname("secret_onion_key.old");
   /* There isn't much point replacing an old key with an empty file */
   if (file_status(fname) == FN_FILE) {
     if (replace_file(fname, fname_prev))
@@ -399,8 +399,8 @@ rotate_onion_key(void)
   }
   tor_free(fname);
   tor_free(fname_prev);
-  fname = get_datadir_fname2("keys", "secret_onion_key_ntor");
-  fname_prev = get_datadir_fname2("keys", "secret_onion_key_ntor.old");
+  fname = get_keydir_fname("secret_onion_key_ntor");
+  fname_prev = get_keydir_fname("secret_onion_key_ntor.old");
   if (curve25519_keypair_generate(&new_curve25519_keypair, 1) < 0)
     goto error;
   /* There isn't much point replacing an old key with an empty file */
@@ -624,7 +624,7 @@ load_authority_keyset(int legacy, crypto_pk_t **key_out,
   crypto_pk_t *signing_key = NULL;
   authority_cert_t *parsed = NULL;
 
-  fname = get_datadir_fname2("keys",
+  fname = get_keydir_fname(
                  legacy ? "legacy_signing_key" : "authority_signing_key");
   signing_key = init_key_from_file(fname, 0, LOG_ERR, 0);
   if (!signing_key) {
@@ -632,7 +632,7 @@ load_authority_keyset(int legacy, crypto_pk_t **key_out,
     goto done;
   }
   tor_free(fname);
-  fname = get_datadir_fname2("keys",
+  fname = get_keydir_fname(
                legacy ? "legacy_certificate" : "authority_certificate");
   cert = read_file_to_str(fname, 0, NULL);
   if (!cert) {
@@ -932,22 +932,9 @@ init_keys(void)
   }
   if (init_keys_common() < 0)
     return -1;
-  /* Make sure DataDirectory exists, and is private. */
-  cpd_check_t cpd_opts = CPD_CREATE;
-  if (options->DataDirectoryGroupReadable)
-    cpd_opts |= CPD_GROUP_READ;
-  if (check_private_dir(options->DataDirectory, cpd_opts, options->User)) {
-    log_err(LD_OR, "Can't create/check datadirectory %s",
-            options->DataDirectory);
-    return -1;
-  }
-  /* Check the key directory. */
-  keydir = get_datadir_fname("keys");
-  if (check_private_dir(keydir, CPD_CREATE, options->User)) {
-    tor_free(keydir);
+
+  if (create_keys_directory(options) < 0)
     return -1;
-  }
-  tor_free(keydir);
 
   /* 1a. Read v3 directory authority key/cert information. */
   memset(v3_digest, 0, sizeof(v3_digest));
@@ -971,7 +958,7 @@ init_keys(void)
   }
 
   /* 1b. Read identity key. Make it if none is found. */
-  keydir = get_datadir_fname2("keys", "secret_id_key");
+  keydir = get_keydir_fname("secret_id_key");
   log_info(LD_GENERAL,"Reading/making identity key \"%s\"...",keydir);
   prkey = init_key_from_file(keydir, 1, LOG_ERR, 1);
   tor_free(keydir);
@@ -999,7 +986,7 @@ init_keys(void)
     return -1;
 
   /* 2. Read onion key.  Make it if none is found. */
-  keydir = get_datadir_fname2("keys", "secret_onion_key");
+  keydir = get_keydir_fname("secret_onion_key");
   log_info(LD_GENERAL,"Reading/making onion key \"%s\"...",keydir);
   prkey = init_key_from_file(keydir, 1, LOG_ERR, 1);
   tor_free(keydir);
@@ -1024,7 +1011,7 @@ init_keys(void)
     }
   }
 
-  keydir = get_datadir_fname2("keys", "secret_onion_key.old");
+  keydir = get_keydir_fname("secret_onion_key.old");
   if (!lastonionkey && file_status(keydir) == FN_FILE) {
     /* Load keys from non-empty files only.
      * Missing old keys won't be replaced with freshly generated keys. */
@@ -1037,14 +1024,14 @@ init_keys(void)
   {
     /* 2b. Load curve25519 onion keys. */
     int r;
-    keydir = get_datadir_fname2("keys", "secret_onion_key_ntor");
+    keydir = get_keydir_fname("secret_onion_key_ntor");
     r = init_curve25519_keypair_from_file(&curve25519_onion_key,
                                           keydir, 1, LOG_ERR, "onion");
     tor_free(keydir);
     if (r<0)
       return -1;
 
-    keydir = get_datadir_fname2("keys", "secret_onion_key_ntor.old");
+    keydir = get_keydir_fname("secret_onion_key_ntor.old");
     if (tor_mem_is_zero((const char *)
                            last_curve25519_onion_key.pubkey.public_key,
                         CURVE25519_PUBKEY_LEN) &&
diff --git a/src/or/routerkeys.c b/src/or/routerkeys.c
index 7295c19653..af230f07bf 100644
--- a/src/or/routerkeys.c
+++ b/src/or/routerkeys.c
@@ -718,7 +718,7 @@ load_ed_keys(const or_options_t *options, time_t now)
   /* First try to get the signing key to see how it is. */
   {
     char *fname =
-      options_get_datadir_fname2(options, "keys", "ed25519_signing");
+      options_get_keydir_fname(options, "ed25519_signing");
     sign = ed_key_init_from_file(
                fname,
                INIT_ED_KEY_NEEDCERT|
@@ -813,26 +813,15 @@ load_ed_keys(const or_options_t *options, time_t now)
       flags |= INIT_ED_KEY_TRY_ENCRYPTED;
 
     /* Check/Create the key directory */
-    cpd_check_t cpd_opts = CPD_CREATE;
-    if (options->DataDirectoryGroupReadable)
-      cpd_opts |= CPD_GROUP_READ;
-    if (check_private_dir(options->DataDirectory, cpd_opts, options->User)) {
-      log_err(LD_OR, "Can't create/check datadirectory %s",
-              options->DataDirectory);
-      goto err;
-    }
-    char *fname = get_datadir_fname("keys");
-    if (check_private_dir(fname, CPD_CREATE, options->User) < 0) {
-      log_err(LD_OR, "Problem creating/checking key directory %s", fname);
-      tor_free(fname);
-      goto err;
-    }
-    tor_free(fname);
+    if (create_keys_directory(options) < 0)
+      return -1;
+
+    char *fname;
     if (options->master_key_fname) {
       fname = tor_strdup(options->master_key_fname);
       flags |= INIT_ED_KEY_EXPLICIT_FNAME;
     } else {
-      fname = options_get_datadir_fname2(options, "keys", "ed25519_master_id");
+      fname = options_get_keydir_fname(options, "ed25519_master_id");
     }
     id = ed_key_init_from_file(
              fname,
@@ -852,8 +841,8 @@ load_ed_keys(const or_options_t *options, time_t now)
         id = tor_malloc_zero(sizeof(*id));
         memcpy(&id->pubkey, &check_signing_cert->signing_key,
                sizeof(ed25519_public_key_t));
-        fname = options_get_datadir_fname2(options, "keys",
-                                           "ed25519_master_id_public_key");
+        fname = options_get_keydir_fname(options,
+                                         "ed25519_master_id_public_key");
         if (ed25519_pubkey_write_to_file(&id->pubkey, fname, "type0") < 0) {
           log_warn(LD_OR, "Error while attempting to write master public key "
                    "to disk");
@@ -894,7 +883,7 @@ load_ed_keys(const or_options_t *options, time_t now)
                       INIT_ED_KEY_NEEDCERT|
                       INIT_ED_KEY_INCLUDE_SIGNING_KEY_IN_CERT);
     char *fname =
-      options_get_datadir_fname2(options, "keys", "ed25519_signing");
+      options_get_keydir_fname(options, "ed25519_signing");
     ed25519_keypair_free(sign);
     tor_cert_free(sign_cert);
     sign = ed_key_init_from_file(fname,
@@ -1185,7 +1174,7 @@ log_master_signing_key_cert_expiration(const or_options_t *options)
   int failed = 0;
   time_t now = approx_time();
 
-  fn = options_get_datadir_fname2(options, "keys", "ed25519_signing_cert");
+  fn = options_get_keydir_fname(options, "ed25519_signing_cert");
 
   /* Try to grab our cached copy of the key. */
   signing_key = get_master_signing_key_cert();
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index e92de77cc7..93c75ef583 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -473,7 +473,7 @@ trusted_dirs_reload_certs(void)
   char *contents;
   int r;
 
-  filename = get_datadir_fname("cached-certs");
+  filename = get_cachedir_fname("cached-certs");
   contents = read_file_to_str(filename, RFTS_IGNORE_MISSING, NULL);
   tor_free(filename);
   if (!contents)
@@ -662,7 +662,7 @@ trusted_dirs_flush_certs_to_disk(void)
           });
   } DIGESTMAP_FOREACH_END;
 
-  filename = get_datadir_fname("cached-certs");
+  filename = get_cachedir_fname("cached-certs");
   if (write_chunks_to_file(filename, chunks, 0, 0)) {
     log_warn(LD_FS, "Error writing certificates to disk.");
   }
@@ -1339,7 +1339,7 @@ static int
 signed_desc_append_to_journal(signed_descriptor_t *desc,
                               desc_store_t *store)
 {
-  char *fname = get_datadir_fname_suffix(store->fname_base, ".new");
+  char *fname = get_cachedir_fname_suffix(store->fname_base, ".new");
   const char *body = signed_descriptor_get_body_impl(desc,1);
   size_t len = desc->signed_descriptor_len + desc->annotations_len;
 
@@ -1410,8 +1410,8 @@ router_rebuild_store(int flags, desc_store_t *store)
 
   log_info(LD_DIR, "Rebuilding %s cache", store->description);
 
-  fname = get_datadir_fname(store->fname_base);
-  fname_tmp = get_datadir_fname_suffix(store->fname_base, ".tmp");
+  fname = get_cachedir_fname(store->fname_base);
+  fname_tmp = get_cachedir_fname_suffix(store->fname_base, ".tmp");
 
   chunk_list = smartlist_new();
 
@@ -1508,7 +1508,7 @@ router_rebuild_store(int flags, desc_store_t *store)
   } SMARTLIST_FOREACH_END(sd);
 
   tor_free(fname);
-  fname = get_datadir_fname_suffix(store->fname_base, ".new");
+  fname = get_cachedir_fname_suffix(store->fname_base, ".new");
   write_str_to_file(fname, "", 1);
 
   r = 0;
@@ -1538,7 +1538,7 @@ router_reload_router_list_impl(desc_store_t *store)
   int extrainfo = (store->type == EXTRAINFO_STORE);
   store->journal_len = store->store_len = 0;
 
-  fname = get_datadir_fname(store->fname_base);
+  fname = get_cachedir_fname(store->fname_base);
 
   if (store->mmap) {
     /* get rid of it first */
@@ -1565,7 +1565,7 @@ router_reload_router_list_impl(desc_store_t *store)
   }
 
   tor_free(fname);
-  fname = get_datadir_fname_suffix(store->fname_base, ".new");
+  fname = get_cachedir_fname_suffix(store->fname_base, ".new");
   /* don't load empty files - we wouldn't get any data, even if we tried */
   if (file_status(fname) == FN_FILE)
     contents = read_file_to_str(fname, RFTS_BIN|RFTS_IGNORE_MISSING, &st);
diff --git a/src/test/test_conscache.c b/src/test/test_conscache.c
index ddb1bc53c1..ffec3149b0 100644
--- a/src/test/test_conscache.c
+++ b/src/test/test_conscache.c
@@ -31,8 +31,8 @@ test_conscache_simple_usage(void *arg)
 
   /* Make a temporary datadir for these tests */
   char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cache"));
-  tor_free(get_options_mutable()->DataDirectory);
-  get_options_mutable()->DataDirectory = tor_strdup(ddir_fname);
+  tor_free(get_options_mutable()->CacheDirectory);
+  get_options_mutable()->CacheDirectory = tor_strdup(ddir_fname);
   check_private_dir(ddir_fname, CPD_CREATE, NULL);
   consensus_cache_t *cache = consensus_cache_open("cons", 128);
 
@@ -124,8 +124,8 @@ test_conscache_cleanup(void *arg)
 
   /* Make a temporary datadir for these tests */
   char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cache"));
-  tor_free(get_options_mutable()->DataDirectory);
-  get_options_mutable()->DataDirectory = tor_strdup(ddir_fname);
+  tor_free(get_options_mutable()->CacheDirectory);
+  get_options_mutable()->CacheDirectory = tor_strdup(ddir_fname);
   check_private_dir(ddir_fname, CPD_CREATE, NULL);
   consensus_cache_t *cache = consensus_cache_open("cons", 128);
 
@@ -267,8 +267,8 @@ test_conscache_filter(void *arg)
 
   /* Make a temporary datadir for these tests */
   char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cache"));
-  tor_free(get_options_mutable()->DataDirectory);
-  get_options_mutable()->DataDirectory = tor_strdup(ddir_fname);
+  tor_free(get_options_mutable()->CacheDirectory);
+  get_options_mutable()->CacheDirectory = tor_strdup(ddir_fname);
   check_private_dir(ddir_fname, CPD_CREATE, NULL);
   consensus_cache_t *cache = consensus_cache_open("cons", 128);
 
diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c
index 80d3f943ab..a9a4b6a98e 100644
--- a/src/test/test_consdiffmgr.c
+++ b/src/test/test_consdiffmgr.c
@@ -24,8 +24,8 @@ consdiffmgr_test_setup(const struct testcase_t *arg)
 {
   (void)arg;
   char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cdm"));
-  tor_free(get_options_mutable()->DataDirectory);
-  get_options_mutable()->DataDirectory = ddir_fname; // now owns the pointer.
+  tor_free(get_options_mutable()->CacheDirectory);
+  get_options_mutable()->CacheDirectory = ddir_fname; // now owns the pointer.
   check_private_dir(ddir_fname, CPD_CREATE, NULL);
 
   consdiff_cfg_t consdiff_cfg = { 300 };
@@ -215,8 +215,8 @@ test_consdiffmgr_init_failure(void *arg)
   /* As in ...test_setup, but do not create the datadir. The missing directory
    * will cause a failure. */
   char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cdm"));
-  tor_free(get_options_mutable()->DataDirectory);
-  get_options_mutable()->DataDirectory = ddir_fname; // now owns the pointer.
+  tor_free(get_options_mutable()->CacheDirectory);
+  get_options_mutable()->CacheDirectory = ddir_fname; // now owns the pointer.
 
   consdiff_cfg_t consdiff_cfg = { 7200, 300 };
 
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index ee4a9780b1..83734ccfd9 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -4874,9 +4874,11 @@ mock_check_private_dir(const char *dirname, cpd_check_t check,
 
 static char *
 mock_get_datadir_fname(const or_options_t *options,
+                       directory_root_t roottype,
                        const char *sub1, const char *sub2,
                        const char *suffix)
 {
+  (void) roottype;
   char *rv = NULL;
 
   /*
@@ -5033,7 +5035,7 @@ test_dir_dump_unparseable_descriptors(void *data)
   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(options_get_dir_fname2_suffix,
        mock_get_datadir_fname);
 
   /*
@@ -5551,7 +5553,7 @@ test_dir_dump_unparseable_descriptors(void *data)
   mock_unlink_reset();
   UNMOCK(write_str_to_file);
   mock_write_str_to_file_reset();
-  UNMOCK(options_get_datadir_fname2_suffix);
+  UNMOCK(options_get_dir_fname2_suffix);
   UNMOCK(check_private_dir);
   UNMOCK(get_options);
   tor_free(mock_options);
diff --git a/src/test/test_dir_handle_get.c b/src/test/test_dir_handle_get.c
index fe26657ad8..28ecc14c49 100644
--- a/src/test/test_dir_handle_get.c
+++ b/src/test/test_dir_handle_get.c
@@ -469,6 +469,7 @@ init_mock_options(void)
   memset(mock_options, 0, sizeof(or_options_t));
   mock_options->TestingTorNetwork = 1;
   mock_options->DataDirectory = tor_strdup(get_fname_rnd("datadir_tmp"));
+  mock_options->CacheDirectory = tor_strdup(mock_options->DataDirectory);
   check_private_dir(mock_options->DataDirectory, CPD_CREATE, NULL);
 }
 
diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c
index 4f0ecd778b..59b28f7580 100644
--- a/src/test/test_microdesc.c
+++ b/src/test/test_microdesc.c
@@ -69,12 +69,12 @@ test_md_cache(void *data)
   time3 = time(NULL) - 15*24*60*60;
 
   /* Possibly, turn this into a test setup/cleanup pair */
-  tor_free(options->DataDirectory);
-  options->DataDirectory = tor_strdup(get_fname("md_datadir_test"));
+  tor_free(options->CacheDirectory);
+  options->CacheDirectory = tor_strdup(get_fname("md_datadir_test"));
 #ifdef _WIN32
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory));
 #else
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory, 0700));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory, 0700));
 #endif
 
   tt_assert(!strcmpstart(test_md3_noannotation, "onion-key"));
@@ -152,7 +152,7 @@ test_md_cache(void *data)
               strlen(test_md3_noannotation));
 
   tor_asprintf(&fn, "%s"PATH_SEPARATOR"cached-microdescs.new",
-               options->DataDirectory);
+               options->CacheDirectory);
   s = read_file_to_str(fn, RFTS_BIN, NULL);
   tt_assert(s);
   tt_mem_op(md1->body, OP_EQ, s + md1->off, md1->bodylen);
@@ -180,7 +180,7 @@ test_md_cache(void *data)
 
   /* read the cache. */
   tor_asprintf(&fn, "%s"PATH_SEPARATOR"cached-microdescs",
-               options->DataDirectory);
+               options->CacheDirectory);
   s = read_file_to_str(fn, RFTS_BIN, NULL);
   tt_mem_op(md1->body, OP_EQ, s + md1->off, strlen(test_md1));
   tt_mem_op(md2->body, OP_EQ, s + md2->off, strlen(test_md2));
@@ -234,7 +234,7 @@ test_md_cache(void *data)
 
  done:
   if (options)
-    tor_free(options->DataDirectory);
+    tor_free(options->CacheDirectory);
   microdesc_free_all();
 
   smartlist_free(added);
@@ -266,17 +266,17 @@ test_md_cache_broken(void *data)
 
   options = get_options_mutable();
   tt_assert(options);
-  tor_free(options->DataDirectory);
-  options->DataDirectory = tor_strdup(get_fname("md_datadir_test2"));
+  tor_free(options->CacheDirectory);
+  options->CacheDirectory = tor_strdup(get_fname("md_datadir_test2"));
 
 #ifdef _WIN32
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory));
 #else
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory, 0700));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory, 0700));
 #endif
 
   tor_asprintf(&fn, "%s"PATH_SEPARATOR"cached-microdescs",
-               options->DataDirectory);
+               options->CacheDirectory);
 
   write_str_to_file(fn, truncated_md, 1);
 
@@ -285,7 +285,7 @@ test_md_cache_broken(void *data)
 
  done:
   if (options)
-    tor_free(options->DataDirectory);
+    tor_free(options->CacheDirectory);
   tor_free(fn);
   microdesc_free_all();
 }
@@ -754,8 +754,8 @@ test_md_reject_cache(void *arg)
   or_options_t *options = get_options_mutable();
   char buf[DIGEST256_LEN];
 
-  tor_free(options->DataDirectory);
-  options->DataDirectory = tor_strdup(get_fname("md_datadir_test_rej"));
+  tor_free(options->CacheDirectory);
+  options->CacheDirectory = tor_strdup(get_fname("md_datadir_test_rej"));
   mock_rgsbd_val_a = tor_malloc_zero(sizeof(routerstatus_t));
   mock_rgsbd_val_b = tor_malloc_zero(sizeof(routerstatus_t));
   mock_ns_val = tor_malloc_zero(sizeof(networkstatus_t));
@@ -765,9 +765,9 @@ test_md_reject_cache(void *arg)
   mock_ns_val->flavor = FLAV_MICRODESC;
 
 #ifdef _WIN32
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory));
 #else
-  tt_int_op(0, OP_EQ, mkdir(options->DataDirectory, 0700));
+  tt_int_op(0, OP_EQ, mkdir(options->CacheDirectory, 0700));
 #endif
 
   MOCK(router_get_mutable_consensus_status_by_descriptor_digest,
@@ -802,7 +802,7 @@ test_md_reject_cache(void *arg)
  done:
   UNMOCK(networkstatus_get_latest_consensus_by_flavor);
   UNMOCK(router_get_mutable_consensus_status_by_descriptor_digest);
-  tor_free(options->DataDirectory);
+  tor_free(options->CacheDirectory);
   microdesc_free_all();
   smartlist_free(added);
   SMARTLIST_FOREACH(wanted, char *, cp, tor_free(cp));
diff --git a/src/test/test_routerkeys.c b/src/test/test_routerkeys.c
index d8b72651a0..e4abcdb92d 100644
--- a/src/test/test_routerkeys.c
+++ b/src/test/test_routerkeys.c
@@ -421,6 +421,7 @@ test_routerkeys_ed_keys_init_all(void *arg)
 {
   (void)arg;
   char *dir = tor_strdup(get_fname("test_ed_keys_init_all"));
+  char *keydir = tor_strdup(get_fname("test_ed_keys_init_all/KEYS"));
   or_options_t *options = tor_malloc_zero(sizeof(or_options_t));
   time_t now = time(NULL);
   ed25519_public_key_t id;
@@ -445,13 +446,14 @@ test_routerkeys_ed_keys_init_all(void *arg)
 
 #ifdef _WIN32
   mkdir(dir);
-  mkdir(get_fname("test_ed_keys_init_all/keys"));
+  mkdir(keydir);
 #else
   mkdir(dir, 0700);
-  mkdir(get_fname("test_ed_keys_init_all/keys"), 0700);
+  mkdir(keydir, 0700);
 #endif /* defined(_WIN32) */
 
   options->DataDirectory = dir;
+  options->KeyDirectory = keydir;
 
   tt_int_op(1, OP_EQ, load_ed_keys(options, now));
   tt_int_op(0, OP_EQ, generate_ed_link_cert(options, now, 0));
@@ -521,7 +523,7 @@ test_routerkeys_ed_keys_init_all(void *arg)
 
   /* Demonstrate that we can start up with no secret identity key */
   routerkeys_free_all();
-  unlink(get_fname("test_ed_keys_init_all/keys/"
+  unlink(get_fname("test_ed_keys_init_all/KEYS/"
                    "ed25519_master_id_secret_key"));
   tt_int_op(1, OP_EQ, load_ed_keys(options, now));
   tt_int_op(0, OP_EQ, generate_ed_link_cert(options, now, 0));
@@ -542,6 +544,7 @@ test_routerkeys_ed_keys_init_all(void *arg)
 
  done:
   tor_free(dir);
+  tor_free(keydir);
   tor_free(options);
   tor_cert_free(link_cert);
   routerkeys_free_all();
diff --git a/src/test/testing_common.c b/src/test/testing_common.c
index e43fa46c84..142c681079 100644
--- a/src/test/testing_common.c
+++ b/src/test/testing_common.c
@@ -293,6 +293,9 @@ main(int c, const char **v)
   setup_directory();
   options_init(options);
   options->DataDirectory = tor_strdup(temp_dir);
+  tor_asprintf(&options->KeyDirectory, "%s"PATH_SEPARATOR"keys",
+               options->DataDirectory);
+  options->CacheDirectory = tor_strdup(temp_dir);
   options->EntryStatistics = 1;
   if (set_options(options, &errmsg) < 0) {
     printf("Failed to set initial options: %s\n", errmsg);