* clean up pidfile and reloads
* reduce log spam during graceful shutdowns
* hold an exclusive advisory lock on the pidfile
* do systemd reloads with the pidfile rather than $MAINPID
* prioritize signals over internal restarts
Adds the new linter and configures it to prevent direct error
comparisions and type assertions in favor of errors.Is and errors.As.
All current violations caught by the linter have been updated
accordingly.
* Add pre- and post-detect hooks to the multiplexer
This gets rid of the dedicated "already written" mechanism in
multiplexer.Conn.
* Add a way for net.Conn to override ServerVersion
* Resumption handshake, client and server
* Enable SSH resumption in teleport and tsh
* Disable resumption in a subtest of TestIntegrations/Disconnection
* Add back the removed tests
* minor refactor of runClientResumable
* Disable resumption for a different subtest
* Add an option to disable SSH connection resumption in Connect (#36712)
Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com>
---------
Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com>
`x/crypto/ssh` is adding a `fipsonly` mode (similar to `crypto/tls/fipsonly`) in
golang/go#64769. Once this has landed and been released, we'll swap to using it.
In the meantime, update our hardcoded algorithm set to match the algorithms listed
in https://go-review.googlesource.com/c/crypto/+/550515.
The only difference is that `aes192-ctr` is kept in `FIPSCiphers`, as it seems to be
approved by NIST SP 800-131A rev 2 (page 17).
Additionally, we now define a list of approved public key authentication algorithms for
SSH server connections. This isn't configurable for normal use case (yet), only handled
for FIPS builds.
* Increase `MinPasswordLength` from 6 to 12 characters
- PCI DSS v4.0 control 8.3.6 requires a minimum password length of 12 characters.
- NIST SP 800-63B section 5.1.1.1 requires a minimum password length of 8 characters.
* Increase `AccountLockInterval` from 20 to 30 minutes
- PCI DSS v4.0 control 8.3.4 requires a minimum lockout duration of 30 minutes.
This will contribute to better supporting customers to meet certain compliance requirements.
Ref #1936.
Ref #7687.
* Special case the subsystems handled by teleport exec
This only applies to sftp, and lets us do a real reexec where that's
possible (i.e. launching /proc/self/exe on Linux), making sftp work even
if Teleport has been replaced by some wrapper or has been deleted.
* Delete leftover comment
* Update godoc for ExecCommand.Command
Ensure that `proxy_ssh_sessions_total` is incremented in both the
root and the leaf cluster if a session to a node in the leaf cluster
is established through the root cluster. This now also prevents
leaf clusters from terminating during a graceful shutdown if there
is an active connection that was created from the root.
Fixes#32605.
This change drains unused SSH channels and requests to prevent a
situation where an attacker could repeatedly open channels and
send data that won't be read, causing Teleport to eventually run
out of memory.
The ssh session was not being closed for web sessions which resulted
in zombie sessions being left around until the ssh service was
restarted. TestTerminal was updated to assert that the session
tracker eventually transitions to the terminated state when the
client terminates the web socket.
Fixes#32120
* Make SSH node listener to use multiplexer to leverage parsing of signed PROXY headers
Before for backward compatibility reason we used regular listener and later
we wrapped connection into handler that could parse signed PROXY header or
proxyHelloSignature for getting correct user IP. But
after removal of proxyHelloSignature we don't need the wrapper anymore.
* Improve error message
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
---------
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
* Added Fuzz Seeds for services/fuzz_test.go
* Fuzzing improvements for `lib/srv/db` fuzzing
This ensures that all fuzzing under `lib/srv/db` has the following:
* Good seeds that provide basic levels of coverage
* Enabled in OSS-Fuzz
* Fuzzing improvements for `lib/sshutils`
This improves the fuzzing in `lib/sshutils/sftp/parse_test.go` and `lib/sshutils/x11/display_test.go`.
Seeds were added and additional local fuzzing was done. In addition these functions are now enabled in `oss-fuzz`.
This commit updates all `err == io.EOF` comparisons to use
`errors.Is(err, io.EOF)`. This is necessary when the error may have been
wrapped and fixes at least one current breakage (in `tsh request ls`).
`golang.org/x/tools/refactor/eg` was very handy for this, I used the
following template:
```go
package teleport
import (
"errors"
"io"
)
func before(err error) bool { return err == io.EOF }
func after(err error) bool { return errors.Is(err, io.EOF) }
```
A complex regex was previously used to parse 'tsh scp'
destinations, which was hard to understand and even harder
to maintain for those not intimately familiar with regex,
despite it being heavily commented. Instead parse destinations
directly and add more test cases and a fuzz test to ensure
parsing won't panic and the new parse function doesn't
have any regressions.
Previously a Teleport client using SFTP would resolve remote host user
home directories by making a subsystem request to a Teleport server
which would return the home directory. The problem was the subsystem
request counted as an open session, which could make the SFTP file
transfer fail. This was frustrating and didn't make much sense, but
after reading the SFTP specification again I realized that SFTP servers
are to handle relative paths by assuming they start at the user's home
directory. So let the server figure out the correct path and remove any
tilde prefixes from remote paths.
* refactor SFTP backend to use upstream dep, not our fork
This change also greatly reduces the number of SFTP audit logs.
Now SFTP events are only sent when files are opened or modified
in any way, instead of for *every* SFTP request.
* added to SFTP integration test
* fix error when handling setstat on dirs
* fix linter warning
* move file/dir permission constants to lib/defaults package
Part of https://github.com/gravitational/teleport/pull/23546
This will add a fileTransferRequest to a session and allow environment variables to be passed from the webUI in order to validate a request that happens "outside" the moderated session (via HTTP request).
* lazily forward SSH agents when connecting to agentless nodes
When connecting to unregistered OpenSSH nodes, the SSH agent is always
forwarded. When connecting to registered OpenSSH (agentless) nodes
however, the SSH agent doesn't *need* to be forwarded, so only do so
if the user explicitly asks to.
* test SSH agent forwarding in agentless integration test
To prevent users from being able to directly connect to registered
OpenSSH (Agentless) nodes, the OpenSSH CA was introduced. When
connecting to Agentless nodes, the Proxy will forward the connection
so the session can be recorded. The forwarding SSH server is configured
to authenticate to the target node with a certificate signed by OpenSSH
CA, which users do not have access to.
* Allow node to handle old and new way of client IP propagation on same listener
With addition of signed PROXY headers, node was listening on multiplexer, but because
of that it couldn't processing incoming connection from older proxies
when ProxyHelloSignature was used, because
both ends were waiting for the other side to send data first.
Here we integrate ability to handle PROXY headers into connection itself,
so we can start ssh server without waiting for multiplexer to detect connection
* Remove unneeded code
* Improve comment's wording
Co-authored-by: Michael Wilson <mike@mdwn.dev>
* Improve comment's wording
Co-authored-by: Michael Wilson <mike@mdwn.dev>
* Fix imports
* Unexport function
* Add godoc
* Remove unneeded comment
* Move ProxyHelloSignature to constants
* Check that proxyline is verified
* Use Warn() instead of Warnf()
* Move ProxyHelloSignature to api/constants
* Add timeout for getting host CA during proxyline verification.
* Rearrange conditions
Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
* Clarify comment.
---------
Co-authored-by: Michael Wilson <mike@mdwn.dev>
Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
* SCP -Change file attrs only when requested
OpenSSH sets 0644 by default and allows copy the source permissions by passing `-p` flag.
Our current implementation sets the file permissions even when client didn't use `-p` flag which can fail if the user doesn't have permissions to set it. This patch matches the OpenSSH behavior.
* Fix formatting
* sshserver: Correctly handle PuTTY winadj channel requests
Fixes#19365
* Actually send a failure
* dispatch changes
* Reimplement wantReply handling
* Also handle windadj in forwarding server
* lint: Fix typo
* lint: Invert return order to put err last
* Use nil return payload instead
* Use trace.ReplyAlreadyHandled instead
* Handle reply internally and set req.WantReply to false
* Use t.Cleanup()
* behaviour is not a misspelling but whatever makes you happy
* Suggestions from code review
* use 'path.Join' instead of 'filepath.Join' to build remote file paths
tsh on Windows when using 'filepath.Join' would create paths with forward
slashes in them, which non-Windows Nodes couldn't handle. Consistently use
'path.Join' to avoid issues.
Fixes#18919
* add comment explaining why 'path.Join' is used
* move comment to make linter happy
* Our current SFTP implementation uses a single channel to transfer data. Connections with a high latency cause very slow transfer. github.com/pkg/sftp allows transferring data using concurrent streams, which seems to solve the problem. In order to utilize the feature, the remote file must implement the WriteTo() or ReadFrom() and Stat() methods. I've moved the progress bar to relay the data from a local file, as we can still call Read() or Write() on it to update the progress.
This PR doesn't address the high CPU usage by github.com/pkg/sftp on low latency links.
Closes#20579
* Cleanup
* Fix fileTransferProgress
* Simplify interfaces
* Move helper to utils package
Add comments
* Added comments
* Change canceler implementation to return correct values from Read/Write
* Fix infinite recursive call in cancelWriter.
Introduces `limiter.Listener` to provide a consistent and reusable
mechanism for limiting incoming connections per client. The new
listener is used by `sshutils/server.go` instead of manually applying
limits in `HandleConnection`.
This is particularly important now that the Proxy SSH port multiplexes
both SSH and gRPC. Each listener is now wrapped in a `limiter.Listener`
that uses the same `limiter.ConnectionsListener` to ensure that the
connection limits for the Proxy are enforced for all traffic on the
port.
- Add a generalized client store made up of a key, profile, and trusted certs store. Each sub store can support different backends (~/.tsh, identity_file, in-memory).
- Replace custom identity file handling with in-memory client store.
- Fix issues with trusted certs handling.
Andrew Burke