[tor-commits] [torsocks/master] socks5: Always use ATYP 0x03 for CONNECT command

dgoulet at torproject.org dgoulet at torproject.org
Fri Apr 20 14:55:17 UTC 2018


commit a0070f02d77315b640dbc00105a246ef64d89011
Author: David Goulet <dgoulet at 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 at 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 */





More information about the tor-commits mailing list