Re: Row Level Security − leakproof-ness and performance implications

From: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
To: Pierre Ducroquet <p(dot)psql(at)pinaraf(dot)info>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Row Level Security − leakproof-ness and performance implications
Date: 2019-02-19 23:43:50
Message-ID: 5cf08fadb2d30c031883f0f78bd48a819428875e.camel@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Pierre Ducroquet wrote:
> In order to increase our security, we have started deploying row-level
> security in order to add another safety net if any issue was to happen in our
> applications.
> After a careful monitoring of our databases, we found out that a lot of
> queries started to go south, going extremely slow.
> The root of these slowdowns is that a lot of the PostgreSQL functions are not
> marked as leakproof, especially the ones used for operators.
> In current git master, the following query returns 258 functions that are used
> by operators returning booleans and not marked leakproof:
>
> SELECT proname FROM pg_proc
> WHERE exists(select 1 from pg_operator where oprcode::name = proname)
> AND NOT(proleakproof)
> AND prorettype = 16;
>
>
> Among these, if you have for instance a table like:
> create table demo_enum (
> username text,
> flag my_enum
> );
>
> With an index on (username, flag), the index will only be used on username
> because flag is an enum and the enum_eq function is not marked leakproof.
>
> For simple functions like enum_eq/enum_ne, marking them leakproof is an
> obvious fix (patch attached to this email, including also textin/textout). And

The enum_eq part looks totally safe, but the text functions allocate memory,
so you could create memory pressure, wait for error messages that tell you
the size of the allocation that failed and this way learn about the data.

Is this a paranoid idea?

> if we had a 'RLS-enabled' context on functions, a way to make a lot of built-
> in functions leakproof would simply be to prevent them from logging errors
> containing values.
>
> For others, like arraycontains, it's much trickier :[...]

I feel a little out of my depth here, so I won't comment.

> A third issue we noticed is the usage of functional indexes. If you create an
> index on, for instance, (username, leaky_function(my_field)), and then search
> on leaky_functon(my_field) = 42, the functional index can be used only if
> leaky_function is marked leakproof, even if it is never going to be executed
> on invalid rows thanks to the index. After a quick look at the source code, it
> also seems complicated to implement since the decision to reject potential
> dangerous calls to leaky_function is done really early in the process, before
> the optimizer starts.

If there is a bitmap index scan, the condition will be rechecked, so the
function will be executed.

> I am willing to spend some time on these issues, but I have no specific
> knowledge of the planner and optimizer, and I fear proper fixes would require
> much digging there. If you think it would be appropriate for functions like
> arraycontains or range_contains to require the 'inner' comparison function to
> be leakproof, or just keep looking at the other functions in pg_proc and
> leakproof the ones that can be, I would be happy to write the corresponding
> patches.

Thanks, and I think that every function that can safely be marked leakproof
is a progress!

Yours,
Laurenz Albe

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ideriha, Takeshi 2019-02-20 00:02:19 RE: Protect syscache from bloating with negative cache entries
Previous Message Tomas Vondra 2019-02-19 23:28:14 Re: WAL insert delay settings