[tor-commits] [tor/release-0.4.5] Make SAVECONF keep only one backup and add sandbox rules for it. #40317

[nickm at torproject.org]

commit e0a84546916bdf1ffb3058970eee5011484980f5
Author: Daniel Pinto <danielpinto52 at gmail.com>
Date:   Wed Apr 7 23:46:59 2021 +0100

    Make SAVECONF keep only one backup and add sandbox rules for it. #40317
    
    When seccomp sandbox is active, SAVECONF failed because it was not
    able to save the backup files for torrc. This commit simplifies
    the implementation of SAVECONF and sandbox by making it keep only
    one backup of the configuration file.
---
 changes/bug40317        |  5 +++++
 src/app/config/config.c | 17 +++++++++--------
 src/app/config/config.h |  3 +++
 src/app/main/main.c     | 22 +++++++++++++++++-----
 4 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/changes/bug40317 b/changes/bug40317
new file mode 100644
index 0000000000..18ec499a51
--- /dev/null
+++ b/changes/bug40317
@@ -0,0 +1,5 @@
+  o Minor bugfixes (control, sandbox):
+    - Allows the control command SAVECONF to succeed when the seccomp
+      sandbox is enabled. Makes SAVECONF keep only one backup file to
+      simplify implementation. Fixes bug 40317; bugfix on 0.2.5.4-alpha.
+      Patch by Daniel Pinto.
diff --git a/src/app/config/config.c b/src/app/config/config.c
index fa74907b3d..abc64ee92d 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -6827,7 +6827,7 @@ validate_data_directories(or_options_t *options)
 /** This string can change; it tries to give the reader an idea
  * that editing this file by hand is not a good plan. */
 #define GENERATED_FILE_COMMENT "# The old torrc file was renamed " \
-  "to torrc.orig.1 or similar, and Tor will ignore it"
+  "to torrc.orig.1, and Tor will ignore it"
 
 /** Save a configuration file for the configuration in <b>options</b>
  * into the file <b>fname</b>.  If the file already exists, and
@@ -6871,17 +6871,18 @@ write_configuration_file(const char *fname, const or_options_t *options)
                GENERATED_FILE_PREFIX, GENERATED_FILE_COMMENT, new_conf);
 
   if (rename_old) {
-    int i = 1;
     char *fn_tmp = NULL;
-    while (1) {
-      tor_asprintf(&fn_tmp, "%s.orig.%d", fname, i);
-      if (file_status(fn_tmp) == FN_NOENT)
-        break;
+    tor_asprintf(&fn_tmp, CONFIG_BACKUP_PATTERN, fname);
+    file_status_t fn_tmp_status = file_status(fn_tmp);
+    if (fn_tmp_status == FN_DIR || fn_tmp_status == FN_ERROR) {
+      log_warn(LD_CONFIG,
+               "Config backup file \"%s\" is not a file? Failing.", fn_tmp);
       tor_free(fn_tmp);
-      ++i;
+      goto err;
     }
+
     log_notice(LD_CONFIG, "Renaming old configuration file to \"%s\"", fn_tmp);
-    if (tor_rename(fname, fn_tmp) < 0) {//XXXX sandbox doesn't allow
+    if (replace_file(fname, fn_tmp) < 0) {
       log_warn(LD_FS,
                "Couldn't rename configuration file \"%s\" to \"%s\": %s",
                fname, fn_tmp, strerror(errno));
diff --git a/src/app/config/config.h b/src/app/config/config.h
index e95ef4a728..ee78d1e0f7 100644
--- a/src/app/config/config.h
+++ b/src/app/config/config.h
@@ -44,6 +44,9 @@ int get_protocol_warning_severity_level(void);
 
 #define LOG_PROTOCOL_WARN (get_protocol_warning_severity_level())
 
+/** Pattern for backing up configuration files */
+#define CONFIG_BACKUP_PATTERN "%s.orig.1"
+
 /** An error from options_trial_assign() or options_init_from_string(). */
 typedef enum setopt_err_t {
   SETOPT_OK = 0,
diff --git a/src/app/main/main.c b/src/app/main/main.c
index 56478a0f71..7c6feb77fe 100644
--- a/src/app/main/main.c
+++ b/src/app/main/main.c
@@ -831,7 +831,6 @@ sandbox_init_filter(void)
 {
   const or_options_t *options = get_options();
   sandbox_cfg_t *cfg = sandbox_cfg_new();
-  int i;
 
   sandbox_cfg_allow_openat_filename(&cfg,
       get_cachedir_fname("cached-status"));
@@ -917,10 +916,23 @@ sandbox_init_filter(void)
   else
     sandbox_cfg_allow_open_filename(&cfg, tor_strdup("/etc/resolv.conf"));
 
-  for (i = 0; i < 2; ++i) {
-    if (get_torrc_fname(i)) {
-      sandbox_cfg_allow_open_filename(&cfg, tor_strdup(get_torrc_fname(i)));
-    }
+  const char *torrc_defaults_fname = get_torrc_fname(1);
+  if (torrc_defaults_fname) {
+    sandbox_cfg_allow_open_filename(&cfg, tor_strdup(torrc_defaults_fname));
+  }
+  const char *torrc_fname = get_torrc_fname(0);
+  if (torrc_fname) {
+    sandbox_cfg_allow_open_filename(&cfg, tor_strdup(torrc_fname));
+    // allow torrc backup and torrc.tmp to make SAVECONF work
+    char *torrc_bck = NULL;
+    tor_asprintf(&torrc_bck, CONFIG_BACKUP_PATTERN, torrc_fname);
+    sandbox_cfg_allow_rename(&cfg, tor_strdup(torrc_fname), torrc_bck);
+    char *torrc_tmp = NULL;
+    tor_asprintf(&torrc_tmp, "%s.tmp", torrc_fname);
+    sandbox_cfg_allow_rename(&cfg, torrc_tmp, tor_strdup(torrc_fname));
+    sandbox_cfg_allow_open_filename(&cfg, tor_strdup(torrc_tmp));
+    // we need to stat the existing backup file
+    sandbox_cfg_allow_stat_filename(&cfg, tor_strdup(torrc_bck));
   }
 
   SMARTLIST_FOREACH(options->FilesOpenedByIncludes, char *, f, {