Re: [HACKERS] copy.c handling for RLS is insecure

2015-07-27 Thread Stephen Frost
All, * Stephen Frost (sfr...@snowman.net) wrote: * Andres Freund (and...@anarazel.de) wrote: On 2015-07-09 01:28:28 -0400, Noah Misch wrote: - Keep the OID check, shouldn't hurt to have it What benefit is left? A bit of defense in depth. We execute user defined code in COPY

Re: [HACKERS] copy.c handling for RLS is insecure

2015-07-09 Thread Andres Freund
On 2015-07-09 01:28:28 -0400, Noah Misch wrote: - Keep the OID check, shouldn't hurt to have it What benefit is left? A bit of defense in depth. We execute user defined code in COPY (e.g. BEFORE triggers). That user defined code could very well replace the relation. Now I think right now

Re: [HACKERS] copy.c handling for RLS is insecure

2015-07-08 Thread Stephen Frost
Noah, * Noah Misch (n...@leadboat.com) wrote: On Tue, Dec 02, 2014 at 11:32:27AM -0500, Stephen Frost wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Nov 27, 2014 at 2:03 AM, Stephen Frost sfr...@snowman.net wrote: Alright, I've done the change to use the RangeVar from

Re: [HACKERS] copy.c handling for RLS is insecure

2015-07-08 Thread Noah Misch
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

Re: [HACKERS] copy.c handling for RLS is insecure

2015-07-06 Thread Stephen Frost
Noah, * Noah Misch (n...@leadboat.com) wrote: On Tue, Dec 02, 2014 at 11:32:27AM -0500, Stephen Frost wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Nov 27, 2014 at 2:03 AM, Stephen Frost sfr...@snowman.net wrote: Alright, I've done the change to use the RangeVar from

Re: [HACKERS] copy.c handling for RLS is insecure

2015-07-03 Thread Noah Misch
On Tue, Dec 02, 2014 at 11:32:27AM -0500, Stephen Frost wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Nov 27, 2014 at 2:03 AM, Stephen Frost sfr...@snowman.net wrote: Alright, I've done the change to use the RangeVar from CopyStmt, but also added a check wherein we verify

Re: [HACKERS] copy.c handling for RLS is insecure

2014-12-02 Thread Robert Haas
On Thu, Nov 27, 2014 at 2:03 AM, Stephen Frost sfr...@snowman.net wrote: Alright, I've done the change to use the RangeVar from CopyStmt, but also added a check wherein we verify that the relation's OID returned from the planned query is the same as the relation's OID that we did the RLS check

Re: [HACKERS] copy.c handling for RLS is insecure

2014-12-02 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: On Thu, Nov 27, 2014 at 2:03 AM, Stephen Frost sfr...@snowman.net wrote: Alright, I've done the change to use the RangeVar from CopyStmt, but also added a check wherein we verify that the relation's OID returned from the planned query is the same

Re: [HACKERS] copy.c handling for RLS is insecure

2014-11-26 Thread Stephen Frost
Robert, * Stephen Frost (sfr...@snowman.net) wrote: * Robert Haas (robertmh...@gmail.com) wrote: In DoCopy, some RLS-specific code constructs a SelectStmt to handle the case where COPY TO is invoked on an RLS-protected relation. But I think this step is bogus in two ways:

[HACKERS] copy.c handling for RLS is insecure

2014-10-06 Thread Robert Haas
In DoCopy, some RLS-specific code constructs a SelectStmt to handle the case where COPY TO is invoked on an RLS-protected relation. But I think this step is bogus in two ways: /* Build FROM clause */ from = makeRangeVar(NULL, RelationGetRelationName(rel), 1); First,

Re: [HACKERS] copy.c handling for RLS is insecure

2014-10-06 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: In DoCopy, some RLS-specific code constructs a SelectStmt to handle the case where COPY TO is invoked on an RLS-protected relation. But I think this step is bogus in two ways: /* Build FROM clause */ from =

Re: [HACKERS] copy.c handling for RLS is insecure

2014-10-06 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes: * Robert Haas (robertmh...@gmail.com) wrote: First, because relations are schema objects, there could be multiple relations with the same name. The RangeVar might end up referring to a different one of those objects than the user originally specified.

Re: [HACKERS] copy.c handling for RLS is insecure

2014-10-06 Thread Robert Haas
On Mon, Oct 6, 2014 at 2:49 PM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: In DoCopy, some RLS-specific code constructs a SelectStmt to handle the case where COPY TO is invoked on an RLS-protected relation. But I think this step is bogus in two ways:

Re: [HACKERS] copy.c handling for RLS is insecure

2014-10-06 Thread Robert Haas
I left out a few words there. On Mon, Oct 6, 2014 at 3:07 PM, Robert Haas robertmh...@gmail.com wrote: Hmm, that's certainly an interesting point, but I'm trying to work out how this is different from normal COPY..? pg_analyze_and_rewrite() happens for both cases down in BeginCopy(). As far

Re: [HACKERS] copy.c handling for RLS is insecure

2014-10-06 Thread David Fetter
On Mon, Oct 06, 2014 at 03:15:25PM -0400, Stephen Frost wrote: As far as I can see, the previous code only looked up any given name once. If you got a relation name, DoCopy() looked it up, and then BeginCopy() references it only by the passed-down Relation descriptor; if you got a query,

Re: [HACKERS] copy.c handling for RLS is insecure

2014-10-06 Thread Stephen Frost
David, On Monday, October 6, 2014, David Fetter da...@fetter.org wrote: On Mon, Oct 06, 2014 at 03:15:25PM -0400, Stephen Frost wrote: As far as I can see, the previous code only looked up any given name once. If you got a relation name, DoCopy() looked it up, and then BeginCopy()