Browse Source

Merge branch 'fix-torrcd-sandbox-22605v2'

Nick Mathewson 6 years ago
parent
commit
b76a161e01
7 changed files with 183 additions and 63 deletions
  1. 52 42
      src/common/confline.c
  2. 4 1
      src/common/confline.h
  3. 1 0
      src/common/sandbox.c
  4. 20 2
      src/or/config.c
  5. 4 0
      src/or/main.c
  6. 3 0
      src/or/or.h
  7. 99 18
      src/test/test_config.c

+ 52 - 42
src/common/confline.c

@@ -12,15 +12,18 @@
 
 static int config_get_lines_aux(const char *string, config_line_t **result,
                                 int extended, int allow_include,
-                                int *has_include, int recursion_level,
-                                config_line_t **last);
-static smartlist_t *config_get_file_list(const char *path);
-static int config_get_included_list(const char *path, int recursion_level,
-                                    int extended, config_line_t **list,
-                                    config_line_t **list_last);
+                                int *has_include, smartlist_t *opened_lst,
+                                int recursion_level, config_line_t **last);
+static smartlist_t *config_get_file_list(const char *path,
+                                         smartlist_t *opened_files);
+static int config_get_included_config(const char *path, int recursion_level,
+                                      int extended, config_line_t **config,
+                                      config_line_t **config_last,
+                                      smartlist_t *opened_lst);
 static int config_process_include(const char *path, int recursion_level,
                                   int extended, config_line_t **list,
-                                  config_line_t **list_last);
+                                  config_line_t **list_last,
+                                  smartlist_t *opened_lst);
 
 /** Helper: allocate a new configuration option mapping 'key' to 'val',
  * append it to *<b>lst</b>. */
