From bfb94dd2ca8e04fb1fe8aba9ad48effbb8b70662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20F=C3=A6r=C3=B8y?= Date: Fri, 23 Nov 2018 05:56:41 +0100 Subject: [PATCH] Use process_t for managed proxies. This patch makes the managed proxy subsystem use the process_t data structure such that we can get events from the PT process while Tor is running and not just when the PT process is being configured. See: https://bugs.torproject.org/28179 --- src/core/mainloop/mainloop.c | 4 - src/feature/client/transports.c | 174 ++++++++++++++++++-------------- src/feature/client/transports.h | 12 ++- src/test/test_pt.c | 66 ++++++------ 4 files changed, 137 insertions(+), 119 deletions(-) diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c index 2e2ae876d4..aaaa5009cb 100644 --- a/src/core/mainloop/mainloop.c +++ b/src/core/mainloop/mainloop.c @@ -1853,10 +1853,6 @@ second_elapsed_callback(time_t now, const or_options_t *options) run_connection_housekeeping(i, now); } - /* 11b. check pending unconfigured managed proxies */ - if (!net_is_disabled() && pt_proxies_configuration_pending()) - pt_configure_remaining_proxies(); - /* Run again in a second. */ return 1; } diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index 85d4d2e080..f0400b713d 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -102,10 +102,11 @@ #include "feature/relay/ext_orport.h" #include "feature/control/control.h" -#include "lib/process/env.h" +#include "lib/process/process.h" #include "lib/process/subprocess.h" +#include "lib/process/env.h" -static process_environment_t * +static smartlist_t * create_managed_proxy_environment(const managed_proxy_t *mp); static inline int proxy_configuration_finished(const managed_proxy_t *mp); @@ -490,8 +491,8 @@ proxy_prepare_for_restart(managed_proxy_t *mp) tor_assert(mp->conf_state == PT_PROTO_COMPLETED); /* destroy the process handle and terminate the process. */ - tor_process_handle_destroy(mp->process_handle, 1); - mp->process_handle = NULL; + process_set_data(mp->process, NULL); + process_terminate(mp->process); /* destroy all its registered transports, since we will no longer use them. */ @@ -520,34 +521,35 @@ proxy_prepare_for_restart(managed_proxy_t *mp) static int launch_managed_proxy(managed_proxy_t *mp) { - int retval; + tor_assert(mp); - process_environment_t *env = create_managed_proxy_environment(mp); + smartlist_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, - env, - &mp->process_handle); -#else /* !(defined(_WIN32)) */ - retval = tor_spawn_background(mp->argv[0], - (const char **)mp->argv, - env, - &mp->process_handle); -#endif /* defined(_WIN32) */ + /* Configure our process. */ + process_set_data(mp->process, mp); + process_set_stdout_read_callback(mp->process, managed_proxy_stdout_callback); + process_set_stderr_read_callback(mp->process, managed_proxy_stderr_callback); + process_set_exit_callback(mp->process, managed_proxy_exit_callback); + process_set_protocol(mp->process, PROCESS_PROTOCOL_LINE); + process_reset_environment(mp->process, env); - process_environment_free(env); + /* Cleanup our env. */ + SMARTLIST_FOREACH(env, char *, x, tor_free(x)); + smartlist_free(env); - if (retval == PROCESS_STATUS_ERROR) { - log_warn(LD_GENERAL, "Managed proxy at '%s' failed at launch.", + /* Skip the argv[0] as we get that from process_new(argv[0]). */ + for (int i = 1; mp->argv[i] != NULL; ++i) + process_append_argument(mp->process, mp->argv[i]); + + if (process_exec(mp->process) != PROCESS_STATUS_RUNNING) { + log_warn(LD_CONFIG, "Managed proxy at '%s' failed at launch.", mp->argv[0]); return -1; } - log_info(LD_CONFIG, "Managed proxy at '%s' has spawned with PID '%d'.", - mp->argv[0], tor_process_get_pid(mp->process_handle)); - + log_info(LD_CONFIG, + "Managed proxy at '%s' has spawned with PID '%" PRIu64 "'.", + mp->argv[0], process_get_pid(mp->process)); mp->conf_state = PT_PROTO_LAUNCHED; return 0; @@ -615,10 +617,6 @@ pt_configure_remaining_proxies(void) STATIC int configure_proxy(managed_proxy_t *mp) { - int configuration_finished = 0; - smartlist_t *proxy_output = NULL; - enum stream_status stream_status = 0; - /* if we haven't launched the proxy yet, do it now */ if (mp->conf_state == PT_PROTO_INFANT) { if (launch_managed_proxy(mp) < 0) { /* launch fail */ @@ -629,45 +627,8 @@ configure_proxy(managed_proxy_t *mp) } tor_assert(mp->conf_state != PT_PROTO_INFANT); - tor_assert(mp->process_handle); - - proxy_output = - tor_get_lines_from_handle(tor_process_get_stdout_pipe(mp->process_handle), - &stream_status); - if (!proxy_output) { /* failed to get input from proxy */ - if (stream_status != IO_STREAM_EAGAIN) { /* bad stream status! */ - mp->conf_state = PT_PROTO_BROKEN; - log_warn(LD_GENERAL, "The communication stream of managed proxy '%s' " - "is '%s'. Most probably the managed proxy stopped running. " - "This might be a bug of the managed proxy, a bug of Tor, or " - "a misconfiguration. Please enable logging on your managed " - "proxy and check the logs for errors.", - mp->argv[0], stream_status_to_string(stream_status)); - } - - goto done; - } - - /* Handle lines. */ - SMARTLIST_FOREACH_BEGIN(proxy_output, const char *, line) { - handle_proxy_line(line, mp); - if (proxy_configuration_finished(mp)) - goto done; - } SMARTLIST_FOREACH_END(line); - - done: - /* if the proxy finished configuring, exit the loop. */ - if (proxy_configuration_finished(mp)) { - handle_finished_proxy(mp); - configuration_finished = 1; - } - - if (proxy_output) { - SMARTLIST_FOREACH(proxy_output, char *, cp, tor_free(cp)); - smartlist_free(proxy_output); - } - - return configuration_finished; + tor_assert(mp->process); + return mp->conf_state == PT_PROTO_COMPLETED; } /** Register server managed proxy mp transports to state */ @@ -748,8 +709,11 @@ managed_proxy_destroy(managed_proxy_t *mp, /* free the outgoing proxy URI */ tor_free(mp->proxy_uri); - tor_process_handle_destroy(mp->process_handle, also_terminate_process); - mp->process_handle = NULL; + /* do we want to terminate our process if it's still running? */ + if (also_terminate_process && mp->process) + process_terminate(mp->process); + + process_free(mp->process); tor_free(mp); } @@ -1257,7 +1221,7 @@ get_bindaddr_for_server_proxy(const managed_proxy_t *mp) /** Return a newly allocated process_environment_t * for mp's * process. */ -static process_environment_t * +static smartlist_t * create_managed_proxy_environment(const managed_proxy_t *mp) { const or_options_t *options = get_options(); @@ -1272,8 +1236,6 @@ create_managed_proxy_environment(const managed_proxy_t *mp) /* The final environment to be passed to mp. */ smartlist_t *merged_env_vars = get_current_process_environment_variables(); - process_environment_t *env; - { char *state_tmp = get_datadir_fname("pt_state/"); /* XXX temp */ smartlist_add_asprintf(envs, "TOR_PT_STATE_LOCATION=%s", state_tmp); @@ -1366,14 +1328,9 @@ create_managed_proxy_environment(const managed_proxy_t *mp) tor_free_, 1); } SMARTLIST_FOREACH_END(env_var); - env = process_environment_make(merged_env_vars); - smartlist_free(envs); - SMARTLIST_FOREACH(merged_env_vars, void *, x, tor_free(x)); - smartlist_free(merged_env_vars); - - return env; + return merged_env_vars; } /** Create and return a new managed proxy for transport using @@ -1392,6 +1349,7 @@ managed_proxy_create(const smartlist_t *with_transport_list, mp->argv = proxy_argv; mp->transports = smartlist_new(); mp->proxy_uri = get_pt_proxy_uri(); + mp->process = process_new(proxy_argv[0]); mp->transports_to_launch = smartlist_new(); SMARTLIST_FOREACH(with_transport_list, const char *, transport, @@ -1736,3 +1694,63 @@ tor_escape_str_for_pt_args(const char *string, const char *chars_to_escape) return new_string; } + +/** Callback function that is called when our PT process have data on its + * stdout. Our process can be found in process, the data can be found in + * line and the length of our line is given in size. */ +STATIC void +managed_proxy_stdout_callback(process_t *process, char *line, size_t size) +{ + tor_assert(process); + tor_assert(line); + + (void)size; + + managed_proxy_t *mp = process_get_data(process); + + handle_proxy_line(line, mp); + + if (proxy_configuration_finished(mp)) { + handle_finished_proxy(mp); + tor_assert(mp->conf_state == PT_PROTO_COMPLETED); + } +} + +/** Callback function that is called when our PT process have data on its + * stderr. Our process can be found in process, the data can be found in + * line and the length of our line is given in size. */ +STATIC void +managed_proxy_stderr_callback(process_t *process, char *line, size_t size) +{ + tor_assert(process); + tor_assert(line); + + (void)size; + + managed_proxy_t *mp = process_get_data(process); + + log_warn(LD_PT, "Managed proxy at '%s' reported: %s", mp->argv[0], line); +} + +/** Callback function that is called when our PT process terminates. The + * process exit code can be found in exit_code and our process can be + * found in process. */ +STATIC void +managed_proxy_exit_callback(process_t *process, process_exit_code_t exit_code) +{ + tor_assert(process); + + log_warn(LD_PT, + "Pluggable Transport process terminated with status code %" PRIu64, + exit_code); + + /* We detach ourself from the MP (if we are attached) and free ourself. */ + managed_proxy_t *mp = process_get_data(process); + + if (BUG(mp != NULL)) { + mp->process = NULL; + process_set_data(process, NULL); + } + + process_free(process); +} diff --git a/src/feature/client/transports.h b/src/feature/client/transports.h index b80875c95c..59df637d81 100644 --- a/src/feature/client/transports.h +++ b/src/feature/client/transports.h @@ -11,6 +11,8 @@ #ifndef TOR_TRANSPORTS_H #define TOR_TRANSPORTS_H +#include "lib/process/process.h" + /** Represents a pluggable transport used by a bridge. */ typedef struct transport_t { /** SOCKS version: One of PROXY_SOCKS4, PROXY_SOCKS5. */ @@ -81,7 +83,7 @@ enum pt_proto_state { PT_PROTO_FAILED_LAUNCH /* failed while launching */ }; -struct process_handle_t; +struct process_t; /** Structure containing information of a managed proxy. */ typedef struct { @@ -94,8 +96,8 @@ typedef struct { int is_server; /* is it a server proxy? */ - /* A pointer to the process handle of this managed proxy. */ - struct process_handle_t *process_handle; + /* A pointer to the process of this managed proxy. */ + struct process_t *process; /** Boolean: We are re-parsing our config, and we are going to * remove this managed proxy if we don't find it any transport @@ -140,6 +142,10 @@ STATIC char* get_pt_proxy_uri(void); STATIC void free_execve_args(char **arg); +STATIC void managed_proxy_stdout_callback(process_t *, char *, size_t); +STATIC void managed_proxy_stderr_callback(process_t *, char *, size_t); +STATIC void managed_proxy_exit_callback(process_t *, process_exit_code_t); + #endif /* defined(PT_PRIVATE) */ #endif /* !defined(TOR_TRANSPORTS_H) */ diff --git a/src/test/test_pt.c b/src/test/test_pt.c index d0160d1148..2501b867bb 100644 --- a/src/test/test_pt.c +++ b/src/test/test_pt.c @@ -9,6 +9,7 @@ #define STATEFILE_PRIVATE #define CONTROL_PRIVATE #define SUBPROCESS_PRIVATE +#define PROCESS_PRIVATE #include "core/or/or.h" #include "app/config/config.h" #include "app/config/confparse.h" @@ -20,6 +21,7 @@ #include "lib/process/subprocess.h" #include "lib/encoding/confline.h" #include "lib/net/resolve.h" +#include "lib/process/process.h" #include "app/config/or_state_st.h" @@ -151,6 +153,8 @@ test_pt_get_transport_options(void *arg) config_line_t *cl = NULL; (void)arg; + process_init(); + execve_args = tor_malloc(sizeof(char*)*2); execve_args[0] = tor_strdup("cheeseshop"); execve_args[1] = NULL; @@ -190,6 +194,7 @@ test_pt_get_transport_options(void *arg) config_free_lines(cl); managed_proxy_destroy(mp, 0); smartlist_free(transport_list); + process_free_all(); } static void @@ -253,6 +258,8 @@ test_pt_get_extrainfo_string(void *arg) char *s = NULL; (void) arg; + process_init(); + argv1 = tor_malloc_zero(sizeof(char*)*3); argv1[0] = tor_strdup("ewige"); argv1[1] = tor_strdup("Blumenkraft"); @@ -286,43 +293,25 @@ test_pt_get_extrainfo_string(void *arg) smartlist_free(t1); smartlist_free(t2); tor_free(s); + process_free_all(); } -#ifdef _WIN32 -#define STDIN_HANDLE HANDLE* -#else -#define STDIN_HANDLE int -#endif - -static smartlist_t * -tor_get_lines_from_handle_replacement(STDIN_HANDLE handle, - enum stream_status *stream_status_out) +static int +process_read_stdout_replacement(process_t *process, buf_t *buffer) { + (void)process; static int times_called = 0; - smartlist_t *retval_sl = smartlist_new(); - - (void) handle; - (void) stream_status_out; /* Generate some dummy CMETHOD lines the first 5 times. The 6th time, send 'CMETHODS DONE' to finish configuring the proxy. */ if (times_called++ != 5) { - smartlist_add_asprintf(retval_sl, "SMETHOD mock%d 127.0.0.1:555%d", + buf_add_printf(buffer, "SMETHOD mock%d 127.0.0.1:555%d\n", times_called, times_called); } else { - smartlist_add_strdup(retval_sl, "SMETHODS DONE"); + buf_add_string(buffer, "SMETHODS DONE\n"); } - return retval_sl; -} - -/* NOP mock */ -static void -tor_process_handle_destroy_replacement(process_handle_t *process_handle, - int also_terminate_process) -{ - (void) process_handle; - (void) also_terminate_process; + return (int)buf_datalen(buffer); } static or_state_t *dummy_state = NULL; @@ -355,12 +344,11 @@ test_pt_configure_proxy(void *arg) managed_proxy_t *mp = NULL; (void) arg; + process_init(); + dummy_state = tor_malloc_zero(sizeof(or_state_t)); - MOCK(tor_get_lines_from_handle, - tor_get_lines_from_handle_replacement); - MOCK(tor_process_handle_destroy, - tor_process_handle_destroy_replacement); + MOCK(process_read_stdout, process_read_stdout_replacement); MOCK(get_or_state, get_or_state_replacement); MOCK(queue_control_event_string, @@ -372,24 +360,34 @@ test_pt_configure_proxy(void *arg) mp->conf_state = PT_PROTO_ACCEPTING_METHODS; mp->transports = smartlist_new(); mp->transports_to_launch = smartlist_new(); - mp->process_handle = tor_malloc_zero(sizeof(process_handle_t)); mp->argv = tor_malloc_zero(sizeof(char*)*2); mp->argv[0] = tor_strdup(""); mp->is_server = 1; + /* Configure the process. */ + mp->process = process_new(""); + process_set_stdout_read_callback(mp->process, managed_proxy_stdout_callback); + process_set_data(mp->process, mp); + /* Test the return value of configure_proxy() by calling it some times while it is uninitialized and then finally finalizing its configuration. */ for (i = 0 ; i < 5 ; i++) { + /* force a read from our mocked stdout reader. */ + process_notify_event_stdout(mp->process); + /* try to configure our proxy. */ retval = configure_proxy(mp); /* retval should be zero because proxy hasn't finished configuring yet */ tt_int_op(retval, OP_EQ, 0); /* check the number of registered transports */ - tt_assert(smartlist_len(mp->transports) == i+1); + tt_int_op(smartlist_len(mp->transports), OP_EQ, i+1); /* check that the mp is still waiting for transports */ tt_assert(mp->conf_state == PT_PROTO_ACCEPTING_METHODS); } + /* Get the SMETHOD DONE written to the process. */ + process_notify_event_stdout(mp->process); + /* this last configure_proxy() should finalize the proxy configuration. */ retval = configure_proxy(mp); /* retval should be 1 since the proxy finished configuring */ @@ -435,8 +433,7 @@ test_pt_configure_proxy(void *arg) done: or_state_free(dummy_state); - UNMOCK(tor_get_lines_from_handle); - UNMOCK(tor_process_handle_destroy); + UNMOCK(process_read_stdout); UNMOCK(get_or_state); UNMOCK(queue_control_event_string); if (controlevent_msgs) { @@ -449,10 +446,11 @@ test_pt_configure_proxy(void *arg) smartlist_free(mp->transports); } smartlist_free(mp->transports_to_launch); - tor_free(mp->process_handle); + process_free(mp->process); tor_free(mp->argv[0]); tor_free(mp->argv); tor_free(mp); + process_free_all(); } /* Test the get_pt_proxy_uri() function. */