commit a0070f02d77315b640dbc00105a246ef64d89011 Author: David Goulet dgoulet@torproject.org Date: Fri Apr 20 10:34:34 2018 -0400
socks5: Always use ATYP 0x03 for CONNECT command
Because of the SafeSocks parameter of Tor, if set, we can't pass a raw IP address to Tor since it will believe we did a DNS resolution from the application.
Now, thanks to #22461, Tor safely accepts an IPv4/IPv6 address withing a FQDN connect request which avoids the SafeSocks warnings.
The #22461 wasn't backported which means that torsocks working with SafeSocks is only possible in tor >= 0.3.2 stable series.
Fixes #23667
Signed-off-by: David Goulet dgoulet@torproject.org --- src/common/socks5.c | 80 ++++++++++++++++++++++-------------------------- tests/unit/test_socks5.c | 54 ++++++-------------------------- 2 files changed, 46 insertions(+), 88 deletions(-)
diff --git a/src/common/socks5.c b/src/common/socks5.c index 1be2ab8..bf6019a 100644 --- a/src/common/socks5.c +++ b/src/common/socks5.c @@ -385,75 +385,60 @@ int socks5_send_connect_request(struct connection *conn) unsigned char buffer[1500]; ssize_t buf_len, ret_send; struct socks5_request msg; + struct socks5_request_domain req_name; + const char *retp;
assert(conn); assert(conn->fd >= 0);
+ memset(&req_name, 0, sizeof(req_name)); memset(buffer, 0, sizeof(buffer)); buf_len = sizeof(msg);
+ /* Set up the header part. It is always the same, even the address type as + * Tor handles IPv4/IPv6 as a Domain Name so SafeSocks doesn't complain. For + * this reason, we always send ATYP 0x03 (domain name) to Tor. */ msg.ver = SOCKS5_VERSION; msg.cmd = SOCKS5_CMD_CONNECT; /* Always zeroed. */ msg.rsv = 0; + msg.atyp = SOCKS5_ATYP_DOMAIN; + memcpy(buffer, &msg, buf_len);
+ /* Depending on the domain of the connection, we'll set the hostname + * accordingly that is transforming the address into a text format. */ switch (conn->dest_addr.domain) { case CONNECTION_DOMAIN_INET: { - struct socks5_request_ipv4 req_ipv4; - - msg.atyp = SOCKS5_ATYP_IPV4; - /* Copy the first part of the request. */ - memcpy(buffer, &msg, buf_len); - - /* Prepare the ipv4 payload to be copied in the send buffer. */ - memcpy(req_ipv4.addr, &conn->dest_addr.u.sin.sin_addr, - sizeof(req_ipv4.addr)); - req_ipv4.port = conn->dest_addr.u.sin.sin_port; - - /* Copy ipv4 request portion in the buffer. */ - memcpy(buffer + buf_len, &req_ipv4, sizeof(req_ipv4)); - buf_len += sizeof(req_ipv4); + retp = inet_ntop(AF_INET, &conn->dest_addr.u.sin.sin_addr, + (char *) req_name.name, sizeof(req_name.name)); + if (retp == NULL) { + ERR("Socks5 connection malformed IPv4"); + ret = -EINVAL; + goto error; + } + req_name.port = conn->dest_addr.u.sin.sin_port; break; } case CONNECTION_DOMAIN_INET6: { - struct socks5_request_ipv6 req_ipv6; - - msg.atyp = SOCKS5_ATYP_IPV6; - /* Copy the first part of the request. */ - memcpy(buffer, &msg, buf_len); - - /* Prepare the ipv6 payload to be copied in the send buffer. */ - memcpy(req_ipv6.addr, &conn->dest_addr.u.sin6.sin6_addr, - sizeof(req_ipv6.addr)); - req_ipv6.port = conn->dest_addr.u.sin6.sin6_port; - - /* Copy ipv6 request portion in the buffer. */ - memcpy(buffer + buf_len, &req_ipv6, sizeof(req_ipv6)); - buf_len += sizeof(req_ipv6); + retp = inet_ntop(AF_INET6, &conn->dest_addr.u.sin6.sin6_addr, + (char *) req_name.name, sizeof(req_name.name)); + if (retp == NULL) { + ERR("Socks5 connection malformed IPv4"); + ret = -EINVAL; + goto error; + } + req_name.port = conn->dest_addr.u.sin6.sin6_port; break; } case CONNECTION_DOMAIN_NAME: { - struct socks5_request_domain req_name; - - msg.atyp = SOCKS5_ATYP_DOMAIN; - /* Copy the first part of the request. */ - memcpy(buffer, &msg, buf_len); - /* Setup domain name request buffer. */ req_name.len = strlen(conn->dest_addr.hostname.addr); - memcpy(req_name.name, conn->dest_addr.hostname.addr, req_name.len); + memcpy(req_name.name, conn->dest_addr.hostname.addr, + strlen(conn->dest_addr.hostname.addr)); req_name.port = conn->dest_addr.hostname.port; - - /* Copy ipv6 request portion in the buffer. */ - memcpy(buffer + buf_len, &req_name.len, sizeof(req_name.len)); - buf_len += sizeof(req_name.len); - memcpy(buffer + buf_len, req_name.name, req_name.len); - buf_len += req_name.len; - memcpy(buffer + buf_len, &req_name.port, sizeof(req_name.port)); - buf_len += sizeof(req_name.port); break; } default: @@ -462,6 +447,15 @@ int socks5_send_connect_request(struct connection *conn) goto error; }
+ /* Common for everyone, copy the filled up request buffer. */ + req_name.len = strlen((char *) req_name.name); + memcpy(buffer + buf_len, &req_name.len, sizeof(req_name.len)); + buf_len += sizeof(req_name.len); + memcpy(buffer + buf_len, req_name.name, req_name.len); + buf_len += req_name.len; + memcpy(buffer + buf_len, &req_name.port, sizeof(req_name.port)); + buf_len += sizeof(req_name.port); + DBG("Socks5 sending connect request to fd %d", conn->fd);
ret_send = send_data(conn->fd, &buffer, buf_len); diff --git a/tests/unit/test_socks5.c b/tests/unit/test_socks5.c index ac8ed3c..284abc4 100644 --- a/tests/unit/test_socks5.c +++ b/tests/unit/test_socks5.c @@ -30,8 +30,6 @@
static struct socks5_method_req method_req; static struct socks5_request req; -static struct socks5_request_ipv4 req_ipv4; -static struct socks5_request_ipv6 req_ipv6; static struct socks5_request_domain req_name; static struct socks5_request_resolve req_resolve; static struct socks5_request_resolve_ptr req_resolve_ptr; @@ -138,37 +136,12 @@ static ssize_t socks5_recv_method_no_accept_stub(int fd, void *buf, size_t len) * send_connect_request test doubles */
-static ssize_t socks5_send_connect_request_ipv4_spy(int fd, const void *buf, - size_t len) -{ - ssize_t buf_len = 0; - - set_socks5_request(buf); - buf_len += sizeof(struct socks5_request); - - req_ipv4 = (*(struct socks5_request_ipv4 *) (buf + buf_len)); - - return 1; -} - -static ssize_t socks5_send_connect_request_ipv6_spy(int fd, const void *buf, - size_t len) -{ - ssize_t buf_len = 0; - - set_socks5_request(buf); - buf_len += sizeof(struct socks5_request); - - req_ipv6 = (*(struct socks5_request_ipv6 *) (buf + buf_len)); - - return 1; -} - static ssize_t socks5_send_connect_request_domain_spy(int fd, const void *buf, size_t len) { ssize_t buf_len = 0;
+ memset(&req_name, 0, sizeof(req_name)); set_socks5_request(buf); buf_len += sizeof(struct socks5_request);
@@ -597,24 +570,19 @@ static void test_socks5_send_connect_request(void) { int ret; struct connection *conn_stub; - char ip[INET6_ADDRSTRLEN];
conn_stub = get_connection_stub(); - socks5_init(socks5_send_connect_request_ipv4_spy, NULL); + socks5_init(socks5_send_connect_request_domain_spy, NULL);
ret = socks5_send_connect_request(conn_stub);
- inet_ntop(AF_INET, - (struct sockaddr_in *)&req_ipv4.addr, - ip, INET6_ADDRSTRLEN); - ok(ret == 0 && req.ver == SOCKS5_VERSION && req.cmd == SOCKS5_CMD_CONNECT && req.rsv == 0 && - req.atyp == SOCKS5_ATYP_IPV4 && - strncmp(ip, "127.0.0.1", INET6_ADDRSTRLEN) == 0 && - req_ipv4.port == htons(9050), + req.atyp == SOCKS5_ATYP_DOMAIN && + strncmp((char *) req_name.name, "127.0.0.1", INET6_ADDRSTRLEN) == 0 && + req_name.port == htons(9050), "socks5 send connect request IPv4");
connection_destroy(conn_stub); @@ -623,21 +591,17 @@ static void test_socks5_send_connect_request(void) /* IPv6 */
conn_stub = get_connection_ipv6_stub(); - socks5_init(socks5_send_connect_request_ipv6_spy, NULL); + socks5_init(socks5_send_connect_request_domain_spy, NULL);
ret = socks5_send_connect_request(conn_stub);
- inet_ntop(AF_INET6, - (struct sockaddr_in *)&req_ipv6.addr, - ip, INET6_ADDRSTRLEN); - ok(ret == 0 && req.ver == SOCKS5_VERSION && req.cmd == SOCKS5_CMD_CONNECT && req.rsv == 0 && - req.atyp == SOCKS5_ATYP_IPV6 && - strncmp(ip, "::1", INET6_ADDRSTRLEN) == 0 && - req_ipv6.port == htons(9050), + req.atyp == SOCKS5_ATYP_DOMAIN && + strncmp((char *) req_name.name, "::1", INET6_ADDRSTRLEN) == 0 && + req_name.port == htons(9050), "socks5 send connect request IPv6");
/* Domain name */