Skip to content
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.

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

enable ssh connection to a unix socket #435

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

oliverkurth
Copy link

@oliverkurth oliverkurth commented Sep 2, 2023

This is based on PRs #431 and #162 (thanks to @pjd and @kalvdans !) which enables connecting to a unix socket where getaddrinfo(3) supports AF_UNIX and adds connecting to a unix socket where it is not (which is the case for MacOS and Linux and probably most others). This is achieved by changing resolve_host to return a faked up struct addrinfo for a unix socket, when the "hostname" starts with a /. The code is partially reused from channels.c / connect_to_helper().

This also enforces HashKnownHosts when a unix socket is used, to prevent corrupting the known_hosts file with special characters - file paths can contain spaces, !, * and others which are not allowed in host names but do have special meaning in known_hosts.

Tested successfully with MacOS 13.5 (using gcc 13) and Ubuntu 22.04.

Example usage:

ssh -A okurth@/run/sshd.sock

There are multiple ways to set up a ssh server listening on a unix socket. One is setting

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

in a systemd socket config (for example /etc/systemd/system/ssh.socket).

Other ways are using socat/netcat, or port forwarding with ssh, using a unix socket on the listening end and TCP on the connecting end.

  • amend man page
  • automatic testing

The added test is failing in the github workflow, but works for me locally on both MacOS and Ubuntu 22.04. I haven't figured out why yet, but I will probably refactor it.

