Testing RLS with SECURITY DEFINER functions returning refcursors

From: Craig Ringer <craig(at)2ndquadrant(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Subject: Testing RLS with SECURITY DEFINER functions returning refcursors
Date: 2013-10-24 03:20:39
Message-ID: 52689207.4060605@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all

I'm going to be contributing a fair bit of time to RLS for 2ndQuadrant,
courtesy of the EU AXLE project (http://axleproject.eu/)

I've been catching up on Kohei KaiGai's work and I've been really
impressed by what's already done and working. I'm currently reading the
patches, mailing list discussion, and the tests.

Prompted by mailing list discussion on the topic, I added some tests to
the 9.4 v4 RLS patch to check behaviour around SECURITY DEFINER
functions and found a bit of an issue.

If one non-superuser user2 with limited row access creates a SECURITY
DEFINER function that returns a refcursor, and the other user user1
fetches from the cursor, the returned result set is what user1 sees when
selecting the table directly, not what user2 sees when it selects the
table. That is inconsistent with how SECURITY DEFINER behaves for other
rights. It's also inconsistent with a superuser-owned SECURITY DEFINER
function, where RLS doesn't add the predicate at all so all rows are
returned.

Another issue occurs when the superuser invokes the SECURITY DEFINER
function created by user2. There are no rows returned from a fetch of
the refcursor. This makes sense given that in the test the RLS condition
is "(dauthor = "current_user"())" and there are no rows with dauthor set
to the superuser's username.

This asymmetry is a bug. Either RLS should be applied consistently for
the definer, or consistently as the caller. Currently it's the caller
unless the definer is superuser, in which case no checks are applied
because the RLS predicate never gets applied.

I'm doing these tests on top of the tables defined by the rowsecurity
test suite in the patch.

On a side note, I also noticed that while psql's \dt+ supports RLS, \d
or \d+ doesn't provide any indication that there's an RLS policy or what
the conditions are.

Anyway - the additional tests are attached, and can also be found in
https://github.com/ringerc/postgres/tree/rls-9.4 along with a patched
expected file showing what I think _should_ be happening. Comments would
be appreciated.

I'm also interested in more details on the mention of "functions that
change the current user ID during a query" that came up in prior RLS
discussion.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachment Content-Type Size
rls-9.4-v4-security-definer-tests.diff text/x-patch 2.5 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message Nigel Heron 2013-10-24 03:48:03 Re: stats for network traffic WIP
Previous Message David Johnston 2013-10-24 02:53:31 Re: Using indexes for ORDER BY and PARTITION BY clause in windowing functions