Postgres trigger side-effect is occurring out of order with row-level security select policy

2018-09-29 Thread Carl Sverre
*Context*
I am using row-level security along with triggers to implement a pure SQL
RBAC implementation. While doing so I encountered a weird behavior between
INSERT triggers and SELECT row-level security policies.

*Question*
I have posted a very detailed question on StackOverflow here:
https://stackoverflow.com/questions/52565720/postgres-trigger-side-effect-is-occurring-out-of-order-with-row-level-security-s

For anyone who is just looking for a summary/repro, I am seeing the
following behavior:

CREATE TABLE a (id TEXT);
ALTER TABLE a ENABLE ROW LEVEL SECURITY;
ALTER TABLE a FORCE ROW LEVEL SECURITY;

CREATE TABLE b (id TEXT);

CREATE POLICY ON a FOR SELECT
USING (EXISTS(
select * from b where a.id = b.id
));

CREATE POLICY ON a FOR INSERT
WITH CHECK (true);

CREATE FUNCTION reproHandler() RETURNS TRIGGER AS $$
BEGIN
RAISE NOTICE USING MESSAGE = 'inside trigger handler';
INSERT INTO b (id) VALUES (NEW.id);
RETURN NEW;
END;
$$ LANGUAGE plpgsql;

CREATE TRIGGER reproTrigger BEFORE INSERT ON a
FOR EACH ROW EXECUTE PROCEDURE reproHandler();

INSERT INTO a VALUES ('fails') returning id;
NOTICE:  inside trigger handler
ERROR:  new row violates row-level security policy for table "a"

Rather than the error, I expect that something along these lines should
occur instead:

1. A new row ('fails') is staged for INSERT
2. The BEFORE trigger fires with NEW set to the new row
3. The row ('fails') is inserted into b and returned from the trigger
procedure unchanged
4. The INSERT's WITH CHECK policy true is evaluated to true
5. The SELECT's USING policy select * from b where a.id = b.id is
evaluated.  *This should return true due to step 3*
6. Having passed all policies, the row ('fails') is inserted in table
7. The id (fails) of the inserted row is returned

If anyone can point me in the right direction I would be extremely thankful.

Carl Sverre

http://www.carlsverre.com


Re: Postgres trigger side-effect is occurring out of order with row-level security select policy

2018-10-01 Thread Carl Sverre
Thanks for the initial results. Can you check that you are not using super
permissions and are enabling row security when running the test? Super
ignores row security.

Also yes, I forgot to add the policy names, sorry about that.
On Sun, Sep 30, 2018 at 1:34 AM Charles Clavadetscher (SwissPUG) <
clavadetsc...@swisspug.org> wrote:

