Skip to content

Commit

Permalink
[v17] Add multi-port support for TCP apps (#49711)
Browse files Browse the repository at this point in the history
* Add Ports field to app spec

* Add Ports to AppSpecV3

* Validate ports of api/types.AppV3

* Add Ports to lib/config and lib/service/servicecfg

* lib/config TestApps: Improve error messages

* lib/service: Convert servicecfg.PortRange to types.PortRange

* Add multi-port TCP apps to config and tctl tests

* Rename Ports to TCPPorts

* Change port fields to uint16 where possible

* Update comments for Port and EndPort

* Extract port range validation to api/utils/net

* Replace custom check type with require.ErrorAssertionFunc

* Add `TargetPort` to `RouteToApp` & use it to route connections to multi-port TCP apps

* Add TargetPort to RouteToApp and AppMetadata proto messages

* Pass TargetPort during cert generation

* Refactor Pack.makeTLSConfig to accept struct

This will make it easier to add targetPort to it.

* Add labels to UUIDs used by appaccess test pack app servers

This makes them easier to distinguish when routing doesn't work as expected.

* Refactor Pack.CreateAppSession to accept a struct

* TestTCP: Create app session within test

If we kept the old code, we'd need to manually create a session for each
target port, which would create a lot of duplication.

* Prepare integration test fixtures for multi-port tests

* Add api/utils/net.IsPortInRange

* Use TargetPort when routing TCP connections

* Inline dialMultiPortTCPApp, centralize logic

* Check target port when connecting to single-port app

* Reorder check in IsPortInRange

* Use int instead of uint16

* Extract picking dialTarget to separate function

* Improve err msg for single-port apps when targetPort != uriPort

* Fix unnecessary conversion to int

* Pass port from VNet to local proxy

* Prepare app specs in tests for specifying TCP ports

* Refactor logging in lib/vnet/app_resolver.go

Use libutils.NewPackageLogger, call it log instead of slog which makes
it harder to use the imported default slog logger instead of the one from
a struct.

Move creation of logger within TCPAppResolver.resolveTCPHandlerForCluster

* Pass port from VNet to local proxy

* Don't create another package logger

* Don't pass logger to newTCPAppHandler

* Fix typo in comment

* Explicitly pass port to dialHost

* Convert multi-line definitions of simple appSpecs to single-line

* Add TODO comment about validating local port

* Empty commit to trigger CI

* ConvertAuditEvent: Set SessionStartEvent.App.IsMultiPort
  • Loading branch information
ravicious authored Dec 17, 2024
1 parent 7b52d03 commit 44d8f50
Show file tree
Hide file tree
Showing 40 changed files with 8,866 additions and 6,825 deletions.
1,962 changes: 1,002 additions & 960 deletions api/client/proto/authservice.pb.go

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions api/proto/teleport/legacy/client/proto/authservice.proto
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,13 @@ message RouteToApp {
// GCPServiceAccount is the GCP service account to assume when accessing GCP API.
string GCPServiceAccount = 7 [(gogoproto.jsontag) = "gcp_service_account,omitempty"];
// URI is the URI of the app. This is the internal endpoint where the application is running and isn't user-facing.
// Used merely for audit events and mirrors the URI from the app spec. Not used as a source of
// truth when routing connections.
string URI = 8 [(gogoproto.jsontag) = "uri,omitempty"];
// TargetPort signifies that the cert grants access to a specific port in a multi-port TCP app, as
// long as the port is defined in the app spec. When specified, it must be between 1 and 65535.
// Used only for routing, should not be used in other contexts (e.g., access requests).
uint32 TargetPort = 9 [(gogoproto.jsontag) = "target_port,omitempty"];
}

// GetUserRequest specifies parameters for the GetUser method.
Expand Down
6 changes: 6 additions & 0 deletions api/proto/teleport/legacy/types/events/events.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2690,6 +2690,9 @@ message AppMetadata {
];
// AppName is the configured application name.
string AppName = 4 [(gogoproto.jsontag) = "app_name,omitempty"];
// AppTargetPort signifies that the app is a multi-port TCP app and says which port was used to
// access the app. This field is not set for other types of apps, including single-port TCP apps.
uint32 AppTargetPort = 5 [(gogoproto.jsontag) = "app_target_port,omitempty"];
}

