Commit Graph

127 Commits (840d69e1ebfc39c6a58ae38a7bf4aae321e7720a)

Author SHA1 Message Date
Maisem Ali 5ba57e4661 ssh/tailssh: add tests for recording failure
Updates tailscale/corp#9967

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali 09d0b632d4 ssh/tailssh: add session recording test for non-pty sessions
Updates tailscale/corp#9967

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali 583e86b7df ssh/tailssh: handle session recording when running in userspace mode
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>
2 years ago
Maisem Ali 8a246487c2 ssh/tailssh: enable recording of non-pty sessions
Updates tailscale/corp#9967

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali 8765568373 ssh/tailssh: add docs to CastHeader fields
Updates tailscale/corp#9967

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Will Norris 57a008a1e1 all: pass log IDs as the proper type rather than strings
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>
2 years ago
Andrew Dunham 13377e6458 ssh/tailssh: always assert our final uid/gid
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
2 years ago
Andrew Dunham 9de8287d47 ssh/tailssh: lock OS thread during incubator
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
2 years ago
Maisem Ali c350cd1f06 ssh/tailssh: use background context for uploading recordings
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>
2 years ago
Maisem Ali d92047cc30 ssh/tailssh: allow recorders to be configured on the first or final action
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>
2 years ago
Maisem Ali 7a97e64ef0 ssh/tailssh: add more metadata to recording header
Updates tailscale/corp#9967

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali 916aa782af ssh/tailssh: stream SSH recordings to configured recorders
Updates tailscale/corp#9967

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Andrew Dunham 39b289578e ssh/tailssh: make uid an int instead of uint64
Follow-up to #7615

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ib4256bff276f6d5cf95838d8e39c87b3643bde37
2 years ago
Andrew Dunham ccace1f7df ssh/tailssh: fix privilege dropping on FreeBSD; add tests
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
2 years ago
Maisem Ali 223713d4a1 tailcfg,all: add and use Node.IsTagged()
Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali e69682678f ssh/tailssh: use context.WithCancelCause
It was using a custom implmentation of the context.WithCancelCause,
replace usage with stdlib.

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali 0582829e00 ssh/tailssh: try launching commands with /usr/bin/login on macOS
Updates #4939

Co-authored-by: Adam Eijdenberg <adam@continusec.com>
Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali 5787989d74 ssh/tailssh: detect user shell correctly on darwin
Updates #6213

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Brad Fitzpatrick b1248442c3 all: update to Go 1.20, use strings.CutPrefix/Suffix instead of our fork
Updates #7123
Updates #5309

Change-Id: I90bcd87a2fb85a91834a0dd4be6e03db08438672
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Maisem Ali 04b57a371e ipn/ipnlocal: drop not required StateKey parameter
This is #cleanup now that #7121 is merged.

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali 5bba65e978 net/memnet: rename from net/nettest
This is just #cleanup to resolve a TODO

Also add a package doc.

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Will Norris 71029cea2d all: update copyright and license headers
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>
2 years ago
Brad Fitzpatrick 1116602d4c ssh/tailssh: add OpenBSD support for Tailscale SSH
And bump go.mod for https://github.com/u-root/u-root/pull/2593

Change-Id: I36ec94c5b2b76d671cb739f1e9a1a43ca1d9d1b1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick be67b8e75b ssh/tailssh: fix Tailscale SSH to non-root tailscaled
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>
2 years ago
Brad Fitzpatrick 8047dfa2dc ssh/tailssh: unify some of the incubator_* GOOS files into incubator.go
In prep for fix for #6888

Change-Id: I79f780c6467a9b7ac03017b27d412d6b0d2f7e6b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 84eaef0bbb ssh/tailssh: don't swallow process exit code in be-child
Thanks to @nshalman and @Soypete for debugging!

Updates #6054

Change-Id: I74550cc31f8a257b37351b8152634c768e1e0a8a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick f837d179b9 ssh/tailssh: fix typo in error message
"look up" is the verb. "lookup" is a noun.

Change-Id: I81c99e12c236488690758fb5c121e7e4e1622a36
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 651e0d8aad ssh/tailssh: add envknob for default PATH
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>
2 years ago
Brad Fitzpatrick 56f7da0cfd
ssh/tailssh: set default Tailscale SSH $PATH for non-interactive commands
Fixes #5285

