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

[Charles Clavadetscher]

I had not seen this.

> -Original Message-
> From: pgsql-hackers-ow...@postgresql.org 
> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas
> Sent: Montag, 28. September 2015 21:43
> To: Stephen Frost <sfr...@snowman.net>
> Cc: Peter Eisentraut <pete...@gmx.net>; pgsql-hackers 
> <pgsql-hackers@postgresql.org>; Charles Clavadetscher
> <clavadetsc...@swisspug.org>
> Subject: Re: [HACKERS] unclear about row-level security USING vs. CHECK
> 
> On Mon, Sep 28, 2015 at 3:15 PM, Stephen Frost <sfr...@snowman.net> wrote:
> > 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.
> 
> My vote is to remove the behavior where USING flows over to WITH
> CHECK.  So you only get a WITH CHECK policy if you explicitly specify
> one.
> 
> If there's some other consensus, OK, but tempus fugit.

If the behaviof of USING doesn't flow to WITH CHECK is the same as making WITH 
CHECK mandatory for ALL and UPDATE, I guess. Otherwise there would be a 
partially unspecified behavior. Or am I misunderstanding your idea?

Charles





-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Dean Rasheed]

On 28 September 2015 at 20:15, Stephen Frost  wrote:
> 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.
>

My vote would be to keep it as-is.

It feels perfectly natural to me. USING clauses add to the query's
WHERE clause controlling which existing rows you can SELECT, UPDATE or
DELETE. WITH CHECK clauses control what new data you can add via
INSERT or UPDATE. UPDATE allows both, but most of the time I expect
you'll want them to be the same.

So having the WITH CHECK clause default to being the same as the USING
clause for UPDATE matches what I expect to be the most common usage.
Users granted permission to update a subset of the table's rows
probably don't want to give those rows away. More advanced use-cases
are still supported, but the simplest/most common case is the default,
which means that you don't have to supply the same expression twice.

I agree that the documentation could be improved.

As things stand, you have to read quite a lot of text on the CREATE
POLICY page before you get to the description of how the USING and
WITH CHECK expressions interact. I'd suggest rewording the 2nd
paragraph where these clauses are first introduced. Perhaps something
like:

"""
A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
which match the relevant policy expression. For SELECT, UPDATE and
DELETE, the USING expression from the policy is combined with the
query's WHERE clause to control which existing table rows can be
retrieved, updated or deleted. For INSERT and UPDATE, the WITH CHECK
expression is used to constrain what new data can be added to the
table. A policy that applies to UPDATE may have both USING and WITH
CHECK expressions, which may be different from one another, but if
they are the same, the WITH CHECK expression can be omitted and the
USING expression will be used automatically in its place.

Policy expressions may be any expressions that evaluate to give a
result of type boolean. When a USING expression returns true for a
given row then the query is allowed to act upon that row, while rows
for which the expression returns false or null are skipped. When a
WITH CHECK expression returns true for a new row then the system
allows that row to be added to the table, but if the expression
returns false or null an error is raised.
"""

Regards,
Dean


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Adam Brightwell]

> My vote would be to keep it as-is.

Same for me.

> It feels perfectly natural to me. USING clauses add to the query's
> WHERE clause controlling which existing rows you can SELECT, UPDATE or
> DELETE. WITH CHECK clauses control what new data you can add via
> INSERT or UPDATE. UPDATE allows both, but most of the time I expect
> you'll want them to be the same.

I agree.  In the current uses cases I have been experimenting with,
this approach has made the most sense.

> So having the WITH CHECK clause default to being the same as the USING
> clause for UPDATE matches what I expect to be the most common usage.

I agree.

> Users granted permission to update a subset of the table's rows
> probably don't want to give those rows away. More advanced use-cases
> are still supported, but the simplest/most common case is the default,
> which means that you don't have to supply the same expression twice.

Yes, I agree.  IMO, having to supply the same expression twice just
seems cumbersome and unnecessary.  While I'd certainly agree that
documentation could always be improved, I have found the current
behavior to be fairly intuitive and easily understood by most (if not
all) DBA's I have spoken with about it.

-Adam

-- 
Adam Brightwell - adam.brightw...@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Stephen Frost]

* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 28 September 2015 at 20:15, Stephen Frost  wrote:
> > 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.
> 
> My vote would be to keep it as-is.

That's my feeling on it as well, particularly as...

> It feels perfectly natural to me. USING clauses add to the query's
> WHERE clause controlling which existing rows you can SELECT, UPDATE or
> DELETE. WITH CHECK clauses control what new data you can add via
> INSERT or UPDATE. UPDATE allows both, but most of the time I expect
> you'll want them to be the same.

exactly this.  Many people are going to want them to be the same and not
supporting a single-expression syntax is going to frustrate them, to no
particularly good end, in my view.  The "USING AND WITH CHECK"
technically solves that but feels very odd to me.

> So having the WITH CHECK clause default to being the same as the USING
> clause for UPDATE matches what I expect to be the most common usage.
> Users granted permission to update a subset of the table's rows
> probably don't want to give those rows away. More advanced use-cases
> are still supported, but the simplest/most common case is the default,
> which means that you don't have to supply the same expression twice.

