[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [torsocks/master] Fix: improve getpeername to actually works



commit 86e0424148aac12965ca1f34dcc9ea5cfbdfa2f0
Author: David Goulet <dgoulet@xxxxxxxxx>
Date:   Sun Oct 12 17:04:54 2014 -0400

    Fix: improve getpeername to actually works
    
    This also adds a test in the test suite for our getpeername() wrapper.
    
    Signed-off-by: David Goulet <dgoulet@xxxxxxxxx>
---
 .gitignore               |    1 +
 src/lib/getpeername.c    |   58 +++++++++++++++++++++----
 tests/Makefile.am        |    5 ++-
 tests/test_getpeername.c |  108 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/test_list          |    1 +
 5 files changed, 163 insertions(+), 10 deletions(-)

diff --git a/.gitignore b/.gitignore
index c836b9b..9eebfbc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -48,6 +48,7 @@ tests/test_connect
 tests/test_dns
 tests/test_socket
 tests/test_fd_passing
+tests/test_getpeername
 tests/unit/test_onion
 tests/unit/test_connection
 tests/unit/test_utils
diff --git a/src/lib/getpeername.c b/src/lib/getpeername.c
index 607c1b1..bd93a2b 100644
--- a/src/lib/getpeername.c
+++ b/src/lib/getpeername.c
@@ -39,15 +39,63 @@ LIBC_GETPEERNAME_RET_TYPE tsocks_getpeername(LIBC_GETPEERNAME_SIG)
 	connection_registry_lock();
 	conn = connection_find(sockfd);
 	if (!conn) {
-		errno = ENOTCONN;
+		connection_registry_unlock();
+		goto libc;
+	}
+
+	if (!addrlen || !addr) {
+		/* Bad address. */
+		errno = EFAULT;
 		ret = -1;
 		goto end;
 	}
 
+	/*
+	 * Extra check for addrlen since we are about to copy the connection
+	 * content into the given address.
+	 */
+	if (*addrlen > sizeof(struct sockaddr)) {
+		/* Ref to the manpage for the returned value here. */
+		errno = EINVAL;
+		ret = -1;
+		goto end;
+	}
+
+	/*
+	 * Copy connected destination address into the given addr with only the
+	 * given len so we don't overflow on purpose.
+	 */
+	switch (conn->dest_addr.domain) {
+	case CONNECTION_DOMAIN_NAME:
+		/*
+		 * This domain is only used with onion address which contains a
+		 * cookie address of domain INET. Use that since that's the address
+		 * that has been returned to the application.
+		 */
+	case CONNECTION_DOMAIN_INET:
+		memcpy(addr, (const struct sockaddr *) &conn->dest_addr.u.sin,
+				*addrlen);
+		break;
+	case CONNECTION_DOMAIN_INET6:
+		memcpy(addr, (const struct sockaddr *) &conn->dest_addr.u.sin6,
+				*addrlen);
+		break;
+	}
+
+	/* Success. */
 	errno = 0;
+	ret = 0;
+
 end:
 	connection_registry_unlock();
 	return ret;
+
+libc:
+	/*
+	 * This is clearly not a socket we are handling so it's safe to pass it to
+	 * the original libc call.
+	 */
+	return tsocks_libc_getpeername(LIBC_GETPEERNAME_ARGS);
 }
 
 /*
@@ -55,18 +103,10 @@ end:
  */
 LIBC_GETPEERNAME_DECL
 {
-	int ret;
-
 	if (!tsocks_libc_getpeername) {
 		tsocks_libc_getpeername = tsocks_find_libc_symbol(
 				LIBC_GETPEERNAME_NAME_STR, TSOCKS_SYM_EXIT_NOT_FOUND);
 	}
 
-	ret = tsocks_libc_getpeername(LIBC_GETPEERNAME_ARGS);
-	if (ret < 0) {
-		/* errno is populated by the previous call at this point. */
-		return ret;
-	}
-
 	return tsocks_getpeername(LIBC_GETPEERNAME_ARGS);
 }
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b916134..630492d 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -6,7 +6,7 @@ LIBTAP=$(top_builddir)/tests/utils/tap/libtap.la
 
 LIBTORSOCKS=$(top_builddir)/src/lib/libtorsocks.la
 
-noinst_PROGRAMS = test_dns test_socket test_connect test_fd_passing
+noinst_PROGRAMS = test_dns test_socket test_connect test_fd_passing test_getpeername
 
 test_dns_SOURCES = test_dns.c
 test_dns_LDADD = $(LIBTAP) $(LIBTORSOCKS)
