From 7920ffb0953423b144126fc56b6bcff77821ca07 Mon Sep 17 00:00:00 2001 From: Daniel Lenski Date: Fri, 31 Dec 2021 12:25:43 -0500 Subject: [PATCH] dumb_socketpair(): try to use AF_UNIX socketpair on Windows 10 and newer As a workaround for the lack of socketpair() on Windows, we use dumb_socketpair() from https://github.com/ncm/selectable-socketpair/blob/master/socketpair.c, which uses a pair of IPv4 local sockets (listening on 127.0.0.1). Unfortunately, and maddeningly, it's possible for the local IPv4 routes (127.0.0.0/8) to be deleted on Windows; this will prevent dumb_socketpair() from working in its current form. See https://gitlab.com/openconnect/openconnect/-/issues/228 and https://gitlab.com/openconnect/openconnect/-/issues/361 for examples of how to trigger this condition. The simplest way to do it is with `route /f`. Fortunately, Windows 10+ supports AF_UNIX sockets, which we should be able to use to sidestep this issue. This feature was announced in December 2017 in https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows. It is evidently incomplete, and also buggy: 1) "Abstract" sockets don't actually seem to work, and this is probably why the socketpair() function still isn't implemented, even though AF_UNIX support would naturally enable it: https://github.com/microsoft/WSL/issues/4240#issuecomment-506437851 2) Actual MSDN documentation for this feature is seemingly nonexistent. 3) MinGW lacks the expected header, but other FLOSS projects show how to embed the needed `struct sockaddr_un` definition: - https://github.com/MisterDA/ocaml/commit/5855ce5ffd931a2802d5b9a5b2987ab0b276fd0a - https://github.com/curl/curl/blob/curl-7_74_0/lib/config-win32.h#L725-L734 Nevertheless, it works well enough that we can use it in OpenConnect. The modified version of dumb_socketpair() in this patch tries to create an AF_UNIX socketpair, and only uses IPv4 local sockets as a fallback. With this modified version, I confirm that I can do the following on Windows 10: 1) Nuke routes with `route /f`. 2) Reconnect network adapter via GUI. 3) Confirm that IPv4 local route (127.0.0.0/8) still hasn't been recreated. 4) Run OpenConnect and successfully create the cmd pipe. So this appears to fix https://gitlab.com/openconnect/openconnect/-/issues/228 and https://gitlab.com/openconnect/openconnect/-/issues/361, at least on Windows 10 and newer. Signed-off-by: Daniel Lenski --- compat.c | 79 +++++++++++++++++++++++++++++++++++++++------------- configure.ac | 4 +++ 2 files changed, 63 insertions(+), 20 deletions(-) diff --git a/compat.c b/compat.c index b3c5be5d..9f7e578a 100644 --- a/compat.c +++ b/compat.c @@ -376,6 +376,17 @@ WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH # include # include +#ifdef HAVE_AFUNIX_H +#include +#else +#define UNIX_PATH_MAX 108 +struct sockaddr_un +{ + ADDRESS_FAMILY sun_family; /* AF_UNIX */ + char sun_path[UNIX_PATH_MAX]; /* pathname */ +} SOCKADDR_UN, *PSOCKADDR_UN;; +#endif /* HAS_AFUNIX_H */ + /* dumb_socketpair: * If make_overlapped is nonzero, both sockets created will be usable for * "overlapped" operations via WSASend etc. If make_overlapped is zero, @@ -387,12 +398,14 @@ WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH int dumb_socketpair(OPENCONNECT_CMD_SOCKET socks[2], int make_overlapped) { union { + struct sockaddr_un unaddr; struct sockaddr_in inaddr; struct sockaddr addr; } a; OPENCONNECT_CMD_SOCKET listener; int e; - socklen_t addrlen = sizeof(a.inaddr); + int domain = AF_UNIX; + socklen_t addrlen = sizeof(a.unaddr); DWORD flags = (make_overlapped ? WSA_FLAG_OVERLAPPED : 0); int reuse = 1; @@ -402,37 +415,63 @@ int dumb_socketpair(OPENCONNECT_CMD_SOCKET socks[2], int make_overlapped) } socks[0] = socks[1] = -1; - listener = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); - if (listener == INVALID_SOCKET) - return SOCKET_ERROR; + listener = socket(domain, SOCK_STREAM, 0); + if (listener == INVALID_SOCKET) { + /* AF_UNIX/SOCK_STREAM became available in Windows 10: + * https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows + * + * We need to fallback to AF_INET on earlier versions of Windows. + */ + domain = AF_INET; + addrlen = sizeof(a.inaddr); + listener = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); + if (listener == INVALID_SOCKET) + return SOCKET_ERROR; + } memset(&a, 0, sizeof(a)); - a.inaddr.sin_family = AF_INET; - a.inaddr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); - a.inaddr.sin_port = 0; + if (domain == AF_UNIX) { + /* XX: Abstract sockets (filesystem-independent) don't work, contrary to + * the claims of the aforementioned blog post: + * https://github.com/microsoft/WSL/issues/4240#issuecomment-549663217 + * + * So we must use a named path, and that comes with all the attendant + * problems of permissions and collisions. + */ + a.unaddr.sun_family = AF_UNIX; + snprintf(a.unaddr.sun_path, UNIX_PATH_MAX, "./openconnect-%ld.cmdsock", GetCurrentProcessId()); + } else { + a.inaddr.sin_family = AF_INET; + a.inaddr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + a.inaddr.sin_port = 0; + } for (;;) { - if (setsockopt(listener, SOL_SOCKET, SO_REUSEADDR, - (char *) &reuse, (socklen_t) sizeof(reuse)) == -1) - break; - if (bind(listener, &a.addr, sizeof(a.inaddr)) == SOCKET_ERROR) + if (domain == AF_INET && + setsockopt(listener, SOL_SOCKET, SO_REUSEADDR, + (char *) &reuse, (socklen_t) sizeof(reuse)) == -1) + break; + if (bind(listener, &a.addr, addrlen) == SOCKET_ERROR) break; - memset(&a, 0, sizeof(a)); - if (getsockname(listener, &a.addr, &addrlen) == SOCKET_ERROR) - break; - // win32 getsockname may only set the port number, p=0.0005. - // ( https://docs.microsoft.com/windows/win32/api/winsock/nf-winsock-getsockname ): - a.inaddr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); - a.inaddr.sin_family = AF_INET; + if (domain == AF_INET) { + memset(&a, 0, sizeof(a)); + if (getsockname(listener, &a.addr, &addrlen) == SOCKET_ERROR) + break; + + // win32 getsockname may only set the port number, p=0.0005. + // ( https://docs.microsoft.com/windows/win32/api/winsock/nf-winsock-getsockname ): + a.inaddr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + a.inaddr.sin_family = AF_INET; + } if (listen(listener, 1) == SOCKET_ERROR) break; - socks[0] = WSASocket(AF_INET, SOCK_STREAM, 0, NULL, 0, flags); + socks[0] = WSASocket(domain, SOCK_STREAM, 0, NULL, 0, flags); if (socks[0] == INVALID_SOCKET) break; - if (connect(socks[0], &a.addr, sizeof(a.inaddr)) == SOCKET_ERROR) + if (connect(socks[0], &a.addr, addrlen) == SOCKET_ERROR) break; socks[1] = accept(listener, NULL, NULL); diff --git a/configure.ac b/configure.ac index 600acf53..a0210929 100644 --- a/configure.ac +++ b/configure.ac @@ -91,6 +91,10 @@ case $host_os in esac AC_SUBST(WINTUN_ARCH, "$wintun_arch") + # Per https://github.com/MisterDA/ocaml/commit/5855ce5ffd931a2802d5b9a5b2987ab0b276fd0a, + # "The header file declares the `struct sockaddr_un` type, but hasn't been picked by mingw-w64." + AC_CHECK_HEADER([afunix.h], AC_DEFINE([HAVE_AF_UNIX_H], 1, [MinGW has afunix.h])) + # MINGW_HAS_SECURE_API may only work on newer MinGW: # https://stackoverflow.com/a/51977723 AC_DEFINE(MINGW_HAS_SECURE_API, 1, [Try to make getenv_s and _putenv_s available]) -- 2.49.0