Re: WITH CHECK and Column-Level Privileges

On 13 January 2015 at 13:50, Stephen Frost <sfrost@snowman.net> wrote:
> * Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
>> On 12 January 2015 at 22:16, Stephen Frost <sfrost@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 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