BUG #15708: RLS 'using' running as wrong user when called from a view
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
Attachments:
rls-perm-check-fix.patchapplication/octet-stream; name=rls-perm-check-fix.patchDownload+77-0
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
Attachments:
rls-perm-check-fix-v2.patchtext/x-patch; charset=US-ASCII; name=rls-perm-check-fix-v2.patchDownload+61-0
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]https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=e2d28c0f404713f564dc2250646551c75172f17b (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