Add changes file for tpo/core/tor#11101.

3 files changed, 111 insertions, 9 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 77cee25c54..403bedc183 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"
@@ -1307,6 +1308,11 @@ 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? */
@@ -1790,16 +1796,22 @@ pt_get_extra_info_descriptor_string(void)
       tor_free(transport_args);
     } SMARTLIST_FOREACH_END(t);
 
-    if (mp->version != NULL) {
-      smartlist_add_asprintf(string_chunks,
-                             "transport-version %s",
-                             mp->version);
-    }
+    /* Set transport-info line. */
+    {
+      char *transport_info_args = NULL;
 
-    if (mp->implementation != NULL) {
-      smartlist_add_asprintf(string_chunks,
-                             "transport-implementation %s",
-                             mp->implementation);
+      if (mp->version) {
+        tor_asprintf(&transport_info_args, " version=%s", mp->version);
+      }
+      if (mp->implementation) {
+        tor_asprintf(&transport_info_args, " implementation=%s",
+                     mp->implementation);
+      }
+      if (transport_info_args) {
+        smartlist_add_asprintf(string_chunks, "transport-info%s",
+                               transport_info_args ? transport_info_args : "");
+        tor_free(transport_info_args);
+      }
     }
   } SMARTLIST_FOREACH_END(mp);
 
diff --git a/src/test/test_pt.c b/src/test/test_pt.c
index 7725a82233..29146a4193 100644
--- a/src/test/test_pt.c
+++ b/src/test/test_pt.c
@@ -162,6 +162,7 @@ test_pt_status_parsing(void *arg)
   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 "
@@ -171,7 +172,92 @@ test_pt_status_parsing(void *arg)
 
   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: