Skip to content

Commit

Permalink
fix direct dialing to deprecated unregistered SSH nodes (#31903)
Browse files Browse the repository at this point in the history
* fix direct dialing to deprecated unregistered SSH nodes

* revert changing reexec error message

* change the dummy server's hostname so the addr shows up in session recordings

* Add test

* make test pass

* address feedback

* return an error if direct dialing with OpenSSH 'ssh' is attempted without proxy rec mode

* fix import ordering

* fix integration test after adding error message

* make error msg when env isn't passed link to agentless docs

* fix unit test

---------

Co-authored-by: Anton Miniailo <[email protected]>
  • Loading branch information
capnspacehook and AntonAM authored Sep 18, 2023
1 parent 0473c5f commit c2207fa
Show file tree
Hide file tree
Showing 8 changed files with 237 additions and 45 deletions.
93 changes: 91 additions & 2 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1334,6 +1334,8 @@ func testIPPropagation(t *testing.T, suite *integrationTestSuite) {
tr := utils.NewTracer(utils.ThisFunction()).Start()
defer tr.Stop()

t.Setenv("TELEPORT_UNSTABLE_UNLISTED_AGENT_DIALING", "yes")

startNodes := func(t *testing.T, root, leaf *helpers.TeleInstance) {
rootNodes := []string{"root-one", "root-two"}
leafNodes := []string{"leaf-one", "leaf-two"}
Expand Down Expand Up @@ -1510,6 +1512,69 @@ func testIPPropagation(t *testing.T, suite *integrationTestSuite) {
require.Equal(t, local.get().String(), pingResp.RemoteAddr, "client IP:port that auth server sees doesn't match the real one")
}

testSSHUnregisteredNodeConnection := func(t *testing.T, instance *helpers.TeleInstance, clusterName string) {
sshListener, err := net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, sshListener.Close())
})

resultChan := make(chan bool)
// Start listening, emulating unregistered node.
go func() {
conn, err := sshListener.Accept()
if err != nil {
assert.Fail(t, err.Error())
return
}

buf := make([]byte, 3)
_, err = conn.Read(buf)
assert.NoError(t, err)

// On the received connection first bytes should be SSH prefix, not PROXY protocol
resultChan <- slices.Equal(buf, []byte("SSH"))
}()

nodeAddr := sshListener.Addr().String()
ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second)
t.Cleanup(cancelFunc)

nodeHost, nodePortStr, err := net.SplitHostPort(nodeAddr)
require.NoError(t, err)

nodePort, err := strconv.Atoi(nodePortStr)
require.NoError(t, err)

tc, err := instance.NewClient(helpers.ClientConfig{
Login: suite.Me.Username,
Cluster: clusterName,
Host: nodeHost,
Port: nodePort,
})
require.NoError(t, err)

clt, err := tc.ConnectToCluster(ctx)
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, clt.Close())
})

nodeDetails := client.NodeDetails{Addr: nodeAddr, Namespace: tc.Namespace, Cluster: tc.SiteName}
nodeClient, err := tc.ConnectToNode(ctx, clt, nodeDetails, tc.Config.HostLogin)
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, nodeClient.Close())
})

select {
case res := <-resultChan:
require.True(t, res, "Didn't receive SSH prefix as first bytes on the connection")
case <-time.After(time.Second):
require.Fail(t, "Timed out waiting for connection to the node")
}
}

testSSHAuthConnection := func(t *testing.T, instance *helpers.TeleInstance, clusterName string) {
ctx := context.Background()

Expand Down Expand Up @@ -1589,6 +1654,16 @@ func testIPPropagation(t *testing.T, suite *integrationTestSuite) {
})
}
})

_, root2, _ := createTrustedClusterPair(t, suite, startNodes, withProxyRecordingMode)
t.Run("We don't propagate IP to non Teleport nodes", func(t *testing.T) {
t.Run("connecting through root cluster", func(t *testing.T) {
testSSHUnregisteredNodeConnection(t, root2, "root-test")
})
t.Run("connecting through leaf cluster", func(t *testing.T) {
testSSHUnregisteredNodeConnection(t, root2, "leaf-test")
})
})
}

