Skip to content

Commit

Permalink
Use TargetPort when routing TCP connections
Browse files Browse the repository at this point in the history
  • Loading branch information
ravicious committed Nov 15, 2024
1 parent 47a9038 commit 823c2d4
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 11 deletions.
79 changes: 71 additions & 8 deletions integration/appaccess/appaccess_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ func testClientCert(p *Pack, t *testing.T) {
wantErr bool
}{
{
desc: "root cluster, valid TLS config, success",
desc: "root cluster, valid TLS config, success",
inTLSConfig: p.makeTLSConfig(t, tlsConfigParams{
sessionID: rootWs.GetName(),
username: rootWs.GetUser(),
Expand All @@ -302,7 +302,7 @@ func testClientCert(p *Pack, t *testing.T) {
outMessage: p.rootMessage,
},
{
desc: "leaf cluster, valid TLS config, success",
desc: "leaf cluster, valid TLS config, success",
inTLSConfig: p.makeTLSConfig(t, tlsConfigParams{
sessionID: leafWs.GetName(),
username: leafWs.GetUser(),
Expand All @@ -318,7 +318,7 @@ func testClientCert(p *Pack, t *testing.T) {
outStatusCode: http.StatusForbidden,
},
{
desc: "root cluster, invalid session owner",
desc: "root cluster, invalid session owner",
inTLSConfig: p.makeTLSConfig(t, tlsConfigParams{
sessionID: rootWs.GetName(),
username: evilUser.GetName(),
Expand All @@ -329,7 +329,7 @@ func testClientCert(p *Pack, t *testing.T) {
outMessage: "",
},
{
desc: "leaf cluster, invalid session owner",
desc: "leaf cluster, invalid session owner",
inTLSConfig: p.makeTLSConfig(t, tlsConfigParams{
sessionID: leafWs.GetName(),
username: evilUser.GetName(),
Expand All @@ -340,7 +340,7 @@ func testClientCert(p *Pack, t *testing.T) {
outMessage: "",
},
{
desc: "root cluster, valid TLS config with pinned IP, success",
desc: "root cluster, valid TLS config with pinned IP, success",
inTLSConfig: p.makeTLSConfig(t, tlsConfigParams{
sessionID: rootWs.GetName(),
username: rootWs.GetUser(),
Expand All @@ -352,7 +352,7 @@ func testClientCert(p *Pack, t *testing.T) {
outMessage: p.rootMessage,
},
{
desc: "root cluster, valid TLS config with wrong pinned IP",
desc: "root cluster, valid TLS config with wrong pinned IP",
inTLSConfig: p.makeTLSConfig(t, tlsConfigParams{
sessionID: rootWs.GetName(),
username: rootWs.GetUser(),
Expand Down Expand Up @@ -682,8 +682,8 @@ func TestTCP(t *testing.T) {
// tlsConfigParams carries information needed to create TLS config for a local proxy.
// tlsConfigParams.sessionID is automatically set from the session created within the test.
tlsConfigParams tlsConfigParams
outMessage string
wantReadErr error
outMessage string
wantReadErr error
}{
{
description: "TCP app in root cluster",
Expand Down Expand Up @@ -721,6 +721,69 @@ func TestTCP(t *testing.T) {
},
wantReadErr: io.EOF, // access denied errors should close the tcp conn
},
{
description: "multi-port TCP app in root cluster",
tlsConfigParams: tlsConfigParams{
username: sessionUsername,
publicAddr: pack.rootTCPMultiPortPublicAddr,
clusterName: pack.rootAppClusterName,
targetPort: pack.rootTCPMultiPortAppPortAlpha,
},
outMessage: pack.rootTCPMultiPortMessageAlpha,
},
{
description: "multi-port TCP app in root cluster, other port",
tlsConfigParams: tlsConfigParams{
username: sessionUsername,
publicAddr: pack.rootTCPMultiPortPublicAddr,
clusterName: pack.rootAppClusterName,
targetPort: pack.rootTCPMultiPortAppPortBeta,
},
outMessage: pack.rootTCPMultiPortMessageBeta,
},
{
description: "multi-port TCP app in leaf cluster",
tlsConfigParams: tlsConfigParams{
username: sessionUsername,
publicAddr: pack.leafTCPMultiPortPublicAddr,
clusterName: pack.leafAppClusterName,
targetPort: pack.leafTCPMultiPortAppPortAlpha,
},
outMessage: pack.leafTCPMultiPortMessageAlpha,
},
{
description: "multi-port TCP app in leaf cluster, other port",
tlsConfigParams: tlsConfigParams{
username: sessionUsername,
publicAddr: pack.leafTCPMultiPortPublicAddr,
clusterName: pack.leafAppClusterName,
targetPort: pack.leafTCPMultiPortAppPortBeta,
},
outMessage: pack.leafTCPMultiPortMessageBeta,
},
{
// This simulates an older client with no TargetPort connecting to a newer app agent.
description: "multi-port TCP app in root cluster, no target port",
tlsConfigParams: tlsConfigParams{
username: sessionUsername,
publicAddr: pack.rootTCPMultiPortPublicAddr,
clusterName: pack.rootAppClusterName,
},
// Such client should still be proxied to the first port found in TCP ports of the app.
outMessage: pack.rootTCPMultiPortMessageAlpha,
},
{
description: "multi-port TCP app, port not in spec",
tlsConfigParams: tlsConfigParams{
username: sessionUsername,
publicAddr: pack.rootTCPMultiPortPublicAddr,
clusterName: pack.rootAppClusterName,
// 42 should not be handed out to a non-root user when creating a listener on port 0, so
// it's unlikely that 42 is going to end up in the app spec.
targetPort: uint16(42),
},
wantReadErr: io.EOF,
},
}

for _, test := range tests {
Expand Down
54 changes: 51 additions & 3 deletions lib/srv/app/tcpserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@ import (
"context"
"log/slog"
"net"
"slices"
"strconv"

"github.com/gravitational/trace"

apidefaults "github.com/gravitational/teleport/api/defaults"
apitypes "github.com/gravitational/teleport/api/types"
apievents "github.com/gravitational/teleport/api/types/events"
netutils "github.com/gravitational/teleport/api/utils/net"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/srv/app/common"
"github.com/gravitational/teleport/lib/tlsca"
Expand All @@ -52,9 +55,21 @@ func (s *tcpServer) handleConnection(ctx context.Context, clientConn net.Conn, i
dialer := net.Dialer{
Timeout: apidefaults.DefaultIOTimeout,
}
serverConn, err := dialer.DialContext(ctx, addr.AddrNetwork, addr.String())
if err != nil {
return trace.Wrap(err)

var serverConn net.Conn
if len(app.GetTCPPorts()) > 0 {
// Multi-port TCP app.
targetPort := int(identity.RouteToApp.TargetPort)
serverConn, err = dialMultiPortTCPApp(ctx, dialer, addr, targetPort, app)
if err != nil {
return trace.Wrap(err)
}
} else {
// Regular TCP app. addr includes port number.
serverConn, err = dialer.DialContext(ctx, addr.AddrNetwork, addr.String())
if err != nil {
return trace.Wrap(err)
}
}

audit, err := common.NewAudit(common.AuditConfig{
Expand All @@ -81,3 +96,36 @@ func (s *tcpServer) handleConnection(ctx context.Context, clientConn net.Conn, i
}
return nil
}

// dialMultiPortTCPApp assumes that app has TCP ports specifies and dials targetPort if it's found
// in TCP ports.
//
// If the client did not include targetPort (it's equal to zero), it dials the first port found in
// TCP ports.
func dialMultiPortTCPApp(ctx context.Context, dialer net.Dialer, appAddr *utils.NetAddr, targetPort int, app apitypes.Application) (net.Conn, error) {
// If the client didn't supply a target port, use the first port found in TCP ports. This is to
// provide backwards compatibility.
//
// In theory, this behavior could be removed in the future if we guarantee that all clients always
// send a target port when connecting to multi-port apps, but no such effort was undertaken so far.
if targetPort == 0 {
firstPort := int(app.GetTCPPorts()[0].Port)
appAddrWithFirstPort := net.JoinHostPort(appAddr.Host(), strconv.Itoa(firstPort))

serverConn, err := dialer.DialContext(ctx, appAddr.AddrNetwork, appAddrWithFirstPort)
return serverConn, trace.Wrap(err)
}

isTargetPortInTCPPorts := slices.ContainsFunc(app.GetTCPPorts(), func(portRange *apitypes.PortRange) bool {
return netutils.IsPortInRange(int(portRange.Port), int(portRange.EndPort), targetPort)
})

if !isTargetPortInTCPPorts {
// This is not treated as an access denied error since there's no RBAC on TCP ports.
return nil, trace.BadParameter("port %d is not in TCP ports of app %q", targetPort, app.GetName())
}

appAddrWithTargetPort := net.JoinHostPort(appAddr.Host(), strconv.Itoa(targetPort))
serverConn, err := dialer.DialContext(ctx, appAddr.AddrNetwork, appAddrWithTargetPort)
return serverConn, trace.Wrap(err)
}

0 comments on commit 823c2d4

Please sign in to comment.