Thread: if (!superuser) checks

All, Andres,

Now that we have begun removing the if (!superuser) checks and instead
relying on the GRANT system to determine who is allowed to call certain
functions, it's time to consider functions beyond the initial set.

In particular, the pg_logical_* functions have superuser checks and
those checks also allow roles who have the replication role attribute.
That isn't something we can represent with the GRANT system currently.

The main question is if it really makes sense for the replication role
attribute to control access to these functions.  Personally, I'd rather
restrict replication roles (who are not also superusers) from connecting
to PG at all.

Andres, I figured you would have the best idea about how impactful such
a change would be on users of those functions.

Thoughts?

Thanks!

Stephen

All,

* Stephen Frost (sfrost@snowman.net) wrote:
> In particular, the pg_logical_* functions have superuser checks and
> those checks also allow roles who have the replication role attribute.
> That isn't something we can represent with the GRANT system currently.

I chatted with Andres a bit at PGConf.NY regarding the pg_logical_* and
the REPLICATION role attribute in general.

This is all 9.7 material, of course, but I wanted to jot down my
recollection of the discussion before it goes out of mind.

The idea we came up with is to have a pg_replication default role which
essentially replaces the REPLICATION role attribute.  Andres didn't see
it as being terribly valuable to disallow a role with the REPLICATION
attribute from logging into the database directly, if it's allowed to do
so by pg_hba.conf.  If the administrator doesn't wish to allow that,
then they can configure pg_hba.conf to not allow it.  Further, with the
permissions handled by the GRANT system, the administrator could have
independent roles for connecting to the replication protocol and for
running the pg_logical_* functions, again, if they wish to configure
their system that way.

Lastly, Andres felt that we could keep the syntax for
setting/unsetting the role attribute and just convert those into
GRANT/REVOKE statements for the pg_replication role.  I'm not thrilled
with that idea as it feels a bit unnecessary at this point, given the
relatively few users of pg_logical_* functions by tools and because it
would introduce a one-off hack in tools such as pgAdmin, which would
have to query pg_auth_members to get the current setting of the
REPLICATION role attribute, even if the code for granting/revoking that
role attribute doesn't change, unless we also hack up the views and
tables which contain role attributes today to reflect membership in
pg_replication as having (or not) that role attribute.  Basically, for
my 2c, it seems like an awful lot of extra code on the backend side for
relatively small gain.  pgAdmin and similar tools are going to want to
provide support for default roles in any case, along with their role
attributes support, and it doesn't seem useful to try and conflate the
two.

Andres, please correct me if any of the above is incorrect.

Thoughts and comments are welcome, of course.

Thanks!

Stephen

Hi,

On 2016-04-22 14:56:44 -0400, Stephen Frost wrote:
> The idea we came up with is to have a pg_replication default role which
> essentially replaces the REPLICATION role attribute.  Andres didn't see
> it as being terribly valuable to disallow a role with the REPLICATION
> attribute from logging into the database directly, if it's allowed to do
> so by pg_hba.conf.

Not just not useful, but rather an anti-feature.

> Lastly, Andres felt that we could keep the syntax for
> setting/unsetting the role attribute and just convert those into
> GRANT/REVOKE statements for the pg_replication role.  I'm not thrilled
> with that idea as it feels a bit unnecessary at this point, given the
> relatively few users of pg_logical_* functions by tools

Uh, that's not just about the pg_logical_ functions. It's primarily
about scripts which setup a postgres cluster, including replicas. Those
will frequently (hopefully at least) include creating/altering a role to
have the replication flag set?


> Andres, please correct me if any of the above is incorrect.

Nothing but the above point stands out.


- Andres