sandbox: Disallow options which would make us call exec()

None of the things we might exec() can possibly run under the
sanbox, so rather than crash later, we have to refuse to accept the
configuration nice and early.

The longer-term solution is to have an exec() helper, but wow is
that risky.

fixes 12043; bugfix on 0.2.5.1-alpha

3 files changed, 41 insertions, 10 deletions

diff --git a/src/common/sandbox.c b/src/common/sandbox.c
index 8516c754f..5c7d8c87b 100644
--- a/src/common/sandbox.c
+++ b/src/common/sandbox.c
@@ -204,6 +204,7 @@ sb_rt_sigaction(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
   return rc;
 }
 
+#if 0
 /**
  * Function responsible for setting up the execve syscall for
  * the seccomp filter sandbox.
@@ -232,6 +233,7 @@ sb_execve(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
 
   return 0;
 }
+#endif
 
 /**
  * Function responsible for setting up the time syscall for
@@ -856,7 +858,9 @@ sb_stat64(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
 static sandbox_filter_func_t filter_func[] = {
     sb_rt_sigaction,
     sb_rt_sigprocmask,
+#if 0
     sb_execve,
+#endif
     sb_time,
     sb_accept4,
 #ifdef __NR_mmap2
@@ -1240,6 +1244,7 @@ sandbox_cfg_allow_openat_filename_array(sandbox_cfg_t **cfg, ...)
   return 0;
 }
 
+#if 0
 int
 sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com)
 {
@@ -1279,6 +1284,7 @@ sandbox_cfg_allow_execve_array(sandbox_cfg_t **cfg, ...)
   va_end(ap);
   return 0;
 }
+#endif
 
 int
 sandbox_getaddrinfo(const char *name, const char *servname,
@@ -1659,6 +1665,7 @@ sandbox_cfg_allow_openat_filename_array(sandbox_cfg_t **cfg, ...)
   return 0;
 }
 
+#if 0
 int
 sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com)
 {
@@ -1672,6 +1679,7 @@ sandbox_cfg_allow_execve_array(sandbox_cfg_t **cfg, ...)
   (void)cfg;
   return 0;
 }
+#endif
 
 int
 sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file)
diff --git a/src/common/sandbox.h b/src/common/sandbox.h
index c40f5e0d1..b57215285 100644
--- a/src/common/sandbox.h
+++ b/src/common/sandbox.h
@@ -198,6 +198,7 @@ int sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file);
  */
 int sandbox_cfg_allow_openat_filename_array(sandbox_cfg_t **cfg, ...);
 
+#if 0
 /**
  * Function used to add a execve allowed filename to a supplied configuration.
  * The (char*) specifies the path to the allowed file; that pointer is stolen.
@@ -211,6 +212,7 @@ int sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com);
  *  one must be NULL.
  */
 int sandbox_cfg_allow_execve_array(sandbox_cfg_t **cfg, ...);
