Re: A little RLS oversight?

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: Joe Conway <mail(at)joeconway(dot)com>, Joe Conway <joe(dot)conway(at)crunchydata(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Yaroslav <ladayaroslav(at)yandex(dot)ru>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: A little RLS oversight?
Date: 2015-07-27 20:58:19
Message-ID: 20150727205819.GK3587@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dean,

* Dean Rasheed (dean(dot)a(dot)rasheed(at)gmail(dot)com) wrote:
> On 27 July 2015 at 18:13, Joe Conway <mail(at)joeconway(dot)com> wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > On 07/27/2015 10:03 AM, Joe Conway wrote:
> >> On 07/26/2015 07:59 AM, Joe Conway wrote:
> >>> On 07/26/2015 07:19 AM, Dean Rasheed wrote:
> >>>> Attached is an updated patch (still needs some docs for the
> >>>> functions).
> >>>
> >>> Thanks for that. I'll add the docs.
> >>
> >> Documentation added. Also added comment to check_enable_rls about
> >> passing InvalidOid versus GetUserId().
> >>
> >> I believe this is ready to go -- any other comments?
> >
> > Strike that - now I really think it is ready to go :-)
> >
> > In this patch I additionally changed instances of:
> > check_enable_rls(indrelid, GetUserId(), true)
> > to
> > check_enable_rls(indrelid, InvalidOid, true)
> > per Dean's earlier remark and my new comment.
>
> Looks good to me, except I'm not sure about those latest changes
> because I don't understand the reasoning behind the logic in
> check_enable_rls() when row_security is set to OFF.
>
> I would expect that if the current user has permission to bypass RLS,
> and they have set row_security to OFF, then it should be off for all
> tables that they have access to, regardless of how they access those
> tables (directly or through a view). If it really is intentional that
> RLS remains active when querying through a view not owned by the
> table's owner, then the other calls to check_enable_rls() should
> probably be left as they were, since the table might have been updated
> through such a view and that code can't really tell at that point.

Joe and I were discussing this earlier and it was certainly intentional
that RLS still be enabled if you're querying through a view as the RLS
rights of the view owner are used, not your own. Note that we don't
allow a user to assume the BYPASSRLS right of the view owner though,
also intentionally.

As a comparison to what we do today, even if you have access to a table,
if you query it through a view, it's the view owner's permissions which
are used to determine access to the table through the view, not your
own. I agree that can be a bit odd at times, as you can get a
permission denied error when using the view even though you have access
to the table which is complained about, but that's how views have worked
for quite a long time.

Thanks!

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2015-07-27 21:02:31 Re: copy.c handling for RLS is insecure
Previous Message Simon Riggs 2015-07-27 20:40:01 Re: optimizing vacuum truncation scans