And don't return a comma-separated string. That's kinda weird
signature-wise, and not needed by half the callers anyway. The callers
that care can do the join themselves.
Updates #cleanup
Change-Id: Ib5ad51a3c6b663d868eba14fe9dc54b2609cfb0d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Shells on OpenBSD don't support the -l option. This means that when
handling SSH in-process, we can't give the user a login shell, but this
change at least allows connecting at all.
Updates #13338
Signed-off-by: Percy Wegmann <percy@tailscale.com>
Shells on FreeBSD don't support the -l option. This means that when
handling SSH in-process, we can't give the user a login shell, but this
change at least allows connecting at all.
Updates #13338
Signed-off-by: Percy Wegmann <percy@tailscale.com>
These erroneously blocked a recent PR, which I fixed by simply
re-running CI. But we might as well fix them anyway.
These are mostly `printf` to `print` and a couple of `!=` to `!Equal()`
Updates #cleanup
Signed-off-by: Will Norris <will@tailscale.com>
This allows passing through any environment variables that we set ourselves, for example DBUS_SESSION_BUS_ADDRESS.
Updates #11175
Co-authored-by: Mario Minardi <mario@tailscale.com>
Signed-off-by: Percy Wegmann <percy@tailscale.com>
Add logic to set environment variables that match the SSH rule's
`acceptEnv` settings in the SSH session's environment.
Updates https://github.com/tailscale/corp/issues/22775
Signed-off-by: Mario Minardi <mario@tailscale.com>
Instead of changing the working directory before launching the incubator process,
this now just changes the working directory after dropping privileges, at which
point we're more likely to be able to enter the user's home directory since we're
running as the user.
For paths that use the 'login' or 'su -l' commands, those already take care of changing
the working directory to the user's home directory.
Fixes#13120
Signed-off-by: Percy Wegmann <percy@tailscale.com>
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 <percy@tailscale.com>
This allows the SSH_AUTH_SOCK environment variable to work inside of
su and agent forwarding to succeed.
Fixes#12467
Signed-off-by: Percy Wegmann <percy@tailscale.com>
Checking in the incubator as this used to do fails because
the getenforce command is not on the PATH.
Updates #12442
Signed-off-by: Percy Wegmann <percy@tailscale.com>
This allows pam authentication to run for ssh sessions, triggering
automation like pam_mkhomedir.
Updates #11854
Signed-off-by: Percy Wegmann <percy@tailscale.com>
If the connection provided to sftp.NewServer is closed,
Serve returns the io.EOF error verbatim from io.Reader.Read.
This is an odd error since this is an expected situation,
so we manually ignore io.EOF.
This is somewhat buggy since the sftp package itself
incorrectly reports io.EOF in cases where it should actually
be reporting io.ErrUnexpectedEOF.
See https://github.com/pkg/sftp/pull/554 which patches Serve to
return nil on clean closes and fixes buggy uses of io.ReadFull.
Fixes#8592
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
There are two race conditions in output handling.
The first race condition is due to a misuse of exec.Cmd.StdoutPipe.
The documentation explicitly forbids concurrent use of StdoutPipe
with exec.Cmd.Wait (see golang/go#60908) because Wait will
close both sides of the pipe once the process ends without
any guarantees that all data has been read from the pipe.
To fix this, we allocate the os.Pipes ourselves and
manage cleanup ourselves when the process has ended.
The second race condition is because sshSession.run waits
upon exec.Cmd to finish and then immediately proceeds to call ss.Exit,
which will close all output streams going to the SSH client.
This may interrupt any asynchronous io.Copy still copying data.
To fix this, we close the write-side of the os.Pipes after
the process has finished (and before calling ss.Exit) and
synchronously wait for the io.Copy routines to finish.
Fixes#7601
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Co-authored-by: Maisem Ali <maisem@tailscale.com>
On redhat 9 and similarly locked down systems, root user does not have
access to a users directory. This fix does not set a directory for the
incubator process and instead sets the directory when the actual process
requested by remote user is executed.
Fixes#8118
Signed-off-by: Derek Burdick <derek-burdick@users.noreply.github.com>
Trying to SSH when SELinux is enforced results in errors like:
```
➜ ~ ssh ec2-user@<ip>
Last login: Thu Jun 1 22:51:44 from <ip2>
ec2-user: no shell: Permission denied
Connection to <ip> closed.
```
while the `/var/log/audit/audit.log` has
```
type=AVC msg=audit(1685661291.067:465): avc: denied { transition } for pid=5296 comm="login" path="/usr/bin/bash" dev="nvme0n1p1" ino=2564 scontext=system_u:system_r:unconfined_service_t:s0 tcontext=unconfined_u:unconfined_r:unconfined_t:s0 tclass=process permissive=0
```
The right fix here would be to somehow install the appropriate context when
tailscale is installed on host, but until we figure out a way to do that
stop using the `login` cmd in these situations.
Updates #4908
Signed-off-by: Maisem Ali <maisem@tailscale.com>
We were only closing on side of the pty/tty pair.
Close the other side too.
Thanks to @fritterhoff for reporting and debugging the issue!
Fixes#8119
Signed-off-by: Maisem Ali <maisem@tailscale.com>
The previous commit 58ab66e added ssh/tailssh/user.go as part of
working on #4945. So move some more user-related code over to it.
Updates #cleanup
Change-Id: I24de66df25ffb8f867e1a0a540d410f9ef16d7b0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This adds an initial and intentionally minimal configuration for
golang-ci, fixes the issues reported, and adds a GitHub Action to check
new pull requests against this linter configuration.
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I8f38fbc315836a19a094d0d3e986758b9313f163
Move the assertions about our post-privilege-drop UID/GID out of the
conditional if statement and always run them; I haven't been able to
find a case where this would fail. Defensively add an envknob to disable
this feature, however, which we can remove after the 1.40 release.
Updates #7616
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Iaec3dba9248131920204bd6c6d34bbc57a148185
This makes it less likely that we trip over bugs like golang/go#1435.
Updates #7616
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ic28c03c3ad8ed5274a795c766b767fa876029f0e
On FreeBSD and Darwin, changing a process's supplementary groups with
setgroups(2) will also change the egid of the process, setting it to the
first entry in the provided list. This is distinct from the behaviour on
other platforms (and possibly a violation of the POSIX standard).
Because of this, on FreeBSD with no TTY, our incubator code would
previously not change the process's gid, because it would read the
newly-changed egid, compare it against the expected egid, and since they
matched, not change the gid. Because we didn't use the 'login' program
on FreeBSD without a TTY, this would propagate to a child process.
This could be observed by running "id -p" in two contexts. The expected
output, and the output returned when running from a SSH shell, is:
andrew@freebsd:~ $ id -p
uid andrew
groups andrew
However, when run via "ssh andrew@freebsd id -p", the output would be:
$ ssh andrew@freebsd id -p
login root
uid andrew
rgid wheel
groups andrew
(this could also be observed via "id -g -r" to print just the gid)
We fix this by pulling the details of privilege dropping out into their
own function and prepending the expected gid to the start of the list on
Darwin and FreeBSD.
Finally, we add some tests that run a child process, drop privileges,
and assert that the final UID/GID/additional groups are what we expect.
More information can be found in the following article:
https://www.usenix.org/system/files/login/articles/325-tsafrir.pdf
Updates #7616
Alternative to #7609
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I0e6513c31b121108b50fe561c89e5816d84a45b9
This updates all source files to use a new standard header for copyright
and license declaration. Notably, copyright no longer includes a date,
and we now use the standard SPDX-License-Identifier header.
This commit was done almost entirely mechanically with perl, and then
some minimal manual fixes.
Updates #6865
Signed-off-by: Will Norris <will@tailscale.com>
Fix regression from 337c77964b where
tailscaled started calling Setgroups. Prior to that, SSH to a non-root
tailscaled was working.
Instead, ignore any failure calling Setgroups if the groups are
already correct.
Fixes#6888
Change-Id: I561991ddb37eaf2620759c6bcaabd36e0fb2a22d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Thanks to @nshalman and @Soypete for debugging!
Updates #6054
Change-Id: I74550cc31f8a257b37351b8152634c768e1e0a8a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
As backup plan, just in case the earlier fix's logic wasn't correct
and we want to experiment in the field or have users have a quicker
fix.
Updates #5285
Change-Id: I7447466374d11f8f609de6dfbc4d9a944770826d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The //go:build syntax was introduced in Go 1.17:
https://go.dev/doc/go1.17#build-lines
gofmt has kept the +build and go:build lines in sync since
then, but enough time has passed. Time to remove them.
Done with:
perl -i -npe 's,^// \+build.*\n,,' $(git grep -l -F '+build')
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Brad Fitzpatrick
Percy Wegmann
Will Norris
Mario Minardi
Joe Tsai
Derek Burdick
Maisem Ali
Andrew Dunham
Andrew Dunham
Pat Maddox
Josh Soref