Skip to content

Commit

Permalink
add ability to disable unqualified hostname lookups (#42952) (#43320)
Browse files Browse the repository at this point in the history
  • Loading branch information
fspmarshall authored Jun 20, 2024
1 parent 545fe48 commit aa568d4
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 6 deletions.
10 changes: 9 additions & 1 deletion api/utils/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"errors"
"net"
"strings"
"unicode/utf8"

"github.com/google/uuid"
Expand Down Expand Up @@ -49,6 +50,8 @@ type SSHRouteMatcherConfig struct {
Resolver HostResolver
// CaseInsensitive enabled case insensitive routing when true.
CaseInsensitive bool
// DisableUnqualifiedLookups disables lookups for unqualified hostnames.
DisableUnqualifiedLookups bool
}

// HostResolver provides an interface matching the net.Resolver.LookupHost method. Typically
Expand Down Expand Up @@ -88,7 +91,12 @@ func newSSHRouteMatcher(cfg SSHRouteMatcherConfig) SSHRouteMatcher {
_, err := uuid.Parse(cfg.Host)
dialByID := err == nil || aws.IsEC2NodeID(cfg.Host)

ips, _ := cfg.Resolver.LookupHost(context.Background(), cfg.Host)
var ips []string
if !(cfg.DisableUnqualifiedLookups && !strings.Contains(cfg.Host, ".")) {
// unqualified lookups are still on by default, but future versions of teleport may disable them as they tend
// to be responsible for the majority of all lookups generated by a teleport cluster and are of questionable utility.
ips, _ = cfg.Resolver.LookupHost(context.Background(), cfg.Host)
}

return SSHRouteMatcher{
cfg: cfg,
Expand Down
63 changes: 63 additions & 0 deletions api/utils/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,3 +323,66 @@ func TestSSHRouteMatcherScoring(t *testing.T) {
})
}
}

type recordingHostResolver struct {
didLookup bool
}

func (r *recordingHostResolver) LookupHost(ctx context.Context, host string) (addrs []string, err error) {
r.didLookup = true
return nil, nil
}

// TestDisableUnqualifiedLookups verifies that unqualified lookups being disabled results
// in single-element/tld style hostname targets not being resolved.
func TestDisableUnqualifiedLookups(t *testing.T) {
tts := []struct {
desc string
target string
lookup bool
}{
{
desc: "qualified hostname",
target: "example.com",
lookup: true,
},
{
desc: "unqualified hostname",
target: "example",
lookup: false,
},
{
desc: "localhost",
target: "localhost",
lookup: false,
},
{
desc: "foo.localhost",
target: "foo.localhost",
lookup: true,
},
{
desc: "uuid",
target: uuid.NewString(),
lookup: false,
},
{
desc: "qualified uuid",
target: "foo." + uuid.NewString(),
lookup: true,
},
}

for _, tt := range tts {
t.Run(tt.desc, func(t *testing.T) {
resolver := &recordingHostResolver{}
_, err := NewSSHRouteMatcherFromConfig(SSHRouteMatcherConfig{
Host: tt.target,
Resolver: resolver,
DisableUnqualifiedLookups: true,
})
require.NoError(t, err)
require.Equal(t, tt.lookup, resolver.didLookup)
})
}
}
13 changes: 12 additions & 1 deletion lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"net/url"
"os"
"slices"
"strings"
"time"
Expand Down Expand Up @@ -1763,6 +1764,8 @@ func (a *ServerWithRoles) authContextForSearch(ctx context.Context, req *proto.L
return extendedContext, nil
}

var disableUnqualifiedLookups = os.Getenv("TELEPORT_UNSTABLE_DISABLE_UNQUALIFIED_LOOKUPS") == "yes"

// GetSSHTargets gets all servers that would match an equivalent ssh dial request. Note that this method
// returns all resources directly accessible to the user *and* all resources available via 'SearchAsRoles',
// which is what we want when handling things like ambiguous host errors and resource-based access requests,
Expand All @@ -1775,7 +1778,15 @@ func (a *ServerWithRoles) GetSSHTargets(ctx context.Context, req *proto.GetSSHTa
caseInsensitiveRouting = cfg.GetCaseInsensitiveRouting()
}

matcher := apiutils.NewSSHRouteMatcher(req.Host, req.Port, caseInsensitiveRouting)
matcher, err := apiutils.NewSSHRouteMatcherFromConfig(apiutils.SSHRouteMatcherConfig{
Host: req.Host,
Port: req.Port,
CaseInsensitive: caseInsensitiveRouting,
DisableUnqualifiedLookups: disableUnqualifiedLookups,
})
if err != nil {
return nil, trace.Wrap(err)
}

lreq := proto.ListResourcesRequest{
ResourceType: types.KindNode,
Expand Down
11 changes: 7 additions & 4 deletions lib/proxy/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,8 @@ func getServer(ctx context.Context, host, port string, site site) (types.Server,
return getServerWithResolver(ctx, host, port, site, nil /* use default resolver */)
}

var disableUnqualifiedLookups = os.Getenv("TELEPORT_UNSTABLE_DISABLE_UNQUALIFIED_LOOKUPS") == "yes"

// getServerWithResolver attempts to locate a node matching the provided host and port in
// the provided site. The resolver argument is used in certain tests to mock DNS resolution
// and can generally be left nil.
Expand All @@ -464,10 +466,11 @@ func getServerWithResolver(ctx context.Context, host, port string, site site, re
}

routeMatcher, err := apiutils.NewSSHRouteMatcherFromConfig(apiutils.SSHRouteMatcherConfig{
Host: host,
Port: port,
CaseInsensitive: caseInsensitiveRouting,
Resolver: resolver,
Host: host,
Port: port,
CaseInsensitive: caseInsensitiveRouting,
Resolver: resolver,
DisableUnqualifiedLookups: disableUnqualifiedLookups,
})
if err != nil {
return nil, trace.Wrap(err)
Expand Down

0 comments on commit aa568d4

Please sign in to comment.