diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 186a226b853cd..e7d8541d22749 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -1458,6 +1458,7 @@ func (a *Server) runPeriodicOperations() { }() case heartbeatCheckKey: go func() { + var invalidNodeHostnames []string req := &proto.ListUnifiedResourcesRequest{Kinds: []string{types.KindNode}, SortBy: types.SortBy{Field: types.ResourceKind}} for { @@ -1467,9 +1468,14 @@ func (a *Server) runPeriodicOperations() { if !ok { return false, nil } + if services.NodeHasMissedKeepAlives(srv) { missedKeepAliveCount++ } + if err := utils.ValidateNodeHostname(srv.GetHostname()); err != nil { + invalidNodeHostnames = append(invalidNodeHostnames, srv.GetHostname()) + } + return false, nil }, req, @@ -1487,6 +1493,52 @@ func (a *Server) runPeriodicOperations() { // Update prometheus gauge heartbeatsMissedByAuth.Set(float64(missedKeepAliveCount)) + + // Send a notification that nodes with invalid hostames have been found + // to users that can update nodes + var msgBuilder strings.Builder + msgBuilder.WriteString(`Nodes have been found that are configured with an invalid hostname. Future versions of Teleport will change the hostname of nodes with invalid hostnames. +Please update these hostnames to hostnames only consisting of alphanumeric characters and the symbols '.' and '-'. +The nodes that contain invalid hostnames are as follows: `) + for i, hostname := range invalidNodeHostnames { + msgBuilder.WriteString(strconv.Quote(hostname)) + if i != len(invalidNodeHostnames)-1 { + msgBuilder.WriteString(", ") + } + } + + notif := ¬ificationsv1.GlobalNotification{ + Spec: ¬ificationsv1.GlobalNotificationSpec{ + Matcher: ¬ificationsv1.GlobalNotificationSpec_ByPermissions{ + ByPermissions: ¬ificationsv1.ByPermissions{ + RoleConditions: []*types.RoleConditions{ + { + Rules: []types.Rule{ + { + Resources: []string{types.KindNode}, + Verbs: []string{types.VerbUpdate}, + }, + }, + }, + }, + }, + }, + Notification: ¬ificationsv1.Notification{ + SubKind: types.NotificationDefaultWarningSubKind, + Metadata: &headerv1.Metadata{ + Labels: map[string]string{ + types.NotificationTitleLabel: "Found nodes with invalid hostnames", + types.NotificationTextContentLabel: msgBuilder.String(), + }, + }, + Spec: ¬ificationsv1.NotificationSpec{}, + }, + }, + } + _, err := a.CreateGlobalNotification(a.closeCtx, notif) + if err != nil { + log.Errorf("Failed to send a global notification about nodes with invalid hostnames: %v", err) + } }() case metricsKey: go a.updateAgentMetrics() diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 931dd4178a1e1..413b851bab7ce 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -62,6 +62,7 @@ import ( "github.com/gravitational/teleport/lib/services/local" "github.com/gravitational/teleport/lib/session" "github.com/gravitational/teleport/lib/tlsca" + "github.com/gravitational/teleport/lib/utils" ) // ServerWithRoles is a wrapper around auth service @@ -976,6 +977,11 @@ func (a *ServerWithRoles) UpsertNode(ctx context.Context, s types.Server) (*type if err := a.action(s.GetNamespace(), types.KindNode, types.VerbCreate, types.VerbUpdate); err != nil { return nil, trace.Wrap(err) } + if hostname := s.GetHostname(); hostname != "" { + if err := utils.ValidateNodeHostname(hostname); err != nil { + return nil, trace.Wrap(err) + } + } return a.authServer.UpsertNode(ctx, s) } diff --git a/lib/auth/auth_with_roles_test.go b/lib/auth/auth_with_roles_test.go index a040e444083da..3701e60491e57 100644 --- a/lib/auth/auth_with_roles_test.go +++ b/lib/auth/auth_with_roles_test.go @@ -9108,3 +9108,49 @@ func TestCloudDefaultPasswordless(t *testing.T) { }) } } + +// TestUpsertInvalidNodeHostname tests that creating a node with +// an invalid hostname results in an error. +func TestUpsertInvalidNodeHostname(t *testing.T) { + t.Parallel() + ctx := context.Background() + srv, err := NewTestAuthServer(TestAuthServerConfig{Dir: t.TempDir()}) + require.NoError(t, err) + + rules := []types.Rule{ + { + Resources: []string{types.KindNode}, + Verbs: []string{types.VerbCreate, types.VerbUpdate}, + }, + } + user, _, err := CreateUserAndRole(srv.AuthServer, "test-user", nil, rules) + require.NoError(t, err) + + authContext, err := srv.Authorizer.Authorize(authz.ContextWithUser(ctx, TestUser(user.GetName()).I)) + require.NoError(t, err, trace.DebugReport(err)) + s := &ServerWithRoles{ + authServer: srv.AuthServer, + alog: srv.AuditLog, + context: *authContext, + } + + name := uuid.NewString() + node, err := types.NewServerWithLabels( + name, + types.KindNode, + types.ServerSpecV2{ + Hostname: "my amazing incredible spectacular node", + }, + map[string]string{"name": name}, + ) + require.NoError(t, err) + + _, err = s.UpsertNode(ctx, node) + require.ErrorContains(t, err, "Invalid hostname") + + nodev2 := node.(*types.ServerV2) + nodev2.Spec.Hostname = "valid-hostname" + + _, err = s.UpsertNode(ctx, node) + require.NoError(t, err) +} diff --git a/lib/services/local/presence.go b/lib/services/local/presence.go index 43fd2b4ae73df..bcdf7d7d58653 100644 --- a/lib/services/local/presence.go +++ b/lib/services/local/presence.go @@ -348,6 +348,14 @@ func (s *PresenceService) UpsertNode(ctx context.Context, server types.Server) ( if n := server.GetNamespace(); n != apidefaults.Namespace { return nil, trace.BadParameter("cannot place node in namespace %q, custom namespaces are deprecated", n) } + if err := utils.ValidateNodeHostname(server.GetHostname()); err != nil { + s.log.Warnf( + `Node %q is configured with an invalid hostname. Future versions of Teleport will change the hostname of nodes with invalid hostnames. +Please update this hostname to a hostname only consisting of alphanumeric characters and the symbols '.' and '-'.`, + server.GetHostname(), + ) + } + rev := server.GetRevision() value, err := services.MarshalServer(server) if err != nil { diff --git a/lib/srv/discovery/discovery.go b/lib/srv/discovery/discovery.go index 095da62b6475f..46fd4a016260c 100644 --- a/lib/srv/discovery/discovery.go +++ b/lib/srv/discovery/discovery.go @@ -61,6 +61,7 @@ import ( aws_sync "github.com/gravitational/teleport/lib/srv/discovery/fetchers/aws-sync" "github.com/gravitational/teleport/lib/srv/discovery/fetchers/db" "github.com/gravitational/teleport/lib/srv/server" + "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/utils/spreadwork" ) @@ -945,6 +946,15 @@ func (s *Server) heartbeatEICEInstance(instances *server.EC2Instances) { continue } + // Only validate the hostname of new nodes to ensure existing nodes that existed + // before hostname validation was checked are unaffected + if len(existingNodes) == 0 { + err := utils.ValidateNodeHostname(eiceNode.GetHostname()) + if err != nil { + s.Log.Warnf("Error validating the hostname %q of node with name %q: %v", eiceNode.GetHostname(), eiceNode.GetName(), err) + continue + } + } eiceNodeExpiration := s.clock.Now().Add(s.jitter(serverExpirationDuration)) eiceNode.SetExpiry(eiceNodeExpiration) diff --git a/lib/utils/utils.go b/lib/utils/utils.go index b1931e2ae8cf4..cd0d2133b0735 100644 --- a/lib/utils/utils.go +++ b/lib/utils/utils.go @@ -29,6 +29,7 @@ import ( "net/url" "os" "path/filepath" + "regexp" "runtime" "sort" "strconv" @@ -323,6 +324,30 @@ func IsValidHostname(hostname string) bool { return true } +const ( + nodeHostnameMaxLen = 256 + nodeHostnameRegexPattern = `^[a-zA-Z0-9]([\.-]?[a-zA-Z0-9]+)*$` +) + +var nodeHostnameRegex = regexp.MustCompile(nodeHostnameRegexPattern) + +// ValidateNodeHostname returns an error if the node hostname does not entirely +// consist of alphanumeric characters as well as '-' and '.'. A valid hostname also +// cannot begin with a symbol, and a symbol cannot be followed immediately by another symbol. +func ValidateNodeHostname(hostname string) error { + if len(hostname) > nodeHostnameMaxLen { + return trace.Errorf("Invalid hostname %q. Valid node hostnames must be under %d characters.", hostname, nodeHostnameMaxLen) + } + if !nodeHostnameRegex.MatchString(hostname) { + return trace.Errorf( + `Invalid hostname %q. Valid node hostnames consist of alphanumeric characters as well as the symbols '-' and '.' +The hostname cannot begin with a symbol, and a symbol cannot be followed immediately by another symbol.`, + hostname, + ) + } + return nil +} + // IsValidUnixUser checks if a string represents a valid // UNIX username. func IsValidUnixUser(u string) bool { diff --git a/lib/utils/utils_test.go b/lib/utils/utils_test.go index 1ff85e1ff8d31..ccb66e2b99611 100644 --- a/lib/utils/utils_test.go +++ b/lib/utils/utils_test.go @@ -358,6 +358,61 @@ func TestIsValidHostname(t *testing.T) { } } +func TestValidateNodeHostname(t *testing.T) { + t.Parallel() + tests := []struct { + name string + hostname string + assert require.ErrorAssertionFunc + }{ + { + name: "normal hostname", + hostname: "some-host-1.example.com", + assert: require.NoError, + }, + { + name: "one component", + hostname: "example", + assert: require.NoError, + }, + { + name: "empty", + hostname: "", + assert: require.Error, + }, + { + name: "invalid characters", + hostname: "some spaces.example.com", + assert: require.Error, + }, + { + name: "empty label", + hostname: "somewhere..example.com", + assert: require.Error, + }, + { + name: "hostname too long", + hostname: strings.Repeat("x.", nodeHostnameMaxLen) + ".example.com", + assert: require.Error, + }, + { + name: "uuid", + hostname: "9b61981f-d5c3-491d-8e58-be500db71d54", + assert: require.NoError, + }, + { + name: "uuid with domain name", + hostname: "9b61981f-d5c3-491d-8e58-be500db71d54.example.com", + assert: require.NoError, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tc.assert(t, ValidateNodeHostname(tc.hostname)) + }) + } +} + // TestReplaceRegexp tests regexp-style replacement of values func TestReplaceRegexp(t *testing.T) { t.Parallel() diff --git a/lib/web/ui/perf_test.go b/lib/web/ui/perf_test.go index 863b0370065ee..0e8ef92ea9e84 100644 --- a/lib/web/ui/perf_test.go +++ b/lib/web/ui/perf_test.go @@ -126,8 +126,9 @@ func insertServers(ctx context.Context, b *testing.B, svc services.Presence, kin Labels: labels, }, Spec: types.ServerSpecV2{ - Addr: addr, - Version: teleport.Version, + Addr: addr, + Hostname: name, + Version: teleport.Version, }, } var err error