Re: WITH CHECK and Column-Level Privileges

* 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.

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

> For CHECK constraints/options, the change looks more reasonable, and I
> guess that approach could be extended to RLS by only including the
> modified columns, not the ones with select permissions, if RLS is
> enabled on the table. One minor comment on the code -- you could save
> a few cycles by only calling GetModifiedColumns() in the exceptional
> case.

Agreed, that sounds like a good approach for how to address the RLS
concern.  Also, good point about GetModifiedColumns.  There's a few
other minor changes that I want to make on re-reading it also.  I should
be able to post a new version later today.
Thanks!
    Stephen