Stephen Frost <sfr...@snowman.net> writes:
> On Sunday, January 3, 2016, Tom Lane <t...@sss.pgh.pa.us> wrote:
>> The fine manual says that when row_security is set to off, "queries fail
>> which would otherwise apply at least one policy". However, a look at
>> check_enable_rls() says that that is a true statement only when the user
>> is not table owner. If the user *is* table owner, turning off
>> row_security seems to amount to just silently disabling RLS, even for
>> tables with FORCE ROW LEVEL SECURITY.
>>
>> I am not sure if this is a documentation bug or a code bug, but it
>> sure looks to be one or the other.
> The original reason for changing how row_security works was to avoid a
> change in behavior based on a GUC changing. As such, I'm thinking that has
> to be a code bug, as otherwise it would be a behavior change due to a GUC
> being changed in the FORCE RLS case for table owners.
Well, I tried changing the code to act the way I gather it should, and
it breaks a whole bunch of regression test cases. See attached.
regards, tom lane
diff --git a/src/backend/utils/misc/rls.c b/src/backend/utils/misc/rls.c
index b6c1d75..4e7b4d1 100644
*** a/src/backend/utils/misc/rls.c
--- b/src/backend/utils/misc/rls.c
*************** check_enable_rls(Oid relid, Oid checkAsU
*** 59,67 ****
Oid user_id = checkAsUser ? checkAsUser : GetUserId();
/* Nothing to do for built-in relations */
! if (relid < FirstNormalObjectId)
return RLS_NONE;
tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
if (!HeapTupleIsValid(tuple))
return RLS_NONE;
--- 59,68 ----
Oid user_id = checkAsUser ? checkAsUser : GetUserId();
/* Nothing to do for built-in relations */
! if (relid < (Oid) FirstNormalObjectId)
return RLS_NONE;
+ /* Fetch relation's relrowsecurity and relforcerowsecurity flags */
tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
if (!HeapTupleIsValid(tuple))
return RLS_NONE;
*************** check_enable_rls(Oid relid, Oid checkAsU
*** 88,96 ****
return RLS_NONE_ENV;
/*
! * Table owners generally bypass RLS, except if row_security=true and the
! * table has been set (by an owner) to FORCE ROW SECURITY, and this is not
! * a referential integrity check.
*
* Return RLS_NONE_ENV to indicate that this decision depends on the
* environment (in this case, the user_id).
--- 89,97 ----
return RLS_NONE_ENV;
/*
! * Table owners generally bypass RLS, except if the table has been set (by
! * an owner) to FORCE ROW SECURITY, and this is not a referential
! * integrity check.
*
* Return RLS_NONE_ENV to indicate that this decision depends on the
* environment (in this case, the user_id).
*************** check_enable_rls(Oid relid, Oid checkAsU
*** 98,128 ****
if (pg_class_ownercheck(relid, user_id))
{
/*
! * If row_security=true and FORCE ROW LEVEL SECURITY has been set on
! * the relation then we return RLS_ENABLED to indicate that RLS should
! * still be applied. If we are in a SECURITY_NOFORCE_RLS context or if
! * row_security=false then we return RLS_NONE_ENV.
*
! * The SECURITY_NOFORCE_RLS indicates that we should not apply RLS even
! * if the table has FORCE RLS set- IF the current user is the owner.
! * This is specifically to ensure that referential integrity checks are
! * able to still run correctly.
*
* This is intentionally only done after we have checked that the user
* is the table owner, which should always be the case for referential
* integrity checks.
*/
! if (row_security && relforcerowsecurity && !InNoForceRLSOperation())
! return RLS_ENABLED;
! else
return RLS_NONE_ENV;
}
! /* row_security GUC says to bypass RLS, but user lacks permission */
if (!row_security && !noError)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("insufficient privilege to bypass row-level security")));
/* RLS should be fully enabled for this relation. */
return RLS_ENABLED;
--- 99,130 ----
if (pg_class_ownercheck(relid, user_id))
{
/*
! * If FORCE ROW LEVEL SECURITY has been set on the relation then we
! * should return RLS_ENABLED to indicate that RLS should be applied.
! * If not, or if we are in an InNoForceRLSOperation context, we return
! * RLS_NONE_ENV.
*
! * InNoForceRLSOperation indicates that we should not apply RLS even
! * if the table has FORCE RLS set - IF the current user is the owner.
! * This is specifically to ensure that referential integrity checks
! * are able to still run correctly.
*
* This is intentionally only done after we have checked that the user
* is the table owner, which should always be the case for referential
* integrity checks.
*/
! if (!relforcerowsecurity || InNoForceRLSOperation())
return RLS_NONE_ENV;
}
! /*
! * We should apply RLS. However, the user may turn off the row_security
! * GUC to get a forced error instead.
! */
if (!row_security && !noError)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("insufficient privilege to bypass row-level security")));
/* RLS should be fully enabled for this relation. */
return RLS_ENABLED;
*** /home/postgres/pgsql/src/test/regress/expected/rowsecurity.out Sat Dec
19 16:47:11 2015
--- /home/postgres/pgsql/src/test/regress/results/rowsecurity.out Sun Jan
3 21:56:18 2016
***************
*** 3217,3244 ****
SET row_security = off;
-- Shows all rows
TABLE r1;
! a
! ----
! 10
! 20
! (2 rows)
!
-- Update all rows
UPDATE r1 SET a = 1;
TABLE r1;
! a
! ---
! 1
! 1
! (2 rows)
!
-- Delete all rows
DELETE FROM r1;
TABLE r1;
! a
! ---
! (0 rows)
!
DROP TABLE r1;
--
-- FORCE ROW LEVEL SECURITY does not break RI
--- 3217,3233 ----
SET row_security = off;
-- Shows all rows
TABLE r1;
! ERROR: insufficient privilege to bypass row-level security
-- Update all rows
UPDATE r1 SET a = 1;
+ ERROR: insufficient privilege to bypass row-level security
TABLE r1;
! ERROR: insufficient privilege to bypass row-level security
-- Delete all rows
DELETE FROM r1;
+ ERROR: insufficient privilege to bypass row-level security
TABLE r1;
! ERROR: insufficient privilege to bypass row-level security
DROP TABLE r1;
--
-- FORCE ROW LEVEL SECURITY does not break RI
***************
*** 3351,3362 ****
SET row_security = off;
-- Rows shown now
TABLE r1;
! a
! ----
! 10
! 20
! (2 rows)
!
SET row_security = on;
-- Error
INSERT INTO r1 VALUES (10), (20) RETURNING *;
--- 3340,3346 ----
SET row_security = off;
-- Rows shown now
TABLE r1;
! ERROR: insufficient privilege to bypass row-level security
SET row_security = on;
-- Error
INSERT INTO r1 VALUES (10), (20) RETURNING *;
***************
*** 3379,3402 ****
-- Show updated rows
SET row_security = off;
TABLE r1;
! a
! ----
! 30
! (1 row)
!
-- reset value in r1 for test with RETURNING
UPDATE r1 SET a = 10;
-- Verify row reset
TABLE r1;
! a
! ----
! 10
! (1 row)
!
SET row_security = on;
-- Error
UPDATE r1 SET a = 30 RETURNING *;
! ERROR: new row violates row-level security policy for table "r1"
DROP TABLE r1;
-- Check dependency handling
RESET SESSION AUTHORIZATION;
--- 3363,3382 ----
-- Show updated rows
SET row_security = off;
TABLE r1;
! ERROR: insufficient privilege to bypass row-level security
-- reset value in r1 for test with RETURNING
UPDATE r1 SET a = 10;
+ ERROR: insufficient privilege to bypass row-level security
-- Verify row reset
TABLE r1;
! ERROR: insufficient privilege to bypass row-level security
SET row_security = on;
-- Error
UPDATE r1 SET a = 30 RETURNING *;
! a
! ---
! (0 rows)
!
DROP TABLE r1;
-- Check dependency handling
RESET SESSION AUTHORIZATION;
======================================================================
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
