[tor-commits] [tor/master] Rewrite managed proxy environment setup code

commit bf1ce3f53d67c4c0e71f79a108e1eaa04fbfb713 Author: Robert Ransom rransom.8774@gmail.com Date: Mon Feb 13 00:46:18 2012 -0800

Rewrite managed proxy environment setup code

Now, the environment setup is entirely OS-independent, as well as less hacky and brittle. --- changes/bug5105 | 11 +++ src/or/transports.c | 234 ++++++++++++++------------------------------------- 2 files changed, 73 insertions(+), 172 deletions(-)

diff --git a/changes/bug5105 b/changes/bug5105 new file mode 100644 index 0000000..6a923d9 --- /dev/null +++ b/changes/bug5105 @@ -0,0 +1,11 @@ + o Minor bugfixes: + + - Ensure that variables set in Tor's environment cannot override + environment variables which Tor tries to pass to a managed + pluggable-transport proxy. Previously, Tor would pass every + variable in its environment to managed proxies along with the + new ones, in such a way that on many operating systems, the + inherited environment variables would override those which Tor + tried to explicitly set. Bugfix on 0.2.3.12-alpha for most + Unixoid systems; bugfix on 0.2.3.9-alpha for Windows. + diff --git a/src/or/transports.c b/src/or/transports.c index d279d7a..b3c8382 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -91,13 +91,8 @@ #include "util.h" #include "router.h"

-#ifdef _WIN32 -static void set_managed_proxy_environment(LPVOID *envp, - const managed_proxy_t *mp); -#else -static void set_managed_proxy_environment(char ***envp, - const managed_proxy_t *mp); -#endif +static process_environment_t * +create_managed_proxy_environment(const managed_proxy_t *mp);

static INLINE int proxy_configuration_finished(const managed_proxy_t *mp);

