Add 'rename' to the sandboxed syscalls

(If we don't restrict rename, there's not much point in restricting
open, since an attacker could always use rename to make us open
whatever they want.)

src/common/compat.c

@@ -175,6 +175,14 @@ tor_fopen_cloexec(const char *path, const char *mode)
   return result;
 }
 
+/** As rename(), but work correctly with the sandbox. */
+int
+tor_rename(const char *path_old, const char *path_new)
+{
+  return rename(sandbox_intern_string(path_old),
+                sandbox_intern_string(path_new));
+}
+
 #if defined(HAVE_SYS_MMAN_H) || defined(RUNNING_DOXYGEN)
 /** Try to create a memory mapping for <b>filename</b> and return it.  On
  * failure, return NULL.  Sets errno properly, using ERANGE to mean
@@ -799,7 +807,7 @@ int
 replace_file(const char *from, const char *to)
 {
 #ifndef _WIN32
-  return rename(from,to);
+  return tor_rename(from, to);
 #else
   switch (file_status(to))
     {
@@ -814,7 +822,7 @@ replace_file(const char *from, const char *to)
       errno = EISDIR;
       return -1;
     }
-  return rename(from,to);
+  return tor_rename(from,to);
 #endif
 }
 

src/common/compat.h

@@ -410,6 +410,7 @@ struct tm *tor_gmtime_r(const time_t *timep, struct tm *result);
 /* ===== File compatibility */
 int tor_open_cloexec(const char *path, int flags, unsigned mode);
 FILE *tor_fopen_cloexec(const char *path, const char *mode);
+int tor_rename(const char *path_old, const char *path_new);
 
 int replace_file(const char *from, const char *to);
 int touch_file(const char *fname);

src/common/sandbox.c

@@ -122,7 +122,6 @@ static int filter_nopar_gen[] = {
     SCMP_SYS(mmap),
     SCMP_SYS(munmap),
     SCMP_SYS(read),
-    SCMP_SYS(rename),
     SCMP_SYS(rt_sigreturn),
     SCMP_SYS(set_robust_list),
     SCMP_SYS(_sysctl),
@@ -361,6 +360,41 @@ sb_open(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
   return 0;
 }
 
+/**
+ * Function responsible for setting up the rename syscall for
+ * the seccomp filter sandbox.
+ */
+static int
+sb_rename(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
+{
+  int rc;
+  sandbox_cfg_t *elem = NULL;
+
+  // for each dynamic parameter filters
+  for (elem = filter; elem != NULL; elem = elem->next) {
+    smp_param_t *param = elem->param;
+
+    if (param != NULL && param->prot == 1 &&
+        param->syscall == SCMP_SYS(rename)) {
+
+      intptr_t value2 = (intptr_t)(void*)sandbox_intern_string(
+                                              (char*)param->value2);
+
+      rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW,
+            SCMP_SYS(rename), 1,
+            SCMP_CMP(0, SCMP_CMP_EQ, param->value),
+            SCMP_CMP(1, SCMP_CMP_EQ, value2));
+      if (rc != 0) {
+        log_err(LD_BUG,"(Sandbox) failed to add rename syscall, received "
+            "libseccomp error %d", rc);
+        return rc;
+      }
+    }
+  }
+
+  return 0;
+}
+
 /**
  * Function responsible for setting up the openat syscall for
  * the seccomp filter sandbox.
@@ -807,6 +841,7 @@ static sandbox_filter_func_t filter_func[] = {
 #endif
     sb_open,
     sb_openat,
+    sb_rename,
 #ifdef __NR_fcntl64
     sb_fcntl64,
 #endif
@@ -888,6 +923,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg)
     size_t param_size = strlen(param_val) + 1;
 
     void *location = strmap_get(locations, param_val);
+
     if (location) {
       // We already interned this string.
       {
@@ -989,22 +1025,30 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg)
  * point.
  */
 static sandbox_cfg_t*
-new_element(int syscall, int index, intptr_t value)
+new_element2(int syscall, int index, int index2, intptr_t value, intptr_t value2)
 {
   smp_param_t *param = NULL;
 
-  sandbox_cfg_t *elem = tor_malloc(sizeof(sandbox_cfg_t));
+  sandbox_cfg_t *elem = tor_malloc_zero(sizeof(sandbox_cfg_t));
-  elem->param = tor_malloc(sizeof(smp_param_t));
+  elem->param = tor_malloc_zero(sizeof(smp_param_t));
 
   param = elem->param;
   param->syscall = syscall;
   param->pindex = index;
+  param->pindex2 = index2;
   param->value = value;
+  param->value2 = value2;
   param->prot = 0;
 
   return elem;
 }
 
+static sandbox_cfg_t*
+new_element(int syscall, int index, intptr_t value)
+{
+  return new_element2(syscall, index, -1, value, 0);
+}
+
 #ifdef __NR_stat64
 #define SCMP_stat SCMP_SYS(stat64)
 #else
@@ -1072,6 +1116,37 @@ sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file, int fr)
   return 0;
 }
 
