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

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

2015-07-09 Thread Stephen Frost
Noah, Andres, * 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 > (e.g. BEFORE triggers). That user

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 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-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 wrote: > > > > Alright, I've done the change to use the RangeVar from CopyStmt, but

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 wrote: > > > > Alright, I've done the change to use the RangeVar from CopyStmt, but

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 wrote: > > > Alright, I've done the change to use the RangeVar from CopyStmt, but > > > also added a check wherein we verify that the rel

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 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 rela

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 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 on- if they're

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: > > > >

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

2014-10-06 Thread Stephen Frost
David, On Monday, October 6, 2014, David Fetter 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() referenc

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

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

2014-10-06 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: > On Mon, Oct 6, 2014 at 2:49 PM, Stephen Frost wrote: > > Argh. That's certainly no good. It should just be using the RangeVar > > relation passed in from CopyStmt, no? > > I don't think that's adequate. You can't do a RangeVar-to-OID > translation

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 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 as I can see,

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 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 Tom Lane
Stephen Frost 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. > Argh. Th

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 = makeRange

[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, becau