Re: [v9.4] row level security

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: Craig Ringer <craig(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stephen Frost <sfrost(at)snowman(dot)net>, Josh Berkus <josh(at)agliodbs(dot)com>, "ktm(at)rice(dot)edu" <ktm(at)rice(dot)edu>, Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Oleg Bartunov <obartunov(at)gmail(dot)com>, Greg Smith <greg(at)2ndquadrant(dot)com>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [v9.4] row level security
Date: 2013-11-07 10:11:38
Message-ID: CAEZATCXUsjGbKt5gABuEa7pa_ROgYVYw_FaPAbfBmNVaYF-wcA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 6 November 2013 19:12, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> 2013/11/6 Craig Ringer <craig(at)2ndquadrant(dot)com>:
>> On 11/05/2013 09:36 PM, Robert Haas wrote:
>>> I haven't studied this patch in detail, but I see why there's some
>>> unhappiness about that code: it's an RLS-specific kluge. Just
>>> shooting from the hip here, maybe we should attack the problem of
>>> making security-barrier views updatable first, as a separate patch.
>>
>> That's the approach I've been considering. There are a few wrinkles with
>> it, though:
>>
>> (a) Updatable views are implemented in the rewriter, not the planner.
>> The rewriter is not re-run when plans are invalidated or when the
>> session authorization changes, etc. This means that we can't simply omit
>> the RLS predicate for superuser because the same rewritten parse tree
>> might get used for both superuser and non-superuser queries.
>>
>> Options:
>>
>> * Store the before-rewrite parse tree when RLS is discovered on one of
>> the rels in the tree. Re-run the rewriter when re-planning. Ensure a
>> change in current user always invalidates plans.
>>
>> * Declare that it's not legal to run a query originally parsed and
>> rewritten as superuser as a non-superuser or vice versa. This would
>> cause a great deal of pain with PL/PgSQL.
>>
>> * Always add the RLS predicate and solve the problem of reliably
>> short-circuiting the user-supplied predicate for RLS-exempt users. We'd
>> need a way to allow direct (not query-based) COPY TO for tables with RLS
>> applied, preventing the rewriting of direct table access into subqueries
>> for COPY, but since we never save plans for COPY that may be fine.
>>
>> * ... ?
>>
> How about an idea that uses two different type of rules: the existing one
> is expanded prior to planner stage as we are doing now, and the newer
> one is expanded on the head of planner stage.
> The argument of planner() is still parse tree, so it seems to me here is
> no serious problem to call rewriter again to handle second stage rules.
> If we go on this approach, ALTER TABLE ... SET ROW SECURITY
> will become a synonym to declare a rule with special attribute.
>

I don't really get this part of the discussion. Why would you want to
make updatable SB views do any of that? With RLS on tables, there is
only one object in play - the table itself. So I can see that there is
this requirement for certain privileged users to be able to bypass the
RLS quals, and hence the need to invalidate cached plans.

With SB views, however, you can have multiple SB views on top of the
same base table, each giving different users access to different
subsets of the data, and controlled by suitable GRANTs, and suitably
privileged users can be given direct access to the base table. This
also gives greater flexibility than the superuser/non-superuser
distinction being discussed here.

I don't think a view should ever show different data to different
users (unless it has been deliberately set up to do so) because that
would likely lead to confusion. Is there some other use-case that I'm
missing here?

Regards,
Dean

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip kumar 2013-11-07 11:42:59 TODO : Allow parallel cores to be used by vacuumdb [ WIP ]
Previous Message Oskari Saarenmaa 2013-11-07 10:05:12 Re: pg_fallocate