Thread: [HACKERS] RLS policy not getting honer while pg_dump on declarative partition
[HACKERS] RLS policy not getting honer while pg_dump on declarative partition
From
Rushabh Lathia
Date:
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.
Consider the below test:
\c postgres rushabh
CREATE USER rls_test_user1;
CREATE TABLE tp_sales
(
visibility VARCHAR(30),
sales_region VARCHAR(30)
) PARTITION BY LIST (sales_region);
create table tp_sales_p_india partition of tp_sales for values in ('INDIA');
create table tp_sales_p_rest partition of tp_sales for values in ('REST');
insert into tp_sales values ( 'hidden', 'INDIA');
insert into tp_sales values ( 'visible', 'INDIA');
insert into tp_sales values ( 'hidden', 'REST');
insert into tp_sales values ( 'visible', 'REST');
GRANT SELECT ON tp_sales to rls_test_user1;
GRANT SELECT ON tp_sales_p_india to rls_test_user1;
GRANT SELECT ON tp_sales_p_rest to rls_test_user1;
ALTER TABLE tp_sales ENABLE ROW LEVEL SECURITY;
CREATE POLICY dump_p1 ON tp_sales FOR ALL USING (visibility = 'visible');
\c - rls_test_user1
-- SELECT honer the policy
SELECT * FROM tp_sales;
When we run the pg_dump using user rls_test_user1, can see the hidden
rows in the pg_dump output.
./db/bin/pg_dump -U rls_test_user1 postgres --inserts
Attaching the dump output.
Thanks,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.
Consider the below test:
\c postgres rushabh
CREATE USER rls_test_user1;
CREATE TABLE tp_sales
(
visibility VARCHAR(30),
sales_region VARCHAR(30)
) PARTITION BY LIST (sales_region);
create table tp_sales_p_india partition of tp_sales for values in ('INDIA');
create table tp_sales_p_rest partition of tp_sales for values in ('REST');
insert into tp_sales values ( 'hidden', 'INDIA');
insert into tp_sales values ( 'visible', 'INDIA');
insert into tp_sales values ( 'hidden', 'REST');
insert into tp_sales values ( 'visible', 'REST');
GRANT SELECT ON tp_sales to rls_test_user1;
GRANT SELECT ON tp_sales_p_india to rls_test_user1;
GRANT SELECT ON tp_sales_p_rest to rls_test_user1;
ALTER TABLE tp_sales ENABLE ROW LEVEL SECURITY;
CREATE POLICY dump_p1 ON tp_sales FOR ALL USING (visibility = 'visible');
\c - rls_test_user1
-- SELECT honer the policy
SELECT * FROM tp_sales;
When we run the pg_dump using user rls_test_user1, can see the hidden
rows in the pg_dump output.
./db/bin/pg_dump -U rls_test_user1 postgres --inserts
Attaching the dump output.
Attachment
Re: [HACKERS] RLS policy not getting honer while pg_dump ondeclarative partition
From
Stephen Frost
Date:
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
Re: [HACKERS] RLS policy not getting honer while pg_dump ondeclarative partition
From
Amit Langote
Date:
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
Re: [HACKERS] RLS policy not getting honer while pg_dump ondeclarative partition
From
Stephen Frost
Date:
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