From 94352368db9045a9704c713dbbc0f41ecc511910 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 9 Aug 2017 10:36:45 -0400 Subject: [PATCH 1/6] Remove the #if 0ed code that was supposed to let the sandbox allow exec --- src/common/sandbox.c | 63 -------------------------------------------- src/common/sandbox.h | 8 ------ 2 files changed, 71 deletions(-) diff --git a/src/common/sandbox.c b/src/common/sandbox.c index c06f9694b1..18beaabe14 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -289,37 +289,6 @@ 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. - */ -static int -sb_execve(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(execve)) { - rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(execve), - SCMP_CMP_STR(0, SCMP_CMP_EQ, param->value)); - if (rc != 0) { - log_err(LD_BUG,"(Sandbox) failed to add execve syscall, received " - "libseccomp error %d", rc); - return rc; - } - } - } - - return 0; -} -#endif - /** * Function responsible for setting up the time syscall for * the seccomp filter sandbox. @@ -1063,9 +1032,6 @@ 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 @@ -1417,26 +1383,6 @@ sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file) return 0; } -#if 0 -int -sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com) -{ - sandbox_cfg_t *elem = NULL; - - elem = new_element(SCMP_SYS(execve), com); - if (!elem) { - log_err(LD_BUG,"(Sandbox) failed to register parameter!"); - return -1; - } - - elem->next = *cfg; - *cfg = elem; - - return 0; -} - -#endif - /** Cache entry for getaddrinfo results; used when sandboxing is implemented * so that we can consult the cache when the sandbox prevents us from doing * getaddrinfo. @@ -1910,15 +1856,6 @@ sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file) return 0; } -#if 0 -int -sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com) -{ - (void)cfg; (void)com; - 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 a6b83153af..55454eaa2b 100644 --- a/src/common/sandbox.h +++ b/src/common/sandbox.h @@ -156,14 +156,6 @@ int sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2); */ int sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file); -#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. - */ -int sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com); -#endif - /** * Function used to add a stat/stat64 allowed filename to a configuration. * The (char*) specifies the path to the allowed file; that pointer is stolen. From eb43401bfb6d69db66d256582a99e63de7d222d8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 9 Aug 2017 10:42:37 -0400 Subject: [PATCH 2/6] Add a 'NoExec' option that causes tor_spawn_background() to fail Core of an implementation for 22976. --- changes/feature22976 | 7 +++++++ doc/tor.1.txt | 7 +++++++ src/common/util.c | 17 +++++++++++++++++ src/common/util.h | 2 ++ src/or/config.c | 11 +++++++++++ src/or/or.h | 4 ++++ 6 files changed, 48 insertions(+) create mode 100644 changes/feature22976 diff --git a/changes/feature22976 b/changes/feature22976 new file mode 100644 index 0000000000..334f47ad0a --- /dev/null +++ b/changes/feature22976 @@ -0,0 +1,7 @@ + o Minor features (integration, hardening): + - Added a new NoExec option to . When this option is set to 1, + Tor will never try to run another program, regardless of + the settings of PortForwardingHelper, ClientTransportPlugin, + or ServerTransportPlugin. Once NoExec is set, it cannot be + disabled without restarting Tor. + Closes ticket 22976. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index b4a3cc5f75..fb19ce38b3 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -773,6 +773,13 @@ GENERAL OPTIONS circuits. If the option is set to "default", we obey a parameter in the consensus document. (Default: auto) +[[NoExec]] **NoExec** **0**|**1**:: + If this option is set to 1, then Tor will never launch another + executable, regardless of the settings of PortForwardingHelper, + ClientTransportPlugin, or ServerTransportPlugin. Once this + option has been set to 1, it cannot be set back to 0 without + restarting Tor. (Default: 0) + CLIENT OPTIONS -------------- diff --git a/src/common/util.c b/src/common/util.c index 0858d17fe6..18108fc242 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -4142,6 +4142,20 @@ process_handle_waitpid_cb(int status, void *arg) #define CHILD_STATE_EXEC 8 #define CHILD_STATE_FAILEXEC 9 /** @} */ +/** + * Boolean. If true, then Tor may call execve or CreateProcess via + * tor_spawn_background. + **/ +static int may_spawn_background_process = 1; +/** + * Turn off may_spawn_background_process, so that all future calls to + * tor_spawn_background are guaranteed to fail. + **/ +void +tor_disable_spawning_background_processes(void) +{ + may_spawn_background_process = 0; +} /** Start a program in the background. If filename contains a '/', then * it will be treated as an absolute or relative path. Otherwise, on * non-Windows systems, the system path will be searched for filename. @@ -4166,6 +4180,9 @@ tor_spawn_background(const char *const filename, const char **argv, process_environment_t *env, process_handle_t **process_handle_out) { + if (may_spawn_background_process == 0) + return PROCESS_STATUS_ERROR; + #ifdef _WIN32 HANDLE stdout_pipe_read = NULL; HANDLE stdout_pipe_write = NULL; diff --git a/src/common/util.h b/src/common/util.h index df581d2405..f50cf043a4 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -396,6 +396,8 @@ void tor_check_port_forwarding(const char *filename, struct smartlist_t *ports_to_forward, time_t now); +void tor_disable_spawning_background_processes(void); + typedef struct process_handle_t process_handle_t; typedef struct process_environment_t process_environment_t; int tor_spawn_background(const char *const filename, const char **argv, diff --git a/src/or/config.c b/src/or/config.c index 9b6bf40ebf..16e4ded696 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -409,6 +409,7 @@ static config_var_t option_vars_[] = { OBSOLETE("PredictedPortsRelevanceTime"), OBSOLETE("WarnUnsafeSocks"), VAR("NodeFamily", LINELIST, NodeFamilies, NULL), + V(NoExec, BOOL, "0"), V(NumCPUs, UINT, "0"), V(NumDirectoryGuards, UINT, "0"), V(NumEntryGuards, UINT, "0"), @@ -1595,6 +1596,10 @@ options_act(const or_options_t *old_options) const int transition_affects_guards = old_options && options_transition_affects_guards(old_options, options); + if (options->NoExec) { + tor_disable_spawning_background_processes(); + } + /* disable ptrace and later, other basic debugging techniques */ { /* Remember if we already disabled debugger attachment */ @@ -4447,6 +4452,12 @@ options_transition_allowed(const or_options_t *old, return -1; } + if (old->NoExec && !new_val->NoExec) { + *msg = tor_strdup("While Tor is running, disabling " + "NoExec is not allowed."); + return -1; + } + if (sandbox_is_active()) { #define SB_NOCHANGE_STR(opt) \ do { \ diff --git a/src/or/or.h b/src/or/or.h index ff11c72790..e24bfd7cd1 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4596,6 +4596,10 @@ typedef struct { * consensuses around so that we can generate diffs from them. If 0, * use the default. */ int MaxConsensusAgeForDiffs; + + /** Bool (default: 0). Tells Tor to never try to exec another program. + */ + int NoExec; } or_options_t; /** Persistent state for an onion router, as saved to disk. */ From 801aa5d03b3a62c7ed09e8c36629ced23e8c663a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 9 Aug 2017 10:48:43 -0400 Subject: [PATCH 3/6] Block the port-forwarding helper at a higher point --- src/or/config.c | 4 ++++ src/or/main.c | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/or/config.c b/src/or/config.c index 16e4ded696..7499dab479 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -3575,6 +3575,10 @@ options_validate(or_options_t *old_options, or_options_t *options, REJECT("PortForwarding is not compatible with Sandbox; at most one can " "be set"); } + if (options->PortForwarding && options->NoExec) { + COMPLAIN("Both PortForwarding and NoExec are set; PortForwarding will " + "be ignored."); + } if (ensure_bandwidth_cap(&options->BandwidthRate, "BandwidthRate", msg) < 0) diff --git a/src/or/main.c b/src/or/main.c index 42d984acfb..45c37159d5 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2036,7 +2036,8 @@ check_fw_helper_app_callback(time_t now, const or_options_t *options) { if (net_is_disabled() || ! server_mode(options) || - ! options->PortForwarding) { + ! options->PortForwarding || + options->NoExec) { return PERIODIC_EVENT_NO_UPDATE; } /* 11. check the port forwarding app */ From 5fa8d05bfa17d61a2cf96c87f0ffd9a2b6e577d2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 9 Aug 2017 10:48:43 -0400 Subject: [PATCH 4/6] Block managed proxies at a higher point --- src/or/config.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/or/config.c b/src/or/config.c index 7499dab479..e282a6cc60 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -5741,6 +5741,15 @@ parse_transport_line(const or_options_t *options, goto err; } + if (is_managed && options->NoExec) { + log_warn(LD_CONFIG, + "Managed proxies are not compatible with NoExec mode; ignoring." + "(%sTransportPlugin line was %s)", + server ? "Server" : "Client", escaped(line)); + r = 0; + goto done; + } + if (is_managed) { /* managed */ From b4963da987f0b374a2bbfd866e1a36358b1e750f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 9 Aug 2017 10:55:30 -0400 Subject: [PATCH 5/6] Treat a bad tor_spawn_background() as a BUG(). The contract is that, if may_spawn_background_process() is 0, you're not even allowed to try to spawn a process. --- src/common/util.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/common/util.c b/src/common/util.c index 18108fc242..eff678d6a4 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -4180,8 +4180,11 @@ tor_spawn_background(const char *const filename, const char **argv, process_environment_t *env, process_handle_t **process_handle_out) { - if (may_spawn_background_process == 0) + if (BUG(may_spawn_background_process == 0)) { + /* We should never reach this point if we're forbidden to spawn + * processes. Instead we should have caught the attempt earlier. */ return PROCESS_STATUS_ERROR; + } #ifdef _WIN32 HANDLE stdout_pipe_read = NULL; From a0bb1ff6ab0be8faa7284aec3f7f93e31e8578d9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 9 Aug 2017 10:56:57 -0400 Subject: [PATCH 6/6] Also disable spawning on Sandbox. This isn't a functional change, but it makes our logic more clear, and catches bugs earlier. --- changes/feature22976 | 3 ++- src/or/config.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/changes/feature22976 b/changes/feature22976 index 334f47ad0a..407fd15b0a 100644 --- a/changes/feature22976 +++ b/changes/feature22976 @@ -1,5 +1,6 @@ o Minor features (integration, hardening): - - Added a new NoExec option to . When this option is set to 1, + - Added a new NoExec option, to prevent Tor from running + other programs. When this option is set to 1, Tor will never try to run another program, regardless of the settings of PortForwardingHelper, ClientTransportPlugin, or ServerTransportPlugin. Once NoExec is set, it cannot be diff --git a/src/or/config.c b/src/or/config.c index e282a6cc60..30853724e4 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1596,7 +1596,7 @@ options_act(const or_options_t *old_options) const int transition_affects_guards = old_options && options_transition_affects_guards(old_options, options); - if (options->NoExec) { + if (options->NoExec || options->Sandbox) { tor_disable_spawning_background_processes(); }