Re: [HACKERS] Row Level Security UPDATE Confusion - Mailing list pgsql-hackers
From | Rod Taylor |
---|---|
Subject | Re: [HACKERS] Row Level Security UPDATE Confusion |
Date | |
Msg-id | CAHz80e51_MdsHii3weawcRs-_=ZHY20K_yJXiZSUzHKtTDLdmg@mail.gmail.com Whole thread Raw |
In response to | Re: [HACKERS] Row Level Security UPDATE Confusion (Rod Taylor <rod.taylor@gmail.com>) |
Responses |
Re: [HACKERS] Row Level Security UPDATE Confusion
|
List | pgsql-hackers |
On Fri, Apr 14, 2017 at 7:41 AM, Rod Taylor <rod.taylor@gmail.com> wrote:
On Thu, Apr 13, 2017 at 5:31 PM, Stephen Frost <sfrost@snowman.net> wrote:-- No error and value is -1 at the end.Rod, all,
* Joe Conway (mail@joeconway.com) wrote:
> On 04/13/2017 01:31 PM, Stephen Frost wrote:
> > * Robert Haas (robertmhaas@gmail.com) wrote:
> >> On Thu, Apr 6, 2017 at 4:05 PM, Rod Taylor <rod.taylor@gmail.com> wrote:
> >> > I'm a little confused on why a SELECT policy fires against the NEW record
> >> > for an UPDATE when using multiple FOR policies. The ALL policy doesn't seem
> >> > to have that restriction.
> >>
> >> My guess is that you have found a bug.
> >
> > Indeed. Joe's been looking into it and I'm hoping to find some time to
> > dig into it shortly.
>
> >> CREATE POLICY split_select ON t FOR SELECT TO split
> >> USING (value > 0);
> >> CREATE POLICY split_update ON t FOR UPDATE TO split
> >> USING (true) WITH CHECK (true);
>
> Yes -- from what I can see in gdb:
Actually, looking at this again, the complaint appears to be that you
can't "give away" records. That was a topic of much discussion and I'm
reasonably sure that was what we ended up deciding made the most sense.
You have to be able to see records to be able to update them (you can't
update records you can't see), and you have to be able to see the result
of your update. I don't doubt that we could improve the documentation
around this (and apparently the code comments, according to Joe..).Then there is a bug in the simpler statement which happily lets you give away records.
CREATE POLICY simple_all ON t TO simple USING (value > 0) WITH CHECK (true);
SET session authorization simple;
SELECT * FROM t;
UPDATE t SET value = value * -1 WHERE value = 1;
My actual use-case involves a range. Most users can see and manipulate the record when CURRENT_TIMESTAMP is within active_period. Some users (staff/account admins) can see recently dead records too. And a 3rd group (senior staff) have no time restriction, though there are a few customers they cannot see due to their information being a touch more sensitive. I've simplified the below rules to just deal with active_period and the majority of user view (@> CURRENT_TIMESTAMP).
NOTE: the active_period range is '[)' by default, so records with upper() = CURRENT_TIMESTAMP are not visible with @> CURRENT_TIMESTAMP restriction.
The below policy is allowed but requires that 1ms slop to accommodate the wi
Updated record invisible to USING but requires a trigger to enforce specific upper
and starting values. I have a trigger enforcing specific upper/lower values for the range
for specific ROLEs. So I had the thought that I might move ROLE specific trigger logic into
the RLS mechanism.
I tried to tighten the above rules, so INSERT must have upper of NULL and UPDATE must
set upper to exactly CURRENT_TIMESTAMP. Clearly I can achieve this using triggers for
enforcement but I tried to abuse RLS instead because it is a role specific restriction.
I was surprised when hide_old_select->USING was preventing the UPDATE when the simple single policy version let it through.
I'm happy to help with testing and documentation but first I need to understand what the intended functionality was. Right now it seems inconsistent between the simple single policy version and the multi policy version; the docs imply the single policy version is correct (it only seems to mention SELECT checks on RETURNING clauses).
NOTE: the active_period range is '[)' by default, so records with upper() = CURRENT_TIMESTAMP are not visible with @> CURRENT_TIMESTAMP restriction.
CREATE A TABLE t (id integer, active_period tstzrange NOT NULL DEFAULT tstzrange(current_timestamp, NULL));
The below policy is allowed but requires that 1ms slop to accommodate the wi
Updated record invisible to USING but requires a trigger to enforce specific upper
and starting values. I have a trigger enforcing specific upper/lower values for the range
for specific ROLEs. So I had the thought that I might move ROLE specific trigger logic into
the RLS mechanism.
CREATE POLICY hide_old ON t TO s;
USING ( active_period @> CURRENT_TIMESTAMP)
WITH CHECK ( active_period && tstzrange(current_timestamp - interval '0.001 seconds', current_timestamp, '[]'));
-- This is effectively a delete for the above policy. It becomes immediately invisible due to USING restriction.
UPDATE t SET active_period = tstzrange(lower(active_period), CURRENT_TIMESTAMP);
SELECT count(*) FROM t; -- 0 records
USING ( active_period @> CURRENT_TIMESTAMP)
WITH CHECK ( active_period && tstzrange(current_timestamp - interval '0.001 seconds', current_timestamp, '[]'));
-- This is effectively a delete for the above policy. It becomes immediately invisible due to USING restriction.
UPDATE t SET active_period = tstzrange(lower(active_period), CURRENT_TIMESTAMP);
SELECT count(*) FROM t; -- 0 records
I tried to tighten the above rules, so INSERT must have upper of NULL and UPDATE must
set upper to exactly CURRENT_TIMESTAMP. Clearly I can achieve this using triggers for
enforcement but I tried to abuse RLS instead because it is a role specific restriction.
I was surprised when hide_old_select->USING was preventing the UPDATE when the simple single policy version let it through.
CREATE POLICY hide_old_select ON t FOR SELECT TO s
USING ( active_period @> CURRENT_TIMESTAMP);
CREATE POLICY hide_old_insert ON t FOR INSERT to s
WITH CHECK ( lower(active_period) = CURRENT_TIMESTAMP AND upper(active_period) IS NULL);
CREATE POLICY hide_old_update ON t FOR UPDATE TO s
USING ( active_period @> CURRENT_TIMESTAMP)
WITH CHECK ( upper(active_period) = CURRENT_TIMESTAMP);
USING ( active_period @> CURRENT_TIMESTAMP);
CREATE POLICY hide_old_insert ON t FOR INSERT to s
WITH CHECK ( lower(active_period) = CURRENT_TIMESTAMP AND upper(active_period) IS NULL);
CREATE POLICY hide_old_update ON t FOR UPDATE TO s
USING ( active_period @> CURRENT_TIMESTAMP)
WITH CHECK ( upper(active_period) = CURRENT_TIMESTAMP);
-- Disallowed due to hide_old_select policy.
UPDATE t SET active_period = tstzrange(lower(active_period), CURRENT_TIMESTAMP);
I'm happy to help with testing and documentation but first I need to understand what the intended functionality was. Right now it seems inconsistent between the simple single policy version and the multi policy version; the docs imply the single policy version is correct (it only seems to mention SELECT checks on RETURNING clauses).
--
Rod Taylor
pgsql-hackers by date: