Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-28 Thread Stephen Frost
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 27 January 2015 at 22:45, Stephen Frost wrote: > > Here's the latest set with a few additional improvements (mostly > > comments but also a couple missed #include's and eliminating unnecessary > > whitespace changes). Unless there are i

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-28 Thread Dean Rasheed
On 27 January 2015 at 22:45, Stephen Frost wrote: > Here's the latest set with a few additional improvements (mostly > comments but also a couple missed #include's and eliminating unnecessary > whitespace changes). Unless there are issues with my testing tonight or > concerns raised, I'll push th

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-23 Thread Stephen Frost
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Stephen Frost wrote: > > > > +#define GetModifiedColumns(relinfo, estate) \ > > > > + (rt_fetch((relinfo)->ri_RangeTableIndex, > > > > (estate)->es_range_table)->modifiedCols) > > > > > > I assume you are aware that this GetModifiedColumn

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-23 Thread Alvaro Herrera
Stephen Frost wrote: > > > +#define GetModifiedColumns(relinfo, estate) \ > > > + (rt_fetch((relinfo)->ri_RangeTableIndex, > > > (estate)->es_range_table)->modifiedCols) > > > > I assume you are aware that this GetModifiedColumns() macro is a > > duplicate of another one found elsewhere. Howeve

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-23 Thread Stephen Frost
Alvaro, Thanks for the review. * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Note the first comment in this hunk was not update to talk about NULL > instead of "": Ah, good catch, will fix. > Hm, this is a bit odd. I thought you were going to return the subset of > columns that the user

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-23 Thread Alvaro Herrera
Note the first comment in this hunk was not update to talk about NULL instead of "": > @@ -163,13 +168,63 @@ BuildIndexValueDescription(Relation indexRelation, > Datum *values, bool *isnull) > { > StringInfoData buf; > + Form_pg_index idx

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-20 Thread Stephen Frost
* Noah Misch (n...@leadboat.com) wrote: > On Mon, Jan 19, 2015 at 11:05:22AM -0500, Stephen Frost wrote: > > One remaining question is about single-column key violations. Should we > > special-case those and allow them to be shown or no? I can't see a > > reason not to currently but I wonder if w

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-19 Thread Noah Misch
On Mon, Jan 19, 2015 at 11:05:22AM -0500, Stephen Frost wrote: > One remaining question is about single-column key violations. Should we > special-case those and allow them to be shown or no? I can't see a > reason not to currently but I wonder if we might have cause to act > differently in the f

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-19 Thread Stephen Frost
All, * Stephen Frost (sfr...@snowman.net) wrote: > > It seems that the reason for this is to be consistent with > > BuildIndexValueDescription, which seems to do the same thing to simplify > > the stuff going on at check_exclusion_constraint() -- by returning an > > empty string, that code doesn't

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-19 Thread Stephen Frost
* Stephen Frost (sfr...@snowman.net) wrote: > Updated patch attached. Ugh. Hit send too quickly. Attached. Thanks! Stephen From f74dcef56bd3507c6bb26b0468655fd8e408fc80 Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Mon, 12 Jan 2015 17:04:11 -0500 Subject: [PATCH]

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-19 Thread Stephen Frost
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > I'm confused. Why does ExecBuildSlotValueDescription() return an empty > string instead of NULL? There doesn't seem to be any value left in that > idea, and returning NULL would make the callsites slightly simpler as > well. (Also, I

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-19 Thread Alvaro Herrera
I'm confused. Why does ExecBuildSlotValueDescription() return an empty string instead of NULL? There doesn't seem to be any value left in that idea, and returning NULL would make the callsites slightly simpler as well. (Also, I think the comment on top of it should be updated to reflect the perm

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-19 Thread Stephen Frost
Noah, * Noah Misch (n...@leadboat.com) wrote: > On Mon, Jan 12, 2015 at 05:16:40PM -0500, Stephen Frost wrote: > > Alright, here's an updated patch which doesn't return any detail if no > > values are visible or if only a partial key is visible. > > I browsed this patch. There's been no mention

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-14 Thread Stephen Frost
Noah, * Noah Misch (n...@leadboat.com) wrote: > On Mon, Jan 12, 2015 at 05:16:40PM -0500, Stephen Frost wrote: > > Alright, here's an updated patch which doesn't return any detail if no > > values are visible or if only a partial key is visible. > > I browsed this patch. There's been no mention

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-13 Thread Noah Misch
On Mon, Jan 12, 2015 at 05:16:40PM -0500, Stephen Frost wrote: > Alright, here's an updated patch which doesn't return any detail if no > values are visible or if only a partial key is visible. I browsed this patch. There's been no mention of foreign key constraints, but ri_ReportViolation() dese

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-13 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote: > One improvement we could trivially make is to only do this for > multi-column indexes. If there is only one column there is no danger > of information leakage, right? That's an interesting thought. If there's only one column then to have a confli

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-13 Thread Dean Rasheed
On 13 January 2015 at 13:50, Stephen Frost wrote: > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: >> On 12 January 2015 at 22:16, Stephen Frost wrote: >> > Alright, here's an updated patch which doesn't return any detail if no >> > values are visible or if only a partial key is visible. >> > >

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-13 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 12 January 2015 at 22:16, Stephen Frost wrote: > > Alright, here's an updated patch which doesn't return any detail if no > > values are visible or if only a partial key is visible. > > > > Please take a look. I'm not thrilled with simply retu

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-13 Thread Dean Rasheed
On 12 January 2015 at 22:16, Stephen Frost wrote: > Alright, here's an updated patch which doesn't return any detail if no > values are visible or if only a partial key is visible. > > Please take a look. I'm not thrilled with simply returning an empty > string and then checking that for BuildInd

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-12 Thread Stephen Frost
All, Apologies, forgot to mention- this is again 9.4. Thanks, Stephen * Stephen Frost (sfr...@snowman.net) wrote: > Dean, Robert, > > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > > On 29 October 2014 13:04, Stephen Frost wrote: > > > * Robert Haas (robertmh...@gmail.com) wrote:

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2015-01-12 Thread Stephen Frost
Dean, Robert, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 29 October 2014 13:04, Stephen Frost wrote: > > * Robert Haas (robertmh...@gmail.com) wrote: > >> On Wed, Oct 29, 2014 at 8:16 AM, Stephen Frost wrote: > >> > suggestions. If the user does not have table-level SELECT rights, >

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-10-30 Thread Dean Rasheed
On 29 October 2014 13:04, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: >> On Wed, Oct 29, 2014 at 8:16 AM, Stephen Frost wrote: >> > suggestions. If the user does not have table-level SELECT rights, >> > they'll see for the "Failing row contains" case, they'll get: >> > >>

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-10-29 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: > On Wed, Oct 29, 2014 at 8:16 AM, Stephen Frost wrote: > > suggestions. If the user does not have table-level SELECT rights, > > they'll see for the "Failing row contains" case, they'll get: > > > > Failing row contains (col1, col2, col3) = (1, 2, 3).

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-10-29 Thread Robert Haas
On Wed, Oct 29, 2014 at 8:16 AM, Stephen Frost wrote: > suggestions. If the user does not have table-level SELECT rights, > they'll see for the "Failing row contains" case, they'll get: > > Failing row contains (col1, col2, col3) = (1, 2, 3). > > Or, if they have no access to any columns: > > Fai

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-10-29 Thread Stephen Frost
Robert, all, * Robert Haas (robertmh...@gmail.com) wrote: > On Mon, Sep 29, 2014 at 10:26 AM, Stephen Frost wrote: > > In the end, it sounds like we all agree that the right approach is to > > simply remove this detail and avoid the issue entirely. > > Well, I think that's an acceptable approach

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-10-02 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 30 September 2014 20:17, Stephen Frost wrote: > > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > >> One of the main things that detail is useful for is identifying the > >> failing row in a multi-row update. In most real-world cases, I woul

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-30 Thread Dean Rasheed
On 30 September 2014 20:17, Stephen Frost wrote: > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: >> On 30 September 2014 16:52, Stephen Frost wrote: >> > If the user only has column-level privileges on the table then I'm not >> > really sure how useful the detail will be. >> >> One of the main

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-30 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 30 September 2014 16:52, Stephen Frost wrote: > > If the user only has column-level privileges on the table then I'm not > > really sure how useful the detail will be. > > One of the main things that detail is useful for is identifying the > f

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-30 Thread Dean Rasheed
On 30 September 2014 16:52, Stephen Frost wrote: > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: >> On 29 September 2014 16:46, Stephen Frost wrote: >> > * Robert Haas (robertmh...@gmail.com) wrote: >> >> Well, I think that's an acceptable approach from the point of view of >> >> fixing the se

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-30 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 29 September 2014 16:46, Stephen Frost wrote: > > * Robert Haas (robertmh...@gmail.com) wrote: > >> Well, I think that's an acceptable approach from the point of view of > >> fixing the security exposure, but it's far from ideal. Good error >

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-30 Thread Dean Rasheed
On 29 September 2014 16:46, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: >> Well, I think that's an acceptable approach from the point of view of >> fixing the security exposure, but it's far from ideal. Good error >> messages are important for usability. I can live with t

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-29 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: > Well, I think that's an acceptable approach from the point of view of > fixing the security exposure, but it's far from ideal. Good error > messages are important for usability. I can live with this as a > short-term fix, but in the long run I strong

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-29 Thread Robert Haas
On Mon, Sep 29, 2014 at 10:26 AM, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: >> On Sat, Sep 27, 2014 at 1:19 PM, Dean Rasheed >> wrote: >> >> Also attached is a patch for 9.4 which does the same, but also removes >> >> the row reporting (completely) from the WITH CHECK c

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-29 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: > On Sat, Sep 27, 2014 at 1:19 PM, Dean Rasheed > wrote: > >> Also attached is a patch for 9.4 which does the same, but also removes > >> the row reporting (completely) from the WITH CHECK case. It could be > >> argued that it would be helpful to have

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-29 Thread Robert Haas
On Sat, Sep 27, 2014 at 1:19 PM, Dean Rasheed wrote: >> Also attached is a patch for 9.4 which does the same, but also removes >> the row reporting (completely) from the WITH CHECK case. It could be >> argued that it would be helpful to have it there also, perhaps, but I'm >> not convinced at thi

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-27 Thread Dean Rasheed
On 27 September 2014 14:33, Stephen Frost wrote: >> Yeah, I take that back. If there is a composite key involved then you >> can run into the same issue- you update one of the columns to a >> conflicting value and get back the entire key, including columns you >> shouldn't be allowed to see. > >

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-27 Thread Stephen Frost
* Stephen Frost (sfr...@snowman.net) wrote: > > Looks like there is an issue here with CHECK constraints and NOT NULL > > constraints, yes. The uniqueness check complains about the key already > > existing and returns the key, but I don't think that's actually a > > problem- to get that to happen

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-26 Thread Stephen Frost
* Stephen Frost (sfr...@snowman.net) wrote: > * Stephen Frost (sfr...@snowman.net) wrote: > > > Is there similar problems with unique or exclusion constraints? > > > > That's certainly an excellent question.. I'll have to go look. > > Looks like there is an issue here with CHECK constraints and

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-26 Thread Stephen Frost
* Stephen Frost (sfr...@snowman.net) wrote: > > Is there similar problems with unique or exclusion constraints? > > That's certainly an excellent question.. I'll have to go look. Looks like there is an issue here with CHECK constraints and NOT NULL constraints, yes. The uniqueness check complai

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-26 Thread Stephen Frost
* Heikki Linnakangas (hlinnakan...@vmware.com) wrote: > On 09/26/2014 05:20 PM, Stephen Frost wrote: > > Note that the entire failing tuple is returned, including the > > 'password' column, even though the 'alice' user does not have select > > rights on that column. > > Is there similar prob

Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-26 Thread Heikki Linnakangas
On 09/26/2014 05:20 PM, Stephen Frost wrote: All, Through continued testing, we've discovered an issue in the WITH CHECK OPTION code when it comes to column-level privileges which impacts 9.4. It's pretty straight-forward, thankfully, but: postgres=# create view myview postgres-# w

[HACKERS] WITH CHECK and Column-Level Privileges

2014-09-26 Thread Stephen Frost
All, Through continued testing, we've discovered an issue in the WITH CHECK OPTION code when it comes to column-level privileges which impacts 9.4. It's pretty straight-forward, thankfully, but: postgres=# create view myview postgres-# with (security_barrier = true, postgres-# check_opti