Making the ENUM operators LEAKPROOF

This is an attempt to get the operators from the "enum_ops" operator
class LEAKPROOF.  There have been previous discussion about that, see
[1], [2] or, recently, in [3].  The required code change is trivial
(see [2]), so I won't discuss it here.  The hard part is to prove that
the operators are actually LEAKPROOF.

I think that we should discuss the goal posts first.  What is required
to prove LEAKPROOF?

I hope that we can all agree that the only information side channels
in the enum_* functions are error messages.  If yes, it will suffice
to discuss the potential error messages and what they can reveal about
the compared values.

I think that any discussion about the requirements of LEAKPROOF needs
to consider the other side of the coin: the guarantees given by
row-level security and security barrier views.  It makes no sense to
hold LEAKPROOF to higher standards than what we are ready to guarantee
for RLS and security barrier views.

Now [4] and the discussion on the security list left me with the
impression that row-level security is not intended to be safe against
attacks by a user who can run arbitrary SQL statements.  For example,
it is easy to work around RLS using EXPLAIN (ANALYZE)).  For security
barrier views, that limitation is explicitly documented in the final
paragraph of
https://www.postgresql.org/docs/current/rules-privileges.html

Consequently, I propose that for proving a function to be LEAKPROOF,
we only consider information leaks that can be triggered by a user
supplying parameters to an SQL statement run by an application.
For example, small differences in function execution time cannot be
exploited that way; they will be dominated by variations in the
application execution time and network latency.
Also, any attacks that require catalog manipulations or other
superuser-only modifications should be disregarded.  Superusers can
always subvert security rules.

One particular case that I want to discuss is out-of-memory conditions.
Some of the OOM error messages show the allocation size that failed
(dshash.c, mbutils.c, dsa.c and mcxt.c), which could leak some
information about the data involved.  My feeling is that that is
something you cannot exploit just by using an application; you'd have
to cause just the right memory pressure to make small allocations fail.
(If we decide to err on the side of caution here, I'd actually prefer
to have the error messages changed to not reveal the allocation size.)

------

Now, to a discussion of the ENUM comparison functions.

enum_eq() is clearly leakproof, and we have to concentrate on
enum_cmp(), and there on enum_cmp_internal(), which has all the tricky
parts.

There is a call to SearchSysCache1() and --- in load_enum_cache_data(),
which is called from compare_values_of_enum() --- an index scan on
pg_enum.  Both of these allocate memory, and both can fail in the
face of data corruption.  I hope we can agree that data corruption is
not a scenario we have to consider.  The memory allocated is to cache
the ENUM values, and their size and definition is already public
information in pg_enum, so I don't consider that a leak.
SearchCatCacheInternal() has some DEBUG2 messages, but they don't
output any catalog values.

Apart from that, I find the following error messages in the code:

- "invalid internal value for enum: %u" in enum_cmp_internal():
  That can only happen if somebody manages to call an ENUM
  comparison function with something that isn't an ENUM.
  But you cannot use SQL to call the ENUM comparison functions with
  something that isn't an ENUM.

- "%s is not an enum" in load_enum_cache_data(): again that can only
  be reached if somebody calls the function with something that is
  not an ENUM, which I argue you cannot do using the comparison
  functions.

- "enum value %u not found in cache for enum %s" in
  compare_values_of_enum(): you can only trigger an error message
  if one of the values you compare is a value that doesn't exist
  for the enum.  I don't think you can trigger that error via SQL.
  The only way this error could leak the (numeric!) value of an
  ENUM value in the database is if that value is illegal, that is,
  if there is data corruption.

My conclusion from all that is that it should be safe to set the
ENUM comparison functions LEAKPROOF.


To recapitulate, I would like to hear your opinion about these
potentially contentions questions:

- Do you agree that it is sufficient that an argument for
  LEAKPROOFness need only consider attacks through an application
  (since row-level security and security barries views are not
  designed to withstand attacks by arbitrary SQL statements)?

- Do you think that we can disregard out-of-memory errors when
  proving LEAKPROOFness?

- Do you agree with my conclusion that the ENUM comparison
  operators should be LEAKPROOF?  Did I miss anything?

Yours,
Laurenz Albe


 [1]: https://postgr.es/m/flat/31042.1546194242@sss.pgh.pa.us
 [2]: https://postgr.es/m/flat/2811772.0XtDgEdalL%40peanuts2
 [3]: https://postgr.es/m/flat/CAMxA3rtdJ2OdsPm8pqUKFXh%3DEyB-7Ypyjny%3DZPXoGZEK2nHTKQ%40mail.gmail.com
 [4]: https://postgr.es/m/flat/3a60be45e7a89f50d166dba49553950d6b8a97f5.camel%40cybertec.at