+#endif
 
 /**
  * Function used to add a stat/stat64 allowed filename to a configuration.
diff --git a/src/or/config.c b/src/or/config.c
index 1faf13871..aa4c0079b 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -536,9 +536,11 @@ static int options_transition_affects_descriptor(
       const or_options_t *old_options, const or_options_t *new_options);
 static int check_nickname_list(char **lst, const char *name, char **msg);
 
-static int parse_client_transport_line(const char *line, int validate_only);
+static int parse_client_transport_line(const or_options_t *options,
+                                       const char *line, int validate_only);
 
-static int parse_server_transport_line(const char *line, int validate_only);
+static int parse_server_transport_line(const or_options_t *options,
+                                       const char *line, int validate_only);
 static char *get_bindaddr_from_transport_listen_line(const char *line,
                                                      const char *transport);
 static int parse_dir_authority_line(const char *line,
@@ -1426,7 +1428,7 @@ options_act(const or_options_t *old_options)
   pt_prepare_proxy_list_for_config_read();
   if (options->ClientTransportPlugin) {
     for (cl = options->ClientTransportPlugin; cl; cl = cl->next) {
-      if (parse_client_transport_line(cl->value, 0)<0) {
+      if (parse_client_transport_line(options, cl->value, 0)<0) {
         log_warn(LD_BUG,
                  "Previously validated ClientTransportPlugin line "
                  "could not be added!");
@@ -1437,7 +1439,7 @@ options_act(const or_options_t *old_options)
 
   if (options->ServerTransportPlugin && server_mode(options)) {
     for (cl = options->ServerTransportPlugin; cl; cl = cl->next) {
-      if (parse_server_transport_line(cl->value, 0)<0) {
+      if (parse_server_transport_line(options, cl->value, 0)<0) {
         log_warn(LD_BUG,
                  "Previously validated ServerTransportPlugin line "
                  "could not be added!");
@@ -3029,6 +3031,11 @@ options_validate(or_options_t *old_options, or_options_t *options,
   if (options->KeepalivePeriod < 1)
     REJECT("KeepalivePeriod option must be positive.");
 
+  if (options->PortForwarding && options->Sandbox) {
+    REJECT("PortForwarding is not compatible with Sandbox; at most one can "
+           "be set");
+  }
+
   if (ensure_bandwidth_cap(&options->BandwidthRate,
                            "BandwidthRate", msg) < 0)
     return -1;
@@ -3284,13 +3291,13 @@ options_validate(or_options_t *old_options, or_options_t *options,
   }
 
   for (cl = options->ClientTransportPlugin; cl; cl = cl->next) {
-    if (parse_client_transport_line(cl->value, 1)<0)
-      REJECT("Transport line did not parse. See logs for details.");
+    if (parse_client_transport_line(options, cl->value, 1)<0)
+      REJECT("Invalid client transport line. See logs for details.");
   }
 
   for (cl = options->ServerTransportPlugin; cl; cl = cl->next) {
-    if (parse_server_transport_line(cl->value, 1)<0)
-      REJECT("Server transport line did not parse. See logs for details.");
+    if (parse_server_transport_line(options, cl->value, 1)<0)
+      REJECT("Invalid server transport line. See logs for details.");
   }
 
   if (options->ServerTransportPlugin && !server_mode(options)) {
@@ -4734,7 +4741,8 @@ parse_bridge_line(const char *line)
  * our internal transport list.
  * - If it's a managed proxy line, launch the managed proxy. */
 static int
-parse_client_transport_line(const char *line, int validate_only)
+parse_client_transport_line(const or_options_t *options,
+                            const char *line, int validate_only)
 {
   smartlist_t *items = NULL;
   int r;
@@ -4801,6 +4809,12 @@ parse_client_transport_line(const char *line, int validate_only)
     goto err;
   }
 
+  if (is_managed && options->Sandbox) {
+    log_warn(LD_CONFIG, "Managed proxies are not compatible with Sandbox mode."
+             "(ClientTransportPlugin line was %s)", escaped(line));
+    goto err;
+  }
+
   if (is_managed) { /* managed */
     if (!validate_only && is_useless_proxy) {
       log_notice(LD_GENERAL, "Pluggable transport proxy (%s) does not provide "
@@ -5027,7 +5041,8 @@ get_options_for_server_transport(const char *transport)
  * If <b>validate_only</b> is 0, the line is well-formed, and it's a
  * managed proxy line, launch the managed proxy. */
 static int
-parse_server_transport_line(const char *line, int validate_only)
+parse_server_transport_line(const or_options_t *options,
+                            const char *line, int validate_only)
 {
   smartlist_t *items = NULL;
   int r;
@@ -5082,6 +5097,12 @@ parse_server_transport_line(const char *line, int validate_only)
     goto err;
   }
 
+  if (is_managed && options->Sandbox) {
+    log_warn(LD_CONFIG, "Managed proxies are not compatible with Sandbox mode."
+             "(ServerTransportPlugin line was %s)", escaped(line));
+    goto err;
+  }
+
   if (is_managed) { /* managed */
     if (!validate_only) {
       proxy_argc = line_length-2;