Merge branch 'maint-0.4.8' into release-0.4.8

4 files changed, 218 insertions, 10 deletions

diff --git a/changes/ticket11101 b/changes/ticket11101
new file mode 100644
index 0000000000..6c898caa5b
--- /dev/null
+++ b/changes/ticket11101
@@ -0,0 +1,4 @@
+  o Minor feature (bridges, pluggable transport):
+    - Add STATUS TYPE=version handler for Pluggable Transport. This allows us to
+      gather version statistics on Pluggable Transport usage from bridge servers
+      on our metrics portal. Closes ticket 11101.
diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c
index 4d92a2a67a..878675ac4d 100644
--- a/src/feature/client/transports.c
+++ b/src/feature/client/transports.c
@@ -89,6 +89,7 @@
  * old transports from the circuitbuild.c subsystem.
  **/
 
+#include "lib/string/printf.h"
 #define PT_PRIVATE
 #include "core/or/or.h"
 #include "feature/client/bridges.h"
@@ -741,6 +742,10 @@ managed_proxy_destroy(managed_proxy_t *mp,
   /* free the outgoing proxy URI */
   tor_free(mp->proxy_uri);
 
+  /* free our version, if any is set. */
+  tor_free(mp->version);
+  tor_free(mp->implementation);
+
   /* do we want to terminate our process if it's still running? */
   if (also_terminate_process && mp->process) {
     /* Note that we do not call process_free(mp->process) here because we let
@@ -1283,15 +1288,8 @@ parse_status_line(const char *line, managed_proxy_t *mp)
     goto done;
   }
 
-  /* We check if we received the TRANSPORT parameter, which is the only
-   * *required* value. */
-  const config_line_t *type = config_line_find(values, "TRANSPORT");
-
-  if (! type) {
-    log_warn(LD_PT, "Managed proxy \"%s\" wrote a STATUS line without "
-                    "TRANSPORT: %s", mp->argv[0], escaped(data));
-    goto done;
-  }
+  /* Handle the different messages. */
+  handle_status_message(values, mp);
 
   /* Prepend the PT name. */
   config_line_prepend(&values, "PT", mp->argv[0]);
@@ -1306,6 +1304,52 @@ parse_status_line(const char *line, managed_proxy_t *mp)
   tor_free(status_message);
 }
 
+STATIC void
+handle_status_message(const config_line_t *values,
+                      managed_proxy_t *mp)
+{
+  if (config_count_key(values, "TYPE") > 1) {
+      log_warn(LD_PT, "Managed proxy \"%s\" has multiple TYPE key which "
+                      "is not allowed.", mp->argv[0]);
+      return;
+  }
+  const config_line_t *message_type = config_line_find(values, "TYPE");
+
+  /* Check if we have a TYPE field? */
+  if (message_type == NULL) {
+    log_debug(LD_PT, "Managed proxy \"%s\" wrote a STATUS line without "
+                     "a defined message TYPE", mp->argv[0]);
+    return;
+  }
+
+  /* Handle VERSION messages. */
+  if (! strcasecmp(message_type->value, "version")) {
+    const config_line_t *version = config_line_find(values, "VERSION");
+    const config_line_t *implementation = config_line_find(values,
+                                                           "IMPLEMENTATION");
+
+    if (version == NULL) {
+      log_warn(LD_PT, "Managed proxy \"%s\" wrote a STATUS TYPE=version line "
+                      "with a missing VERSION field", mp->argv[0]);
+      return;
+    }
+
+    if (implementation == NULL) {
+      log_warn(LD_PT, "Managed proxy \"%s\" wrote a STATUS TYPE=version line "
+                      "with a missing IMPLEMENTATION field", mp->argv[0]);
+      return;
+    }
+
+    tor_free(mp->version);
+    mp->version = tor_strdup(version->value);
+
+    tor_free(mp->implementation);
+    mp->implementation = tor_strdup(implementation->value);
+
+    return;
+  }
+}
+
 /** Return a newly allocated string that tor should place in
  * TOR_PT_SERVER_TRANSPORT_OPTIONS while configuring the server
  * manged proxy in <b>mp</b>. Return NULL if no such options are found. */
@@ -1748,9 +1792,28 @@ pt_get_extra_info_descriptor_string(void)
                              "transport %s %s%s",
                              t->name, addrport,
                              transport_args ? transport_args : "");
