Re: Improving RLS planning

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Improving RLS planning
Date: 2016-10-27 19:42:52
Message-ID: CAEZATCV37=gyLumJATkk-24_4cKGztpk77Ack6j9G6MyZaaUAg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 25 October 2016 at 22:58, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> The alternative I'm now thinking about pursuing is to get rid of the
> conversion of RLS quals to subqueries. Instead, we can label individual
> qual clauses with security precedence markings. Concretely, suppose we
> add an "int security_level" field to struct RestrictInfo. The semantics
> of this would be that a qual with a lower security_level value must be
> evaluated before a qual with a higher security_level value, unless the
> latter qual is leakproof. (It would likely also behoove us to add a
> "leakproof" bool field to struct RestrictInfo, to avoid duplicate
> leakproof-ness checks on quals. But that's just an optimization.)
>

+1 for this approach. It looks like it could potentially be much
simpler. There's some ugly code in the inheritance planner (and
probably one or two other places) that it might be possible to chop
out, which would probably also speed up planning times.

> In the initial implementation, quals coming from a RangeTblEntry's
> securityQuals field would have security_level 0, quals coming from
> anywhere else would have security_level 1; except that if we know
> there are no security quals anywhere (ie not Query->hasRowSecurity),
> we could give all quals security_level 0. (I think this exception
> may be worth making because there's no need to test leakproofness
> for a qual with security level 0; it could never be a candidate
> for security delay anyway.)
>

Note that the securityQuals list represents nested levels of security
barrier (e.g., nested SB views), so you'd have to actually assign
security_level 0 to the first security qual, security_level 1 to the
second security qual, and so on. Then the quals coming from anywhere
else would have to have a security_level one greater than the maximum
of all the other security levels.

> Having done that much, I think all we need in order to get rid of
> RLS subqueries, and just stick RLS quals into their relation's
> baserestrictinfo list, are two rules:
>
> 1. When selecting potential indexquals, a RestrictInfo can be considered
> for indexqual use only if it is leakproof or has security_level <= the
> minimum among the table's baserestrictinfo clauses.
>
> 2. In order_qual_clauses, sort first by security_level and second by cost.
>

I think that ordering might be sub-optimal if you had a mix of
leakproof quals and security quals and the cost of some security quals
were significantly higher than the cost of some other quals. Perhaps
all leakproof quals should be assigned security_level 0, to allow them
to be checked earlier if they have a lower cost (whether or not they
are security quals), and only leaky quals would have a security_level
greater than zero. Rule 1 would then not need to check whether the
qual was leakproof, and you probably wouldn't need the separate
"leakproof" bool field on RestrictInfo.

Regards,
Dean

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2016-10-27 20:15:23 Re: Proposal : For Auto-Prewarm.
Previous Message Robert Haas 2016-10-27 19:04:17 Re: [bug fix] Stats collector is not restarted on the standby