commit 3b8687973757cbecfe22a7e5708da15523111c55 Author: David Goulet dgoulet@ev0ke.net Date: Mon Mar 31 13:49:32 2014 -0400
Fix: socket() type check SOCK_STREAM
Even though connect() makes a check, deny socket creation that are INET/INET6 but NOT of type SOCK_STREAM. This fix makes our wrapper handle socket type flags that can be passed to the kernel such as SOCK_NONBLOCK and SOCK_CLOEXEC.
Furthermore, the type check was *not* right since having a type set to SOCK_DGRAM also matches SOCK_STREAM when using the & operator.
A unit test is added for the IS_SOCK_STREAM(type) macro that test if a socket type is a SOCK_STREAM.
Signed-off-by: David Goulet dgoulet@ev0ke.net --- .gitignore | 1 + src/common/compat.h | 12 ++++++++ src/lib/connect.c | 2 +- src/lib/socket.c | 39 ++++++++++++++++---------- tests/test_list | 1 + tests/unit/Makefile.am | 5 +++- tests/unit/test_compat.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 114 insertions(+), 16 deletions(-)
diff --git a/.gitignore b/.gitignore index 5394c5b..59fcadb 100644 --- a/.gitignore +++ b/.gitignore @@ -49,3 +49,4 @@ tests/unit/test_connection tests/unit/test_utils tests/unit/test_config-file tests/unit/test_socks5 +tests/unit/test_compat diff --git a/src/common/compat.h b/src/common/compat.h index 2058a23..87191f0 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -107,4 +107,16 @@ void tsocks_mutex_unlock(tsocks_mutex_t *m); #define TSOCKS_ANY ((unsigned long int) 0x00000000) #define TSOCKS_ANY6 { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 }
+/* + * Macro to tell if a given socket type is a SOCK_STREAM or not. The macro + * resolve to 1 if yes else 0. + */ +#if defined(__NetBSD__) +#define IS_SOCK_STREAM(type) \ + ((type & ~(SOCK_NONBLOCK | SOCK_CLOEXEC | SOCK_NOSIGPIPE)) == SOCK_STREAM) +#else /* __NetBSD__ */ +#define IS_SOCK_STREAM(type) \ + ((type & ~(SOCK_NONBLOCK | SOCK_CLOEXEC)) == SOCK_STREAM) +#endif /* __NetBSD__ */ + #endif /* TORSOCKS_COMPAT_H */ diff --git a/src/lib/connect.c b/src/lib/connect.c index 6622119..0973e1e 100644 --- a/src/lib/connect.c +++ b/src/lib/connect.c @@ -58,7 +58,7 @@ LIBC_CONNECT_RET_TYPE tsocks_connect(LIBC_CONNECT_SIG) * 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. */ - if (sock_type != SOCK_STREAM) { + if (!IS_SOCK_STREAM(sock_type)) { WARN("[connect] UDP or ICMP stream can't be handled. Rejecting."); errno = EBADF; goto error; diff --git a/src/lib/socket.c b/src/lib/socket.c index cf080b5..bdac610 100644 --- a/src/lib/socket.c +++ b/src/lib/socket.c @@ -32,23 +32,34 @@ LIBC_SOCKET_RET_TYPE tsocks_socket(LIBC_SOCKET_SIG) DBG("[socket] Creating socket with domain %d, type %d and protocol %d", domain, type, protocol);
- if (type & SOCK_STREAM) { + if (IS_SOCK_STREAM(type)) { + /* + * The socket family is not checked here since we accept local socket + * (AF_UNIX) that can NOT do outbound traffic. + */ goto end; } else { - if (domain == AF_INET || domain == AF_INET6) { - /* - * Print this message only in debug mode. Very often, applications - * uses the libc to do DNS resolution which first tries with UDP - * and then with TCP. It's not critical for the user to know that a - * non TCP socket has been denied and since the libc has a fallback - * that works, this message most of the time, simply polutes the - * application's output which can cause issues with external - * applications parsing the output. - */ - DBG("Non TCP inet socket denied. Tor network can't handle it."); - errno = EINVAL; - return -1; + /* + * Non INET[6] socket can't be handle by tor else create the socket. + * The connect function will deny anything that Tor can NOT handle. + */ + if (domain != AF_INET && domain != AF_INET6) { + goto end; } + + /* + * Print this message only in debug mode. Very often, applications uses + * the libc to do DNS resolution which first tries with UDP and then + * with TCP. It's not critical for the user to know that a non TCP + * socket has been denied and since the libc has a fallback that works, + * this message most of the time, simply polutes the application's + * output which can cause issues with external applications parsing the + * output. + */ + DBG("IPv4/v6 non TCP socket denied. Tor network can't handle it."); + errno = EPERM; + return -1; + }
end: diff --git a/tests/test_list b/tests/test_list index 0c22c5e..d5f09ba 100644 --- a/tests/test_list +++ b/tests/test_list @@ -4,3 +4,4 @@ ./unit/test_utils ./unit/test_config-file ./unit/test_socks5 +./unit/test_compat diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am index 3fd9c19..b85f910 100644 --- a/tests/unit/Makefile.am +++ b/tests/unit/Makefile.am @@ -11,7 +11,7 @@ LIBCOMMON=$(top_builddir)/src/common/libcommon.la
LIBTORSOCKS=$(top_builddir)/src/lib/libtorsocks.la
-noinst_PROGRAMS = test_onion test_connection test_utils test_config-file test_socks5 +noinst_PROGRAMS = test_onion test_connection test_utils test_config-file test_socks5 test_compat
EXTRA_DIST = fixtures
@@ -29,3 +29,6 @@ test_config_file_LDADD = $(LIBTAP) $(LIBCOMMON)
test_socks5_SOURCES = test_socks5.c test_socks5_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBTORSOCKS) + +test_compat_SOURCES = test_compat.c +test_compat_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBTORSOCKS) diff --git a/tests/unit/test_compat.c b/tests/unit/test_compat.c new file mode 100644 index 0000000..dec12c3 --- /dev/null +++ b/tests/unit/test_compat.c @@ -0,0 +1,70 @@ +/* + * Copyright (C) 2013 - 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 <common/compat.h> + +#include <tap/tap.h> + +#define NUM_TESTS 7 + +static void test_socket_stream(void) +{ + int type, ret; + + type = SOCK_STREAM; + ret = IS_SOCK_STREAM(type); + ok (ret == 1, "Type SOCK_STREAM valid"); + + type = SOCK_STREAM | SOCK_NONBLOCK; + ret = IS_SOCK_STREAM(type); + ok (ret == 1, "Type SOCK_STREAM | SOCK_NONBLOCK valid"); + + type = SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC; + ret = IS_SOCK_STREAM(type); + ok (ret == 1, "Type SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC valid"); + + type = SOCK_STREAM | 42; + ret = IS_SOCK_STREAM(type); + ok (ret == 0, "Type SOCK_STREAM | 42 is NOT a stream socket"); + + type = SOCK_DGRAM; + ret = IS_SOCK_STREAM(type); + ok (ret == 0, "Type SOCK_DGRAM is NOT a stream socket"); + + type = SOCK_DGRAM | SOCK_NONBLOCK; + ret = IS_SOCK_STREAM(type); + ok (ret == 0, "Type SOCK_DGRAM | SOCK_NONBLOCK is NOT a stream socket"); + + type = SOCK_RAW; + ret = IS_SOCK_STREAM(type); + ok (ret == 0, "Type SOCK_RAW is NOT a stream socket"); +} + +int main(int argc, char **argv) +{ + /* Libtap call for the number of tests planned. */ + plan_tests(NUM_TESTS); + + test_socket_stream(); + + return 0; +}