Agreed.

> I agree that the documentation could be improved.
> 
> As things stand, you have to read quite a lot of text on the CREATE
> POLICY page before you get to the description of how the USING and
> WITH CHECK expressions interact. I'd suggest rewording the 2nd
> paragraph where these clauses are first introduced. Perhaps something
> like:
> 
> """
> A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
> which match the relevant policy expression. For SELECT, UPDATE and
> DELETE, the USING expression from the policy is combined with the
> query's WHERE clause to control which existing table rows can be
> retrieved, updated or deleted. For INSERT and UPDATE, the WITH CHECK
> expression is used to constrain what new data can be added to the
> table. A policy that applies to UPDATE may have both USING and WITH
> CHECK expressions, which may be different from one another, but if
> they are the same, the WITH CHECK expression can be omitted and the
> USING expression will be used automatically in its place.
> 
> Policy expressions may be any expressions that evaluate to give a
> result of type boolean. When a USING expression returns true for a
> given row then the query is allowed to act upon that row, while rows
> for which the expression returns false or null are skipped. When a
> WITH CHECK expression returns true for a new row then the system
> allows that row to be added to the table, but if the expression
> returns false or null an error is raised.
> """

I'm not convinced that this really helps, but I don't have anything
dramatically better yet either.  I'll try to come up with something
though.

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Charles Clavadetscher]

Good morning

> -Original Message-
> From: pgsql-hackers-ow...@postgresql.org 
> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Stephen Frost
> Sent: Montag, 28. September 2015 21:16
> To: Peter Eisentraut <pete...@gmx.net>; Robert Haas <robertmh...@gmail.com>
> Cc: pgsql-hackers <pgsql-hackers@postgresql.org>; Charles Clavadetscher 
> <clavadetsc...@swisspug.org>
> Subject: Re: [HACKERS] unclear about row-level security USING vs. CHECK
> 
> * Peter Eisentraut (pete...@gmx.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?

I think that many people will look first into ddl-rowsecurity to get an 
understanding what it can do and how it can be used.
Detailed information is then in the CREATE POLICY doc. So it could make sense 
to move parts that contribute to understand the
mechanics as a whole from the CREATE POLICY doc to ddl-rowsecurity. As an 
alternative, when it comes to the characteristics of a
specific command, a link to the place in CREATE POLICY doc may be enough. Just 
no duplicated information. That would be difficult to
keep in sync.

> * Robert Haas (robertmh...@gmail.com) wrote:
> > On Sat, Sep 26, 2015 at 9:46 PM, Peter Eisentraut <pete...@gmx.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

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

[Stephen Frost]

* Peter Eisentraut (pete...@gmx.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 (robertmh...@gmail.com) wrote:
> On Sat, Sep 26, 2015 at 9:46 PM, Peter Eisentraut  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


signature.asc
Description: Digital signature

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

[Robert Haas]

On Mon, Sep 28, 2015 at 3:15 PM, Stephen Frost  wrote:
> 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.

My vote is to remove the behavior where USING flows over to WITH
CHECK.  So you only get a WITH CHECK policy if you explicitly specify
one.

If there's some other consensus, OK, but tempus fugit.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Robert Haas]

On Sat, Sep 26, 2015 at 9:46 PM, Peter Eisentraut  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.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Peter Eisentraut]

On 9/23/15 3:41 PM, Stephen Frost wrote:
> The CREATE POLICY documentation discusses how lack of a WITH CHECK
> policy means the USING expression is used:
> 
> """
> Policies can be applied for specific commands or for specific roles. The
> default for newly created policies is that they apply for all commands
> and roles, unless otherwise specified. If multiple policies apply to a
> given query, they will be combined using OR (although ON CONFLICT DO
> UPDATE and INSERT policies are not combined in this way, but rather
> enforced as noted at each stage of ON CONFLICT execution). Further, for
> commands which can have both USING and WITH CHECK policies (ALL and
> UPDATE), if no WITH CHECK policy is defined then the USING policy will
> be used for both what rows are visible (normal USING case) and which
> rows will be allowed to be added (WITH CHECK case).
> """

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?

>> (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.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Robert Haas]

On Tue, Sep 22, 2015 at 10:36 PM, Charles Clavadetscher
 wrote:
> Since the policy is defined for ALL commands and no WITH CHECK is specified 
> then the same condition defined in USING takes effect for all commands, i.e. 
> including INSERT.
>
> From the docs 
> (http://www.postgresql.org/docs/9.5/static/sql-createpolicy.html): "Further, 
> for commands which can have both USING and WITH CHECK policies (ALL and 
> UPDATE), if no WITH CHECK policy is defined then the USING policy will be 
> used for both what rows are visible (normal USING case) and which rows will 
> be allowed to be added (WITH CHECK case)."
>
> If you want e.g. to allow users to insert rows without the restriction of 
> being the current_user in column entered_by then you would need separate 
> policies for each command. If you define a policy for INSERT, USING does not 
> make sense. In the thread above there is a similar example to this as well as 
> in the documentation:
>
> http://www.postgresql.org/docs/9.5/static/ddl-rowsecurity.html
>
>> (Btw., what's the meaning of a policy for DELETE?)
>
> In your example it means that users can delete only the rows where entered_by 
> = current_user. A WITH CHECK policy does not make sense in this case.

Gosh, I think it would have been better to have a cleaner separation
of USING and WITH CHECK.  That sounds far too unnecessarily magical.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Robert Haas]

On Wed, Sep 23, 2015 at 11:05 AM, Stephen Frost  wrote:
>> Gosh, I think it would have been better to have a cleaner separation
>> of USING and WITH CHECK.  That sounds far too unnecessarily magical.
>
> That the USING policy is used if WITH CHECK isn't defined?  That was
> simply done to make policy management simple as in quite a few cases
> only one policy is needed.  If a WITH CHECK was always required then
> you'd be constantly writing:
>
> CREATE POLICY p1 ON t1
> USING (entered_by = current_user)
> WITH CHECK (entered_by = current_user);
>
> With potentially quite lengthy expressions.
>
> I'm not against changing that if people feel strongly about it, but I
> certainly find it extremely handy.
>
> If that wasn't what you were referring to then please clarify as I
> didn't follow.

No, that's what I was talking about.  Maybe it is the most useful
behavior, but it seems to have surprised Peter, and it surprised me,
too.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Tue, Sep 22, 2015 at 10:36 PM, Charles Clavadetscher
>  wrote:
> > Since the policy is defined for ALL commands and no WITH CHECK is specified 
> > then the same condition defined in USING takes effect for all commands, 
> > i.e. including INSERT.
> >
> > From the docs 
> > (http://www.postgresql.org/docs/9.5/static/sql-createpolicy.html): 
> > "Further, for commands which can have both USING and WITH CHECK policies 
> > (ALL and UPDATE), if no WITH CHECK policy is defined then the USING policy 
> > will be used for both what rows are visible (normal USING case) and which 
> > rows will be allowed to be added (WITH CHECK case)."
> >
> > If you want e.g. to allow users to insert rows without the restriction of 
> > being the current_user in column entered_by then you would need separate 
> > policies for each command. If you define a policy for INSERT, USING does 
> > not make sense. In the thread above there is a similar example to this as 
> > well as in the documentation:
> >
> > http://www.postgresql.org/docs/9.5/static/ddl-rowsecurity.html
> >
> >> (Btw., what's the meaning of a policy for DELETE?)
> >
> > In your example it means that users can delete only the rows where 
> > entered_by = current_user. A WITH CHECK policy does not make sense in this 
> > case.
> 
> Gosh, I think it would have been better to have a cleaner separation
> of USING and WITH CHECK.  That sounds far too unnecessarily magical.

That the USING policy is used if WITH CHECK isn't defined?  That was
simply done to make policy management simple as in quite a few cases
only one policy is needed.  If a WITH CHECK was always required then
you'd be constantly writing:

CREATE POLICY p1 ON t1
USING (entered_by = current_user)
WITH CHECK (entered_by = current_user);

With potentially quite lengthy expressions.

I'm not against changing that if people feel strongly about it, but I
certainly find it extremely handy.

If that wasn't what you were referring to then please clarify as I
didn't follow.

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Wed, Sep 23, 2015 at 11:05 AM, Stephen Frost  wrote:
> >> Gosh, I think it would have been better to have a cleaner separation
> >> of USING and WITH CHECK.  That sounds far too unnecessarily magical.
> >
> > That the USING policy is used if WITH CHECK isn't defined?  That was
> > simply done to make policy management simple as in quite a few cases
> > only one policy is needed.  If a WITH CHECK was always required then
> > you'd be constantly writing:
> >
> > CREATE POLICY p1 ON t1
> > USING (entered_by = current_user)
> > WITH CHECK (entered_by = current_user);
> >
> > With potentially quite lengthy expressions.
> >
> > I'm not against changing that if people feel strongly about it, but I
> > certainly find it extremely handy.
> >
> > If that wasn't what you were referring to then please clarify as I
> > didn't follow.
> 
> No, that's what I was talking about.  Maybe it is the most useful
> behavior, but it seems to have surprised Peter, and it surprised me,
> too.

I'm working on a documentation patch with Adam to improve the docs
around this (and other parts as well).  I agree it doesn't come off as
naturally intuitive to everyone (it did to me, but I'm clearly biased
as, I think anyway, it was my idea) and so I'm not sure that's enough.

Is there strong feeling that USING and WITH CHECK should both always be
required when specifying ALL and UPDATE policies?  It's not a difficult
change to make if people want it.

I will mention that on another thread there was discussion about having
WITH CHECK for all policy types as a way to let users control if an
error should be thrown rather than skipping over a row due to lack of
visibility.  In all cases, USING controls visibility and WITH CHECK will
throw an error on a violation and that would remain the case with this
approach.  Now that I think about it, it might be a bit cleaner if
USING and WITH CHECK are always kept independent for that case, but I'm
not sure it's really all that much of a difference.  The USING will
always be applied first and then the WITH CHECK applied to any rows
which remain, which comes across, to me at least (which isn't fair, of
course, but it's what I can comment on) as quite clear to understand.

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Robert Haas]

On Wed, Sep 23, 2015 at 12:01 PM, Stephen Frost  wrote:
> * Robert Haas (robertmh...@gmail.com) wrote:
>> On Wed, Sep 23, 2015 at 11:24 AM, Stephen Frost  wrote:
>> > I'm working on a documentation patch with Adam to improve the docs
>> > around this (and other parts as well).  I agree it doesn't come off as
>> > naturally intuitive to everyone (it did to me, but I'm clearly biased
>> > as, I think anyway, it was my idea) and so I'm not sure that's enough.
>> >
>> > Is there strong feeling that USING and WITH CHECK should both always be
>> > required when specifying ALL and UPDATE policies?  It's not a difficult
>> > change to make if people want it.
>>
>> My expectation would have been:
>>
>> If you specify USING, you can see only those rows, but you can give
>> rows away freely.  If you don't want to allow giving rows away under
>> any circumstances, then specify the same expression for USING and WITH
>> CHECK.
>
> Having an implicit 'true' for WITH CHECK would be very much against what
> I would ever expect.  If anything, I'd think we would have an implicit
> 'false' there or simply not allow it to ever be unspecified.

Huh?  If you had an implicit false, wouldn't that prevent updating or
deleting any rows at all?

>> I don't really get that.  If you could make skipping a row trigger an
>> error, then that would create a bunch of covert channel attacks.
>
> Apparently I didn't explain it correctly.  Skipping a row doesn't
> trigger an error.  An example would perhaps help here to clarify:
>
> CREATE POLICY p1 ON t1 FOR DELETE
> USING (true)
> WITH CHECK (inserted_by = current_user);
>
> What would happen above is that, in a DELETE case, you're allowed to
> *try* and delete any record in the table, but if you try to delete a
> record which isn't yours, we throw an error.  Currently the only option,
> if you want to prevent users from deleteing records which are not
> theirs, is to have:
>
> CREATE POLICY p1 ON t1 FOR DELETE
> USING (inserted_by = current_user)
>
> Which certainly has the effect that you can only delete records you own,
> but I can see use-cases where you'd like to know that someone tried to
> delete a record which isn't their own and that isn't something you can
> get directly today.

Well, you can use a trigger, I think.  But the point is that right
now, if you try to delete a record that you don't own, it just says
DELETE 0.  Maybe there was a record there that you can't see, and
maybe there wasn't.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Peter Eisentraut]

On 9/23/15 11:05 AM, Stephen Frost wrote:
> That the USING policy is used if WITH CHECK isn't defined?  That was
> simply done to make policy management simple as in quite a few cases
> only one policy is needed.  If a WITH CHECK was always required then
> you'd be constantly writing:
> 
> CREATE POLICY p1 ON t1
> USING (entered_by = current_user)
> WITH CHECK (entered_by = current_user);
> 
> With potentially quite lengthy expressions.

That might be reasonable, but the documentation is completely wrong
about that.

That said, why even have USING and CHECK as separate clauses?  Can't you
just create different policies if you want them different?

Hypothetical example:

CREATE POLICY p1 ON t1 FOR SELECT CHECK (extract(year from entered_on) =
extract(year from current_timestamp));
CREATE POLICY p2 ON t2 FOR INSERT, UPDATE, DELETE CHECK (entered_by =
current_user);


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Wed, Sep 23, 2015 at 11:24 AM, Stephen Frost  wrote:
> > I'm working on a documentation patch with Adam to improve the docs
> > around this (and other parts as well).  I agree it doesn't come off as
> > naturally intuitive to everyone (it did to me, but I'm clearly biased
> > as, I think anyway, it was my idea) and so I'm not sure that's enough.
> >
> > Is there strong feeling that USING and WITH CHECK should both always be
> > required when specifying ALL and UPDATE policies?  It's not a difficult
> > change to make if people want it.
> 
> My expectation would have been:
> 
> If you specify USING, you can see only those rows, but you can give
> rows away freely.  If you don't want to allow giving rows away under
> any circumstances, then specify the same expression for USING and WITH
> CHECK.

Having an implicit 'true' for WITH CHECK would be very much against what
I would ever expect.  If anything, I'd think we would have an implicit
'false' there or simply not allow it to ever be unspecified.

> > I will mention that on another thread there was discussion about having
> > WITH CHECK for all policy types as a way to let users control if an
> > error should be thrown rather than skipping over a row due to lack of
> > visibility.  In all cases, USING controls visibility and WITH CHECK will
> > throw an error on a violation and that would remain the case with this
> > approach.  Now that I think about it, it might be a bit cleaner if
> > USING and WITH CHECK are always kept independent for that case, but I'm
> > not sure it's really all that much of a difference.  The USING will
> > always be applied first and then the WITH CHECK applied to any rows
> > which remain, which comes across, to me at least (which isn't fair, of
> > course, but it's what I can comment on) as quite clear to understand.
> 
> I don't really get that.  If you could make skipping a row trigger an
> error, then that would create a bunch of covert channel attacks.

Apparently I didn't explain it correctly.  Skipping a row doesn't
trigger an error.  An example would perhaps help here to clarify:

CREATE POLICY p1 ON t1 FOR DELETE
USING (true)
WITH CHECK (inserted_by = current_user); 

What would happen above is that, in a DELETE case, you're allowed to
*try* and delete any record in the table, but if you try to delete a
record which isn't yours, we throw an error.  Currently the only option,
if you want to prevent users from deleteing records which are not
theirs, is to have:

CREATE POLICY p1 ON t1 FOR DELETE
USING (inserted_by = current_user)

Which certainly has the effect that you can only delete records you own,
but I can see use-cases where you'd like to know that someone tried to
delete a record which isn't their own and that isn't something you can
get directly today.

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Robert Haas]

On Wed, Sep 23, 2015 at 11:24 AM, Stephen Frost  wrote:
> I'm working on a documentation patch with Adam to improve the docs
> around this (and other parts as well).  I agree it doesn't come off as
> naturally intuitive to everyone (it did to me, but I'm clearly biased
> as, I think anyway, it was my idea) and so I'm not sure that's enough.
>
> Is there strong feeling that USING and WITH CHECK should both always be
> required when specifying ALL and UPDATE policies?  It's not a difficult
> change to make if people want it.

My expectation would have been:

If you specify USING, you can see only those rows, but you can give
rows away freely.  If you don't want to allow giving rows away under
any circumstances, then specify the same expression for USING and WITH
CHECK.

> I will mention that on another thread there was discussion about having
> WITH CHECK for all policy types as a way to let users control if an
> error should be thrown rather than skipping over a row due to lack of
> visibility.  In all cases, USING controls visibility and WITH CHECK will
> throw an error on a violation and that would remain the case with this
> approach.  Now that I think about it, it might be a bit cleaner if
> USING and WITH CHECK are always kept independent for that case, but I'm
> not sure it's really all that much of a difference.  The USING will
> always be applied first and then the WITH CHECK applied to any rows
> which remain, which comes across, to me at least (which isn't fair, of
> course, but it's what I can comment on) as quite clear to understand.

I don't really get that.  If you could make skipping a row trigger an
error, then that would create a bunch of covert channel attacks.
Granted we will have some of those anyway, but I see no reason to
manufacture more.  You can set row_security=off if you want an attempt
to query a table with RLS enabled to fail outright, but you're not
entitled to know whether a particular query skipped an invisible row.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Robert Haas]

On Wed, Sep 23, 2015 at 2:39 PM, Stephen Frost  wrote:
> * Robert Haas (robertmh...@gmail.com) wrote:
>> On Wed, Sep 23, 2015 at 12:01 PM, Stephen Frost  wrote:
>> > * Robert Haas (robertmh...@gmail.com) wrote:
>> >> My expectation would have been:
>> >>
>> >> If you specify USING, you can see only those rows, but you can give
>> >> rows away freely.  If you don't want to allow giving rows away under
>> >> any circumstances, then specify the same expression for USING and WITH
>> >> CHECK.
>> >
>> > Having an implicit 'true' for WITH CHECK would be very much against what
>> > I would ever expect.  If anything, I'd think we would have an implicit
>> > 'false' there or simply not allow it to ever be unspecified.
>>
>> Huh?  If you had an implicit false, wouldn't that prevent updating or
>> deleting any rows at all?
>
> Right, just the same as how, if RLS is enabled and no explicit policies
> are provided, non-owners can't see the rows or insert/update/delete
> anything in the table.  The same is true for the GRANT system, where
> there are no permissions granted by default.  I view the lack of an
> explicit definition of a WITH CHECK clause to be the same, excepting the
> simple case where it's the same as USING.

Hmm, interesting.  I guess that's a defensible position, but I still
think that having them default to be the same thing implicitly is
kinda weird.  I'll defer to whatever the consensus, is, though.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Stephen Frost]

* Peter Eisentraut (pete...@gmx.net) wrote:
> On 9/23/15 11:05 AM, Stephen Frost wrote:
> > That the USING policy is used if WITH CHECK isn't defined?  That was
> > simply done to make policy management simple as in quite a few cases
> > only one policy is needed.  If a WITH CHECK was always required then
> > you'd be constantly writing:
> > 
> > CREATE POLICY p1 ON t1
> > USING (entered_by = current_user)
> > WITH CHECK (entered_by = current_user);
> > 
> > With potentially quite lengthy expressions.
> 
> That might be reasonable, but the documentation is completely wrong
> about that.

Really?  I feel pretty confident that it's at least mentioned.  I
agree that it should be made more clear.

> That said, why even have USING and CHECK as separate clauses?  Can't you
> just create different policies if you want them different?
> 
> Hypothetical example:
> 
> CREATE POLICY p1 ON t1 FOR SELECT CHECK (extract(year from entered_on) =
> extract(year from current_timestamp));
> CREATE POLICY p2 ON t2 FOR INSERT, UPDATE, DELETE CHECK (entered_by =
> current_user);

USING is about visibility of existing records, WITH CHECK is in regards
to new rows being added to the relation (either through an INSERT or an
UPDATE).  It would be possible to change WITH CHECK for INSERT to be
USING, but that doesn't work for UPDATE as there are many use-cases
where you want a different policy for the UPDATE visibility vs. the
resulting record.

To say it another way, you may be allowed to update lots of records but
the resulting records have to pass a different policy to be allowed.

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Stephen Frost]

Peter,

* Peter Eisentraut (pete...@gmx.net) wrote:
> I'm testing the new row-level security feature.  I'm not clear on the
> difference between the USING and CHECK clauses in the CREATE POLICY
> statement.
> 
> The documentation says:
> 
> """
> A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
> which match the relevant policy expression. Existing table rows are
> checked against the expression specified via USING, while new rows that
> would be created via INSERT or UPDATE are checked against the expression
> specified via WITH CHECK. When a USING expression returns true for a
> given row then that row is visible to the user, while if a false or null
> is returned then the row is not visible. When a WITH CHECK expression
> returns true for a row then that row is added, while if a false or null
> is returned then an error occurs.
> """

The CREATE POLICY documentation discusses how lack of a WITH CHECK
policy means the USING expression is used:

"""
Policies can be applied for specific commands or for specific roles. The
default for newly created policies is that they apply for all commands
and roles, unless otherwise specified. If multiple policies apply to a
given query, they will be combined using OR (although ON CONFLICT DO
UPDATE and INSERT policies are not combined in this way, but rather
enforced as noted at each stage of ON CONFLICT execution). Further, for
commands which can have both USING and WITH CHECK policies (ALL and
UPDATE), if no WITH CHECK policy is defined then the USING policy will
be used for both what rows are visible (normal USING case) and which
rows will be allowed to be added (WITH CHECK case).
"""

> So basically, USING filters out what you see, CHECK controls what you
> can write.

Right.

> But then this doesn't work correctly:
> 
> CREATE TABLE test1 (content text, entered_by text);
> ALTER TABLE test1 ENABLE ROW LEVEL SECURITY;
> CREATE POLICY test1_policy ON test1 FOR ALL TO PUBLIC USING (entered_by
> = current_user);
> GRANT ALL ON TABLE test1 TO PUBLIC;
> 
> CREATE USER foo1;
> SET SESSION AUTHORIZATION foo1;
> INSERT INTO test1 VALUES ('blah', 'foo2');  -- fails

You didn't specify a WITH CHECK policy and so the USING policy of
(entered_by = current_user) was used, as described above in the CREATE
POLICY documentation.

> This is a typical you-can-only-see-your-own-rows setup, which works for
> the reading case, but it evidently also controls writes.  So I'm not
> sure what the CHECK clause is supposed to add on top of that.

It could any number of additional checks; in this example perhaps
'content' which is being updated or newly added must have include
'Copyright 2015' or some such.

> (Btw., what's the meaning of a policy for DELETE?)

The DELETE policy controls what records a user is able to delete.

* Peter Eisentraut (pete...@gmx.net) wrote:
> On 9/23/15 2:52 PM, Stephen Frost wrote:
> >> That might be reasonable, but the documentation is completely wrong
> >> about that.
> > 
> > Really?  I feel pretty confident that it's at least mentioned.  I
> > agree that it should be made more clear.
> 
> I quoted the documentation at the beginning of the thread.  That's all I
> could find about it.

Hopefully the above helps.  There's a lot of information in the
individual POLICY commands, especially in CREATE POLICY.  Perhaps some
of that needs to be brought into the overall RLS section, but I'm not
sure we really want to duplicate it all.

> > USING is about visibility of existing records, WITH CHECK is in regards
> > to new rows being added to the relation (either through an INSERT or an
> > UPDATE).
> 
> That makes sense, but then the current behavior that I mentioned at the
> beginning of the thread is wrong.  If you think these clauses are
> clearly separate, then they should be, er, clearly separate.

They're not seperate as implemented and documented.  The current
discussion is about if we wish to change that.

> Maybe the syntax can be tweaked a little, like USING AND CHECK or
> whatever.  Not that USING and CHECK are terribly intuitive in this
> context anyway.

Ah, so that would be a fourth option along the lines of:

CREATE POLICY p1 ON t1
USING AND WITH CHECK ();

That'd certainly be straight-forward to implement.  Would we then
require the user to explicitly state the WITH CHECK piece, where it
applies, then?

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Wed, Sep 23, 2015 at 12:01 PM, Stephen Frost  wrote:
> > * Robert Haas (robertmh...@gmail.com) wrote:
> >> My expectation would have been:
> >>
> >> If you specify USING, you can see only those rows, but you can give
> >> rows away freely.  If you don't want to allow giving rows away under
> >> any circumstances, then specify the same expression for USING and WITH
> >> CHECK.
> >
> > Having an implicit 'true' for WITH CHECK would be very much against what
> > I would ever expect.  If anything, I'd think we would have an implicit
> > 'false' there or simply not allow it to ever be unspecified.
> 
> Huh?  If you had an implicit false, wouldn't that prevent updating or
> deleting any rows at all?

Right, just the same as how, if RLS is enabled and no explicit policies
are provided, non-owners can't see the rows or insert/update/delete
anything in the table.  The same is true for the GRANT system, where
there are no permissions granted by default.  I view the lack of an
explicit definition of a WITH CHECK clause to be the same, excepting the
simple case where it's the same as USING.

> >> I don't really get that.  If you could make skipping a row trigger an
> >> error, then that would create a bunch of covert channel attacks.
> >
> > Apparently I didn't explain it correctly.  Skipping a row doesn't
> > trigger an error.  An example would perhaps help here to clarify:
> >
> > CREATE POLICY p1 ON t1 FOR DELETE
> > USING (true)
> > WITH CHECK (inserted_by = current_user);
> >
> > What would happen above is that, in a DELETE case, you're allowed to
> > *try* and delete any record in the table, but if you try to delete a
> > record which isn't yours, we throw an error.  Currently the only option,
> > if you want to prevent users from deleteing records which are not
> > theirs, is to have:
> >
> > CREATE POLICY p1 ON t1 FOR DELETE
> > USING (inserted_by = current_user)
> >
> > Which certainly has the effect that you can only delete records you own,
> > but I can see use-cases where you'd like to know that someone tried to
> > delete a record which isn't their own and that isn't something you can
> > get directly today.
> 
> Well, you can use a trigger, I think.  But the point is that right
> now, if you try to delete a record that you don't own, it just says
> DELETE 0.  Maybe there was a record there that you can't see, and
> maybe there wasn't.

Yes, a trigger would also work for this.  I do understand that right now
the way it works is that there isn't an error thrown.  The notion was to
provide the administrator with the option.  The user in this case likely
would already have access to view the row or at least infer that the row
exists through a FK relationship.  These are all post-9.5 considerations
though.

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Peter Eisentraut]

On 9/23/15 2:52 PM, Stephen Frost wrote:
>> That might be reasonable, but the documentation is completely wrong
>> about that.
> 
> Really?  I feel pretty confident that it's at least mentioned.  I
> agree that it should be made more clear.

I quoted the documentation at the beginning of the thread.  That's all I
could find about it.

>> That said, why even have USING and CHECK as separate clauses?  Can't you
>> just create different policies if you want them different?
>>
>> Hypothetical example:
>>
>> CREATE POLICY p1 ON t1 FOR SELECT CHECK (extract(year from entered_on) =
>> extract(year from current_timestamp));
>> CREATE POLICY p2 ON t2 FOR INSERT, UPDATE, DELETE CHECK (entered_by =
>> current_user);
> 
> USING is about visibility of existing records, WITH CHECK is in regards
> to new rows being added to the relation (either through an INSERT or an
> UPDATE).

That makes sense, but then the current behavior that I mentioned at the
beginning of the thread is wrong.  If you think these clauses are
clearly separate, then they should be, er, clearly separate.

Maybe the syntax can be tweaked a little, like USING AND CHECK or
whatever.  Not that USING and CHECK are terribly intuitive in this
context anyway.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Alvaro Herrera]

Robert Haas wrote:
> On Wed, Sep 23, 2015 at 2:39 PM, Stephen Frost  wrote:
> > * Robert Haas (robertmh...@gmail.com) wrote:
> >> On Wed, Sep 23, 2015 at 12:01 PM, Stephen Frost  wrote:
> >> > * Robert Haas (robertmh...@gmail.com) wrote:
> >> >> My expectation would have been:
> >> >>
> >> >> If you specify USING, you can see only those rows, but you can give
> >> >> rows away freely.  If you don't want to allow giving rows away under
> >> >> any circumstances, then specify the same expression for USING and WITH
> >> >> CHECK.
> >> >
> >> > Having an implicit 'true' for WITH CHECK would be very much against what
> >> > I would ever expect.  If anything, I'd think we would have an implicit
> >> > 'false' there or simply not allow it to ever be unspecified.
> >>
> >> Huh?  If you had an implicit false, wouldn't that prevent updating or
> >> deleting any rows at all?
> >
> > Right, just the same as how, if RLS is enabled and no explicit policies
> > are provided, non-owners can't see the rows or insert/update/delete
> > anything in the table.  The same is true for the GRANT system, where
> > there are no permissions granted by default.  I view the lack of an
> > explicit definition of a WITH CHECK clause to be the same, excepting the
> > simple case where it's the same as USING.
> 
> Hmm, interesting.  I guess that's a defensible position, but I still
> think that having them default to be the same thing implicitly is
> kinda weird.  I'll defer to whatever the consensus, is, though.

I think an explicit statement of a "true" as WITH CHECK makes more sense
-- I think Stephen suggested it upthread as making the WITH CHECK be
mandatory.  If you really want to allow rows to be "given away" (which
could be a security issue), a "WITH CHECK (true)" is easy enough to
specify.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Robert Haas wrote:
> > On Wed, Sep 23, 2015 at 2:39 PM, Stephen Frost  wrote:
> > > * Robert Haas (robertmh...@gmail.com) wrote:
> > >> On Wed, Sep 23, 2015 at 12:01 PM, Stephen Frost  
> > >> wrote:
> > >> > * Robert Haas (robertmh...@gmail.com) wrote:
> > >> >> My expectation would have been:
> > >> >>
> > >> >> If you specify USING, you can see only those rows, but you can give
> > >> >> rows away freely.  If you don't want to allow giving rows away under
> > >> >> any circumstances, then specify the same expression for USING and WITH
> > >> >> CHECK.
> > >> >
> > >> > Having an implicit 'true' for WITH CHECK would be very much against 
> > >> > what
> > >> > I would ever expect.  If anything, I'd think we would have an implicit
> > >> > 'false' there or simply not allow it to ever be unspecified.
> > >>
> > >> Huh?  If you had an implicit false, wouldn't that prevent updating or
> > >> deleting any rows at all?
> > >
> > > Right, just the same as how, if RLS is enabled and no explicit policies
> > > are provided, non-owners can't see the rows or insert/update/delete
> > > anything in the table.  The same is true for the GRANT system, where
> > > there are no permissions granted by default.  I view the lack of an
> > > explicit definition of a WITH CHECK clause to be the same, excepting the
> > > simple case where it's the same as USING.
> > 
> > Hmm, interesting.  I guess that's a defensible position, but I still
> > think that having them default to be the same thing implicitly is
> > kinda weird.  I'll defer to whatever the consensus, is, though.
> 
> I think an explicit statement of a "true" as WITH CHECK makes more sense
> -- I think Stephen suggested it upthread as making the WITH CHECK be
> mandatory.  If you really want to allow rows to be "given away" (which
> could be a security issue), a "WITH CHECK (true)" is easy enough to
> specify.

Right, the options, in my view at least, are:

1) keep it as-is
2) make WITH CHECK mandatory
3) keep WITH CHECK optional, but default it to 'false' instead

