Re: Per-role disabling of LEAKPROOF requirements for row-level security?

Hi,

On Sat, 19 Apr 2025 18:14:01 +0200
Andreas Lind <andreaslindpetersen(at)gmail(dot)com> wrote:

> Hi hackers,
>
> Row-level security is an awesome feature. I was originally won over by
> the simple mental model of implicitly adding WHERE clauses to all
> queries, and that it generally comes for free when they can be
> satisfied by Index Conds.
>
> I've recently helped implement row-level security in a fairly big
> multi-tenant application with about 100 tables. RLS is being used
> "voluntarily" by the application, which switches to a role that is
> subject to RLS as the first thing after starting a transaction. I
> realize that this doesn't offer bulletproof tenant isolation, but it
> still makes a lot of sense as a defense-in-depth measure.
>
> It took some time to work through various performance regressions
> caused by selectivity estimates changing due to "WHERE tenantId = ..."
> implicitly being added to all relations in all queries. Extended
> statistics (dependencies) helped with that, as tenantId is highly
> correlated with the other id columns that we were already querying and
> joining on.
>
> The roughest edge turned out to be the well-known challenge with the
> LEAKPROOFness of built-in functions, previously discussed in earlier
> threads [1], in blog posts [2], on Stack Overflow [3] and pointed out
> in the docs for Row Security Policies [4].
>
> I'm aware that the usual workaround is to ALTER FUNCTION ... LEAKPROOF
> for each of the relevant built-in functions after making an evaluation
> of whether that's acceptable for the given use case. We weren't able to
> do this because we're using a hosted Postgres solution where it's not
> permitted. We've tried to talk them (AWS) into offering a way to do it,
> but they haven't budged yet.
>
> As in the earlier thread we were affected by enum_eq not being
> leakproof, which caused our indexes involving enum columns to not be
> fully utilized while RLS is active (because WHERE clauses on the enum
> columns cannot be implemented as Index Conds). We mostly overcame that
> by replacing each index with a series of partial ones, each with
> WHERE enum_column = ...
>
> We also had some functional indexes that either explicitly used a
> non-leakproof built-in function, or implicitly exercised a non-leakproof
> conversion function such as textin or int8out. Here we ended up cheating
> the system by adding LEAKPROOF wrappers around the built-in functions,
> similar to [5].
>
> The most difficult thing to overcome was GIN indexes on bigint[]
> columns not being used because the built-in arrayoverlap/arraycontained/
> array_eq/... functions aren't LEAKPROOF. We eventually figured out that
> we could make our own version of gin_array_ops which used functions
> marked as LEAKPROOF, and then recreate all the GIN indices with that.
>
> My hunch is that there's a big class of applications like ours where
> enforcing LEAKPROOFness isn't really important. When application code is
> interacting with the database via its own role, you're not that
> concerned with invisible tuples leaking out via error messages or
> elaborate timing attacks. That seems more relevant when your
> adversaries have direct access to the database and are able to create
> their own functions and execute arbitrary queries. I think it would be
> extremely useful to be able to forefeit the LEAKPROOF guarantees when
> the database is being accessed by an application in exchange for
> avoiding the footguns described above.
>
> I dug into the code and noticed that restrictinfo->leakproof is only
> being checked in two places (createplan.c and equivclass.c), so it seems
> fairly easy to only selectively enforce it. Then there's the question of
> how to configure it. I can think of a few possible ways:
>
> 1) Add a BYPASSLEAKPROOF role attribute that can only be granted by a
> superuser, similar to the BYPASSRLS flag.
> 2) Add a session variable, eg. enable_security_leakproof, that can only
> be set or granted to another role by a superuser.
> 3) Make it a property of the individual POLICY that grants access to the
> table. This would be a bit more granular than a global switch, but
> there'd be some ambiguity when multiple policies are involved.
>
> I took a stab at implementing solution 1) above, mostly to understand if
> I had found the right places in the code and how hard it'd be. This is
> my first time hacking on Postgres, and I'm not really sure if it's OK to
> introduce this extra check in a possibly hot code path. I thought about
> instead adding a "does the current user have bypassleakproof" flag in
> some struct available to the planner, but this is beyond my current
> abilities. Also, it's probably better to get some feedback on the idea
> before spending too much time optimizing.
>
> What do you think?

I'm not sure whether multi-tenant applications fall into the category where
LEAKPROOFness isn't considered important, since security is typically a key
concern for users of such systems.

Even if that were the case, I believe the impact of lacking LEAKPROOFness would
still depend more on how much each function can actually leak information, rather
than on the roles accessing the data.

Therefore, if bypassing the LEAKPROOF check is allowed when RLS is enabled,
I think it would be better to make this configurable per individual policy
as in solution 3 you suggested.

I'm not a security expert, so I can't say with confidence whether the overall
idea is sound or not. For broader review and discussion, I would suggest submitting
it to the CommitFest [1].

[1] https://commitfest.postgresql.org/53/

Best regards,
Yugo Nagata

>
> [1] https://www.postgresql.org/message-id/2811772.0XtDgEdalL%40peanuts2
> [2] https://pganalyze.com/blog/5mins-postgres-row-level-security-bypassrls-security-invoker-views-leakproof-functions
> [3] https://stackoverflow.com/questions/63008838/postgresql-ignores-pg-trgm-gin-indexes-when-row-level-security-is-enabled-and-no
> [4] https://www.postgresql.org/docs/current/ddl-rowsecurity.html#DDL-ROWSECURITY
> [5] https://news.ycombinator.com/item?id=20066350

--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>