> Hello
>
>
> On 29.09.2018 20:24:45, Adrian Klaver  wrote:
> On 9/28/18 11:35 PM, Carl Sverre wrote:
> > *Context*
> > I am using row-level security along with triggers to implement a pure
> > SQL RBAC implementation. While doing so I encountered a weird behavior
> > between INSERT triggers and SELECT row-level security policies.
> >
> > *Question*
> > I have posted a very detailed question on StackOverflow here:
> >
> https://stackoverflow.com/questions/52565720/postgres-trigger-side-effect-is-occurring-out-of-order-with-row-level-security-s
> >
> > For anyone who is just looking for a summary/repro, I am seeing the
> > following behavior:
> >
> > CREATE TABLE a (id TEXT);
> > ALTER TABLE a ENABLE ROW LEVEL SECURITY;
> > ALTER TABLE a FORCE ROW LEVEL SECURITY;
> >
> > CREATE TABLE b (id TEXT);
> >
> > CREATE POLICY ON a FOR SELECT
> > USING (EXISTS(
> > select * from b where a.id = b.id
> > ));
> >
> > CREATE POLICY ON a FOR INSERT
> > WITH CHECK (true);
> >
> > CREATE FUNCTION reproHandler() RETURNS TRIGGER AS $$
> > BEGIN
> > RAISE NOTICE USING MESSAGE = 'inside trigger handler';
> > INSERT INTO b (id) VALUES (NEW.id);
> > RETURN NEW;
> > END;
> > $$ LANGUAGE plpgsql;
> >
> > CREATE TRIGGER reproTrigger BEFORE INSERT ON a
> > FOR EACH ROW EXECUTE PROCEDURE reproHandler();
> >
> > INSERT INTO a VALUES ('fails') returning id;
> > NOTICE:  inside trigger handler
> > ERROR:  new row violates row-level security policy for table "a"
> >
> > Rather than the error, I expect that something along these lines should
> > occur instead:
> >
> > 1. A new row ('fails') is staged for INSERT
> > 2. The BEFORE trigger fires with NEW set to the new row
> > 3. The row ('fails') is inserted into b and returned from the trigger
> > procedure unchanged
> > 4. The INSERT's WITH CHECK policy true is evaluated to true
> > 5. The SELECT's USING policy select * from b where a.id =
> > b.id is evaluated.  *This should return true due to step 3*
>
> > 6. Having passed all policies, the row ('fails') is inserted in table
> > 7. The id (fails) of the inserted row is returned
> >
> > If anyone can point me in the right direction I would be extremely
> thankful.
>
> When I tried to reproduce the above I got:
>
> test=# CREATE POLICY ON a FOR SELECT
> test-# USING (EXISTS(
> test(# select * from b where a.id = b.id
> test(# ));
> ERROR: syntax error at or near "ON"
> LINE 1: CREATE POLICY ON a FOR SELECT
> ^
> test=#
> test=# CREATE POLICY ON a FOR INSERT
> test-# WITH CHECK (true);
> ERROR: syntax error at or near "ON"
> LINE 1: CREATE POLICY ON a FOR INSERT
>
> Changing your code to:
>
> CREATE TABLE a (id TEXT);
> ALTER TABLE a ENABLE ROW LEVEL SECURITY;
> ALTER TABLE a FORCE ROW LEVEL SECURITY;
>
> CREATE TABLE b (id TEXT);
>
> CREATE POLICY a_select ON a FOR SELECT
> USING (EXISTS(
> select * from b where a.id = b.id
> ));
>
> CREATE POLICY a_insert ON a FOR INSERT
> WITH CHECK (true);
>
> CREATE FUNCTION reproHandler() RETURNS TRIGGER AS $$
> BEGIN
> RAISE NOTICE USING MESSAGE = 'inside trigger handler';
> INSERT INTO b (id) VALUES (NEW.id);
> RETURN NEW;
> END;
> $$ LANGUAGE plpgsql;
>
> CREATE TRIGGER reproTrigger BEFORE INSERT ON a
> FOR EACH ROW EXECUTE PROCEDURE reproHandler();
>
> Resulted in:
>
> test=# INSERT INTO a VALUES ('fails') returning id;
> NOTICE: inside trigger handler
> id
> ---
> fails
> (1 row)
>
> INSERT 0 1
> test=# select * from a;
> id
> ---
> fails
> (1 row)
>
>
> >
> > Carl Sverre
> >
> > http://www.carlsverre.com
>
>
> --
> Adrian Klaver
> adrian.kla...@aklaver.com
>
> *[Charles] :* I did the same test with PG version 10 on Windows and PG
> 9.6.2 on Linux (RedHat) with exactly the same result.
>
> db=# INSERT INTO a VALUES ('fails') returning id;
> NOTICE:  inside trigger handler
>   id
> ---
>  fails
> (1 row)
>
> INSERT 0 1
> db=# select * from a;
>   id
> ---
>  fails
> (1 row)
>
> db=# select * from b;
>   id
> ---
>  fails
> (1 row)
>
> Regards
> Charles
>
>
> --
Carl Sverre


Re: Postgres trigger side-effect is occurring out of order with row-level security select policy

2018-10-01 Thread Carl Sverre
Thank you for the detailed report Charles. I think you may be missing the
“returning id” clause in the insert. Can you verify it works when you use
“returning id”? Thanks!
On Sun, Sep 30, 2018 at 7:57 PM Charles Clavadetscher (SwissPUG) <
clavadetsc...@swisspug.org> wrote:

> Hello
>
> On 30.09.2018 23:31:32, Adrian Klaver  wrote:
> On 9/30/18 1:13 PM, Carl Sverre wrote:
> > Thanks for the initial results. Can you check that you are not using
> > super permissions and are enabling row security when running the test?
> > Super ignores row security.
>
> Yeah, big oops on my part, I was running as superuser. Running as
> non-superuser resulted in the failure you see. I tried to get around
> this with no success. My suspicion is that the new row in b is not
> visible to the returning(SELECT) query in a until after the transaction
> completes. Someone with more knowledge on this then I will have to
> confirm/deny my suspicion.
>
>
> >
> > Also yes, I forgot to add the policy names, sorry about that.
> > On Sun, Sep 30, 2018 at 1:34 AM Charles Clavadetscher (SwissPUG)
> > > wrote:
> >
> > Hello
>
> --
> Adrian Klaver
> adrian.kla...@aklaver.com
>
> *[Charles] :* I also made the first test as super. However I still don't
> get any errors when executing the test query as non superuser.
>
> The user is not superuser:
>
> testuser@charles.localhost=> SELECT CURRENT_USER;
>  current_user
> --
>  testuser
> (1 row)
>
> testuser@charles.localhost=> \du testuser
>List of roles
>  Role name | Attributes | Member of
> ---++---
>  testuser  || {}
>
> The table privileges show that RLS is enabled and that testuser has
> SELECT and INSERT privilege on both tables. This is not related to RLS but
> simple precondition for the test:
>
> testuser@charles.localhost=> \d a
> Table "public.a"
>  Column | Type | Collation | Nullable | Default
> +--+---+--+-
>  id | text |   |  |
> Policies (forced row security enabled):
> POLICY "a_insert" FOR INSERT
>   WITH CHECK (true)
> POLICY "a_select" FOR SELECT
>   USING ((EXISTS ( SELECT b.id
>FROM b
>   WHERE (a.id = b.id
> Triggers:
> reprotrigger BEFORE INSERT ON a FOR EACH ROW EXECUTE PROCEDURE
> reprohandler()
>
> testuser@charles.localhost=> \dp a
>  Access privileges
>  Schema | Name | Type  |Access privileges| Column privileges |
>   Policies
>
> +--+---+-+---+--
>  public | a| table | charles=arwdDxt/charles+|   |
> a_select (r):   +
> |  |   | testuser=ar/charles |   |
> (u): (EXISTS ( SELECT b.id+
> |  |   | |   |
>  FROM b   +
> |  |   | |   |
> WHERE (a.id = b.id))) +
> |  |   | |   |
> a_insert (a):   +
> |  |   | |   |
> (c): true
>
> testuser@charles.localhost=> \d b
> Table "public.b"
>  Column | Type | Collation | Nullable | Default
> +--+---+--+-
>  id | text |   |  |
>
> testuser@charles.localhost=> \dp b
>Access privileges
>  Schema | Name | Type  |Access privileges| Column privileges |
> Policies
>
> +--+---+-+---+--
>  public | b| table | charles=arwdDxt/charles+|   |
> |  |   | testuser=ar/charles |   |
>
> And now the test:
>
> testuser@charles.localhost=> SELECT * FROM a;
>  id
> 
> (0 rows)
>
> testuser@charles.localhost=> SELECT * FROM b;
>  id
> 
> (0 rows)
>
> testuser@charles.localhost=> INSERT INTO a VALUES ('fails');
> NOTICE:  inside trigger handler
> INSERT 0 1
> testuser@charles.localhost=> SELECT * FROM a;
>   id
> ---
>  fails
> (1 row)
>
> testuser@charles.localhost=> SELECT * FROM b;
>   id
> ---
>  fails
> (1 row)
>
> Version of PG:
> testuser@charles.localhost=> SELECT version();
>   version
> 
>  PostgreSQL 10.5, compiled by Visual C++ build 1800, 64-bit
> (1 row)
>
> Regards
> Charles
>
> --
Carl Sverre


Re: Postgres trigger side-effect is occurring out of order with row-level security select policy

2018-10-02 Thread Carl Sverre
Dean,
Thank you for the pointer towards visibility/volatility.  I think that
completely explains the effect that I am seeing in my repro.  I
experimented with using a VOLATILE function for the SELECT RLS using
statement and while it completely solves my issue, it incurs too high a
cost for query execution due to the RLS policy no longer being inlined into
the scan.

I have documented your answer and my experimentation on the stack overflow
answer:
https://stackoverflow.com/questions/52565720/postgres-trigger-side-effect-is-occurring-out-of-order-with-row-level-security-s

Feel free to make edits/suggestions if you feel I missed something in
summarizing the solution.  Also, this thread is still open to anyone who
can provide a solution which does not incur an optimization penalty -
however based on my new understanding of the underlying behavior I don't
believe this is possible.

Thank's to everyone for their contribution in figuring this out - much
appreciated.

Carl Sverre

http://www.carlsverre.com


On Mon, Oct 1, 2018 at 4:02 AM Dean Rasheed 
wrote:

> The real issue here is to do with the visibility of the data inserted
> by the trigger function from within the same command. In general, data
> inserted by a command is not visible from within that same command.
>
> The easiest way to see what's going on is with a simple example.
> Consider the following (based on the original example, but without any
> RLS):
>
>
> DROP TABLE IF EXISTS a,b;
>
> CREATE TABLE a (id text);
> CREATE TABLE b (id text);
>
> CREATE OR REPLACE FUNCTION reproHandler() RETURNS TRIGGER AS $$
> BEGIN
> RAISE NOTICE USING MESSAGE = 'inside trigger handler';
> INSERT INTO b (id) VALUES (NEW.id);
> RETURN NEW;
> END;
> $$ LANGUAGE plpgsql;
>
> CREATE TRIGGER reproTrigger BEFORE INSERT ON a
> FOR EACH ROW EXECUTE PROCEDURE reproHandler();
>
> CREATE OR REPLACE FUNCTION check_b1(text) RETURNS boolean AS $$
> BEGIN
>   RETURN (EXISTS (SELECT * FROM b WHERE b.id = $1));
> END
> $$ LANGUAGE plpgsql STABLE;
>
> CREATE OR REPLACE FUNCTION check_b2(text) RETURNS boolean AS $$
> BEGIN
>   RETURN (EXISTS (SELECT * FROM b WHERE b.id = $1));
> END
> $$ LANGUAGE plpgsql VOLATILE;
>
> INSERT INTO a VALUES ('xxx')
>   RETURNING id, check_b1(id), check_b2(id),
> (EXISTS (SELECT * FROM b WHERE b.id = a.id));
>
> NOTICE:  inside trigger handler
>  id  | check_b1 | check_b2 | exists
> -+--+--+
>  xxx | f| t| f
> (1 row)
>
> INSERT 0 1
>
>
> Notice that the functions check_b1() and check_b2() are identical,
> except that check_b1() is declared STABLE and check_b2() is declared
> VOLATILE, and that makes all the difference. Quoting from the
> documentation for function volatility [1]:
>
> For functions written in SQL or in any of the standard procedural
> languages, there is a second important property determined by the
> volatility category, namely the visibility of any data changes that
> have been made by the SQL command that is calling the function. A
> VOLATILE function will see such changes, a STABLE or IMMUTABLE
> function will not.
>
> [1] https://www.postgresql.org/docs/10/static/xfunc-volatility.html
>
> Also notice that the inline EXISTS query behaves in the same way as
> the STABLE function -- i.e., it does not see changes made in the
> current query.
>
> So returning to the RLS example, because the RLS SELECT policy is
> defined using inline SQL, it cannot see the changes made by the
> trigger. If you want to see such changes, you need to define a
> VOLATILE function to do the RLS check.
>
> Regards,
> Dean
>


Re: Postgres trigger side-effect is occurring out of order with row-level security select policy

2018-10-02 Thread Carl Sverre
Dean,
Thank you for catching that bug, I have updated the StackOverflow answer to
account for that issue.

As for the optimization problem I mentioned, the issue seems to be that
running a function that acquires a snapshot for each row is much slower
than in-lining a nested loop over table b into the query.  I have attached
a psql session that demonstrates the exact performance issue I am referring
to.

Carl Sverre

http://www.carlsverre.com


On Tue, Oct 2, 2018 at 1:28 AM Dean Rasheed 
wrote:

> On Mon, 1 Oct 2018 at 21:45, Carl Sverre  wrote:
> > Dean,
> > Thank you for the pointer towards visibility/volatility.  I think that
> completely explains the effect that I am seeing in my repro.  I
> experimented with using a VOLATILE function for the SELECT RLS using
> statement and while it completely solves my issue, it incurs too high a
> cost for query execution due to the RLS policy no longer being inlined into
> the scan.
> >
> > I have documented your answer and my experimentation on the stack
> overflow answer:
> >
> https://stackoverflow.com/questions/52565720/postgres-trigger-side-effect-is-occurring-out-of-order-with-row-level-security-s
> >
>
> I had a quick look at that and found a bug in your implementation. The
> RLS check function is defined as follows:
>
> CREATE OR REPLACE FUNCTION rlsCheck(id text) RETURNS TABLE (id text) AS $$
> select * from b where b.id = id
> $$ LANGUAGE sql VOLATILE;
>
> which is incorrect because of the ambiguous reference to "id". That
> final "id" will, by default, refer to the table column b.id, not the
> parameter "id". Thus that function will return every row of b, and
> your check won't be doing what you want. That's also going to hurt
> performance, but you didn't provide enough information to diagnose the
> actual performance problem that you are seeing.
>
> In any case, the above needs to be written as
>
> CREATE OR REPLACE FUNCTION rlsCheck(text) RETURNS TABLE (id text) AS $$
> select id from b where b.id = $1
> $$ LANGUAGE sql VOLATILE;
>
> to work as expected.
>
> Regards,
> Dean
>


rls_performance_notes
Description: Binary data