Re: unclear about row-level security USING vs. CHECK

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Charles Clavadetscher <clavadetscher(at)swisspug(dot)org>
Subject: Re: unclear about row-level security USING vs. CHECK
Date: 2015-09-28 19:15:34
Message-ID: 20150928191533.GU3685@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

* Peter Eisentraut (peter_e(at)gmx(dot)net) wrote:
> I see. But it is a bit odd to hide this very fundamental behavior
> somewhere in a paragraph that starts out with something about roles.

I'm happy to change that. You're right, it should be a paragraph by
itself.

> There is also a mistake, I believe: DELETE policies also take both a
> CHECK and a USING clause.

DELETE never adds records and therefore does not take a CHECK clause,
only a USING clause:

=*# create policy p1 on t1 for delete using (c1 > 5) with check (c1 > 10);
ERROR: WITH CHECK cannot be applied to SELECT or DELETE

There has been some discussion about changing that, but that would be a
future change and not for 9.5.

> I still find something about this weird, but I'm not sure what. It's
> not clear to me at what level this USING->CHECK mapping is applied. I
> can write FOR ALL USING and it will be mapped to CHECK for all actions,
> including INSERT, but when I write FOR INSERT USING it complains. Why
> doesn't it do the mapping that case, too?

INSERT is only adding records and therefore only the CHECK policy
applies:

=*# create policy p1 on t1 for insert using (c1 > 5) with check (c1 > 10);
ERROR: only WITH CHECK expression allowed for INSERT

The USING clause is for existing records while the CHECK option is for
new records, which is why DELETE only has a USING clause and INSERT only
has a WITH CHECK clause. ALL allows you to specify clauses for all
commands, which is why it accepts both. The only other case which
allows both is UPDATE, where records are both retrived and added.

> >> (Btw., what's the meaning of a policy for DELETE?)
> >
> > The DELETE policy controls what records a user is able to delete.
>
> That needs to be documented somewhere.

This is included in the CREATE POLICY documentation:

DELETE

Using DELETE for a policy means that it will apply to DELETE
commands. Only rows which pass this policy will be seen by a DELETE
command. Rows may be visible through a SELECT which are not seen by
a DELETE, as they do not pass the USING expression for the DELETE,
and rows which are not visible through the SELECT policy may be
deleted if they pass the DELETE USING policy. The DELETE policy only
accepts the USING expression as it only ever applies in cases where
records are being extracted from the relation for deletion.

I'm certainly all for improving the documentation, of course. What
about the above isn't clear regarding what DELETE policies do? Or is
the issue that it wasn't covered in ddl-rowsecurity? Perhaps we should
simply move much of the CREATE POLICY documentation into ddl-rowsecurity
instead, since that's where people seem to be looking for this
information?

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Sat, Sep 26, 2015 at 9:46 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> > On 9/23/15 3:41 PM, Stephen Frost wrote:
> > I see. But it is a bit odd to hide this very fundamental behavior
> > somewhere in a paragraph that starts out with something about roles.
> >
> > There is also a mistake, I believe: DELETE policies also take both a
> > CHECK and a USING clause.
> >
> > I still find something about this weird, but I'm not sure what. It's
> > not clear to me at what level this USING->CHECK mapping is applied. I
> > can write FOR ALL USING and it will be mapped to CHECK for all actions,
> > including INSERT, but when I write FOR INSERT USING it complains. Why
> > doesn't it do the mapping that case, too?
>
> We are really pushing our luck only hammering this stuff out now. But
> I think I agree with Peter's concerns, FWIW.

I listed out the various alternatives but didn't end up getting any
responses to it. I'm still of the opinion that the documentation is the
main thing which needs improving here, but we can also change CREATE
POLICY, et al, to require an explicit WITH CHECK clause for the commands
where that makes sense if that's the consensus.

Thanks!

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2015-09-28 19:42:40 Re: Rename withCheckOptions to insertedCheckClauses
Previous Message Stephen Frost 2015-09-28 19:03:51 Re: RLS open items are vague and unactionable