@@ -287,35 +282,23 @@ launch_managed_proxy(managed_proxy_t *mp) { int retval;

-#ifdef _WIN32 - - LPVOID envp=NULL; - - set_managed_proxy_environment(&envp, mp); - tor_assert(envp); + process_environment_t *env = create_managed_proxy_environment(mp);

+#ifdef _WIN32 /* Passing NULL as lpApplicationName makes Windows search for the .exe */ - retval = tor_spawn_background(NULL, (const char **)mp->argv, envp, + retval = tor_spawn_background(NULL, + (const char **)mp->argv, + env->windows_environment_block, &mp->process_handle); - - tor_free(envp); - #else - - char **envp=NULL; - - /* prepare the environment variables for the managed proxy */ - set_managed_proxy_environment(&envp, mp); - tor_assert(envp); - - retval = tor_spawn_background(mp->argv[0], (const char **)mp->argv, - (const char **)envp, &mp->process_handle); - - /* free the memory allocated by set_managed_proxy_environment(). */ - free_execve_args(envp); - + retval = tor_spawn_background(mp->argv[0], + (const char **)mp->argv, + env->unixoid_environment_block, + &mp->process_handle); #endif

+ process_environment_free(env); + if (retval == PROCESS_STATUS_ERROR) { log_warn(LD_GENERAL, "Managed proxy at '%s' failed at launch.", mp->argv[0]); @@ -971,171 +954,78 @@ get_bindaddr_for_server_proxy(const managed_proxy_t *mp) return bindaddr_result; }

-#ifdef _WIN32 - -/** Prepare the environment <b>envp</b> of managed proxy <b>mp</b>. - * <b>envp</b> is allocated on the heap and should be freed by the - * caller after its use. */ -static void -set_managed_proxy_environment(LPVOID *envp, const managed_proxy_t *mp) +/** Return a newly allocated process_environment_t * for <b>mp</b>'s + * process. */ +static process_environment_t * +create_managed_proxy_environment(const managed_proxy_t *mp) { const or_options_t *options = get_options();

- char *tmp=NULL; - char *state_tmp=NULL; - char *state_env=NULL; - char *transports_to_launch=NULL; - char *transports_env=NULL; - char *bindaddr_tmp=NULL; - char *bindaddr_env=NULL; - char *orport_env=NULL; - - char version_env[31]; /* XXX temp */ - char extended_env[43]; /* XXX temp */ - - int env_size = 0; - - /* A smartlist carrying all the env. variables that the managed - proxy should inherit. */ + /* Environment variables to be added to or set in mp's environment. + * These are heap-allocated and are freed before this function + * returns. */ smartlist_t *envs = smartlist_new();

- /* Copy the whole environment of the Tor process. - It should also copy PATH and HOME of the Tor process.*/ - char **environ_tmp = environ; - while (*environ_tmp) - smartlist_add(envs, *environ_tmp++); + /* The final environment to be passed to mp. Inherited variables are + * statically allocated; new ones are heap-allocated. */ + smartlist_t *merged_env_vars = get_current_process_environment_variables();

- /* Create the TOR_PT_* environment variables. */ - state_tmp = get_datadir_fname("pt_state/"); /* XXX temp */ - tor_asprintf(&state_env, "TOR_PT_STATE_LOCATION=%s", state_tmp); + process_environment_t *env;

- strcpy(version_env, "TOR_PT_MANAGED_TRANSPORT_VER=1"); - - transports_to_launch = - smartlist_join_strings(mp->transports_to_launch, ",", 0, NULL); - - tor_asprintf(&transports_env, - mp->is_server ? - "TOR_PT_SERVER_TRANSPORTS=%s" : "TOR_PT_CLIENT_TRANSPORTS=%s", - transports_to_launch); - - smartlist_add(envs, state_env); - smartlist_add(envs, version_env); - smartlist_add(envs, transports_env); + { + char *state_tmp = get_datadir_fname("pt_state/"); /* XXX temp */ + smartlist_add_asprintf(envs, "TOR_PT_STATE_LOCATION=%s", state_tmp); + tor_free(state_tmp); + }

- if (mp->is_server) { - tor_asprintf(&orport_env, "TOR_PT_ORPORT=127.0.0.1:%s", - options->ORPort->value); + smartlist_add(envs, tor_strdup("TOR_PT_MANAGED_TRANSPORT_VER=1"));

- bindaddr_tmp = get_bindaddr_for_server_proxy(mp); - tor_asprintf(&bindaddr_env, "TOR_PT_SERVER_BINDADDR=%s", bindaddr_tmp); + { + char *transports_to_launch = + smartlist_join_strings(mp->transports_to_launch, ",", 0, NULL);

- strlcpy(extended_env, "TOR_PT_EXTENDED_SERVER_PORT=", - sizeof(extended_env)); + smartlist_add_asprintf(envs, + mp->is_server ? + "TOR_PT_SERVER_TRANSPORTS=%s" : + "TOR_PT_CLIENT_TRANSPORTS=%s", + transports_to_launch);

- smartlist_add(envs, orport_env); - smartlist_add(envs, extended_env); - smartlist_add(envs, bindaddr_env); + tor_free(transports_to_launch); }

- /* It seems like some versions of Windows need a sorted lpEnvironment - block. */ - smartlist_sort_strings(envs); - - /* An environment block consists of a null-terminated block of - null-terminated strings: */ - - /* Calculate the block's size. */ - SMARTLIST_FOREACH(envs, const char *, s, - env_size += strlen(s) + 1); - env_size += 1; /* space for last NUL */ - - *envp = tor_malloc(env_size); - tmp = *envp; - - /* Create the block. */ - SMARTLIST_FOREACH_BEGIN(envs, const char *, s) { - memcpy(tmp, s, strlen(s)); /* copy the env. variable string */ - tmp += strlen(s); - memset(tmp, '\0', 1); /* append NUL at the end of the string */ - tmp += 1; - } SMARTLIST_FOREACH_END(s); - memset(tmp, '\0', 1); /* last NUL */ - - /* Finally, free the whole mess. */ - tor_free(state_tmp); - tor_free(state_env); - tor_free(transports_to_launch); - tor_free(transports_env); - tor_free(bindaddr_tmp); - tor_free(bindaddr_env); - tor_free(orport_env); - - smartlist_free(envs); -} - -#else /* _WIN32 */ + if (mp->is_server) { + smartlist_add_asprintf(envs, "TOR_PT_ORPORT=127.0.0.1:%s", + options->ORPort->value);

-extern char **environ; + { + char *bindaddr_tmp = get_bindaddr_for_server_proxy(mp); + smartlist_add_asprintf(envs, "TOR_PT_SERVER_BINDADDR=%s", bindaddr_tmp); + tor_free(bindaddr_tmp); + }

-/** Prepare the environment <b>envp</b> of managed proxy <b>mp</b>. - * <b>envp</b> is allocated on the heap and should be freed by the - * caller after its use. */ -static void -set_managed_proxy_environment(char ***envp, const managed_proxy_t *mp) -{ - const or_options_t *options = get_options(); - char **tmp=NULL; - char *state_loc=NULL; - char *transports_to_launch=NULL; - char *bindaddr=NULL; - int environ_size=0; - char **environ_tmp = environ; - - int n_envs = mp->is_server ? ENVIRON_SIZE_SERVER : ENVIRON_SIZE_CLIENT; - - while (*environ_tmp) { - environ_size++; - environ_tmp++; + /* XXX023 Remove the '=' here once versions of obfsproxy which + * assert that this env var exists are sufficiently dead. + * + * (If we remove this line entirely, some joker will stick this + * variable in Tor's environment and crash PTs that try to parse + * it even when not run in server mode.) */ + smartlist_add(envs, tor_strdup("TOR_PT_EXTENDED_SERVER_PORT=")); } - environ_tmp = environ;

- /* allocate enough space for our env. vars and a NULL pointer */ - *envp = tor_malloc(sizeof(char*)*(environ_size+n_envs+1)); - tmp = *envp; + SMARTLIST_FOREACH_BEGIN(envs, const char *, env_var) { + set_environment_variable_in_smartlist(merged_env_vars, env_var, NULL, 0); + } SMARTLIST_FOREACH_END(env_var);

- state_loc = get_datadir_fname("pt_state/"); /* XXX temp */ - transports_to_launch = - smartlist_join_strings(mp->transports_to_launch, ",", 0, NULL); + env = process_environment_make(merged_env_vars);

- while (*environ_tmp) { - *tmp = tor_strdup(*environ_tmp); - tmp++, environ_tmp++; - } + smartlist_free(merged_env_vars);

- tor_asprintf(tmp++, "TOR_PT_STATE_LOCATION=%s", state_loc); - tor_asprintf(tmp++, "TOR_PT_MANAGED_TRANSPORT_VER=1"); /* temp */ - if (mp->is_server) { - bindaddr = get_bindaddr_for_server_proxy(mp); - - /* XXX temp */ - tor_asprintf(tmp++, "TOR_PT_ORPORT=127.0.0.1:%d", - router_get_advertised_or_port(options)); - tor_asprintf(tmp++, "TOR_PT_SERVER_BINDADDR=%s", bindaddr); - tor_asprintf(tmp++, "TOR_PT_SERVER_TRANSPORTS=%s", transports_to_launch); - tor_asprintf(tmp++, "TOR_PT_EXTENDED_SERVER_PORT="); - } else { - tor_asprintf(tmp++, "TOR_PT_CLIENT_TRANSPORTS=%s", transports_to_launch); - } - *tmp = NULL; + SMARTLIST_FOREACH(envs, void *, x, tor_free(x)); + smartlist_free(envs);

- tor_free(state_loc); - tor_free(transports_to_launch); - tor_free(bindaddr); + return env; }

-#endif /* _WIN32 */ - /** Create and return a new managed proxy for <b>transport</b> using * <b>proxy_argv</b>. If <b>is_server</b> is true, it's a server * managed proxy. */