Co-authored-by: Andrew Dunham <andrew@tailscale.com>
Change-Id: Ic7e967bf6a53b056cac5f21dd39565d9c31563af
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Maisem Ali 2d653230ef ssh/tailssh: only call CloseWrite when both stdout and stderr are done
Updates #5209

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali 4d330bac14 ipn/ipnlocal: add support for multiple user profiles
Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Pat Maddox 9bf3ef4167 ssh/tailssh: add Tailscale SSH (server) support on FreeBSD
Change-Id: I607194b6ef99205e777f3df93a74ffe1a2e0344c
Signed-off-by: Pat Maddox <pat@ratiopbc.com>
2 years ago
Brad Fitzpatrick da8def8e13 all: remove old +build tags
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>
2 years ago
Maisem Ali 1440742a1c ssh/tailssh: use root / as cmd.Dir when users HomeDir doesn't exist
Fixes #5224

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Brad Fitzpatrick e24de8a617 ssh/tailssh: add password-forcing workaround for buggy SSH clients
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>
2 years ago
Emmanuel T Odeke 680f8d9793 all: fix more resource leaks found by staticmajor
Updates #5706

Signed-off-by: Emmanuel T Odeke <emmanuel@orijtech.com>
2 years ago
Maisem Ali f172fc42f7 ssh/tailssh: close sshContext on context cancellation
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>
2 years ago
Maisem Ali 4de1601ef4 ssh/tailssh: add support for sending multiple banners
Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali ecf6cdd830 ssh/tailssh: add TestSSHAuthFlow
Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali f16b77de5d ssh/tailssh: do the full auth flow during ssh auth
Fixes #5091

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali b84ec521bf ssh/tailssh: do not send EOT on session disconnection
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>
2 years ago
Brad Fitzpatrick b1bd96f114 go.mod, ssh/tailssh: fix ImplictAuthMethod typo
Fixes #5745

Change-Id: Ie8bc88bd465a9cb35b0ae7782d61ce96480473ee
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Josh Soref d4811f11a0 all: fix spelling mistakes
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
2 years ago
Eng Zer Jun f0347e841f refactor: move from io/ioutil to io and os packages
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>
2 years ago
Brad Fitzpatrick 74674b110d envknob: support changing envknobs post-init
Updates #5114

Change-Id: Ia423fc7486e1b3f3180a26308278be0086fae49b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 4950fe60bd syncs, all: move to using Go's new atomic types instead of ours
Fixes #5185

Change-Id: I850dd532559af78c3895e2924f8237ccc328449d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 8725b14056 all: migrate more code code to net/netip directly
Instead of going through the tailscale.com/net/netaddr transitional
wrappers.

Updates #5162

Change-Id: I3dafd1c2effa1a6caa9b7151ecf6edd1a3fda3dd
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Maisem Ali 02a765743e ssh/tailssh: fix deadlock in expandDelegateURL
Also rename it to expandDelegateURLLocked, previously it was trying
to acquire the mutex while holding the mutex.

Fixes #5235

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Brad Fitzpatrick a12aad6b47 all: convert more code to use net/netip directly
perl -i -npe 's,netaddr.IPPrefixFrom,netip.PrefixFrom,' $(git grep -l -F netaddr.)
    perl -i -npe 's,netaddr.IPPortFrom,netip.AddrPortFrom,' $(git grep -l -F netaddr. )
    perl -i -npe 's,netaddr.IPPrefix,netip.Prefix,g' $(git grep -l -F netaddr. )
    perl -i -npe 's,netaddr.IPPort,netip.AddrPort,g' $(git grep -l -F netaddr. )
    perl -i -npe 's,netaddr.IP\b,netip.Addr,g' $(git grep -l -F netaddr. )
    perl -i -npe 's,netaddr.IPv6Raw\b,netip.AddrFrom16,g' $(git grep -l -F netaddr. )
    goimports -w .

Then delete some stuff from the net/netaddr shim package which is no
longer neeed.

Updates #5162

Change-Id: Ia7a86893fe21c7e3ee1ec823e8aba288d4566cd8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 6a396731eb all: use various net/netip parse funcs directly
Mechanical change with perl+goimports.

Changed {Must,}Parse{IP,IPPrefix,IPPort} to their netip variants, then
goimports -d .

Finally, removed the net/netaddr wrappers, to prevent future use.

Updates #5162

Change-Id: I59c0e38b5fbca5a935d701645789cddf3d7863ad
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago