Skip to content

Add support for AF_UNIX #674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: latestw_all
Choose a base branch
from
Open

Add support for AF_UNIX #674

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3a77587
Add support for AF_UNIX
bilby91 Apr 12, 2023
f7a8825
Guard AF_UNIX paths with HAVE_AFUNIX_H
bilby91 Apr 13, 2023
6dd4311
Use Windows GetTempPath instead of /tmp
bilby91 Apr 13, 2023
b31d181
Conditionally use af_unix and fileio
bilby91 Apr 14, 2023
6cc801b
Comment regress/cfgmatch.sh temporary
bilby91 Apr 14, 2023
705ed7e
Add back regress/cfgmatch.sh
bilby91 Apr 14, 2023
8ca026c
Fix regress/cgmatch.sh comments
bilby91 Apr 14, 2023
36e5db5
WIP: Add agent.sh tests
bilby91 Apr 18, 2023
fd096f5
Compare with AGENT_PIPE_ID
bilby91 Apr 19, 2023
0d38b15
Remove unnecessary HAVE_AFUNIX_H conditions
bilby91 Feb 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion authfd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,13 @@ ssh_get_authentication_socket_path(const char *authsocket, int *fdp)
sunaddr.sun_family = AF_UNIX;
strlcpy(sunaddr.sun_path, authsocket, sizeof(sunaddr.sun_path));

