commit 86e0424148aac12965ca1f34dcc9ea5cfbdfa2f0 Author: David Goulet <dgoulet@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@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@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