Re: WITH CHECK and Column-Level Privileges

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: WITH CHECK and Column-Level Privileges
Date: 2015-01-13 14:09:14
Message-ID: CAEZATCVGr2GP1K=iOXn64F6h5M6qN8kcPz6OgE-KeJFrkZgt_A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 13 January 2015 at 13:50, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Dean Rasheed (dean(dot)a(dot)rasheed(at)gmail(dot)com) wrote:
>> On 12 January 2015 at 22:16, Stephen Frost <sfrost(at)snowman(dot)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 that for BuildIndexValueDescription and
>> > ExecBuildSlotValueDescription, but I figured we might have other users
>> > of BuildIndexValueDescription and I wasn't seeing a particularly better
>> > solution. Suggestions welcome, of course.
>>
>> Actually I'm starting to wonder if it's even worth bothering about the
>> index case. To leak information, you'd need to have a composite key
>> for which you only had access to a subset of the key columns (which in
>> itself seems like a pretty rare case), and then you'd need to specify
>> new values to make the entire key conflict with an existing value, at
>> which point the fact that an exception is thrown tells you that the
>> values in the index must be the same as your new values. I'm
>> struggling to imagine a realistic scenario where this would be a
>> problem.
>
> I'm not sure that I follow.. From re-reading the above a couple times,
> I take it you're making an argument that "people would be silly to set
> their database up that way," but that's not an argument we can stand on
> when it comes to privileges. Additionally, as the regression tests
> hopefully show, if you have update on one column of a composite key, you
> could find out the entire key today by issuing an update against that
> column to set it to the same value throughout. You don't need to know
> what the rest of the key is but only that two records somewhere have the
> same values except for the one column you have update rights on.
>

Hmm, yes I guess that's right.

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?

>> Also, if we change BuildIndexValueDescription() in this way, it's
>> going to make it more or less useless for updatable views, since in
>> the most common case the user won't have permissions on the underlying
>> table.
>
> That's certainly something to consider. I looked at trying to get which
> columns the user had provided down to BuildIndexValueDescription, but I
> couldn't find a straight-forward way to do that as it involves the index
> AMs which we can't change (and I'm not really sure we'd want to anyway).
>

Yeah I couldn't see any easy way of doing it. 2 possibilities sprung
to mind -- (1) wrap the index update in a PG_TRY() and add the detail
in the catch block, or (2) track the currently active EState and make
GetModifiedColumns() into an exported function taking a single EState
argument (the EState has the currently active ResultRelInfo on it).
Neither of those alternatives seems particularly attractive to me
though.

Regards,
Dean

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexey Bashtanov 2015-01-13 14:24:42 Re: OOM on EXPLAIN with lots of nodes
Previous Message David Fetter 2015-01-13 14:00:47 Re: POLA violation with \c service=