Re: A little RLS oversight?

On 28 July 2015 at 03:19, Joe Conway <mail(at)joeconway(dot)com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 07/27/2015 03:05 PM, Stephen Frost wrote:
>> AFK at the moment, but my thinking was that we should avoid having
>> the error message change based on what a GUC is set to. I agree
>> that there should be comments which explain that.
>

Except it's already dependant on the GUC if it's set to FORCE.

> I changed back to using GetUserId() for the call to check_enable_rls()
> at those three call sites, and added to the comments to explain why.
>

I'm not entirely convinced about this. The more I think about it, the
more I think that if we know the user has BYPASSRLS, and they've set
row_security to OFF, then they ought to get the more detailed error
message, as they would if there was no RLS. That error detail is
highly useful, and we know the user has been granted privilege by a
superuser, and that they have direct access to the underlying table in
this context, so we're not leaking any info that they cannot directly
SELECT anyway.

> While looking at ri_ReportViolation() I spotted what I believe to be a
> bug in the current logic -- namely, has_perm is initialized to true,
> and when check_enable_rls() returns RLS_ENABLED we never reset
> has_perm to false, and thus leak info even though the comments claim
> we don't. I fixed that here, but someone please take a look and
> confirm I am reading that correctly.
>

Ah yes, well spotted. That looks correct to me.

Regards,
Dean