validate node hostnames when they are being created or updated

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 := &notificationsv1.GlobalNotification{
+						Spec: &notificationsv1.GlobalNotificationSpec{
+							Matcher: &notificationsv1.GlobalNotificationSpec_ByPermissions{
+								ByPermissions: &notificationsv1.ByPermissions{
+									RoleConditions: []*types.RoleConditions{
+										{
+											Rules: []types.Rule{
+												{
+													Resources: []string{types.KindNode},
+													Verbs:     []string{types.VerbUpdate},
+												},
+											},
+										},
+									},
+								},
+							},
+							Notification: &notificationsv1.Notification{
+								SubKind: types.NotificationDefaultWarningSubKind,
+								Metadata: &headerv1.Metadata{
+									Labels: map[string]string{
+										types.NotificationTitleLabel:       "Found nodes with invalid hostnames",
+										types.NotificationTextContentLabel: msgBuilder.String(),
+									},
+								},
+								Spec: &notificationsv1.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()

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)
 }
 

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)
+}

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 {

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)

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 {

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()

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