sql, server: add new system privileges

for observability

This patch introduces 2 new system privileges
VIEWDEBUG and VIEWCLUSTERMETADATA. VIEWDEBUG
will now be used to gate taking traces and viewing
debug endpoints. VIEWCLUSTERMETADATA will now be
used to gate the node and range reports.

Resolves #83844, #83856, #83857, #83858, #83861

Release note (sql change): add VIEWDEBUG and
VIEWCLUSTERMETADATA system privileges.

docs/generated/sql/bnf/stmt_block.bnf

@@ -1359,7 +1359,9 @@ unreserved_keyword ::=
 	| 'VIEW'
 	| 'VIEWACTIVITY'
 	| 'VIEWACTIVITYREDACTED'
+	| 'VIEWCLUSTERMETADATA'
 	| 'VIEWCLUSTERSETTING'
+	| 'VIEWDEBUG'
 	| 'VISIBLE'
 	| 'VOLATILE'
 	| 'VOTERS'

pkg/server/admin.go

@@ -1515,14 +1515,17 @@ func (s *adminServer) RangeLog(
 	ctx = s.server.AnnotateCtx(ctx)
 
 	// Range keys, even when pretty-printed, contain PII.
-	userName, err := s.requireAdminUser(ctx)
+	user, _, err := s.getUserAndRole(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	err = s.requireViewClusterMetadataPermission(ctx)
 	if err != nil {
-		// NB: not using serverError() here since the priv checker
-		// already returns a proper gRPC error status.
 		return nil, err
 	}
 
-	r, err := s.rangeLogHelper(ctx, req, userName)
+	r, err := s.rangeLogHelper(ctx, req, user)
 	if err != nil {
 		return nil, serverError(ctx, err)
 	}
@@ -3543,6 +3546,50 @@ func (c *adminPrivilegeChecker) requireViewActivityAndNoViewActivityRedactedPerm
 	return nil
 }
 
+// requireViewClusterMetadataPermission requires the user have admin or the VIEWCLUSTERMETADATA
+// system privilege and returns an error if the user does not have it.
+func (c *adminPrivilegeChecker) requireViewClusterMetadataPermission(
+	ctx context.Context,
+) (err error) {
+	userName, isAdmin, err := c.getUserAndRole(ctx)
+	if err != nil {
+		return serverError(ctx, err)
+	}
+	if !isAdmin {
+		if c.st.Version.IsActive(ctx, clusterversion.SystemPrivilegesTable) {
+			if hasViewClusterMetadata := c.checkHasSystemPrivilege(ctx, userName, privilege.VIEWCLUSTERMETADATA); !hasViewClusterMetadata {
+				return status.Errorf(
+					codes.PermissionDenied, "this operation requires the %s system privilege",
+					privilege.VIEWCLUSTERMETADATA)
+			}
+		} else {
+			return status.Error(codes.PermissionDenied, "this operation requires admin privilege")
+		}
+	}
+	return nil
+}
+
+// requireViewDebugPermission requires the user have admin or the VIEWDEBUG system privilege
+// and returns an error if the user does not have it.
+func (c *adminPrivilegeChecker) requireViewDebugPermission(ctx context.Context) (err error) {
+	userName, isAdmin, err := c.getUserAndRole(ctx)
+	if err != nil {
+		return serverError(ctx, err)
+	}
+	if !isAdmin {
+		if c.st.Version.IsActive(ctx, clusterversion.SystemPrivilegesTable) {
+			if hasViewDebug := c.checkHasSystemPrivilege(ctx, userName, privilege.VIEWDEBUG); !hasViewDebug {
+				return status.Errorf(
+					codes.PermissionDenied, "this operation requires the %s system privilege",
+					privilege.VIEWDEBUG)
+			}
+		} else {
+			return status.Error(codes.PermissionDenied, "this operation requires admin privilege")
+		}
+	}
+	return nil
+}
+
 // Note that the function returns plain errors, and it is the caller's
 // responsibility to convert them to serverErrors.
 func (c *adminPrivilegeChecker) getUserAndRole(
@@ -3638,7 +3685,7 @@ func (s *adminServer) ListTracingSnapshots(
 	ctx context.Context, req *serverpb.ListTracingSnapshotsRequest,
 ) (*serverpb.ListTracingSnapshotsResponse, error) {
 	ctx = s.server.AnnotateCtx(ctx)
-	_, err := s.requireAdminUser(ctx)
+	err := s.requireViewDebugPermission(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -3665,7 +3712,7 @@ func (s *adminServer) TakeTracingSnapshot(
 	ctx context.Context, req *serverpb.TakeTracingSnapshotRequest,
 ) (*serverpb.TakeTracingSnapshotResponse, error) {
 	ctx = s.server.AnnotateCtx(ctx)
-	_, err := s.requireAdminUser(ctx)
+	err := s.requireViewDebugPermission(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -3709,7 +3756,7 @@ func (s *adminServer) GetTracingSnapshot(
 	ctx context.Context, req *serverpb.GetTracingSnapshotRequest,
 ) (*serverpb.GetTracingSnapshotResponse, error) {
 	ctx = s.server.AnnotateCtx(ctx)
-	_, err := s.requireAdminUser(ctx)
+	err := s.requireViewDebugPermission(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -3768,7 +3815,7 @@ func (s *adminServer) GetTrace(
 	ctx context.Context, req *serverpb.GetTraceRequest,
 ) (*serverpb.GetTraceResponse, error) {
 	ctx = s.server.AnnotateCtx(ctx)
-	_, err := s.requireAdminUser(ctx)
+	err := s.requireViewDebugPermission(ctx)
 	if err != nil {
 		return nil, err
 	}

pkg/server/admin_test.go

@@ -2834,6 +2834,20 @@ func TestAdminPrivilegeChecker(t *testing.T) {
 				withAdmin: false, withVa: false, withVaRedacted: true, withVaAndRedacted: true, withoutPrivs: true,
 			},
 		},
+		{
+			"requireViewClusterMetadataPermission",
+			underTest.requireViewClusterMetadataPermission,
+			map[username.SQLUsername]bool{
+				withAdmin: false, withoutPrivs: true,
+			},
+		},
+		{
+			"requireViewDebugPermission",
+			underTest.requireViewDebugPermission,
+			map[username.SQLUsername]bool{
+				withAdmin: false, withoutPrivs: true,
+			},
+		},
 	}
 	// test system privileges if valid version
 	if s.ClusterSettings().Version.IsActive(ctx, clusterversion.SystemPrivilegesTable) {
@@ -2844,10 +2858,16 @@ func TestAdminPrivilegeChecker(t *testing.T) {
 		sqlDB.Exec(t, "CREATE USER withvaandredactedsystemprivilege")
 		sqlDB.Exec(t, "GRANT SYSTEM VIEWACTIVITY TO withvaandredactedsystemprivilege")
 		sqlDB.Exec(t, "GRANT SYSTEM VIEWACTIVITYREDACTED TO withvaandredactedsystemprivilege")
+		sqlDB.Exec(t, "CREATE USER withviewclustermetadata")
+		sqlDB.Exec(t, "GRANT SYSTEM VIEWCLUSTERMETADATA TO withviewclustermetadata")
+		sqlDB.Exec(t, "CREATE USER withviewdebug")
+		sqlDB.Exec(t, "GRANT SYSTEM VIEWDEBUG TO withviewdebug")
 
 		withVaSystemPrivilege := username.MakeSQLUsernameFromPreNormalizedString("withvasystemprivilege")
 		withVaRedactedSystemPrivilege := username.MakeSQLUsernameFromPreNormalizedString("withvaredactedsystemprivilege")
 		withVaAndRedactedSystemPrivilege := username.MakeSQLUsernameFromPreNormalizedString("withvaandredactedsystemprivilege")
+		withviewclustermetadata := username.MakeSQLUsernameFromPreNormalizedString("withviewclustermetadata")
+		withViewDebug := username.MakeSQLUsernameFromPreNormalizedString("withviewdebug")
 
 		tests[0].usernameWantErr[withVaSystemPrivilege] = false
 		tests[1].usernameWantErr[withVaSystemPrivilege] = false
@@ -2858,6 +2878,8 @@ func TestAdminPrivilegeChecker(t *testing.T) {
 		tests[0].usernameWantErr[withVaAndRedactedSystemPrivilege] = false
 		tests[1].usernameWantErr[withVaAndRedactedSystemPrivilege] = false
 		tests[2].usernameWantErr[withVaAndRedactedSystemPrivilege] = true
+		tests[3].usernameWantErr[withviewclustermetadata] = false
+		tests[4].usernameWantErr[withViewDebug] = false
 
 	}
 	for _, tt := range tests {

pkg/server/server_http.go

@@ -185,13 +185,9 @@ func makeAdminAuthzCheckHandler(
 		md := forwardAuthenticationMetadata(req.Context(), req)
 		authCtx := metadata.NewIncomingContext(req.Context(), md)
 		// Check the privileges of the requester.
-		_, err := adminAuthzCheck.requireAdminUser(authCtx)
-		if errors.Is(err, errRequiresAdmin) {
-			http.Error(w, "admin privilege required", http.StatusUnauthorized)
-			return
-		} else if err != nil {
-			log.Ops.Infof(authCtx, "web session error: %s", err)
-			http.Error(w, "error checking authentication", http.StatusInternalServerError)
+		err := adminAuthzCheck.requireViewDebugPermission(authCtx)
+		if err != nil {
+			http.Error(w, "admin privilege or VIEWDEBUG system privilege required", http.StatusUnauthorized)
 			return
 		}
 		// Forward the request to the inner handler.

pkg/server/status.go

@@ -800,9 +800,8 @@ func (s *statusServer) AllocatorRange(
 	ctx = propagateGatewayMetadata(ctx)
 	ctx = s.AnnotateCtx(ctx)
 
-	if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil {
-		// NB: not using serverError() here since the priv checker
-		// already returns a proper gRPC error status.
+	err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx)
+	if err != nil {
 		return nil, err
 	}
 
@@ -1453,7 +1452,7 @@ func (s *statusServer) Nodes(
 	ctx = propagateGatewayMetadata(ctx)
 	ctx = s.AnnotateCtx(ctx)
 
-	err := s.privilegeChecker.requireViewActivityPermission(ctx)
+	err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -1471,14 +1470,14 @@ func (s *statusServer) NodesUI(
 	ctx = propagateGatewayMetadata(ctx)
 	ctx = s.AnnotateCtx(ctx)
 
-	hasViewActivity := false
-	err := s.privilegeChecker.requireViewActivityPermission(ctx)
+	hasViewClusterMetadata := false
+	err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx)
 	if err != nil {
 		if !grpcutil.IsAuthError(err) {
 			return nil, err
 		}
 	} else {
-		hasViewActivity = true
+		hasViewClusterMetadata = true
 	}
 
 	internalResp, _, err := s.nodesHelper(ctx, 0 /* limit */, 0 /* offset */)
@@ -1490,13 +1489,13 @@ func (s *statusServer) NodesUI(
 		LivenessByNodeID: internalResp.LivenessByNodeID,
 	}
 	for i, nodeStatus := range internalResp.Nodes {
-		resp.Nodes[i] = nodeStatusToResp(&nodeStatus, hasViewActivity)
+		resp.Nodes[i] = nodeStatusToResp(&nodeStatus, hasViewClusterMetadata)
 	}
 
 	return resp, nil
 }
 
-func nodeStatusToResp(n *statuspb.NodeStatus, hasViewActivity bool) serverpb.NodeResponse {
+func nodeStatusToResp(n *statuspb.NodeStatus, hasViewClusterMetadata bool) serverpb.NodeResponse {
 	tiers := make([]serverpb.Tier, len(n.Desc.Locality.Tiers))
 	for j, t := range n.Desc.Locality.Tiers {
 		tiers[j] = serverpb.Tier{
@@ -1552,7 +1551,7 @@ func nodeStatusToResp(n *statuspb.NodeStatus, hasViewActivity bool) serverpb.Nod
 			sfsprops := &roachpb.FileStoreProperties{
 				FsType: fsprops.FsType,
 			}
-			if hasViewActivity {
+			if hasViewClusterMetadata {
 				sfsprops.Path = fsprops.Path
 				sfsprops.BlockDevice = fsprops.BlockDevice
 				sfsprops.MountPoint = fsprops.MountPoint
@@ -1577,7 +1576,7 @@ func nodeStatusToResp(n *statuspb.NodeStatus, hasViewActivity bool) serverpb.Nod
 		NumCpus:           n.NumCpus,
 	}
 
-	if hasViewActivity {
+	if hasViewClusterMetadata {
 		resp.Args = n.Args
 		resp.Env = n.Env
 		resp.Desc.Attrs = n.Desc.Attrs
@@ -1916,7 +1915,8 @@ func (s *statusServer) rangesHelper(
 	ctx = propagateGatewayMetadata(ctx)
 	ctx = s.AnnotateCtx(ctx)
 
-	if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil {
+	err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx)
+	if err != nil {
 		return nil, 0, err
 	}
 

pkg/sql/parser/sql.y

@@ -932,8 +932,8 @@ func (u *sqlSymUnion) functionObjs() tree.FuncObjs {
 %token <str> UNBOUNDED UNCOMMITTED UNION UNIQUE UNKNOWN UNLOGGED UNSPLIT
 %token <str> UPDATE UPSERT UNSET UNTIL USE USER USERS USING UUID
 
-%token <str> VALID VALIDATE VALUE VALUES VARBIT VARCHAR VARIADIC VIEW VARYING VIEWACTIVITY VIEWACTIVITYREDACTED
-%token <str> VIEWCLUSTERSETTING VIRTUAL VISIBLE VOLATILE VOTERS
+%token <str> VALID VALIDATE VALUE VALUES VARBIT VARCHAR VARIADIC VIEW VARYING VIEWACTIVITY VIEWACTIVITYREDACTED VIEWDEBUG
+%token <str> VIEWCLUSTERMETADATA VIEWCLUSTERSETTING VIRTUAL VISIBLE VOLATILE VOTERS
 
 %token <str> WHEN WHERE WINDOW WITH WITHIN WITHOUT WORK WRITE
 
@@ -15198,7 +15198,9 @@ unreserved_keyword:
 | VIEW
 | VIEWACTIVITY
 | VIEWACTIVITYREDACTED
+| VIEWCLUSTERMETADATA
 | VIEWCLUSTERSETTING
+| VIEWDEBUG
 | VISIBLE
 | VOLATILE
 | VOTERS

pkg/sql/privilege/privilege.go

@@ -53,6 +53,8 @@ const (
 	CANCELQUERY          Kind = 18
 	NOSQLLOGIN           Kind = 19
 	EXECUTE              Kind = 20
+	VIEWCLUSTERMETADATA  Kind = 21
+	VIEWDEBUG            Kind = 22
 )
 
 // Privilege represents a privilege parsed from an Access Privilege Inquiry
@@ -120,7 +122,7 @@ var (
 	// certain privileges unavailable after upgrade migration.
 	// Note that "CREATE, INSERT, DELETE, ZONECONFIG" are no-op privileges on sequences.
 	SequencePrivileges           = List{ALL, USAGE, SELECT, UPDATE, CREATE, DROP, INSERT, DELETE, ZONECONFIG}
-	SystemPrivileges             = List{ALL, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN}
+	SystemPrivileges             = List{ALL, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG}
 	VirtualTablePrivileges       = List{ALL, SELECT}
 	ExternalConnectionPrivileges = List{ALL, USAGE}
 )
@@ -137,8 +139,7 @@ func (k Kind) IsSetIn(bits uint32) bool {
 
 // ByValue is just an array of privilege kinds sorted by value.
 var ByValue = [...]Kind{
-	ALL, CREATE, DROP, SELECT, INSERT, DELETE, UPDATE, USAGE, ZONECONFIG, CONNECT, RULE, MODIFYCLUSTERSETTING,
-	EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, EXECUTE,
+	ALL, CREATE, DROP, SELECT, INSERT, DELETE, UPDATE, USAGE, ZONECONFIG, CONNECT, RULE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, EXECUTE, VIEWCLUSTERMETADATA, VIEWDEBUG,
 }
 
 // ByName is a map of string -> kind value.
@@ -162,6 +163,8 @@ var ByName = map[string]Kind{
 	"CANCELQUERY":          CANCELQUERY,
 	"NOSQLLOGIN":           NOSQLLOGIN,
 	"EXECUTE":              EXECUTE,
+	"VIEWCLUSTERMETADATA":  VIEWCLUSTERMETADATA,
+	"VIEWDEBUG":            VIEWDEBUG,
 }
 
 // List is a list of privileges.