ssh.c Outdated Show resolved Hide resolved
sshconnect.c Outdated Show resolved Hide resolved
sshconnect.c Outdated Show resolved Hide resolved
sshconnect.c Outdated
Comment on lines 516 to 523
error("ssh: connect to host %s port %s: %s",
error("ssh: connect to %s%s: %s",
host, strport, errno == 0 ? "failure" : strerror(errno));
Copy link

Choose a reason for hiding this comment

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

Adapt error message for unix paths.

Copy link
Author

Choose a reason for hiding this comment

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

It's not needed here. For a UNIX socket, strport will be an empty string, so %s%s will just print what's in host, which contains the path. See:

% /usr/local/bin/ssh okurth@/foo.sock
ssh: connect to /foo.sock: No such file or directory

Copy link

Choose a reason for hiding this comment

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

Yes, but it is broken for IP addresses:

$ ./ssh user@localhost -p 3333
ssh: connect to localhost3333: Connection refused

Copy link
Author

Choose a reason for hiding this comment

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

Oh, I missed that, thanks! This is from one of pjd's original changes.

And fixed, with 5e81370

Copy link

Choose a reason for hiding this comment

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

I think pjd's original changes were fine, but later destroyed by my attempts to simplify it :/ Thanks for restoring!

@oliverkurth oliverkurth force-pushed the topic/okurth/unix-socket branch 3 times, most recently from c387c28 to 01454b1 Compare September 5, 2023 16:43
Copy link

@kalvdans kalvdans left a comment

Choose a reason for hiding this comment

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

Test works but spams a non-fatal error message:

$ make t-exec TEST_SSH_UNSAFE_PERMISSIONS=1
[...]
run test connect-unix.sh ...
/home/chn/repo/openssh-portable/regress/test-exec.sh: 32: kill: No such process

ok simple connect

Thanks a lot for writing a test!

regress/connect-unix.sh Outdated Show resolved Hide resolved
regress/connect-unix.sh Outdated Show resolved Hide resolved
sshconnect.c Show resolved Hide resolved
@oliverkurth oliverkurth force-pushed the topic/okurth/unix-socket branch 6 times, most recently from 8ce8cde to 5e25bd6 Compare December 31, 2023 00:26
@oliverkurth
Copy link
Author

Alright, test is mostly working now, except two jobs:

  • CIFuzz / Fuzzing fails with:
+ clang++ -O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link -stdlib=libc++ -std=c++11 -DCIPHER_NONE_AVAIL=1 -I. -L. -Lopenbsd-compat -g regress/misc/fuzz-harness/agent_fuzz.cc -o /github/workspace/build-out/agent_fuzz sk-dummy.o agent_fuzz_helper.o ssh-sk.o -lssh -lopenbsd-compat -lz -Wl,-Bstatic -lcrypto -Wl,-Bdynamic -fsanitize=fuzzer
/usr/bin/ld: /usr/bin/ld: DWARF error: invalid or unhandled FORM value: 0x25
agent_fuzz_helper.o: in function `process_add_smartcard_key':
agent_fuzz_helper.c:(.text.process_add_smartcard_key[process_add_smartcard_key]+0xdb6): undefined reference to `pkcs11_make_cert'
clang-15: error: linker command failed with exit code 1 (use -v to see invocation)
2023-12-31 03:26:07,809 - root - ERROR - Building fuzzers failed.
2023-12-31 03:26:07,809 - root - ERROR - Error building fuzzers for (commit: 2b76606bae5cf45aa14de283e7778a811804ef33, pr_ref: refs/pull/435/merge).

I don't think it's related.

  • ubuntu-20.04, valgrind-1 is related to the change, and fails with:
run test connect-unix.sh ...
Extra argument OBJ=/home/runner/work/openssh-portable/openssh-portable/regress.
FATAL: no sshd.socket created

But I don't know how to fix this.


start_sshd_unix()
{
${SUDO} env NC=${NC} SSHD=${SSHD} OBJ=${OBJ} ${SCRIPT_DIR}/sshd-unix.sh&

Choose a reason for hiding this comment

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

One reason for the CI error FATAL: no sshd.socket created is because if a timing issue. Here we run the whole script sshd-unix.sh in the background, which first cleans up sshd.socket before starting netcat to create a new file. This creates a timing window where the wait for creating of sshd.socket finishes too early. I suggest the following change:

modified   regress/connect-unix.sh
@@ -4,7 +4,7 @@ SCRIPT_DIR=`dirname $0`
 
 start_sshd_unix()
 {
-	${SUDO} env NC=${NC} SSHD=${SSHD} OBJ=${OBJ} ${SCRIPT_DIR}/sshd-unix.sh&
+	${SUDO} env NC=${NC} SSHD=${SSHD} OBJ=${OBJ} ${SCRIPT_DIR}/sshd-unix.sh
 
 	trace "wait for sshd-unix"
 	i=0;
modified   regress/sshd-unix.sh
@@ -7,7 +7,7 @@ run_sshd_unix() {
 	rm -f ${OBJ}/sshd.fifo && mkfifo ${OBJ}/sshd.fifo
 	rm -f ${OBJ}/sshd.socket
 	touch ${OBJ}/sshd-unix.log ; chmod 0644 ${OBJ}/sshd-unix.log
-	cat ${OBJ}/sshd.fifo | ${SSHD} -i -f ${OBJ}/sshd_config "$@" -E ${OBJ}/sshd-unix.log | ${NC} -l -U ${OBJ}/sshd.socket > ${OBJ}/sshd.fifo
+	cat ${OBJ}/sshd.fifo | ${SSHD} -i -f ${OBJ}/sshd_config "$@" -E ${OBJ}/sshd-unix.log | ${NC} -l -U ${OBJ}/sshd.socket > ${OBJ}/sshd.fifo &
 }
 
 run_sshd_unix

I haven't been able to run the test suite in full on my own machine, so I can't tell if this fixes the CI error. But at least it should be more logically correct.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I just pushed a change with that fix.

@svmhdvn
Copy link

svmhdvn commented May 2, 2024

I found this patch after reading https://tim.siosm.fr/blog/2023/12/19/ssh-over-unix-socket/ and I'm really interested to see it merged. What's the status on this, or is there any help in pushing it forward if needed?

@oliverkurth
Copy link
Author

@svmhdvn I am using this on a daily basis, and I find it very useful (I use it to connect to a VM running under VMware Fusion, without having to bother with the network).

Unfortunately, I did not get any response from the maintainers, which has been very disappointing. But maybe I haven't reached out to the right people.

@svmhdvn
Copy link

svmhdvn commented May 2, 2024

I agree, I think this is a very useful PR, I'd like to help push this so that we don't have to maintain local patches to security-sensitive software like this. @djmdjm who is the right maintainer to take a look at this? It seems to be in a good working state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants