From 14b84858c062955dc52fbcdcf1146ce0031a95da Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 17 Dec 2012 15:01:31 +0200 Subject: Send SOCKS arguments when doing SOCKS4. --- src/or/connection.c | 92 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 70 insertions(+), 22 deletions(-) (limited to 'src/or/connection.c') 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 -- cgit v1.2.3-54-g00ecf From 8f2e98015989faf708a5294c3028a319fd45f16c Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 17 Dec 2012 15:07:26 +0200 Subject: 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(-) (limited to 'src/or/connection.c') 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; + } + + 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; + } - /* XXX len of user and pass must be <= 255 !!! */ - usize = strlen(user); - psize = strlen(pass); - tor_assert(usize <= 255 && psize <= 255); + 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); -- cgit v1.2.3-54-g00ecf From b5dceab1751dfa12b27b3042a49d90e0b02c2e0c Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Sat, 9 Feb 2013 18:46:10 +0000 Subject: 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(-) (limited to 'src/or/connection.c') 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("=")); /* ??? */ -- cgit v1.2.3-54-g00ecf