Re: Add support for restrictive RLS policies

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Thom Brown <thom(at)linux(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add support for restrictive RLS policies
Date: 2016-09-04 15:01:13
Message-ID: 20160904150113.GM4028@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Thu, Sep 1, 2016 at 12:04 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > As outlined in the commit message, this adds support for restrictive RLS
> > policies. We've had this in the backend since 9.5, but they were only
> > available via hooks and therefore extensions. This adds support for
> > them to be configured through regular DDL commands. These policies are,
> > essentially "AND"d instead of "OR"d.
> >
> > Includes updates to the catalog, grammer, psql, pg_dump, and regression
> > tests. Documentation will be added soon, but until then, would be great
> > to get feedback on the grammer, catalog and code changes.
>
> I don't like CREATE RESTRICT POLICY much. It's not very good grammar,
> for one thing. I think putting the word RESTRICT, or maybe AS
> RESTRICT, somewhere later in the command would be better.

I had been notionally thinking RESTRICTIVE, but ended up just using
RESTRICT since it was already an unreserved keyword. Of course, that's
not a good reason.

> I also think that it is very strange to have the grammar keyword be
> "restrict" but the internal flag be called "permissive". It would be
> better to have the sense of those flags match.

Permissive is the default and should just be added to the grammar, so
users can be explicit, if they wish to.

* Thom Brown (thom(at)linux(dot)com) wrote:
> On 1 September 2016 at 10:02, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > I don't like CREATE RESTRICT POLICY much. It's not very good grammar,
> > for one thing. I think putting the word RESTRICT, or maybe AS
> > RESTRICT, somewhere later in the command would be better.
> >
> > I also think that it is very strange to have the grammar keyword be
> > "restrict" but the internal flag be called "permissive". It would be
> > better to have the sense of those flags match.
> >
> > (This is not intended as a full review, just a quick comment.)
>
> I had proposed this sort of functionality a couple years back:
> https://www.depesz.com/2014/10/02/waiting-for-9-5-row-level-security-policies-rls/#comment-187800
>
> And I suggested CREATE RESTRICTIVE POLICY, but looking back at that,
> perhaps you're right, and it would be better to add it later in the
> command.

Ah, I had recalled seeing something along those lines somewhere, but
didn't know where, thanks.

Based on Robert's suggestion and using Thom's verbiage, I've tested this
out:

CREATE POLICY pol ON tab AS [PERMISSIVE|RESTRICTIVE] ...

and it appears to work fine with the grammar, etc.

Unless there's other thoughts on this, I'll update the patch to reflect
this grammar in a couple days.

Thanks!

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-09-04 15:33:55 Re: [PATCH] COPY vs \copy HINT
Previous Message Andreas Karlsson 2016-09-04 14:39:10 Re: [PATCH] Reload SSL certificates on SIGHUP