Re: RLS with check option - surprised design

* Peter Geoghegan (pg(at)heroku(dot)com) wrote:
> On Sun, Oct 5, 2014 at 5:16 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> >> next a message:
> >>
> >> ERROR: new row violates WITH CHECK OPTION for "data"
> >> DETAIL: Failing row contains (2014-10-05 12:28:30.79652, petr, 1000).
> >>
> >> Doesn't inform about broken policy.
> >
> > I'm guessing this is referring to the above policies and so my comments
> > there apply.. One thing to note about this is that there is an active
> > discussion about removing the 'DETAIL' part of that error message as it
> > may be an information leak.
>
> I should point out that there is an issue with the ON CONFLICT UPDATE
> patch and RLS, as described here:
>
> https://wiki.postgresql.org/wiki/UPSERT#RLS
>
> I think it'll be possible to prevent the current information leak that
> my example illustrates (by making sure that there is an appropriate
> predicate associated with the auxiliary UPDATE plan, like any other
> UPDATE). After all, the auxiliary UPDATE accepts a WHERE clause,
> subject only to a few restrictions that are not relevant for the
> purposes of appending security quals.
>
> I actually spent over a day trying to figure out how to make this
> work, but gave up before the most recent revision, V1.4 was submitted.
> I guess I'll have to look at the problem again soon. I don't grok RLS,
> but offhand I think simply not including the DETAIL message might be
> good enough to fix my case. Maybe you have an opinion on that.

Are you sure this isn't just another example of an existing issue we
have wrt column privileges..? I'm working on a patch already to address
those issues in back-branches and will be considering what needs to be
done for RLS also. One option for RLS is to not produce the 'detail'
info when RLS exists on the relation, but Robert makes a good point that
the detail information is valuable results for normal users, so I'm
hopeful that we'll be able to do better than that.

Another way to address this general concern (I've not looked to see if
it would help with your UPSERT work specifically) might be to change
where the RLS checks are done to be earlier- in particular, *before*
checking for constraint and key violations. The concern there is that
RLS is currently using the security barrier views WITH CHECK
infrastructure and Dean was concerned that the SQL spec requires that
such key violations be reported before WITH CHECK violations. RLS isn't
part of spec, of course, and so we can be open to change that if we feel
it's appropriate but I'm really curious why the spec would require that
key violations be checked first; that doesn't make a huge amount of
sense to me.. Another question is how other databases with similar
capabilities address this.

Thanks!

Stephen