Re: postgres_fdw super user checks

From: Andreas Karlsson <andreas(at)proxel(dot)se>
To: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgres_fdw super user checks
Date: 2017-09-12 08:13:14
Message-ID: 9b03ff0a-168c-5f7b-f3c8-21acddffd8f9@proxel.se
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 07/27/2017 09:45 PM, Jeff Janes wrote:> Here is an updated patch. 
This version allows you use the password-less
> connection if you either are the super-user directly (which is the
> existing committed behavior), or if you are using the super-user's
> mapping because you are querying a super-user-owned view which you have
> been granted access to.

I have tested the patch and it passes the tests and works, and the code
looks good (I have a small nitpick below).

The feature seems useful, especially for people who already use views
for security, so the question is if this is a potential footgun. I am
leaning towards no since the superuser should be careful when grant
access to is views anyway.

It would have been nice if there was a more generic way to handle this
since 1) the security issue is not unique to postgres_fdw and 2) this
requires you to create a view. But since the patch is simple, an
improvement in itself and does not prevent any future further
improvements in this era I see no reason to let perfect be the enemy of
good.

= Nitpicking/style

I would prefer if

/* no check required if superuser */
if (superuser())
return;

if (superuser_arg(user->userid))
return;

was, for consistency with the if clause in connect_pg_server(), written as

/* no check required if superuser */
if (superuser() || superuser_arg(user->userid))
return;

Andreas

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2017-09-12 08:19:29 Re: More efficient truncation of pg_stat_activity query strings
Previous Message Amit Langote 2017-09-12 08:12:23 Re: Partition-wise join for join between (declaratively) partitioned tables