+
       tor_free(transport_args);
     } SMARTLIST_FOREACH_END(t);
 
+    /* Set transport-info line. */
+    {
+      char *version = NULL;
+      char *impl = NULL;
+
+      if (mp->version) {
+        tor_asprintf(&version, " version=%s", mp->version);
+      }
+      if (mp->implementation) {
+        tor_asprintf(&impl, " implementation=%s", mp->implementation);
+      }
+      /* Always put in the line even if empty. Else, we don't know to which
+       * transport this applies to. */
+      smartlist_add_asprintf(string_chunks, "transport-info%s%s",
+                             version ? version: "", impl ? impl: "");
+      tor_free(version);
+      tor_free(impl);
+    }
   } SMARTLIST_FOREACH_END(mp);
 
   if (smartlist_len(string_chunks) == 0) {
diff --git a/src/feature/client/transports.h b/src/feature/client/transports.h
index 535689537c..71e7feea37 100644
--- a/src/feature/client/transports.h
+++ b/src/feature/client/transports.h
@@ -114,11 +114,19 @@ typedef struct {
   /* transports to-be-launched by this proxy */
   smartlist_t *transports_to_launch;
 
+  /** Version as set by STATUS TYPE=version messages. */
+  char *version;
+
+  /** Implementation as set by the STATUS TYPE=version messages. */
+  char *implementation;
+
   /* The 'transports' list contains all the transports this proxy has
      launched. */
   smartlist_t *transports;
 } managed_proxy_t;
 
+struct config_line_t;
+
 STATIC transport_t *transport_new(const tor_addr_t *addr, uint16_t port,
                                   const char *name, int socks_ver,
                                   const char *extra_info_args);
@@ -131,6 +139,8 @@ STATIC void parse_proxy_error(const char *line);
 STATIC void handle_proxy_line(const char *line, managed_proxy_t *mp);
 STATIC void parse_log_line(const char *line, managed_proxy_t *mp);
 STATIC void parse_status_line(const char *line, managed_proxy_t *mp);
+STATIC void handle_status_message(const struct config_line_t *values,
+                                  managed_proxy_t *mp);
 STATIC char *get_transport_options_for_server_proxy(const managed_proxy_t *mp);
 
 STATIC void managed_proxy_destroy(managed_proxy_t *mp,
diff --git a/src/test/test_pt.c b/src/test/test_pt.c
index 07c5032933..10bf7829d1 100644
--- a/src/test/test_pt.c
+++ b/src/test/test_pt.c
@@ -31,6 +31,9 @@ reset_mp(managed_proxy_t *mp)
   mp->conf_state = PT_PROTO_LAUNCHED;
   SMARTLIST_FOREACH(mp->transports, transport_t *, t, transport_free(t));
   smartlist_clear(mp->transports);
+
+  tor_free(mp->version);
+  tor_free(mp->implementation);
 }
 
 static void
@@ -143,6 +146,131 @@ test_pt_parsing(void *arg)
 }
 
 static void
