Re: WITH CHECK and Column-Level Privileges

On Mon, Sep 29, 2014 at 10:26 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
>> On Sat, Sep 27, 2014 at 1:19 PM, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)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 convinced at this point that it's really valuable- and we'd have to
>> >> think about how this would work with views (permission on the view? or
>> >> on the table underneath? what if there is more than one?, etc).
>> >
>> > Well by that point in the code, the query has been rewritten and the
>> > row being reported is for the underlying table, so it's the table's
>> > ACLs that should apply. In fact not all the values from the table are
>> > even necessarily exported in the view, so its ACLs are not appropriate
>> > to the values being reported. I suspect that in many/most real-world
>> > cases, the user will only have permissions on the view, not on the
>> > underlying table, in which case it won't work anyway. So +1 for just
>> > removing it.
>>
>> Wait, what?
>>
>> I think it's clear that the right thing to report would be the columns
>> that the user had permission to see via the view. The decision as to
>> what is visible in the error message has to be consistent with the
>> underlying permissions structure. Removing the detail altogether is
>> OK security-wise because it's just a subset of what the user can be
>> allowed to see, but I think checking the table permissions can never
>> be right.
>
> What I believe Dean was getting at is that if the user has direct
> permissions on the underlying table then they could see the row by
> querying the table directly too, so it'd be alright to report the detail
> in the error.

Hmm, yeah. True.

> He then further points out that you're not likely to be
> using a view over top of a table which you have direct access to, so
> this is not a very interesting case.
>
> 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 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 believe we should try
to do better.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company