[tor-commits] [torsocks/master] Fix: socket() type check SOCK_STREAM
dgoulet at torproject.org
dgoulet at torproject.org
Fri Apr 4 22:40:28 UTC 2014
commit 3b8687973757cbecfe22a7e5708da15523111c55
Author: David Goulet <dgoulet at 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 at 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 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 <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;
+}
More information about the tor-commits
mailing list