Обсуждение: Postgres trigger side-effect is occurring out of order with row-levelsecurity select policy

Hello

On 29.09.2018 20:24:45, Adrian Klaver <adrian.klaver@aklaver.com> 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.klaver@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

Hello

On 30.09.2018 23:31:32, Adrian Klaver <adrian.klaver@aklaver.com> 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.klaver@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

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

On Mon, 1 Oct 2018 at 21:45, Carl Sverre <sverre.carl@gmail.com> 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