// AppCreate is emitted when a new application resource is created.
Expand Down Expand Up @@ -4848,6 +4851,9 @@ message RouteToApp {
string GCPServiceAccount = 7 [(gogoproto.jsontag) = "gcp_service_account,omitempty"];
// URI is the application URI.
string URI = 8 [(gogoproto.jsontag) = "uri,omitempty"];
// TargetPort signifies that the user accessed a specific port in a multi-port TCP app. The value
// must be between 1 and 65535.
uint32 TargetPort = 9 [(gogoproto.jsontag) = "target_port,omitempty"];
}

// RouteToDatabase combines parameters for database service routing information.
Expand Down
17 changes: 17 additions & 0 deletions api/proto/teleport/legacy/types/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,11 @@ message AppSpecV3 {
// IdentityCenter encasulates AWS identity-center specific information. Only
// valid for Identity Center account apps.
AppIdentityCenter IdentityCenter = 12 [(gogoproto.jsontag) = "identity_center,omitempty"];
// TCPPorts is a list of ports and port ranges that an app agent can forward connections to.
// Only applicable to TCP App Access.
// If this field is not empty, URI is expected to contain no port number and start with the tcp
// protocol.
repeated PortRange TCPPorts = 13 [(gogoproto.jsontag) = "tcp_ports,omitempty"];
}

// AppServerOrSAMLIdPServiceProviderV1 holds either an AppServerV3 or a SAMLIdPServiceProviderV1 resource (never both).
Expand Down Expand Up @@ -1062,6 +1067,18 @@ message Header {
string Value = 2 [(gogoproto.jsontag) = "value"];
}

// PortRange describes a port range for TCP apps. The range starts with Port and ends with EndPort.
// PortRange can be used to describe a single port in which case the Port field is the port and the
// EndPort field is 0.
message PortRange {
// Port describes the start of the range. It must be between 1 and 65535.
uint32 Port = 1 [(gogoproto.jsontag) = "port"];
// EndPort describes the end of the range, inclusive. If set, it must be between 2 and 65535 and
// be greater than Port when describing a port range. When omitted or set to zero, it signifies
// that the port range defines a single port.
uint32 EndPort = 2 [(gogoproto.jsontag) = "end_port,omitempty"];
}

// CommandLabelV2 is a label that has a value as a result of the
// output generated by running command, e.g. hostname
message CommandLabelV2 {
Expand Down
57 changes: 54 additions & 3 deletions api/types/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/gravitational/teleport/api/constants"
"github.com/gravitational/teleport/api/types/compare"
"github.com/gravitational/teleport/api/utils"
netutils "github.com/gravitational/teleport/api/utils/net"
)

var _ compare.IsEqual[Application] = (*AppV3)(nil)
Expand Down Expand Up @@ -86,6 +87,10 @@ type Application interface {
GetRequiredAppNames() []string
// GetCORS returns the CORS configuration for the app.
GetCORS() *CORSPolicy
// GetTCPPorts returns port ranges supported by the app to which connections can be forwarded to.
GetTCPPorts() []*PortRange
// SetTCPPorts sets port ranges to which connections can be forwarded to.
SetTCPPorts([]*PortRange)
// GetIdentityCenter fetches identity center info for the app, if any.
GetIdentityCenter() *AppIdentityCenter
}
Expand Down Expand Up @@ -308,6 +313,16 @@ func (a *AppV3) SetUserGroups(userGroups []string) {
a.Spec.UserGroups = userGroups
}

// GetTCPPorts returns port ranges supported by the app to which connections can be forwarded to.
func (a *AppV3) GetTCPPorts() []*PortRange {
return a.Spec.TCPPorts
}

// SetTCPPorts sets port ranges to which connections can be forwarded to.
func (a *AppV3) SetTCPPorts(ports []*PortRange) {
a.Spec.TCPPorts = ports
}

