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

2015-01-28 Thread Dean Rasheed
On 27 January 2015 at 22:45, Stephen Frost sfr...@snowman.net 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

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 had

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. However I think this is

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 GetModifiedColumns() macro is a

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 idxrec;

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 we

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 need

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

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 of

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

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 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 sfr...@snowman.net Date: Mon, 12 Jan 2015 17:04:11 -0500

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 of

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()

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

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

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

2015-01-13 Thread Dean Rasheed
On 12 January 2015 at 22:16, Stephen Frost sfr...@snowman.net 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

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 sfr...@snowman.net 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

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 conflict

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 sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Wed, Oct 29, 2014 at 8:16 AM, Stephen Frost sfr...@snowman.net wrote: suggestions. If the user does not have

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 sfr...@snowman.net wrote: * Robert Haas

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

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

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 sfr...@snowman.net 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

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 sfr...@snowman.net 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

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 sfr...@snowman.net 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) =

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

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

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 sfr...@snowman.net 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

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

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

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 dean.a.rash...@gmail.com 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

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 dean.a.rash...@gmail.com 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

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 sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Sat, Sep 27, 2014 at 1:19 PM, Dean Rasheed dean.a.rash...@gmail.com wrote: Also attached is a patch for 9.4 which does the same, but also removes the row reporting

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 strongly

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 you

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

2014-09-27 Thread Dean Rasheed
On 27 September 2014 14:33, Stephen Frost sfr...@snowman.net 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

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

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 problems

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 complains

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 NOT NULL