// verifySessionJoin covers SSH into shell and joining the same session from another client
Expand Down Expand Up @@ -7304,7 +7379,15 @@ outer:
t.FailNow()
}

func createTrustedClusterPair(t *testing.T, suite *integrationTestSuite, extraServices func(*testing.T, *helpers.TeleInstance, *helpers.TeleInstance)) (*client.TeleportClient, *helpers.TeleInstance, *helpers.TeleInstance) {
type serviceCfgOpt func(*servicecfg.Config)

func withProxyRecordingMode(cfg *servicecfg.Config) {
recCfg := types.DefaultSessionRecordingConfig()
recCfg.SetMode(types.RecordAtProxy)
cfg.Auth.SessionRecordingConfig = recCfg
}

func createTrustedClusterPair(t *testing.T, suite *integrationTestSuite, extraServices func(*testing.T, *helpers.TeleInstance, *helpers.TeleInstance), cfgOpts ...serviceCfgOpt) (*client.TeleportClient, *helpers.TeleInstance, *helpers.TeleInstance) {
ctx := context.Background()
username := suite.Me.Username
name := "test"
Expand Down Expand Up @@ -7355,6 +7438,11 @@ func createTrustedClusterPair(t *testing.T, suite *integrationTestSuite, extraSe
tconf.Proxy.DisableWebInterface = true
tconf.SSH.Enabled = false
tconf.CachePolicy.MaxRetryPeriod = time.Millisecond * 500

for _, opt := range cfgOpts {
opt(tconf)
}

return t, nil, tconf
}

Expand Down Expand Up @@ -7618,7 +7706,8 @@ func testListResourcesAcrossClusters(t *testing.T, suite *integrationTestSuite)

func testJoinOverReverseTunnelOnly(t *testing.T, suite *integrationTestSuite) {
for _, proxyProtocolMode := range []multiplexer.PROXYProtocolMode{
multiplexer.PROXYProtocolOn, multiplexer.PROXYProtocolOff, multiplexer.PROXYProtocolUnspecified} {
multiplexer.PROXYProtocolOn, multiplexer.PROXYProtocolOff, multiplexer.PROXYProtocolUnspecified,
} {
t.Run(fmt.Sprintf("proxy protocol mode: %v", proxyProtocolMode), func(t *testing.T) {
lib.SetInsecureDevMode(true)
t.Cleanup(func() { lib.SetInsecureDevMode(false) })
Expand Down
37 changes: 29 additions & 8 deletions lib/proxy/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ import (
"github.com/gravitational/teleport/lib/utils"
)

const errDirectDialing = `Direct dialing to nodes not found in the inventory is not supported.
If you want to connect to a node without installing Teleport on it, consider registering it with
your cluster with 'teleport join openssh'.
See https://goteleport.com/docs/ver/14.x/server-access/guides/openssh/ for more details.`

var (
// proxiedSessions counts successful connections to nodes
proxiedSessions = prometheus.NewGauge(
Expand Down Expand Up @@ -236,11 +242,12 @@ func (r *Router) DialHost(ctx context.Context, clientSrcAddr, clientDstAddr net.
principals := []string{host}

var (
isAgentlessNode bool
serverID string
serverAddr string
proxyIDs []string
sshSigner ssh.Signer
isAgentlessNode bool
isNotInventoryNode bool
serverID string
serverAddr string
proxyIDs []string
sshSigner ssh.Signer
)

if target != nil {
Expand Down Expand Up @@ -282,16 +289,29 @@ func (r *Router) DialHost(ctx context.Context, clientSrcAddr, clientDstAddr net.
}
}
}

} else {
if !r.permitUnlistedDialing {
return nil, trace.ConnectionProblem(errors.New("connection problem"), "direct dialing to nodes not found in inventory is not supported")
return nil, trace.ConnectionProblem(errors.New("connection problem"), errDirectDialing)
}

// Prepare a dummy server resource so this connection will not be
// treated like a connection to a Teleport node
isNotInventoryNode = true
isAgentlessNode = true
if port == "" || port == "0" {
port = strconv.Itoa(defaults.SSHServerListenPort)
}

serverAddr = net.JoinHostPort(host, port)
name := "unknown server " + serverAddr
target, err = types.NewServer(name, types.KindNode, types.ServerSpecV2{
Addr: serverAddr,
Hostname: host,
})
if err != nil {
return nil, trace.Wrap(err)
}
target.SetSubKind(types.SubKindOpenSSHNode)

r.log.Warnf("server lookup failed: using default=%v", serverAddr)
}

Expand All @@ -300,6 +320,7 @@ func (r *Router) DialHost(ctx context.Context, clientSrcAddr, clientDstAddr net.
To: &utils.NetAddr{AddrNetwork: "tcp", Addr: serverAddr},
OriginalClientDstAddr: clientDstAddr,
GetUserAgent: agentGetter,
IsNotInventoryNode: isNotInventoryNode,
IsAgentlessNode: isAgentlessNode,
AgentlessSigner: sshSigner,
Address: host,
Expand Down
33 changes: 4 additions & 29 deletions lib/reversetunnel/localsite.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (
"github.com/gravitational/teleport/api/utils/sshutils"
"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/integrations/awsoidc"
"github.com/gravitational/teleport/lib/multiplexer"
"github.com/gravitational/teleport/lib/observability/metrics"
"github.com/gravitational/teleport/lib/proxy/peer"
"github.com/gravitational/teleport/lib/reversetunnel/track"
Expand Down Expand Up @@ -239,30 +238,14 @@ func (s *localSite) DialAuthServer(params reversetunnelclient.DialParams) (net.C
return conn, nil
}

// shouldDialAndForward returns whether a connection should be proxied
// and forwarded or not.
func shouldDialAndForward(params reversetunnelclient.DialParams, recConfig types.SessionRecordingConfig) bool {
// connection is already being tunneled, do not forward
if params.FromPeerProxy {
return false
}
// the node is an agentless node, the connection must be forwarded
if params.TargetServer != nil && params.TargetServer.IsOpenSSHNode() {
return true
}
// proxy session recording mode is being used and an SSH session
// is being requested, the connection must be forwarded
if params.ConnType == types.NodeTunnel && services.IsRecordAtProxy(recConfig.GetMode()) {
return true
}
return false
}

func (s *localSite) Dial(params reversetunnelclient.DialParams) (net.Conn, error) {
recConfig, err := s.accessPoint.GetSessionRecordingConfig(s.srv.Context)
if err != nil {
return nil, trace.Wrap(err)
}
if err := checkNodeAndRecConfig(params, recConfig); err != nil {
return nil, trace.Wrap(err)
}

// If the proxy is in recording mode and a SSH connection is being
// requested or the target server is a registered OpenSSH node, build
Expand All @@ -275,14 +258,6 @@ func (s *localSite) Dial(params reversetunnelclient.DialParams) (net.Conn, error
return s.DialTCP(params)
}

func shouldSendSignedPROXYHeader(signer multiplexer.PROXYHeaderSigner, useTunnel, isAgentlessNode bool, srcAddr, dstAddr net.Addr) bool {
return !(signer == nil ||
useTunnel ||
isAgentlessNode ||
srcAddr == nil ||
dstAddr == nil)
}

func (s *localSite) maybeSendSignedPROXYHeader(params reversetunnelclient.DialParams, conn net.Conn, useTunnel bool) error {
if !shouldSendSignedPROXYHeader(s.srv.proxySigner, useTunnel, params.IsAgentlessNode, params.From, params.OriginalClientDstAddr) {
return nil
Expand Down Expand Up @@ -399,7 +374,7 @@ func (s *localSite) dialAndForward(params reversetunnelclient.DialParams) (_ net
serverConfig := forward.ServerConfig{
AuthClient: s.client,
UserAgent: userAgent,
IsAgentlessNode: params.IsAgentlessNode,
IsAgentlessNode: isAgentlessNode(params),
AgentlessSigner: params.AgentlessSigner,
TargetConn: targetConn,
SrcAddr: params.From,
Expand Down
5 changes: 4 additions & 1 deletion lib/reversetunnel/remotesite.go
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,9 @@ func (s *remoteSite) Dial(params reversetunnelclient.DialParams) (net.Conn, erro
if err != nil {
return nil, trace.Wrap(err)
}
if err := checkNodeAndRecConfig(params, recConfig); err != nil {
return nil, trace.Wrap(err)
}

// If the proxy is in recording mode and a SSH connection is being
// requested or the target server is a registered OpenSSH node, build
Expand Down Expand Up @@ -837,7 +840,7 @@ func (s *remoteSite) dialAndForward(params reversetunnelclient.DialParams) (_ ne
serverConfig := forward.ServerConfig{
AuthClient: s.localClient,
UserAgent: userAgent,
IsAgentlessNode: params.IsAgentlessNode,
IsAgentlessNode: isAgentlessNode(params),
AgentlessSigner: params.AgentlessSigner,
TargetConn: targetConn,
SrcAddr: params.From,
Expand Down
96 changes: 96 additions & 0 deletions lib/reversetunnel/site.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
Copyright 2023 Gravitational, Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package reversetunnel

import (
"errors"
"net"

"github.com/gravitational/trace"

"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/multiplexer"
"github.com/gravitational/teleport/lib/reversetunnelclient"
"github.com/gravitational/teleport/lib/services"
)

var errDirectDialNoProxyRec = errors.New("direct dialing to nodes not found in inventory requires that the session recording mode is set to record at the proxy")

func checkNodeAndRecConfig(params reversetunnelclient.DialParams, recConfig types.SessionRecordingConfig) error {
if params.IsNotInventoryNode && !services.IsRecordAtProxy(recConfig.GetMode()) {
return trace.Wrap(errDirectDialNoProxyRec)
}
return nil
}

// shouldDialAndForward returns whether a connection should be proxied
// and forwarded or not.
func shouldDialAndForward(params reversetunnelclient.DialParams, recConfig types.SessionRecordingConfig) bool {
// connection is already being tunneled, do not forward
if params.FromPeerProxy {
return false
}
// the node is an agentless node, the connection must be forwarded
if params.TargetServer != nil && params.TargetServer.IsOpenSSHNode() {
return true
}
// proxy session recording mode is being used and an SSH session
// is being requested, the connection must be forwarded
if params.ConnType == types.NodeTunnel && services.IsRecordAtProxy(recConfig.GetMode()) {
return true
}
// if the node was directly dialed and not in the inventory, the
// connection must be forwarded
if params.IsNotInventoryNode {
return true
}

return false
}

// shouldSendSignedPROXYHeader returns whether a connection should send
// a signed PROXY header at the start of the connection or not.
func shouldSendSignedPROXYHeader(signer multiplexer.PROXYHeaderSigner, useTunnel, isAgentlessNode bool, srcAddr, dstAddr net.Addr) bool {
// nothing to sign with, can't send a signed header
if signer == nil {
return false
}
// signed PROXY headers aren't sent over a tunnel
if useTunnel {
return false
}
// we are connecting to an agentless node which won't understand the
// PROXY protocol
if isAgentlessNode {
return false
}
// we have to have both the source and destination to populate the
// signed PROXY header with if we want to send it
if srcAddr == nil || dstAddr == nil {
return false
}

return true
}

func isAgentlessNode(params reversetunnelclient.DialParams) bool {
// If the node is not in the inventory (was directly dialed) tell
// the forwarding server it isn't an agentless node so config checks
// pass. params.TargetServer will ensure the node is not treated as
// a Teleport node in this case.
return params.IsAgentlessNode && !params.IsNotInventoryNode
}
Loading

0 comments on commit c2207fa

Please sign in to comment.