Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v17] Add multi-port support for TCP apps #49711

Merged
merged 6 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
ravicious marked this conversation as resolved.
Show resolved Hide resolved
// 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
Loading