@@ -20,6 +20,9 @@ test_connect_LDADD = $(LIBTAP) $(LIBTORSOCKS)
 test_fd_passing_SOURCES = test_fd_passing.c
 test_fd_passing_LDADD = $(LIBTAP) $(LIBTORSOCKS) -lpthread
 
+test_getpeername_SOURCES = test_getpeername.c
+test_getpeername_LDADD = $(LIBTAP) $(LIBTORSOCKS)
+
 check-am:
 	./run.sh test_list
 
diff --git a/tests/test_getpeername.c b/tests/test_getpeername.c
new file mode 100644
index 0000000..516596e
--- /dev/null
+++ b/tests/test_getpeername.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2014 - David Goulet <dgoulet@xxxxxxxxx>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License, version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 51
+ * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <stdio.h>
+#include <sys/socket.h>
+
+#include <lib/torsocks.h>
+
+#include <tap/tap.h>
+
+#define NUM_TESTS 7
+
+static void test_getpeername(void)
+{
+	int pipe_fds[2], ret, inet_sock = -1;
+	char buf[INET_ADDRSTRLEN];
+	struct sockaddr addr;
+	struct sockaddr_in addrv4;
+	socklen_t addrlen;
+	const char *ip = "93.95.227.222";
+
+	ret = pipe(pipe_fds);
+	if (ret < 0) {
+		fail("Unable to create pipe");
+		goto error;
+	}
+
+	/* This test is to see if we go through the libc or not. */
+	ret = getpeername(pipe_fds[0], NULL, NULL);
+	ok(ret == -1 && errno == ENOTSOCK, "Invalid socket fd");
+
+	close(pipe_fds[0]);
+	close(pipe_fds[1]);
+
+	/* Create inet socket. */
+	inet_sock = socket(AF_INET, SOCK_STREAM, 0);
+	ok(inet_sock >= 0, "Inet socket created");
+
+	/* This test is to see if we go through the libc or not. */
+	ret = getpeername(inet_sock, &addr, &addrlen);
+	ok(ret == -1 && errno == ENOTCONN, "Socket not connected");
+
+	/* Connect socket through Tor so we can test the wrapper. */
+	addrv4.sin_family = AF_INET;
+	addrv4.sin_port = htons(443);
+	inet_pton(addrv4.sin_family, ip, &addrv4.sin_addr);
+	memset(addrv4.sin_zero, 0, sizeof(addrv4.sin_zero));
+
+	ret = connect(inet_sock, (struct sockaddr *) &addrv4, sizeof(addrv4));
+	if (ret < 0) {
+		fail("Unable to connect to %s", ip);
+		goto error;
+	}
+
+	/* Very large addrlen. */
+	addrlen = -1;
+	ret = getpeername(inet_sock, &addr, &addrlen);
+	ok(ret == -1 && errno == EINVAL, "Invalid addrlen");
+
+	addrlen = sizeof(addr);
+	ret = getpeername(inet_sock, NULL, &addrlen);
+	ok(ret == -1 && errno == EFAULT, "Invalid addr ptr");
+
+	ret = getpeername(inet_sock, &addr, NULL);
+	ok(ret == -1 && errno == EFAULT, "Invalid addrlen ptr");
+
+	addrlen = sizeof(addrv4);
+	memset(&addrv4, 0, addrlen);
+	ret = getpeername(inet_sock, (struct sockaddr *) &addrv4, &addrlen);
+
+	/* Validate returned IP address. */
+	memset(buf, 0, sizeof(buf));
+	inet_ntop(addrv4.sin_family, &addrv4.sin_addr, buf, sizeof(buf));
+	ok(ret == 0 && strncmp(buf, ip, strlen(ip)) == 0,
+			"Valid returned IP address from getpeername()");
+
+error:
+	if (inet_sock >= 0) {
+		close(inet_sock);
+	}
+	return;
+}
+
+int main(int argc, char **argv)
+{
+	/* Libtap call for the number of tests planned. */
+	plan_tests(NUM_TESTS);
+
+	test_getpeername();
+
+    return 0;
+}
diff --git a/tests/test_list b/tests/test_list
index fb320db..e66a239 100644
--- a/tests/test_list
+++ b/tests/test_list
@@ -2,6 +2,7 @@
 ./test_dns
 ./test_fd_passing
 ./test_socket
+./test_getpeername
 ./unit/test_onion
 ./unit/test_connection
 ./unit/test_utils

_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits