Allow ssh(1) to connect through UNIX domain sockets on systems where getaddrinfo(3) supports AF_UNIX

[kalvdans]

This is a restart of #162 by @pjd which was accidentally closed.

On systems where getaddrinfo(3) supports AF_LOCAL family allow ssh(1) to connect through UNIX domain sockets.

This is useful in combination with connections forwarding over UNIX domain
sockets.

Let's say I have system A behind a NAT and system B accessible in the Internet.
I want to enable some users of system B to connect to sshd on system A.
I can leverage stream forwarding and run on system A the following command:

ssh -f -N -R /tmp/ssh.sock:127.0.0.1:22 -l pjd B.example.org

Now, on the system B only I will be able to connect to system A over UNIX domain socket.
On system B with this patch I can run:

ssh -v /tmp/ssh.sock
[...]
debug1: Connecting to /tmp/ssh.sock [/tmp/ssh.sock].
debug1: Connection established.
[...]
debug1: Authenticating to /tmp/ssh.sock as 'root'
[...]
Authenticated to /tmp/ssh.sock.

% tail -1 ~/.ssh/known_hosts
/tmp/ssh.sock ecdsa-sha2-nistp256 [...]

TODO:

• Decide how to do with abstract socket address (sun_path starting with a NUL byte)
• Documentation
• Any security impacts?

[rwmjones]

We want to use this for doing CI testing of our nbdkit-ssh-plugin. The current test must allocate a TCP port which is not very safe or scalable in a CI system.

[oliverkurth]

Thanks for doing this - I was ready to do the same, but you beat me to it.

I will test this as soon as I find some time for it. My use case is on MacOS, so I will try that.

[kalvdans]

Hmm, I couldn't get this to work under Linux, but it is not very strange because AF_UNIX has been removed from getaddrinfo in glibc. From glibc's Changelog:

2001-05-23 Thorsten Kukuk kukuk@suse.de

• sysdeps/posix/getaddrinfo.c: Support for AF_UNIX commented out.
• posix/tst-getaddrinfo.c: Remove AF_UNIX test.

[oliverkurth]

That is weird. If you grep for AF_LOCAL there is no match. But lots of matches with AF_UNIX:

okurth@okurth-a01 openssh-portable % git grep AF_UNIX      
auth-pam.c:     if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, socks) == -1) {
authfd.c:       sunaddr.sun_family = AF_UNIX;
authfd.c:       if ((sock = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
canohost.c:     case AF_UNIX:
...

It's just a minor detail though.

[kalvdans]

I will re-implement from scratch without using getaddrinfo(). Expect another pull request coming up.

[kalvdans]

My use case is on MacOS, so I will try that.

Seems MacOS doesn't support AF_UNIX in its libSystem getaddrinfo either. [source]

[randomstuff]

ssh -v /tmp/ssh.sock

Which server identity is used to authenticate the server? /tmp/ssh.sock?

Note that you can already do that using:

ssh machinea -o"ProxyCommand nc -U /tmp/ssh.sock

Or with ssh_config:

Host machinea
ProxyCommand nc -U /tmp/ssh.sock

[rwmjones]

TBH I think a more important element is the ability for sshd to listen on a Unix domain socket ... and when I looked at the patch I see that's unfortunately not possible. Is this planned too?

[randomstuff]

sshd to listen on a Unix domain socket

If you are wondering about -R /tmp/ssh.sock:127.0.0.1:22, this is already possible with OpenSSH (client+server) since 6.7.

[rwmjones]

If you are wondering about -R /tmp/ssh.sock:127.0.0.1:22, this is already possible with OpenSSH (client+server) since 6.7.

That's not sshd listening directly (and only) on a Unix domain socket.

[oliverkurth]

@rwmjones

TBH I think a more important element is the ability for sshd to listen on a Unix domain socket ... and when I looked at the patch I see that's unfortunately not possible. Is this planned too?

There is no need if you use systemd. You can make it listen on a unix socket with a simple .socket file.

Example:

[Unit]
Description=OpenBSD Secure Shell server socket
Before=ssh.service
ConditionPathExists=!/etc/ssh/sshd_not_to_be_run

[Socket]
ListenStream=/var/run/sshd.sock
Accept=yes

[Install]
WantedBy=sockets.target

Where /var/run/sshd.sock is the socket.

[kalvdans]

TBH I think a more important element is the ability for sshd to listen on a Unix domain socket ... and when I looked at the patch I see that's unfortunately not possible. Is this planned too?

Yes, I was confused how this patch could help your usecase :) A subset of your tests could be done by communicating via stdin/stdout directly to /usr/lib/openssh/sftp-server. I have no use myself for sshd listening to unix socket unfortunately...

[rwmjones]

Oh I see, passing in a pre-opened socket using systemd socket activation (it doesn't actually need systemd to do this since the protocol is well-documented). I will have to investigate this as I hadn't thought about that before.

[oliverkurth]

I missed something important about this PR - the limitation that "On systems where getaddrinfo(3) supports AF_LOCAL".

So looks like neither Linux (or actually glibc) nor MacOS support this - and I was wondering why the patch could be so simple ;-). Anyway, that doesn't address my use case, so I need to make changes.

I am wondering though - which systems do support this?

[oliverkurth]

It works now with this change: fe3ccc9 , but there are things to iron out - it currently segfaults when accepting the host key, unless I give the option -o StrictHostKeyChecking=no.

This works: ./ssh -o StrictHostKeyChecking=no -l okurth /path/to/socket>

Currently the socket file path needs to start with a /, but I'll do better checks for that too.

[kalvdans]

It works now with this change: fe3ccc9 , but there are things to iron out

Thanks! That was my first idea too, but we can't call freeaddrinfo() on homemade struct addrinfo's, maybe that's why it crashes. How can we set up a pull request that we can both push to?

[oliverkurth]

I fixed it, and force pushed it to my fork: f965e1e . Or just look at my branch there: https://github.com/oliverkurth/openssh-portable/commits/topic/okurth/unix-socket .

The crash has nothing to do with freeaddrinfo(). It's just that hostaddr can be NULL in get_hostfile_hostname_ipaddr(). Using freeaddrinfo() may be an issue if getaddrinfo() allocates the structure in a different way - for example, maybe it allocates addrinfo and ai_addr in one chunk, not two. Not sure if that is a concern.

I tested this with both MacOS and Linux (Ubuntu 22.04).

[oliverkurth]

How can we set up a pull request that we can both push to?

Not sure, but it's okay if you just cherry-pick my change and add it here.

[oliverkurth]

Using freeaddrinfo() may be an issue if getaddrinfo() allocates the structure in a different way - for example, maybe it allocates addrinfo and ai_addr in one chunk, not two. Not sure if that is a concern.

Ah, looks like that is indeed how they do it for unix sockets for forwardings, see channels.c / connect_to_helper() :

		/*
		 * Fake up a struct addrinfo for AF_UNIX connections.
		 * channel_connect_ctx_free() must check ai_family
		 * and use free() not freeaddirinfo() for AF_UNIX.
		 */
		ai = xmalloc(sizeof(*ai) + sizeof(*sunaddr));

Alright, I can do the same.

[oliverkurth]

Latest commit acafda4 addresses (not using) freeaddrinfo().

There is an issue with the known hosts checking when the path to the socket contains spaces (and unfortunately, even though I abhor spaces in file names, that's in my use case). But with the option -oHashKnownHosts=yes it works. The reason is that spaces are used to delimit items in the known_hosts file. Using hashes works around that.

$ ./ssh -oHashKnownHosts=yes -l okurth /run/sshd.sock
Welcome to Ubuntu 22.04.3 LTS (GNU/Linux 6.2.0-26-generic x86_64)

 ...

Last login: Sat Aug 26 21:57:46 2023 from UNKNOWN
okurth@ubuntu2-devel:~ $ 

[kalvdans]

I had less time than I though to do this... but the plan is let this PR rest in peace and start over from master with an implementation that doesn't try to use getaddrinfo(). Then write a regression test and documentation. If you @oliverkurth could squash your commits on top of master it would be helpful!

[oliverkurth]

My changes so far are all in one commit, I always amended and force pushed to my branch https://github.com/oliverkurth/openssh-portable/commits/topic/okurth/unix-socket .

I think it's actually mostly done, the only thing I need to decide is whether to just set HashKnownHosts=yes implicitly when using a unix socket - trying to get a file path into the known_hosts file is opening a can of worms, because file names allow almost all characters except /, and lots of chracters are used for special purposes in that file. Which works because those are not allowed in host names.

But I wonder - is this even the right place to get this merged into upstream? When I look at https://github.com/openssh/openssh-portable/pulls?q=is%3Apr+is%3Amerged , almost no PRs get merged.

[randomstuff]

I think it's actually mostly done, the only thing I need to decide is whether to just set HashKnownHosts=yes implicitly when using a unix socket - trying to get a file path into the known_hosts file is opening a can of worms, because file names allow almost all characters except /, and lots of characters are used for special purposes in that file. Which works because those are not allowed in host names.

Maybe is should always be used with HostKeyAlias or through Hostname:

Host foo
Hostname /path/to/socket

Host /path/to/socket
HostKeyAlias foo

i.e. a UNIX socket should no be used a a server identity in known_hosts.

[oliverkurth]

Maybe is should always be used with HostKeyAlias or through Hostname

That's one way to address it, but I think it's not very convenient. Maybe I don't want to edit my config file every time I want to connect to a new socket.

I think enforcing HashKnownHosts=yes is a limitation that we can live with.

[oliverkurth]

I think enforcing HashKnownHosts=yes is a limitation that we can live with.

Did that, with 30b4f1b on https://github.com/oliverkurth/openssh-portable/commits/topic/okurth/unix-socket

Contributors, @pjd , @kalvdans , would it be okay if I create another PR from my branch to replace this one and squash all commits together? I have already rebased on current master.

It would be nice if this gets tested. Not sure how to add automatic tests for this, I will try to figure that out. Also, documentation.

Decide how to do with abstract socket address (sun_path starting with a NUL byte)

I don't think we need to worry about that.

Next, we'd need to figure out how to get this into upstream.

[oliverkurth]

New PR, let's continue the discussion there: #435

[kalvdans]

Closing, since not many systems support UNIX domain sockets in getaddrinfo().