+int
+sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2)
+{
+  sandbox_cfg_t *elem = NULL;
+
+  elem = new_element2(SCMP_SYS(rename), 0, 1,
+                      (intptr_t)(void *)tor_strdup(file1),
+                      (intptr_t)(void *)tor_strdup(file2));
+
+  if (!elem) {
+    log_err(LD_BUG,"(Sandbox) failed to register parameter!");
+    return -1;
+  }
+
+  elem->next = *cfg;
+  *cfg = elem;
+
+  /* For interning */
+  elem = new_element(-1, 0, (intptr_t)(void*)tor_strdup(file2));
+  if (!elem) {
+    log_err(LD_BUG,"(Sandbox) failed to register parameter!");
+    return -1;
+  }
+  elem->next = *cfg;
+  *cfg = elem;
+
+  tor_free(file1);
+  tor_free(file2);
+  return 0;
+}
+
 int
 sandbox_cfg_allow_open_filename_array(sandbox_cfg_t **cfg, ...)
 {

src/common/sandbox.h

@@ -67,8 +67,12 @@ typedef struct smp_param {
 
   /** parameter index. */
   int pindex;
+  /** parameter index, second one. */
+  int pindex2;
   /** parameter value. */
   intptr_t value;
+  /** parameter value, second argument. */
+  intptr_t value2;
 
   /**  parameter flag (0 = not protected, 1 = protected). */
   int prot;
@@ -174,6 +178,9 @@ sandbox_cfg_t * sandbox_cfg_new(void);
 int sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file,
     int fr);
 
+/**DOCDOC*/
+int sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2);
+
 /** Function used to add a series of open allowed filenames to a supplied
  * configuration.
  *  @param cfg  sandbox configuration.

src/or/config.c

@@ -6276,7 +6276,7 @@ write_configuration_file(const char *fname, const or_options_t *options)
       ++i;
     }
     log_notice(LD_CONFIG, "Renaming old configuration file to \"%s\"", fn_tmp);
-    if (rename(fname, fn_tmp) < 0) {
+    if (tor_rename(fname, fn_tmp) < 0) {//XXXX sandbox doesn't allow
       log_warn(LD_FS,
                "Couldn't rename configuration file \"%s\" to \"%s\": %s",
                fname, fn_tmp, strerror(errno));

src/or/main.c

@@ -2743,7 +2743,6 @@ sandbox_init_filter(void)
       get_datadir_fname("cached-microdescs.tmp"), 1,
       get_datadir_fname("cached-microdescs.new"), 1,
       get_datadir_fname("cached-microdescs.new.tmp"), 1,
-      get_datadir_fname("unverified-microdesc-consensus"), 1,
       get_datadir_fname("cached-descriptors"), 1,
       get_datadir_fname("cached-descriptors.new"), 1,
       get_datadir_fname("cached-descriptors.tmp"), 1,
@@ -2765,6 +2764,34 @@ sandbox_init_filter(void)
       NULL, 0
   );
 
+#define RENAME_SUFFIX(name, suffix)        \
+  sandbox_cfg_allow_rename(&cfg,           \
+      get_datadir_fname(name suffix),      \
+      get_datadir_fname(name))
+
+#define RENAME_SUFFIX2(prefix, name, suffix) \
+  sandbox_cfg_allow_rename(&cfg,                                        \
+                           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");
+  RENAME_SUFFIX("state", ".tmp");
+  RENAME_SUFFIX("unparseable-desc", ".tmp");
+  RENAME_SUFFIX("v3-status-votes", ".tmp");
+
   sandbox_cfg_allow_stat_filename_array(&cfg,
       get_datadir_fname(NULL), 1,
       get_datadir_fname("lock"), 1,
@@ -2790,12 +2817,18 @@ sandbox_init_filter(void)
         get_datadir_fname("fingerprint.tmp"), 1,
         get_datadir_fname("hashed-fingerprint"), 1,
         get_datadir_fname("hashed-fingerprint.tmp"), 1,
-        get_datadir_fname("cached-consensus"), 1,
-        get_datadir_fname("cached-consensus.tmp"), 1,
         "/etc/resolv.conf", 0,
         NULL, 0
     );
 
+    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_SUFFIX("hashed-fingerprint", ".tmp");
+
     sandbox_cfg_allow_stat_filename_array(&cfg,
         get_datadir_fname("keys"), 1,
         get_datadir_fname("stats/dirreq-stats"), 1,

src/or/statefile.c

@@ -13,6 +13,7 @@
 #include "hibernate.h"
 #include "rephist.h"
 #include "router.h"
+#include "sandbox.h"
 #include "statefile.h"
 
 /** A list of state-file "abbreviations," for compatibility. */
@@ -285,7 +286,7 @@ or_state_save_broken(char *fname)
     log_warn(LD_BUG, "Unable to parse state in \"%s\". Moving it aside "
              "to \"%s\".  This could be a bug in Tor; please tell "
              "the developers.", fname, fname2);
-    if (rename(fname, fname2) < 0) {
+    if (tor_rename(fname, fname2) < 0) {//XXXX sandbox prohibits
       log_warn(LD_BUG, "Weirdly, I couldn't even move the state aside. The "
                "OS gave an error of %s", strerror(errno));
     }