Merge #152532

152532: unsafesql: report break glass usage of unsafe internals to the sql_exec log 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 adds a log to SENSITIVE_ACCESS to audit each time the operator breaks glass and bypasses these access controls.

There is refactoring as part of this PR, specifically the transformation of a statement to a redactable string (`FormatAstAsRedactableString`) is pulled out of the sql package and into the tree package, so that it could be leveraged by the optbuilder package.

Fixes: #151488
Epic: CRDB-24527
Release note (ops change): A log will be emitted now when users override the unsafesql safety gate.

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

Author: craig[bot]

docs/generated/eventlog.md

@@ -853,6 +853,56 @@ a table marked as audited.
 | `BulkJobId` | The job id for bulk job (IMPORT/BACKUP/RESTORE). | no |
 | `StmtPosInTxn` | The statement's index in the transaction, starting at 1. | no |
 
+### `unsafe_internals_accessed`
+
+UnsafeInternalsAccess is recorded when a query accesses unsafe internals
+using the allow_unsafe_internals override.
+
+
+| Field | Description | Sensitive |
+|--|--|--|
+| `Query` | The query that triggered the unsafe internals access. | partially |
+
+
+#### Common fields
+
+| Field | Description | Sensitive |
+|--|--|--|
+| `Timestamp` | The timestamp of the event. Expressed as nanoseconds since the Unix epoch. | no |
+| `EventType` | The type of the event. | no |
+| `Statement` | A normalized copy of the SQL statement that triggered the event. The statement string contains a mix of sensitive and non-sensitive details (it is redactable). | partially |
+| `Tag` | The statement tag. This is separate from the statement string, since the statement string can contain sensitive information. The tag is guaranteed not to. | no |
+| `User` | The user account that triggered the event. The special usernames `root` and `node` are not considered sensitive. | depends |
+| `DescriptorID` | The primary object descriptor affected by the operation. Set to zero for operations that don't affect descriptors. | no |
+| `ApplicationName` | The application name for the session where the event was emitted. This is included in the event to ease filtering of logging output by application. | no |
+| `PlaceholderValues` | The mapping of SQL placeholders to their values, for prepared statements. | yes |
+| `TxnReadTimestamp` | The current read timestamp of the transaction that triggered the event, if in a transaction. | no |
+
+### `unsafe_internals_denied`
+
+An event of type `unsafe_internals_denied` is recorded when a query attempts to access unsafe internals
+but lacks the appropriate session variables.
+
+
+| Field | Description | Sensitive |
+|--|--|--|
+| `Query` | The query that triggered the unsafe internals access. | partially |
+
+
+#### Common fields
+
+| Field | Description | Sensitive |
+|--|--|--|
+| `Timestamp` | The timestamp of the event. Expressed as nanoseconds since the Unix epoch. | no |
+| `EventType` | The type of the event. | no |
+| `Statement` | A normalized copy of the SQL statement that triggered the event. The statement string contains a mix of sensitive and non-sensitive details (it is redactable). | partially |
+| `Tag` | The statement tag. This is separate from the statement string, since the statement string can contain sensitive information. The tag is guaranteed not to. | no |
+| `User` | The user account that triggered the event. The special usernames `root` and `node` are not considered sensitive. | depends |
+| `DescriptorID` | The primary object descriptor affected by the operation. Set to zero for operations that don't affect descriptors. | no |
+| `ApplicationName` | The application name for the session where the event was emitted. This is included in the event to ease filtering of logging output by application. | no |
+| `PlaceholderValues` | The mapping of SQL placeholders to their values, for prepared statements. | yes |
+| `TxnReadTimestamp` | The current read timestamp of the transaction that triggered the event, if in a transaction. | no |
+
 ## SQL Execution Log
 
 Events in this category report executed queries.

pkg/cli/interactive_tests/test_audit_log.tcl

@@ -9,14 +9,9 @@ eexpect root@
 
 set logfile logs/db/logs/cockroach-sql-audit.log
 
-start_test "Check that the audit log is not created by default"
-system "if test -e $logfile; then false; fi"
-end_test
-
 start_test "Check that statements do not get logged to the audit log directly"
 send "CREATE DATABASE t; USE t; CREATE TABLE helloworld(abc INT) WITH (schema_locked=false); INSERT INTO helloworld VALUES (123);\r"
 eexpect root@
-system "if test -e $logfile; then false; fi"
 end_test
 
 start_test "Check that statements start being logged synchronously if auditing is enabled"

pkg/sql/authorization.go

@@ -125,7 +125,7 @@ func (p *planner) HasPrivilege(
 	// 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 {
+		if err := unsafesql.CheckInternalsAccess(ctx, p.SessionData(), p.stmt.AST, p.extendedEvalCtx.Annotations, &p.ExecCfg().Settings.SV); err != nil {
 			return false, err
 		}
 	}

pkg/sql/copy_from.go

@@ -1248,6 +1248,12 @@ func (c *copyMachine) insertRowsInternal(ctx context.Context, finalBatch bool) (
 		Returning: tree.AbsentReturningClause,
 	}
 
+	// Initialize annotations for the statement. This is required for proper
+	// error handling and logging during plan optimization. Since this is a
+	// synthetic INSERT statement, we provide an empty annotations array.
+	c.p.semaCtx.Annotations = tree.MakeAnnotations(0)
+	c.p.extendedEvalCtx.Annotations = &c.p.semaCtx.Annotations
+
 	// TODO(cucaroach): We shouldn't need to do this for every batch.
 	if err := c.p.makeOptimizerPlan(ctx); err != nil {
 		return err

pkg/sql/exec_util.go

@@ -4455,38 +4455,11 @@ func scrubStmtStatKey(vt VirtualTabler, key string, ns eval.ClientNoticeSender)
 	return f.CloseAndGetString(), true
 }
 
-var redactNamesInSQLStatementLog = settings.RegisterBoolSetting(
-	settings.ApplicationLevel,
-	"sql.log.redact_names.enabled",
-	"if set, schema object identifers are redacted in SQL statements that appear in event logs",
-	false,
-	settings.WithPublic,
-)
-
 // FormatAstAsRedactableString implements scbuild.AstFormatter
 func (p *planner) FormatAstAsRedactableString(
 	statement tree.Statement, annotations *tree.Annotations,
 ) redact.RedactableString {
-	fs := tree.FmtSimple | tree.FmtAlwaysQualifyTableNames | tree.FmtMarkRedactionNode
-	if !redactNamesInSQLStatementLog.Get(&p.extendedEvalCtx.Settings.SV) {
-		fs = fs | tree.FmtOmitNameRedaction
-	}
-	return formatStmtKeyAsRedactableString(statement, annotations, fs)
-}
-
-// formatStmtKeyAsRedactableString given an AST node this function will fully
-// qualify names using annotations to format it out into a redactable string.
-// Object names are not redacted, but constants and datums are.
-func formatStmtKeyAsRedactableString(
-	rootAST tree.Statement, ann *tree.Annotations, fs tree.FmtFlags,
-) redact.RedactableString {
-	f := tree.NewFmtCtx(
-		fs,
-		tree.FmtAnnotations(ann),
-	)
-	f.FormatNode(rootAST)
-	formattedRedactableStatementString := f.CloseAndGetString()
-	return redact.RedactableString(formattedRedactableStatementString)
+	return tree.FormatAstAsRedactableString(statement, annotations, &p.extendedEvalCtx.Settings.SV)
 }
 
 // FailedHashedValue is used as a default return value for when HashForReporting

pkg/sql/opt/optbuilder/builder.go

@@ -504,7 +504,7 @@ func (b *Builder) buildStmt(
 			// invalidation). We don't care about caching plans for these statements.
 			b.DisableMemoReuse = true
 			// It's considered acceptable when we delegate to unsafe internals.
-			defer b.disableUnsafeInternalCheck()()
+			defer b.DisableUnsafeInternalCheck()()
 			return b.buildStmt(newStmt, desiredTypes, inScope)
 		}
 
@@ -590,15 +590,20 @@ func (b *Builder) maybeTrackUserDefinedTypeDepsForViews(texpr tree.TypedExpr) {
 	}
 }
 
-// disableUnsafeInternalCheck is used to disable the check that the
+// DisableUnsafeInternalCheck is used to disable the check that the
 // prevents external users from accessing unsafe internals.
-func (b *Builder) disableUnsafeInternalCheck() func() {
+func (b *Builder) DisableUnsafeInternalCheck() func() {
 	b.skipUnsafeInternalsCheck = true
-	cleanup := b.catalog.DisableUnsafeInternalCheck()
+	var cleanup func()
+	if b.catalog != nil {
+		cleanup = b.catalog.DisableUnsafeInternalCheck()
+	}
 
 	return func() {
 		b.skipUnsafeInternalsCheck = false
-		cleanup()
+		if cleanup != nil {
+			cleanup()
+		}
 	}
 }
 

pkg/sql/opt/optbuilder/scalar.go

@@ -550,7 +550,7 @@ func (b *Builder) buildFunction(
 		return b.buildUDF(f, def, inScope, outScope, outCol, colRefs)
 	}
 	if b.isUnsafeBuiltin(overload, def) {
-		if err := unsafesql.CheckInternalsAccess(b.evalCtx.SessionData()); err != nil {
+		if err := unsafesql.CheckInternalsAccess(b.ctx, b.evalCtx.SessionData(), b.stmt, b.evalCtx.Annotations, &b.evalCtx.Settings.SV); err != nil {
 			panic(err)
 		}
 	}

pkg/sql/opt/testutils/opttester/opt_tester.go

@@ -2316,6 +2316,7 @@ func (ot *OptTester) buildExpr(factory *norm.Factory) error {
 	ot.semaCtx.Placeholders.Init(stmt.NumPlaceholders, nil /* typeHints */)
 	ot.semaCtx.Annotations = tree.MakeAnnotations(stmt.NumAnnotations)
 	ot.semaCtx.TypeResolver = ot.catalog
+	ot.evalCtx.Annotations = &ot.semaCtx.Annotations
 	b := optbuilder.New(ot.ctx, &ot.semaCtx, &ot.evalCtx, ot.catalog, factory, stmt.AST)
 	return b.Build()
 }

pkg/sql/sem/eval/eval_test.go

@@ -95,6 +95,7 @@ func optBuildScalar(evalCtx *eval.Context, e tree.Expr) (tree.TypedExpr, error)
 	o.Init(ctx, evalCtx, nil /* catalog */)
 	semaCtx := tree.MakeSemaContext(nil /* resolver */)
 	b := optbuilder.NewScalar(ctx, &semaCtx, evalCtx, o.Factory())
+	defer b.DisableUnsafeInternalCheck()()
 	scalar, err := b.Build(e)
 	if err != nil {
 		return nil, err

pkg/sql/sem/tree/BUILD.bazel

@@ -86,6 +86,7 @@ go_library(
         "prepare.go",
         "pretty.go",
         "reassign_owned_by.go",
+        "redact_ast.go",
         "regexp_cache.go",
         "region.go",
         "rename.go",
@@ -140,6 +141,7 @@ go_library(
         "//pkg/geo",
         "//pkg/geo/geopb",
         "//pkg/kv/kvserver/concurrency/isolation",
+        "//pkg/settings",
         "//pkg/sql/lex",
         "//pkg/sql/lexbase",
         "//pkg/sql/oidext",