Previously we would error out when the recording server disappeared after the in memory
buffer filled up for the io.Copy. This makes it so that we handle failing open correctly
in that path.
Updates tailscale/corp#9967
Signed-off-by: Maisem Ali <maisem@tailscale.com>
This adds support to try dialing out to multiple recorders each
with a 5s timeout and an overall 30s timeout. It also starts respecting
the actions `OnRecordingFailure` field if set, if it is not set
it fails open.
Updates tailscale/corp#9967
Signed-off-by: Maisem Ali <maisem@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
There were two code paths that could fail depending on how fast
the recorder responses. This fixes that by returning the correct
error from both paths.
Fixes#7707
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Previously it would dial out using the http.DefaultClient, however that doesn't work
when tailscaled is running in userspace mode (e.g. when testing).
Updates tailscale/corp#9967
Signed-off-by: Maisem Ali <maisem@tailscale.com>
This change focuses on the backend log ID, which is the mostly commonly
used in the client. Tests which don't seem to make use of the log ID
just use the zero value.
Signed-off-by: Will Norris <will@tailscale.com>
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
Otherwise we see errors like
```
ssh-session(sess-20230322T005655-5562985593): recording: error sending recording to <addr>:80: Post "http://<addr>:80/record": context canceled
```
The ss.ctx is closed when the session closes, but we don't want to break the upload at that time. Instead we want to wait for the session to
close the writer when it finishes, which it is already doing.
Updates tailscale/corp#9967
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Currently we only send down recorders in first action, allow the final action
to replace them but not to drop them.
Updates tailscale/corp#9967
Signed-off-by: Maisem Ali <maisem@tailscale.com>
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>
"look up" is the verb. "lookup" is a noun.
Change-Id: I81c99e12c236488690758fb5c121e7e4e1622a36
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>
If the username includes a suffix of +password, then we accept
password auth and just let them in like it were no auth.
This exists purely for SSH clients that get confused by seeing success
to their initial auth type "none".
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Change-Id: I616d4c64d042449fb164f615012f3bae246e91ec
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This was preventing tailscaled from shutting down properly if there were
active sessions in certain states (e.g. waiting in check mode).
Signed-off-by: Maisem Ali <maisem@tailscale.com>
This was assumed to be the fix for mosh not working, however turns out
all we really needed was the duplicate fd also introduced in the same
commit (af412e8874).
Fixes#5103
Signed-off-by: Maisem Ali <maisem@tailscale.com>
The io/ioutil package has been deprecated as of Go 1.16 [1]. This commit
replaces the existing io/ioutil functions with their new definitions in
io and os packages.
Reference: https://golang.org/doc/go1.16#ioutil
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Maisem Ali
Andrew Dunham
Brad Fitzpatrick
Will Norris
Andrew Dunham
Pat Maddox
Emmanuel T Odeke
Josh Soref
Eng Zer Jun