Re: [v9.3] Row-Level Security

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Florian Pflug <fgp(at)phlo(dot)org>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [v9.3] Row-Level Security
Date: 2012-10-08 19:49:24
Message-ID: CADyhKSXHQxKYpnm4VizVnh=6Gv-TnxbjnucjWaH1am8zpvXY0Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2012/10/8 Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>:
> On 8 October 2012 15:57, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> The attached patch is a refreshed version towards the latest master branch,
>> to fix up patch conflicts.
>> Here is no other difference from the previous revision.
>>
>> Thanks,
>>
>
> I had another look at this over the weekend and I found couple of
> additional problems (test cases attached):
>
> 1). It is possible to define a RLS qual that refers back to the table
> that it's defined on, in such a way that causes infinite recursion in
> the planner, giving "ERROR: stack depth limit exceeded". I think it
> would be preferable to trap this and report a more meaningful error
> back to the user, along similar lines to a self-referencing view.
>
> 2). In other cases it is possible to define a RLS qual that refers to
> another table with a RLS qual in such a way that the second table's
> RLS qual is not checked, thus allowing a user to bypass the security
> check.
>
> 3). If a RLS qual refers to a view it errors, since the RLS quals are
> added after rule expansion, and so the view is not rewritten.
>
> To me this suggests that perhaps the expansion of RLS quals should be
> done in the rewriter. I've not thought that through in any detail, but
> ISTM that a RIR rule could add a table with a RLS qual, and a RLS qual
> could add a relation with a RIR rule that needs expanding, and so the
> 2 need to be processed together. This could also make use of the
> existing recursion-checking code in the rewriter.
>
Thanks for your checks. I missed some cases that you suggested.

The reason why we need to put RLS expansion at planner stage is
requirement towards plan cache invalidation. Due to special case
handling for superuser, plan cache has to be invalidated if user-id
to run executor was switched since planner stage. The planner shall
be invoked again, but not rewritter, on its invalidation.

Probably, it make sense to invoke rewriter's logic to solve RLS
policy from planner stage (that allows plan-cache invalidation).
Let me investigate the code of rewriter.

Best regards,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2012-10-08 19:53:40 Re: getopt() and strdup()
Previous Message Peter Geoghegan 2012-10-08 19:43:51 Re: embedded list v3