[tor-commits] [torsocks/master] Fix: improve getpeername to actually works
dgoulet at torproject.org
dgoulet at torproject.org
Wed Oct 22 22:35:16 UTC 2014
commit 86e0424148aac12965ca1f34dcc9ea5cfbdfa2f0
Author: David Goulet <dgoulet at ev0ke.net>
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 at ev0ke.net>
---
.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 at ev0ke.net>
+ *
+ * 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
More information about the tor-commits
mailing list