@@ -80,11 +83,13 @@ config_line_find(const config_line_t *lines,
 
 /** Auxiliary function that does all the work of config_get_lines.
  * <b>recursion_level</b> is the count of how many nested %includes we have.
+ * <b>opened_lst</b> will have a list of opened files if provided.
  * Returns the a pointer to the last element of the <b>result</b> in
  * <b>last</b>. */
 static int
 config_get_lines_aux(const char *string, config_line_t **result, int extended,
-                     int allow_include, int *has_include, int recursion_level,
+                     int allow_include, int *has_include,
+                     smartlist_t *opened_lst, int recursion_level,
                      config_line_t **last)
 {
   config_line_t *list = NULL, **next, *list_last = NULL;
@@ -134,7 +139,7 @@ config_get_lines_aux(const char *string, config_line_t **result, int extended,
 
         config_line_t *include_list;
         if (config_process_include(v, recursion_level, extended, &include_list,
-                                   &list_last) < 0) {
+                                   &list_last, opened_lst) < 0) {
           log_warn(LD_CONFIG, "Error reading included configuration "
                    "file or directory: \"%s\".", v);
           config_free_lines(list);
@@ -176,24 +181,27 @@ config_get_lines_aux(const char *string, config_line_t **result, int extended,
 /** Helper: parse the config string and strdup into key/value
  * strings. Set *result to the list, or NULL if parsing the string
  * failed. Set *has_include to 1 if <b>result</b> has values from
- * %included files.  Return 0 on success, -1 on failure. Warn and ignore any
+ * %included files. <b>opened_lst</b> will have a list of opened files if
+ * provided. Return 0 on success, -1 on failure. Warn and ignore any
  * misformatted lines.
  *
  * If <b>extended</b> is set, then treat keys beginning with / and with + as
  * indicating "clear" and "append" respectively. */
 int
 config_get_lines_include(const char *string, config_line_t **result,
-                         int extended, int *has_include)
+                         int extended, int *has_include,
+                         smartlist_t *opened_lst)
 {
-  return config_get_lines_aux(string, result, extended, 1, has_include, 1,
-                              NULL);
+  return config_get_lines_aux(string, result, extended, 1, has_include,
+                              opened_lst, 1, NULL);
 }
 
 /** Same as config_get_lines_include but does not allow %include */
 int
 config_get_lines(const char *string, config_line_t **result, int extended)
 {
-  return config_get_lines_aux(string, result, extended, 0, NULL, 1, NULL);
+  return config_get_lines_aux(string, result, extended, 0, NULL, NULL, 1,
+                              NULL);
 }
 
 /** Adds a list of configuration files present on <b>path</b> to
@@ -201,12 +209,18 @@ config_get_lines(const char *string, config_line_t **result, int extended)
  * only that file will be added to <b>file_list</b>. If it is a directory,
  * all paths for files on that directory root (no recursion) except for files
  * whose name starts with a dot will be added to <b>file_list</b>.
- * Return 0 on success, -1 on failure. Ignores empty files.
+ * <b>opened_files</b> will have a list of files opened by this function
+ * if provided. Return 0 on success, -1 on failure. Ignores empty files.
  */
 static smartlist_t *
-config_get_file_list(const char *path)
+config_get_file_list(const char *path, smartlist_t *opened_files)
 {
   smartlist_t *file_list = smartlist_new();
+
+  if (opened_files) {
+    smartlist_add_strdup(opened_files, path);
+  }
+
   file_status_t file_type = file_status(path);
   if (file_type == FN_FILE) {
     smartlist_add_strdup(file_list, path);
@@ -228,6 +242,10 @@ config_get_file_list(const char *path)
       tor_asprintf(&fullname, "%s"PATH_SEPARATOR"%s", path, f);
       tor_free(f);
 
+      if (opened_files) {
+        smartlist_add_strdup(opened_files, fullname);
+      }
+
       if (file_status(fullname) != FN_FILE) {
         tor_free(fullname);
         continue;
@@ -245,19 +263,21 @@ config_get_file_list(const char *path)
 }
 
 /** Creates a list of config lines present on included <b>path</b>.
- * Set <b>list</b> to the list and <b>list_last</b> to the last element of
- * <b>list</b>. Return 0 on success, -1 on failure. */
+ * Set <b>config</b> to the list and <b>config_last</b> to the last element of
+ * <b>config</b>. <b>opened_lst</b> will have a list of opened files if
+ * provided. Return 0 on success, -1 on failure. */
 static int
-config_get_included_list(const char *path, int recursion_level, int extended,
-                         config_line_t **list, config_line_t **list_last)
+config_get_included_config(const char *path, int recursion_level, int extended,
+                           config_line_t **config, config_line_t **config_last,
+                           smartlist_t *opened_lst)
 {
   char *included_conf = read_file_to_str(path, 0, NULL);
   if (!included_conf) {
     return -1;
   }
 
-  if (config_get_lines_aux(included_conf, list, extended, 1, NULL,
-                           recursion_level+1, list_last) < 0) {
+  if (config_get_lines_aux(included_conf, config, extended, 1, NULL,
+                           opened_lst, recursion_level+1, config_last) < 0) {
     tor_free(included_conf);
     return -1;
   }
@@ -268,41 +288,31 @@ config_get_included_list(const char *path, int recursion_level, int extended,
 
 /** Process an %include <b>path</b> in a config file. Set <b>list</b> to the
  * list of configuration settings obtained and <b>list_last</b> to the last
- * element of the same list. Return 0 on success, -1 on failure. */
+ * element of the same list. <b>opened_lst</b> will have a list of opened
+ * files if provided. Return 0 on success, -1 on failure. */
 static int
 config_process_include(const char *path, int recursion_level, int extended,
-                       config_line_t **list, config_line_t **list_last)
+                       config_line_t **list, config_line_t **list_last,
+                       smartlist_t *opened_lst)
 {
   config_line_t *ret_list = NULL;
   config_line_t **next = &ret_list;
-#if 0
-  // Disabled -- we already unescape_string() on the result. */
-  char *unquoted_path = get_unquoted_path(path);
-  if (!unquoted_path) {
-    return -1;
-  }
 
-  smartlist_t *config_files = config_get_file_list(unquoted_path);
-  if (!config_files) {
-    tor_free(unquoted_path);
-    return -1;
-  }
-  tor_free(unquoted_path);
-#endif /* 0 */
-  smartlist_t *config_files = config_get_file_list(path);
+  smartlist_t *config_files = config_get_file_list(path, opened_lst);
   if (!config_files) {
     return -1;
   }
 
   int rv = -1;
   SMARTLIST_FOREACH_BEGIN(config_files, const char *, config_file) {
-    config_line_t *included_list = NULL;
-    if (config_get_included_list(config_file, recursion_level, extended,
-                                  &included_list, list_last) < 0) {
+    config_line_t *included_config = NULL;
+    if (config_get_included_config(config_file, recursion_level, extended,
+                                   &included_config, list_last,
+                                   opened_lst) < 0) {
       goto done;
     }
 
-    *next = included_list;
+    *next = included_config;
     if (*list_last)
       next = &(*list_last)->next;
 

+ 4 - 1
src/common/confline.h

@@ -7,6 +7,8 @@
 #ifndef TOR_CONFLINE_H
 #define TOR_CONFLINE_H
 
+#include "container.h"
+
 /** Ordinary configuration line. */
 #define CONFIG_LINE_NORMAL 0
 /** Appends to previous configuration for the same option, even if we
@@ -44,7 +46,8 @@ int config_lines_eq(config_line_t *a, config_line_t *b);
 int config_count_key(const config_line_t *a, const char *key);
 int config_get_lines(const char *string, config_line_t **result, int extended);
 int config_get_lines_include(const char *string, config_line_t **result,
-                             int extended, int *has_include);
+                             int extended, int *has_include,
+                             smartlist_t *opened_lst);
 void config_free_lines(config_line_t *front);
 const char *parse_config_line_from_str_verbose(const char *line,
                                        char **key_out, char **value_out,

+ 1 - 0
src/common/sandbox.c

@@ -151,6 +151,7 @@ static int filter_nopar_gen[] = {
     SCMP_SYS(fstat64),
 #endif
     SCMP_SYS(futex),
+    SCMP_SYS(getdents),
     SCMP_SYS(getdents64),
     SCMP_SYS(getegid),
 #ifdef __NR_getegid32

+ 20 - 2
src/or/config.c

@@ -939,6 +939,10 @@ or_options_free(or_options_t *options)
     SMARTLIST_FOREACH(options->SchedulerTypes_, int *, i, tor_free(i));
     smartlist_free(options->SchedulerTypes_);
   }
+  if (options->FilesOpenedByIncludes) {
+    SMARTLIST_FOREACH(options->FilesOpenedByIncludes, char *, f, tor_free(f));
+    smartlist_free(options->FilesOpenedByIncludes);
+  }
   tor_free(options->BridgePassword_AuthDigest_);
   tor_free(options->command_arg);
   tor_free(options->master_key_fname);
@@ -5273,13 +5277,16 @@ options_init_from_string(const char *cf_defaults, const char *cf,
   newoptions->command = command;
   newoptions->command_arg = command_arg ? tor_strdup(command_arg) : NULL;
 
+  smartlist_t *opened_files = smartlist_new();
   for (int i = 0; i < 2; ++i) {
     const char *body = i==0 ? cf_defaults : cf;
     if (!body)
       continue;
+
     /* get config lines, assign them */
     retval = config_get_lines_include(body, &cl, 1,
-                                      body == cf ? &cf_has_include : NULL);
+                                      body == cf ? &cf_has_include : NULL,
+                                      opened_files);
     if (retval < 0) {
       err = SETOPT_ERR_PARSE;
       goto err;
@@ -5308,6 +5315,7 @@ options_init_from_string(const char *cf_defaults, const char *cf,
   }
 
   newoptions->IncludeUsed = cf_has_include;
+  newoptions->FilesOpenedByIncludes = opened_files;
 
   /* If this is a testing network configuration, change defaults
    * for a list of dependent config options, re-initialize newoptions
@@ -5347,13 +5355,16 @@ options_init_from_string(const char *cf_defaults, const char *cf,
     newoptions->command_arg = command_arg ? tor_strdup(command_arg) : NULL;
 
     /* Assign all options a second time. */
+    opened_files = smartlist_new();
     for (int i = 0; i < 2; ++i) {
       const char *body = i==0 ? cf_defaults : cf;
       if (!body)
         continue;
+
       /* get config lines, assign them */
       retval = config_get_lines_include(body, &cl, 1,
-                                        body == cf ? &cf_has_include : NULL);
+                                        body == cf ? &cf_has_include : NULL,
+                                        opened_files);
       if (retval < 0) {
         err = SETOPT_ERR_PARSE;
         goto err;
@@ -5378,6 +5389,7 @@ options_init_from_string(const char *cf_defaults, const char *cf,
 
   newoptions->IncludeUsed = cf_has_include;
   in_option_validation = 1;
+  newoptions->FilesOpenedByIncludes = opened_files;
 
   /* Validate newoptions */
   if (options_validate(oldoptions, newoptions, newdefaultoptions,
@@ -5404,6 +5416,12 @@ options_init_from_string(const char *cf_defaults, const char *cf,
 
  err:
   in_option_validation = 0;
+  if (opened_files) {
+    SMARTLIST_FOREACH(opened_files, char *, f, tor_free(f));
+    smartlist_free(opened_files);
+  }
+  // may have been set to opened_files, avoid double free
+  newoptions->FilesOpenedByIncludes = NULL;
   or_options_free(newoptions);
   or_options_free(newdefaultoptions);
   if (*msg) {

+ 4 - 0
src/or/main.c

@@ -3609,6 +3609,10 @@ sandbox_init_filter(void)
     }
   }
 
+  SMARTLIST_FOREACH(options->FilesOpenedByIncludes, char *, f, {
+    OPEN(f);
+  });
+
 #define RENAME_SUFFIX(name, suffix)        \
   sandbox_cfg_allow_rename(&cfg,           \
       get_datadir_fname(name suffix),      \

+ 3 - 0
src/or/or.h

@@ -4628,6 +4628,9 @@ typedef struct {
   smartlist_t *Schedulers;
   /* An ordered list of scheduler_types mapped from Schedulers. */
   smartlist_t *SchedulerTypes_;
+
+  /** List of files that were opened by %include in torrc and torrc-defaults */
+   smartlist_t *FilesOpenedByIncludes;
 } or_options_t;
 
 #define LOG_PROTOCOL_WARN (get_protocol_warning_severity_level())

+ 99 - 18
src/test/test_config.c

@@ -4833,7 +4833,7 @@ test_config_include_limit(void *data)
                torrc_path);
   tt_int_op(write_str_to_file(torrc_path, torrc_contents, 0), OP_EQ, 0);
 
-  tt_int_op(config_get_lines_include(torrc_contents, &result, 0, NULL),
+  tt_int_op(config_get_lines_include(torrc_contents, &result, 0, NULL, NULL),
             OP_EQ, -1);
 
  done:
@@ -4863,7 +4863,7 @@ test_config_include_does_not_exist(void *data)
   tor_snprintf(torrc_contents, sizeof(torrc_contents), "%%include %s",
                missing_path);
 
-  tt_int_op(config_get_lines_include(torrc_contents, &result, 0, NULL),
+  tt_int_op(config_get_lines_include(torrc_contents, &result, 0, NULL, NULL),
             OP_EQ, -1);
 
  done:
@@ -4895,7 +4895,7 @@ test_config_include_error_in_included_file(void *data)
   tor_snprintf(torrc_contents, sizeof(torrc_contents), "%%include %s",
                invalid_path);
 
-  tt_int_op(config_get_lines_include(torrc_contents, &result, 0, NULL),
+  tt_int_op(config_get_lines_include(torrc_contents, &result, 0, NULL, NULL),
             OP_EQ, -1);
 
  done:
@@ -4937,8 +4937,8 @@ test_config_include_empty_file_folder(void *data)
                folder_path, file_path);
 
   int include_used;
-  tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used),
-            OP_EQ, 0);
+  tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used,
+            NULL), OP_EQ, 0);
   tt_ptr_op(result, OP_EQ, NULL);
   tt_int_op(include_used, OP_EQ, 1);
 
@@ -4975,7 +4975,8 @@ test_config_include_no_permission(void *data)
                folder_path);
 
   int include_used;
-  tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used),
+  tt_int_op(config_get_lines_include(torrc_contents, &result, 0,
+                                     &include_used, NULL),
             OP_EQ, -1);
   tt_ptr_op(result, OP_EQ, NULL);
 
@@ -5031,8 +5032,8 @@ test_config_include_recursion_before_after(void *data)
   }
 
   int include_used;
-  tt_int_op(config_get_lines_include(file_contents, &result, 0, &include_used),
-            OP_EQ, 0);
+  tt_int_op(config_get_lines_include(file_contents, &result, 0, &include_used,
+            NULL), OP_EQ, 0);
   tt_ptr_op(result, OP_NE, NULL);
   tt_int_op(include_used, OP_EQ, 1);
 
@@ -5096,8 +5097,8 @@ test_config_include_recursion_after_only(void *data)
   }
 
   int include_used;
-  tt_int_op(config_get_lines_include(file_contents, &result, 0, &include_used),
-            OP_EQ, 0);
+  tt_int_op(config_get_lines_include(file_contents, &result, 0, &include_used,
+            NULL), OP_EQ, 0);
   tt_ptr_op(result, OP_NE, NULL);
   tt_int_op(include_used, OP_EQ, 1);
 
@@ -5185,8 +5186,8 @@ test_config_include_folder_order(void *data)
                torrcd);
 
   int include_used;
-  tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used),
-            OP_EQ, 0);
+  tt_int_op(config_get_lines_include(torrc_contents, &result, 0, &include_used,
+            NULL), OP_EQ, 0);
   tt_ptr_op(result, OP_NE, NULL);
   tt_int_op(include_used, OP_EQ, 1);
 
@@ -5239,8 +5240,8 @@ test_config_include_path_syntax(void *data)
                esc_dir_with_pathsep);
 
   int include_used;
-  tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used),
-            OP_EQ, 0);
+  tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used,
+            NULL), OP_EQ, 0);
   tt_ptr_op(result, OP_EQ, NULL);
   tt_int_op(include_used, OP_EQ, 1);
 
@@ -5294,14 +5295,14 @@ test_config_include_has_include(void *data)
   char torrc_contents[1000] = "Test 1\n";
   int include_used;
 
-  tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used),
-            OP_EQ, 0);
+  tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used,
+            NULL), OP_EQ, 0);
   tt_int_op(include_used, OP_EQ, 0);
   config_free_lines(result);
 
   tor_snprintf(torrc_contents, sizeof(torrc_contents), "%%include %s\n", dir);
-  tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used),
-            OP_EQ, 0);
+  tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used,
+            NULL), OP_EQ, 0);
   tt_int_op(include_used, OP_EQ, 1);
 
  done:
@@ -5513,6 +5514,85 @@ test_config_check_bridge_distribution_setting_unrecognised(void *arg)
   return;
 }
 
+static void
+test_config_include_opened_file_list(void *data)
+{
+  (void)data;
+
+  config_line_t *result = NULL;
+  smartlist_t *opened_files = smartlist_new();
+  char *dir = tor_strdup(get_fname("test_include_opened_file_list"));
+  tt_ptr_op(dir, OP_NE, NULL);
+
+#ifdef _WIN32
+  tt_int_op(mkdir(dir), OP_EQ, 0);
+#else
+  tt_int_op(mkdir(dir, 0700), OP_EQ, 0);
+#endif
+
+  char torrcd[PATH_MAX+1];
+  tor_snprintf(torrcd, sizeof(torrcd), "%s"PATH_SEPARATOR"%s", dir, "torrc.d");
+
+#ifdef _WIN32
+  tt_int_op(mkdir(torrcd), OP_EQ, 0);
+#else
+  tt_int_op(mkdir(torrcd, 0700), OP_EQ, 0);
+#endif
+
+  char subfolder[PATH_MAX+1];
+  tor_snprintf(subfolder, sizeof(subfolder), "%s"PATH_SEPARATOR"%s", torrcd,
+               "subfolder");
+
+#ifdef _WIN32
+  tt_int_op(mkdir(subfolder), OP_EQ, 0);
+#else
+  tt_int_op(mkdir(subfolder, 0700), OP_EQ, 0);
+#endif
+
+  char path[PATH_MAX+1];
+  tor_snprintf(path, sizeof(path), "%s"PATH_SEPARATOR"%s", subfolder,
+               "01_file_in_subfolder");
+  tt_int_op(write_str_to_file(path, "Test 1\n", 0), OP_EQ, 0);
+
+  char empty[PATH_MAX+1];
+  tor_snprintf(empty, sizeof(empty), "%s"PATH_SEPARATOR"%s", torrcd, "empty");
+  tt_int_op(write_str_to_file(empty, "", 0), OP_EQ, 0);
+
+  char file[PATH_MAX+1];
+  tor_snprintf(file, sizeof(file), "%s"PATH_SEPARATOR"%s", torrcd, "file");
+  tt_int_op(write_str_to_file(file, "Test 2\n", 0), OP_EQ, 0);
+
+  char dot[PATH_MAX+1];
+  tor_snprintf(dot, sizeof(dot), "%s"PATH_SEPARATOR"%s", torrcd, ".dot");
+  tt_int_op(write_str_to_file(dot, "Test 3\n", 0), OP_EQ, 0);
+
+  char torrc_contents[1000];
+  tor_snprintf(torrc_contents, sizeof(torrc_contents),
+               "%%include %s\n",
+               torrcd);
+
+  int include_used;
+  tt_int_op(config_get_lines_include(torrc_contents, &result, 0, &include_used,
+            opened_files), OP_EQ, 0);
+  tt_ptr_op(result, OP_NE, NULL);
+  tt_int_op(include_used, OP_EQ, 1);
+
+  tt_int_op(smartlist_len(opened_files), OP_EQ, 4);
+  tt_int_op(smartlist_contains_string(opened_files, torrcd), OP_EQ, 1);
+  tt_int_op(smartlist_contains_string(opened_files, subfolder), OP_EQ, 1);
+  // files inside subfolders are not opended, only the subfolder is opened
+  tt_int_op(smartlist_contains_string(opened_files, empty), OP_EQ, 1);
+  tt_int_op(smartlist_contains_string(opened_files, file), OP_EQ, 1);
+  // dot files are not opened as we ignore them when we get their name from
+  // their parent folder
+
+ done:
+  SMARTLIST_FOREACH(opened_files, char *, f, tor_free(f));
+  smartlist_free(opened_files);
+  config_free_lines(result);
+  tor_free(dir);
+}
+
 #define CONFIG_TEST(name, flags)                          \
   { #name, test_config_ ## name, flags, NULL, NULL }
 
@@ -5560,6 +5640,7 @@ struct testcase_t config_tests[] = {
   CONFIG_TEST(check_bridge_distribution_setting_valid, 0),
   CONFIG_TEST(check_bridge_distribution_setting_invalid, 0),
   CONFIG_TEST(check_bridge_distribution_setting_unrecognised, 0),
+  CONFIG_TEST(include_opened_file_list, 0),
   END_OF_TESTCASES
 };