sql,server: fix the listing of sessions for admin/root users

Admin users should be able to list all sessions not just their own.
This was broken in cockroachdb#32253 and thus in 2.1.1 (was ok in 2.1.0 and
prior). This patch fixes this and also clarifies the code responsible.

Release note (bug fix): CockroachDB now again enables admin users,
including `root`, to list all user sessions besides their own.

pkg/server/status.go

@@ -1352,31 +1352,35 @@ func (s *statusServer) ListLocalSessions(
 		return nil, remoteDebuggingErr
 	}
 
-	var showAll bool
 	sessionUser, err := userFromContext(ctx)
 	if err != nil {
 		return nil, err
 	}
-	// If the client is asking to see more than just their own sessions, verify
-	// that they should be allowed to.
-	if sessionUser != req.Username {
-		superuser := (sessionUser == security.RootUser || s.isSuperUser(ctx, sessionUser))
-		if req.Username != "" && !superuser {
+
+	superuser := s.isSuperUser(ctx, sessionUser)
+	if !superuser {
+		// For non-superusers, requests with an empty username is
+		// implicitly a request for the client's own sessions.
+		if req.Username == "" {
+			req.Username = sessionUser
+		}
+
+		// Non-superusers are not allowed to query sessions others than their own.
+		if sessionUser != req.Username {
 			return nil, grpcstatus.Errorf(
 				codes.PermissionDenied,
 				"client user %q does not have permission to view sessions from user %q",
 				sessionUser, req.Username)
 		}
-		// If the user isn't a superuser, then a request with an empty username is
-		// implicitly a request for the client's own sessions.
-		if req.Username == "" && !superuser {
-			req.Username = sessionUser
-		}
-		showAll = (req.Username == "" && superuser)
 	}
 
-	registry := s.sessionRegistry
+	// There are two ways to query "all sessions":
+	// - an empty username in the request (subject to the checks above).
+	// - the request is internal (coming from crdb_internal.{node,cluster}_sessions)
+	//   and the requested user is a superuser.
+	showAll := req.Username == ""
 
+	registry := s.sessionRegistry
 	sessions := registry.SerializeAll()
 	userSessions := make([]serverpb.Session, 0, len(sessions))
 
@@ -1743,6 +1747,9 @@ type superUserChecker interface {
 }
 
 func (s *statusServer) isSuperUser(ctx context.Context, username string) bool {
+	if username == security.RootUser {
+		return true
+	}
 	planner, cleanup := sql.NewInternalPlanner(
 		"check-superuser",
 		client.NewTxn(ctx, s.db, s.gossip.NodeID.Get(), client.RootTxn),

pkg/sql/crdb_internal.go

@@ -734,12 +734,21 @@ CREATE TABLE crdb_internal.%s (
 );
 `
 
+func (p *planner) makeSessionsRequest(ctx context.Context) serverpb.ListSessionsRequest {
+	req := serverpb.ListSessionsRequest{Username: p.SessionData().User}
+	if err := p.RequireSuperUser(ctx, "list sessions"); err == nil {
+		// The root user can see all sessions.
+		req.Username = ""
+	}
+	return req
+}
+
 // crdbInternalLocalQueriesTable exposes the list of running queries
 // on the current node. The results are dependent on the current user.
 var crdbInternalLocalQueriesTable = virtualSchemaTable{
 	schema: fmt.Sprintf(queriesSchemaPattern, "node_queries"),
 	populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error {
-		req := serverpb.ListSessionsRequest{Username: p.SessionData().User}
+		req := p.makeSessionsRequest(ctx)
 		response, err := p.extendedEvalCtx.StatusServer.ListLocalSessions(ctx, &req)
 		if err != nil {
 			return err
@@ -753,7 +762,7 @@ var crdbInternalLocalQueriesTable = virtualSchemaTable{
 var crdbInternalClusterQueriesTable = virtualSchemaTable{
 	schema: fmt.Sprintf(queriesSchemaPattern, "cluster_queries"),
 	populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error {
-		req := serverpb.ListSessionsRequest{Username: p.SessionData().User}
+		req := p.makeSessionsRequest(ctx)
 		response, err := p.extendedEvalCtx.StatusServer.ListSessions(ctx, &req)
 		if err != nil {
 			return err
@@ -836,7 +845,7 @@ CREATE TABLE crdb_internal.%s (
 var crdbInternalLocalSessionsTable = virtualSchemaTable{
 	schema: fmt.Sprintf(sessionsSchemaPattern, "node_sessions"),
 	populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error {
-		req := serverpb.ListSessionsRequest{Username: p.SessionData().User}
+		req := p.makeSessionsRequest(ctx)
 		response, err := p.extendedEvalCtx.StatusServer.ListLocalSessions(ctx, &req)
 		if err != nil {
 			return err
@@ -850,7 +859,7 @@ var crdbInternalLocalSessionsTable = virtualSchemaTable{
 var crdbInternalClusterSessionsTable = virtualSchemaTable{
 	schema: fmt.Sprintf(sessionsSchemaPattern, "cluster_sessions"),
 	populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error {
-		req := serverpb.ListSessionsRequest{Username: p.SessionData().User}
+		req := p.makeSessionsRequest(ctx)
 		response, err := p.extendedEvalCtx.StatusServer.ListSessions(ctx, &req)
 		if err != nil {
 			return err

pkg/sql/pgwire/conn.go

@@ -254,6 +254,10 @@ func (c *conn) serveImpl(
 		if err := sendStatusParam("session_authorization", c.sessionArgs.User); err != nil {
 			return err
 		}
+
+		// TODO(knz): this should retrieve the admin status during
+		// authentication using the roles table, instead of using a
+		// simple/naive username match.
 		isSuperUser := c.sessionArgs.User == security.RootUser
 		superUserVal := "off"
 		if isSuperUser {

pkg/sql/show_test.go

@@ -19,6 +19,7 @@ import (
 	gosql "database/sql"
 	"fmt"
 	"math"
+	"net/url"
 	"reflect"
 	"strings"
 	"testing"
@@ -736,6 +737,80 @@ func TestShowSessions(t *testing.T) {
 	}
 }
 
+func TestShowSessionPrivileges(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	params, _ := tests.CreateTestServerParams()
+	params.Insecure = true
+	s, rawSQLDBroot, _ := serverutils.StartServer(t, params)
+	sqlDBroot := sqlutils.MakeSQLRunner(rawSQLDBroot)
+	defer s.Stopper().Stop(context.TODO())
+
+	// Prepare a non-root session.
+	_ = sqlDBroot.Exec(t, `CREATE USER nonroot`)
+	pgURL := url.URL{
+		Scheme:   "postgres",
+		User:     url.User("nonroot"),
+		Host:     s.ServingAddr(),
+		RawQuery: "sslmode=disable",
+	}
+	rawSQLDBnonroot, err := gosql.Open("postgres", pgURL.String())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer rawSQLDBnonroot.Close()
+	sqlDBnonroot := sqlutils.MakeSQLRunner(rawSQLDBnonroot)
+
+	// Ensure the non-root session is open.
+	sqlDBnonroot.Exec(t, `SELECT version()`)
+
+	t.Run("root", func(t *testing.T) {
+		// Verify that the root session can use SHOW SESSIONS properly and
+		// can observe other sessions than its own.
+		rows := sqlDBroot.Query(t, `SELECT user_name FROM [SHOW CLUSTER SESSIONS]`)
+		defer rows.Close()
+		count := 0
+		foundNonRoot := false
+		for rows.Next() {
+			var userName string
+			if err := rows.Scan(&userName); err != nil {
+				t.Fatal(err)
+			}
+			if userName == "nonroot" {
+				foundNonRoot = true
+			}
+			count++
+		}
+		if count == 0 {
+			t.Fatal("root session is unable to see its own session")
+		}
+		if !foundNonRoot {
+			t.Fatal("root session is unable to see non-root session")
+		}
+	})
+
+	t.Run("non-root", func(t *testing.T) {
+		// Verify that the non-root session can use SHOW SESSIONS properly
+		// and cannot observe other sessions than its own.
+		rows := sqlDBnonroot.Query(t, `SELECT user_name FROM [SHOW CLUSTER SESSIONS]`)
+		defer rows.Close()
+		count := 0
+		for rows.Next() {
+			var userName string
+			if err := rows.Scan(&userName); err != nil {
+				t.Fatal(err)
+			}
+			if userName != "nonroot" {
+				t.Fatalf("non-root session is able to see other users: %q", userName)
+			}
+			count++
+		}
+		if count == 0 {
+			t.Fatal("non-root session is unable to see its own session")
+		}
+	})
+}
+
 // TestShowJobs manually inserts a row into system.jobs and checks that the
 // encoded protobuf payload is properly decoded and visible in
 // crdb_internal.jobs.