From 7d83056a1bb3273940ac162a42ae383b42911ee7 Mon Sep 17 00:00:00 2001 From: Percy Wegmann Date: Mon, 5 Aug 2024 17:09:34 -0500 Subject: [PATCH] ssh/tailssh: fix SSH on busybox systems This involved the following: 1. Pass the su command path as first of args in call to unix.Exec to make sure that busybox sees the correct program name. Busybox is a single executable userspace that implements various core userspace commands in a single binary. You'll see it used via symlinking, so that for example /bin/su symlinks to /bin/busybox. Busybox knows that you're trying to execute /bin/su because argv[0] is '/bin/su'. When we called unix.Exec, we weren't including the program name for argv[0], which caused busybox to fail with 'applet not found', meaning that it didn't know which command it was supposed to run. 2. Tell su to whitelist the SSH_AUTH_SOCK environment variable in order to support ssh agent forwarding. 3. Run integration tests on alpine, which uses busybox. 4. Increment CurrentCapabilityVersion to allow turning on SSH V2 behavior from control. Fixes #12849 Signed-off-by: Percy Wegmann --- Makefile | 3 +- ssh/tailssh/incubator.go | 23 ++++++--- ssh/tailssh/tailssh_integration_test.go | 4 +- ssh/tailssh/testcontainers/Dockerfile | 64 ++++++++++++++----------- tailcfg/tailcfg.go | 3 +- 5 files changed, 59 insertions(+), 38 deletions(-) diff --git a/Makefile b/Makefile index eae85f0cb..98c3d36cc 100644 --- a/Makefile +++ b/Makefile @@ -117,7 +117,8 @@ sshintegrationtest: ## Run the SSH integration tests in various Docker container echo "Testing on ubuntu:focal" && docker build --build-arg="BASE=ubuntu:focal" -t ssh-ubuntu-focal ssh/tailssh/testcontainers && \ echo "Testing on ubuntu:jammy" && docker build --build-arg="BASE=ubuntu:jammy" -t ssh-ubuntu-jammy ssh/tailssh/testcontainers && \ echo "Testing on ubuntu:mantic" && docker build --build-arg="BASE=ubuntu:mantic" -t ssh-ubuntu-mantic ssh/tailssh/testcontainers && \ - echo "Testing on ubuntu:noble" && docker build --build-arg="BASE=ubuntu:noble" -t ssh-ubuntu-noble ssh/tailssh/testcontainers + echo "Testing on ubuntu:noble" && docker build --build-arg="BASE=ubuntu:noble" -t ssh-ubuntu-noble ssh/tailssh/testcontainers && \ + echo "Testing on alpine:latest" && docker build --build-arg="BASE=alpine:latest" -t ssh-alpine-latest ssh/tailssh/testcontainers help: ## Show this help @echo "\nSpecify a command. The choices are:\n" diff --git a/ssh/tailssh/incubator.go b/ssh/tailssh/incubator.go index aafe0c743..2572bc2a2 100644 --- a/ssh/tailssh/incubator.go +++ b/ssh/tailssh/incubator.go @@ -399,7 +399,7 @@ func tryExecLogin(dlogf logger.Logf, ia incubatorArgs) error { return nil } loginArgs := ia.loginArgs(loginCmdPath) - dlogf("logging in with %s %+v", loginCmdPath, loginArgs) + dlogf("logging in with %+v", loginArgs) // If Exec works, the Go code will not proceed past this: err = unix.Exec(loginCmdPath, loginArgs, os.Environ()) @@ -435,13 +435,18 @@ func trySU(dlogf logger.Logf, ia incubatorArgs) (handled bool, err error) { defer sessionCloser() } - loginArgs := []string{"-l", ia.localUser} + loginArgs := []string{ + su, + "-w", "SSH_AUTH_SOCK", // pass through SSH_AUTH_SOCK environment variable to support ssh agent forwarding + "-l", + ia.localUser, + } if ia.cmd != "" { // Note - unlike the login command, su allows using both -l and -c. loginArgs = append(loginArgs, "-c", ia.cmd) } - dlogf("logging in with %s %q", su, loginArgs) + dlogf("logging in with %+v", loginArgs) // If Exec works, the Go code will not proceed past this: err = unix.Exec(su, loginArgs, os.Environ()) @@ -473,9 +478,15 @@ func findSU(dlogf logger.Logf, ia incubatorArgs) string { return "" } - // First try to execute su -l -c true to make sure su supports the - // necessary arguments. - err = exec.Command(su, "-l", ia.localUser, "-c", "true").Run() + // First try to execute su -w SSH_AUTH_SOCK -l -c true + // to make sure su supports the necessary arguments. + err = exec.Command( + su, + "-w", "SSH_AUTH_SOCK", + "-l", + ia.localUser, + "-c", "true", + ).Run() if err != nil { dlogf("su check failed: %s", err) return "" diff --git a/ssh/tailssh/tailssh_integration_test.go b/ssh/tailssh/tailssh_integration_test.go index 62413e3a5..695c26d9b 100644 --- a/ssh/tailssh/tailssh_integration_test.go +++ b/ssh/tailssh/tailssh_integration_test.go @@ -349,7 +349,7 @@ func TestSSHAgentForwarding(t *testing.T) { // Run tailscale SSH server and connect to it username := "testuser" - tailscaleAddr := testServer(t, username, false) // TODO: make this false to use V2 behavior + tailscaleAddr := testServer(t, username, false) tcl, err := ssh.Dial("tcp", tailscaleAddr, &ssh.ClientConfig{ HostKeyCallback: ssh.InsecureIgnoreHostKey(), }) @@ -387,7 +387,7 @@ func TestSSHAgentForwarding(t *testing.T) { o, err := s.CombinedOutput(fmt.Sprintf(`ssh -T -o StrictHostKeyChecking=no -p %s upstreamuser@%s "true"`, upstreamPort, upstreamHost)) if err != nil { - t.Fatalf("unable to call true command: %s\n%s", err, o) + t.Fatalf("unable to call true command: %s\n%s\n-------------------------", err, o) } } diff --git a/ssh/tailssh/testcontainers/Dockerfile b/ssh/tailssh/testcontainers/Dockerfile index ff27981ef..887c71a40 100644 --- a/ssh/tailssh/testcontainers/Dockerfile +++ b/ssh/tailssh/testcontainers/Dockerfile @@ -1,19 +1,24 @@ ARG BASE FROM ${BASE} +ARG BASE + RUN echo "Install openssh, needed for scp." -RUN apt-get update -y && apt-get install -y openssh-client +RUN if echo "$BASE" | grep "ubuntu:"; then apt-get update -y && apt-get install -y openssh-client; fi +RUN if echo "$BASE" | grep "alpine:"; then apk add openssh; fi -RUN groupadd -g 10000 groupone -RUN groupadd -g 10001 grouptwo -# Note - we do not create the user's home directory, pam_mkhomedir will do that +# Note - on Ubuntu, we do not create the user's home directory, pam_mkhomedir will do that # for us, and we want to test that PAM gets triggered by Tailscale SSH. -RUN useradd -g 10000 -G 10001 -u 10002 testuser +RUN if echo "$BASE" | grep "ubuntu:"; then groupadd -g 10000 groupone && groupadd -g 10001 grouptwo && useradd -g 10000 -G 10001 -u 10002 testuser; fi +# On Alpine, we can't configure pam_mkhomdir, so go ahead and create home directory. +RUN if echo "$BASE" | grep "alpine:"; then addgroup -g 10000 groupone && addgroup -g 10001 grouptwo && adduser -u 10002 -D testuser && addgroup testuser groupone && addgroup testuser grouptwo; fi -RUN echo "Set up pam_mkhomedir." -RUN sed -i -e 's/Default: no/Default: yes/g' /usr/share/pam-configs/mkhomedir || echo "might not be ubuntu" -RUN cat /usr/share/pam-configs/mkhomedir -RUN pam-auth-update --enable mkhomedir +RUN if echo "$BASE" | grep "ubuntu:"; then \ + echo "Set up pam_mkhomedir." && \ + sed -i -e 's/Default: no/Default: yes/g' /usr/share/pam-configs/mkhomedir && \ + cat /usr/share/pam-configs/mkhomedir && \ + pam-auth-update --enable mkhomedir \ + ; fi COPY tailscaled . COPY tailssh.test . @@ -22,11 +27,11 @@ RUN chmod 755 tailscaled # RUN echo "First run tests normally." RUN eval `ssh-agent -s` && TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestSSHAgentForwarding -RUN rm -Rf /home/testuser +RUN if echo "$BASE" | grep "ubuntu:"; then rm -Rf /home/testuser; fi RUN TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegrationSFTP -RUN rm -Rf /home/testuser +RUN if echo "$BASE" | grep "ubuntu:"; then rm -Rf /home/testuser; fi RUN TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegrationSCP -RUN rm -Rf /home/testuser +RUN if echo "$BASE" | grep "ubuntu:"; then rm -Rf /home/testuser; fi RUN TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegrationSSH RUN echo "Then run tests as non-root user testuser and make sure tests still pass." @@ -35,28 +40,31 @@ RUN TAILSCALED_PATH=`pwd`tailscaled eval `su -m testuser -c ssh-agent -s` && su RUN TAILSCALED_PATH=`pwd`tailscaled su -m testuser -c "./tailssh.test -test.v -test.run TestIntegration TestDoDropPrivileges" RUN chown root:root /tmp/tailscalessh.log -RUN echo "Then run tests in a system that's pretending to be SELinux in enforcing mode" -RUN mv /usr/bin/login /tmp/login_orig -# Use nonsense for /usr/bin/login so that it fails. -# It's not the same failure mode as in SELinux, but failure is good enough for test. -RUN echo "adsfasdfasdf" > /usr/bin/login -RUN chmod 755 /usr/bin/login -# Simulate getenforce command -RUN printf "#!/bin/bash\necho 'Enforcing'" > /usr/bin/getenforce -RUN chmod 755 /usr/bin/getenforce -RUN eval `ssh-agent -s` && TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestSSHAgentForwarding -RUN TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegration -RUN mv /tmp/login_orig /usr/bin/login -RUN rm /usr/bin/getenforce +RUN if echo "$BASE" | grep "ubuntu:"; then \ + echo "Then run tests in a system that's pretending to be SELinux in enforcing mode" && \ + # Remove execute permissions for /usr/bin/login so that it fails. + mv /usr/bin/login /tmp/login_orig && \ + # Use nonsense for /usr/bin/login so that it fails. + # It's not the same failure mode as in SELinux, but failure is good enough for test. + echo "adsfasdfasdf" > /usr/bin/login && \ + chmod 755 /usr/bin/login && \ + # Simulate getenforce command + printf "#!/bin/bash\necho 'Enforcing'" > /usr/bin/getenforce && \ + chmod 755 /usr/bin/getenforce && \ + eval `ssh-agent -s` && TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestSSHAgentForwarding && \ + TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegration && \ + mv /tmp/login_orig /usr/bin/login && \ + rm /usr/bin/getenforce \ + ; fi RUN echo "Then remove the login command and make sure tests still pass." RUN rm `which login` RUN eval `ssh-agent -s` && TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestSSHAgentForwarding -RUN rm -Rf /home/testuser +RUN if echo "$BASE" | grep "ubuntu:"; then rm -Rf /home/testuser; fi RUN TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegrationSFTP -RUN rm -Rf /home/testuser +RUN if echo "$BASE" | grep "ubuntu:"; then rm -Rf /home/testuser; fi RUN TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegrationSCP -RUN rm -Rf /home/testuser +RUN if echo "$BASE" | grep "ubuntu:"; then rm -Rf /home/testuser; fi RUN TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegrationSSH RUN echo "Then remove the su command and make sure tests still pass." diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 5a06c89ff..90be4bb83 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -147,7 +147,8 @@ // - 102: 2024-07-12: NodeAttrDisableMagicSockCryptoRouting support // - 103: 2024-07-24: Client supports NodeAttrDisableCaptivePortalDetection // - 104: 2024-08-03: SelfNodeV6MasqAddrForThisPeer now works -const CurrentCapabilityVersion CapabilityVersion = 104 +// - 105: 2024-08-05: Fixed SSH behavior on systems that use busybox (issue #12849) +const CurrentCapabilityVersion CapabilityVersion = 105 type StableID string