Re: [PATCH] Add reloption for views to enable RLS

From: walther(at)technowledgy(dot)de
To: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Christoph Heiss <christoph(dot)heiss(at)cybertec(dot)at>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Add reloption for views to enable RLS
Date: 2022-02-09 16:40:01
Message-ID: 07369f06-5f9e-5ab3-942d-91b7c918bbe4@technowledgy.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Laurenz Albe:
> So even though the view owner "duff" has no permissions
> on the schema "viewtest", we can still select from the table.
> Permissions on the schema containing the table are not
> checked, only permissions on the table itself.
>
> [...]
>
> If not, I don't know if it is the business of this patch to
> change the behavior.

Ah, good find. In that case, I suggest to change the docs slightly to
say that the schema will not be checked.

In one place it's described as "it will cause all access to the
underlying tables to be checked as ..." which is fine, I think. But in
another place it's "access to tables, functions and *other objects*
referenced in the view, ..." which is misleading.

> I agree that the name "security_invoker" is suggestive of SECURITY INVOKER
> in CREATE FUNCTION, but the behavior is different.
> Perhaps the solution is as simple as choosing a different name that does
> not prompt this association, for example "permissions_invoker".

Yes, given that there is not much that can be done about the
functionality anymore, a different name would be better. This would also
avoid the implicit "if security_invoker=false, the view behaves like
SECURITY DEFINER" association, which is also clearly wrong. And this
assumption is actually what made me think the chained views example was
somehow off.

I am not convinced "permissions_invoker" is much better, though. The
difference between SECURITY INVOKER and SECURITY DEFINER is invoker vs
definer... where, I think, we need something else to describe what we
currently have and what the patch provides.

Maybe we can look at it from the other perspective: Both ways of
operating keep the CURRENT_USER the same, pretty much like what we
understand "security invoker" should do. The difference, however, is the
current default in which the permissions are checked with the view
*owner*. Let's treat this difference as the thing that can be set:
security_owner=true|false. Or run_as_owner=true|false.

xxx_owner=true would be the default and xxx_owner=false could be set
explicitly to get the behavior we are looking for in this patch?

> I guess more documentation how this works would be a good idea.
> [...] but since it exposes this
> in new ways, it might as well clarify how all this works.

+1

Best

Wolfgang

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message 603_Annada Dash 2022-02-09 17:13:42 How to get started with contribution
Previous Message Alvaro Herrera 2022-02-09 16:25:08 Re: Database-level collation version tracking