mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-27 06:49:07 +01:00
[client] Validate and bound SSH session requests and channels (#8344)
This commit is contained in:
@@ -267,7 +267,7 @@ func (s *Server) prepareCommandEnv(logger *log.Entry, localUser *user.User, sess
|
||||
return env
|
||||
}
|
||||
|
||||
func (s *Server) handlePtyLogin(logger *log.Entry, session ssh.Session, privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, _ <-chan ssh.Window) bool {
|
||||
func (s *Server) handlePtyLogin(logger *log.Entry, session ssh.Session, privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, winCh <-chan ssh.Window) bool {
|
||||
if privilegeResult.User == nil {
|
||||
logger.Errorf("no user in privilege result")
|
||||
return false
|
||||
@@ -276,7 +276,7 @@ func (s *Server) handlePtyLogin(logger *log.Entry, session ssh.Session, privileg
|
||||
shell := getUserShell(privilegeResult.User.Uid)
|
||||
logger.Infof("starting interactive shell: %s", shell)
|
||||
|
||||
s.executeCommandWithPty(logger, session, nil, privilegeResult, ptyReq, nil)
|
||||
s.executeCommandWithPty(logger, session, nil, privilegeResult, ptyReq, winCh)
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -376,7 +376,9 @@ func (s *Server) detectUtilLinuxLogin(context.Context) bool {
|
||||
}
|
||||
|
||||
// executeCommandWithPty executes a command with PTY allocation on Windows using ConPty
|
||||
func (s *Server) executeCommandWithPty(logger *log.Entry, session ssh.Session, _ *exec.Cmd, privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, _ <-chan ssh.Window) bool {
|
||||
func (s *Server) executeCommandWithPty(logger *log.Entry, session ssh.Session, _ *exec.Cmd, privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, winCh <-chan ssh.Window) bool {
|
||||
discardWindowChanges(winCh)
|
||||
|
||||
localUser := privilegeResult.User
|
||||
if localUser == nil {
|
||||
logger.Errorf("no user in privilege result")
|
||||
@@ -406,3 +408,18 @@ func (s *Server) executeCommandWithPty(logger *log.Entry, session ssh.Session, _
|
||||
logger.Debug("ConPty execution completed")
|
||||
return true
|
||||
}
|
||||
|
||||
// discardWindowChanges drains the session's window-change channel. The ConPty is sized
|
||||
// once at start, and an undrained channel blocks the session's request loop on the
|
||||
// next resize.
|
||||
func discardWindowChanges(winCh <-chan ssh.Window) {
|
||||
if winCh == nil {
|
||||
return
|
||||
}
|
||||
go func() {
|
||||
for range winCh {
|
||||
// The ConPty keeps the size it was started with.
|
||||
continue
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
@@ -58,6 +58,10 @@ const (
|
||||
// the forwarded destination before rejecting the SSH channel.
|
||||
const directTCPIPDialTimeout = 30 * time.Second
|
||||
|
||||
// defaultHandshakeTimeout bounds how long a connection may take to complete the SSH
|
||||
// handshake and authentication before it is dropped.
|
||||
const defaultHandshakeTimeout = 60 * time.Second
|
||||
|
||||
var (
|
||||
ErrPrivilegedUserDisabled = errors.New(msgPrivilegedUserDisabled)
|
||||
ErrUserNotFound = errors.New("user not found")
|
||||
@@ -165,6 +169,7 @@ type Server struct {
|
||||
allowRemotePortForwarding bool
|
||||
allowRootLogin bool
|
||||
allowSFTP bool
|
||||
handshakeTimeout time.Duration
|
||||
jwtEnabled bool
|
||||
|
||||
netstackNet *wgbackend.NetstackNet
|
||||
@@ -227,6 +232,7 @@ func New(config *Config) *Server {
|
||||
jwtEnabled: config.JWT != nil,
|
||||
jwtConfig: config.JWT,
|
||||
authorizer: sshauth.NewAuthorizer(),
|
||||
handshakeTimeout: defaultHandshakeTimeout,
|
||||
}
|
||||
|
||||
if config.Auth != nil {
|
||||
@@ -884,15 +890,16 @@ func (s *Server) createSSHServer(addr net.Addr) (*ssh.Server, error) {
|
||||
},
|
||||
HostSigners: []ssh.Signer{},
|
||||
ChannelHandlers: map[string]ssh.ChannelHandler{
|
||||
"session": ssh.DefaultSessionHandler,
|
||||
"session": s.sessionChannelHandler,
|
||||
"direct-tcpip": s.directTCPIPHandler,
|
||||
},
|
||||
RequestHandlers: map[string]ssh.RequestHandler{
|
||||
"tcpip-forward": s.tcpipForwardHandler,
|
||||
"cancel-tcpip-forward": s.cancelTcpipForwardHandler,
|
||||
},
|
||||
ConnCallback: s.connectionValidator,
|
||||
Version: serverVersion,
|
||||
ConnCallback: s.connectionValidator,
|
||||
HandshakeTimeout: s.handshakeTimeout,
|
||||
Version: serverVersion,
|
||||
}
|
||||
|
||||
if s.jwtEnabled {
|
||||
@@ -934,6 +941,14 @@ func (s *Server) removeRemoteForwardListener(key forwardKey) bool {
|
||||
func (s *Server) directTCPIPHandler(srv *ssh.Server, conn *cryptossh.ServerConn, newChan cryptossh.NewChannel, ctx ssh.Context) {
|
||||
logger := s.getRequestLogger(ctx)
|
||||
|
||||
release, ok := acquireConnSlot(ctx, directTCPIPCountKey{}, maxDirectTCPIPPerConn)
|
||||
if !ok {
|
||||
logger.Warnf("local port forwarding denied: too many forwarded connections")
|
||||
_ = newChan.Reject(cryptossh.ResourceShortage, "too many forwarded connections")
|
||||
return
|
||||
}
|
||||
defer release()
|
||||
|
||||
var payload struct {
|
||||
Host string
|
||||
Port uint32
|
||||
|
||||
@@ -0,0 +1,340 @@
|
||||
//go:build !js
|
||||
|
||||
package server
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/binary"
|
||||
"errors"
|
||||
"io"
|
||||
"net"
|
||||
"os/user"
|
||||
"runtime"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
cryptossh "golang.org/x/crypto/ssh"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
nbssh "github.com/netbirdio/netbird/client/ssh"
|
||||
)
|
||||
|
||||
func dialNoAuthServer(t *testing.T, addr string, hostKey []byte) *cryptossh.Client {
|
||||
t.Helper()
|
||||
|
||||
clientKey, err := nbssh.GeneratePrivateKey(nbssh.ED25519)
|
||||
require.NoError(t, err)
|
||||
signer, err := cryptossh.ParsePrivateKey(clientKey)
|
||||
require.NoError(t, err)
|
||||
hostSigner, err := cryptossh.ParsePrivateKey(hostKey)
|
||||
require.NoError(t, err)
|
||||
current, err := user.Current()
|
||||
require.NoError(t, err)
|
||||
|
||||
client, err := cryptossh.Dial("tcp", addr, &cryptossh.ClientConfig{
|
||||
User: current.Username,
|
||||
Auth: []cryptossh.AuthMethod{cryptossh.PublicKeys(signer)},
|
||||
HostKeyCallback: cryptossh.FixedHostKey(hostSigner.PublicKey()),
|
||||
Timeout: 3 * time.Second,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = client.Close() })
|
||||
return client
|
||||
}
|
||||
|
||||
func startNoAuthServer(t *testing.T) (string, []byte) {
|
||||
t.Helper()
|
||||
|
||||
hostKey, err := nbssh.GeneratePrivateKey(nbssh.ED25519)
|
||||
require.NoError(t, err)
|
||||
srv := New(&Config{HostKeyPEM: hostKey})
|
||||
srv.SetAllowRootLogin(true)
|
||||
addr := StartTestServer(t, srv)
|
||||
t.Cleanup(func() { require.NoError(t, srv.Stop()) })
|
||||
return addr, hostKey
|
||||
}
|
||||
|
||||
// ptyReqPayload builds a pty-req payload whose term string claims the given length.
|
||||
func ptyReqPayload(termLen uint32, term string) []byte {
|
||||
buf := make([]byte, 4, 64)
|
||||
binary.BigEndian.PutUint32(buf, termLen)
|
||||
buf = append(buf, term...)
|
||||
buf = append(buf, make([]byte, 16)...)
|
||||
return buf
|
||||
}
|
||||
|
||||
// A pty-req with a term length close to the uint32 maximum must be refused and must
|
||||
// not take the daemon down.
|
||||
func TestSession_MalformedPtyReqDoesNotCrashServer(t *testing.T) {
|
||||
addr, hostKey := startNoAuthServer(t)
|
||||
client := dialNoAuthServer(t, addr, hostKey)
|
||||
|
||||
session, err := client.NewSession()
|
||||
require.NoError(t, err)
|
||||
|
||||
ok, err := session.SendRequest("pty-req", true, ptyReqPayload(0xFFFFFFFF, "xterm"))
|
||||
require.NoError(t, err)
|
||||
assert.False(t, ok, "malformed pty-req should be rejected")
|
||||
_ = session.Close()
|
||||
|
||||
// The server must still serve a fresh connection.
|
||||
again := dialNoAuthServer(t, addr, hostKey)
|
||||
s2, err := again.NewSession()
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, s2.Close())
|
||||
}
|
||||
|
||||
// Env requests past the per-session cap are refused in order, and the ones before it
|
||||
// are still accepted.
|
||||
func TestSession_EnvRequestsAreCapped(t *testing.T) {
|
||||
addr, hostKey := startNoAuthServer(t)
|
||||
client := dialNoAuthServer(t, addr, hostKey)
|
||||
|
||||
session, err := client.NewSession()
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = session.Close() })
|
||||
|
||||
payload := cryptossh.Marshal(struct{ Key, Value string }{"FOO", "bar"})
|
||||
accepted := 0
|
||||
for i := 0; i < maxEnvRequests+50; i++ {
|
||||
ok, err := session.SendRequest("env", true, payload)
|
||||
require.NoError(t, err)
|
||||
if ok {
|
||||
accepted++
|
||||
}
|
||||
}
|
||||
assert.Equal(t, maxEnvRequests, accepted, "only the first maxEnvRequests env requests should be accepted")
|
||||
|
||||
oversized := cryptossh.Marshal(struct{ Key, Value string }{"BIG", string(make([]byte, maxEnvPayload))})
|
||||
fresh, err := client.NewSession()
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = fresh.Close() })
|
||||
ok, err := fresh.SendRequest("env", true, oversized)
|
||||
require.NoError(t, err)
|
||||
assert.False(t, ok, "an env request over the payload limit should be refused")
|
||||
}
|
||||
|
||||
// A flood of window-change requests before the command starts must not stall the
|
||||
// session's request loop: the next request still gets its reply.
|
||||
func TestSession_WindowChangeFloodDoesNotStallRequests(t *testing.T) {
|
||||
addr, hostKey := startNoAuthServer(t)
|
||||
client := dialNoAuthServer(t, addr, hostKey)
|
||||
|
||||
session, err := client.NewSession()
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = session.Close() })
|
||||
|
||||
require.NoError(t, session.RequestPty("xterm", 24, 80, cryptossh.TerminalModes{}))
|
||||
|
||||
winch := cryptossh.Marshal(struct{ Columns, Rows, Width, Height uint32 }{100, 40, 0, 0})
|
||||
for i := 0; i < 500; i++ {
|
||||
_, err := session.SendRequest("window-change", false, winch)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
done := make(chan bool, 1)
|
||||
go func() {
|
||||
ok, err := session.SendRequest("env", true, cryptossh.Marshal(struct{ Key, Value string }{"A", "b"}))
|
||||
done <- err == nil && ok
|
||||
}()
|
||||
select {
|
||||
case ok := <-done:
|
||||
assert.True(t, ok, "env request after the flood should be accepted")
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("session request loop stalled behind window-change requests")
|
||||
}
|
||||
}
|
||||
|
||||
func TestSession_PerConnectionSessionLimit(t *testing.T) {
|
||||
addr, hostKey := startNoAuthServer(t)
|
||||
client := dialNoAuthServer(t, addr, hostKey)
|
||||
|
||||
for i := 0; i < maxSessionsPerConn; i++ {
|
||||
s, err := client.NewSession()
|
||||
require.NoError(t, err, "session %d should open", i)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
}
|
||||
|
||||
_, err := client.NewSession()
|
||||
assert.Error(t, err, "session beyond the limit should be refused")
|
||||
|
||||
// The limit is per connection: another connection is unaffected.
|
||||
other := dialNoAuthServer(t, addr, hostKey)
|
||||
s, err := other.NewSession()
|
||||
require.NoError(t, err, "a second connection should still open sessions")
|
||||
require.NoError(t, s.Close())
|
||||
}
|
||||
|
||||
func TestValidPtyReqPayload(t *testing.T) {
|
||||
assert.True(t, validPtyReqPayload(ptyReqPayload(5, "xterm")))
|
||||
assert.False(t, validPtyReqPayload(nil), "empty payload")
|
||||
assert.False(t, validPtyReqPayload([]byte{0, 0, 0}), "short length prefix")
|
||||
assert.False(t, validPtyReqPayload(ptyReqPayload(0xFFFFFFFF, "xterm")), "length that wraps uint32")
|
||||
assert.False(t, validPtyReqPayload(ptyReqPayload(1000, "xterm")), "length beyond the payload")
|
||||
}
|
||||
|
||||
// A resize sent while the command runs reaches the pty.
|
||||
func TestSession_ResizeWhileRunningIsApplied(t *testing.T) {
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("stty is not available")
|
||||
}
|
||||
addr, hostKey := startNoAuthServer(t)
|
||||
client := dialNoAuthServer(t, addr, hostKey)
|
||||
|
||||
session, err := client.NewSession()
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = session.Close() })
|
||||
|
||||
var out bytes.Buffer
|
||||
session.Stdout = &out
|
||||
require.NoError(t, session.RequestPty("xterm", 24, 80, cryptossh.TerminalModes{}))
|
||||
require.NoError(t, session.Start("sleep 1; stty size"))
|
||||
|
||||
// Let the handler read the initial window before resizing.
|
||||
time.Sleep(300 * time.Millisecond)
|
||||
require.NoError(t, session.WindowChange(50, 132))
|
||||
require.NoError(t, session.Wait())
|
||||
// Only the first line is the resize report. Coverage-instrumented binaries can append
|
||||
// runtime warnings to the same output.
|
||||
firstLine := strings.TrimSpace(strings.SplitN(out.String(), "\n", 2)[0])
|
||||
assert.Equal(t, "50 132", firstLine, "pty should have the resized dimensions")
|
||||
}
|
||||
|
||||
// Forwarded connections beyond the per-connection cap are refused, and a closed one
|
||||
// frees its slot.
|
||||
func TestDirectTCPIP_PerConnectionLimit(t *testing.T) {
|
||||
hostKey, err := nbssh.GeneratePrivateKey(nbssh.ED25519)
|
||||
require.NoError(t, err)
|
||||
srv := New(&Config{HostKeyPEM: hostKey})
|
||||
srv.SetAllowRootLogin(true)
|
||||
srv.SetAllowLocalPortForwarding(true)
|
||||
addr := StartTestServer(t, srv)
|
||||
t.Cleanup(func() { require.NoError(t, srv.Stop()) })
|
||||
|
||||
// Target that holds each connection open until the peer half-closes, then closes it
|
||||
// like a regular server.
|
||||
target, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = target.Close() })
|
||||
go func() {
|
||||
for {
|
||||
c, err := target.Accept()
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
go func() {
|
||||
_, _ = io.Copy(io.Discard, c)
|
||||
_ = c.Close()
|
||||
}()
|
||||
}
|
||||
}()
|
||||
|
||||
client := dialNoAuthServer(t, addr, hostKey)
|
||||
|
||||
conns := make([]net.Conn, 0, maxDirectTCPIPPerConn)
|
||||
for i := 0; i < maxDirectTCPIPPerConn; i++ {
|
||||
c, err := client.Dial("tcp", target.Addr().String())
|
||||
require.NoError(t, err, "forward %d should open", i)
|
||||
conns = append(conns, c)
|
||||
}
|
||||
|
||||
_, err = client.Dial("tcp", target.Addr().String())
|
||||
assert.Error(t, err, "forward beyond the limit should be refused")
|
||||
|
||||
// The limit is per connection: another connection is unaffected.
|
||||
other := dialNoAuthServer(t, addr, hostKey)
|
||||
oc, err := other.Dial("tcp", target.Addr().String())
|
||||
require.NoError(t, err, "a second connection should still open forwards")
|
||||
require.NoError(t, oc.Close())
|
||||
|
||||
require.NoError(t, conns[0].Close())
|
||||
assert.Eventually(t, func() bool {
|
||||
c, err := client.Dial("tcp", target.Addr().String())
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
_ = c.Close()
|
||||
return true
|
||||
}, 3*time.Second, 50*time.Millisecond, "a closed forward should free its slot")
|
||||
|
||||
for _, c := range conns[1:] {
|
||||
_ = c.Close()
|
||||
}
|
||||
}
|
||||
|
||||
// A connection that never completes the SSH handshake is dropped, and one that does
|
||||
// complete it is not subject to the handshake deadline afterwards.
|
||||
func TestHandshakeTimeout(t *testing.T) {
|
||||
hostKey, err := nbssh.GeneratePrivateKey(nbssh.ED25519)
|
||||
require.NoError(t, err)
|
||||
srv := New(&Config{HostKeyPEM: hostKey})
|
||||
srv.handshakeTimeout = 500 * time.Millisecond
|
||||
addr := StartTestServer(t, srv)
|
||||
t.Cleanup(func() { require.NoError(t, srv.Stop()) })
|
||||
|
||||
stalled, err := net.Dial("tcp", addr)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = stalled.Close() })
|
||||
|
||||
// The server sends its version line first; after that it waits for ours.
|
||||
require.NoError(t, stalled.SetReadDeadline(time.Now().Add(5*time.Second)))
|
||||
_, err = io.Copy(io.Discard, stalled)
|
||||
var nerr net.Error
|
||||
assert.False(t, errors.As(err, &nerr) && nerr.Timeout(), "server should close the stalled connection, not leave it open")
|
||||
|
||||
client := dialNoAuthServer(t, addr, hostKey)
|
||||
time.Sleep(time.Second)
|
||||
session, err := client.NewSession()
|
||||
require.NoError(t, err, "an established connection must outlive the handshake deadline")
|
||||
require.NoError(t, session.Close())
|
||||
}
|
||||
|
||||
// Refused requests are forwarded to the session loop under a type it answers with a
|
||||
// failure, so every reply is sent by the loop in request order.
|
||||
func TestFilterSessionRequests_RefusedRequestsStayInOrder(t *testing.T) {
|
||||
in := make(chan *cryptossh.Request)
|
||||
out := filterSessionRequests(nil, in)
|
||||
|
||||
validWinch := cryptossh.Marshal(struct{ Columns, Rows, Width, Height uint32 }{100, 40, 0, 0})
|
||||
reqs := []*cryptossh.Request{
|
||||
{Type: "pty-req", Payload: ptyReqPayload(0xFFFFFFFF, "xterm")},
|
||||
{Type: "pty-req", Payload: ptyReqPayload(5, "xterm")},
|
||||
{Type: "window-change", Payload: validWinch},
|
||||
{Type: "shell"},
|
||||
{Type: "window-change", Payload: validWinch},
|
||||
{Type: "window-change", Payload: validWinch},
|
||||
}
|
||||
want := []string{
|
||||
refusedRequestType, // malformed pty-req
|
||||
"pty-req",
|
||||
refusedRequestType, // resize before the command starts
|
||||
"shell",
|
||||
refusedRequestType, // older of two queued resizes
|
||||
"window-change",
|
||||
}
|
||||
|
||||
// Hold all requests in the filter's queue before the loop consumes any, so the
|
||||
// coalescing of the two trailing resizes is exercised.
|
||||
for _, r := range reqs {
|
||||
in <- r
|
||||
}
|
||||
for i, w := range want {
|
||||
select {
|
||||
case got := <-out:
|
||||
assert.Equal(t, w, got.Type, "request %d", i)
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatalf("request %d not delivered", i)
|
||||
}
|
||||
}
|
||||
|
||||
close(in)
|
||||
select {
|
||||
case _, ok := <-out:
|
||||
assert.False(t, ok, "filter should close its output when the input closes")
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("filter did not close its output")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,246 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"encoding/binary"
|
||||
"math"
|
||||
"sync/atomic"
|
||||
|
||||
"github.com/gliderlabs/ssh"
|
||||
log "github.com/sirupsen/logrus"
|
||||
cryptossh "golang.org/x/crypto/ssh"
|
||||
)
|
||||
|
||||
const (
|
||||
// maxSessionsPerConn bounds concurrent session channels on one SSH connection.
|
||||
maxSessionsPerConn = 64
|
||||
|
||||
// maxDirectTCPIPPerConn bounds concurrent direct-tcpip channels, dialing or
|
||||
// relaying, on one SSH connection.
|
||||
maxDirectTCPIPPerConn = 512
|
||||
|
||||
// maxEnvRequests and maxEnvPayload bound the environment a single session can
|
||||
// accumulate before its command starts.
|
||||
maxEnvRequests = 128
|
||||
maxEnvPayload = 8 * 1024
|
||||
|
||||
// maxQueuedRequests bounds the requests held for a session whose request loop is
|
||||
// not consuming them. A client that exceeds it has its channel closed.
|
||||
maxQueuedRequests = 256
|
||||
)
|
||||
|
||||
type (
|
||||
sessionCountKey struct{}
|
||||
directTCPIPCountKey struct{}
|
||||
)
|
||||
|
||||
// sessionChannelHandler serves "session" channels like ssh.DefaultSessionHandler, with
|
||||
// the per-connection session count capped, the request stream bounded, and any panic
|
||||
// from the session request loop contained to its channel.
|
||||
func (s *Server) sessionChannelHandler(srv *ssh.Server, conn *cryptossh.ServerConn, newChan cryptossh.NewChannel, ctx ssh.Context) {
|
||||
release, ok := acquireConnSlot(ctx, sessionCountKey{}, maxSessionsPerConn)
|
||||
if !ok {
|
||||
if err := newChan.Reject(cryptossh.ResourceShortage, "too many sessions"); err != nil {
|
||||
log.Debugf("reject session channel: %v", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
defer release()
|
||||
|
||||
guarded := &guardedSessionChannel{NewChannel: newChan}
|
||||
defer func() {
|
||||
r := recover()
|
||||
if r == nil {
|
||||
return
|
||||
}
|
||||
log.Errorf("SSH session request loop panicked from %s: %v", ctx.RemoteAddr(), r)
|
||||
guarded.closeChannel()
|
||||
}()
|
||||
|
||||
ssh.DefaultSessionHandler(srv, conn, guarded, ctx)
|
||||
}
|
||||
|
||||
// acquireConnSlot takes one of limit slots counted under key for the connection that
|
||||
// ctx belongs to. The returned func releases the slot.
|
||||
func acquireConnSlot(ctx ssh.Context, key any, limit int32) (func(), bool) {
|
||||
counter := connCounter(ctx, key)
|
||||
if counter.Add(1) > limit {
|
||||
counter.Add(-1)
|
||||
return nil, false
|
||||
}
|
||||
return func() { counter.Add(-1) }, true
|
||||
}
|
||||
|
||||
func connCounter(ctx ssh.Context, key any) *atomic.Int32 {
|
||||
ctx.Lock()
|
||||
defer ctx.Unlock()
|
||||
|
||||
if c, ok := ctx.Value(key).(*atomic.Int32); ok {
|
||||
return c
|
||||
}
|
||||
c := new(atomic.Int32)
|
||||
ctx.SetValue(key, c)
|
||||
return c
|
||||
}
|
||||
|
||||
// guardedSessionChannel hands the session loop a filtered request stream.
|
||||
type guardedSessionChannel struct {
|
||||
cryptossh.NewChannel
|
||||
ch atomic.Pointer[cryptossh.Channel]
|
||||
}
|
||||
|
||||
// Accept accepts the channel and returns its requests after filtering.
|
||||
func (g *guardedSessionChannel) Accept() (cryptossh.Channel, <-chan *cryptossh.Request, error) {
|
||||
ch, reqs, err := g.NewChannel.Accept()
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
g.ch.Store(&ch)
|
||||
return ch, filterSessionRequests(ch, reqs), nil
|
||||
}
|
||||
|
||||
func (g *guardedSessionChannel) closeChannel() {
|
||||
if ch := g.ch.Load(); ch != nil {
|
||||
_ = (*ch).Close()
|
||||
}
|
||||
}
|
||||
|
||||
// refusedRequestType is the type given to a request the filter refuses. The session
|
||||
// loop answers unknown request types with a failure reply, so a refused request is
|
||||
// answered by the loop itself and in its position among the replies, which the
|
||||
// protocol requires since replies carry no request identifier.
|
||||
const refusedRequestType = "refused@netbird.io"
|
||||
|
||||
// requestFilter passes session channel requests to the session loop while keeping
|
||||
// what the loop can accumulate or block on bounded:
|
||||
// - a malformed pty-req is refused instead of reaching the loop's parser
|
||||
// - env requests beyond maxEnvRequests, or with an oversized payload, are refused
|
||||
// - window-change requests are refused until the command starts and coalesced
|
||||
// after, as the loop blocks sending on a one-slot channel that only the command
|
||||
// handler drains
|
||||
// - the queue behind a stalled loop is capped, after which the channel is closed
|
||||
type requestFilter struct {
|
||||
ch cryptossh.Channel
|
||||
queue []*cryptossh.Request
|
||||
lastWinch *cryptossh.Request
|
||||
started bool
|
||||
envCount int
|
||||
}
|
||||
|
||||
// filterSessionRequests returns the requests of in as the session loop should see them.
|
||||
func filterSessionRequests(ch cryptossh.Channel, in <-chan *cryptossh.Request) <-chan *cryptossh.Request {
|
||||
out := make(chan *cryptossh.Request)
|
||||
f := &requestFilter{ch: ch}
|
||||
go func() {
|
||||
defer close(out)
|
||||
f.run(in, out)
|
||||
}()
|
||||
return out
|
||||
}
|
||||
|
||||
func (f *requestFilter) run(in <-chan *cryptossh.Request, out chan<- *cryptossh.Request) {
|
||||
for {
|
||||
var (
|
||||
sendCh chan<- *cryptossh.Request
|
||||
next *cryptossh.Request
|
||||
)
|
||||
if len(f.queue) > 0 {
|
||||
sendCh = out
|
||||
next = f.queue[0]
|
||||
}
|
||||
|
||||
select {
|
||||
case req, ok := <-in:
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
if !f.enqueue(req) {
|
||||
return
|
||||
}
|
||||
case sendCh <- next:
|
||||
f.dropHead()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (f *requestFilter) dropHead() {
|
||||
if f.queue[0] == f.lastWinch {
|
||||
f.lastWinch = nil
|
||||
}
|
||||
f.queue = f.queue[1:]
|
||||
}
|
||||
|
||||
// enqueue classifies req and queues it. It returns false after closing the channel
|
||||
// because the queue is full.
|
||||
func (f *requestFilter) enqueue(req *cryptossh.Request) bool {
|
||||
if len(f.queue) >= maxQueuedRequests {
|
||||
log.Warnf("SSH session request queue full, closing channel")
|
||||
_ = f.ch.Close()
|
||||
return false
|
||||
}
|
||||
|
||||
switch req.Type {
|
||||
case "pty-req":
|
||||
if !validPtyReqPayload(req.Payload) {
|
||||
refuseRequest(req)
|
||||
}
|
||||
case "env":
|
||||
f.envCount++
|
||||
if f.envCount > maxEnvRequests || len(req.Payload) > maxEnvPayload {
|
||||
refuseRequest(req)
|
||||
}
|
||||
case "shell", "exec", "subsystem":
|
||||
f.started = true
|
||||
case "window-change":
|
||||
f.coalesceWindowChange(req)
|
||||
}
|
||||
f.queue = append(f.queue, req)
|
||||
return true
|
||||
}
|
||||
|
||||
func (f *requestFilter) coalesceWindowChange(req *cryptossh.Request) {
|
||||
if !f.started || !validWinchPayload(req.Payload) {
|
||||
refuseRequest(req)
|
||||
return
|
||||
}
|
||||
if f.lastWinch != nil {
|
||||
refuseRequest(f.lastWinch)
|
||||
}
|
||||
f.lastWinch = req
|
||||
}
|
||||
|
||||
// validWinchPayload reports whether p parses as a window-change payload whose columns and
|
||||
// rows are non-zero and fit the uint16 the PTY takes. A malformed or unusable replacement
|
||||
// must not displace a valid queued resize.
|
||||
func validWinchPayload(p []byte) bool {
|
||||
var win struct{ Columns, Rows, Width, Height uint32 }
|
||||
if err := cryptossh.Unmarshal(p, &win); err != nil {
|
||||
return false
|
||||
}
|
||||
return usablePtyDimension(win.Columns) && usablePtyDimension(win.Rows)
|
||||
}
|
||||
|
||||
func usablePtyDimension(v uint32) bool {
|
||||
return v > 0 && v <= math.MaxUint16
|
||||
}
|
||||
|
||||
func refuseRequest(req *cryptossh.Request) {
|
||||
req.Type = refusedRequestType
|
||||
req.Payload = nil
|
||||
}
|
||||
|
||||
// validPtyReqPayload reports whether p holds a term string followed by the two
|
||||
// uint32 window dimensions, without overflowing on a hostile string length. The
|
||||
// dimensions must fit the Windows int16 ConPty coordinate, which is narrower than the
|
||||
// Unix uint16 winsize; zero is allowed and defaulted by the Unix path.
|
||||
func validPtyReqPayload(p []byte) bool {
|
||||
if len(p) < 4 {
|
||||
return false
|
||||
}
|
||||
termLen := uint64(binary.BigEndian.Uint32(p))
|
||||
if uint64(len(p)) < 4+termLen+8 {
|
||||
return false
|
||||
}
|
||||
dims := p[4+termLen:]
|
||||
return binary.BigEndian.Uint32(dims) <= math.MaxInt16 &&
|
||||
binary.BigEndian.Uint32(dims[4:]) <= math.MaxInt16
|
||||
}
|
||||
@@ -0,0 +1,82 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
cryptossh "golang.org/x/crypto/ssh"
|
||||
)
|
||||
|
||||
// A malformed window-change arriving after a valid one must not cost the valid resize:
|
||||
// the valid request stays queued for the PTY, and only the malformed one is refused.
|
||||
func TestRequestFilter_MalformedWinchKeepsQueuedValidResize(t *testing.T) {
|
||||
f := &requestFilter{}
|
||||
require.True(t, f.enqueue(&cryptossh.Request{Type: "shell"}))
|
||||
|
||||
valid := &cryptossh.Request{Type: "window-change", Payload: cryptossh.Marshal(struct{ Columns, Rows, Width, Height uint32 }{100, 40, 0, 0})}
|
||||
require.True(t, f.enqueue(valid))
|
||||
|
||||
malformed := &cryptossh.Request{Type: "window-change", Payload: []byte{1, 2, 3}}
|
||||
require.True(t, f.enqueue(malformed))
|
||||
|
||||
assert.Equal(t, "window-change", valid.Type, "valid resize must still reach the PTY handler")
|
||||
assert.Equal(t, refusedRequestType, malformed.Type, "malformed resize must be refused")
|
||||
}
|
||||
|
||||
// A zero-row or zero-column resize parses but would size the PTY to nothing, so it must
|
||||
// not displace a valid queued resize either.
|
||||
func TestRequestFilter_ZeroSizeWinchKeepsQueuedValidResize(t *testing.T) {
|
||||
f := &requestFilter{}
|
||||
require.True(t, f.enqueue(&cryptossh.Request{Type: "shell"}))
|
||||
|
||||
valid := &cryptossh.Request{Type: "window-change", Payload: cryptossh.Marshal(struct{ Columns, Rows, Width, Height uint32 }{100, 40, 0, 0})}
|
||||
require.True(t, f.enqueue(valid))
|
||||
|
||||
zeroRows := &cryptossh.Request{Type: "window-change", Payload: cryptossh.Marshal(struct{ Columns, Rows, Width, Height uint32 }{100, 0, 0, 0})}
|
||||
require.True(t, f.enqueue(zeroRows))
|
||||
|
||||
assert.Equal(t, "window-change", valid.Type, "valid resize must still reach the PTY handler")
|
||||
assert.Equal(t, refusedRequestType, zeroRows.Type, "zero-row resize must be refused")
|
||||
}
|
||||
|
||||
// The initial pty-req size must fit both the Unix uint16 winsize and the Windows int16
|
||||
// ConPty coordinate, so larger values are refused instead of wrapping. Zero stays
|
||||
// accepted because the Unix path defaults it to 80x24.
|
||||
func TestValidPtyReqPayload_BoundsInitialSize(t *testing.T) {
|
||||
ptyReq := func(columns, rows uint32) []byte {
|
||||
return cryptossh.Marshal(struct {
|
||||
Term string
|
||||
Columns, Rows, Width, Height uint32
|
||||
Modes string
|
||||
}{"xterm", columns, rows, 0, 0, ""})
|
||||
}
|
||||
|
||||
assert.True(t, validPtyReqPayload(ptyReq(80, 24)), "ordinary size")
|
||||
assert.True(t, validPtyReqPayload(ptyReq(0, 0)), "zero size is defaulted later")
|
||||
assert.True(t, validPtyReqPayload(ptyReq(32767, 32767)), "largest size both platforms take")
|
||||
assert.False(t, validPtyReqPayload(ptyReq(32768, 24)), "columns past the Windows int16 range")
|
||||
assert.False(t, validPtyReqPayload(ptyReq(80, 32768)), "rows past the Windows int16 range")
|
||||
assert.False(t, validPtyReqPayload(ptyReq(65536, 24)), "columns that wrap to zero as uint16")
|
||||
}
|
||||
|
||||
// Sizes that are zero or do not fit the PTY's uint16 dimensions are refused, so they
|
||||
// cannot replace a valid queued resize.
|
||||
func TestValidWinchPayload_RejectsUnusableSizes(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
columns, rows uint32
|
||||
}{
|
||||
{name: "zero columns", columns: 0, rows: 40},
|
||||
{name: "zero rows", columns: 100, rows: 0},
|
||||
{name: "columns wrap to zero as uint16", columns: 65536, rows: 40},
|
||||
{name: "rows wrap to zero as uint16", columns: 100, rows: 65536},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
payload := cryptossh.Marshal(struct{ Columns, Rows, Width, Height uint32 }{tc.columns, tc.rows, 0, 0})
|
||||
assert.False(t, validWinchPayload(payload))
|
||||
})
|
||||
}
|
||||
assert.True(t, validWinchPayload(cryptossh.Marshal(struct{ Columns, Rows, Width, Height uint32 }{65535, 65535, 0, 0})), "largest uint16 size is usable")
|
||||
}
|
||||
@@ -54,7 +54,7 @@ require (
|
||||
github.com/eko/gocache/store/go_cache/v4 v4.2.2
|
||||
github.com/eko/gocache/store/redis/v4 v4.2.2
|
||||
github.com/fsnotify/fsnotify v1.9.0
|
||||
github.com/gliderlabs/ssh v0.3.8
|
||||
github.com/gliderlabs/ssh v0.3.9-0.20250127223818-909fa952d4e4
|
||||
github.com/go-jose/go-jose/v4 v4.1.4
|
||||
github.com/go-ole/go-ole v1.3.0
|
||||
github.com/gobwas/ws v1.4.0
|
||||
|
||||
@@ -200,8 +200,8 @@ github.com/fxamacker/cbor/v2 v2.9.1 h1:2rWm8B193Ll4VdjsJY28jxs70IdDsHRWgQYAI80+r
|
||||
github.com/fxamacker/cbor/v2 v2.9.1/go.mod h1:vM4b+DJCtHn+zz7h3FFp/hDAI9WNWCsZj23V5ytsSxQ=
|
||||
github.com/gin-contrib/sse v0.1.0/go.mod h1:RHrZQHXnP2xjPF+u1gW/2HnVO7nvIa9PG3Gm+fLHvGI=
|
||||
github.com/gin-gonic/gin v1.5.0/go.mod h1:Nd6IXA8m5kNZdNEHMBd93KT+mdY3+bewLgRvmCsR2Do=
|
||||
github.com/gliderlabs/ssh v0.3.8 h1:a4YXD1V7xMF9g5nTkdfnja3Sxy1PVDCj1Zg4Wb8vY6c=
|
||||
github.com/gliderlabs/ssh v0.3.8/go.mod h1:xYoytBv1sV0aL3CavoDuJIQNURXkkfPA/wxQ1pL1fAU=
|
||||
github.com/gliderlabs/ssh v0.3.9-0.20250127223818-909fa952d4e4 h1:iLDHIkpEUuk277c53BF7v9yCAzgQHOcaY8tOu5ZX9Ws=
|
||||
github.com/gliderlabs/ssh v0.3.9-0.20250127223818-909fa952d4e4/go.mod h1:1nh+SYql7iCMwTrbe9K8l/lah6xcgJTH7W4ZlVoq22c=
|
||||
github.com/go-asn1-ber/asn1-ber v1.5.8-0.20250403174932-29230038a667 h1:BP4M0CvQ4S3TGls2FvczZtj5Re/2ZzkV9VwqPHH/3Bo=
|
||||
github.com/go-asn1-ber/asn1-ber v1.5.8-0.20250403174932-29230038a667/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0=
|
||||
github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI=
|
||||
|
||||
Reference in New Issue
Block a user