Skip to content

Commit

Permalink
Adds per-node ability to disable ssh tcp forwarding
Browse files Browse the repository at this point in the history
Prior to this change, TCP forwarding over SSH could only be disallowed
by user-based rules, rather than by individual target nodes.

This change adds
 * the `allow_tcp_forwarding` key to the yaml SSH config block, with values
   compatable with the equivalent setting for OpenSSH `sshd`, i.e.
   "yes", "no", "all" and "local"
 * Plumbing to pipe the resulting config value through to the SSH server
 * A predicate check in the SSH server to [dis]allow port forwarding
   based on the setting.

See-Also: Issue #6783
  • Loading branch information
tcsc committed May 28, 2021
1 parent 492f4ec commit eaf67eb
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 1 deletion.
2 changes: 2 additions & 0 deletions lib/config/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,8 @@ func applySSHConfig(fc *FileConfig, cfg *service.Config) (err error) {
}
}

cfg.SSH.AllowTCPForwarding = fc.SSH.AllowTCPForwarding.AsForwardingMode()

return nil
}

Expand Down
39 changes: 39 additions & 0 deletions lib/config/fileconf.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"github.com/gravitational/teleport/lib/pam"
"github.com/gravitational/teleport/lib/service"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/srv/regular"
"github.com/gravitational/teleport/lib/utils"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -613,6 +614,40 @@ func (u *UniversalSecondFactor) Parse() (services.U2F, error) {
return res, nil
}

// AllowTCPForwarding describes the possible values of the
// `allow_tcp_forwarding` SSH settings key. Essentially a wrapper around
// regular.SSHPortForwardingMode to plug it into the YAML parser.
type AllowTCPForwarding regular.SSHPortForwardingMode

// UnmarshalYAML parses a YAML representation of a SSHPortForwardingMode,
// failing if the value cannot be converted.
func (mode *AllowTCPForwarding) UnmarshalYAML(unmarshal func(interface{}) error) error {
var text string
err := unmarshal(&text)
if err != nil {
return trace.Wrap(err)
}

var value regular.SSHPortForwardingMode
switch strings.ToLower(text) {
case "yes", "all":
value = regular.SSHPortForwardingModeAll
case "no":
value = regular.SSHPortForwardingModeNone
case "local":
value = regular.SSHPortForwardingModeLocal
default:
return trace.BadParameter("Invalid SSH port forwarding mode %q", text)
}

(*mode) = AllowTCPForwarding(value)
return nil
}

func (mode *AllowTCPForwarding) AsForwardingMode() regular.SSHPortForwardingMode {
return regular.SSHPortForwardingMode(*mode)
}

// SSH is 'ssh_service' section of the config file
type SSH struct {
Service `yaml:",inline"`
Expand All @@ -626,6 +661,10 @@ type SSH struct {

// BPF is used to configure BPF-based auditing for this node.
BPF *BPF `yaml:"enhanced_recording,omitempty"`

// AllowTCPForwarding configures the TCP port forwarding modes that the
// server is allowed to offer.
AllowTCPForwarding AllowTCPForwarding `yaml:"allow_tcp_forwarding,omitempty"`
}

// CommandLabel is `command` section of `ssh_service` in the config file
Expand Down
65 changes: 65 additions & 0 deletions lib/config/fileconf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/gravitational/teleport/lib/srv/regular"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v2"
)

func TestAuthenticationSection(t *testing.T) {
Expand Down Expand Up @@ -108,3 +110,66 @@ fake certificate
})
}
}

