Thread: BUG #15708: RLS 'using' running as wrong user when called from a view
BUG #15708: RLS 'using' running as wrong user when called from a view
From
PG Bug reporting form
Date:
The following bug has been logged on the website: Bug reference: 15708 Logged by: Daurnimator Email address: quae@daurnimator.com PostgreSQL version: 11.2 Operating system: linux Description: (from https://gist.github.com/daurnimator/b1d2c16359e346a466b3093ae2757acf ) This fails, seemingly because the RLS on 'bar' is being checked by alice, instead of the view owner bob: ```sql create role alice; create table bar(a integer); alter table bar enable row level security; create table qux(b integer); create role bob; create policy blahblah on bar to bob using(exists(select 1 from qux)); grant select on table bar to bob; grant select on table qux to bob; create view foo as select * from bar; alter view foo owner to bob; grant select on table foo to alice; -- grant select on table qux to alice; -- shouldn't be required set role alice; select * from foo; ``` ``` $ psql -f rls_trouble.sql CREATE ROLE CREATE TABLE ALTER TABLE CREATE TABLE CREATE ROLE CREATE POLICY GRANT GRANT CREATE VIEW ALTER VIEW GRANT SET psql:rls_trouble.sql:18: ERROR: permission denied for table qux ``` If we add an indirection via another view, then I get the result I expected... ```sql create role alice; create table bar(a integer); alter table bar enable row level security; create table qux(b integer); -- if we add a layer of indirection it works.... wat? create view indirection as select * from bar; create role bob; create policy blahblah on bar to bob using(exists(select 1 from qux)); grant select on table bar to bob; grant select on table indirection to bob; grant select on table qux to bob; create view foo as select * from indirection; alter view foo owner to bob; grant select on table foo to alice; set role alice; select * from foo; ```
On Thu, 21 Mar 2019 at 00:39, PG Bug reporting form <noreply@postgresql.org> wrote: > > This fails, seemingly because the RLS on 'bar' is being checked by alice, > instead of the view owner bob: > Yes I agree, that appears to be a bug. The subquery in the RLS policy should be checked as the view owner -- i.e., we need to propagate the checkAsUser for the RTE with RLS to any subqueries in its RLS policies. It looks like the best place to fix it is in get_policies_for_relation(), since that's where all the policies to be applied for a given RTE are pulled together. Patch attached. Regards, Dean
On Thu, 21 Mar 2019 at 00:39, PG Bug reporting form <noreply@postgresql.org> wrote: > > This fails, seemingly because the RLS on 'bar' is being checked by alice, > instead of the view owner bob: > Yes I agree, that appears to be a bug. The subquery in the RLS policy should be checked as the view owner -- i.e., we need to propagate the checkAsUser for the RTE with RLS to any subqueries in its RLS policies. It looks like the best place to fix it is in get_policies_for_relation(), since that's where all the policies to be applied for a given RTE are pulled together. Patch attached. Regards, Dean
Attachment
Greetings, * Dean Rasheed (dean.a.rasheed@gmail.com) wrote: > On Thu, 21 Mar 2019 at 00:39, PG Bug reporting form > <noreply@postgresql.org> wrote: > > > > This fails, seemingly because the RLS on 'bar' is being checked by alice, > > instead of the view owner bob: > > Yes I agree, that appears to be a bug. The subquery in the RLS policy > should be checked as the view owner -- i.e., we need to propagate the > checkAsUser for the RTE with RLS to any subqueries in its RLS > policies. Agreed. > It looks like the best place to fix it is in > get_policies_for_relation(), since that's where all the policies to be > applied for a given RTE are pulled together. Patch attached. Yes, on a quick review, that looks like a good solution to me as well. Thanks! Stephen
Attachment
Greetings, * Dean Rasheed (dean.a.rasheed@gmail.com) wrote: > On Thu, 21 Mar 2019 at 00:39, PG Bug reporting form > <noreply@postgresql.org> wrote: > > > > This fails, seemingly because the RLS on 'bar' is being checked by alice, > > instead of the view owner bob: > > Yes I agree, that appears to be a bug. The subquery in the RLS policy > should be checked as the view owner -- i.e., we need to propagate the > checkAsUser for the RTE with RLS to any subqueries in its RLS > policies. Agreed. > It looks like the best place to fix it is in > get_policies_for_relation(), since that's where all the policies to be > applied for a given RTE are pulled together. Patch attached. Yes, on a quick review, that looks like a good solution to me as well. Thanks! Stephen
On Mon, 25 Mar 2019 at 20:27, Stephen Frost <sfrost@snowman.net> wrote: > > * Dean Rasheed (dean.a.rasheed@gmail.com) wrote: > > > It looks like the best place to fix it is in > > get_policies_for_relation(), since that's where all the policies to be > > applied for a given RTE are pulled together. Patch attached. > > Yes, on a quick review, that looks like a good solution to me as well. > On second thoughts, it actually needs to be in get_row_security_policies(), after making copies of the quals from the policies, otherwise it would be scribbling on the copies from the relcache. Actually that makes the code change a bit simpler too. Regards, Dean
Attachment
On Mon, 25 Mar 2019 at 20:27, Stephen Frost <sfrost@snowman.net> wrote: > > * Dean Rasheed (dean.a.rasheed@gmail.com) wrote: > > > It looks like the best place to fix it is in > > get_policies_for_relation(), since that's where all the policies to be > > applied for a given RTE are pulled together. Patch attached. > > Yes, on a quick review, that looks like a good solution to me as well. > On second thoughts, it actually needs to be in get_row_security_policies(), after making copies of the quals from the policies, otherwise it would be scribbling on the copies from the relcache. Actually that makes the code change a bit simpler too. Regards, Dean
On Wed, 27 Mar 2019 at 23:46, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > On second thoughts, it actually needs to be in > get_row_security_policies(), after making copies of the quals from the > policies, otherwise it would be scribbling on the copies from the > relcache. Actually that makes the code change a bit simpler too. Thanks for writing the patch! I'm sad this missed the last commit fest; I think this bug might be causing security issues in a few deployments. Could you submit the patch for the next commit fest?
On Wed, 27 Mar 2019 at 23:46, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > On second thoughts, it actually needs to be in > get_row_security_policies(), after making copies of the quals from the > policies, otherwise it would be scribbling on the copies from the > relcache. Actually that makes the code change a bit simpler too. Thanks for writing the patch! I'm sad this missed the last commit fest; I think this bug might be causing security issues in a few deployments. Could you submit the patch for the next commit fest?
On Mon, 29 Apr 2019 at 04:56, Daurnimator <quae@daurnimator.com> wrote: > > On Wed, 27 Mar 2019 at 23:46, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > > On second thoughts, it actually needs to be in > > get_row_security_policies(), after making copies of the quals from the > > policies, otherwise it would be scribbling on the copies from the > > relcache. Actually that makes the code change a bit simpler too. > > Thanks for writing the patch! > > I'm sad this missed the last commit fest; I think this bug might be > causing security issues in a few deployments. > Could you submit the patch for the next commit fest? Actually I pushed the fix for this a while ago [1] (sorry I forgot to reply back to this thread), so it will be available in the next set of minor version updates later this week. Regards, Dean [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=e2d28c0f404713f564dc2250646551c75172f17b
On Mon, 29 Apr 2019 at 04:56, Daurnimator <quae@daurnimator.com> wrote: > > On Wed, 27 Mar 2019 at 23:46, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > > On second thoughts, it actually needs to be in > > get_row_security_policies(), after making copies of the quals from the > > policies, otherwise it would be scribbling on the copies from the > > relcache. Actually that makes the code change a bit simpler too. > > Thanks for writing the patch! > > I'm sad this missed the last commit fest; I think this bug might be > causing security issues in a few deployments. > Could you submit the patch for the next commit fest? Actually I pushed the fix for this a while ago [1] (sorry I forgot to reply back to this thread), so it will be available in the next set of minor version updates later this week. Regards, Dean [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=e2d28c0f404713f564dc2250646551c75172f17b