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

2 files changed, 109 insertions, 85 deletions

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;
-
-  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,
-                                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) */
-
-  process_environment_free(env);
-
-  if (retval == PROCESS_STATUS_ERROR) {
-    log_warn(LD_GENERAL, "Managed proxy at '%s' failed at launch.",
+  tor_assert(mp);
+
+  smartlist_t *env = create_managed_proxy_environment(mp);
+
+  /* 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);
+
+  /* Cleanup our env. */
+  SMARTLIST_FOREACH(env, char *, x, tor_free(x));
+  smartlist_free(env);
+
+  /* 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 <b>mp</b> 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 <b>mp</b>'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 <b>transport</b> 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 <b>process</b>, the data can be found in
+ * <b>line</b> and the length of our line is given in <b>size</b>. */
+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 <b>process</b>, the data can be found in
+ * <b>line</b> and the length of our line is given in <b>size</b>. */
+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 <b>exit_code</b> and our process can be
+ * found in <b>process</b>. */
+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) */