Re: copy.c handling for RLS is insecure - Mailing list pgsql-hackers

From Noah Misch
Subject Re: copy.c handling for RLS is insecure
Date
Msg-id 20150709052828.GC998998@tornado.leadboat.com
Whole thread Raw
In response to Re: copy.c handling for RLS is insecure  (Stephen Frost <sfrost@snowman.net>)
Responses Re: copy.c handling for RLS is insecure
List pgsql-hackers
On Wed, Jul 08, 2015 at 10:55:47AM -0400, Stephen Frost wrote:
> It's interesting to consider that COPY purportedly operates under the
> SELECT privilege, yet fails to respect on-select rules.

In released branches, COPY consistently refuses to operate directly on a view.
(There's no (longer?) such thing as a table with an ON SELECT rule.  CREATE
RULE "_RETURN" AS ON SELECT ... converts a table to a view.)

> Having spent a bit of time thinking about this now, it occurs to me that
> we've drifted from the original concern and are now trying to solve an
> insolvable issue here.  We're not trying to prevent against an attacker
> who owns the table we're going to COPY and wants to get us to run code
> they've written- that can happen by simply having RLS and that's why
> it's not enabled by default for superusers and why we have
> 'row_security = off', which pg_dump sets by default.

The problem I wanted to see solved was the fact that, by running a DDL command
concurrent with a "COPY rel TO" command, you can make the COPY read from a
view.  That is not possible in any serial execution of COPY with DDL.  Now,
you make a good point that before this undesirable outcome can happen, the
user issuing the COPY command will have already trusted the roles that can
pass owner checks for "rel".  That probably makes this useless as an attack
tool.  Nonetheless, I don't want "COPY rejects views" to soften into "COPY
rejects views, except in XYZ race condition."

> After a bit of discussion with Andres, my thinking on this is to do the
> following:
> 
> - Fully qualify the name based on the opened relation
> - Keep the initial lock on the relation throughout
> - Remove the Assert() (other relations can be pulled in by RLS)

That's much better.  We have considerable experience with designs like that.

> - Keep the OID check, shouldn't hurt to have it

What benefit is left?  The planner does not promise any particular order
within relationOids, and an order change could make false alarms here.



pgsql-hackers by date:

Previous
From: Noah Misch
Date:
Subject: Re: Solaris testers wanted for strxfrm() behavior
Next
From: Pavel Stehule
Date:
Subject: Re: Implementation of global temporary tables?