Commit Graph

104 Commits (2c782d742cd63098c8d54cdae67b9e29ea65337f)

Author SHA1 Message Date
Joe Tsai 61886e031e
ssh/tailssh: fix double race condition with non-pty command (#8405)
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>
1 year ago
Maisem Ali 2e0aa151c9 ssh/tailssh: add support for remote/reverse port forwarding
This basically allows running services on the SSH client and reaching
them from the SSH server during the session.

Updates #6575

Signed-off-by: Maisem Ali <maisem@tailscale.com>
1 year ago
Charlotte Brandhorst-Satzkorn 4e86857313 ssh/tailssh: add ssh session recording failed event type
This change introduces a SSHSessionRecordingFailed event type
that is used when a session recording fails to start or fails during a
session, and the on failure indicates that it should fail open.

Updates tailscale/corp#9967

Signed-off-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
2 years ago
Maisem Ali 0ca8bf1e26 ssh/tailssh: close tty on session close
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>
2 years ago
Brad Fitzpatrick 58ab66ec51 ssh/tailssh: support LDAP users for Tailscale SSH
Fixes #4945

Change-Id: Ie013cb47684cb87928a44f92c66352310bfe53f1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Charlotte Brandhorst-Satzkorn 29ded8f9f9 ssh/tailssh,tailcfg: add connID to ssheventnotifyrequest and castheader
This change adds a ConnectionID field to both SSHEventNotifyRequest and
CastHeader that identifies the ID of a connection to the SSH server.

Updates tailscale/corp#9967

Signed-off-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
2 years ago
Charlotte Brandhorst-Satzkorn 68307c1411 ssh/tailssh: send ssh event notifications on recording failures
This change sends an SSHEventNotificationRequest over noise when a
SSH session is set to fail closed and the session is unable to start
because a recorder is not available or a session is terminated because
connection to the recorder is ended. Each of these scenarios have their
own event type.

Updates tailscale/corp#9967

Signed-off-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
2 years ago
Maisem Ali be190e990f ssh/tailssh: restore support for recording locally
We removed it earlier in 916aa782af, but we still want to support it for some time longer.

Updates tailscale/corp#9967

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Tom DNetto c5bf868940 ssh/tailssh: improve debug logging around revoked sessions
Updates https://github.com/tailscale/corp/issues/10943
Signed-off-by: Tom DNetto <tom@tailscale.com>
2 years ago
Maisem Ali 1b8a0dfe5e ssh/tailssh: also handle recording upload failure during writes
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>
2 years ago
Maisem Ali 7778d708a6 ssh/tailssh: handle dialing multiple recorders and failing open
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>
2 years ago
Andrew Dunham 280255acae
various: add golangci-lint, fix issues (#7905)
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
2 years ago
Brad Fitzpatrick 2c0bda6e2e ssh/tailssh: make Tailscale SSH work on gokrazy
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Maisem Ali e04acabfde ssh/tailssh: fix race in errors returned when starting recorder
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>
2 years ago
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 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
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
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
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 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
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
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
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
Brad Fitzpatrick 7eaf5e509f net/netaddr: start migrating to net/netip via new netaddr adapter package
Updates #5162

Change-Id: Id7bdec303b25471f69d542f8ce43805328d56c12
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Maisem Ali 3e06b9ea7a ssh/tailssh: add "ssh" to conn logs
Fixes #5089

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali 480fd6c797 ssh/tailssh: handle not-authenticated-yet connections in matchRule
Also make more fields in conn.info thread safe, there was previously a
data race here.

Fixes #5110

Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali af412e8874 ssh/tailssh: better handling of signals and exits
We were not handling errors occurred while copying data between the subprocess and the connection.
This makes it so that we pass the appropriate signals when to the process and the connection.

This also fixes mosh.

Updates #4919

Co-authored-by: James Tucker <raggi@tailscale.com>
Co-authored-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago
Maisem Ali 40503ef07a ssh/tailssh: fix logging typo
Signed-off-by: Maisem Ali <maisem@tailscale.com>
2 years ago