If an administrator really wants WITH CHECK to be 'true', then they can
always add that clause in explicitly, but that really shouldn't be the
default.

For my part at least, I'm still preferring #1, but if there's a
consensus around #2 or #3 among the others interested then I'm happy to
make the actual code changes required.

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Charles Clavadetscher]

Hello Peter

> I'm testing the new row-level security feature.  I'm not clear on the
> difference between the USING and CHECK clauses in the CREATE POLICY
> statement.
> 
> The documentation says:
> 
> """
> A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
> which match the relevant policy expression. Existing table rows are
> checked against the expression specified via USING, while new rows that
> would be created via INSERT or UPDATE are checked against the expression
> specified via WITH CHECK. When a USING expression returns true for a
> given row then that row is visible to the user, while if a false or null
> is returned then the row is not visible. When a WITH CHECK expression
> returns true for a row then that row is added, while if a false or null
> is returned then an error occurs.
> """
> 
> So basically, USING filters out what you see, CHECK controls what you
> can write.

Yes, for the command that you specified in the FOR clause. This is quite 
important if you need different conditions for different commands, e.g. see all 
rows, modify only some.

This may help to better understand how this is meant:
http://www.postgresql.org/message-id/20150711132144.gs12...@tamriel.snowman.net

> But then this doesn't work correctly:
> 
> CREATE TABLE test1 (content text, entered_by text);
> ALTER TABLE test1 ENABLE ROW LEVEL SECURITY;
> CREATE POLICY test1_policy ON test1 FOR ALL TO PUBLIC USING (entered_by
> = current_user);
> GRANT ALL ON TABLE test1 TO PUBLIC;
> 
> CREATE USER foo1;
> SET SESSION AUTHORIZATION foo1;
> INSERT INTO test1 VALUES ('blah', 'foo2');  -- fails
> 
> This is a typical you-can-only-see-your-own-rows setup, which works for
> the reading case, but it evidently also controls writes. So I'm not
> sure what the CHECK clause is supposed to add on top of that.

Since the policy is defined for ALL commands and no WITH CHECK is specified 
then the same condition defined in USING takes effect for all commands, i.e. 
including INSERT.
 
>From the docs 
>(http://www.postgresql.org/docs/9.5/static/sql-createpolicy.html): "Further, 
>for commands which can have both USING and WITH CHECK policies (ALL and 
>UPDATE), if no WITH CHECK policy is defined then the USING policy will be used 
>for both what rows are visible (normal USING case) and which rows will be 
>allowed to be added (WITH CHECK case)."

If you want e.g. to allow users to insert rows without the restriction of being 
the current_user in column entered_by then you would need separate policies for 
each command. If you define a policy for INSERT, USING does not make sense. In 
the thread above there is a similar example to this as well as in the 
documentation:

http://www.postgresql.org/docs/9.5/static/ddl-rowsecurity.html

> (Btw., what's the meaning of a policy for DELETE?)

In your example it means that users can delete only the rows where entered_by = 
current_user. A WITH CHECK policy does not make sense in this case.

I assume that having USING and WITH CHECK for filtering and controlling added 
rows was introduced for use cases where these conditions are not the same, i.e. 
to allow for more flexibility. On the spot I don't have an example, but maybe 
somebody else can deliver one.

Regards
Charles




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers