Merge #151804

151804: sql: gate access to crdb_internal tables based on session var r=angles-n-daemons a=angles-n-daemons

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 that crdb_internal access is limited to a slim set of
"allowed" internal objects.

Fixes: #151803
Epic: CRDB-24527
Release note (ops change): restricts access to all but allowed list of internal
tables in the crdb_internal schema if the session variable
`allow_unsafe_internals` is set to true, or if the caller is internal.

Co-authored-by: Brian Dillmann <[email protected]>

Author: craig[bot]

pkg/sql/authorization.go

@@ -122,13 +122,11 @@ func (p *planner) HasPrivilege(
 		return false, errors.AssertionFailedf("cannot use CheckPrivilege without a txn")
 	}
 
-	// Check for system table access restrictions before any admin bypasses
-	if d, ok := privilegeObject.(catalog.TableDescriptor); ok {
-		// Check for system table access restrictions before any admin bypasses
-		if catalog.IsSystemDescriptor(d) {
-			if err := unsafesql.CheckInternalsAccess(p.SessionData()); err != nil {
-				return false, err
-			}
+	// Do a safety check on the object, if it is considered unsafe
+	// does the caller have the appropriate session data to access it?
+	if p.objectIsUnsafe(ctx, privilegeObject) {
+		if err := unsafesql.CheckInternalsAccess(p.SessionData()); err != nil {
+			return false, err
 		}
 	}
 
@@ -951,6 +949,29 @@ func (p *planner) HasViewActivityOrViewActivityRedactedRole(
 	return false, false, nil
 }
 
+// objectIsUnsafe checks if the privilege object is considered unsafe for external usage.
+// Unsafe objects are any system tables, and crdb_internal tables which are not listed as externally supported.
+func (p *planner) objectIsUnsafe(ctx context.Context, privilegeObject privilege.Object) bool {
+	d, ok := privilegeObject.(catalog.TableDescriptor)
+	if !ok {
+		return true
+	}
+
+	// All system descriptors are considered unsafe.
+	if catalog.IsSystemDescriptor(d) {
+		return true
+	}
+
+	// Unsupported crdb_internal tables are considered unsafe.
+	if d.GetParentSchemaID() == catconstants.CrdbInternalID {
+		if _, ok := SupportedCRDBInternalTables[d.GetName()]; !ok {
+			return true
+		}
+	}
+
+	return false
+}
+
 func insufficientPrivilegeError(
 	user username.SQLUsername, kind privilege.Kind, object privilege.Object,
 ) error {

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 TestCheckUnsafeInternalsAccess(t *testing.T) {
+func TestCheckUnsafeSystemAccess(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
@@ -375,3 +375,38 @@ func TestCheckUnsafeInternalsAccess(t *testing.T) {
 	})
 
 }
+
+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,
+	})
+	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("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")
+	})
+
+	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)
+	})
+}

pkg/sql/crdb_internal.go

@@ -235,29 +235,29 @@ var crdbInternal = virtualSchema{
 	validWithNoDatabaseContext: true,
 }
 
-// SupportedVTables are the crdb_internal tables that are "supported" for real
+// SupportedCRDBInternal are the crdb_internal tables that are "supported" for real
 // customer use in production for legacy reasons. Avoid adding to this list if
 // possible and prefer to add new customer-facing tables that should be public
 // under the non-"internal" namespace of information_schema.
-var SupportedVTables = map[string]struct{}{
-	`"".crdb_internal.cluster_contended_indexes`:     {},
-	`"".crdb_internal.cluster_contended_keys`:        {},
-	`"".crdb_internal.cluster_contended_tables`:      {},
-	`"".crdb_internal.cluster_contention_events`:     {},
-	`"".crdb_internal.cluster_locks`:                 {},
-	`"".crdb_internal.cluster_queries`:               {},
-	`"".crdb_internal.cluster_sessions`:              {},
-	`"".crdb_internal.cluster_transactions`:          {},
-	`"".crdb_internal.index_usage_statistics`:        {},
-	`"".crdb_internal.statement_statistics`:          {},
-	`"".crdb_internal.transaction_contention_events`: {},
-	`"".crdb_internal.transaction_statistics`:        {},
-	`"".crdb_internal.zones`:                         {},
+var SupportedCRDBInternalTables = map[string]struct{}{
+	`cluster_contended_indexes`:     {},
+	`cluster_contended_keys`:        {},
+	`cluster_contended_tables`:      {},
+	`cluster_contention_events`:     {},
+	`cluster_locks`:                 {},
+	`cluster_queries`:               {},
+	`cluster_sessions`:              {},
+	`cluster_transactions`:          {},
+	`index_usage_statistics`:        {},
+	`statement_statistics`:          {},
+	`transaction_contention_events`: {},
+	`transaction_statistics`:        {},
+	`zones`:                         {},
 }
 
 // Note that this map is currently unused but serves to document which vtables
 // are expected to be used in production setting.
-var _ = SupportedVTables
+var _ = SupportedCRDBInternalTables
 
 var crdbInternalBuildInfoTable = virtualSchemaTable{
 	comment: `detailed identification strings (RAM, local node only)`,