Additional role attributes && superuser review

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Additional role attributes && superuser review
Date: 2014-10-15 05:22:59
Message-ID: 20141015052259.GG28859@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greetings,

The attached patch for review implements a few additional role
attributes (all requested by users or clients in various forums) for
common operations which currently require superuser privileges. This
is not a complete solution for all of the superuser-only privileges we
have but it's certainly good progress and along the correct path, as
shown below in a review of the existing superuser checks in the
backend.

First though, the new privileges, about which the bikeshedding can
begin, short-and-sweet format:

BACKUP:
pg_start_backup()
pg_stop_backup()
pg_switch_xlog()
pg_create_restore_point()

LOGROTATE:
pg_rotate_logfile()

MONITOR:
View detailed information regarding other processes.
pg_stat_get_wal_senders()
pg_stat_get_activity()

PROCSIGNAL:
pg_signal_backend()
(not allowed to signal superuser-owned backends)

Before you ask, yes, this patch also does a bit of rework and cleanup.

Yes, that could be done as an independent patch- just ask, but the
changes are relatively minor. One curious item to note is that the
current if(!superuser()) {} block approach has masked an inconsistency
in at least the FDW code which required a change to the regression
tests- namely, we normally force FDW owners to have USAGE rights on
the FDW, but that was being bypassed when a superuser makes the call.
I seriously doubt that was intentional. I won't push back if someone
really wants it to stay that way though.

This also includes the previously discussed changes for pgstat and
friends to use role membership instead of GetUserId() == backend_role.

Full documentation is not included but will be forthcoming, of course.

As part of working through what the best solution is regarding the
existing superuser-only checks, I did a review of more-or-less all of
the ones which exist in the backend. From that review, I came to the
conclusion (certainly one which can be debated, but still) that most
cases of superuser() checks that should be possible for non-superusers
to do are yes/no privileges, except for when it comes to server-side
COPY and CREATE TABLESPACE operations, which need a form of
directory-level access control also (patch for that will be showing up
shortly..).

For posterity's sake, here's my review and comments on the various
existing superuser checks in the backend (those not addressed above):

CREATE EXTENSION
This could be a role attribute as the others above, but I didn't
want to try and include it in this patch as it has a lot of hairy
parts, I expect.

Connect using 'reserved' slots
This is another one which we might add as another role attribute.

Only needed during recovery, where it's fine to require superuser:
pg_xlog_replay_pause()
pg_xlog_replay_resume()

Directory / File access (addressed independently):
libpq/be/fsstubs.c
lo_import()
lo_export()

commands/tablespace.c - create tablespace
commands/copy.c - server-side COPY TO/FROM

utils/adt/genfile.c
pg_read_file() / pg_read_text_file() / pg_read_binary_file()
pg_stat_file()
pg_ls_dir()

Lots of things depend on being able to create C functions. These, in
my view, should either be done through extensions or should remain the
purview of the superuser. Below are the ones which I identified as
falling into this category:

commands/tsearchcmd.c
Create text search parser
Create text search template

tcop/utility.c
LOAD (load shared library)

commands/foreigncmds.c
Create FDW, Alter FDW
FDW ownership

commands/functioncmds.c
create binary-compatible casts
create untrusted-language functions

command/proclang.c
Create procedural languages (unless PL exists in pg_pltemplate)
Create custom procedural language

commands/opclasscmds.c
Create operator class
Create operator family
Alter operator family

commands/aggregatecmds.c
Create aggregates which use 'internal' types

commands/functioncmds.c
create leakproof functions
alter function to be leakproof

command/event_trigger.c
create event trigger

Other cases which I don't think would make sense to change (and some I
wonder if we should prevent the superuser from doing, unless they have
rolcatupdate or similar...):

commands/alter.c
rename objects (for objects which don't have an 'owner')
change object schema (ditto)

commands/trigger.c
enable or disable internal triggers

commands/functioncmds.c
execute DO blocks with untrusted languages

commands/dbcommands.c
allow encoding/locale to be different

commands/user.c
set 'superuser' on a role
alter superuser roles (other options)
drop superuser roles
alter role membership for superusers
force 'granted by' on a role grant
alter global settings
rename superuser roles

utils/misc/guc.c
set superuser-only GUCs
use ALTER SYSTEM
show ALL GUCs
get superuser-only GUC info
define custom placeholder GUC

utils/adt/misc.c
reload database configuration

utils/init/postinit.c
connect in binary upgrade mode
connect while database is shutting down

Another discussion could be had regarding the superuser-only GUCs.

Thanks!

Stephen

Attachment Content-Type Size
role-attributes.patch text/x-diff 51.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2014-10-15 05:28:16 Re: Better support of exported snapshots with pg_dump
Previous Message Andres Freund 2014-10-15 05:06:33 Re: Better support of exported snapshots with pg_dump