sql, server: add new builtin roles and 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.
The patch also introduces 3 new builtin roles:
crdb_internal_cluster_activity_reader which has
the system privilege VIEWACTIVITY.
crdb_internal_cluster_activity_writer which has
the system privilege CANCELQUERY.
crdb_internal_cluster_metadata_reader which has
the system privileges VIEWCLUSTERMETADATA,
VIEWCLUSTERSETTINGS, and VIEWDEBUG.

Resolves cockroachdb#17301, cockroachdb#17302, cockroachdb#17312, cockroachdb#17313, cockroachdb#17316

Release note (sql change): add VIEWDEBUG and
VIEWCLUSTERMETADATA system privileges. Add
cluster_activity_reader, cluster_activity_writer,
cluster_metadata_operator builtin roles.

docs/generated/sql/bnf/stmt_block.bnf

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

pkg/ccl/backupccl/backup_test.go

@@ -712,9 +712,17 @@ func TestBackupAndRestoreJobDescription(t *testing.T) {
 	full1 := strings.TrimPrefix(matches[0], "/full")
 	asOf1 := strings.TrimPrefix(matches[1], "/full")
 
+	updatingUsersString := "updating version for users table"
+	updatingRoleOptionsString := "updating version for role options table"
 	sqlDB.CheckQueryResults(
 		t, "SELECT description FROM [SHOW JOBS] WHERE status != 'failed'",
 		[][]string{
+			{updatingUsersString},
+			{updatingRoleOptionsString},
+			{updatingUsersString},
+			{updatingRoleOptionsString},
+			{updatingUsersString},
+			{updatingRoleOptionsString},
 			{fmt.Sprintf("BACKUP TO ('%s', '%s', '%s')", backups[0].(string), backups[1].(string),
 				backups[2].(string))},
 			{fmt.Sprintf("BACKUP TO ('%s', '%s', '%s') INCREMENTAL FROM '%s'", incrementals[0],
@@ -5604,11 +5612,19 @@ func TestBackupRestoreShowJob(t *testing.T) {
 	// run by an unrelated startup migration.
 	// TODO (lucy): Update this if/when we decide to change how these jobs queued by
 	// the startup migration are handled.
+	updatingUsersString := "updating version for users table"
+	updatingRoleOptionsString := "updating version for role options table"
 	sqlDB.CheckQueryResults(
 		t, "SELECT description FROM [SHOW JOBS] WHERE description != 'updating privileges' ORDER BY description",
 		[][]string{
 			{"BACKUP DATABASE data TO 'nodelocal://0/foo' WITH revision_history = true"},
 			{"RESTORE TABLE data.bank FROM 'nodelocal://0/foo' WITH into_db = 'data 2', skip_missing_foreign_keys"},
+			{updatingRoleOptionsString},
+			{updatingRoleOptionsString},
+			{updatingRoleOptionsString},
+			{updatingUsersString},
+			{updatingUsersString},
+			{updatingUsersString},
 		},
 	)
 }
@@ -9580,6 +9596,9 @@ func TestBackupRestoreSystemUsers(t *testing.T) {
 			{"admin", "", "true"},
 			{"app", "NULL", "false"},
 			{"app_role", "NULL", "true"},
+			{"crdb_internal_cluster_activity_reader", "NULL", "true"},
+			{"crdb_internal_cluster_activity_writer", "NULL", "true"},
+			{"crdb_internal_cluster_metadata_reader", "NULL", "true"},
 			{"root", "", "false"},
 			{"test", "NULL", "false"},
 			{"test_role", "NULL", "true"},
@@ -9594,6 +9613,9 @@ func TestBackupRestoreSystemUsers(t *testing.T) {
 			{"admin", "", "{}"},
 			{"app", "", "{admin,app_role}"},
 			{"app_role", "", "{}"},
+			{"crdb_internal_cluster_activity_reader", "NOLOGIN", "{}"},
+			{"crdb_internal_cluster_activity_writer", "NOLOGIN", "{}"},
+			{"crdb_internal_cluster_metadata_reader", "NOLOGIN", "{}"},
 			{"root", "", "{admin}"},
 			{"test", "", "{}"},
 			{"test_role", "", "{app_role}"},
@@ -9614,6 +9636,9 @@ func TestBackupRestoreSystemUsers(t *testing.T) {
 			{"admin", "", "true"},
 			{"app", "NULL", "false"},
 			{"app_role", "NULL", "true"},
+			{"crdb_internal_cluster_activity_reader", "NULL", "true"},
+			{"crdb_internal_cluster_activity_writer", "NULL", "true"},
+			{"crdb_internal_cluster_metadata_reader", "NULL", "true"},
 			{"root", "", "false"},
 			{"test", "NULL", "false"},
 			{"test_role", "NULL", "true"},
@@ -9625,6 +9650,9 @@ func TestBackupRestoreSystemUsers(t *testing.T) {
 			{"admin", "", "{}"},
 			{"app", "", "{}"},
 			{"app_role", "", "{}"},
+			{"crdb_internal_cluster_activity_reader", "NOLOGIN", "{}"},
+			{"crdb_internal_cluster_activity_writer", "NOLOGIN", "{}"},
+			{"crdb_internal_cluster_metadata_reader", "NOLOGIN", "{}"},
 			{"root", "", "{admin}"},
 			{"test", "", "{}"},
 			{"test_role", "", "{}"},

pkg/server/admin.go

@@ -1513,14 +1513,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 {
-		// 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)
+	err = s.requireViewClusterMetadataPermission(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	r, err := s.rangeLogHelper(ctx, req, user)
 	if err != nil {
 		return nil, serverError(ctx, err)
 	}
@@ -3535,6 +3538,50 @@ func (c *adminPrivilegeChecker) requireViewActivityAndNoViewActivityRedactedPerm
 	return nil
 }
 
+// requireViewClusterMetadataPermission requires the user have 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 {
+		hasViewClusterMetadata := false
+		if c.st.Version.IsActive(ctx, clusterversion.SystemPrivilegesTable) {
+			hasViewClusterMetadata = c.checkHasSystemPrivilege(ctx, userName, privilege.VIEWCLUSTERMETADATA)
+		}
+		if !hasViewClusterMetadata {
+			return status.Errorf(
+				codes.PermissionDenied, "this operation requires the %s system privilege",
+				privilege.VIEWCLUSTERMETADATA)
+		}
+	}
+	return nil
+}
+
+// requireViewDebugPermission requires the user have 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 {
+		hasViewDebug := false
+		if c.st.Version.IsActive(ctx, clusterversion.SystemPrivilegesTable) {
+			hasViewDebug = c.checkHasSystemPrivilege(ctx, userName, privilege.VIEWDEBUG)
+		}
+		if !hasViewDebug {
+			return status.Errorf(
+				codes.PermissionDenied, "this operation requires the %s system privilege",
+				privilege.VIEWCLUSTERMETADATA)
+		}
+	}
+	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(
@@ -3630,7 +3677,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
 	}
@@ -3657,7 +3704,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
 	}
@@ -3701,7 +3748,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
 	}
@@ -3760,7 +3807,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

@@ -2828,6 +2828,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) {
@@ -2838,10 +2852,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
@@ -2852,6 +2872,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/create_role.go

@@ -98,7 +98,8 @@ func (p *planner) CreateRoleNode(
 		return nil, err
 	}
 	// Reject the reserved roles.
-	if roleName.IsReserved() {
+	user := p.SessionData().User()
+	if roleName.IsReserved() && !user.IsNodeUser() {
 		return nil, pgerror.Newf(
 			pgcode.ReservedName,
 			"role name %q is reserved",