// GetIntegration will return the Integration.
// If present, the Application must use the Integration's credentials instead of ambient credentials to access Cloud APIs.
func (a *AppV3) GetIntegration() string {
Expand Down Expand Up @@ -381,13 +396,13 @@ func (a *AppV3) CheckAndSetDefaults() error {
if !strings.Contains(publicAddr, "//") && strings.Contains(publicAddr, ":") {
publicAddr = "//" + publicAddr
}
url, err := url.Parse(publicAddr)
publicAddrURL, err := url.Parse(publicAddr)
if err != nil {
return trace.BadParameter("invalid PublicAddr format: %v", err)
}
host := a.Spec.PublicAddr
if url.Host != "" {
host = url.Host
if publicAddrURL.Host != "" {
host = publicAddrURL.Host
}

if strings.HasPrefix(host, constants.KubeTeleportProxyALPNPrefix) {
Expand All @@ -404,6 +419,42 @@ func (a *AppV3) CheckAndSetDefaults() error {
}
}

if len(a.Spec.TCPPorts) != 0 {
if err := a.checkTCPPorts(); err != nil {
return trace.Wrap(err)
}
}

return nil
}

func (a *AppV3) checkTCPPorts() error {
// Parsing the URI here does not break compatibility. The URI is parsed only if Ports are present.
// This means that old apps that do have invalid URIs but don't use Ports can continue existing.
uri, err := url.Parse(a.Spec.URI)
if err != nil {
return trace.BadParameter("invalid app URI format: %v", err)
}

// The scheme of URI is enforced to be "tcp" on purpose. This way in the future we can add
// multi-port support to web apps without throwing hard errors when a cluster with a multi-port
// web app gets downgraded to a version which supports multi-port only for TCP apps.
//
// For now, we simply ignore the Ports field set on non-TCP apps.
if uri.Scheme != "tcp" {
return nil
}

if uri.Port() != "" {
return trace.BadParameter("TCP app URI %q must not include a port number when the app spec defines a list of ports", a.Spec.URI)
}

for _, portRange := range a.Spec.TCPPorts {
if err := netutils.ValidatePortRange(int(portRange.Port), int(portRange.EndPort)); err != nil {
return trace.Wrap(err, "validating a port range of a TCP app")
}
}

return nil
}

Expand Down
156 changes: 135 additions & 21 deletions api/types/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,58 +29,45 @@ import (
// TestAppPublicAddrValidation tests PublicAddr field validation to make sure that
// an app with internal "kube-teleport-proxy-alpn." ServerName prefix won't be created.
func TestAppPublicAddrValidation(t *testing.T) {
type check func(t *testing.T, err error)

hasNoErr := func() check {
return func(t *testing.T, err error) {
require.NoError(t, err)
}
}
hasErrTypeBadParameter := func() check {
return func(t *testing.T, err error) {
require.True(t, trace.IsBadParameter(err))
}
}

tests := []struct {
name string
publicAddr string
check check
check require.ErrorAssertionFunc
}{
{
name: "kubernetes app",
publicAddr: "kubernetes.example.com:3080",
check: hasNoErr(),
check: hasNoErr,
},
{
name: "kubernetes app public addr without port",
publicAddr: "kubernetes.example.com",
check: hasNoErr(),
check: hasNoErr,
},
{
name: "kubernetes app http",
publicAddr: "http://kubernetes.example.com:3080",
check: hasNoErr(),
check: hasNoErr,
},
{
name: "kubernetes app https",
publicAddr: "https://kubernetes.example.com:3080",
check: hasNoErr(),
check: hasNoErr,
},
{
name: "public address with internal kube ServerName prefix",
publicAddr: constants.KubeTeleportProxyALPNPrefix + "example.com:3080",
check: hasErrTypeBadParameter(),
check: hasErrTypeBadParameter,
},
{
name: "https public address with internal kube ServerName prefix",
publicAddr: "https://" + constants.KubeTeleportProxyALPNPrefix + "example.com:3080",
check: hasErrTypeBadParameter(),
check: hasErrTypeBadParameter,
},
{
name: "addr with numbers in the host",
publicAddr: "123456789012.teleport.example.com:3080",
check: hasNoErr(),
check: hasNoErr,
},
}

Expand All @@ -97,6 +84,112 @@ func TestAppPublicAddrValidation(t *testing.T) {
}
}

func TestAppPortsValidation(t *testing.T) {
tests := []struct {
name string
tcpPorts []*PortRange
uri string
check require.ErrorAssertionFunc
}{
{
name: "valid ranges and single ports",
tcpPorts: []*PortRange{
&PortRange{Port: 22, EndPort: 25},
&PortRange{Port: 26},
&PortRange{Port: 65535},
},
check: hasNoErr,
},
{
name: "valid overlapping ranges",
tcpPorts: []*PortRange{
&PortRange{Port: 100, EndPort: 200},
&PortRange{Port: 150, EndPort: 175},
&PortRange{Port: 111},
&PortRange{Port: 150, EndPort: 210},
&PortRange{Port: 1, EndPort: 65535},
},
check: hasNoErr,
},
{
name: "valid non-TCP app with ports ignored",
uri: "http://localhost:8000",
tcpPorts: []*PortRange{
&PortRange{Port: 123456789},
&PortRange{Port: 10, EndPort: 2},
},
check: hasNoErr,
},
// Test cases for invalid ports.
{
name: "port smaller than 1",
tcpPorts: []*PortRange{
&PortRange{Port: 0},
},
check: hasErrTypeBadParameter,
},
{
name: "port bigger than 65535",
tcpPorts: []*PortRange{
&PortRange{Port: 78787},
},
check: hasErrTypeBadParameter,
},
{
name: "end port smaller than 2",
tcpPorts: []*PortRange{
&PortRange{Port: 5, EndPort: 1},
},
check: hasErrTypeBadParameterAndContains("end port must be between 6 and 65535"),
},
{
name: "end port bigger than 65535",
tcpPorts: []*PortRange{
&PortRange{Port: 1, EndPort: 78787},
},
check: hasErrTypeBadParameter,
},
{
name: "end port smaller than port",
tcpPorts: []*PortRange{
&PortRange{Port: 10, EndPort: 5},
},
check: hasErrTypeBadParameterAndContains("end port must be between 11 and 65535"),
},
{
name: "uri specifies port",
uri: "tcp://localhost:1234",
tcpPorts: []*PortRange{
&PortRange{Port: 1000, EndPort: 1500},
},
check: hasErrTypeBadParameterAndContains("must not include a port number"),
},
{
name: "invalid uri",
uri: "%",
tcpPorts: []*PortRange{
&PortRange{Port: 1000, EndPort: 1500},
},
check: hasErrAndContains("invalid URL escape"),
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
spec := AppSpecV3{
URI: "tcp://localhost",
TCPPorts: tc.tcpPorts,
}
if tc.uri != "" {
spec.URI = tc.uri
}

_, err := NewAppV3(Metadata{Name: "TestApp"}, spec)
tc.check(t, err)
})
}
}

func TestAppServerSorter(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -469,3 +562,24 @@ func TestNewAppV3(t *testing.T) {
})
}
}

func hasNoErr(t require.TestingT, err error, msgAndArgs ...interface{}) {
require.NoError(t, err, msgAndArgs...)
}

func hasErrTypeBadParameter(t require.TestingT, err error, msgAndArgs ...interface{}) {
require.True(t, trace.IsBadParameter(err), "expected bad parameter error, got %+v", err)
}

func hasErrTypeBadParameterAndContains(msg string) require.ErrorAssertionFunc {
return func(t require.TestingT, err error, msgAndArgs ...interface{}) {
require.True(t, trace.IsBadParameter(err), "err should be trace.BadParameter")
require.ErrorContains(t, err, msg, msgAndArgs...)
}
}

func hasErrAndContains(msg string) require.ErrorAssertionFunc {
return func(t require.TestingT, err error, msgAndArgs ...interface{}) {
require.ErrorContains(t, err, msg, msgAndArgs...)
}
}
Loading

0 comments on commit 44d8f50

Please sign in to comment.