Skip to content

Commit b54ddb9

Browse files
committed
Improve client tools host resolution (#50175)
Host resolution performed because labels, fuzzy search, or predicate expressions were supplied to commands that establish connections to a single host has historically been performed client side in tsh. While that works in most cases, it can prevent correctly resolving hosts in some situations, i.e. when there are ambiguous hosts and tsh is unaware that the cluster routing strategy is set to ROUTE_TO_MOST_RECENT. To improve the experience, a new ResolveSSHTarget was added to Auth to allow host resolution to be performed server side. The resolution works in a similar manner to, and was inspired by GetSSHTargets. In the event that the new RPC is not implemented, because the client is newer than Auth, tsh has also been updated to pull the cluster networking config and address any host ambiguity if allowed. As a result tsh scp and tsh proxy ssh should be much more tolerant to, and still permit access in situations where ambiguous hosts are present for some amount of time. Prior to this the only way to connect in these situations was to find the UUID of the correct target instance and try again after seeing an ambiguous host error.
1 parent d5742bc commit b54ddb9

File tree

21 files changed

+2403
-1136
lines changed

21 files changed

+2403
-1136
lines changed

api/client/client.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4311,6 +4311,12 @@ func (c *Client) GetSSHTargets(ctx context.Context, req *proto.GetSSHTargetsRequ
43114311
return rsp, trace.Wrap(err)
43124312
}
43134313

4314+
// ResolveSSHTarget gets a server that would match an equivalent ssh dial request.
4315+
func (c *Client) ResolveSSHTarget(ctx context.Context, req *proto.ResolveSSHTargetRequest) (*proto.ResolveSSHTargetResponse, error) {
4316+
rsp, err := c.grpc.ResolveSSHTarget(ctx, req)
4317+
return rsp, trace.Wrap(err)
4318+
}
4319+
43144320
// CreateSessionTracker creates a tracker resource for an active session.
43154321
func (c *Client) CreateSessionTracker(ctx context.Context, st types.SessionTracker) (types.SessionTracker, error) {
43164322
v1, ok := st.(*types.SessionTrackerV1)

api/client/proto/authservice.pb.go

Lines changed: 1793 additions & 1039 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/proto/teleport/legacy/client/proto/authservice.proto

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2144,6 +2144,36 @@ message ListResourcesRequest {
21442144
bool IncludeLogins = 13 [(gogoproto.jsontag) = "include_logins,omitempty"];
21452145
}
21462146

2147+
// ResolveSSHTargetRequest provides details about a server to be resolved in
2148+
// an equivalent manner to a ssh dial request.
2149+
//
2150+
// Resolution can happen in two modes:
2151+
// 1) searching for hosts based on labels, a predicate expression, or keywords
2152+
// 2) searching based on hostname
2153+
//
2154+
// If a Host is provided, resolution will only operate in the second mode and
2155+
// will not perform any resolution based on labels. In order to resolve via
2156+
// labels the Host must not be populated.
2157+
message ResolveSSHTargetRequest {
2158+
// The target host as would be sent to the proxy during a dial request.
2159+
string host = 1;
2160+
// The ssh port. This value is optional, and both empty string and "0" are typically
2161+
// treated as meaning that any port should match.
2162+
string port = 2;
2163+
// If not empty, a label-based matcher.
2164+
map<string, string> labels = 3;
2165+
// Boolean conditions that will be matched against the resource.
2166+
string predicate_expression = 4;
2167+
// A list of search keywords to match against resource field values.
2168+
repeated string search_keywords = 5;
2169+
}
2170+
2171+
// GetSSHTargetsResponse holds ssh servers that match an ssh targets request.
2172+
message ResolveSSHTargetResponse {
2173+
// The target matching the supplied request.
2174+
types.ServerV2 server = 1;
2175+
}
2176+
21472177
// GetSSHTargetsRequest gets all servers that might match an equivalent ssh dial request.
21482178
message GetSSHTargetsRequest {
21492179
// Host is the target host as would be sent to the proxy during a dial request.
@@ -3551,6 +3581,9 @@ service AuthService {
35513581
// but may result in confusing behavior if it is used outside of those contexts.
35523582
rpc GetSSHTargets(GetSSHTargetsRequest) returns (GetSSHTargetsResponse);
35533583

3584+
// ResolveSSHTarget returns the server that would be resolved in an equivalent ssh dial request.
3585+
rpc ResolveSSHTarget(ResolveSSHTargetRequest) returns (ResolveSSHTargetResponse);
3586+
35543587
// GetDomainName returns local auth domain of the current auth server
35553588
rpc GetDomainName(google.protobuf.Empty) returns (GetDomainNameResponse);
35563589
// GetClusterCACert returns the PEM-encoded TLS certs for the local cluster

constants.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ package teleport
2121
import (
2222
"strings"
2323
"time"
24+
25+
"github.com/gravitational/trace"
2426
)
2527

2628
const (
@@ -823,9 +825,15 @@ const (
823825
UsageWindowsDesktopOnly = "usage:windows_desktop"
824826
)
825827

828+
// ErrNodeIsAmbiguous serves as an identifying error string indicating that
829+
// the proxy subsystem found multiple nodes matching the specified hostname.
830+
var ErrNodeIsAmbiguous = &trace.NotFoundError{Message: "ambiguous host could match multiple nodes"}
831+
826832
const (
827833
// NodeIsAmbiguous serves as an identifying error string indicating that
828834
// the proxy subsystem found multiple nodes matching the specified hostname.
835+
// TODO(tross) DELETE IN v20.0.0
836+
// Deprecated: Prefer using ErrNodeIsAmbiguous
829837
NodeIsAmbiguous = "err-node-is-ambiguous"
830838

831839
// MaxLeases serves as an identifying error string indicating that the

integration/integration_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -811,9 +811,7 @@ func testUUIDBasedProxy(t *testing.T, suite *integrationTestSuite) {
811811
// attempting to run a command by hostname should generate NodeIsAmbiguous error.
812812
_, err = runCommand(t, teleportSvr, []string{"echo", "Hello there!"}, helpers.ClientConfig{Login: suite.Me.Username, Cluster: helpers.Site, Host: Host}, 1)
813813
require.Error(t, err)
814-
if !strings.Contains(err.Error(), teleport.NodeIsAmbiguous) {
815-
require.FailNowf(t, "Expected %s, got %s", teleport.NodeIsAmbiguous, err.Error())
816-
}
814+
require.ErrorContains(t, err, "ambiguous")
817815

818816
// attempting to run a command by uuid should succeed.
819817
_, err = runCommand(t, teleportSvr, []string{"echo", "Hello there!"}, helpers.ClientConfig{Login: suite.Me.Username, Cluster: helpers.Site, Host: uuid1}, 1)

lib/auth/auth_with_roles.go

Lines changed: 93 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1661,22 +1661,23 @@ func (a *ServerWithRoles) GetSSHTargets(ctx context.Context, req *proto.GetSSHTa
16611661
return nil, trace.Wrap(err)
16621662
}
16631663

1664-
lreq := proto.ListResourcesRequest{
1665-
ResourceType: types.KindNode,
1664+
lreq := &proto.ListUnifiedResourcesRequest{
1665+
Kinds: []string{types.KindNode},
1666+
SortBy: types.SortBy{Field: types.ResourceMetadataName},
16661667
UseSearchAsRoles: true,
16671668
}
16681669
var servers []*types.ServerV2
16691670
for {
1670-
// note that we're calling ServerWithRoles.ListResources here rather than some internal method. This method
1671+
// note that we're calling ServerWithRoles.ListUnifiedResources here rather than some internal method. This method
16711672
// delegates all RBAC filtering to ListResources, and then performs additional filtering on top of that.
1672-
lrsp, err := a.ListResources(ctx, lreq)
1673+
lrsp, err := a.ListUnifiedResources(ctx, lreq)
16731674
if err != nil {
16741675
return nil, trace.Wrap(err)
16751676
}
16761677

16771678
for _, rsc := range lrsp.Resources {
1678-
srv, ok := rsc.(*types.ServerV2)
1679-
if !ok {
1679+
srv := rsc.GetNode()
1680+
if srv == nil {
16801681
log.Warnf("Unexpected resource type %T, expected *types.ServerV2 (skipping)", rsc)
16811682
continue
16821683
}
@@ -1700,6 +1701,92 @@ func (a *ServerWithRoles) GetSSHTargets(ctx context.Context, req *proto.GetSSHTa
17001701
}, nil
17011702
}
17021703

1704+
// ResolveSSHTarget gets a server that would match an equivalent ssh dial request.
1705+
func (a *ServerWithRoles) ResolveSSHTarget(ctx context.Context, req *proto.ResolveSSHTargetRequest) (*proto.ResolveSSHTargetResponse, error) {
1706+
// try to detect case-insensitive routing setting, but default to false if we can't load
1707+
// networking config (equivalent to proxy routing behavior).
1708+
var routeToMostRecent bool
1709+
if cfg, err := a.authServer.GetReadOnlyClusterNetworkingConfig(ctx); err == nil {
1710+
routeToMostRecent = cfg.GetRoutingStrategy() == types.RoutingStrategy_MOST_RECENT
1711+
}
1712+
1713+
var servers []*types.ServerV2
1714+
switch {
1715+
case req.Host != "":
1716+
if len(req.Labels) > 0 || req.PredicateExpression != "" || len(req.SearchKeywords) > 0 {
1717+
a.authServer.logger.WarnContext(ctx, "ssh target resolution request contained both host and a resource matcher - ignoring resource matcher")
1718+
}
1719+
1720+
resp, err := a.GetSSHTargets(ctx, &proto.GetSSHTargetsRequest{
1721+
Host: req.Host,
1722+
Port: req.Port,
1723+
})
1724+
if err != nil {
1725+
return nil, trace.Wrap(err)
1726+
}
1727+
1728+
servers = resp.Servers
1729+
case len(req.Labels) > 0 || req.PredicateExpression != "" || len(req.SearchKeywords) > 0:
1730+
lreq := &proto.ListUnifiedResourcesRequest{
1731+
Kinds: []string{types.KindNode},
1732+
SortBy: types.SortBy{Field: types.ResourceMetadataName},
1733+
Labels: req.Labels,
1734+
PredicateExpression: req.PredicateExpression,
1735+
SearchKeywords: req.SearchKeywords,
1736+
}
1737+
for {
1738+
// note that we're calling ServerWithRoles.ListUnifiedResources here rather than some internal method. This method
1739+
// delegates all RBAC filtering to ListResources, and then performs additional filtering on top of that.
1740+
lrsp, err := a.ListUnifiedResources(ctx, lreq)
1741+
if err != nil {
1742+
return nil, trace.Wrap(err)
1743+
}
1744+
1745+
for _, rsc := range lrsp.Resources {
1746+
srv := rsc.GetNode()
1747+
if srv == nil {
1748+
log.Warnf("Unexpected resource type %T, expected *types.ServerV2 (skipping)", rsc)
1749+
continue
1750+
}
1751+
1752+
servers = append(servers, srv)
1753+
}
1754+
1755+
// If the routing strategy doesn't permit ambiguous matches, then abort
1756+
// early if more than one server has been found already
1757+
if !routeToMostRecent && len(servers) > 1 {
1758+
break
1759+
}
1760+
1761+
if lrsp.NextKey == "" || len(lrsp.Resources) == 0 {
1762+
break
1763+
}
1764+
1765+
lreq.StartKey = lrsp.NextKey
1766+
1767+
}
1768+
default:
1769+
return nil, trace.BadParameter("request did not contain any host information or resource matcher")
1770+
}
1771+
1772+
switch len(servers) {
1773+
case 1:
1774+
return &proto.ResolveSSHTargetResponse{Server: servers[0]}, nil
1775+
case 0:
1776+
return nil, trace.NotFound("no matching hosts")
1777+
default:
1778+
if !routeToMostRecent {
1779+
return nil, trace.Wrap(teleport.ErrNodeIsAmbiguous)
1780+
}
1781+
1782+
// Return the most recent version of the resource.
1783+
server := slices.MaxFunc(servers, func(a, b *types.ServerV2) int {
1784+
return a.Expiry().Compare(b.Expiry())
1785+
})
1786+
return &proto.ResolveSSHTargetResponse{Server: server}, nil
1787+
}
1788+
}
1789+
17031790
// ListResources returns a paginated list of resources filtered by user access.
17041791
func (a *ServerWithRoles) ListResources(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error) {
17051792
// Check if auth server has a license for this resource type but only return an

lib/auth/authclient/clt.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1875,6 +1875,9 @@ type ClientI interface {
18751875
// but may result in confusing behavior if it is used outside of those contexts.
18761876
GetSSHTargets(ctx context.Context, req *proto.GetSSHTargetsRequest) (*proto.GetSSHTargetsResponse, error)
18771877

1878+
// ResolveSSHTarget returns the server that would be resolved in an equivalent ssh dial request.
1879+
ResolveSSHTarget(ctx context.Context, req *proto.ResolveSSHTargetRequest) (*proto.ResolveSSHTargetResponse, error)
1880+
18781881
// PerformMFACeremony retrieves an MFA challenge from the server with the given challenge extensions
18791882
// and prompts the user to answer the challenge with the given promptOpts, and ultimately returning
18801883
// an MFA challenge response for the user.

lib/auth/grpcserver.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4197,6 +4197,21 @@ func (g *GRPCServer) GetSSHTargets(ctx context.Context, req *authpb.GetSSHTarget
41974197
return rsp, nil
41984198
}
41994199

4200+
// ResolveSSHTarget gets a server that would match an equivalent ssh dial request.
4201+
func (g *GRPCServer) ResolveSSHTarget(ctx context.Context, req *authpb.ResolveSSHTargetRequest) (*authpb.ResolveSSHTargetResponse, error) {
4202+
auth, err := g.authenticate(ctx)
4203+
if err != nil {
4204+
return nil, trace.Wrap(err)
4205+
}
4206+
4207+
rsp, err := auth.ServerWithRoles.ResolveSSHTarget(ctx, req)
4208+
if err != nil {
4209+
return nil, trace.Wrap(err)
4210+
}
4211+
4212+
return rsp, nil
4213+
}
4214+
42004215
// CreateSessionTracker creates a tracker resource for an active session.
42014216
func (g *GRPCServer) CreateSessionTracker(ctx context.Context, req *authpb.CreateSessionTrackerRequest) (*types.SessionTrackerV1, error) {
42024217
auth, err := g.authenticate(ctx)

lib/auth/grpcserver_test.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2988,6 +2988,69 @@ func TestGetSSHTargets(t *testing.T) {
29882988
require.ElementsMatch(t, []string{rsp.Servers[0].GetHostname(), rsp.Servers[1].GetHostname()}, []string{"foo", "Foo"})
29892989
}
29902990

2991+
func TestResolveSSHTarget(t *testing.T) {
2992+
t.Parallel()
2993+
ctx := context.Background()
2994+
srv := newTestTLSServer(t)
2995+
2996+
clt, err := srv.NewClient(TestAdmin())
2997+
require.NoError(t, err)
2998+
2999+
upper, err := types.NewServerWithLabels(uuid.New().String(), types.KindNode, types.ServerSpecV2{
3000+
Hostname: "Foo",
3001+
UseTunnel: true,
3002+
}, nil)
3003+
require.NoError(t, err)
3004+
upper.SetExpiry(time.Now().Add(time.Hour))
3005+
3006+
lower, err := types.NewServerWithLabels(uuid.New().String(), types.KindNode, types.ServerSpecV2{
3007+
Hostname: "foo",
3008+
UseTunnel: true,
3009+
}, nil)
3010+
require.NoError(t, err)
3011+
3012+
other, err := types.NewServerWithLabels(uuid.New().String(), types.KindNode, types.ServerSpecV2{
3013+
Hostname: "bar",
3014+
UseTunnel: true,
3015+
}, nil)
3016+
require.NoError(t, err)
3017+
3018+
for _, node := range []types.Server{upper, lower, other} {
3019+
_, err = clt.UpsertNode(ctx, node)
3020+
require.NoError(t, err)
3021+
}
3022+
3023+
rsp, err := clt.ResolveSSHTarget(ctx, &proto.ResolveSSHTargetRequest{
3024+
Host: "foo",
3025+
Port: "0",
3026+
})
3027+
require.NoError(t, err)
3028+
require.Equal(t, "foo", rsp.Server.GetHostname())
3029+
3030+
cnc := types.DefaultClusterNetworkingConfig()
3031+
cnc.SetCaseInsensitiveRouting(true)
3032+
_, err = clt.UpsertClusterNetworkingConfig(ctx, cnc)
3033+
require.NoError(t, err)
3034+
3035+
rsp, err = clt.ResolveSSHTarget(ctx, &proto.ResolveSSHTargetRequest{
3036+
Host: "foo",
3037+
Port: "0",
3038+
})
3039+
require.Error(t, err)
3040+
require.Nil(t, rsp)
3041+
3042+
cnc.SetRoutingStrategy(types.RoutingStrategy_MOST_RECENT)
3043+
_, err = clt.UpsertClusterNetworkingConfig(ctx, cnc)
3044+
require.NoError(t, err)
3045+
3046+
rsp, err = clt.ResolveSSHTarget(ctx, &proto.ResolveSSHTargetRequest{
3047+
Host: "foo",
3048+
Port: "0",
3049+
})
3050+
require.NoError(t, err)
3051+
require.Equal(t, "Foo", rsp.Server.GetHostname())
3052+
}
3053+
29913054
func TestNodesCRUD(t *testing.T) {
29923055
t.Parallel()
29933056
ctx := context.Background()

0 commit comments

Comments
 (0)