From b8532bcb1ec51fcfae4ceff869be116fec4ccbb9 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 17 Dec 2012 14:14:09 +0200 Subject: [PATCH 1/9] Add utility functions needed for SOCKS argument parsing. --- src/common/util.c | 67 ++++++++++++++++++++++++++++++++++++++++++++ src/common/util.h | 4 +++ src/test/test_util.c | 50 +++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+) diff --git a/src/common/util.c b/src/common/util.c index 93e2ba8e14..b2f12bfb6a 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -865,6 +865,36 @@ tor_digest_is_zero(const char *digest) return tor_memeq(digest, ZERO_DIGEST, DIGEST_LEN); } +/** Return true if string is a valid '=' string. + * is optional, to indicate the empty string. */ +int +string_is_key_value(const char *string) +{ + /* position of equal sign in string */ + char *equal_sign_pos = NULL; + + tor_assert(string); + + if (strlen(string) < 2) { /* "x=a" is shortest args string */ + log_warn(LD_GENERAL, "'%s' is too short to be a k=v value.", string); + return 0; + } + + equal_sign_pos = strchr(string, '='); + if (!equal_sign_pos) { + log_warn(LD_GENERAL, "'%s' is not a k=v value.", string); + return 0; + } + + /* validate that the '=' is not in the beginning of the string. */ + if (equal_sign_pos == string) { + log_warn(LD_GENERAL, "'%s' is not a valid k=v value.", string); + return 0; + } + + return 1; +} + /** Return true iff the DIGEST256_LEN bytes in digest are all zero. */ int tor_digest256_is_zero(const char *digest) @@ -1249,6 +1279,43 @@ wrap_string(smartlist_t *out, const char *string, size_t width, } } +/** Escape every character of string that belongs to the set of + * characters set. Use escape_char as the character to + * use for escaping. */ +char * +tor_escape_str_for_socks_arg(const char *string) +{ + char *new_string = NULL; + char *new_cp = NULL; + size_t length, new_length; + static const char *chars_to_escape = ";\\"; + + tor_assert(string); + + length = strlen(string); + + if (!length) + return NULL; + /* (new_length > SIZE_MAX) => ((length * 2) + 1 > SIZE_MAX) => + (length*2 > SIZE_MAX - 1) => (length > (SIZE_MAX - 1)/2) */ + if (length > (SIZE_MAX - 1)/2) /* check for overflow */ + return NULL; + + /* this should be enough even if all characters must be escaped */ + new_length = (length * 2) + 1; + + new_string = new_cp = tor_malloc_zero(new_length); + + while (*string) { + if (strchr(chars_to_escape, *string)) + *new_cp++ = '\\'; + + *new_cp++ = *string++; + } + + return new_string; +} + /* ===== * Time * ===== */ diff --git a/src/common/util.h b/src/common/util.h index 911b1b5a37..e3cd72118c 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -209,12 +209,16 @@ const char *find_whitespace_eos(const char *s, const char *eos); const char *find_str_at_start_of_line(const char *haystack, const char *needle); int string_is_C_identifier(const char *string); +int string_is_key_value(const char *string); int tor_mem_is_zero(const char *mem, size_t len); int tor_digest_is_zero(const char *digest); int tor_digest256_is_zero(const char *digest); char *esc_for_log(const char *string) ATTR_MALLOC; const char *escaped(const char *string); + +char *tor_escape_str_for_socks_arg(const char *string); + struct smartlist_t; void wrap_string(struct smartlist_t *out, const char *string, size_t width, const char *prefix0, const char *prefixRest); diff --git a/src/test/test_util.c b/src/test/test_util.c index bed33fac25..b41f23571b 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -795,6 +795,54 @@ test_util_expand_filename(void) } #endif +/** Test tor_escape_str_for_socks_arg(). */ +static void +test_util_escape_string_socks(void) +{ + char *escaped_string = NULL; + + /** Simple backslash escape. */ + escaped_string = tor_escape_str_for_socks_arg("This is a backslash: \\"); + test_assert(escaped_string); + test_streq(escaped_string, "This is a backslash: \\\\"); + tor_free(escaped_string); + + /** Simple semicolon escape. */ + escaped_string = tor_escape_str_for_socks_arg("First rule: Do not use ;"); + test_assert(escaped_string); + test_streq(escaped_string, "First rule: Do not use \\;"); + tor_free(escaped_string); + + /** Ilegal: Empty string. */ + escaped_string = tor_escape_str_for_socks_arg(""); + test_assert(!escaped_string); + + /** Escape all characters. */ + escaped_string = tor_escape_str_for_socks_arg(";\\;\\"); + test_assert(escaped_string); + test_streq(escaped_string, "\\;\\\\\\;\\\\"); + tor_free(escaped_string); + + done: + tor_free(escaped_string); +} + +static void +test_util_string_is_key_value(void *ptr) +{ + (void)ptr; + test_assert(string_is_key_value("key=value")); + test_assert(string_is_key_value("k=v")); + test_assert(string_is_key_value("key=")); + test_assert(!string_is_key_value("=value")); + test_assert(!string_is_key_value("=")); + + /* ??? */ + /* test_assert(!string_is_key_value("===")); */ + done: + ; +} + /** Test basic string functionality. */ static void test_util_strmisc(void) @@ -3271,6 +3319,8 @@ struct testcase_t util_tests[] = { #ifndef _WIN32 UTIL_LEGACY(expand_filename), #endif + UTIL_LEGACY(escape_string_socks), + UTIL_LEGACY(string_is_key_value), UTIL_LEGACY(strmisc), UTIL_LEGACY(pow2), UTIL_LEGACY(gzip), From 757b03aacbf7051194bbe9faa2bfcc59e4cc3392 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 17 Dec 2012 14:31:13 +0200 Subject: [PATCH 2/9] Add support for parsing SOCKS arguments. --- src/or/config.c | 71 ++++++++++++++++++++++++++++++++++++++++----- src/or/entrynodes.c | 20 +++++++++++-- src/or/entrynodes.h | 4 ++- 3 files changed, 84 insertions(+), 11 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 31695baa73..3e7535966c 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -4036,10 +4036,11 @@ parse_bridge_line(const char *line, int validate_only) int r; char *addrport=NULL, *fingerprint=NULL; char *transport_name=NULL; - char *field1=NULL; + char *field=NULL; tor_addr_t addr; uint16_t port = 0; char digest[DIGEST_LEN]; + smartlist_t *socks_args = NULL; items = smartlist_new(); smartlist_split_string(items, line, NULL, @@ -4049,13 +4050,13 @@ parse_bridge_line(const char *line, int validate_only) goto err; } - /* field1 is either a transport name or addrport */ - field1 = smartlist_get(items, 0); + /* field is either a transport name or addrport */ + field = smartlist_get(items, 0); smartlist_del_keeporder(items, 0); - if (!(strstr(field1, ".") || strstr(field1, ":"))) { + if (!(strstr(field, ".") || strstr(field, ":"))) { /* new-style bridge line */ - transport_name = field1; + transport_name = field; if (smartlist_len(items) < 1) { log_warn(LD_CONFIG, "Too few items to Bridge line."); goto err; @@ -4063,7 +4064,7 @@ parse_bridge_line(const char *line, int validate_only) addrport = smartlist_get(items, 0); smartlist_del_keeporder(items, 0); } else { - addrport = field1; + addrport = field; } if (tor_addr_port_lookup(addrport, &addr, &port)<0) { @@ -4077,8 +4078,28 @@ parse_bridge_line(const char *line, int validate_only) port = 443; } + /* If transports are enabled, next field could be a fingerprint or a + socks argument. If transports are disabled, next field should be + a fingerprint. */ if (smartlist_len(items)) { - fingerprint = smartlist_join_strings(items, "", 0, NULL); + if (transport_name) { /* transports enabled: */ + field = smartlist_get(items, 0); + smartlist_del_keeporder(items, 0); + + /* If '=', it's a k=v value pair. */ + if (strchr(field, '=')) { + socks_args = smartlist_new(); + smartlist_add(socks_args, field); + } else { /* If no '=', it's the fingerprint. */ + fingerprint = field; + } + + } else { /* transports disabled: */ + fingerprint = smartlist_join_strings(items, "", 0, NULL); + } + } + + if (fingerprint) { if (strlen(fingerprint) != HEX_DIGEST_LEN) { log_warn(LD_CONFIG, "Key digest for Bridge is wrong length."); goto err; @@ -4089,13 +4110,39 @@ parse_bridge_line(const char *line, int validate_only) } } + /* If we are using transports, any remaining items in the smartlist + must be k=v values. */ + if (transport_name && smartlist_len(items)) { + if (!socks_args) + socks_args = smartlist_new(); + + /* append remaining items of 'items' to 'socks_args' */ + smartlist_add_all(socks_args, items); + smartlist_clear(items); + + tor_assert(smartlist_len(socks_args) > 0); + } + if (!validate_only) { log_debug(LD_DIR, "Bridge at %s (transport: %s) (%s)", fmt_addrport(&addr, port), transport_name ? transport_name : "no transport", fingerprint ? fingerprint : "no key listed"); + + if (socks_args) { /* print socks arguments */ + int i = 0; + + tor_assert(smartlist_len(socks_args) > 0); + + log_debug(LD_DIR, "Bridge uses %d SOCKS arguments:", + smartlist_len(socks_args)); + SMARTLIST_FOREACH(socks_args, const char *, arg, + log_debug(LD_CONFIG, "%d: %s", ++i, arg)); + } + bridge_add_from_config(&addr, port, - fingerprint ? digest : NULL, transport_name); + fingerprint ? digest : NULL, + transport_name, socks_args); } r = 0; @@ -4110,6 +4157,14 @@ parse_bridge_line(const char *line, int validate_only) tor_free(addrport); tor_free(transport_name); tor_free(fingerprint); + + /* We only have to free socks_args if we are validating, since + otherwise bridge_add_from_config() steals its reference. */ + if (socks_args && validate_only) { + SMARTLIST_FOREACH(socks_args, char*, s, tor_free(s)); + smartlist_free(socks_args); + } + return r; } diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 4ca56cbacf..63545ce9b8 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -52,6 +52,10 @@ typedef struct { /** When should we next try to fetch a descriptor for this bridge? */ download_status_t fetch_status; + + /** A smartlist of k=v values to be passed to the SOCKS proxy, if + transports are used for this bridge. */ + smartlist_t *socks_args; } bridge_info_t; /** A list of our chosen entry guards. */ @@ -1446,6 +1450,11 @@ bridge_free(bridge_info_t *bridge) return; tor_free(bridge->transport_name); + if (bridge->socks_args) { + SMARTLIST_FOREACH(bridge->socks_args, char*, s, tor_free(s)); + smartlist_free(bridge->socks_args); + } + tor_free(bridge); } @@ -1628,10 +1637,16 @@ bridge_resolve_conflicts(const tor_addr_t *addr, uint16_t port, * is set, it tells us the identity key too. If we already had the * bridge in our list, unmark it, and don't actually add anything new. * If transport_name is non-NULL - the bridge is associated with a - * pluggable transport - we assign the transport to the bridge. */ + * pluggable transport - we assign the transport to the bridge. + * If transport_name is non-NULL - the bridge is associated + * with a pluggable transport - we assign the transport to the bridge. + * If socks_args is non-NULL, it's a smartlist carrying + * key=value pairs to be passed to the pluggable transports + * proxy. This function steals reference of the smartlist. */ void bridge_add_from_config(const tor_addr_t *addr, uint16_t port, - const char *digest, const char *transport_name) + const char *digest, const char *transport_name, + smartlist_t *socks_args) { bridge_info_t *b; @@ -1645,6 +1660,7 @@ bridge_add_from_config(const tor_addr_t *addr, uint16_t port, if (transport_name) b->transport_name = tor_strdup(transport_name); b->fetch_status.schedule = DL_SCHED_BRIDGE; + b->socks_args = socks_args; if (!bridge_list) bridge_list = smartlist_new(); diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index b673d02681..6865231f57 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -97,9 +97,11 @@ int routerinfo_is_a_configured_bridge(const routerinfo_t *ri); int node_is_a_configured_bridge(const node_t *node); void learned_router_identity(const tor_addr_t *addr, uint16_t port, const char *digest); +struct smartlist_t; void bridge_add_from_config(const tor_addr_t *addr, uint16_t port, const char *digest, - const char *transport_name); + const char *transport_name, + struct smartlist_t *socks_args); void retry_bridge_descriptor_fetch_directly(const char *digest); void fetch_bridge_descriptors(const or_options_t *options, time_t now); void learned_bridge_descriptor(routerinfo_t *ri, int from_cache); From faf4f6c6d1da54b0a6b0c9946112f2e448867a8f Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 17 Dec 2012 14:39:24 +0200 Subject: [PATCH 3/9] Validate SOCKS arguments. --- src/or/config.c | 39 +++++++++++++++++++++++++++++++++++++++ src/or/connection.h | 8 ++++++++ src/or/transports.c | 35 +++++++++++++++++++++++++++++++++++ src/or/transports.h | 1 + 4 files changed, 83 insertions(+) diff --git a/src/or/config.c b/src/or/config.c index 3e7535966c..d057dd8ae3 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -4025,6 +4025,40 @@ options_init_logs(or_options_t *options, int validate_only) return ok?0:-1; } +/** Given a smartlist of SOCKS arguments to be passed to a transport + * proxy in args, validate them and return -1 if they are + * corrupted. Return 0 if they seem OK. */ +static int +validate_transport_socks_arguments(const smartlist_t *args) +{ + char *socks_string = NULL; + size_t socks_string_len; + + tor_assert(args); + tor_assert(smartlist_len(args) > 0); + + SMARTLIST_FOREACH_BEGIN(args, const char *, s) { + if (!string_is_key_value(s)) /* arguments should be k=v items */ + return -1; + } SMARTLIST_FOREACH_END(s); + + socks_string = pt_stringify_socks_args(args); + if (!socks_string) + return -1; + + socks_string_len = strlen(socks_string); + tor_free(socks_string); + + if (socks_string_len > MAX_SOCKS5_AUTH_SIZE_TOTAL) { + log_warn(LD_CONFIG, "SOCKS arguments can't be more than %u bytes (%lu).", + MAX_SOCKS5_AUTH_SIZE_TOTAL, + (unsigned long) socks_string_len); + return -1; + } + + return 0; +} + /** Read the contents of a Bridge line from line. Return 0 * if the line is well-formed, and -1 if it isn't. If * validate_only is 0, and the line is well-formed, then add @@ -4143,6 +4177,11 @@ parse_bridge_line(const char *line, int validate_only) bridge_add_from_config(&addr, port, fingerprint ? digest : NULL, transport_name, socks_args); + } else { + if (socks_args) { + if (validate_transport_socks_arguments(socks_args) < 0) + goto err; + } } r = 0; diff --git a/src/or/connection.h b/src/or/connection.h index c78fe6e652..3e656ec06e 100644 --- a/src/or/connection.h +++ b/src/or/connection.h @@ -89,6 +89,14 @@ int connection_connect(connection_t *conn, const char *address, const tor_addr_t *addr, uint16_t port, int *socket_error); +/** Maximum size of information that we can fit into SOCKS5 username + or password fields. */ +#define MAX_SOCKS5_AUTH_FIELD_SIZE 255 + +/** Total maximum size of information that we can fit into SOCKS5 + username and password fields. */ +#define MAX_SOCKS5_AUTH_SIZE_TOTAL 2*MAX_SOCKS5_AUTH_FIELD_SIZE + int connection_proxy_connect(connection_t *conn, int type); int connection_read_proxy_handshake(connection_t *conn); void log_failed_proxy_connection(connection_t *conn); diff --git a/src/or/transports.c b/src/or/transports.c index 647b293168..a9f7fa2bf5 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -1424,6 +1424,41 @@ pt_get_extra_info_descriptor_string(void) return the_string; } +/** Stringify the SOCKS arguments in socks_args according to + * 180_pluggable_transport.txt. The string is allocated on the heap + * and it's the responsibility of the caller to free it after use. */ +char * +pt_stringify_socks_args(const smartlist_t *socks_args) +{ + /* tmp place to store escaped socks arguments, so that we can + concatenate them up afterwards */ + smartlist_t *sl_tmp = NULL; + char *escaped_string = NULL; + char *new_string = NULL; + + tor_assert(socks_args); + tor_assert(smartlist_len(socks_args) > 0); + + sl_tmp = smartlist_new(); + + SMARTLIST_FOREACH_BEGIN(socks_args, const char *, s) { + /* Escape ';' and '\'. */ + escaped_string = tor_escape_str_for_socks_arg(s); + if (!escaped_string) + goto done; + + smartlist_add(sl_tmp, escaped_string); + } SMARTLIST_FOREACH_END(s); + + new_string = smartlist_join_strings(sl_tmp, ";", 0, NULL); + + done: + SMARTLIST_FOREACH(sl_tmp, char *, s, tor_free(s)); + smartlist_free(sl_tmp); + + return new_string; +} + /** The tor config was read. * Destroy all managed proxies that were marked by a previous call to * prepare_proxy_list_for_config_read() and are not used by the new diff --git a/src/or/transports.h b/src/or/transports.h index 6ee82f4556..16fd5d44a9 100644 --- a/src/or/transports.h +++ b/src/or/transports.h @@ -55,6 +55,7 @@ void pt_prepare_proxy_list_for_config_read(void); void sweep_proxy_list(void); smartlist_t *get_transport_proxy_ports(void); +char *pt_stringify_socks_args(const smartlist_t *socks_args); #ifdef PT_PRIVATE /** State of the managed proxy configuration protocol. */ From 14b84858c062955dc52fbcdcf1146ce0031a95da Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 17 Dec 2012 15:01:31 +0200 Subject: [PATCH 4/9] Send SOCKS arguments when doing SOCKS4. --- src/or/connection.c | 92 ++++++++++++++++++++++++++++++++++----------- src/or/entrynodes.c | 11 ++++++ src/or/entrynodes.h | 3 ++ src/or/or.h | 4 +- src/or/transports.c | 17 +++++++++ src/or/transports.h | 3 ++ 6 files changed, 107 insertions(+), 23 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index 7b0f081fde..5860d5f40e 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -44,6 +44,7 @@ #include "router.h" #include "transports.h" #include "routerparse.h" +#include "transports.h" #ifdef USE_BUFFEREVENTS #include @@ -1560,6 +1561,32 @@ connection_proxy_state_to_string(int state) return states[state]; } +/** Returns the global proxy type used by tor. Use this function for + * logging or high-level purposes, don't use it to fill the + * proxy_type field of or_connection_t; use the actual proxy + * protocol instead.*/ +static int +get_proxy_type(void) +{ + const or_options_t *options = get_options(); + + if (options->HTTPSProxy) + return PROXY_CONNECT; + else if (options->Socks4Proxy) + return PROXY_SOCKS4; + else if (options->Socks5Proxy) + return PROXY_SOCKS5; + else if (options->ClientTransportPlugin) + return PROXY_PLUGGABLE; + else + return PROXY_NONE; +} + +/* One byte for the version, one for the command, two for the + port, and four for the addr... and, one more for the + username NUL: */ +#define SOCKS4_STANDARD_BUFFER_SIZE 1 + 1 + 2 + 4 + 1 + /** Write a proxy request of type (socks4, socks5, https) to conn * for conn->addr:conn->port, authenticating with the auth details given * in the configuration (if available). SOCKS 5 and HTTP CONNECT proxies @@ -1614,17 +1641,45 @@ connection_proxy_connect(connection_t *conn, int type) } case PROXY_SOCKS4: { - unsigned char buf[9]; + unsigned char *buf; uint16_t portn; uint32_t ip4addr; + size_t buf_size = 0; + char *socks_args_string = NULL; - /* Send a SOCKS4 connect request with empty user id */ + /* Send a SOCKS4 connect request */ if (tor_addr_family(&conn->addr) != AF_INET) { log_warn(LD_NET, "SOCKS4 client is incompatible with IPv6"); return -1; } + { /* If we are here because we are trying to connect to a + pluggable transport proxy, check if we have any SOCKS + arguments to transmit. If we do, compress all arguments to + a single string in 'socks_args_string': */ + + if (get_proxy_type() == PROXY_PLUGGABLE) { + socks_args_string = + pt_get_socks_args_for_proxy_addrport(&conn->addr, conn->port); + if (socks_args_string) + log_debug(LD_NET, "Sending out '%s' as our SOCKS argument string.", + socks_args_string); + } + } + + { /* Figure out the buffer size we need for the SOCKS message: */ + + buf_size = SOCKS4_STANDARD_BUFFER_SIZE; + + /* If we have a SOCKS argument string, consider its size when + calculating the buffer size: */ + if (socks_args_string) + buf_size += strlen(socks_args_string); + } + + buf = tor_malloc_zero(buf_size); + ip4addr = tor_addr_to_ipv4n(&conn->addr); portn = htons(conn->port); @@ -1632,9 +1687,20 @@ connection_proxy_connect(connection_t *conn, int type) buf[1] = SOCKS_COMMAND_CONNECT; /* command */ memcpy(buf + 2, &portn, 2); /* port */ memcpy(buf + 4, &ip4addr, 4); /* addr */ - buf[8] = 0; /* userid (empty) */ - connection_write_to_buf((char *)buf, sizeof(buf), conn); + if (socks_args_string) { /* place the SOCKS args string: */ + tor_assert(strlen(socks_args_string) > 0); + tor_assert(buf_size >= + SOCKS4_STANDARD_BUFFER_SIZE + strlen(socks_args_string)); + strlcpy((char *)buf + 8, socks_args_string, buf_size - 8); + tor_free(socks_args_string); + } else { + buf[8] = 0; /* no userid */ + } + + connection_write_to_buf((char *)buf, buf_size, conn); + tor_free(buf); + conn->proxy_state = PROXY_SOCKS4_WANT_CONNECT_OK; break; } @@ -4339,24 +4405,6 @@ get_proxy_addrport(tor_addr_t *addr, uint16_t *port, int *proxy_type, return 0; } -/** Returns the global proxy type used by tor. */ -static int -get_proxy_type(void) -{ - const or_options_t *options = get_options(); - - if (options->HTTPSProxy) - return PROXY_CONNECT; - else if (options->Socks4Proxy) - return PROXY_SOCKS4; - else if (options->Socks5Proxy) - return PROXY_SOCKS5; - else if (options->ClientTransportPlugin) - return PROXY_PLUGGABLE; - else - return PROXY_NONE; -} - /** Log a failed connection to a proxy server. * conn is the connection we use the proxy server for. */ void diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 63545ce9b8..9e9379c049 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1751,6 +1751,17 @@ find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port, return 0; } +/** Return a smartlist containing all the SOCKS arguments that we + * should pass to the SOCKS proxy. */ +const smartlist_t * +get_socks_args_by_bridge_addrport(const tor_addr_t *addr, uint16_t port) +{ + bridge_info_t *bridge = get_configured_bridge_by_addr_port_digest(addr, + port, + NULL); + return bridge ? bridge->socks_args : NULL; +} + /** We need to ask bridge for its server descriptor. */ static void launch_direct_bridge_descriptor_fetch(bridge_info_t *bridge) diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 6865231f57..ddf386c0d4 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -110,6 +110,9 @@ int any_pending_bridge_descriptor_fetches(void); int entries_known_but_down(const or_options_t *options); void entries_retry_all(const or_options_t *options); +const smartlist_t *get_socks_args_by_bridge_addrport(const tor_addr_t *addr, + uint16_t port); + int any_bridges_dont_support_microdescriptors(void); void entry_guards_free_all(void); diff --git a/src/or/or.h b/src/or/or.h index 04640d050a..2cb8b0e570 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -240,7 +240,9 @@ typedef enum { #define PROXY_SOCKS5 3 /* !!!! If there is ever a PROXY_* type over 2, we must grow the proxy_type * field in or_connection_t */ -/* pluggable transports proxy type */ + +/* Pluggable transport proxy type. Don't use this in or_connection_t, + * instead use the actual underlying proxy type (see above). */ #define PROXY_PLUGGABLE 4 /* Proxy client handshake states */ diff --git a/src/or/transports.c b/src/or/transports.c index a9f7fa2bf5..ebc3864e73 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -95,6 +95,7 @@ #include "util.h" #include "router.h" #include "statefile.h" +#include "entrynodes.h" static process_environment_t * create_managed_proxy_environment(const managed_proxy_t *mp); @@ -1459,6 +1460,22 @@ pt_stringify_socks_args(const smartlist_t *socks_args) return new_string; } +/** Return a string of the SOCKS arguments that we should pass to the + * pluggable transports proxy in addr:port according to + * 180_pluggable_transport.txt. The string is allocated on the heap + * and it's the responsibility of the caller to free it after use. */ +char * +pt_get_socks_args_for_proxy_addrport(const tor_addr_t *addr, uint16_t port) +{ + const smartlist_t *socks_args = NULL; + + socks_args = get_socks_args_by_bridge_addrport(addr, port); + if (!socks_args) + return NULL; + + return pt_stringify_socks_args(socks_args); +} + /** The tor config was read. * Destroy all managed proxies that were marked by a previous call to * prepare_proxy_list_for_config_read() and are not used by the new diff --git a/src/or/transports.h b/src/or/transports.h index 16fd5d44a9..4a5498cb58 100644 --- a/src/or/transports.h +++ b/src/or/transports.h @@ -57,6 +57,9 @@ void sweep_proxy_list(void); smartlist_t *get_transport_proxy_ports(void); char *pt_stringify_socks_args(const smartlist_t *socks_args); +char *pt_get_socks_args_for_proxy_addrport(const tor_addr_t *addr, + uint16_t port); + #ifdef PT_PRIVATE /** State of the managed proxy configuration protocol. */ enum pt_proto_state { From 8f2e98015989faf708a5294c3028a319fd45f16c Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 17 Dec 2012 15:07:26 +0200 Subject: [PATCH 5/9] Send SOCKS arguments when doing SOCKS5. --- src/or/connection.c | 58 ++++++++++++++++++++++++++++++++++++++------- src/or/entrynodes.c | 2 +- src/or/entrynodes.h | 2 +- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index 5860d5f40e..6bac59b20c 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1712,8 +1712,13 @@ connection_proxy_connect(connection_t *conn, int type) buf[0] = 5; /* version */ + /* We have to use SOCKS5 authentication, if we have a + Socks5ProxyUsername or if we want to pass arguments to our + pluggable transport proxy: */ + if ((options->Socks5ProxyUsername) || + (get_proxy_type() == PROXY_PLUGGABLE && + (get_socks_args_by_bridge_addrport(&conn->addr, conn->port)))) { /* number of auth methods */ - if (options->Socks5ProxyUsername) { buf[1] = 2; buf[2] = 0x00; /* no authentication */ buf[3] = 0x02; /* rfc1929 Username/Passwd auth */ @@ -1907,15 +1912,47 @@ connection_read_proxy_handshake(connection_t *conn) unsigned char buf[1024]; size_t reqsize, usize, psize; const char *user, *pass; + char *socks_args_string = NULL; - user = get_options()->Socks5ProxyUsername; - pass = get_options()->Socks5ProxyPassword; - tor_assert(user && pass); + if (get_proxy_type() == PROXY_PLUGGABLE) { + socks_args_string = + pt_get_socks_args_for_proxy_addrport(&conn->addr, conn->port); + if (!socks_args_string) { + log_warn(LD_NET, "Could not create SOCKS args string."); + ret = -1; + break; + } - /* XXX len of user and pass must be <= 255 !!! */ - usize = strlen(user); - psize = strlen(pass); - tor_assert(usize <= 255 && psize <= 255); + log_debug(LD_NET, "SOCKS5 arguments: %s", socks_args_string); + tor_assert(strlen(socks_args_string) > 0); + tor_assert(strlen(socks_args_string) <= MAX_SOCKS5_AUTH_SIZE_TOTAL); + + if (strlen(socks_args_string) > MAX_SOCKS5_AUTH_FIELD_SIZE) { + user = socks_args_string; + usize = MAX_SOCKS5_AUTH_FIELD_SIZE; + pass = socks_args_string + MAX_SOCKS5_AUTH_FIELD_SIZE; + psize = strlen(socks_args_string) - MAX_SOCKS5_AUTH_FIELD_SIZE; + } else { + user = socks_args_string; + usize = strlen(socks_args_string); + pass = "\0"; + psize = 1; + } + } else if (get_options()->Socks5ProxyUsername) { + user = get_options()->Socks5ProxyUsername; + pass = get_options()->Socks5ProxyPassword; + tor_assert(user && pass); + usize = strlen(user); + psize = strlen(pass); + } else { + log_err(LD_BUG, "We entered %s for no reason!", __func__); + tor_fragile_assert(); + ret = -1; + break; + } + + tor_assert(usize <= MAX_SOCKS5_AUTH_FIELD_SIZE && + psize <= MAX_SOCKS5_AUTH_FIELD_SIZE); reqsize = 3 + usize + psize; buf[0] = 1; /* negotiation version */ @@ -1924,6 +1961,9 @@ connection_read_proxy_handshake(connection_t *conn) buf[2 + usize] = psize; memcpy(buf + 3 + usize, pass, psize); + if (socks_args_string) + tor_free(socks_args_string); + connection_write_to_buf((char *)buf, reqsize, conn); conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_RFC1929_OK; @@ -4390,7 +4430,7 @@ get_proxy_addrport(tor_addr_t *addr, uint16_t *port, int *proxy_type, options->Bridges) { const transport_t *transport = NULL; int r; - r = find_transport_by_bridge_addrport(&conn->addr, conn->port, &transport); + r = get_transport_by_bridge_addrport(&conn->addr, conn->port, &transport); if (r<0) return -1; if (transport) { /* transport found */ diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 9e9379c049..a07670bbd8 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1724,7 +1724,7 @@ find_transport_name_by_bridge_addrport(const tor_addr_t *addr, uint16_t port) * transport, but the transport could not be found. */ int -find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port, +get_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port, const transport_t **transport) { *transport = NULL; diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index ddf386c0d4..48f678a184 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -120,7 +120,7 @@ void entry_guards_free_all(void); const char *find_transport_name_by_bridge_addrport(const tor_addr_t *addr, uint16_t port); struct transport_t; -int find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port, +int get_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port, const struct transport_t **transport); int validate_pluggable_transports_config(void); From d54efda869ee522d81bc0ccb80820f46c4f1439e Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 17 Dec 2012 15:17:19 +0200 Subject: [PATCH 6/9] Add changes files. --- changes/bug3594 | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changes/bug3594 diff --git a/changes/bug3594 b/changes/bug3594 new file mode 100644 index 0000000000..110252d008 --- /dev/null +++ b/changes/bug3594 @@ -0,0 +1,3 @@ + o Major bugfixes: + - Add support for passing arguments to managed pluggable transport + proxies. Implements ticket #3594. From b5dceab1751dfa12b27b3042a49d90e0b02c2e0c Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Sat, 9 Feb 2013 18:46:10 +0000 Subject: [PATCH 7/9] Fix various issues pointed out by Nick and Andrea. - Document the key=value format. - Constify equal_sign_pos. - Pass some strings that are about to be logged to escape(). - Update documentation and fix some bugs in tor_escape_str_for_socks_arg(). - Use string_is_key_value() in parse_bridge_line(). - Parenthesize a forgotten #define - Add some more comments. - Add some more unit test cases. --- src/common/util.c | 27 +++++++++++++++------------ src/or/config.c | 11 ++++++----- src/or/connection.c | 7 ++++++- src/test/test_util.c | 14 ++++++++++++-- 4 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index b2f12bfb6a..9aba7d6c5d 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -865,30 +865,30 @@ tor_digest_is_zero(const char *digest) return tor_memeq(digest, ZERO_DIGEST, DIGEST_LEN); } -/** Return true if string is a valid '=' string. +/** Return true if string is a valid '=[]' string. * is optional, to indicate the empty string. */ int string_is_key_value(const char *string) { /* position of equal sign in string */ - char *equal_sign_pos = NULL; + const char *equal_sign_pos = NULL; tor_assert(string); - if (strlen(string) < 2) { /* "x=a" is shortest args string */ - log_warn(LD_GENERAL, "'%s' is too short to be a k=v value.", string); + if (strlen(string) < 2) { /* "x=" is shortest args string */ + log_warn(LD_GENERAL, "'%s' is too short to be a k=v value.", escaped(string)); return 0; } equal_sign_pos = strchr(string, '='); if (!equal_sign_pos) { - log_warn(LD_GENERAL, "'%s' is not a k=v value.", string); + log_warn(LD_GENERAL, "'%s' is not a k=v value.", escaped(string)); return 0; } /* validate that the '=' is not in the beginning of the string. */ if (equal_sign_pos == string) { - log_warn(LD_GENERAL, "'%s' is not a valid k=v value.", string); + log_warn(LD_GENERAL, "'%s' is not a valid k=v value.", escaped(string)); return 0; } @@ -1279,9 +1279,10 @@ wrap_string(smartlist_t *out, const char *string, size_t width, } } -/** Escape every character of string that belongs to the set of - * characters set. Use escape_char as the character to - * use for escaping. */ +/** Escape every ";" or "\" character of string. Use + * escape_char as the character to use for escaping. + * The returned string is allocated on the heap and it's the + * responsibility of the caller to free it. */ char * tor_escape_str_for_socks_arg(const char *string) { @@ -1294,8 +1295,8 @@ tor_escape_str_for_socks_arg(const char *string) length = strlen(string); - if (!length) - return NULL; + if (!length) /* If we were given the empty string, return the same. */ + return tor_strdup(""); /* (new_length > SIZE_MAX) => ((length * 2) + 1 > SIZE_MAX) => (length*2 > SIZE_MAX - 1) => (length > (SIZE_MAX - 1)/2) */ if (length > (SIZE_MAX - 1)/2) /* check for overflow */ @@ -1304,7 +1305,7 @@ tor_escape_str_for_socks_arg(const char *string) /* this should be enough even if all characters must be escaped */ new_length = (length * 2) + 1; - new_string = new_cp = tor_malloc_zero(new_length); + new_string = new_cp = tor_malloc(new_length); while (*string) { if (strchr(chars_to_escape, *string)) @@ -1313,6 +1314,8 @@ tor_escape_str_for_socks_arg(const char *string) *new_cp++ = *string++; } + *new_cp = '\0'; /* NUL-terminate the new string */ + return new_string; } diff --git a/src/or/config.c b/src/or/config.c index d057dd8ae3..a09dda996b 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -2875,14 +2875,14 @@ options_validate(or_options_t *old_options, or_options_t *options, size_t len; len = strlen(options->Socks5ProxyUsername); - if (len < 1 || len > 255) + if (len < 1 || len > MAX_SOCKS5_AUTH_FIELD_SIZE) REJECT("Socks5ProxyUsername must be between 1 and 255 characters."); if (!options->Socks5ProxyPassword) REJECT("Socks5ProxyPassword must be included with Socks5ProxyUsername."); len = strlen(options->Socks5ProxyPassword); - if (len < 1 || len > 255) + if (len < 1 || len > MAX_SOCKS5_AUTH_FIELD_SIZE) REJECT("Socks5ProxyPassword must be between 1 and 255 characters."); } else if (options->Socks5ProxyPassword) REJECT("Socks5ProxyPassword must be included with Socks5ProxyUsername."); @@ -4120,11 +4120,12 @@ parse_bridge_line(const char *line, int validate_only) field = smartlist_get(items, 0); smartlist_del_keeporder(items, 0); - /* If '=', it's a k=v value pair. */ - if (strchr(field, '=')) { + /* If it's a key=value pair, then it's a SOCKS argument for the + transport proxy... */ + if (string_is_key_value(field)) { socks_args = smartlist_new(); smartlist_add(socks_args, field); - } else { /* If no '=', it's the fingerprint. */ + } else { /* ...otherwise, it's the bridge fingerprint. */ fingerprint = field; } diff --git a/src/or/connection.c b/src/or/connection.c index 6bac59b20c..b0fbe520b2 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1585,7 +1585,7 @@ get_proxy_type(void) /* One byte for the version, one for the command, two for the port, and four for the addr... and, one more for the username NUL: */ -#define SOCKS4_STANDARD_BUFFER_SIZE 1 + 1 + 2 + 4 + 1 +#define SOCKS4_STANDARD_BUFFER_SIZE (1 + 1 + 2 + 4 + 1) /** Write a proxy request of type (socks4, socks5, https) to conn * for conn->addr:conn->port, authenticating with the auth details given @@ -1688,6 +1688,9 @@ connection_proxy_connect(connection_t *conn, int type) memcpy(buf + 2, &portn, 2); /* port */ memcpy(buf + 4, &ip4addr, 4); /* addr */ + /* Next packet field is the userid. If we have pluggable + transport SOCKS arguments, we have to embed them + there. Otherwise, we use an empty userid. */ if (socks_args_string) { /* place the SOCKS args string: */ tor_assert(strlen(socks_args_string) > 0); tor_assert(buf_size >= @@ -1951,6 +1954,8 @@ connection_read_proxy_handshake(connection_t *conn) break; } + /* Username and password lengths should have been checked + above and during torrc parsing. */ tor_assert(usize <= MAX_SOCKS5_AUTH_FIELD_SIZE && psize <= MAX_SOCKS5_AUTH_FIELD_SIZE); reqsize = 3 + usize + psize; diff --git a/src/test/test_util.c b/src/test/test_util.c index b41f23571b..a307a79c80 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -813,9 +813,11 @@ test_util_escape_string_socks(void) test_streq(escaped_string, "First rule: Do not use \\;"); tor_free(escaped_string); - /** Ilegal: Empty string. */ + /** Empty string. */ escaped_string = tor_escape_str_for_socks_arg(""); - test_assert(!escaped_string); + test_assert(escaped_string); + test_streq(escaped_string, ""); + tor_free(escaped_string); /** Escape all characters. */ escaped_string = tor_escape_str_for_socks_arg(";\\;\\"); @@ -823,6 +825,11 @@ test_util_escape_string_socks(void) test_streq(escaped_string, "\\;\\\\\\;\\\\"); tor_free(escaped_string); + escaped_string = tor_escape_str_for_socks_arg(";"); + test_assert(escaped_string); + test_streq(escaped_string, "\\;"); + tor_free(escaped_string); + done: tor_free(escaped_string); } @@ -834,7 +841,10 @@ test_util_string_is_key_value(void *ptr) test_assert(string_is_key_value("key=value")); test_assert(string_is_key_value("k=v")); test_assert(string_is_key_value("key=")); + test_assert(string_is_key_value("x=")); + test_assert(string_is_key_value("xx=")); test_assert(!string_is_key_value("=value")); + test_assert(!string_is_key_value("=x")); test_assert(!string_is_key_value("=")); /* ??? */ From 266f8cddd87f8cf507e094725b3f6028bb8d803b Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 11 Feb 2013 13:43:20 +0000 Subject: [PATCH 8/9] Refactoring to make parse_bridge_line() unittestable. - Make parse_bridge_line() return a struct. - Make bridge_add_from_config() accept a struct. - Make string_is_key_value() less hysterical. --- src/common/util.c | 13 +++-- src/common/util.h | 2 +- src/or/config.c | 133 ++++++++++++++++++++----------------------- src/or/config.h | 14 +++++ src/or/entrynodes.c | 57 ++++++++++++------- src/or/entrynodes.h | 7 +-- src/test/test_util.c | 18 +++--- 7 files changed, 133 insertions(+), 111 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 9aba7d6c5d..bcb69f2081 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -866,9 +866,10 @@ tor_digest_is_zero(const char *digest) } /** Return true if string is a valid '=[]' string. - * is optional, to indicate the empty string. */ + * is optional, to indicate the empty string. Log at logging + * severity if something ugly happens. */ int -string_is_key_value(const char *string) +string_is_key_value(int severity, const char *string) { /* position of equal sign in string */ const char *equal_sign_pos = NULL; @@ -876,19 +877,21 @@ string_is_key_value(const char *string) tor_assert(string); if (strlen(string) < 2) { /* "x=" is shortest args string */ - log_warn(LD_GENERAL, "'%s' is too short to be a k=v value.", escaped(string)); + tor_log(severity, LD_GENERAL, "'%s' is too short to be a k=v value.", + escaped(string)); return 0; } equal_sign_pos = strchr(string, '='); if (!equal_sign_pos) { - log_warn(LD_GENERAL, "'%s' is not a k=v value.", escaped(string)); + tor_log(severity, LD_GENERAL, "'%s' is not a k=v value.", escaped(string)); return 0; } /* validate that the '=' is not in the beginning of the string. */ if (equal_sign_pos == string) { - log_warn(LD_GENERAL, "'%s' is not a valid k=v value.", escaped(string)); + tor_log(severity, LD_GENERAL, "'%s' is not a valid k=v value.", + escaped(string)); return 0; } diff --git a/src/common/util.h b/src/common/util.h index e3cd72118c..624202c8dd 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -209,7 +209,7 @@ const char *find_whitespace_eos(const char *s, const char *eos); const char *find_str_at_start_of_line(const char *haystack, const char *needle); int string_is_C_identifier(const char *string); -int string_is_key_value(const char *string); +int string_is_key_value(int severity, const char *string); int tor_mem_is_zero(const char *mem, size_t len); int tor_digest_is_zero(const char *digest); diff --git a/src/or/config.c b/src/or/config.c index a09dda996b..9d0d564365 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -481,7 +481,6 @@ static int options_transition_affects_descriptor( const or_options_t *old_options, const or_options_t *new_options); static int check_nickname_list(const char *lst, const char *name, char **msg); -static int parse_bridge_line(const char *line, int validate_only); static int parse_client_transport_line(const char *line, int validate_only); static int parse_server_transport_line(const char *line, int validate_only); @@ -1293,11 +1292,13 @@ options_act(const or_options_t *old_options) if (options->Bridges) { mark_bridge_list(); for (cl = options->Bridges; cl; cl = cl->next) { - if (parse_bridge_line(cl->value, 0)<0) { + bridge_line_t *bridge_line = parse_bridge_line(cl->value); + if (!bridge_line) { log_warn(LD_BUG, "Previously validated Bridge line could not be added!"); return -1; } + bridge_add_from_config(bridge_line); } sweep_bridge_list(); } @@ -2966,8 +2967,10 @@ options_validate(or_options_t *old_options, or_options_t *options, REJECT("If you set UseBridges, you must set TunnelDirConns."); for (cl = options->Bridges; cl; cl = cl->next) { - if (parse_bridge_line(cl->value, 1)<0) - REJECT("Bridge line did not parse. See logs for details."); + bridge_line_t *bridge_line = parse_bridge_line(cl->value); + if (!bridge_line) + REJECT("Bridge line did not parse. See logs for details."); + bridge_line_free(bridge_line); } for (cl = options->ClientTransportPlugin; cl; cl = cl->next) { @@ -4038,8 +4041,10 @@ validate_transport_socks_arguments(const smartlist_t *args) tor_assert(smartlist_len(args) > 0); SMARTLIST_FOREACH_BEGIN(args, const char *, s) { - if (!string_is_key_value(s)) /* arguments should be k=v items */ + if (!string_is_key_value(LOG_WARN, s)) { /* items should be k=v items */ + log_warn(LD_CONFIG, "'%s' is not a k=v item.", s); return -1; + } } SMARTLIST_FOREACH_END(s); socks_string = pt_stringify_socks_args(args); @@ -4059,22 +4064,36 @@ validate_transport_socks_arguments(const smartlist_t *args) return 0; } +/** Deallocate a bridge_line_t structure. */ +/* private */ void +bridge_line_free(bridge_line_t *bridge_line) +{ + if (!bridge_line) + return; + + if (bridge_line->socks_args) { + SMARTLIST_FOREACH(bridge_line->socks_args, char*, s, tor_free(s)); + smartlist_free(bridge_line->socks_args); + } + tor_free(bridge_line->transport_name); + tor_free(bridge_line); +} + /** Read the contents of a Bridge line from line. Return 0 * if the line is well-formed, and -1 if it isn't. If * validate_only is 0, and the line is well-formed, then add - * the bridge described in the line to our internal bridge list. */ -static int -parse_bridge_line(const char *line, int validate_only) + * the bridge described in the line to our internal bridge list. + * + * Bridge line format: + * Bridge [transport] IP:PORT [id-fingerprint] [k=v] [k=v] ... + */ +/* private */ bridge_line_t * +parse_bridge_line(const char *line) { smartlist_t *items = NULL; - int r; char *addrport=NULL, *fingerprint=NULL; - char *transport_name=NULL; char *field=NULL; - tor_addr_t addr; - uint16_t port = 0; - char digest[DIGEST_LEN]; - smartlist_t *socks_args = NULL; + bridge_line_t *bridge_line = tor_malloc_zero(sizeof(bridge_line_t)); items = smartlist_new(); smartlist_split_string(items, line, NULL, @@ -4084,47 +4103,49 @@ parse_bridge_line(const char *line, int validate_only) goto err; } - /* field is either a transport name or addrport */ + /* first field is either a transport name or addrport */ field = smartlist_get(items, 0); smartlist_del_keeporder(items, 0); - if (!(strstr(field, ".") || strstr(field, ":"))) { - /* new-style bridge line */ - transport_name = field; + if (string_is_C_identifier(field)) { + /* It's a transport name. */ + bridge_line->transport_name = field; if (smartlist_len(items) < 1) { log_warn(LD_CONFIG, "Too few items to Bridge line."); goto err; } - addrport = smartlist_get(items, 0); + addrport = smartlist_get(items, 0); /* Next field is addrport then. */ smartlist_del_keeporder(items, 0); } else { addrport = field; } - if (tor_addr_port_lookup(addrport, &addr, &port)<0) { + /* Parse addrport. */ + if (tor_addr_port_lookup(addrport, + &bridge_line->addr, &bridge_line->port)<0) { log_warn(LD_CONFIG, "Error parsing Bridge address '%s'", addrport); goto err; } - if (!port) { + if (!bridge_line->port) { log_info(LD_CONFIG, "Bridge address '%s' has no port; using default port 443.", addrport); - port = 443; + bridge_line->port = 443; } /* If transports are enabled, next field could be a fingerprint or a - socks argument. If transports are disabled, next field should be + socks argument. If transports are disabled, next field must be a fingerprint. */ if (smartlist_len(items)) { - if (transport_name) { /* transports enabled: */ + if (bridge_line->transport_name) { /* transports enabled: */ field = smartlist_get(items, 0); smartlist_del_keeporder(items, 0); /* If it's a key=value pair, then it's a SOCKS argument for the transport proxy... */ - if (string_is_key_value(field)) { - socks_args = smartlist_new(); - smartlist_add(socks_args, field); + if (string_is_key_value(LOG_DEBUG, field)) { + bridge_line->socks_args = smartlist_new(); + smartlist_add(bridge_line->socks_args, field); } else { /* ...otherwise, it's the bridge fingerprint. */ fingerprint = field; } @@ -4134,78 +4155,50 @@ parse_bridge_line(const char *line, int validate_only) } } + /* Handle fingerprint, if it was provided. */ if (fingerprint) { if (strlen(fingerprint) != HEX_DIGEST_LEN) { log_warn(LD_CONFIG, "Key digest for Bridge is wrong length."); goto err; } - if (base16_decode(digest, DIGEST_LEN, fingerprint, HEX_DIGEST_LEN)<0) { + if (base16_decode(bridge_line->digest, DIGEST_LEN, + fingerprint, HEX_DIGEST_LEN)<0) { log_warn(LD_CONFIG, "Unable to decode Bridge key digest."); goto err; } } /* If we are using transports, any remaining items in the smartlist - must be k=v values. */ - if (transport_name && smartlist_len(items)) { - if (!socks_args) - socks_args = smartlist_new(); + should be k=v values. */ + if (bridge_line->transport_name && smartlist_len(items)) { + if (!bridge_line->socks_args) + bridge_line->socks_args = smartlist_new(); /* append remaining items of 'items' to 'socks_args' */ - smartlist_add_all(socks_args, items); + smartlist_add_all(bridge_line->socks_args, items); smartlist_clear(items); - tor_assert(smartlist_len(socks_args) > 0); + tor_assert(smartlist_len(bridge_line->socks_args) > 0); } - if (!validate_only) { - log_debug(LD_DIR, "Bridge at %s (transport: %s) (%s)", - fmt_addrport(&addr, port), - transport_name ? transport_name : "no transport", - fingerprint ? fingerprint : "no key listed"); - - if (socks_args) { /* print socks arguments */ - int i = 0; - - tor_assert(smartlist_len(socks_args) > 0); - - log_debug(LD_DIR, "Bridge uses %d SOCKS arguments:", - smartlist_len(socks_args)); - SMARTLIST_FOREACH(socks_args, const char *, arg, - log_debug(LD_CONFIG, "%d: %s", ++i, arg)); - } - - bridge_add_from_config(&addr, port, - fingerprint ? digest : NULL, - transport_name, socks_args); - } else { - if (socks_args) { - if (validate_transport_socks_arguments(socks_args) < 0) - goto err; - } + if (bridge_line->socks_args) { + if (validate_transport_socks_arguments(bridge_line->socks_args) < 0) + goto err; } - r = 0; goto done; err: - r = -1; + bridge_line_free(bridge_line); + bridge_line = NULL; done: SMARTLIST_FOREACH(items, char*, s, tor_free(s)); smartlist_free(items); tor_free(addrport); - tor_free(transport_name); tor_free(fingerprint); - /* We only have to free socks_args if we are validating, since - otherwise bridge_add_from_config() steals its reference. */ - if (socks_args && validate_only) { - SMARTLIST_FOREACH(socks_args, char*, s, tor_free(s)); - smartlist_free(socks_args); - } - - return r; + return bridge_line; } /** Read the contents of a ClientTransportPlugin line from diff --git a/src/or/config.h b/src/or/config.h index 8e34655805..b5c0c734bf 100644 --- a/src/or/config.h +++ b/src/or/config.h @@ -96,5 +96,19 @@ int addressmap_register_auto(const char *from, const char *to, addressmap_entry_source_t addrmap_source, const char **msg); +/** Represents the information stored in a torrc Bridge line. */ +typedef struct bridge_line_t { + tor_addr_t addr; /* The IP address of the bridge. */ + uint16_t port; /* The TCP port of the bridge. */ + char *transport_name; /* The name of the pluggable transport that + should be used to connect to the bridge. */ + char digest[DIGEST_LEN]; /* The bridge's identity key digest. */ + smartlist_t *socks_args;; /* SOCKS arguments for the pluggable + transport proxy. */ +} bridge_line_t; + +void bridge_line_free(bridge_line_t *bridge_line); +bridge_line_t *parse_bridge_line(const char *line); + #endif diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index a07670bbd8..44041d35dd 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1633,37 +1633,52 @@ bridge_resolve_conflicts(const tor_addr_t *addr, uint16_t port, } SMARTLIST_FOREACH_END(bridge); } -/** Remember a new bridge at addr:port. If digest - * is set, it tells us the identity key too. If we already had the - * bridge in our list, unmark it, and don't actually add anything new. - * If transport_name is non-NULL - the bridge is associated with a - * pluggable transport - we assign the transport to the bridge. - * If transport_name is non-NULL - the bridge is associated - * with a pluggable transport - we assign the transport to the bridge. - * If socks_args is non-NULL, it's a smartlist carrying - * key=value pairs to be passed to the pluggable transports - * proxy. This function steals reference of the smartlist. */ +/** Register the bridge information in bridge_line to the + * bridge subsystem. Steals reference of bridge_line. */ void -bridge_add_from_config(const tor_addr_t *addr, uint16_t port, - const char *digest, const char *transport_name, - smartlist_t *socks_args) +bridge_add_from_config(bridge_line_t *bridge_line) { bridge_info_t *b; - bridge_resolve_conflicts(addr, port, digest, transport_name); + { /* Log the bridge we are about to register: */ + log_debug(LD_GENERAL, "Registering bridge at %s (transport: %s) (%s)", + fmt_addrport(&bridge_line->addr, bridge_line->port), + bridge_line->transport_name ? + bridge_line->transport_name : "no transport", + tor_digest_is_zero(bridge_line->digest) ? + "no key listed" : hex_str(bridge_line->digest, DIGEST_LEN)); + + if (bridge_line->socks_args) { /* print socks arguments */ + int i = 0; + + tor_assert(smartlist_len(bridge_line->socks_args) > 0); + + log_debug(LD_GENERAL, "Bridge uses %d SOCKS arguments:", + smartlist_len(bridge_line->socks_args)); + SMARTLIST_FOREACH(bridge_line->socks_args, const char *, arg, + log_debug(LD_CONFIG, "%d: %s", ++i, arg)); + } + } + + bridge_resolve_conflicts(&bridge_line->addr, + bridge_line->port, + bridge_line->digest, + bridge_line->transport_name); b = tor_malloc_zero(sizeof(bridge_info_t)); - tor_addr_copy(&b->addr, addr); - b->port = port; - if (digest) - memcpy(b->identity, digest, DIGEST_LEN); - if (transport_name) - b->transport_name = tor_strdup(transport_name); + tor_addr_copy(&b->addr, &bridge_line->addr); + b->port = bridge_line->port; + if (bridge_line->digest) + memcpy(b->identity, bridge_line->digest, DIGEST_LEN); + if (bridge_line->transport_name) + b->transport_name = bridge_line->transport_name; b->fetch_status.schedule = DL_SCHED_BRIDGE; - b->socks_args = socks_args; + b->socks_args = bridge_line->socks_args; if (!bridge_list) bridge_list = smartlist_new(); + tor_free(bridge_line); /* Deallocate bridge_line now. */ + smartlist_add(bridge_list, b); } diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 48f678a184..6a4bcea48d 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -97,11 +97,8 @@ int routerinfo_is_a_configured_bridge(const routerinfo_t *ri); int node_is_a_configured_bridge(const node_t *node); void learned_router_identity(const tor_addr_t *addr, uint16_t port, const char *digest); -struct smartlist_t; -void bridge_add_from_config(const tor_addr_t *addr, uint16_t port, - const char *digest, - const char *transport_name, - struct smartlist_t *socks_args); +struct bridge_line_t; +void bridge_add_from_config(struct bridge_line_t *bridge_line); void retry_bridge_descriptor_fetch_directly(const char *digest); void fetch_bridge_descriptors(const or_options_t *options, time_t now); void learned_bridge_descriptor(routerinfo_t *ri, int from_cache); diff --git a/src/test/test_util.c b/src/test/test_util.c index a307a79c80..606f8316a0 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -838,17 +838,17 @@ static void test_util_string_is_key_value(void *ptr) { (void)ptr; - test_assert(string_is_key_value("key=value")); - test_assert(string_is_key_value("k=v")); - test_assert(string_is_key_value("key=")); - test_assert(string_is_key_value("x=")); - test_assert(string_is_key_value("xx=")); - test_assert(!string_is_key_value("=value")); - test_assert(!string_is_key_value("=x")); - test_assert(!string_is_key_value("=")); + test_assert(string_is_key_value(LOG_WARN, "key=value")); + test_assert(string_is_key_value(LOG_WARN, "k=v")); + test_assert(string_is_key_value(LOG_WARN, "key=")); + test_assert(string_is_key_value(LOG_WARN, "x=")); + test_assert(string_is_key_value(LOG_WARN, "xx=")); + test_assert(!string_is_key_value(LOG_WARN, "=value")); + test_assert(!string_is_key_value(LOG_WARN, "=x")); + test_assert(!string_is_key_value(LOG_WARN, "=")); /* ??? */ - /* test_assert(!string_is_key_value("===")); */ + /* test_assert(!string_is_key_value(LOG_WARN, "===")); */ done: ; } From 9bdd33eae68b93db688c4537e5c11841a5d37a3b Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 11 Feb 2013 23:45:18 +0100 Subject: [PATCH 9/9] Add parse_bridge_line() unittests. --- src/test/test_config.c | 150 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/src/test/test_config.c b/src/test/test_config.c index e20fe73295..d1e7ccf597 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -10,6 +10,8 @@ #include "confparse.h" #include "connection_edge.h" #include "test.h" +#include "util.h" +#include "address.h" static void test_config_addressmap(void *arg) @@ -169,11 +171,159 @@ test_config_addressmap(void *arg) ; } +/* Test helper function: Make sure that a bridge line gets parsed + * properly. Also make sure that the resulting bridge_line_t structure + * has its fields set correctly. */ +static void +good_bridge_line_test(const char *string, const char *test_addrport, + const char *test_digest, const char *test_transport, + const smartlist_t *test_socks_args) +{ + char *tmp = NULL; + bridge_line_t *bridge_line = parse_bridge_line(string); + test_assert(bridge_line); + + /* test addrport */ + tmp = tor_strdup(fmt_addrport(&bridge_line->addr, bridge_line->port)); + test_streq(test_addrport, tmp); + tor_free(tmp); + + /* If we were asked to validate a digest, but we did not get a + digest after parsing, we failed. */ + if (test_digest && tor_digest_is_zero(bridge_line->digest)) + test_assert(0); + + /* If we were not asked to validate a digest, and we got a digest + after parsing, we failed again. */ + if (!test_digest && !tor_digest_is_zero(bridge_line->digest)) + test_assert(0); + + /* If we were asked to validate a digest, and we got a digest after + parsing, make sure it's correct. */ + if (test_digest) { + tmp = tor_strdup(hex_str(bridge_line->digest, DIGEST_LEN)); + tor_strlower(tmp); + test_streq(test_digest, tmp); + tor_free(tmp); + } + + /* If we were asked to validate a transport name, make sure tha it + matches with the transport name that was parsed. */ + if (test_transport && !bridge_line->transport_name) + test_assert(0); + if (!test_transport && bridge_line->transport_name) + test_assert(0); + if (test_transport) + test_streq(test_transport, bridge_line->transport_name); + + /* Validate the SOCKS argument smartlist. */ + if (test_socks_args && !bridge_line->socks_args) + test_assert(0); + if (!test_socks_args && bridge_line->socks_args) + test_assert(0); + if (test_socks_args) + test_assert(smartlist_strings_eq(test_socks_args, + bridge_line->socks_args)); + + done: + tor_free(tmp); + bridge_line_free(bridge_line); +} + +/* Test helper function: Make sure that a bridge line is + * unparseable. */ +static void +bad_bridge_line_test(const char *string) +{ + bridge_line_t *bridge_line = parse_bridge_line(string); + test_assert(!bridge_line); + + done: + bridge_line_free(bridge_line); +} + +static void +test_config_parse_bridge_line(void *arg) +{ + (void) arg; + good_bridge_line_test("192.0.2.1:4123", + "192.0.2.1:4123", NULL, NULL, NULL); + + good_bridge_line_test("192.0.2.1", + "192.0.2.1:443", NULL, NULL, NULL); + + good_bridge_line_test("transport [::1]", + "[::1]:443", NULL, "transport", NULL); + + good_bridge_line_test("transport 192.0.2.1:12 " + "4352e58420e68f5e40bf7c74faddccd9d1349413", + "192.0.2.1:12", + "4352e58420e68f5e40bf7c74faddccd9d1349413", + "transport", NULL); + + { + smartlist_t *sl_tmp = smartlist_new(); + smartlist_add_asprintf(sl_tmp, "twoandtwo=five"); + + good_bridge_line_test("transport 192.0.2.1:12 " + "4352e58420e68f5e40bf7c74faddccd9d1349413 twoandtwo=five", + "192.0.2.1:12", "4352e58420e68f5e40bf7c74faddccd9d1349413", + "transport", sl_tmp); + + SMARTLIST_FOREACH(sl_tmp, char *, s, tor_free(s)); + smartlist_free(sl_tmp); + } + + { + smartlist_t *sl_tmp = smartlist_new(); + smartlist_add_asprintf(sl_tmp, "twoandtwo=five"); + smartlist_add_asprintf(sl_tmp, "z=z"); + + good_bridge_line_test("transport 192.0.2.1:12 twoandtwo=five z=z", + "192.0.2.1:12", NULL, "transport", sl_tmp); + + SMARTLIST_FOREACH(sl_tmp, char *, s, tor_free(s)); + smartlist_free(sl_tmp); + } + + good_bridge_line_test("192.0.2.1:1231 " + "4352e58420e68f5e40bf7c74faddccd9d1349413", + "192.0.2.1:1231", + "4352e58420e68f5e40bf7c74faddccd9d1349413", + NULL, NULL); + + /* Empty line */ + bad_bridge_line_test(""); + /* bad transport name */ + bad_bridge_line_test("tr$n_sp0r7 190.20.2.2"); + /* weird ip address */ + bad_bridge_line_test("a.b.c.d"); + /* invalid fpr */ + bad_bridge_line_test("2.2.2.2:1231 4352e58420e68f5e40bf7c74faddccd9d1349"); + /* no k=v in the end */ + bad_bridge_line_test("obfs2 2.2.2.2:1231 " + "4352e58420e68f5e40bf7c74faddccd9d1349413 what"); + /* no addrport */ + bad_bridge_line_test("asdw"); + /* huge k=v value that can't fit in SOCKS fields */ + bad_bridge_line_test( + "obfs2 2.2.2.2:1231 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aa=b"); +} + #define CONFIG_TEST(name, flags) \ { #name, test_config_ ## name, flags, NULL, NULL } struct testcase_t config_tests[] = { CONFIG_TEST(addressmap, 0), + CONFIG_TEST(parse_bridge_line, 0), END_OF_TESTCASES };