+test_pt_status_parsing(void *arg)
+{
+  char line[200];
+  char *test_binary = tor_strdup("test-pt");
+  char *argv[] = {
+    test_binary,
+    NULL,
+  };
+
+  managed_proxy_t *mp = tor_malloc_zero(sizeof(managed_proxy_t));
+  (void)arg;
+  mp->conf_state = PT_PROTO_INFANT;
+  mp->transports = smartlist_new();
+  mp->argv = argv;
+
+  /* STATUS TYPE=version messages. */
+  tt_ptr_op(mp->version, OP_EQ, NULL);
+  tt_ptr_op(mp->implementation, OP_EQ, NULL);
+
+  /* Normal case. */
+  strlcpy(line, "STATUS "
+                "IMPLEMENTATION=xyz "
+                "TYPE=version "
+                "VERSION=\"1.33.7-hax beta\"",
+                sizeof(line));
+  handle_proxy_line(line, mp);
+
+  tt_str_op(mp->version, OP_EQ, "1.33.7-hax beta");
+  tt_str_op(mp->implementation, OP_EQ, "xyz");
+  reset_mp(mp);
+
+  /* Normal case but different case for TYPE value. */
+  strlcpy(line, "STATUS "
+                "IMPLEMENTATION=xyz "
+                "TYPE=vErSiON "
+                "VERSION=\"1.33.7-hax beta\"",
+                sizeof(line));
+  handle_proxy_line(line, mp);
+
+  tt_str_op(mp->version, OP_EQ, "1.33.7-hax beta");
+  tt_str_op(mp->implementation, OP_EQ, "xyz");
+  reset_mp(mp);
+
+  /* IMPLEMENTATION and VERSION set but no TYPE. */
+  strlcpy(line, "STATUS "
+                "IMPLEMENTATION=xyz "
+                "VERSION=\"1.33.7-hax beta\"",
+                sizeof(line));
+  handle_proxy_line(line, mp);
+
+  tt_assert(mp->version == NULL);
+  tt_assert(mp->implementation == NULL);
+  reset_mp(mp);
+
+  /* Multiple TYPE= is not allowed. */
+  strlcpy(line, "STATUS "
+                "IMPLEMENTATION=xyz "
+                "TYPE=version "
+                "VERSION=\"1.33.7-hax beta\" "
+                "TYPE=nothing",
+                sizeof(line));
+  handle_proxy_line(line, mp);
+
+  tt_assert(mp->version == NULL);
+  tt_assert(mp->implementation == NULL);
+  reset_mp(mp);
+
+  /* Multiple TYPE= is not allowed. */
+  strlcpy(line, "STATUS "
+                "IMPLEMENTATION=xyz "
+                "TYPE=version "
+                "VERSION=\"1.33.7-hax beta\" "
+                "TYPE=version",
+                sizeof(line));
+  handle_proxy_line(line, mp);
+
+  tt_assert(mp->version == NULL);
+  tt_assert(mp->implementation == NULL);
+  reset_mp(mp);
+
+  /* Missing VERSION. */
+  strlcpy(line, "STATUS "
+                "TYPE=version "
+                "IMPLEMENTATION=xyz ",
+                sizeof(line));
+  handle_proxy_line(line, mp);
+
+  tt_assert(mp->version == NULL);
+  tt_assert(mp->implementation == NULL);
+  reset_mp(mp);
+
+  /* Many IMPLEMENTATION and VERSION. First found are used. */
+  strlcpy(line, "STATUS "
+                "TYPE=version "
+                "IMPLEMENTATION=xyz "
+                "VERSION=\"1.33.7-hax beta\" "
+                "IMPLEMENTATION=abc "
+                "VERSION=\"2.33.7-hax beta\" ",
+                sizeof(line));
+  handle_proxy_line(line, mp);
+
+  tt_str_op(mp->version, OP_EQ, "1.33.7-hax beta");
+  tt_str_op(mp->implementation, OP_EQ, "xyz");
+  reset_mp(mp);
+
+  /* Control characters. Invalid input. */
+  strlcpy(line, "STATUS "
+                "TYPE=version "
+                "IMPLEMENTATION=xyz\0abc "
+                "VERSION=\"1.33.7-hax beta\"\0.3 ",
+                sizeof(line));
+  handle_proxy_line(line, mp);
+
+  tt_assert(mp->version == NULL);
+  tt_assert(mp->implementation == NULL);
+  reset_mp(mp);
+
+ done:
+  reset_mp(mp);
+  smartlist_free(mp->transports);
+  tor_free(mp);
+  tor_free(test_binary);
+}
+
+static void
 test_pt_get_transport_options(void *arg)
 {
   char **execve_args;
@@ -285,7 +413,9 @@ test_pt_get_extrainfo_string(void *arg)
   tt_assert(s);
   tt_str_op(s, OP_EQ,
             "transport hagbard 127.0.0.1:5555\n"
-            "transport celine 127.0.0.1:1723 card=no-enemy\n");
+            "transport-info\n"
+            "transport celine 127.0.0.1:1723 card=no-enemy\n"
+            "transport-info\n");
 
  done:
   /* XXXX clean up better */
@@ -590,6 +720,7 @@ test_get_pt_proxy_uri(void *arg)
 
 struct testcase_t pt_tests[] = {
   PT_LEGACY(parsing),
+  PT_LEGACY(status_parsing),
   PT_LEGACY(protocol),
   { "get_transport_options", test_pt_get_transport_options, TT_FORK,
     NULL, NULL },