Обсуждение: [HACKERS] RLS policy not getting honer while pg_dump on declarative partition

Greetings,

* Rushabh Lathia (rushabh.lathia@gmail.com) wrote:
> While doing some testing I noticed that RLS policy not getting honer
> while pg_dump on declarative partition.
>
> I can understand that while doing SELECT on individual child
> table, policy of parent is not getting applied. But is this desirable
> behaviour? I think for partitions, any policy on the root table should
> get redirect to the child, thoughts?
>
> If current behaviour is desirable then atleast we should document this.

The current behaviour matches how the GRANT system works, unless it's
been changed as part of the partitioning patches, we don't check the
privileges on tthe parent to see if an individual has access to the
child.

I think we could certainly consider if this behavior is desirable in a
system which includes partitioning instead of inheritance, but if we
wish to do so then I think we should be considering if the GRANT system
should also be changed as I do feel the two should be consistent.

Thinking it through a bit though, I would imagine someone certainly
might want to GRANT access to a given partition and not others, though
that could actually be done with an appropriate RLS policy on the
parent, but until we improve the performance of constraint exclusion (or
change entirely how all of that works with partitions...), I'm not sure
that's a practical answer in all cases.  It might also be the case that
one would wish for different policies to be used when a user is
accessing a table directly vs. going through the parent.

Thanks!

Stephen

On 2017/06/17 9:20, Stephen Frost wrote:
> Greetings,
> 
> * Rushabh Lathia (rushabh.lathia@gmail.com) wrote:
>> While doing some testing I noticed that RLS policy not getting honer
>> while pg_dump on declarative partition.
>>
>> I can understand that while doing SELECT on individual child
>> table, policy of parent is not getting applied. But is this desirable
>> behaviour? I think for partitions, any policy on the root table should
>> get redirect to the child, thoughts?
>>
>> If current behaviour is desirable then atleast we should document this.
> 
> The current behaviour matches how the GRANT system works, unless it's
> been changed as part of the partitioning patches, we don't check the
> privileges on tthe parent to see if an individual has access to the
> child.

The partitioning patch itself (either f0e44751d71 or any subsequent
patches) didn't change any of how that works.  One would get the same
behavior as one would with inheritance, as described in the inheritance
section:

https://www.postgresql.org/docs/devel/static/ddl-inherit.html

"Inherited queries perform access permission checks on the parent table
only. Thus, for example, granting UPDATE permission on the cities table
implies permission to update rows in the capitals table as well, when they
are accessed through cities. This preserves the appearance that the data
is (also) in the parent table. But the capitals table could not be updated
directly without an additional grant. In a similar way, the parent table's
row security policies (see Section 5.7) are applied to rows coming from
child tables during an inherited query. A child table's policies, if any,
are applied only when it is the table explicitly named in the query; and
in that case, any policies attached to its parent(s) are ignored."

> I think we could certainly consider if this behavior is desirable in a
> system which includes partitioning instead of inheritance,

Do we want CREATE POLICY foo ON parent creating the policy objects for all
the partitions?  That is, cascade the policy object definition?

> but if we
> wish to do so then I think we should be considering if the GRANT system
> should also be changed as I do feel the two should be consistent.

IIUC, you are saying here that GRANT should be taught to cascade the
permission grant/revokes to partitions.


Also, the somewhat related nearby discussion about dumping the partition
data through the root parent will perhaps have to think about some of
these things.  Dumping data through the root table will take care of the
problem that Rushabh is complaining about, because only rows visible per
the parent's policies would be dumped.  Of course then the the set of rows
dumped will be different from what it is today, because one would expect
that a different set of policies would get applied - the root table's
policies when dumping data through it vs. the individual partitions'
policies when dumping data per partition.

Thanks,
Amit

Amit,

* Amit Langote (Langote_Amit_f8@lab.ntt.co.jp) wrote:
> On 2017/06/17 9:20, Stephen Frost wrote:
> > I think we could certainly consider if this behavior is desirable in a
> > system which includes partitioning instead of inheritance,
>
> Do we want CREATE POLICY foo ON parent creating the policy objects for all
> the partitions?  That is, cascade the policy object definition?

That seems to be what the OP is suggesting we should have.  I'm
certainly not convinced that we actually do want that though.  There are
clear reasons why it doesn't make sense for inheritance that aren't an
issue for partitions (for one thing, we don't have to worry about the
visible columns being different between the partitioned table and the
partitions), but that doesn't necessairly mean we want to make this
different for partitions vs. inheritance.

In any case though, I do tend to feel that it's rather too late to
consider changing things for PG10 in this area, even if we all felt that
it was the right/correct thing to do, which isn't clear.

> > but if we
> > wish to do so then I think we should be considering if the GRANT system
> > should also be changed as I do feel the two should be consistent.
>
> IIUC, you are saying here that GRANT should be taught to cascade the
> permission grant/revokes to partitions.

What I'm saying here is that the way GRANT works and the way policies
are applied to partitioned tables should be consistent with each other.

> Also, the somewhat related nearby discussion about dumping the partition
> data through the root parent will perhaps have to think about some of
> these things.  Dumping data through the root table will take care of the
> problem that Rushabh is complaining about, because only rows visible per
> the parent's policies would be dumped.  Of course then the the set of rows
> dumped will be different from what it is today, because one would expect
> that a different set of policies would get applied - the root table's
> policies when dumping data through it vs. the individual partitions'
> policies when dumping data per partition.

While somewhat related, I don't think we should allow concerns about
pg_dump to drive what we're doing in the backend.  If pg_dump isn't
smart enough to allow different ways to dump the data out given the
considerations for what the backend supports/does, then we should add
new features to pg_dump, not reconsider what we're doing in the backend.

Thanks!

Stephen