Re: A little RLS oversight? - Mailing list pgsql-hackers

From Joe Conway
Subject Re: A little RLS oversight?
Date
Msg-id 55B7C750.2080902@joeconway.com
Whole thread Raw
In response to Re: A little RLS oversight?  (Dean Rasheed <dean.a.rasheed@gmail.com>)
Responses Re: A little RLS oversight?  (Joe Conway <joe.conway@crunchydata.com>)
List pgsql-hackers
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/28/2015 12:32 AM, Dean Rasheed wrote:
>> On 07/27/2015 03:05 PM, Stephen Frost wrote:
>>> AFK at the moment, but my thinking was that we should avoid
>>> having the error message change based on what a GUC is set to.
>>> I agree that there should be comments which explain that.
>
> Except it's already dependent on the GUC if it's set to FORCE.

Dean is correct. See test case below...

>> I changed back to using GetUserId() for the call to
>> check_enable_rls() at those three call sites, and added to the
>> comments to explain why.
>>
>
> I'm not entirely convinced about this. The more I think about it,
> the more I think that if we know the user has BYPASSRLS, and
> they've set row_security to OFF, then they ought to get the more
> detailed error message, as they would if there was no RLS. That
> error detail is highly useful, and we know the user has been
> granted privilege by a superuser, and that they have direct access
> to the underlying table in this context, so we're not leaking any
> info that they cannot directly SELECT anyway.

Agreed -- this patch version goes back to using InvalidOid at those
three call sites and the behavior is what I now believe to be correct.
Here is a test case to illustrate:

8<--------------------
BEGIN;
CREATE ROLE alice;
CREATE ROLE bob WITH BYPASSRLS;

SET SESSION AUTHORIZATION alice;
CREATE TABLE t1 (id int primary key, f1 text);
INSERT INTO t1 VALUES(1,'a');
CREATE TABLE t2 (id int primary key, f1 text, t1_id int REFERENCES
t1(id));
GRANT ALL ON t2 TO bob;
ALTER TABLE t2 ENABLE ROW LEVEL SECURITY;
CREATE POLICY P ON t2 TO alice, bob USING (true);

SET SESSION AUTHORIZATION bob;
INSERT INTO t2 VALUES(1,'a',1); -- should succeed

SAVEPOINT q;
INSERT INTO t2 VALUES(2,'b',2); -- should fail, no details
ROLLBACK TO q;

SET row_security = OFF;
SAVEPOINT q;
INSERT INTO t2 VALUES(2,'b',2); -- should fail, full details
ROLLBACK TO q;

SET SESSION AUTHORIZATION alice;
SAVEPOINT q;
INSERT INTO t2 VALUES(2,'b',2); -- should fail, full details
ROLLBACK TO q;

SET row_security = FORCE;
SAVEPOINT q;
INSERT INTO t2 VALUES(2,'b',2); -- should fail, no details
ROLLBACK TO q;

ROLLBACK;
8<--------------------

I'm going to commit the attached in the next few hours unless someone
has serious objections. We can always revisit the specific behavior of
those messages separately if we change our minds...

Joe

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVt8dQAAoJEDfy90M199hl1ncQAI/XUZ3VSoW0Vegf09Y2DqlJ
f8lGnwSf+djSXgKVrUsKQsuLn7c+Ac9fqoRUQJMuCcOvnu7auljzaMjuMjrXhOIC
hhiP8QyYUoEMF+5Sggh/A532rFXRbI1R/g9eu8TTT9vJGkITVMGAucizSY+fPiBg
gH3JyuCVaIZbvlVv0OkqPXPiP1VR/7bDTBcIbv56XHOk1AavNlUMW5BWWR0/9Mt8
VMh9ri3eQ5beKxrDhAZ+39ddlQzk9yJsN5pd/Pu0zPNxwBcvTNra0ZZGv7PoPwUF
7F98A1bL/NWFDdFuOI2E61/a5lA70t5HV4UTPPugQr82NhBS9JdpxgqA8W7B9+P9
4TKqmmYIvOcxM+TtglIlyr+JBwfJERw8j3+IcnM3mjLnkyflNbX2kbOF0B5Ghpt/
EzrVIJi/Pl3ctm+9r/oQYiwo/6Qsy8hco9QLCY4GVhBEE93Wr8P6NVlcjzyocMRs
FBjgvxeL/1wL8g3Q8ZDsAVOu9Ld0OCGEkA27XRS3sXbZfHroeTNW5aUqvKIzFkKB
gsr09pIVtdd7ysEdxxHZpELaU8H2rcA5O8b380HauIi41GaDc5E0XLXJSu6dIWCP
x/Em3qTpt74YgZiqsbs3a21Ak5n8fBdTMyXhmPQbXctllALI3Kj7bbyqoeGywpxi
PKhGDzgw+M7OQzfWS7UF
=e5Qo
-----END PGP SIGNATURE-----

Attachment

pgsql-hackers by date:

Previous
From: Josh Berkus
Date:
Subject: Re: Shouldn't we document "don't use a mountpoint as $PGDATA"?
Next
From: Andrew Dunstan
Date:
Subject: Re: Shouldn't we document "don't use a mountpoint as $PGDATA"?