From b6e260e69946dc6ce9c7de64438d5af29959d037 Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Thu, 24 Mar 2022 19:13:41 +0000 Subject: Add support for PT STATUS TYPE=version messages. This patch adds support for handling the version status message. Once we receive such message, we add the given version string to the managed_proxy_t instance. Note this value can be NULL and the value can change throughout the lifetime of the PT as multiple status version messages are handled. See: tpo/core/tor#11101 --- src/feature/client/transports.c | 36 ++++++++++++++++++++++++++++++++++++ src/feature/client/transports.h | 7 +++++++ src/test/test_pt.c | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index 4d92a2a67a..7c5132695a 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -741,6 +741,9 @@ 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); + /* 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 @@ -1293,6 +1296,9 @@ parse_status_line(const char *line, managed_proxy_t *mp) goto done; } + /* Handle the different messages. */ + handle_status_message(values, mp); + /* Prepend the PT name. */ config_line_prepend(&values, "PT", mp->argv[0]); status_message = kvline_encode(values, KV_QUOTED); @@ -1306,6 +1312,36 @@ 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) +{ + 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"); + + 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; + } + + tor_free(mp->version); + mp->version = tor_strdup(version->value); + + return; + } +} + /** Return a newly allocated string that tor should place in * TOR_PT_SERVER_TRANSPORT_OPTIONS while configuring the server * manged proxy in mp. Return NULL if no such options are found. */ diff --git a/src/feature/client/transports.h b/src/feature/client/transports.h index 535689537c..1f6c10961d 100644 --- a/src/feature/client/transports.h +++ b/src/feature/client/transports.h @@ -114,11 +114,16 @@ typedef struct { /* transports to-be-launched by this proxy */ smartlist_t *transports_to_launch; + /** Version as set by STATUS TYPE=version messages. */ + char *version; + /* 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 +136,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..e97b0b2087 100644 --- a/src/test/test_pt.c +++ b/src/test/test_pt.c @@ -142,6 +142,40 @@ test_pt_parsing(void *arg) tor_free(mp); } +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); + strlcpy(line, "STATUS TRANSPORT=x " + "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"); + + 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) { @@ -590,6 +624,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 }, -- cgit v1.2.3-54-g00ecf From d27ce6b8f0ad716597df888f19b7593a5a11f476 Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Fri, 21 Jul 2023 02:09:33 +0200 Subject: Drop requirement for TRANSPORT being present in STATUS messages. --- src/feature/client/transports.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index 7c5132695a..8c3b71f7d0 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -1286,16 +1286,6 @@ 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); -- cgit v1.2.3-54-g00ecf From f45934448849a931be09aed7dd0ac3d275b218e4 Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Fri, 21 Jul 2023 02:10:21 +0200 Subject: Include "IMPLEMENTATION" parameter to STATUS TYPE=version PT messages. --- src/feature/client/transports.c | 12 ++++++++++++ src/feature/client/transports.h | 3 +++ src/test/test_pt.c | 7 ++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index 8c3b71f7d0..416e538efc 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -743,6 +743,7 @@ managed_proxy_destroy(managed_proxy_t *mp, /* 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) { @@ -1318,6 +1319,8 @@ handle_status_message(const config_line_t *values, /* 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 " @@ -1325,9 +1328,18 @@ handle_status_message(const config_line_t *values, 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; } } diff --git a/src/feature/client/transports.h b/src/feature/client/transports.h index 1f6c10961d..71e7feea37 100644 --- a/src/feature/client/transports.h +++ b/src/feature/client/transports.h @@ -117,6 +117,9 @@ typedef struct { /** 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; diff --git a/src/test/test_pt.c b/src/test/test_pt.c index e97b0b2087..7725a82233 100644 --- a/src/test/test_pt.c +++ b/src/test/test_pt.c @@ -160,12 +160,17 @@ test_pt_status_parsing(void *arg) /* STATUS TYPE=version messages. */ tt_ptr_op(mp->version, OP_EQ, NULL); - strlcpy(line, "STATUS TRANSPORT=x " + tt_ptr_op(mp->implementation, OP_EQ, NULL); + + 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); -- cgit v1.2.3-54-g00ecf From b4f8518f8fdedc2711cbaad0abb5a4b85bf66fec Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Fri, 21 Jul 2023 02:11:16 +0200 Subject: Add implementation and version metadata to bridge extra-info. This patch adds two new keys to bridges' extra-info document: "transport-version" and "transport-implementation". These two new values always appear together (if one is missing, the other one will be missing too) and is parsed from PT's STATUS TYPE=version messages. See: tpo/core/tor#11101. --- src/feature/client/transports.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index 416e538efc..77cee25c54 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -1786,9 +1786,21 @@ 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); + if (mp->version != NULL) { + smartlist_add_asprintf(string_chunks, + "transport-version %s", + mp->version); + } + + if (mp->implementation != NULL) { + smartlist_add_asprintf(string_chunks, + "transport-implementation %s", + mp->implementation); + } } SMARTLIST_FOREACH_END(mp); if (smartlist_len(string_chunks) == 0) { -- cgit v1.2.3-54-g00ecf From 3c8035b452579824f7e171846ab3ec33248d753f Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Fri, 21 Jul 2023 02:15:40 +0200 Subject: Add changes file for tpo/core/tor#11101. --- changes/ticket11101 | 4 ++ src/feature/client/transports.c | 30 +++++++++----- src/test/test_pt.c | 86 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 changes/ticket11101 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: -- cgit v1.2.3-54-g00ecf From 1941f25f4ccdab3f0a94ccac8232c958153e3f2a Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Fri, 21 Jul 2023 02:53:12 +0200 Subject: Clean-up Managed Proxy state in PT tests. This patch makes sure we clean up our version and implementation fields in our Managed Proxy struct after each test run. This was detected by LeakSanitizer. See: tpo/core/tor#11101. --- src/test/test_pt.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/test_pt.c b/src/test/test_pt.c index 29146a4193..62519a235f 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 -- cgit v1.2.3-54-g00ecf From d587ba01a70b81e8c15f6e53e72c133ebe977719 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 11 Jun 2024 12:38:56 -0400 Subject: bridge: Always put transport-info line Signed-off-by: David Goulet --- src/feature/client/transports.c | 19 ++++++++++--------- src/test/test_pt.c | 4 +++- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index 403bedc183..878675ac4d 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -1798,20 +1798,21 @@ pt_get_extra_info_descriptor_string(void) /* Set transport-info line. */ { - char *transport_info_args = NULL; + char *version = NULL; + char *impl = NULL; if (mp->version) { - tor_asprintf(&transport_info_args, " version=%s", mp->version); + tor_asprintf(&version, " 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); + 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); diff --git a/src/test/test_pt.c b/src/test/test_pt.c index 62519a235f..10bf7829d1 100644 --- a/src/test/test_pt.c +++ b/src/test/test_pt.c @@ -413,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 */ -- cgit v1.2.3-54-g00ecf