optbuilder: gate crdb_internal builtins from unauthorized access

The epic below outlines a set of work to prevent users from unauthorized
access to what we deem unsafe internals. These unsafe internals lie
mostly in the crdb_internal schema and the system database. This PR
makes it so crdb_internal builtins are disallowed from external use
unless the specified override is enabled.

Fixes: #151501
Epic: CRDB-24527
Release note (ops change): restricts access to all crdb_internal builtins
unless session variable `allow_unsafe_internals` is set to true, or if
the caller is internal.

Author: angles-n-daemons

pkg/sql/authorization_test.go

@@ -311,7 +311,7 @@ func checkUnsafeErr(t *testing.T, err error) {
 	t.Fatal("expected unsafe access error, got", err)
 }
 
-func TestCheckUnsafeSystemAccess(t *testing.T) {
+func TestCheckUnsafeInternalsAccess(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
@@ -321,92 +321,105 @@ func TestCheckUnsafeSystemAccess(t *testing.T) {
 	})
 	defer s.Stopper().Stop(ctx)
 
-	q := "SELECT * FROM system.namespace"
-
-	t.Run("as an external querier", func(t *testing.T) {
-		for _, test := range []struct {
-			AllowUnsafeInternals bool
-			Passes               bool
-		}{
-			{AllowUnsafeInternals: false, Passes: false},
-			{AllowUnsafeInternals: true, Passes: true},
-		} {
-			t.Run(fmt.Sprintf("%t", test), func(t *testing.T) {
-				conn := s.SQLConn(t)
-				_, err := conn.Exec("SET allow_unsafe_internals = $1", test.AllowUnsafeInternals)
-				require.NoError(t, err)
-
-				_, err = conn.Query(q)
-				if test.Passes {
+	t.Run("accessing the system database", func(t *testing.T) {
+		q := "SELECT * FROM system.namespace"
+
+		t.Run("as an external querier", func(t *testing.T) {
+			for _, test := range []struct {
+				AllowUnsafeInternals bool
+				Passes               bool
+			}{
+				{AllowUnsafeInternals: false, Passes: false},
+				{AllowUnsafeInternals: true, Passes: true},
+			} {
+				t.Run(fmt.Sprintf("%t", test), func(t *testing.T) {
+					conn := s.SQLConn(t)
+					_, err := conn.Exec("SET allow_unsafe_internals = $1", test.AllowUnsafeInternals)
 					require.NoError(t, err)
-				} else {
-					checkUnsafeErr(t, err)
-				}
-			})
-		}
-	})
 
-	t.Run("as an internal querier", func(t *testing.T) {
-		for _, test := range []struct {
-			AllowUnsafeInternals bool
-			Passes               bool
-		}{
-			{AllowUnsafeInternals: false, Passes: true},
-			{AllowUnsafeInternals: true, Passes: true},
-		} {
-			t.Run(fmt.Sprintf("%t", test), func(t *testing.T) {
-				idb := s.InternalDB().(isql.DB)
-				err := idb.Txn(ctx, func(ctx context.Context, txn isql.Txn) error {
-					txn.SessionData().LocalOnlySessionData.AllowUnsafeInternals = test.AllowUnsafeInternals
-
-					_, err := txn.QueryBuffered(ctx, "internal-query", txn.KV(), q)
-					return err
+					_, err = conn.Query(q)
+					if test.Passes {
+						require.NoError(t, err)
+					} else {
+						checkUnsafeErr(t, err)
+					}
 				})
+			}
+		})
 
-				require.NoError(t, err)
+		t.Run("as an internal querier", func(t *testing.T) {
+			for _, test := range []struct {
+				AllowUnsafeInternals bool
+				Passes               bool
+			}{
+				{AllowUnsafeInternals: false, Passes: true},
+				{AllowUnsafeInternals: true, Passes: true},
+			} {
+				t.Run(fmt.Sprintf("%t", test), func(t *testing.T) {
+					idb := s.InternalDB().(isql.DB)
+					err := idb.Txn(ctx, func(ctx context.Context, txn isql.Txn) error {
+						txn.SessionData().LocalOnlySessionData.AllowUnsafeInternals = test.AllowUnsafeInternals
+
+						_, err := txn.QueryBuffered(ctx, "internal-query", txn.KV(), q)
+						return err
+					})
 
-				if test.Passes {
 					require.NoError(t, err)
-				} else {
-					checkUnsafeErr(t, err)
-				}
-			})
-		}
-	})
-
-}
-
-func TestCheckUnsafeCRDBInternalAccess(t *testing.T) {
-	defer leaktest.AfterTest(t)()
-	defer log.Scope(t).Close(t)
 
-	ctx := context.Background()
-	s := serverutils.StartServerOnly(t, base.TestServerArgs{
-		DefaultTestTenant: base.TestControlsTenantsExplicitly,
+					if test.Passes {
+						require.NoError(t, err)
+					} else {
+						checkUnsafeErr(t, err)
+					}
+				})
+			}
+		})
 	})
-	defer s.Stopper().Stop(ctx)
 
-	// Test that with allow_unsafe_internals = false:
-	// - Supported tables (zones) are allowed
-	// - Unsupported tables (gossip_alerts) are denied
+	t.Run("accessing the crdb_internal schema", func(t *testing.T) {
+		t.Run("supported table allowed", func(t *testing.T) {
+			conn := s.SQLConn(t)
+			_, err := conn.Exec("SET allow_unsafe_internals = false")
+			require.NoError(t, err)
 
-	t.Run("supported table allowed", func(t *testing.T) {
-		conn := s.SQLConn(t)
-		_, err := conn.Exec("SET allow_unsafe_internals = false")
-		require.NoError(t, err)
+			// Supported crdb_internal tables should be allowed even when allow_unsafe_internals = false
+			_, err = conn.Query("SELECT * FROM crdb_internal.zones")
+			require.NoError(t, err, "supported crdb_internal table (zones) should be accessible when allow_unsafe_internals = false")
+		})
 
-		// Supported crdb_internal tables should be allowed even when allow_unsafe_internals = false
-		_, err = conn.Query("SELECT * FROM crdb_internal.zones")
-		require.NoError(t, err, "supported crdb_internal table (zones) should be accessible when allow_unsafe_internals = false")
+		t.Run("unsupported table denied", func(t *testing.T) {
+			conn := s.SQLConn(t)
+			_, err := conn.Exec("SET allow_unsafe_internals = false")
+			require.NoError(t, err)
+
+			// Unsupported crdb_internal tables should be denied when allow_unsafe_internals = false
+			_, err = conn.Query("SELECT * FROM crdb_internal.gossip_alerts")
+			checkUnsafeErr(t, err)
+		})
 	})
 
-	t.Run("unsupported table denied", func(t *testing.T) {
-		conn := s.SQLConn(t)
-		_, err := conn.Exec("SET allow_unsafe_internals = false")
-		require.NoError(t, err)
+	// The functionality for this lies in the pkg/sql/optbuilder/scalar.go
+	// file, but it is tested here as that package does not setup a test
+	// server.
+	t.Run("accessing crdb_internal builtins", func(t *testing.T) {
+		t.Run("non crdb_internal builtin allowed", func(t *testing.T) {
+			conn := s.SQLConn(t)
+			_, err := conn.Exec("SET allow_unsafe_internals = false")
+			require.NoError(t, err)
+
+			// Non crdb_internal tables should be allowed.
+			_, err = conn.Query("SELECT * FROM generate_series(1,5)")
+			require.NoError(t, err)
+		})
+
+		t.Run("crdb_internal builtin not allowed", func(t *testing.T) {
+			conn := s.SQLConn(t)
+			_, err := conn.Exec("SET allow_unsafe_internals = false")
+			require.NoError(t, err)
 
-		// Unsupported crdb_internal tables should be denied when allow_unsafe_internals = false
-		_, err = conn.Query("SELECT * FROM crdb_internal.gossip_alerts")
-		checkUnsafeErr(t, err)
+			// Unsupported crdb_internal builtins should be denied.
+			_, err = conn.Query("SELECT * FROM crdb_internal.tenant_span_stats()")
+			checkUnsafeErr(t, err)
+		})
 	})
 }

pkg/sql/opt/optbuilder/BUILD.bazel

@@ -101,6 +101,7 @@ go_library(
         "//pkg/sql/sqltelemetry",
         "//pkg/sql/syntheticprivilege",
         "//pkg/sql/types",
+        "//pkg/sql/unsafesql",
         "//pkg/util",
         "//pkg/util/buildutil",
         "//pkg/util/errorutil",

pkg/sql/opt/optbuilder/scalar.go

@@ -20,13 +20,15 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree/treebin"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree/treecmp"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
+	"github.com/cockroachdb/cockroach/pkg/sql/unsafesql"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
 	"github.com/cockroachdb/errors"
@@ -547,6 +549,11 @@ func (b *Builder) buildFunction(
 	if overload.HasSQLBody() {
 		return b.buildUDF(f, def, inScope, outScope, outCol, colRefs)
 	}
+	if b.isUnsafeBuiltin(overload, def) {
+		if err := unsafesql.CheckInternalsAccess(b.evalCtx.SessionData()); err != nil {
+			panic(err)
+		}
+	}
 	b.factory.Metadata().AddBuiltin(f.Func.ReferenceByName)
 
 	if overload.Class == tree.AggregateClass {
@@ -878,6 +885,22 @@ func (b *Builder) constructUnary(
 	panic(errors.AssertionFailedf("unhandled unary operator: %s", redact.Safe(un)))
 }
 
+// isUnsafeBuiltin returns true if the given function definition
+// is a CRDB internal builtin function.
+func (b *Builder) isUnsafeBuiltin(
+	overload *tree.Overload, def *tree.ResolvedFunctionDefinition,
+) bool {
+	if overload.Type != tree.BuiltinRoutine {
+		return false
+	}
+	for _, o := range def.Overloads {
+		if o.Schema == catconstants.CRDBInternalSchemaName {
+			return true
+		}
+	}
+	return false
+}
+
 // ScalarBuilder is a specialized variant of Builder that can be used to create
 // a scalar from a TypedExpr. This is used to build scalar expressions for
 // testing. It is also used temporarily to interface with the old planning code.

pkg/sql/sem/eval/context.go

@@ -465,10 +465,14 @@ func MakeTestingEvalContext(st *cluster.Settings) Context {
 // MemoryMonitor. Ownership of the memory monitor is transferred to the
 // EvalContext so do not start or close the memory monitor.
 func MakeTestingEvalContextWithMon(st *cluster.Settings, monitor *mon.BytesMonitor) Context {
+	sessionData := &sessiondata.SessionData{}
+	// Set defaults that match what the session variables system expects.
+	// allow_unsafe_internals defaults to true.
+	sessionData.AllowUnsafeInternals = true
 	ctx := Context{
 		Codec:            keys.SystemSQLCodec,
 		Txn:              &kv.Txn{},
-		SessionDataStack: sessiondata.NewStack(&sessiondata.SessionData{}),
+		SessionDataStack: sessiondata.NewStack(sessionData),
 		Settings:         st,
 		NodeID:           base.TestingIDContainer,
 	}