func TestSSHSection(t *testing.T) {
testCases := []struct {
desc string
yaml string
expectError require.ErrorAssertionFunc
expectedConfig SSH
}{
{
desc: "default",
yaml: "",
expectError: require.NoError,
expectedConfig: SSH{},
}, {
desc: "enabled",
yaml: "enabled: yes",
expectError: require.NoError,
expectedConfig: SSH{Service: Service{EnabledFlag: "yes"}},
}, {
desc: "Port forwarding is enabled (yes)",
yaml: "allow_tcp_forwarding: yes",
expectError: require.NoError,
expectedConfig: SSH{
AllowTCPForwarding: AllowTCPForwarding(regular.SSHPortForwardingModeAll),
},
}, {
desc: "Port forwarding is enabled (all)",
yaml: "allow_tcp_forwarding: all",
expectError: require.NoError,
expectedConfig: SSH{
AllowTCPForwarding: AllowTCPForwarding(regular.SSHPortForwardingModeAll),
},
}, {
desc: "Port forwarding is disabled",
yaml: "allow_tcp_forwarding: no",
expectError: require.NoError,
expectedConfig: SSH{
AllowTCPForwarding: AllowTCPForwarding(regular.SSHPortForwardingModeNone),
},
}, {
desc: "Port forwarding is local only",
yaml: "allow_tcp_forwarding: local",
expectError: require.NoError,
expectedConfig: SSH{
AllowTCPForwarding: AllowTCPForwarding(regular.SSHPortForwardingModeLocal),
},
}, {
desc: "Invalid port forwarding value is an error",
yaml: "allow_tcp_forwarding: banana",
expectError: require.Error,
expectedConfig: SSH{},
},
}

for _, testCase := range testCases {
t.Run(testCase.desc, func(t *testing.T) {
var sshConfig SSH
err := yaml.UnmarshalStrict([]byte(testCase.yaml), &sshConfig)
testCase.expectError(t, err)
require.Equal(t, testCase.expectedConfig, sshConfig)
})
}
}
5 changes: 5 additions & 0 deletions lib/service/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import (
"github.com/gravitational/teleport/lib/plugin"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/srv/app"
"github.com/gravitational/teleport/lib/srv/regular"
"github.com/gravitational/teleport/lib/sshca"
"github.com/gravitational/teleport/lib/tlsca"
"github.com/gravitational/teleport/lib/utils"
Expand Down Expand Up @@ -534,6 +535,10 @@ type SSHConfig struct {
//
// See github.com/gravitational/teleport/issues/4141 for details.
ProxyReverseTunnelFallbackAddr *utils.NetAddr

// AllowTCPForwarding describes what port forwarding mechanisms the SSH service
// is allowed to offer. Defaults to SSHPortForwardingModeAll
AllowTCPForwarding regular.SSHPortForwardingMode
}

// KubeConfig specifies configuration for kubernetes service
Expand Down
2 changes: 2 additions & 0 deletions lib/service/cfg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/fixtures"
"github.com/gravitational/teleport/lib/srv/app"
"github.com/gravitational/teleport/lib/srv/regular"
"github.com/gravitational/teleport/lib/utils"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -83,6 +84,7 @@ func TestDefaultConfig(t *testing.T) {
ssh := config.SSH
require.Equal(t, ssh.Limiter.MaxConnections, int64(defaults.LimiterMaxConnections))
require.Equal(t, ssh.Limiter.MaxNumberOfUsers, defaults.LimiterMaxConcurrentUsers)
require.Equal(t, ssh.AllowTCPForwarding, regular.SSHPortForwardingModeAll)

// proxy section
proxy := config.Proxy
Expand Down
1 change: 1 addition & 0 deletions lib/srv/forward/sshserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,7 @@ func (s *Server) handleChannel(ctx context.Context, nch ssh.NewChannel) {
}
return
}

ch, _, err := nch.Accept()
if err != nil {
s.log.Warnf("Unable to accept channel: %v", err)
Expand Down
61 changes: 60 additions & 1 deletion lib/srv/regular/sshserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,23 @@ import (
"github.com/sirupsen/logrus"
)

// SSHPortForwardingMode encodes a port forwarding mode.
type SSHPortForwardingMode int

const (
// SSHPortForwardingModeAll indicates both local and remote port forwarding
// is allowed
SSHPortForwardingModeAll SSHPortForwardingMode = iota

// SSHPortForwardingMode indicates that only local (i.e. forwarding
// connections from the client out to the Internet) is allowed.
SSHPortForwardingModeLocal

// SSHPortForwardingModeNone indicates that no port forwarding of any kind
// is allowed.
SSHPortForwardingModeNone
)

var (
log = logrus.WithFields(logrus.Fields{
trace.Component: teleport.ComponentNode,
Expand Down Expand Up @@ -169,6 +186,10 @@ type Server struct {

// wtmpPath is the path to the user accounting log.
wtmpPath string

// portForwardingMode is the TCP port forwarding mode allowed
// port by this server.
portForwardingMode SSHPortForwardingMode
}

// GetClock returns server clock implementation
Expand Down Expand Up @@ -499,6 +520,13 @@ func SetOnHeartbeat(fn func(error)) ServerOption {
}
}

func SetPortForwardingMode(mode SSHPortForwardingMode) ServerOption {
return func(s *Server) error {
s.portForwardingMode = mode
return nil
}
}

// New returns an unstarted server
func New(addr utils.NetAddr,
hostname string,
Expand Down Expand Up @@ -636,6 +664,9 @@ func New(addr utils.NetAddr,
return nil, trace.Wrap(err)
}
s.heartbeat = heartbeat

log.Warnf("******************** NEWSERVER @%p IN PROXY MODE: %v", s, s.proxyMode)

return s, nil
}

Expand Down Expand Up @@ -944,14 +975,18 @@ func (s *Server) HandleNewConn(ctx context.Context, ccx *sshutils.ConnectionCont

// HandleNewChan is called when new channel is opened
func (s *Server) HandleNewChan(ctx context.Context, ccx *sshutils.ConnectionContext, nch ssh.NewChannel) {
s.Logger.Warnf("******************** HANDLENEWCHAN (%v) @ %p->%p", s.portForwardingMode, s, s.srv)

identityContext, err := s.authHandlers.CreateIdentityContext(ccx.ServerConn)
if err != nil {
rejectChannel(nch, ssh.Prohibited, fmt.Sprintf("Unable to create identity from connection: %v", err))
return
}

channelType := nch.ChannelType()
s.Logger.Warnf("******************** ChanType %v", channelType)
if s.proxyMode {
s.Logger.Warnf("******************** IN Proxy Mode")
switch channelType {
// Channels of type "direct-tcpip", for proxies, it's equivalent
// of teleport proxy: subsystem
Expand Down Expand Up @@ -988,6 +1023,8 @@ func (s *Server) HandleNewChan(ctx context.Context, ccx *sshutils.ConnectionCont
}
}

s.Logger.Warnf("******************** NOT IN Proxy Mode")

switch channelType {
// Channels of type "session" handle requests that are involved in running
// commands on a server, subsystem requests, and agent forwarding.
Expand Down Expand Up @@ -1019,7 +1056,7 @@ func (s *Server) HandleNewChan(ctx context.Context, ccx *sshutils.ConnectionCont
Reason: events.SessionRejectedReasonMaxSessions,
Maximum: max,
}); err != nil {
log.WithError(err).Warn("Failed to emit sesion reject event.")
log.WithError(err).Warn("Failed to emit session reject event.")
}
rejectChannel(nch, ssh.Prohibited, fmt.Sprintf("too many session channels for user %q (max=%d)", identityContext.TeleportUser, max))
return
Expand Down Expand Up @@ -1061,6 +1098,22 @@ func (s *Server) HandleNewChan(ctx context.Context, ccx *sshutils.ConnectionCont
}
}

// NodeAllowsPortForward checks if this SSH service is allowed forward TCP
// connections for the client.
func (s *Server) NodeAllowsPortForward() bool {
// Because we do not support remote port forwarding (i.e. from the Internet
// back to the client), the values `SSHPortForwardingModeAll` and
// `SSHPortForwardingModeLocal` are effectively synonyms.
s.Logger.Warnf("************ Checking if node allows port forwarding: %v\n", s.portForwardingMode)
switch s.portForwardingMode {
case SSHPortForwardingModeAll, SSHPortForwardingModeLocal:
return true

default:
return false
}
}

// handleDirectTCPIPRequest handles port forwarding requests.
func (s *Server) handleDirectTCPIPRequest(ctx context.Context, ccx *sshutils.ConnectionContext, identityContext srv.IdentityContext, channel ssh.Channel, req *sshutils.DirectTCPIPReq) {
// Create context for this channel. This context will be closed when
Expand All @@ -1080,6 +1133,12 @@ func (s *Server) handleDirectTCPIPRequest(ctx context.Context, ccx *sshutils.Con

channel = scx.TrackActivity(channel)

// Check if this node allows port forwarding at all
if !s.NodeAllowsPortForward() {
writeStderr(channel, "Node does not allow port forwarding")
return
}

// Check if the role allows port forwarding for this user.
err = s.authHandlers.CheckPortForward(scx.DstAddr, scx)
if err != nil {
Expand Down

0 comments on commit eaf67eb

Please sign in to comment.