Skip to content

Commit

Permalink
Add TargetPort to RouteToApp & use it to route connections to mul…
Browse files Browse the repository at this point in the history
…ti-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
  • Loading branch information
ravicious committed Dec 3, 2024
1 parent a0017af commit 511d7dd
Show file tree
Hide file tree
Showing 19 changed files with 2,622 additions and 1,941 deletions.
1,683 changes: 862 additions & 821 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 @@ -2675,6 +2675,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 @@ -4829,6 +4832,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
2,159 changes: 1,111 additions & 1,048 deletions api/types/events/events.pb.go

Large diffs are not rendered by default.

11 changes: 11 additions & 0 deletions api/utils/net/ports.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,14 @@ func ValidatePortRange(port, endPort int) error {

return nil
}

// IsPortInRange checks if targetPort is between port and endPort (inclusive). If endPort is zero,
// it checks if targetPort equals port. It assumes that the provided port range is valid (see
// [ValidatePortRange]).
func IsPortInRange(port, endPort, targetPort int) bool {
if endPort == 0 {
return targetPort == port
}

return port <= targetPort && targetPort <= endPort
}
73 changes: 73 additions & 0 deletions api/utils/net/ports_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,79 @@ func TestValidatePortRange(t *testing.T) {
}
}

func TestIsPortInRange(t *testing.T) {
tests := []struct {
name string
port int
endPort int
targetPort int
check require.BoolAssertionFunc
}{
{
name: "within single port range",
port: 1337,
endPort: 0,
targetPort: 1337,
check: require.True,
},
{
name: "within port range",
port: 1337,
endPort: 3456,
targetPort: 2550,
check: require.True,
},
{
name: "equal to range start",
port: 1337,
endPort: 3456,
targetPort: 1337,
check: require.True,
},
{
name: "equal to range end",
port: 1337,
endPort: 3456,
targetPort: 3456,
check: require.True,
},
{
name: "outside of single port range",
port: 1337,
endPort: 0,
targetPort: 7331,
check: require.False,
},
{
name: "equal to end of single port range",
port: 1337,
endPort: 0,
targetPort: 0,
check: require.False,
},
{
name: "smaller than range start",
port: 1337,
endPort: 3456,
targetPort: 42,
check: require.False,
},
{
name: "bigger than range end",
port: 1337,
endPort: 3456,
targetPort: 7331,
check: require.False,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.check(t, IsPortInRange(tt.port, tt.endPort, tt.targetPort), "compared %d against %d-%d", tt.targetPort, tt.port, tt.endPort)
})
}
}

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

0 comments on commit 511d7dd

Please sign in to comment.