Re: postgres_fdw super user checks

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