Additional role attributes && superuser review
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
Attachments:
role-attributes.patchtext/x-diff; charset=us-asciiDownload+597-340
On 10/15/14, 12:22 AM, Stephen Frost wrote:
BACKUP:
pg_start_backup()
pg_stop_backup()
pg_switch_xlog()
pg_create_restore_point()
It seems odd to me that this (presumably) supports PITR but not pg_dump*. I realize that most folks probably don't use pg_dump for actual backups, but I think it is very common to use it to produce schema-only (maybe with data from a few tables as well) dumps for developers. I've certainly wished I could offer that ability without going full-blown super-user.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 15 October 2014 06:22, Stephen Frost <sfrost@snowman.net> wrote:
BACKUP:
pg_start_backup()
pg_stop_backup()
pg_switch_xlog()
pg_create_restore_point()
Yes, but its more complex. As Jim says, you need to include pg_dump,
plus you need to include the streaming utilities, e.g. pg_basebackup.
LOGROTATE:
pg_rotate_logfile()
Do we need one just for that?
MONITOR:
View detailed information regarding other processes.
pg_stat_get_wal_senders()
pg_stat_get_activity()
+1
Connect using 'reserved' slots
This is another one which we might add as another role attribute.
RESERVED?
Perhaps we need a few others also - BASHFUL, HAPPY, GRUMPY etc
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 15/10/14 07:22, Stephen Frost wrote:
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()
As others have commented, I too think this should support pg_dump.
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.
Yeah it will, mainly because extensions can load modules and can have
untrusted functions, we might want to limit which extensions are
possible to create without being superuser.
tcop/utility.c
LOAD (load shared library)
This already somewhat handles non-superuser access. You can do LOAD as
normal user as long as the library is in $libdir/plugins directory so it
probably does not need separate role attribute (might be somehow useful
in combination with CREATE EXTENSION though).
commands/functioncmds.c
create untrusted-language functions
I often needed more granularity there (plproxy).
commands/functioncmds.c
execute DO blocks with untrusted languages
I am not sure if this is significantly different from untrusted-language
functions.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Petr Jelinek (petr@2ndquadrant.com) wrote:
On 15/10/14 07:22, Stephen Frost wrote:
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()As others have commented, I too think this should support pg_dump.
I'm uttly mystified as to what that *means*. Everyone asking for it is
great but until someone can define what "support pg_dump" means, there's
not much progress I can make towards it..
pg_dump doesn't require superuser rights today. If you're looking for a
role which allows a user to arbitrairly read all data, fine, but that's
a different consideration and would be a different role attribute, imv.
If you'd like the role attribute renamed to avoid confusion, I'm all for
that, just suggest something.
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.Yeah it will, mainly because extensions can load modules and can
have untrusted functions, we might want to limit which extensions
are possible to create without being superuser.
The extension has to be available on the filesystem before it can be
created, of course. I'm not against providing some kind of whitelist or
similar which a superuser could control.. That's similar to how PLs
work wrt pltemplate, no?
tcop/utility.c
LOAD (load shared library)This already somewhat handles non-superuser access. You can do LOAD
as normal user as long as the library is in $libdir/plugins
directory so it probably does not need separate role attribute
(might be somehow useful in combination with CREATE EXTENSION
though).
Ah, fair enough. Note that I wasn't suggesting this be changed, just
noting it in my overall review.
commands/functioncmds.c
create untrusted-language functionsI often needed more granularity there (plproxy).
Not sure what you're getting at..? Is there a level of 'granularity'
for this which would make it safe for non-superusers to create
untrusted-language functions? I would think that'd be handled mainly
through extensions, but certainly curious what others think.
commands/functioncmds.c
execute DO blocks with untrusted languagesI am not sure if this is significantly different from
untrusted-language functions.
Nope, just another case where we're doing a superuser() check.
Thanks!
Stephen
* Simon Riggs (simon@2ndQuadrant.com) wrote:
On 15 October 2014 06:22, Stephen Frost <sfrost@snowman.net> wrote:
BACKUP:
pg_start_backup()
pg_stop_backup()
pg_switch_xlog()
pg_create_restore_point()Yes, but its more complex. As Jim says, you need to include pg_dump,
plus you need to include the streaming utilities, e.g. pg_basebackup.
I'd rather have more, simpler, role attributes than one 'catch-all'.
Once I understand what "include pg_dump" and "include pg_basebackup"
mean, I'd be happy to work on adding those.
LOGROTATE:
pg_rotate_logfile()Do we need one just for that?
It didn't seem to "belong" to any others and it's currently limited to
superuser but useful for non-superusers, so I would argue 'yes'. Now,
another option (actually, for many of these...) would be to trust in our
authorization system (GRANT EXECUTE) and remove the explicit superuser
check inside the functions, revoke EXECUTE from public, and tell users
to GRANT EXECUTE to the roles which should be allowed.
MONITOR:
View detailed information regarding other processes.
pg_stat_get_wal_senders()
pg_stat_get_activity()+1
Connect using 'reserved' slots
This is another one which we might add as another role attribute.RESERVED?
Seems reasonable, but do we need another GUC for how many connections
are reserved for 'RESERVED' roles? Or are we happy to allow those with
the RESERVED role attribute to contend for the same slots as superusers?
For my 2c- I'm happy to continue to have just one 'pool' of reserved
connections and just allow roles with RESERVED to connect using those
slots also.
Perhaps we need a few others also - BASHFUL, HAPPY, GRUMPY etc
Hah. :)
There was a suggestion raised (by Robert, I believe) that we store these
additional role attributes as a bitmask instead of individual columns.
I'm fine with either way, but it'd be a backwards-incompatible change
for anyone who looks at pg_authid. This would be across a major version
change, of course, so we are certainly within rights to do so, but I'm
also not sure how much we need to worry about a few bytes per pg_authid
row; we still have other issues if we want to try and support millions
of roles, starting with the inability to partition catalogs..
Thanks!
Stephen
Jim,
* Jim Nasby (Jim.Nasby@BlueTreble.com) wrote:
On 10/15/14, 12:22 AM, Stephen Frost wrote:
BACKUP:
pg_start_backup()
pg_stop_backup()
pg_switch_xlog()
pg_create_restore_point()It seems odd to me that this (presumably) supports PITR but not pg_dump*. I realize that most folks probably don't use pg_dump for actual backups, but I think it is very common to use it to produce schema-only (maybe with data from a few tables as well) dumps for developers. I've certainly wished I could offer that ability without going full-blown super-user.
Can you clarify what you mean by "supports PITR but not pg_dump"?
The role attribute specifically allows a user to execute those
functions. Further, yes, this capability could be given to a role which
is used by, say, barman, to backup the database, or by other backup
solutions which do filesystem backups, but what do you mean by "does not
support pg_dump"?
What I think you're getting at here is a role attribute which can read
all data, which could certainly be done (as, say, a "READONLY"
attribute), but I view that a bit differently, as it could be used for
auditing and other purposes besides just non-superuser pg_dump support.
Thanks!
Stephen
On Oct 16, 2014 1:59 PM, "Stephen Frost" <sfrost@snowman.net> wrote:
* Simon Riggs (simon@2ndQuadrant.com) wrote:
On 15 October 2014 06:22, Stephen Frost <sfrost@snowman.net> wrote:
BACKUP:
pg_start_backup()
pg_stop_backup()
pg_switch_xlog()
pg_create_restore_point()Yes, but its more complex. As Jim says, you need to include pg_dump,
plus you need to include the streaming utilities, e.g. pg_basebackup.I'd rather have more, simpler, role attributes than one 'catch-all'.
Once I understand what "include pg_dump" and "include pg_basebackup"
mean, I'd be happy to work on adding those.
Include pg_basebackup would mean the replication protocol methods for base
backup and streaming. Which is already covered by the REPLICATION flag.
But in think it's somewhat useful to separate these. Being able to execute
pg_stop_backup allows you to break somebody else's backup currently
running, which pg_basebackup is safe against. So being able to call those
functions is clearly a higher privilege than being able to use
pg_basebackup.
/Magnus
On 16/10/14 13:44, Stephen Frost wrote:
* Petr Jelinek (petr@2ndquadrant.com) wrote:
On 15/10/14 07:22, Stephen Frost wrote:
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()As others have commented, I too think this should support pg_dump.
I'm uttly mystified as to what that *means*. Everyone asking for it is
great but until someone can define what "support pg_dump" means, there's
not much progress I can make towards it..
The explanation you wrote to Jim makes sense, plus given the comment
from Magnus about REPLICATION flag I guess I am happy enough with it (I
might have liked to have REPLICATION flag to somehow be part of BACKUP
flag but that's very minor thing).
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.Yeah it will, mainly because extensions can load modules and can
have untrusted functions, we might want to limit which extensions
are possible to create without being superuser.The extension has to be available on the filesystem before it can be
created, of course. I'm not against providing some kind of whitelist or
similar which a superuser could control.. That's similar to how PLs
work wrt pltemplate, no?
Makes sense, there is actually extension called pgextwlist which does this.
commands/functioncmds.c
create untrusted-language functionsI often needed more granularity there (plproxy).
Not sure what you're getting at..? Is there a level of 'granularity'
for this which would make it safe for non-superusers to create
untrusted-language functions? I would think that'd be handled mainly
through extensions, but certainly curious what others think.
I've been in situation where I wanted to give access to *some* untrusted
languages to non-superuser (as not every untrusted language can do same
kind of damage) and had to solve it via scripting outside of pg.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 16 October 2014 12:59, Stephen Frost <sfrost@snowman.net> wrote:
LOGROTATE:
pg_rotate_logfile()Do we need one just for that?
It didn't seem to "belong" to any others and it's currently limited to
superuser but useful for non-superusers, so I would argue 'yes'. Now,
another option (actually, for many of these...) would be to trust in our
authorization system (GRANT EXECUTE) and remove the explicit superuser
check inside the functions, revoke EXECUTE from public, and tell users
to GRANT EXECUTE to the roles which should be allowed.
Seems like OPERATOR would be a general description that could be
useful elsewhere.
There was a suggestion raised (by Robert, I believe) that we store these
additional role attributes as a bitmask instead of individual columns.
I'm fine with either way, but it'd be a backwards-incompatible change
for anyone who looks at pg_authid. This would be across a major version
change, of course, so we are certainly within rights to do so, but I'm
also not sure how much we need to worry about a few bytes per pg_authid
row; we still have other issues if we want to try and support millions
of roles, starting with the inability to partition catalogs..
I assumed that was an internal change for fast access.
An array of role attributes would be extensible and more databasey.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Magnus Hagander (magnus@hagander.net) wrote:
On Oct 16, 2014 1:59 PM, "Stephen Frost" <sfrost@snowman.net> wrote:
Once I understand what "include pg_dump" and "include pg_basebackup"
mean, I'd be happy to work on adding those.Include pg_basebackup would mean the replication protocol methods for base
backup and streaming. Which is already covered by the REPLICATION flag.
Well, right. I had the impression there was some distinction that I
just wasn't getting.
But in think it's somewhat useful to separate these. Being able to execute
pg_stop_backup allows you to break somebody else's backup currently
running, which pg_basebackup is safe against. So being able to call those
functions is clearly a higher privilege than being able to use
pg_basebackup.
Agreed.
Thanks!
Stephen
* Petr Jelinek (petr@2ndquadrant.com) wrote:
The explanation you wrote to Jim makes sense, plus given the comment
from Magnus about REPLICATION flag I guess I am happy enough with it
(I might have liked to have REPLICATION flag to somehow be part of
BACKUP flag but that's very minor thing).
k. :)
The extension has to be available on the filesystem before it can be
created, of course. I'm not against providing some kind of whitelist or
similar which a superuser could control.. That's similar to how PLs
work wrt pltemplate, no?Makes sense, there is actually extension called pgextwlist which does this.
Yeah. Not sure if that should only exist as an extension, but that's
really a conversation for a different thread.
Not sure what you're getting at..? Is there a level of 'granularity'
for this which would make it safe for non-superusers to create
untrusted-language functions? I would think that'd be handled mainly
through extensions, but certainly curious what others think.I've been in situation where I wanted to give access to *some*
untrusted languages to non-superuser (as not every untrusted
language can do same kind of damage) and had to solve it via
scripting outside of pg.
Really curious what untrusted language you're referring to which isn't
as risky as others..? Any kind of filesystem or network access, or the
ability to run external commands, is dangerous to give non-superusers.
Perhaps more to the point- what would the more granular solution look
like..? Though, this is still getting a bit off-topic for this thread.
Thanks!
Stephen
* Simon Riggs (simon@2ndQuadrant.com) wrote:
On 16 October 2014 12:59, Stephen Frost <sfrost@snowman.net> wrote:
LOGROTATE:
pg_rotate_logfile()Do we need one just for that?
It didn't seem to "belong" to any others and it's currently limited to
superuser but useful for non-superusers, so I would argue 'yes'. Now,
another option (actually, for many of these...) would be to trust in our
authorization system (GRANT EXECUTE) and remove the explicit superuser
check inside the functions, revoke EXECUTE from public, and tell users
to GRANT EXECUTE to the roles which should be allowed.Seems like OPERATOR would be a general description that could be
useful elsewhere.
Ok.. but what else? Are there specific functions which aren't covered
by these role attributes which should be and could fall into this
category? I'm not against the idea of an 'operator' role, but I don't
like the idea that it means 'logrotate, until we figure out some other
thing which makes sense to put into this category'. For one thing, this
approach would mean that future version which add more rights to the
'operator' role attribute would mean that upgrades are granting rights
which didn't previously exist..
There was a suggestion raised (by Robert, I believe) that we store these
additional role attributes as a bitmask instead of individual columns.
I'm fine with either way, but it'd be a backwards-incompatible change
for anyone who looks at pg_authid. This would be across a major version
change, of course, so we are certainly within rights to do so, but I'm
also not sure how much we need to worry about a few bytes per pg_authid
row; we still have other issues if we want to try and support millions
of roles, starting with the inability to partition catalogs..I assumed that was an internal change for fast access.
We could make it that way by turning pg_authid into a view and using a
new catalog table for roles instead (similar to what we did with
pg_user ages ago when we added roles), but that feels like overkill to
me.
An array of role attributes would be extensible and more databasey.
Hrm. Agreed, and it would save a bit of space for the common case where
the user hasn't got any of these attributes, though it wouldn't be as
efficient as a bitmap.
For my part, I'm not really wedded to any particular catalog
representation. Having reviewed the various superuser checks, I think
there's a few more role attributes which could/should be added beyond
the ones listed, but I don't think we'll ever get to 64 of them, so a
single int64 would work if we want the most compact solution.
Thanks!
Stephen
Stephen Frost <sfrost@snowman.net> writes:
* Petr Jelinek (petr@2ndquadrant.com) wrote:
Yeah it will, mainly because extensions can load modules and can
have untrusted functions, we might want to limit which extensions
are possible to create without being superuser.
The extension has to be available on the filesystem before it can be
created, of course. I'm not against providing some kind of whitelist or
similar which a superuser could control.. That's similar to how PLs
work wrt pltemplate, no?
The existing behavior is "you can create an extension if you can execute
all the commands contained in its script". I'm not sure that messing
with that rule is a good idea; in any case it seems well out of scope
for this patch.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Petr Jelinek (petr@2ndquadrant.com) wrote:
Yeah it will, mainly because extensions can load modules and can
have untrusted functions, we might want to limit which extensions
are possible to create without being superuser.The extension has to be available on the filesystem before it can be
created, of course. I'm not against providing some kind of whitelist or
similar which a superuser could control.. That's similar to how PLs
work wrt pltemplate, no?The existing behavior is "you can create an extension if you can execute
all the commands contained in its script". I'm not sure that messing
with that rule is a good idea; in any case it seems well out of scope
for this patch.
Right, that's the normal rule. I still like the idea of letting
non-superusers create "safe" extensions, but I completely agree- beyond
the scope of this patch (as I noted in my initial post).
Thanks!
Stephen
Stephen Frost wrote:
* Petr Jelinek (petr@2ndquadrant.com) wrote:
On 15/10/14 07:22, Stephen Frost wrote:
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()As others have commented, I too think this should support pg_dump.
I'm uttly mystified as to what that *means*. Everyone asking for it is
great but until someone can define what "support pg_dump" means, there's
not much progress I can make towards it..
To me, what this repeated discussion on this particular BACKUP point
says, is that the ability to run pg_start/stop_backend and the xlog
related functions should be a different privilege, i.e. something other
than BACKUP; because later we will want the ability to grant someone the
ability to run pg_dump on the whole database without being superuser,
and we will want to use the name BACKUP for that. So I'm inclined to
propose something more specific for this like WAL_CONTROL or
XLOG_OPERATOR, say.
pg_dump doesn't require superuser rights today. If you're looking for a
role which allows a user to arbitrairly read all data, fine, but that's
a different consideration and would be a different role attribute, imv.
If you'd like the role attribute renamed to avoid confusion, I'm all for
that, just suggest something.
There.
(Along the same lines, perhaps the log rotate thingy that's being
mentioned elsewhere could be LOG_OPERATOR instead of just OPERATOR, to
avoid privilege "upgrades" as later releases include more capabilities
to the hypothetical OPERATOR capability.)
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Alvaro,
* Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
Stephen Frost wrote:
* Petr Jelinek (petr@2ndquadrant.com) wrote:
On 15/10/14 07:22, Stephen Frost wrote:
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()As others have commented, I too think this should support pg_dump.
I'm uttly mystified as to what that *means*. Everyone asking for it is
great but until someone can define what "support pg_dump" means, there's
not much progress I can make towards it..To me, what this repeated discussion on this particular BACKUP point
says, is that the ability to run pg_start/stop_backend and the xlog
related functions should be a different privilege, i.e. something other
than BACKUP; because later we will want the ability to grant someone the
ability to run pg_dump on the whole database without being superuser,
and we will want to use the name BACKUP for that. So I'm inclined to
propose something more specific for this like WAL_CONTROL or
XLOG_OPERATOR, say.
Ok. Not sure that I see 'XLOG_OPERATOR' as making sense for
'pg_start_backup' though, and I see the need to support pg_dump'ing the
whole database as a non-superuser being more like a 'READONLY' kind of
role.
We've actually already looked at the notion of a 'READONLY' or 'READALL'
role attribute and, well, it's quite simple to implement.. We're
talking about a few lines of code to implicitly allow 'USAGE' on all
schemas, plus a minor change in ExecCheckRTEPerms to always (or perhaps
*only*) include SELECT rights. As there's so much interest in that
capability, perhaps we should add it..
One of the big question is- do we take steps to try and prevent a role
with this attribute from being able to modify the database in any way?
Or would this role attribute only ever increase your rights?
pg_dump doesn't require superuser rights today. If you're looking for a
role which allows a user to arbitrairly read all data, fine, but that's
a different consideration and would be a different role attribute, imv.
If you'd like the role attribute renamed to avoid confusion, I'm all for
that, just suggest something.There.
Fair enough, just don't like the specific suggestions. :) In the docs,
we talk about pg_start_backup being for 'on-line' backups, perhaps we
use ONLINE_BACKUP ? Or PITR_BACKUP ?
(Along the same lines, perhaps the log rotate thingy that's being
mentioned elsewhere could be LOG_OPERATOR instead of just OPERATOR, to
avoid privilege "upgrades" as later releases include more capabilities
to the hypothetical OPERATOR capability.)
I'd be fine calling it LOG_OPERATOR instead, sure.
Thanks!
Stephen
On Thu, Oct 16, 2014 at 11:24 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Stephen Frost wrote:
* Petr Jelinek (petr@2ndquadrant.com) wrote:
On 15/10/14 07:22, Stephen Frost wrote:
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()As others have commented, I too think this should support pg_dump.
I'm uttly mystified as to what that *means*. Everyone asking for it is
great but until someone can define what "support pg_dump" means, there's
not much progress I can make towards it..To me, what this repeated discussion on this particular BACKUP point
says, is that the ability to run pg_start/stop_backend and the xlog
related functions should be a different privilege, i.e. something other
than BACKUP; because later we will want the ability to grant someone the
ability to run pg_dump on the whole database without being superuser,
and we will want to use the name BACKUP for that. So I'm inclined to
propose something more specific for this like WAL_CONTROL or
XLOG_OPERATOR, say.
I'm a little nervous that we're going to end up with a whole bunch of
things with names like X_control, Y_operator, and Z_admin, which I
think is particularly bad if we end up with a mix of styles and also
bad (though less so) if we end up just tacking the word "operator"
onto the end of everything.
I'd suggest calling these capabilities, and allow:
GRANT CAPABILITY whatever TO somebody;
...but keep extraneous words like "control" or "operator" out of the
capabilities names themselves. So just wal, xlog, logfile, etc.
rather than wal_operator, xlog_operator, logfile_operator and so on.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Robert Haas (robertmhaas@gmail.com) wrote:
On Thu, Oct 16, 2014 at 11:24 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
To me, what this repeated discussion on this particular BACKUP point
says, is that the ability to run pg_start/stop_backend and the xlog
related functions should be a different privilege, i.e. something other
than BACKUP; because later we will want the ability to grant someone the
ability to run pg_dump on the whole database without being superuser,
and we will want to use the name BACKUP for that. So I'm inclined to
propose something more specific for this like WAL_CONTROL or
XLOG_OPERATOR, say.I'm a little nervous that we're going to end up with a whole bunch of
things with names like X_control, Y_operator, and Z_admin, which I
think is particularly bad if we end up with a mix of styles and also
bad (though less so) if we end up just tacking the word "operator"
onto the end of everything.
Yeah, that's certainly a good point.
I'd suggest calling these capabilities, and allow:
GRANT CAPABILITY whatever TO somebody;
So, we went back to just role attributes to avoid the keyword issue..
The above would require making 'CAPABILITY' a reserved word, and there
really isn't a 'good' already-reserved word we can use there that I
found.
Also, role attributes aren't inheirited nor is there an 'ADMIN' option
for them as there is for GRANT- both of which I feel are correct for
these capabilities. Or, to say it another way, I don't think these
should have an 'ADMIN' option and I don't think they need to be
inheirited through role membership the way granted privileges are.
We could still use 'GRANT <keyword> whatever TO somebody;' without the
admin opton and without inheiritance, but I think it'd just be
confusing for users who are familiar with how GRANT works already.
Thanks!
Stephen
On Thu, Oct 16, 2014 at 2:59 PM, Stephen Frost <sfrost@snowman.net> wrote:
* Robert Haas (robertmhaas@gmail.com) wrote:
On Thu, Oct 16, 2014 at 11:24 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
To me, what this repeated discussion on this particular BACKUP point
says, is that the ability to run pg_start/stop_backend and the xlog
related functions should be a different privilege, i.e. something other
than BACKUP; because later we will want the ability to grant someone the
ability to run pg_dump on the whole database without being superuser,
and we will want to use the name BACKUP for that. So I'm inclined to
propose something more specific for this like WAL_CONTROL or
XLOG_OPERATOR, say.I'm a little nervous that we're going to end up with a whole bunch of
things with names like X_control, Y_operator, and Z_admin, which I
think is particularly bad if we end up with a mix of styles and also
bad (though less so) if we end up just tacking the word "operator"
onto the end of everything.Yeah, that's certainly a good point.
I'd suggest calling these capabilities, and allow:
GRANT CAPABILITY whatever TO somebody;
So, we went back to just role attributes to avoid the keyword issue..
The above would require making 'CAPABILITY' a reserved word, and there
really isn't a 'good' already-reserved word we can use there that I
found.
Ah, good point. Using ALTER ROLE is better. Maybe we should do ALTER
ROLE .. [ ADD | DROP ] CAPABILITY x. That would still require making
CAPABILITY a keyword, but it could be unreserved.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers