release 25-2: sql: check view owner privileges when selecting from a view#166743
Conversation
|
Thanks for opening a backport. Before merging, please confirm that it falls into one of the following categories (select one):
Add a brief release justification to the PR description explaining your selection. Also, confirm that the change does not break backward compatibility and complies with all aspects of the backport policy. All backports must be reviewed by the TL and EM for the owning area. |
|
😎 Merged successfully - details. |
|
❌ PR #166743 does not comply with backport policy Confidence: medium ✅ ENGREQ Check Passed: No ENGREQ required (non-production code or serious issues). 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
79adcdd to
791b78c
Compare
Previously, when a user selected from a view, privilege checks on the view's underlying tables were either entirely. This diverged from PostgreSQL, where the view owner's (definer's) privileges are used to access underlying tables, while the invoker only needs SELECT on the view itself. Fixed this by introducing definer privilege checking in the builder: 1. When building a view's query, set dataSourcePrivilegeUserOverride to the view owner so that SELECT privilege on underlying tables is checked as the definer. A SkipUnderlyingViewPrivilegeChecks cluster setting was added to preserve the current behavior. 2. Store the definer's username in the privilege dependency key (privilegeKey) so that memo re-validation checks the correct user. For invoker contexts such as udfs, an empty username is stored and re-validation uses the current session user via GetCurrentUser(). This avoids unnecessary memo invalidation on SET ROLE. 3. Separate EXECUTE privilege checks from data source privilege checks. Views override data source privileges to the view owner, but EXECUTE privilege on functions called by the view is still checked against the invoker, matching PostgreSQL. SECURITY DEFINER routines set both overrides to the routine owner. SECURITY INVOKER routines reset the data source override back to the invoker. Informs: cockroachdb#164426 Release note (bug fix): When selecting from a view, the view owner's privileges on the underlying tables are not checked. This means no privilege checks are performed on the underlying tables, so a view would continue to work even after the owner lost access to the underlying tables. This also affects row-level security (RLS): the view invoker's RLS policies are enforced instead of the view owner's. If you need to enforce privilege checks on the underlying tables, you can set cluster setting `sql.auth.skip_underlying_view_privilege_checks.enabled` to false. This setting was enabled by default to prevent backward incompatible behavior.
791b78c to
c98ec5f
Compare
spilchen
left a comment
There was a problem hiding this comment.
@spilchen made 1 comment.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on shghasemi and ZhouXing19).
docs/generated/settings/settings.html line 215 at r1 (raw file):
<tr><td><div id="setting-sql-auth-grant-option-inheritance-enabled" class="anchored"><code>sql.auth.grant_option_inheritance.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>determines whether the GRANT OPTION for privileges is inherited through role membership</td><td>Serverless/Dedicated/Self-Hosted</td></tr> <tr><td><div id="setting-sql-auth-public-schema-create-privilege-enabled" class="anchored"><code>sql.auth.public_schema_create_privilege.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>determines whether to grant all users the CREATE privileges on the public schema when it is created</td><td>Serverless/Dedicated/Self-Hosted</td></tr> <tr><td><div id="setting-sql-auth-skip-underlying-view-privilege-checks-enabled" class="anchored"><code>sql.auth.skip_underlying_view_privilege_checks.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>determines whether to skip privilege checks on tables underlying views. When enabled, users with SELECT privileges on a view can query it regardless of their privileges on the underlying tables, and row-level security policies are evaluated as the invoking user rather than the view owner. This restores pre-v26.2 behavior.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
nit: is this comment about pre-v26.2 behaviour accurate? This is a backport for 25.2. So, we are changing the pre-v26.2 behaviour
spilchen
left a comment
There was a problem hiding this comment.
@spilchen made 2 comments and resolved 1 discussion.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on ZhouXing19).
docs/generated/settings/settings.html line 215 at r1 (raw file):
Previously, spilchen wrote…
nit: is this comment about pre-v26.2 behaviour accurate? This is a backport for 25.2. So, we are changing the pre-v26.2 behaviour
Clarified here: #166744 (review)
|
TFTR! /trunk merge |
NOTE TO REVIEWERS: This backport had logic conflicts in
pkg/sql/opt/metadata.go. It's because this PR was part of 26.1 and only backported to 25.4.sql: check view owner privileges when selecting from a view
Previously, when a user selected from a view, privilege checks on the
view's underlying tables were entirely skipped. This diverged
from PostgreSQL, where the view owner's (definer's) privileges are used
to access underlying tables, while the invoker only needs SELECT on the
view itself.
Fixed this by introducing definer privilege checking in the builder:
When building a view's query, set dataSourcePrivilegeUserOverride to
the view owner so that SELECT privilege on underlying tables is
checked as the definer. A SkipUnderlyingViewPrivilegeChecks cluster
setting was added to preserve the current behavior.
Store the definer's username in the privilege dependency key
(privilegeKey) so that memo re-validation checks the correct user.
For invoker contexts such as udfs, an empty username is stored and
re-validation uses the current session user via GetCurrentUser().
This avoids unnecessary memo invalidation on SET ROLE.
Separate EXECUTE privilege checks from data source privilege checks.
Views override data source privileges to the view owner, but
EXECUTE privilege on functions called by the view is still checked
against the invoker, matching PostgreSQL. SECURITY DEFINER routines
set both overrides to the routine owner. SECURITY INVOKER routines
reset the data source override back to the invoker.
Informs: #164426
Release note (bug fix): When selecting from a view,
the view owner's privileges on the underlying tables are not checked.
This means no privilege checks are performed on the underlying tables,
so a view would continue to work even after the owner lost access to the
underlying tables. This also affects row-level security (RLS): the view
invoker's RLS policies are enforced instead of the view owner's. If you
need to enforce privilege checks on the underlying tables, you can set
cluster setting sql.auth.skip_underlying_view_privilege_checks.enabled
to false. This setting was enabled by default to prevent backward
incompatible behavior.
Release justification: Security bug fix. Changes are gated by a cluster setting that is off by default.