From: | Noah Misch <noah(at)leadboat(dot)com> |
---|---|
To: | Stephen Frost <sfrost(at)snowman(dot)net> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: copy.c handling for RLS is insecure |
Date: | 2015-07-09 05:28:28 |
Message-ID: | 20150709052828.GC998998@tornado.leadboat.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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.
From | Date | Subject | |
---|---|---|---|
Next Message | Zhaomo Yang | 2015-07-09 05:32:26 | Re: Implementation of global temporary tables? |
Previous Message | Noah Misch | 2015-07-09 05:18:14 | Re: Solaris testers wanted for strxfrm() behavior |