if ((sock = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
#ifdef HAVE_AFUNIX_H
sock = w32_afunix_socket(&sunaddr);
#else
sock = socket(AF_UNIX, SOCK_STREAM, 0);
#endif

if (sock == -1)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this #ifdef condition? Shouldn't you be able to always call w32_afunix_socket as this method already handles the #ifdef condition internally? Same question applies to misc.c and mux.c.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right. Will look into simplifying! Thanks

return SSH_ERR_SYSTEM_ERROR;

/* close on exec */
Expand Down
2 changes: 1 addition & 1 deletion contrib/win32/openssh/bash_tests_iterator.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ try

# These are the known failed testcases.
# transfer.sh, rekey.sh tests fail on CygWin v3.4.0, but succeeds with v3.3.6
$known_failed_testcases = @("agent.sh", "key-options.sh", "forward-control.sh", "integrity.sh", "krl.sh", "cert-hostkey.sh", "cert-userkey.sh", "percent.sh", "transfer.sh", "rekey.sh")
$known_failed_testcases = @("key-options.sh", "forward-control.sh", "integrity.sh", "krl.sh", "cert-hostkey.sh", "cert-userkey.sh", "percent.sh", "transfer.sh", "rekey.sh")
Copy link

@Onur-TURAN Onur-TURAN Aug 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't use agent.sh? regress folder have agent.sh

$known_failed_testcases_skipped = @()

$start_time = (Get-Date)
Expand Down
6 changes: 6 additions & 0 deletions contrib/win32/openssh/config.h.vs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1526,6 +1526,12 @@
/* Use PIPES instead of a socketpair() */
#define USE_PIPES 1

/* Define name for the ssh-agent Windows named pipe */
#define AGENT_PIPE_ID L"\\\\.\\pipe\\openssh-ssh-agent"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to consider this issue specifically, and I have the following reservations in mind. maybe This command has probability an LFL vulnerability. For example, the X user has 600 chmod permissions. But the folder path '\pipe\openssh-ssh-agent' may have directory path access. If you must define another folder path, use a command line shortcut or pipeline. This will only affect the permission of one file and will not affect any other folder paths.


/* define 1 if afunix.h is available */
#define HAVE_AFUNIX_H 1

/* Define if you want to sanitize fds */
/* #undef USE_SANITISE_STDFD */

Expand Down
19 changes: 19 additions & 0 deletions contrib/win32/win32compat/socketio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@
#include "inc\utf.h"
#include "misc_internal.h"
#include "debug.h"
#include "../../../config.h"
#ifdef HAVE_AFUNIX_H
#include <afunix.h>
#endif

#define INTERNAL_SEND_BUFFER_SIZE 70*1024 //70KB
#define INTERNAL_RECV_BUFFER_SIZE 70*1024 //70KB
Expand Down Expand Up @@ -118,7 +122,12 @@ socketio_acceptEx(struct w32_io* pio)
}

/* create accepting socket */
#ifdef HAVE_AFUNIX_H
context->accept_socket = socket(addr.ss_family, SOCK_STREAM, IPPROTO_IP);
#else
context->accept_socket = socket(addr.ss_family, SOCK_STREAM, IPPROTO_TCP);
#endif
Comment on lines +125 to +129
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this #ifdef condition necessary? If IPPROTO_TCP does not work for some reason, would it be possible to always use IPPROTO_IP (or just 0)? man socket(2) suggests that specifying 0 (which is the value of IPPROTO_IP) will use the default, which again is TCP.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would need to try this out. I don't recall if IPPROTO_TCP was not working.


if (context->accept_socket == INVALID_SOCKET) {
errno = errno_from_WSALastError();
debug3("acceptEx - socket() ERROR:%d, io:%p", WSAGetLastError(), pio);
Expand Down Expand Up @@ -756,6 +765,9 @@ socketio_accept(struct w32_io* pio, struct sockaddr* addr, int* addrlen)
int
socketio_connectex(struct w32_io* pio, const struct sockaddr* name, int namelen)
{
#ifdef HAVE_AFUNIX_H
struct sockaddr_un tmp_unix;
#endif

struct sockaddr_in tmp_addr4;
struct sockaddr_in6 tmp_addr6;
Expand All @@ -778,6 +790,13 @@ socketio_connectex(struct w32_io* pio, const struct sockaddr* name, int namelen)
tmp_addr4.sin_port = 0;
tmp_addr = (SOCKADDR*)&tmp_addr4;
tmp_addr_len = sizeof(tmp_addr4);
#ifdef HAVE_AFUNIX_H
} else if (name->sa_family == AF_UNIX) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this project runs on a Unix operating system without a hybrid kernel, the connection type requirements should avoid pointer access. On Unix systems, memory assets cannot be controlled while the process is running, which could lead to potential vulnerabilities.

ZeroMemory(&tmp_unix, sizeof(tmp_unix));
tmp_unix.sun_family = AF_UNIX;
tmp_addr = (SOCKADDR*)&tmp_unix;
tmp_addr_len = sizeof(tmp_unix);
#endif
} else {
errno = ENOTSUP;
debug3("connectex - ERROR: unsuppored address family:%d, io:%p", name->sa_family, pio);
Expand Down
2 changes: 0 additions & 2 deletions contrib/win32/win32compat/ssh-agent/agent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ HANDLE sshagent_client_primary_token;
static HANDLE ioc_port = NULL;
static BOOL debug_mode = FALSE;

#define AGENT_PIPE_ID L"\\\\.\\pipe\\openssh-ssh-agent"

static HANDLE event_stop_agent;
static OVERLAPPED ol;
static HANDLE pipe;
Expand Down
70 changes: 67 additions & 3 deletions contrib/win32/win32compat/w32fd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include <sys\utime.h>
#include "misc_internal.h"
#include "debug.h"
#include "../../../config.h"

/* internal table that stores the fd to w32_io mapping*/
struct w32fd_table {
Expand Down Expand Up @@ -298,9 +299,9 @@ w32_socket(int domain, int type, int protocol)
errno = 0;
if (min_index == -1)
return -1;

if (domain == AF_UNIX && type == SOCK_STREAM) {
pio = fileio_afunix_socket();
pio = fileio_afunix_socket();
if (pio == NULL)
return -1;
pio->type = NONSOCK_FD;
Expand All @@ -309,7 +310,70 @@ w32_socket(int domain, int type, int protocol)
if (pio == NULL)
return -1;
pio->type = SOCK_FD;
}
}

fd_table_set(pio, min_index);
debug4("socket:%d, socktype:%d, io:%p, fd:%d ", pio->sock, type, pio, min_index);
return min_index;
}

int
w32_afunix_socket(struct sockaddr_un* addr)
{
#ifdef HAVE_AFUNIX_H
/*
If HAVE_AFUNIX_H is defined, we can be dealing with the ssh-agent named pipe or
a AF_UNIX socket if ssh forwarding is enabled. If the addr->sun_path is the
the well known named pipe, we open the socket with w32_fileio.
*/
int len = wcslen(AGENT_PIPE_ID);
char* pipeid = (char*)malloc(len + 1);
memset(pipeid, 0, len + 1);

if(wcstombs(pipeid, AGENT_PIPE_ID, len + 1) != (size_t) -1 && strcmp(addr->sun_path, pipeid) == 0)
return w32_fileio_socket(SOCK_STREAM, 0);
else
return w32_unix_socket(SOCK_STREAM, 0);
Comment on lines +333 to +336
Copy link

@JojOatXGME JojOatXGME Feb 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand the code correctly, you could technically just call w32_socket(AF_INET, SOCK_STREAM, 0) and w32_socket(AF_UNIX, SOCK_STREAM, 0), instead of introducing two new methods? Although I understand that AF_INET would be misleading.

Besides, instead of checking whether addr->sun_path equals AGENT_PIPE_ID, I guess you could also just check the prefix. If the path starts with \\.\pipe\, I think you can assume that it is a named pipe. This way, the solution stays compatible with other named pipes as well. For example, if a user uses Pageant (PuTTY authentication agent), they might also use named pipes, but the pipe may be named //./pipe/pageant.<username>.<random_sequence>. (This means you should probably also accept both, slash (/) and backslash (\) in the prefix.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally I was thinking of behaving this way just for this implementation but I think the prefix makes sense in order to support other implementations.

#else
return w32_socket(AF_UNIX, SOCK_STREAM, 0);
#endif
}

int
w32_unix_socket(int type, int protocol)
{
int domain = AF_UNIX;
int min_index = fd_table_get_min_index();
struct w32_io* pio = NULL;

errno = 0;
if (min_index == -1)
return -1;

pio = socketio_socket(domain, type, protocol);
if (pio == NULL)
return -1;
pio->type = SOCK_FD;

fd_table_set(pio, min_index);
debug4("socket:%d, socktype:%d, io:%p, fd:%d ", pio->sock, type, pio, min_index);
return min_index;
}

int
w32_fileio_socket(int type, int protocol)
{
int min_index = fd_table_get_min_index();
struct w32_io* pio = NULL;

errno = 0;
if (min_index == -1)
return -1;

pio = fileio_afunix_socket();
if (pio == NULL)
return -1;
pio->type = NONSOCK_FD;

fd_table_set(pio, min_index);
debug4("socket:%d, socktype:%d, io:%p, fd:%d ", pio->sock, type, pio, min_index);
Expand Down
1 change: 1 addition & 0 deletions contrib/win32/win32compat/w32fd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ struct w32_io {
#define FILETYPE(pio) (GetFileType(WINHANDLE(pio)))
extern HANDLE main_thread;

int w32_afunix_socket(struct sockaddr_un* addr);
BOOL w32_io_is_blocking(struct w32_io*);
BOOL w32_io_is_io_available(struct w32_io* pio, BOOL rd);
int wait_for_any_event(HANDLE* events, int num_events, DWORD milli_seconds);
Expand Down
4 changes: 4 additions & 0 deletions misc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1928,7 +1928,11 @@ unix_listener(const char *path, int backlog, int unlink_first)
return -1;
}

#ifdef HAVE_AFUNIX_H
sock = w32_afunix_socket(&sunaddr);
#else
sock = socket(PF_UNIX, SOCK_STREAM, 0);
#endif
if (sock == -1) {
saved_errno = errno;
error_f("socket: %.100s", strerror(errno));
Expand Down
8 changes: 7 additions & 1 deletion mux.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2268,7 +2268,13 @@ muxclient(const char *path)
fatal("ControlPath too long ('%s' >= %u bytes)", path,
(unsigned int)sizeof(addr.sun_path));

if ((sock = socket(PF_UNIX, SOCK_STREAM, 0)) == -1)
#ifdef HAVE_AFUNIX_H
sock = w32_afunix_socket(&addr);
#elif
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this elif intentional? It doesn't even compile.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thedarkcolour That's wrong, it should be an #else. I'll fix it.

sock = socket(PF_UNIX, SOCK_STREAM, 0);
#endif

if (sock == -1)
fatal_f("socket(): %s", strerror(errno));

if (connect(sock, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
Expand Down
Loading