[tor-commits] [torsocks/master] Refactor the connect() code flow for clarity
dgoulet at torproject.org
dgoulet at torproject.org
Fri Apr 4 22:40:28 UTC 2014
commit aba207d0033ae149bfa19c2dfeb0823af6a2ebc3
Author: David Goulet <dgoulet at ev0ke.net>
Date: Mon Mar 31 19:55:47 2014 -0400
Refactor the connect() code flow for clarity
This adds a "validate_socket()" function that is called first to make
sure all criteria of a valid socket that torsocks can handle are met.
This has been done to have a single callsite that can do this validation
thus improving the clarity and flow of the code.
It now also returns EPERM for things that we deny instead of EINVAL or
EBADF. Because of that, the connect test has been changed to handle this
new errno value.
Signed-off-by: David Goulet <dgoulet at ev0ke.net>
---
src/lib/connect.c | 95 ++++++++++++++++++++++++++++++++++++++------------
tests/test_connect.c | 12 +++----
2 files changed, 79 insertions(+), 28 deletions(-)
diff --git a/src/lib/connect.c b/src/lib/connect.c
index 0973e1e..5ecb2ff 100644
--- a/src/lib/connect.c
+++ b/src/lib/connect.c
@@ -30,51 +30,101 @@
TSOCKS_LIBC_DECL(connect, LIBC_CONNECT_RET_TYPE, LIBC_CONNECT_SIG)
/*
- * Torsocks call for connect(2).
+ * Validate the given sock fd and address that we receive in the connect()
+ * call. Criteria are:
+ *
+ * -) Non INET/INET6 socket should return to the libc, LIBC.
+ * -) Non stream socket can't be handled, DENY.
+ * -) Connection to the any address won't work with Tor, DENY.
+ * -) ALLOW.
+ *
+ * Return 0 if validation passes and socket handling should continue. Return 1
+ * if the socket can't be handle by Tor but is still valid thus the caller
+ * should send it directly to the libc connect function.
+ *
+ * On error or if validation fails, errno is set and -1 is returned. The caller
+ * should *return* right away an error.
*/
-LIBC_CONNECT_RET_TYPE tsocks_connect(LIBC_CONNECT_SIG)
+static int validate_socket(int sockfd, const struct sockaddr *addr)
{
int ret, sock_type;
socklen_t optlen;
- struct connection *new_conn;
- struct onion_entry *on_entry;
- DBG("Connect catched on fd %d", sockfd);
+ if (!addr) {
+ /* Go directly to libc, connect() will handle a NULL value. */
+ goto libc_call;
+ }
+
+ /*
+ * We can't handle a non inet socket thus directly go to the libc. This is
+ * to allow AF_UNIX/_LOCAL socket to work with torsocks.
+ */
+ if (addr->sa_family != AF_INET && addr->sa_family != AF_INET6) {
+ DBG("[conect] Connection is not IPv4/v6. Ignoring.");
+ /* Ask the call to use the libc connect. */
+ goto libc_call;
+ }
optlen = sizeof(sock_type);
ret = getsockopt(sockfd, SOL_SOCKET, SO_TYPE, &sock_type, &optlen);
if (ret < 0) {
- /* Use the getsockopt() errno value. */
+ DBG("[connect] Fail getsockopt() on sock %d", sockfd);
+ errno = EBADF;
goto error;
}
- /* We can't handle a non inet socket. */
- if (addr->sa_family != AF_INET && addr->sa_family != AF_INET6) {
- DBG("[conect] Connection is not IPv4/v6. Ignoring.");
- goto libc_connect;
- }
+ DBG("[connect] Socket family %s and type %d",
+ addr->sa_family == AF_INET ? "AF_INET" : "AF_INET6", sock_type);
- /*
- * Refuse non stream socket. There is a chance that this might be a DNS
- * request that we can't pass through Tor using raw UDP packet.
- */
+ /* Refuse non stream socket since Tor can't handle that. */
if (!IS_SOCK_STREAM(sock_type)) {
- WARN("[connect] UDP or ICMP stream can't be handled. Rejecting.");
- errno = EBADF;
+ DBG("[connect] UDP or ICMP stream can't be handled. Rejecting.");
+ errno = EPERM;
goto error;
}
/*
* Trying to connect to ANY address will evidently not work for Tor thus we
- * deny the call.
+ * deny the call with an invalid argument error.
*/
if (utils_is_addr_any(addr)) {
- errno = EINVAL;
+ errno = EPERM;
goto error;
}
- DBG("[connect] Socket family %s and type %d",
- addr->sa_family == AF_INET ? "AF_INET" : "AF_INET6", sock_type);
+ return 0;
+
+libc_call:
+ return 1;
+error:
+ return -1;
+}
+
+/*
+ * Torsocks call for connect(2).
+ */
+LIBC_CONNECT_RET_TYPE tsocks_connect(LIBC_CONNECT_SIG)
+{
+ int ret;
+ struct connection *new_conn;
+ struct onion_entry *on_entry;
+
+ DBG("Connect catched on fd %d", sockfd);
+
+ /*
+ * Validate socket values in order to see if we can handle this connect
+ * through Tor.
+ */
+ ret = validate_socket(sockfd, addr);
+ if (ret == 1) {
+ /* Tor can't handle it so send it to the libc. */
+ goto libc_connect;
+ } else if (ret == -1) {
+ /* Validation failed. Stop right now. */
+ goto error;
+ }
+ /* Implicit else statement meaning we continue processing the connect. */
+ assert(!ret);
/*
* Lock registry to get the connection reference if one. In this code path,
@@ -118,7 +168,8 @@ LIBC_CONNECT_RET_TYPE tsocks_connect(LIBC_CONNECT_SIG)
} else {
/*
* Check if address is localhost. At this point, we are sure it's not a
- * .onion cookie address that is by default in the loopback network.
+ * .onion cookie address that is by default in the loopback network
+ * thus this check is done after the onion entry lookup.
*/
if (utils_sockaddr_is_localhost(addr)) {
WARN("[connect] Connection to a local address are denied since it "
diff --git a/tests/test_connect.c b/tests/test_connect.c
index 705219d..a282255 100644
--- a/tests/test_connect.c
+++ b/tests/test_connect.c
@@ -36,7 +36,7 @@ static void test_connect_deny(void)
struct sockaddr_in sin;
struct sockaddr_in6 sin6;
- fd = tsocks_libc_socket(AF_INET, SOCK_RAW, IPPROTO_RAW);
+ fd = tsocks_libc_socket(AF_INET, SOCK_RAW, IPPROTO_TCP);
ret = connect(fd, (struct sockaddr *) &sin, sizeof(sin));
ok (ret == -1 && errno == EBADF, "Connect with RAW socket NOT valid");
close(fd);
@@ -44,13 +44,13 @@ static void test_connect_deny(void)
sin.sin_family = AF_INET;
fd = tsocks_libc_socket(sin.sin_family, SOCK_DGRAM, 0);
ret = connect(fd, (struct sockaddr *) &sin, sizeof(sin));
- ok (ret == -1 && errno == EBADF, "Connect with UDP socket NOT valid");
+ ok (ret == -1 && errno == EPERM, "Connect with UDP socket NOT valid");
close(fd);
inet_pton(sin.sin_family, "0.0.0.0", &sin.sin_addr);
fd = tsocks_libc_socket(sin.sin_family, SOCK_STREAM, 0);
ret = connect(fd, (struct sockaddr *) &sin, sizeof(sin));
- ok (ret == -1 && errno == EINVAL,
+ ok (ret == -1 && errno == EPERM,
"Connect with ANY address is NOT valid.");
close(fd);
@@ -64,13 +64,13 @@ static void test_connect_deny(void)
sin6.sin6_family = AF_INET6;
fd = tsocks_libc_socket(sin6.sin6_family, SOCK_DGRAM, 0);
ret = connect(fd, (struct sockaddr *) &sin6, sizeof(sin6));
- ok (ret == -1 && errno == EBADF, "Connect with UDPv6 socket NOT valid");
+ ok (ret == -1 && errno == EPERM, "Connect with UDPv6 socket NOT valid");
close(fd);
inet_pton(sin6.sin6_family, "::", &sin6.sin6_addr);
fd = tsocks_libc_socket(sin6.sin6_family, SOCK_STREAM, 0);
ret = connect(fd, (struct sockaddr *) &sin6, sizeof(sin6));
- ok (ret == -1 && errno == EINVAL,
+ ok (ret == -1 && errno == EPERM,
"Connect with ANYv6 address is NOT valid.");
close(fd);
@@ -82,7 +82,7 @@ static void test_connect_deny(void)
close(fd);
/* Bad fd. */
- ret = connect(42, NULL, 42);
+ ret = connect(42, (struct sockaddr *) &sin, 42);
ok (ret == -1 && errno == EBADF, "Bad socket FD");
}
More information about the tor-commits
mailing list