Re: 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

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

On 15 October 2014 06:22, Stephen Frost <sfrost(at)snowman(dot)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

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

* Petr Jelinek (petr(at)2ndquadrant(dot)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 functions
>
> I 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 languages
>
> I 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(at)2ndQuadrant(dot)com) wrote:
> On 15 October 2014 06:22, Stephen Frost <sfrost(at)snowman(dot)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(dot)Nasby(at)BlueTreble(dot)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(at)snowman(dot)net> wrote:
>
> * Simon Riggs (simon(at)2ndQuadrant(dot)com) wrote:
> > On 15 October 2014 06:22, Stephen Frost <sfrost(at)snowman(dot)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(at)2ndquadrant(dot)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 functions
>>
>> I 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

On 16 October 2014 12:59, Stephen Frost <sfrost(at)snowman(dot)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

* Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> On Oct 16, 2014 1:59 PM, "Stephen Frost" <sfrost(at)snowman(dot)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(at)2ndquadrant(dot)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(at)2ndQuadrant(dot)com) wrote:
> On 16 October 2014 12:59, Stephen Frost <sfrost(at)snowman(dot)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(at)snowman(dot)net> writes:
> * Petr Jelinek (petr(at)2ndquadrant(dot)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

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > * Petr Jelinek (petr(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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

Alvaro,

* Alvaro Herrera (alvherre(at)2ndquadrant(dot)com) wrote:
> Stephen Frost wrote:
> > * Petr Jelinek (petr(at)2ndquadrant(dot)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(at)2ndquadrant(dot)com> wrote:
> Stephen Frost wrote:
>> * Petr Jelinek (petr(at)2ndquadrant(dot)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

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Thu, Oct 16, 2014 at 11:24 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)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(at)snowman(dot)net> wrote:
> * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
>> On Thu, Oct 16, 2014 at 11:24 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)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

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> 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.

That works for me- would we change the existing role attributes to be
configurable this way and change everything over to using an int64 in
the catalog? Unless I'm having trouble counting, I think that would
actually result in the pg_authid catalog not changing in size at all
while giving us the ability to add these capabilities and something like
50 others if we had cause to.

Thanks,

Stephen

On Thu, Oct 16, 2014 at 3:09 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
>> 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.
>
> That works for me- would we change the existing role attributes to be
> configurable this way and change everything over to using an int64 in
> the catalog? Unless I'm having trouble counting, I think that would
> actually result in the pg_authid catalog not changing in size at all
> while giving us the ability to add these capabilities and something like
> 50 others if we had cause to.

I definitely think we should support the new syntax for the existing
attributes. I could go either way on whether to change the catalog
storage for the existing attributes. Some people might prefer to
avoid the backward compatibility break, and I can see that argument.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Thu, Oct 16, 2014 at 3:09 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> >> 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.
> >
> > That works for me- would we change the existing role attributes to be
> > configurable this way and change everything over to using an int64 in
> > the catalog? Unless I'm having trouble counting, I think that would
> > actually result in the pg_authid catalog not changing in size at all
> > while giving us the ability to add these capabilities and something like
> > 50 others if we had cause to.
>
> I definitely think we should support the new syntax for the existing
> attributes.

Ok.

> I could go either way on whether to change the catalog
> storage for the existing attributes. Some people might prefer to
> avoid the backward compatibility break, and I can see that argument.

There's really two issues when it comes to backwards compatibility here-
the catalog representation and the syntax.

My feeling is basically this- either we make a clean break to the new
syntax and catalog representation, or we just use the same approach the
existing attriubtes use. Long term, I think your proposed syntax and an
int64 representation is better but it'll mean a lot of client code that
has to change. I don't really like the idea of changing the syntax but
not the representation, nor am I thrilled with the idea of supporting
both syntaxes, and changing the syntax without changing the
representation just doesn't make sense to me as I think we'd end up
wanting to change it later, making clients have to update their code
twice.

Thanks!

Stephen

On 16 October 2014 20:04, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

>>> 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.

I thought you had it right first time. It is mighty annoying that some
privileges are GRANTed and others ALTER ROLEd.

And we did agree earlier to call these capabilities.

How about

GRANT EXECUTE [PRIVILEGES] ON CAPABILITY foo TO bar;

That is similar to granting execution privs on a function. And I think
gets round the keyword issue?

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

* Simon Riggs (simon(at)2ndQuadrant(dot)com) wrote:
> On 16 October 2014 20:04, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> >>> 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.
>
> I thought you had it right first time. It is mighty annoying that some
> privileges are GRANTed and others ALTER ROLEd.

Yeah- but there's a material difference in the two, as I tried to
outline previously..

> How about
>
> GRANT EXECUTE [PRIVILEGES] ON CAPABILITY foo TO bar;
>
> That is similar to granting execution privs on a function. And I think
> gets round the keyword issue?

No, it doesn't.. EXECUTE isn't reserved at all.

Thanks,

Stephen

On 10/16/14, 10:47 AM, Stephen Frost wrote:
>>>> 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?

Let me address the pg_dump case first, because it's simpler. I want a way to allow certain roles to successfully run pg_dump without being superuser and without having to manually try and maintain a magic read-all role. Note that pg_dump might (today or in the future) need more than just read-all so it's probably wise to treat the two features (pg_dump vs a plain read-all) as separate.

For PITR, I see 3 different needs:

1) The ability for someone to start a backup, and if needed cancel that backup
2) The ability to manage running backups/archiving
3) The ability to actually setup/modify PITR infrastructure

#2 is probably a weak case that may not be needed; I include it because someone mentioned stopping a backup that someone else started.

I think #3 should just require superuser.

#1 is what you'd want a more junior person to handle. "Bob needs a snapshot of cluster A". This role needs to be able to run everything you need to get that backup started, monitor it, and cancel if needed.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

On Thu, Oct 16, 2014 at 3:34 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
>> On Thu, Oct 16, 2014 at 3:09 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> > * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
>> >> 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.
>> >
>> > That works for me- would we change the existing role attributes to be
>> > configurable this way and change everything over to using an int64 in
>> > the catalog? Unless I'm having trouble counting, I think that would
>> > actually result in the pg_authid catalog not changing in size at all
>> > while giving us the ability to add these capabilities and something like
>> > 50 others if we had cause to.
>>
>> I definitely think we should support the new syntax for the existing
>> attributes.
>
> Ok.
>
>> I could go either way on whether to change the catalog
>> storage for the existing attributes. Some people might prefer to
>> avoid the backward compatibility break, and I can see that argument.
>
> There's really two issues when it comes to backwards compatibility here-
> the catalog representation and the syntax.
>
> My feeling is basically this- either we make a clean break to the new
> syntax and catalog representation, or we just use the same approach the
> existing attriubtes use. Long term, I think your proposed syntax and an
> int64 representation is better but it'll mean a lot of client code that
> has to change. I don't really like the idea of changing the syntax but
> not the representation, nor am I thrilled with the idea of supporting
> both syntaxes, and changing the syntax without changing the
> representation just doesn't make sense to me as I think we'd end up
> wanting to change it later, making clients have to update their code
> twice.

I don't see any reason why it has to be both or neither.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Thu, Oct 16, 2014 at 3:34 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > My feeling is basically this- either we make a clean break to the new
> > syntax and catalog representation, or we just use the same approach the
> > existing attriubtes use. Long term, I think your proposed syntax and an
> > int64 representation is better but it'll mean a lot of client code that
> > has to change. I don't really like the idea of changing the syntax but
> > not the representation, nor am I thrilled with the idea of supporting
> > both syntaxes, and changing the syntax without changing the
> > representation just doesn't make sense to me as I think we'd end up
> > wanting to change it later, making clients have to update their code
> > twice.
>
> I don't see any reason why it has to be both or neither.

My reasoning on that is that either breaks existing clients and so
they'll have to update and if we're going to force that then we might as
well go whole-hog and get it over with once.

If my assumption is incorrect and we don't think changes to the catalog
representation will break existing clients then I withdraw the argument
that they should be done together.

For the syntax piece of it, my feeling is that all these role attributes
should be handled the same way. There's three ways to address that-
support them using the existing syntax, change to a new syntax and only
support that, or have two different syntaxes. I don't like the idea
that these new attributes will be supported with one syntax but the old
attributes would be supported with a different syntax.

If we think it's too much to change in one release, I could see changing
the catalog representation and then providing the new syntax while
supporting the old syntax as deprecated, but we do have a pretty bad
history of such changes and any maintained client should be getting
updated for the new role attributes anyway..

Thanks!

Stephen

Robert Haas wrote:
> On Thu, Oct 16, 2014 at 3:34 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> > My feeling is basically this- either we make a clean break to the new
> > syntax and catalog representation, or we just use the same approach the
> > existing attriubtes use. Long term, I think your proposed syntax and an
> > int64 representation is better but it'll mean a lot of client code that
> > has to change. I don't really like the idea of changing the syntax but
> > not the representation, nor am I thrilled with the idea of supporting
> > both syntaxes, and changing the syntax without changing the
> > representation just doesn't make sense to me as I think we'd end up
> > wanting to change it later, making clients have to update their code
> > twice.
>
> I don't see any reason why it has to be both or neither.

I was thinking we would change the catalogs and implement the new syntax
for new and old settings, but also keep the old syntax working as a
backward compatibility measure. I don't see what's so terrible about
continuing to support the old syntax; we do that in COPY and EXPLAIN,
for example.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 16 October 2014 20:37, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

>> How about
>>
>> GRANT EXECUTE [PRIVILEGES] ON CAPABILITY foo TO bar;
>>
>> That is similar to granting execution privs on a function. And I think
>> gets round the keyword issue?
>
> No, it doesn't.. EXECUTE isn't reserved at all.

Yet GRANT EXECUTE is already valid syntax, so that should work.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

* Simon Riggs (simon(at)2ndQuadrant(dot)com) wrote:
> On 16 October 2014 20:37, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
> >> How about
> >>
> >> GRANT EXECUTE [PRIVILEGES] ON CAPABILITY foo TO bar;
> >>
> >> That is similar to granting execution privs on a function. And I think
> >> gets round the keyword issue?
> >
> > No, it doesn't.. EXECUTE isn't reserved at all.
>
> Yet GRANT EXECUTE is already valid syntax, so that should work.

Yeah, sorry, the issue with the above is that the "ON CAPABILITY" would
mean CAPABILITY needs to be reserved as otherwise we don't know if it's
a function or something else.

The keyword issue is with

GRANT <something> TO <role>;

As <something> could be a role.

Not sure offhand if

GRANT EXECUTE PRIVILEGES ON CAPABILITY foo TO bar;

would work.. In general, I'm not anxious to get involved in the
SQL-specified GRANT syntax though unless there's really good reason to.

Also, these aren't like normally granted privileges which can have an
ADMIN option and which are inheirited through role membership..

Thanks,

Stephen

* Alvaro Herrera (alvherre(at)2ndquadrant(dot)com) wrote:
> Robert Haas wrote:
> > On Thu, Oct 16, 2014 at 3:34 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
> > > My feeling is basically this- either we make a clean break to the new
> > > syntax and catalog representation, or we just use the same approach the
> > > existing attriubtes use. Long term, I think your proposed syntax and an
> > > int64 representation is better but it'll mean a lot of client code that
> > > has to change. I don't really like the idea of changing the syntax but
> > > not the representation, nor am I thrilled with the idea of supporting
> > > both syntaxes, and changing the syntax without changing the
> > > representation just doesn't make sense to me as I think we'd end up
> > > wanting to change it later, making clients have to update their code
> > > twice.
> >
> > I don't see any reason why it has to be both or neither.
>
> I was thinking we would change the catalogs and implement the new syntax
> for new and old settings, but also keep the old syntax working as a
> backward compatibility measure. I don't see what's so terrible about
> continuing to support the old syntax; we do that in COPY and EXPLAIN,
> for example.

It just complicates things and I'm not sure there's much benefit to it.
Clients are going to need to be updated to support the new attributes
anyway, and if the new attributes can only be used through the new
syntax, well, I don't know why they'd want to deal with the old syntax
too.

That said, I don't feel very strongly about that position, so if you and
Robert (and others on the thread) agree that's the right approach then
I'll see about getting it done.

Thanks!

Stephen

All,

> That said, I don't feel very strongly about that position, so if you and
> Robert (and others on the thread) agree that's the right approach then
> I'll see about getting it done.

We haven't reached consensus on this one yet and I didn't want it to fall
too far off the radar.

Here is what I summarize as the current state of the discussion:

1. Syntax:

ALTER ROLE <role> { ADD | DROP } CAPABILITY <capability>

* I think this is the most straight forward approach as it still close to
the current syntax.
* Perhaps keeping the current syntax around as deprecated to be removed in
a scheduled future version. (provide a "deprecated" message to the user?)

or

GRANT EXECUTE PRIVILEGES ON <capability> TO <role>

* Though, this will be tricky since EXECUTE is not reserved and the
currently state of GRANT in the grammar would require either refactoring or
making it RESERVED... neither I think are acceptable at this point in time
for obvious reasons.

2. Catalog Representation:

Condense all attributes in pg_authid to single int64 column and create
bitmasks accordingly.

Obviously there is some concern for upgrading and whether to do both at
once or to do them incrementally. IMHO, I think if the changes are going
to be made, then we should go ahead and do them at the same time. Though,
would it be beneficial to separate in to two distinct patches?

-Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

* Adam Brightwell (adam(dot)brightwell(at)crunchydatasolutions(dot)com) wrote:
> > That said, I don't feel very strongly about that position, so if you and
> > Robert (and others on the thread) agree that's the right approach then
> > I'll see about getting it done.

Thanks for trying to make progress on this.

> We haven't reached consensus on this one yet and I didn't want it to fall
> too far off the radar.
>
> Here is what I summarize as the current state of the discussion:

For my 2c- I don't think we'd be able to drop the old syntax and
therefore I'm not sure that I really see the point in adding new syntax.
I don't hold that position strongly, but I don't like the idea that
we're just going to be sitting on our thumbs because we can't agree on
the color of the bike shed.

If there is agreement to move forward with using the syntax below,
great, that can be implemented in relatively short order. If there
isn't, then let's move forward with the existing syntax and change it
later, if there is agreement to do so at some point in the future.

I don't like the idea of tying it into GRANT. GRANT is SQL-spec
defined, quite overloaded already, and role attributes don't act like
GRANTs anyway (there's no ADMIN option and they aren't inheirited).

> 2. Catalog Representation:
>
> Condense all attributes in pg_authid to single int64 column and create
> bitmasks accordingly.

I'd suggest we do this regardless and the only question is if we feel
there is need to provide a view to split them back out again or not.
I'm inclined to say 'no' to another view and instead suggest we provide
SQL-level "has_whatever_privilege" for these which match the existing C
functions, update our clients to use those, and encourage others to do
the same. Probably also helpful would be a single function that returns
a simple list of the non-default role attributes which a user has and
we'd simplify psql's describeRoles by having it use that instead.

> Obviously there is some concern for upgrading and whether to do both at
> once or to do them incrementally. IMHO, I think if the changes are going
> to be made, then we should go ahead and do them at the same time. Though,
> would it be beneficial to separate in to two distinct patches?

I agree that doing these changes at the same time makes sense, if we can
get agreement on what exactly to do. I've shared my thoughts, but we
really need input from others, ideally of the form of "I prefer X over
Y" rather than "well, we could do X or Y".

Thanks!

Stephen

On Mon, Nov 3, 2014 at 11:44 AM, Adam Brightwell
<adam(dot)brightwell(at)crunchydatasolutions(dot)com> wrote:
>> That said, I don't feel very strongly about that position, so if you and
>> Robert (and others on the thread) agree that's the right approach then
>> I'll see about getting it done.
>
> We haven't reached consensus on this one yet and I didn't want it to fall
> too far off the radar.
>
> Here is what I summarize as the current state of the discussion:
>
> 1. Syntax:
>
> ALTER ROLE <role> { ADD | DROP } CAPABILITY <capability>

Seems reasonable.

> * Perhaps keeping the current syntax around as deprecated to be removed in a
> scheduled future version. (provide a "deprecated" message to the user?)

Yeah, I don't think we should remove the existing syntax because a lot
of people are used to it. We still have some very old COPY syntax
around for backward-compatibility, and it's not hurting us, either.
At the same time, I think recasting the existing capability-like
things as capabilities per se is a good idea, because otherwise we've
got this old stuff that is randomly different for no especially good
reason.

> GRANT EXECUTE PRIVILEGES ON <capability> TO <role>

Yuck.

The only other approach I can think of is have some magic, hard-coded
roles that implicitly have each capability, and then those can be
granted. e.g. have a role pg_unrestricted_copy or whatever with a
given OID, and then test has_privs_of_role() with that OID.

> Condense all attributes in pg_authid to single int64 column and create
> bitmasks accordingly.
>
> Obviously there is some concern for upgrading and whether to do both at once
> or to do them incrementally. IMHO, I think if the changes are going to be
> made, then we should go ahead and do them at the same time. Though, would
> it be beneficial to separate in to two distinct patches?

If we're going to change the catalog representation for the existing
capabilities, I'd recommend that the first patch change the catalog
representation and add the new syntax without adding any more
capabilities; and then the second and subsequent patches can add
additional capabilities.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

All,

> 2. Catalog Representation:
>
> Condense all attributes in pg_authid to single int64 column and create
> bitmasks accordingly.
>

I have been working on these changes and I was hoping for some
assistance/recommendations.

Currently, I am using int32 simply because int64 is causing some issues.
The issue is that genbki.pl is not able to associate it with the int8 type
as defined in pg_type.h. Therefore Schema_pg_authid in schemapg.h isn't
defined correctly. I've been digging and scratching my head on this one
but I have reached a point where I think it would be better just to ask.

Any thoughts or recommendations on how I should approach this?

-Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

All,

> Currently, I am using int32 simply because int64 is causing some issues.
> The issue is that genbki.pl is not able to associate it with the int8
> type as defined in pg_type.h. Therefore Schema_pg_authid in schemapg.h
> isn't defined correctly. I've been digging and scratching my head on this
> one but I have reached a point where I think it would be better just to ask.
>

Attached is a quite trivial patch that addresses the int64 (C) to int8
(SQL) mapping issue.

Further digging revealed that Catalog.pm wasn't accounting for int64
(thanks Stephen). Would it be better to include this change as a separate
patch (as attached) or would it be preferable to include with a larger role
attribute bitmask patch?

Thanks,
Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

Adam Brightwell <adam(dot)brightwell(at)crunchydatasolutions(dot)com> writes:
> Currently, I am using int32 simply because int64 is causing some issues.
> The issue is that genbki.pl is not able to associate it with the int8 type
> as defined in pg_type.h. Therefore Schema_pg_authid in schemapg.h isn't
> defined correctly. I've been digging and scratching my head on this one
> but I have reached a point where I think it would be better just to ask.

> Any thoughts or recommendations on how I should approach this?

Teach src/backend/catalog/Catalog.pm about it.

There once was a policy against using int64 in the system catalogs, which
is why it wasn't there already; but the reason for that policy is long
gone.

regards, tom lane

On 11/18/2014 04:58 PM, Adam Brightwell wrote:
> All,
>
> Currently, I am using int32 simply because int64 is causing some
> issues. The issue is that genbki.pl <http://genbki.pl> is not
> able to associate it with the int8 type as defined in pg_type.h.
> Therefore Schema_pg_authid in schemapg.h isn't defined correctly.
> I've been digging and scratching my head on this one but I have
> reached a point where I think it would be better just to ask.
>
>
> Attached is a quite trivial patch that addresses the int64 (C) to int8
> (SQL) mapping issue.
>
> Further digging revealed that Catalog.pm wasn't accounting for int64
> (thanks Stephen). Would it be better to include this change as a
> separate patch (as attached) or would it be preferable to include with
> a larger role attribute bitmask patch?
>
>

I think we should just apply this now. As Tom said the reason for not
doing it is long gone.

cheers

andrew

All,

> If we're going to change the catalog representation for the existing
> capabilities, I'd recommend that the first patch change the catalog
> representation and add the new syntax without adding any more
> capabilities; and then the second and subsequent patches can add
> additional capabilities.

Attached is an initial patch for review and discussion that takes a swing
at changing the catalog representation.

This patch includes:

* int64 (C) to int8 (SQL) mapping for genbki.
* replace all role attributes columns in pg_authid with single int64 column
named rolattr.
* update CreateRole and AlterRole to use rolattr.
* update all has_*_privilege functions to check rolattr.
* builtin SQL function 'has_role_attribute' that takes a role oid and text
name of the attribute as input and returns a boolean.

Items not currently addressed:

* New syntax - more discussion needs to occur around these potential
changes. Specifically, how would CREATE USER/ROLE be affected? I suppose
it is OK to keep it as WITH <attribute_or_capability>, though if ALTER ROLE
is modified to have ADD | DROP CAPABILITY for consistency would WITH
CAPABILITY <value,...>, make more sense for CREATE? At any rate, I think
there is obviously much more discussion that needs to be had.
* Documentation - want to gain feedback on implementation prior to making
changes.
* Update regression tests, rules test for system_views - want to gain
feedback on approach to handling pg_roles, etc. before updating.

Also, what would be the community preference on tracking these
attached/proposed changes? Should I create a separate entry in the next CF
for this item (seems prudent) or would it be preferred to keep it attached
to the current 'new attributes' CF entry?

Thanks,
Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

On 11/06/2014 03:31 AM, Robert Haas wrote:
> [snip]
>> We haven't reached consensus on this one yet and I didn't want it to fall
>> too far off the radar.
>>
>> Here is what I summarize as the current state of the discussion:
>>
>> 1. Syntax:
>>
>> ALTER ROLE <role> { ADD | DROP } CAPABILITY <capability>

Though a bit late to this thread, I would like to request comments on
potentially beneficial new roles ?? and/or capabilities which I have
recently found needing myself.
The suggested syntax looks intuitive and potentially very flexible.
I'll try to summarize up what I recall from the thread plus my own
itchs, to try and get others to comment and expand on the matter.

We currently have:
* SUPERUSER / CREATEUSER
* CREATEDB
* CREATEROLE
* LOGIN
* REPLICATION

(plus INHERITS and ADMIN options, of course)

It has also been suggested to include a
* BACKUP role (capability?) i.e. ability to take an snapshot and
read all relations, views, triggers and functions (even bypassing RLS)
and the catalog in order to produce a full, consistent dump of the whole
cluster.

and I seem to recall something along the lines of
* AUDIT, potentially limited to just engage

I am hereby suggesting the addition of a
* MAINTENANCE role, which would be able to perform VACUUM, ANALYZE,
REINDEX *CONCURRENTLY* and REFRESH MATERIALIZED VIEW *CONCURRENTLY* ...
and potentially even ALTER TABLE VALIDATE CONSTRAINT (if we are able to
produce a non-blocking/fully concurrent version)

... which might become very useful for DBAs wishing to use some
password-less roles for scheduled maintenance routines while at the same
time reducing the exposure.

While at it, the replication role might as well gain the ability to
promote/demote a cluster (standby<->active), or shall it be some kind of
FAILOVER role/capability ?

Thanks in advance.

/ J.L.

* Andrew Dunstan (andrew(at)dunslane(dot)net) wrote:
> On 11/18/2014 04:58 PM, Adam Brightwell wrote:
> >Attached is a quite trivial patch that addresses the int64 (C) to
> >int8 (SQL) mapping issue.
>
> I think we should just apply this now. As Tom said the reason for
> not doing it is long gone.

Alright, done.

Thanks!

Stephen

All,

I want to revive this thread and continue to move these new role attributes
forward.

In summary, the ultimate goal is to include new role attributes for common
operations which currently require superuser privileges.

Initially proposed were the following attributes:

* BACKUP - allows role to perform backup operations
* LOGROTATE - allows role to rotate log files
* MONITOR - allows role to view pg_stat_* details
* PROCSIGNAL - allows role to signal backend processes

It seems that PROCSIGNAL and MONITOR were generally well received and
probably don't warrant much more discussion at this point.

However, based on previous discussions, there seemed to be some uncertainty
on how to handle BACKUP and LOGROTATE.

Concerns:

* LOGROTATE - only associated with one function/operation.
* BACKUP - perceived to be too broad of a permission as it it would provide
the ability to run pg_start/stop_backend and the xlog related functions.
It is general sentiment is that these should be handled as separate
privileges.
* BACKUP - preferred usage is with pg_dump to giving a user the ability to
run pg_dump on the whole database without being superuser.

Previous Recommendations:

* LOGROTATE - Use OPERATOR - concern was expressed that this might be too
general of an attribute for this purpose. Also, concern for privilege
'upgrades' as it includes more capabilities in later releases.
* LOGROTATE - Use LOG_OPERATOR - generally accepted, but concern was raise
for using extraneous descriptors such as '_OPERATOR' and '_ADMIN', etc.
* BACKUP - Use WAL_CONTROL for pg_start/stop_backup - no major
disagreement, though same concern regarding extraneous descriptors.
* BACKUP - Use XLOG_OPERATOR for xlog operations - no major disagreement,
though same concern regarding extraneous descriptors.
* BACKUP - Use BACKUP for granting non-superuser ability to run pg_dump on
whole database.

Given the above and previous discussions:

I'd like to propose the following new role attributes:

BACKUP - allows role to perform pg_dump* backups of whole database.
WAL - allows role to execute pg_start_backup/pg_stop_backup functions.
XLOG - allows role to execute xlog operations.
LOG - allows role to rotate log files - remains broad enough to consider
future log related operations.
MONITOR - allows role to view pg_stat_* details.
PROCSIGNAL - allows role to signal backend processes.

If these seem reasonable, then I'll begin updating the initial/current
patch submitted. But in either case, feedback and suggestions are
certainly welcome and appreciated.

Thanks,
Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

On Wed, Dec 24, 2014 at 6:48 PM, Adam Brightwell <
adam(dot)brightwell(at)crunchydatasolutions(dot)com> wrote:

> All,
>
> I want to revive this thread and continue to move these new role
> attributes forward.
>
> In summary, the ultimate goal is to include new role attributes for common
> operations which currently require superuser privileges.
>
> Initially proposed were the following attributes:
>
> * BACKUP - allows role to perform backup operations
> * LOGROTATE - allows role to rotate log files
> * MONITOR - allows role to view pg_stat_* details
> * PROCSIGNAL - allows role to signal backend processes
>
> It seems that PROCSIGNAL and MONITOR were generally well received and
> probably don't warrant much more discussion at this point.
>
> However, based on previous discussions, there seemed to be some
> uncertainty on how to handle BACKUP and LOGROTATE.
>
> Concerns:
>
> * LOGROTATE - only associated with one function/operation.
> * BACKUP - perceived to be too broad of a permission as it it would
> provide the ability to run pg_start/stop_backend and the xlog related
> functions. It is general sentiment is that these should be handled as
> separate privileges.
>
* BACKUP - preferred usage is with pg_dump to giving a user the ability to
> run pg_dump on the whole database without being superuser.
>
> Previous Recommendations:
>
> * LOGROTATE - Use OPERATOR - concern was expressed that this might be too
> general of an attribute for this purpose. Also, concern for privilege
> 'upgrades' as it includes more capabilities in later releases.
> * LOGROTATE - Use LOG_OPERATOR - generally accepted, but concern was raise
> for using extraneous descriptors such as '_OPERATOR' and '_ADMIN', etc.
> * BACKUP - Use WAL_CONTROL for pg_start/stop_backup - no major
> disagreement, though same concern regarding extraneous descriptors.
> * BACKUP - Use XLOG_OPERATOR for xlog operations - no major disagreement,
> though same concern regarding extraneous descriptors.
> * BACKUP - Use BACKUP for granting non-superuser ability to run pg_dump on
> whole database.
>
> Given the above and previous discussions:
>
> I'd like to propose the following new role attributes:
>
> BACKUP - allows role to perform pg_dump* backups of whole database.
>

I'd suggest it's called DUMP if that's what it allows, to keep it separate
from the backup parts.

> WAL - allows role to execute pg_start_backup/pg_stop_backup functions.
>
XLOG - allows role to execute xlog operations.

That seems really bad names, IMHO. Why? Because we use WAL and XLOG
throughout documentation and parameters and code to mean *the same thing*.
And here they'd suddenly mean different things. If we need them as separate
privileges, I think we need much better names. (And a better description -
what is "xlog operations" really?)

And the one under WAL would be very similar to what REPLICATION does today.
Or are you saying that it should specifically *not* allow base backups done
through the replication protocol, only the exclusive ones?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus,

Thanks for the feedback.

>> BACKUP - allows role to perform pg_dump* backups of whole database.
>>
>
> I'd suggest it's called DUMP if that's what it allows, to keep it separate
> from the backup parts.
>

Makes sense to me.

That seems really bad names, IMHO. Why? Because we use WAL and XLOG
> throughout documentation and parameters and code to mean *the same thing*.
> And here they'd suddenly mean different things. If we need them as separate
> privileges, I think we need much better names. (And a better description -
> what is "xlog operations" really?)
>

Fair enough, ultimately what I was trying to address is the following
concern raised by Alvaro:

"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."

Upon re-reading it (and other discussions around it) I believe that I must
have misinterpreted. Initially, I read it to mean that we needed the
following separate permissions.

1) ability to pg_dump
2) ability to start/stop backups
3) ability to execute xlog related functions

When indeed, what it meant was to have the following separate (effectively
merging #2 and #3):

1) ability to pg_dump
2) ability to start/stop backups *and* ability to execute xlog related
functions.

My apologies on that confusion.

Given this clarification:

I think #1 could certainly be answered by using DUMP. I have no strong
opinion in either direction, though I do think that BACKUP does make the
most sense for #2. Previously, Stephen had mentioned a READONLY capability
that could effectively work for pg_dump, though, Jim's suggestion of
keeping 'read-all' separate from 'ability to pg_dump' seems logical. In
either case, I certainly wouldn't mind having a wider agreement/consensus
on this approach.

So, here is a revised list of proposed attributes:

* BACKUP - allows role to perform backup operations (as originally proposed)
* LOG - allows role to rotate log files - remains broad enough to consider
future log related operations
* DUMP - allows role to perform pg_dump* backups of whole database
* MONITOR - allows role to view pg_stat_* details (as originally proposed)
* PROCSIGNAL - allows role to signal backend processes (as originally
proposed)well

Thanks,
Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

Adam,

* Adam Brightwell (adam(dot)brightwell(at)crunchydatasolutions(dot)com) wrote:
> > I'd suggest it's called DUMP if that's what it allows, to keep it separate
> > from the backup parts.
>
> Makes sense to me.

I'm fine calling it 'DUMP', but for different reasons.

We have no (verifiable) idea what client program is being used to
connect and therefore we shouldn't try to tie the name of the client
program to the permission.

That said, a 'DUMP' privilege which allows the user to dump the contents
of the entire database is entirely reasonable. We need to be clear in
the documentation though- such a 'DUMP' privilege is essentially
granting USAGE on all schemas and table-level SELECT on all tables and
sequences (anything else..?). To be clear, a user with this privilege
can utilize that access without using pg_dump.

One other point is that this shouldn't imply any other privileges, imv.
I'm specifically thinking of BYPASSRLS- that's independently grantable
and therefore should be explicitly set, if it's intended. Things
should work 'sanely' with any combination of the two options.
Similairly, DUMP shouldn't imply BACKUP or visa-versa. In particular,
I'd like to see roles which have only the BACKUP privilege be unable to
directly read any data (modulo things granted to PUBLIC). This would
allow an unprivileged user to manage backups, kick off ad-hoc ones, etc,
without being able to actually access any of the data (this would
require the actual backup system to have a similar control, but that's
entirely possible with more advanced SANs and enterprise backup
solutions).

> > That seems really bad names, IMHO. Why? Because we use WAL and XLOG
> > throughout documentation and parameters and code to mean *the same thing*.
> > And here they'd suddenly mean different things. If we need them as separate
> > privileges, I think we need much better names. (And a better description -
> > what is "xlog operations" really?)
> >
>
> Fair enough, ultimately what I was trying to address is the following
> concern raised by Alvaro:
>
> "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."

Note that the BACKUP role attribute was never intended to cover the
pg_dump use-case. Simply the name of it caused confusion though. I'm
not sure if adding a DUMP role attribute is sufficient enough to address
that confusion, but I haven't got a better idea either.

> When indeed, what it meant was to have the following separate (effectively
> merging #2 and #3):
>
> 1) ability to pg_dump
> 2) ability to start/stop backups *and* ability to execute xlog related
> functions.

That sounds reasonable to me (and is what was initially proposed, though
I've come around to the thinking that this BACKUP role attribute should
also allow pg_xlog_replay_pause/resume(), as those can be useful on
replicas).

> Given this clarification:
>
> I think #1 could certainly be answered by using DUMP. I have no strong
> opinion in either direction, though I do think that BACKUP does make the
> most sense for #2. Previously, Stephen had mentioned a READONLY capability
> that could effectively work for pg_dump, though, Jim's suggestion of
> keeping 'read-all' separate from 'ability to pg_dump' seems logical. In
> either case, I certainly wouldn't mind having a wider agreement/consensus
> on this approach.

The read-all vs. ability-to-pg_dump distinction doesn't really exist for
role attributes, as I see it (see my comments above). That said, having
DUMP or read-all is different from read-*only*, which would probably be
good to have independently. I can imagine a use-case for a read-only
account which only has read ability for those tables, schemas, etc,
explicitly granted to it.

There is one issue that occurs to me, however. We're talking about
pg_dump, but what about pg_dumpall? In particular, I don't think the
DUMP privilege should provide access to pg_authid, as that would allow
the user to bypass the privilege system in some environments by using
the hash to log in as a superuser. Now, I don't encourage using
password based authentication, especially for superuser accounts, but
lots of people do. The idea with these privileges is to allow certain
operations to be performed by a non-superuser while preventing trivial
access to superuser. Perhaps it's pie-in-the-sky, but my hope is to
achieve that.

> So, here is a revised list of proposed attributes:
>
> * BACKUP - allows role to perform backup operations (as originally proposed)
> * LOG - allows role to rotate log files - remains broad enough to consider
> future log related operations
> * DUMP - allows role to perform pg_dump* backups of whole database
> * MONITOR - allows role to view pg_stat_* details (as originally proposed)
> * PROCSIGNAL - allows role to signal backend processes (as originally
> proposed)well

Looks reasonable to me.

Thanks!

Stephen

On 12/29/14, 4:01 PM, Stephen Frost wrote:
> Adam,
>
> * Adam Brightwell (adam(dot)brightwell(at)crunchydatasolutions(dot)com) wrote:
>>> I'd suggest it's called DUMP if that's what it allows, to keep it separate
>>> from the backup parts.
>>
>> Makes sense to me.
>
> I'm fine calling it 'DUMP', but for different reasons.
>
> We have no (verifiable) idea what client program is being used to
> connect and therefore we shouldn't try to tie the name of the client
> program to the permission.
>
> That said, a 'DUMP' privilege which allows the user to dump the contents
> of the entire database is entirely reasonable. We need to be clear in
> the documentation though- such a 'DUMP' privilege is essentially
> granting USAGE on all schemas and table-level SELECT on all tables and
> sequences (anything else..?). To be clear, a user with this privilege
> can utilize that access without using pg_dump.

Yeah... it may be better to call this something other than DUMP (see below).

> One other point is that this shouldn't imply any other privileges, imv.
> I'm specifically thinking of BYPASSRLS- that's independently grantable
> and therefore should be explicitly set, if it's intended. Things
> should work 'sanely' with any combination of the two options.

That does violate POLA, but it's probably worth doing so...

> Similairly, DUMP shouldn't imply BACKUP or visa-versa. In particular,
> I'd like to see roles which have only the BACKUP privilege be unable to
> directly read any data (modulo things granted to PUBLIC). This would
> allow an unprivileged user to manage backups, kick off ad-hoc ones, etc,
> without being able to actually access any of the data (this would
> require the actual backup system to have a similar control, but that's
> entirely possible with more advanced SANs and enterprise backup
> solutions).

Yes, since a binary backup doesn't need to actually "read" data, it shouldn't implicitly allow a user granted that role to either.

>> Given this clarification:
>>
>> I think #1 could certainly be answered by using DUMP. I have no strong
>> opinion in either direction, though I do think that BACKUP does make the
>> most sense for #2. Previously, Stephen had mentioned a READONLY capability
>> that could effectively work for pg_dump, though, Jim's suggestion of
>> keeping 'read-all' separate from 'ability to pg_dump' seems logical. In
>> either case, I certainly wouldn't mind having a wider agreement/consensus
>> on this approach.
>
> The read-all vs. ability-to-pg_dump distinction doesn't really exist for
> role attributes, as I see it (see my comments above). That said, having
> DUMP or read-all is different from read-*only*, which would probably be
> good to have independently. I can imagine a use-case for a read-only
> account which only has read ability for those tables, schemas, etc,
> explicitly granted to it.

My specific concern about DUMP vs read all/only is IIRC dump needs to do more extensive locking than regular selects do, which can cause production problems.

> There is one issue that occurs to me, however. We're talking about
> pg_dump, but what about pg_dumpall? In particular, I don't think the
> DUMP privilege should provide access to pg_authid, as that would allow
> the user to bypass the privilege system in some environments by using
> the hash to log in as a superuser. Now, I don't encourage using
> password based authentication, especially for superuser accounts, but
> lots of people do. The idea with these privileges is to allow certain
> operations to be performed by a non-superuser while preventing trivial
> access to superuser. Perhaps it's pie-in-the-sky, but my hope is to
> achieve that.

Ugh, hadn't thought about that. :(

>> So, here is a revised list of proposed attributes:
>>
>> * BACKUP - allows role to perform backup operations (as originally proposed)
>> * LOG - allows role to rotate log files - remains broad enough to consider
>> future log related operations
>> * DUMP - allows role to perform pg_dump* backups of whole database
>> * MONITOR - allows role to view pg_stat_* details (as originally proposed)
>> * PROCSIGNAL - allows role to signal backend processes (as originally
>> proposed)well

Given the confusion that can exist with the data reading stuff, perhaps BINARY BACKUP or XLOG would be a better term, especially since this will probably have some overlap with streaming replication.

Likewise, if we segregate "DUMP" and BYPASSRLS then I think we need to call DUMP something else. Otherwise, it's a massive foot-gun; you get a "successful" backup only to find out it contains only a small part of the database.

My how this has become a can of worms...
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

Jim,

* Jim Nasby (Jim(dot)Nasby(at)BlueTreble(dot)com) wrote:
> On 12/29/14, 4:01 PM, Stephen Frost wrote:
> >That said, a 'DUMP' privilege which allows the user to dump the contents
> >of the entire database is entirely reasonable. We need to be clear in
> >the documentation though- such a 'DUMP' privilege is essentially
> >granting USAGE on all schemas and table-level SELECT on all tables and
> >sequences (anything else..?). To be clear, a user with this privilege
> >can utilize that access without using pg_dump.
>
> Yeah... it may be better to call this something other than DUMP (see below).

Did you favor something else below..? I don't see it.

> >One other point is that this shouldn't imply any other privileges, imv.
> >I'm specifically thinking of BYPASSRLS- that's independently grantable
> >and therefore should be explicitly set, if it's intended. Things
> >should work 'sanely' with any combination of the two options.
>
> That does violate POLA, but it's probably worth doing so...

That really depends on your view of the privilege. I'm inclined to view
it from the more-tightly-constrained point of view which matches up with
what I anticipate it actually providing. That doesn't translate into a
short name very well though, unfortunately.

> >The read-all vs. ability-to-pg_dump distinction doesn't really exist for
> >role attributes, as I see it (see my comments above). That said, having
> >DUMP or read-all is different from read-*only*, which would probably be
> >good to have independently. I can imagine a use-case for a read-only
> >account which only has read ability for those tables, schemas, etc,
> >explicitly granted to it.
>
> My specific concern about DUMP vs read all/only is IIRC dump needs to do more extensive locking than regular selects do, which can cause production problems.

The locks aren't any more extensive than regular selects, it's just that
the entire pg_dump works inside of one large transaction which lasts a
good long time and simply that can cause issues with high-rate update
systems.

> >There is one issue that occurs to me, however. We're talking about
> >pg_dump, but what about pg_dumpall? In particular, I don't think the
> >DUMP privilege should provide access to pg_authid, as that would allow
> >the user to bypass the privilege system in some environments by using
> >the hash to log in as a superuser. Now, I don't encourage using
> >password based authentication, especially for superuser accounts, but
> >lots of people do. The idea with these privileges is to allow certain
> >operations to be performed by a non-superuser while preventing trivial
> >access to superuser. Perhaps it's pie-in-the-sky, but my hope is to
> >achieve that.
>
> Ugh, hadn't thought about that. :(

I don't think it kills the whole idea, but we do need to work out how to
address it.

> >>* BACKUP - allows role to perform backup operations (as originally proposed)
> >>* LOG - allows role to rotate log files - remains broad enough to consider
> >>future log related operations
> >>* DUMP - allows role to perform pg_dump* backups of whole database
> >>* MONITOR - allows role to view pg_stat_* details (as originally proposed)
> >>* PROCSIGNAL - allows role to signal backend processes (as originally
> >>proposed)well
>
> Given the confusion that can exist with the data reading stuff, perhaps BINARY BACKUP or XLOG would be a better term, especially since this will probably have some overlap with streaming replication.

I don't really like 'xlog' as a permission. BINARY BACKUP is
interesting but if we're going to go multi-word, I would think we would
do PITR BACKUP or something FILESYSTEM BACKUP or similar. I'm not
really a big fan of the two-word options though.

> Likewise, if we segregate "DUMP" and BYPASSRLS then I think we need to call DUMP something else. Otherwise, it's a massive foot-gun; you get a "successful" backup only to find out it contains only a small part of the database.

That's actually already dealt with. pg_dump defaults to setting the
row_security GUC to 'off', in which case you'll get an error if you hit
a table that has RLS enabled and you don't have BYPASSRLS. If you're
not checking for errors from pg_dump, well, there's a lot of ways you
could run into problems.

> My how this has become a can of worms...

I'm not sure it's as bad as all that, but it does require some
thinking..

Thanks,

Stephen

On 12/29/14, 7:22 PM, Stephen Frost wrote:
>>> One other point is that this shouldn't imply any other privileges, imv.
>>> > >I'm specifically thinking of BYPASSRLS- that's independently grantable
>>> > >and therefore should be explicitly set, if it's intended. Things
>>> > >should work 'sanely' with any combination of the two options.
>> >
>> >That does violate POLA, but it's probably worth doing so...
> That really depends on your view of the privilege. I'm inclined to view
> it from the more-tightly-constrained point of view which matches up with
> what I anticipate it actually providing. That doesn't translate into a
> short name very well though, unfortunately.

READ MOST? ;)

>>> > >The read-all vs. ability-to-pg_dump distinction doesn't really exist for
>>> > >role attributes, as I see it (see my comments above). That said, having
>>> > >DUMP or read-all is different from read-*only*, which would probably be
>>> > >good to have independently. I can imagine a use-case for a read-only
>>> > >account which only has read ability for those tables, schemas, etc,
>>> > >explicitly granted to it.
>> >
>> >My specific concern about DUMP vs read all/only is IIRC dump needs to do more extensive locking than regular selects do, which can cause production problems.
> The locks aren't any more extensive than regular selects, it's just that
> the entire pg_dump works inside of one large transaction which lasts a
> good long time and simply that can cause issues with high-rate update
> systems.

Good, so really DUMP and READ ALL are the same.

>>> > >There is one issue that occurs to me, however. We're talking about
>>> > >pg_dump, but what about pg_dumpall? In particular, I don't think the
>>> > >DUMP privilege should provide access to pg_authid, as that would allow
>>> > >the user to bypass the privilege system in some environments by using
>>> > >the hash to log in as a superuser. Now, I don't encourage using
>>> > >password based authentication, especially for superuser accounts, but
>>> > >lots of people do. The idea with these privileges is to allow certain
>>> > >operations to be performed by a non-superuser while preventing trivial
>>> > >access to superuser. Perhaps it's pie-in-the-sky, but my hope is to
>>> > >achieve that.
>> >
>> >Ugh, hadn't thought about that.:(
> I don't think it kills the whole idea, but we do need to work out how to
> address it.

Yeah... it does indicate that we shouldn't call this thing DUMP, but perhaps as long as we put appropriate HINTS in the error paths that pg_dumpall would hit it's OK to call it DUMP. (My specific concern is it's natural to assume that DUMP would include pg_dumpall).

>> >Given the confusion that can exist with the data reading stuff, perhaps BINARY BACKUP or XLOG would be a better term, especially since this will probably have some overlap with streaming replication.
> I don't really like 'xlog' as a permission. BINARY BACKUP is
> interesting but if we're going to go multi-word, I would think we would
> do PITR BACKUP or something FILESYSTEM BACKUP or similar. I'm not
> really a big fan of the two-word options though.

BINARY? Though that's pretty generic. STREAM might be better, even though it's not exactly accurate for a simple backup.

Perhaps this is just a documentation issue, but there's enough caveats floating around here that I'm reluctant to rely on that.

>> >Likewise, if we segregate "DUMP" and BYPASSRLS then I think we need to call DUMP something else. Otherwise, it's a massive foot-gun; you get a "successful" backup only to find out it contains only a small part of the database.
> That's actually already dealt with. pg_dump defaults to setting the
> row_security GUC to 'off', in which case you'll get an error if you hit
> a table that has RLS enabled and you don't have BYPASSRLS. If you're
> not checking for errors from pg_dump, well, there's a lot of ways you
> could run into problems.

This also indicates DUMP is better than something like READ ALL, if we can handle the pg_dumpall case.

Based on all this, my inclination is DUMP with appropriate hints for pg_dumpall, and BINARY.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

On Tue, Dec 30, 2014 at 6:52 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > >There is one issue that occurs to me, however. We're talking about
> > >pg_dump, but what about pg_dumpall? In particular, I don't think the
> > >DUMP privilege should provide access to pg_authid, as that would allow
> > >the user to bypass the privilege system in some environments by using
> > >the hash to log in as a superuser. Now, I don't encourage using
> > >password based authentication, especially for superuser accounts, but
> > >lots of people do. The idea with these privileges is to allow certain
> > >operations to be performed by a non-superuser while preventing trivial
> > >access to superuser. Perhaps it's pie-in-the-sky, but my hope is to
> > >achieve that.
> >
> > Ugh, hadn't thought about that. :(
>
> I don't think it kills the whole idea, but we do need to work out how to
> address it.
>

One way to address this might be to allow this only for superusers,
another could be have a separate privilege (DBA) or a role which is
not a superuser, however can be used to perform such tasks.

> > >>* BACKUP - allows role to perform backup operations (as originally
proposed)
> > >>* LOG - allows role to rotate log files - remains broad enough to
consider
> > >>future log related operations
> > >>* DUMP - allows role to perform pg_dump* backups of whole database
> > >>* MONITOR - allows role to view pg_stat_* details (as originally
proposed)
> > >>* PROCSIGNAL - allows role to signal backend processes (as originally
> > >>proposed)well
> >
> > Given the confusion that can exist with the data reading stuff, perhaps
BINARY BACKUP or XLOG would be a better term, especially since this will
probably have some overlap with streaming replication.
>
> I don't really like 'xlog' as a permission. BINARY BACKUP is
> interesting but if we're going to go multi-word, I would think we would
> do PITR BACKUP or something FILESYSTEM BACKUP or similar. I'm not
> really a big fan of the two-word options though.
>

How about PHYSICAL BACKUP (for basebackup) and LOGICAL BACKUP
for pg_dump?

This is normally the terminology I have seen people using for these
backup's.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

* Amit Kapila (amit(dot)kapila16(at)gmail(dot)com) wrote:
> On Tue, Dec 30, 2014 at 6:52 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > > >There is one issue that occurs to me, however. We're talking about
> > > >pg_dump, but what about pg_dumpall? In particular, I don't think the
> > > >DUMP privilege should provide access to pg_authid, as that would allow
> > > >the user to bypass the privilege system in some environments by using
> > > >the hash to log in as a superuser. Now, I don't encourage using
> > > >password based authentication, especially for superuser accounts, but
> > > >lots of people do. The idea with these privileges is to allow certain
> > > >operations to be performed by a non-superuser while preventing trivial
> > > >access to superuser. Perhaps it's pie-in-the-sky, but my hope is to
> > > >achieve that.
> > >
> > > Ugh, hadn't thought about that. :(
> >
> > I don't think it kills the whole idea, but we do need to work out how to
> > address it.
>
> One way to address this might be to allow this only for superusers,

Huh? The point here is to have a role account which isn't a superuser
who can perform these actions.

> another could be have a separate privilege (DBA) or a role which is
> not a superuser, however can be used to perform such tasks.

I'm pretty sure we've agreed that having a catch-all role attribute like
'DBA' is a bad idea because we'd likely be adding privileges to it later
which would expand the rights of users with that attribute, possibly
beyond what is intended.

> > I don't really like 'xlog' as a permission. BINARY BACKUP is
> > interesting but if we're going to go multi-word, I would think we would
> > do PITR BACKUP or something FILESYSTEM BACKUP or similar. I'm not
> > really a big fan of the two-word options though.
>
> How about PHYSICAL BACKUP (for basebackup) and LOGICAL BACKUP
> for pg_dump?

Again, this makes it look like the read-all right would be specific to
users executing pg_dump, which is incorrect and misleading. "PHYSICAL"
might also imply the ability to do pg_basebackup.

> This is normally the terminology I have seen people using for these
> backup's.

I do like the idea of trying to find a correlation in the documentation
for these rights, though I'm not sure we'll be able to.

Thanks,

Stephen

On Mon, Dec 29, 2014 at 11:01 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

>
> * Adam Brightwell (adam(dot)brightwell(at)crunchydatasolutions(dot)com) wrote:
> > > I'd suggest it's called DUMP if that's what it allows, to keep it
> separate
> > > from the backup parts.
> >
> > Makes sense to me.
>
> I'm fine calling it 'DUMP', but for different reasons.
>
> We have no (verifiable) idea what client program is being used to
> connect and therefore we shouldn't try to tie the name of the client
> program to the permission.
>
> That said, a 'DUMP' privilege which allows the user to dump the contents
> of the entire database is entirely reasonable. We need to be clear in
> the documentation though- such a 'DUMP' privilege is essentially
> granting USAGE on all schemas and table-level SELECT on all tables and

sequences (anything else..?). To be clear, a user with this privilege
> can utilize that access without using pg_dump.
>

Well, it would not give you full USAGE - granted USAGE on a schema, you can
execute functions in there for example (provided permissions). This
privilege would not do that, it would only give you COPY access. (And also
COPY != SELECT in the way that the rule system applies, I think? And this
one could be for COPY only)

But other than that I agree - for *all* these privileges, it needs to be
clearly documented that they are not limited to a specific client side
application, even if their name happens to be similar to one.

One other point is that this shouldn't imply any other privileges, imv.
> I'm specifically thinking of BYPASSRLS- that's independently grantable
> and therefore should be explicitly set, if it's intended. Things
>

I think BYPASSRLS would have to be implicitly granted by the DUMP
privilege. Without that, the DUMP privilege is more or less meaningless
(for anybody who uses RLS - but if they don't use RLS, then having it
include BYPASSRLS makes no difference). Worse than that, you may end up
with a dump that you cannot restore.

Similar concerns would exist for the existing REPLICATION role for example
- that one clearly lets you bypass RLS as well, just not with a SQL
statement.

should work 'sanely' with any combination of the two options.
> Similairly, DUMP shouldn't imply BACKUP or visa-versa. In particular,
> I'd like to see roles which have only the BACKUP privilege be unable to
> directly read any data (modulo things granted to PUBLIC). This would
> allow an unprivileged user to manage backups, kick off ad-hoc ones, etc,
> without being able to actually access any of the data (this would
> require the actual backup system to have a similar control, but that's
> entirely possible with more advanced SANs and enterprise backup
> solutions).
>

So you're saying a privilege that would allow you to do
pg_start_backup()/pg_stop_backup() but *not* actually use pg_basebackup?
That would be "EXCLUSIVEBACKUP" or something like that, to be consistent
with existing terminology though.

> > That seems really bad names, IMHO. Why? Because we use WAL and XLOG
> > > throughout documentation and parameters and code to mean *the same
> thing*.
> > > And here they'd suddenly mean different things. If we need them as
> separate
> > > privileges, I think we need much better names. (And a better
> description -
> > > what is "xlog operations" really?)
> > >
> >
> > Fair enough, ultimately what I was trying to address is the following
> > concern raised by Alvaro:
> >
> > "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."
>
> Note that the BACKUP role attribute was never intended to cover the
> pg_dump use-case. Simply the name of it caused confusion though. I'm
> not sure if adding a DUMP role attribute is sufficient enough to address
> that confusion, but I haven't got a better idea either.
>

We need to separate the logical backups (pg_dump) from the physical ones
(start/stop+filesystem and pg_basebackup). We might also need to separate
the two different ways of doing physical backups.

Personalyl I think using the DUMP name makes that a lot more clear. Maybe
we need to avoid using BACKUP alone as well, to make sure it doesn't go the
other way - using BASEBACKUP and EXCLUSIVEBACKUP for those two different
ones perhaps?

> When indeed, what it meant was to have the following separate (effectively
> > merging #2 and #3):
> >
> > 1) ability to pg_dump
> > 2) ability to start/stop backups *and* ability to execute xlog related
> > functions.
>

We probably also need to define what those "xlog related functions"
actually arse. pg_current_xlog_location() is definitely an xlog related
function, but does it need the privilege? pg_switch_xlog()?
pg_start_backup()? pg_xlog_replay_pause()?

I think just calling them "xlog related functions" is doing us a disservice
there. Definitely once we have an actual documentation to write for it, but
also in this discussion.

That sounds reasonable to me (and is what was initially proposed, though
> I've come around to the thinking that this BACKUP role attribute should
> also allow pg_xlog_replay_pause/resume(), as those can be useful on
> replicas).
>

If it's for replicas, then why are we not using the REPLICATION privilege
which is extremely similar to this?

> Given this clarification:
> >
> > I think #1 could certainly be answered by using DUMP. I have no strong
> > opinion in either direction, though I do think that BACKUP does make the
> > most sense for #2. Previously, Stephen had mentioned a READONLY
> capability
> > that could effectively work for pg_dump, though, Jim's suggestion of
> > keeping 'read-all' separate from 'ability to pg_dump' seems logical. In
> > either case, I certainly wouldn't mind having a wider agreement/consensus
> > on this approach.
>
> The read-all vs. ability-to-pg_dump distinction doesn't really exist for
> role attributes, as I see it (see my comments above). That said, having
> DUMP or read-all is different from read-*only*, which would probably be
> good to have independently. I can imagine a use-case for a read-only
> account which only has read ability for those tables, schemas, etc,
> explicitly granted to it.
>

You mean something that restricts the user to read even *if* write
permissions has been granted on an individual table? Yeah, that would
actually be quite useful, I think - sort of a "reverse privilege".

There is one issue that occurs to me, however. We're talking about
> pg_dump, but what about pg_dumpall? In particular, I don't think the
> DUMP privilege should provide access to pg_authid, as that would allow
> the user to bypass the privilege system in some environments by using
> the hash to log in as a superuser. Now, I don't encourage using
> password based authentication, especially for superuser accounts, but
> lots of people do. The idea with these privileges is to allow certain
> operations to be performed by a non-superuser while preventing trivial
> access to superuser. Perhaps it's pie-in-the-sky, but my hope is to
> achieve that.
>

Well, from an actual security perspective that would make it equivalent to
superuser in the case of anybody using password auth. I'm not sure we cant
to grant that out to DUMP by default - perhaps we need a separate one for
DUMPAUTH or DUMPPASSWORDS.

(We could dump all the users *without* passwords with just the DUMP
privilege)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

* Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> On Mon, Dec 29, 2014 at 11:01 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > That said, a 'DUMP' privilege which allows the user to dump the contents
> > of the entire database is entirely reasonable. We need to be clear in
> > the documentation though- such a 'DUMP' privilege is essentially
> > granting USAGE on all schemas and table-level SELECT on all tables and
>
> sequences (anything else..?). To be clear, a user with this privilege
> > can utilize that access without using pg_dump.
>
> Well, it would not give you full USAGE - granted USAGE on a schema, you can
> execute functions in there for example (provided permissions). This
> privilege would not do that,

The approach I was thinking was to document and implement this as
impliciting granting exactly USAGE and SELECT rights, no more (not
BYPASSRLS) and no less (yes, the role could execute functions). I agree
that doing so would be strictly more than what pg_dump actually requires
but it's also what we actually have support for in our privilege system.

> it would only give you COPY access. (And also
> COPY != SELECT in the way that the rule system applies, I think? And this
> one could be for COPY only)

COPY certainly does equal SELECT rights.. We haven't got an independent
COPY privilege and I don't think it makes sense to invent one. It
sounds like you're suggesting that we add a hack directly into COPY for
this privilege, but my thinking is that the right place to change for
this is in the privilege system and not hack it into individual
commands.. I'm also a bit nervous that we'll end up painting ourselves
into a corner if we hack this to mean exactly what pg_dump needs today.

Lastly, I've been considering other use-cases for this privilege beyond
the pg_dump one (thinking about auditing, for example).

> But other than that I agree - for *all* these privileges, it needs to be
> clearly documented that they are not limited to a specific client side
> application, even if their name happens to be similar to one.

Right.

> One other point is that this shouldn't imply any other privileges, imv.
> > I'm specifically thinking of BYPASSRLS- that's independently grantable
> > and therefore should be explicitly set, if it's intended. Things
>
> I think BYPASSRLS would have to be implicitly granted by the DUMP
> privilege. Without that, the DUMP privilege is more or less meaningless
> (for anybody who uses RLS - but if they don't use RLS, then having it
> include BYPASSRLS makes no difference). Worse than that, you may end up
> with a dump that you cannot restore.

The dump would fail, as I mentioned before. I don't see why BYPASSRLS
has to be implicitly granted by the DUMP privilege and can absolutely
see use-cases for it to *not* be. Those use-cases would require passing
pg_dump the --enable-row-security option, but that's why it's there.

Implementations which don't use RLS are not impacted either way, so we
don't need to consider them. Implementations which *do* use RLS, in my
experience, would certainly understand and expect BYPASSRLS to be
required if they want this role to be able to dump all data out
regardless of the RLS policies. What does implicitly including
BYPASSRLS in this privilege gain us? A slightly less irritated DBA who
forgot to include BYPASSRLS and ended up getting a pg_dump error because
of it. On the other hand, it walls off any possibility of using this
privilege for roles who shouldn't be allowed to bypass RLS or for
pg_dumps to be done across the entire system for specific RLS policies.

> Similar concerns would exist for the existing REPLICATION role for example
> - that one clearly lets you bypass RLS as well, just not with a SQL
> statement.

I'm not sure that I see the point you're making here. Yes, REPLICATION
allows you to do a filesystem copy of the entire database and that
clearly bypasses RLS and *all* of our privilege system. I'm suggesting
that this role attribute work as an implicit grant of privileges we
already have. That strikes me as easy to document and very clear for
users.

> should work 'sanely' with any combination of the two options.
> > Similairly, DUMP shouldn't imply BACKUP or visa-versa. In particular,
> > I'd like to see roles which have only the BACKUP privilege be unable to
> > directly read any data (modulo things granted to PUBLIC). This would
> > allow an unprivileged user to manage backups, kick off ad-hoc ones, etc,
> > without being able to actually access any of the data (this would
> > require the actual backup system to have a similar control, but that's
> > entirely possible with more advanced SANs and enterprise backup
> > solutions).
>
> So you're saying a privilege that would allow you to do
> pg_start_backup()/pg_stop_backup() but *not* actually use pg_basebackup?

Yes.

> That would be "EXCLUSIVEBACKUP" or something like that, to be consistent
> with existing terminology though.

Ok. I agree that working out the specific naming is difficult and would
like to focus on that, but we probably need to agree on the specific
capabilities first. :)

> > > Fair enough, ultimately what I was trying to address is the following
> > > concern raised by Alvaro:
> > >
> > > "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."
> >
> > Note that the BACKUP role attribute was never intended to cover the
> > pg_dump use-case. Simply the name of it caused confusion though. I'm
> > not sure if adding a DUMP role attribute is sufficient enough to address
> > that confusion, but I haven't got a better idea either.
>
> We need to separate the logical backups (pg_dump) from the physical ones
> (start/stop+filesystem and pg_basebackup). We might also need to separate
> the two different ways of doing physical backups.

Right, I view those as three distinct types of privileges (see below).

> Personalyl I think using the DUMP name makes that a lot more clear. Maybe
> we need to avoid using BACKUP alone as well, to make sure it doesn't go the
> other way - using BASEBACKUP and EXCLUSIVEBACKUP for those two different
> ones perhaps?

DUMP - implicitly grants existing rights, to facilitate pg_dump and
perhaps some other use-cases
BASEBACKUP - allows pg_basebackup, which means bypassing all in-DB
privilege systems
EXCLUSIVEBACKUP - just allows pg_start/stop_backup and friends

I'm still not entirely happy with the naming, but I feel like we're
getting there. One thought I had was using ARCHIVE somewhere, but I
kind of like BASEBACKUP meaning what's needed to run pg_basebackup, and,
well, we say 'backup' in the pg_start/stop_backup function names, so
it's kind of hard to avoid that. EXCLUSIVEBACKUP seems really long tho.

> > When indeed, what it meant was to have the following separate (effectively
> > > merging #2 and #3):
> > >
> > > 1) ability to pg_dump
> > > 2) ability to start/stop backups *and* ability to execute xlog related
> > > functions.
> >
>
> We probably also need to define what those "xlog related functions"
> actually arse. pg_current_xlog_location() is definitely an xlog related
> function, but does it need the privilege? pg_switch_xlog()?
> pg_start_backup()? pg_xlog_replay_pause()?

I had defined them when I started the thread:

pg_start_backup
pg_stop_backup
pg_switch_xlog
pg_create_restore_point

Later I added:

pg_xlog_replay_pause
pg_xlog_replay_resume

Though I'd be find if the xlog_replay ones were their own privilege (eg:
REPLAY or something).

> I think just calling them "xlog related functions" is doing us a disservice
> there. Definitely once we have an actual documentation to write for it, but
> also in this discussion.

It wasn't my intent to be ambiguous about it, I just wasn't repeating
the list with each post. The discussion starts here:

20141015052259(dot)GG28859(at)tamriel(dot)snowman(dot)net

> That sounds reasonable to me (and is what was initially proposed, though
> > I've come around to the thinking that this BACKUP role attribute should
> > also allow pg_xlog_replay_pause/resume(), as those can be useful on
> > replicas).
> >
>
> If it's for replicas, then why are we not using the REPLICATION privilege
> which is extremely similar to this?

I don't actually see REPLICATION as being the same.

The point is to have a role which can log into the replica, pause
the stream to be able to run whatever queries they're permitted to
(which might not include all of the data) and then resume it when done.
Perhaps that needs to be independent of the EXCLUSIVEBACKUP role, but
it's definitely different from the REPLICATION privilege.

> > The read-all vs. ability-to-pg_dump distinction doesn't really exist for
> > role attributes, as I see it (see my comments above). That said, having
> > DUMP or read-all is different from read-*only*, which would probably be
> > good to have independently. I can imagine a use-case for a read-only
> > account which only has read ability for those tables, schemas, etc,
> > explicitly granted to it.
>
> You mean something that restricts the user to read even *if* write
> permissions has been granted on an individual table? Yeah, that would
> actually be quite useful, I think - sort of a "reverse privilege".

Yes. My thinking on how to approach this was to forcibly set all of
their transactions to read-only.

> There is one issue that occurs to me, however. We're talking about
> > pg_dump, but what about pg_dumpall? In particular, I don't think the
> > DUMP privilege should provide access to pg_authid, as that would allow
> > the user to bypass the privilege system in some environments by using
> > the hash to log in as a superuser. Now, I don't encourage using
> > password based authentication, especially for superuser accounts, but
> > lots of people do. The idea with these privileges is to allow certain
> > operations to be performed by a non-superuser while preventing trivial
> > access to superuser. Perhaps it's pie-in-the-sky, but my hope is to
> > achieve that.
>
> Well, from an actual security perspective that would make it equivalent to
> superuser in the case of anybody using password auth. I'm not sure we cant
> to grant that out to DUMP by default - perhaps we need a separate one for
> DUMPAUTH or DUMPPASSWORDS.

That makes sense to me- DUMPAUTH or maybe just DUMPALL (to go with
pg_dumpall).

> (We could dump all the users *without* passwords with just the DUMP
> privilege)

Agreed. That's actually something that I think would be *really* nice-
an option to dump the necessary globals for whatever database you're
running pg_dump against. We have existing problems in this area
(database-level GUCs aren't considered database-specific and therefore
aren't picked up by pg_dump, for example..), but being able to also dump
the roles which are used in a given database with pg_dump would be
*really* nice..

Thanks,

Stephen

On Tue, Dec 30, 2014 at 6:35 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Amit Kapila (amit(dot)kapila16(at)gmail(dot)com) wrote:
> > On Tue, Dec 30, 2014 at 6:52 AM, Stephen Frost <sfrost(at)snowman(dot)net>
wrote:
>
> > another could be have a separate privilege (DBA) or a role which is
> > not a superuser, however can be used to perform such tasks.
>
> I'm pretty sure we've agreed that having a catch-all role attribute like
> 'DBA' is a bad idea because we'd likely be adding privileges to it later
> which would expand the rights of users with that attribute, possibly
> beyond what is intended.
>

Yes, that could happen but do you want to say that is the only reason
to consider server level roles (such as DBA) a bad idea. Can't we make
users aware of such things via documentation and then there will be
some onus on user's to give such a privilege with care.

On looking around, it seems many of the databases provides such
roles
https://dbbulletin.wordpress.com/2013/05/29/backup-privileges-needed-to-backup-databases/

In the link, though they are talking about physical (file level) backup,
there is mention about such roles in other databases.

The other way as discussed on this thread is to use something like
DUMPALL (or DUMPFULL) privilege which also sounds to be a good
way, apart from the fact that the same privilege can be used for
non-dump purposes to extract the information from database/cluster.

> > > I don't really like 'xlog' as a permission. BINARY BACKUP is
> > > interesting but if we're going to go multi-word, I would think we
would
> > > do PITR BACKUP or something FILESYSTEM BACKUP or similar. I'm not
> > > really a big fan of the two-word options though.
> >
> > How about PHYSICAL BACKUP (for basebackup) and LOGICAL BACKUP
> > for pg_dump?
>
> Again, this makes it look like the read-all right would be specific to
> users executing pg_dump, which is incorrect and misleading. "PHYSICAL"
> might also imply the ability to do pg_basebackup.
>

That's right, however having unrelated privileges for doing physical
backup via pg_basebackup and pg_start*/pg_stop* method also
doesn't sound to be the best way (can be slightly difficult for
users to correlate after reading docs). Don't you think in this case
we should have some form of hierarchy for privileges (like user
having privilege to do pg_basebackup can also perform the
backup via pg_start*/pg_stop* method or some other way to relate
both forms of physical backup)?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 12/30/2014 04:16 PM, Stephen Frost wrote:
> [snip]
> The approach I was thinking was to document and implement this as
> impliciting granting exactly USAGE and SELECT rights, no more (not
> BYPASSRLS) and no less (yes, the role could execute functions). I agree
> that doing so would be strictly more than what pg_dump actually requires
> but it's also what we actually have support for in our privilege system.

Hmmm.... coupled with your comments below, I'd say some tweaking of the
existing privileges is actually needed if we want to add these new
"capabilities".

BTW, and since this is getting a bit bigger than originally considered:
would it be interesting to support some extension-defined capabilities, too?
(for things can't be easily controlled by the existing USAGE /
SELECT / ... rights, I mean)

>> it would only give you COPY access. (And also
>> COPY != SELECT in the way that the rule system applies, I think? And this
>> one could be for COPY only)
> COPY certainly does equal SELECT rights.. We haven't got an independent
> COPY privilege and I don't think it makes sense to invent one.

FWIW, a COPY(DUMP?) privilege different from SELECT would make sense.
Considering your below comments it would be better that it not imply
BYPASSRLS, though.

> It
> sounds like you're suggesting that we add a hack directly into COPY for
> this privilege, but my thinking is that the right place to change for
> this is in the privilege system and not hack it into individual
> commands.. I'm also a bit nervous that we'll end up painting ourselves
> into a corner if we hack this to mean exactly what pg_dump needs today.
Agreed.

> Lastly, I've been considering other use-cases for this privilege beyond
> the pg_dump one (thinking about auditing, for example).

ISTR there was something upthread on an AUDIT role, right?
This might be the moment to add it....

> [snip]
>> Similar concerns would exist for the existing REPLICATION role for example
>> - that one clearly lets you bypass RLS as well, just not with a SQL
>> statement.
> I'm not sure that I see the point you're making here. Yes, REPLICATION
> allows you to do a filesystem copy of the entire database and that
> clearly bypasses RLS and *all* of our privilege system. I'm suggesting
> that this role attribute work as an implicit grant of privileges we
> already have. That strikes me as easy to document and very clear for
> users.

+1
> [snip]
> So you're saying a privilege that would allow you to do
> pg_start_backup()/pg_stop_backup() but *not* actually use pg_basebackup?
> Yes.
>
>> That would be "EXCLUSIVEBACKUP" or something like that, to be consistent
>> with existing terminology though.
> Ok. I agree that working out the specific naming is difficult and would
> like to focus on that, but we probably need to agree on the specific
> capabilities first. :)

Yes :)
For the record, LOGBACKUP vs PHYSBACKUP was suggested a couple days ago.
I'd favor DUMP(logical) and BACKUP(physical) --- for lack of a better name.
The above reasoning would have pg_basebackup requiring REPLICATION,
which is a logical consequence of the implementation but strikes me as a
bit surprising in the light of these other privileges.

>
> [snip]
>> Personalyl I think using the DUMP name makes that a lot more clear. Maybe
>> we need to avoid using BACKUP alone as well, to make sure it doesn't go the
>> other way - using BASEBACKUP and EXCLUSIVEBACKUP for those two different
>> ones perhaps?
> DUMP - implicitly grants existing rights, to facilitate pg_dump and
> perhaps some other use-cases
> BASEBACKUP - allows pg_basebackup, which means bypassing all in-DB
> privilege systems
> EXCLUSIVEBACKUP - just allows pg_start/stop_backup and friends
>
> I'm still not entirely happy with the naming, but I feel like we're
> getting there. One thought I had was using ARCHIVE somewhere, but I
> kind of like BASEBACKUP meaning what's needed to run pg_basebackup, and,
> well, we say 'backup' in the pg_start/stop_backup function names, so
> it's kind of hard to avoid that. EXCLUSIVEBACKUP seems really long tho.

ARCHIVE, though completely appropriate for the "exclusivebackup" case
above (so this would become DUMP/BASEBACKUP/ARCHIVE +REPLICATION) might
end up causing quite some confusion ... ("what? WAL archiving is NOT
granted by the "archive" privilege, but requires a superuser to turn it
on(via ALTER SYSTEM)?

POLA again....
> I had defined them when I started the thread:
>
> pg_start_backup
> pg_stop_backup
> pg_switch_xlog
> pg_create_restore_point

... for "BACKUP" / EXCLUSIVEBACKUP (or, actually, FSBACKUP/PHYSBACKUP ...)
> Later I added:
>
> pg_xlog_replay_pause
> pg_xlog_replay_resume
>
> Though I'd be find if the xlog_replay ones were their own privilege (eg:
> REPLAY or something).
+1
>> I think just calling them "xlog related functions" is doing us a disservice
>> there. Definitely once we have an actual documentation to write for it, but
>> also in this discussion.
> [snip]
>> If it's for replicas, then why are we not using the REPLICATION privilege
>> which is extremely similar to this?
> I don't actually see REPLICATION as being the same.

REPLICATION (ability to replicate) vs REPLICAOPERATOR (ability to
control *the replica* or the R/O behaviour of the server, for that
matter...)

> The point is to have a role which can log into the replica, pause
> the stream to be able to run whatever queries they're permitted to
> (which might not include all of the data) and then resume it when done.
> Perhaps that needs to be independent of the EXCLUSIVEBACKUP role, but
> it's definitely different from the REPLICATION privilege.

Looks like it is.
>>> The read-all vs. ability-to-pg_dump distinction doesn't really exist for
>>> role attributes, as I see it (see my comments above). That said, having
>>> DUMP or read-all is different from read-*only*, which would probably be
>>> good to have independently. I can imagine a use-case for a read-only
>>> account which only has read ability for those tables, schemas, etc,
>>> explicitly granted to it.
>> You mean something that restricts the user to read even *if* write
>> permissions has been granted on an individual table? Yeah, that would
>> actually be quite useful, I think - sort of a "reverse privilege".
> Yes. My thinking on how to approach this was to forcibly set all of
> their transactions to read-only.

So "READONLY" ?

>> There is one issue that occurs to me, however. We're talking about
>>> pg_dump, but what about pg_dumpall? In particular, I don't think the
>>> DUMP privilege should provide access to pg_authid, as that would allow
>>> the user to bypass the privilege system in some environments by using
>>> the hash to log in as a superuser. Now, I don't encourage using
>>> password based authentication, especially for superuser accounts, but
>>> lots of people do. The idea with these privileges is to allow certain
>>> operations to be performed by a non-superuser while preventing trivial
>>> access to superuser. Perhaps it's pie-in-the-sky, but my hope is to
>>> achieve that.
>> Well, from an actual security perspective that would make it equivalent to
>> superuser in the case of anybody using password auth. I'm not sure we cant
>> to grant that out to DUMP by default - perhaps we need a separate one for
>> DUMPAUTH or DUMPPASSWORDS.
> That makes sense to me- DUMPAUTH or maybe just DUMPALL (to go with
> pg_dumpall).

+1
>> (We could dump all the users *without* passwords with just the DUMP
>> privilege)
> Agreed. That's actually something that I think would be *really* nice-
> an option to dump the necessary globals for whatever database you're
> running pg_dump against. We have existing problems in this area
> (database-level GUCs aren't considered database-specific and therefore
> aren't picked up by pg_dump, for example..), but being able to also dump
> the roles which are used in a given database with pg_dump would be
> *really* nice..

Ok, so this would imply modifying pg_dump to include database-level
configs. I would heartily welcome this.

So, it seems to me that we are actually evolving towards a set of
"low-level" "capabilities" and some "high-level" (use case-focused)
"privileges".
I am hereby volunteering to compile this thread into some wiki page. I'm
thinking "Privileges" as the title for starters. Suggestions?

Thanks,

/ J.L.

On Tue, Dec 30, 2014 at 4:16 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> * Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> > On Mon, Dec 29, 2014 at 11:01 PM, Stephen Frost <sfrost(at)snowman(dot)net>
> wrote:
> > > That said, a 'DUMP' privilege which allows the user to dump the
> contents
> > > of the entire database is entirely reasonable. We need to be clear in
> > > the documentation though- such a 'DUMP' privilege is essentially
> > > granting USAGE on all schemas and table-level SELECT on all tables and
> >
> > sequences (anything else..?). To be clear, a user with this privilege
> > > can utilize that access without using pg_dump.
> >
> > Well, it would not give you full USAGE - granted USAGE on a schema, you
> can
> > execute functions in there for example (provided permissions). This
> > privilege would not do that,
>
> The approach I was thinking was to document and implement this as
> impliciting granting exactly USAGE and SELECT rights, no more (not
> BYPASSRLS) and no less (yes, the role could execute functions). I agree
>

If the role could execute functions, it's *not* "exactly USAGE and SELECT
rights", it's now "USAGE and SELECT and EXECUTE" rights... Just to be
nitpicking, but see below.

> that doing so would be strictly more than what pg_dump actually requires
> but it's also what we actually have support for in our privilege system.
>

Yeah, but would it also be what people would actually *want*?

I think having it do exactly what pg_dump needs, and not things like
execute functions etc, would be the thing people want for a 'DUMP'
privilege.

We might *also* want a USAGEANDSELECTANDEXECUTEANDWHATNOTBUTNOBYPASSURL
(crap, it has to fit within NAMEDATALEN?) privilege, but I think that's a
different thing.

> it would only give you COPY access. (And also
> > COPY != SELECT in the way that the rule system applies, I think? And this
> > one could be for COPY only)
>
> COPY certainly does equal SELECT rights.. We haven't got an independent
> COPY privilege and I don't think it makes sense to invent one. It
>

We don't, but I'm saying it might make sense to implement this. Not as a
regular privilige, but as a role attribute. I'm not sure it's the right
thing, but it might actually be interesting to entertain.

> sounds like you're suggesting that we add a hack directly into COPY for
> this privilege, but my thinking is that the right place to change for
> this is in the privilege system and not hack it into individual
> commands.. I'm also a bit nervous that we'll end up painting ourselves
> into a corner if we hack this to mean exactly what pg_dump needs today.
>

One point would be that if we define it as "exactly what pg_dump needs",
that definition can change in a future major version.

> One other point is that this shouldn't imply any other privileges, imv.
> > > I'm specifically thinking of BYPASSRLS- that's independently grantable
> > > and therefore should be explicitly set, if it's intended. Things
> >
> > I think BYPASSRLS would have to be implicitly granted by the DUMP
> > privilege. Without that, the DUMP privilege is more or less meaningless
> > (for anybody who uses RLS - but if they don't use RLS, then having it
> > include BYPASSRLS makes no difference). Worse than that, you may end up
> > with a dump that you cannot restore.
>
> The dump would fail, as I mentioned before. I don't see why BYPASSRLS
> has to be implicitly granted by the DUMP privilege and can absolutely
> see use-cases for it to *not* be. Those use-cases would require passing
> pg_dump the --enable-row-security option, but that's why it's there.
>

So you're basically saying that if RLS is in use anywhere, this priviliege
alone would be useless, correct? And you would get a hard failure at *dump
time*, not at reload time? That would at least make it acceptable, as you
would know it's wrong. and we could make the error messages shown
specifically hint that you need to grant both privileges to make it useful.

We could/should also throw a WARNING if DUMP Is granted to a role without
BYPASSRLS in case row level security is enabled in the system, I think. But
that's more of an implementation detail.

Implementations which don't use RLS are not impacted either way, so we
> don't need to consider them. Implementations which *do* use RLS, in my
> experience, would certainly understand and expect BYPASSRLS to be
> required if they want this role to be able to dump all data out
> regardless of the RLS policies. What does implicitly including
> BYPASSRLS in this privilege gain us? A slightly less irritated DBA who
> forgot to include BYPASSRLS and ended up getting a pg_dump error because
> of it. On the other hand, it walls off any possibility of using this
> privilege for roles who shouldn't be allowed to bypass RLS or for
> pg_dumps to be done across the entire system for specific RLS policies.
>

Yeah, I think that's acceptable as long as we get the error at dump, and
not at reload (when it's too late to fix it).

> Similar concerns would exist for the existing REPLICATION role for example
> > - that one clearly lets you bypass RLS as well, just not with a SQL
> > statement.
>
> I'm not sure that I see the point you're making here. Yes, REPLICATION
> allows you to do a filesystem copy of the entire database and that
> clearly bypasses RLS and *all* of our privilege system. I'm suggesting
> that this role attribute work as an implicit grant of privileges we
> already have. That strikes me as easy to document and very clear for
> users.
>

It does - though documenting that it implicitly gives you a different
privilege as well is also easy :)

But it does potentially limit us to what we can actually do with the
priviliges. One reason to use them is exactly because we *cannot* express
this with our regular permissions (such as BYPASSRLS or REPLICATION).

For regular permissions, we could just pre-populate the system with
predefined roles and use regular GRANTs to those roles, instead of relying
on role attributes, which might in that case make it even more clear?

> should work 'sanely' with any combination of the two options.
> > > Similairly, DUMP shouldn't imply BACKUP or visa-versa. In particular,
> > > I'd like to see roles which have only the BACKUP privilege be unable to
> > > directly read any data (modulo things granted to PUBLIC). This would
> > > allow an unprivileged user to manage backups, kick off ad-hoc ones,
> etc,
> > > without being able to actually access any of the data (this would
> > > require the actual backup system to have a similar control, but that's
> > > entirely possible with more advanced SANs and enterprise backup
> > > solutions).
> >
> > So you're saying a privilege that would allow you to do
> > pg_start_backup()/pg_stop_backup() but *not* actually use pg_basebackup?
>
> Yes.
>

What's really the usecase for that? I'd say that pg_start/stop backup is a
superset and a higher privilige than using pg_basebackup, since you can for
example destroy other peoples backups using it (by running pg_stop_backup
while they are running).

> That would be "EXCLUSIVEBACKUP" or something like that, to be consistent
> > with existing terminology though.
>
> Ok. I agree that working out the specific naming is difficult and would
> like to focus on that, but we probably need to agree on the specific
> capabilities first. :)
>

:)

> > > Fair enough, ultimately what I was trying to address is the following
> > > > concern raised by Alvaro:
> > > >
> > > > "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."
> > >
> > > Note that the BACKUP role attribute was never intended to cover the
> > > pg_dump use-case. Simply the name of it caused confusion though. I'm
> > > not sure if adding a DUMP role attribute is sufficient enough to
> address
> > > that confusion, but I haven't got a better idea either.
> >
> > We need to separate the logical backups (pg_dump) from the physical ones
> > (start/stop+filesystem and pg_basebackup). We might also need to separate
> > the two different ways of doing physical backups.
>
> Right, I view those as three distinct types of privileges (see below).

> Personalyl I think using the DUMP name makes that a lot more clear. Maybe
> > we need to avoid using BACKUP alone as well, to make sure it doesn't go
> the
> > other way - using BASEBACKUP and EXCLUSIVEBACKUP for those two different
> > ones perhaps?
>
> DUMP - implicitly grants existing rights, to facilitate pg_dump and
> perhaps some other use-cases
> BASEBACKUP - allows pg_basebackup, which means bypassing all in-DB
> privilege systems
>

That's equivalent of REPLICATION, yes?

> EXCLUSIVEBACKUP - just allows pg_start/stop_backup and friends
>

I'd say there is no "just" there really - that's a higher level privilege,
but I see your point :)

I'm still not entirely happy with the naming, but I feel like we're
> getting there. One thought I had was using ARCHIVE somewhere, but I
> kind of like BASEBACKUP meaning what's needed to run pg_basebackup, and,
> well, we say 'backup' in the pg_start/stop_backup function names, so
> it's kind of hard to avoid that. EXCLUSIVEBACKUP seems really long tho.
>

I don't like ARCHIVE in that it sounds like something to do with log
archiving. If anything, that would affect pg_receivexlog, but that's
definitely REPLICATION. I'd rather keep that one for a future time where we
might let users actually do something about log archiving directly, which
would then use the name.

The reason I like EXCLUSIVEBACKUP is that it's a term we already use. It's
only 4 chars longer than REPLICATION for example - does that really matter?
As long as we don't store the name, it should have no effect at all, and
even if we do, I'm not sure it matters. If you want it shorter, we can have
EXCLBACKUP :)

> > When indeed, what it meant was to have the following separate
> (effectively
> > > > merging #2 and #3):
> > > >
> > > > 1) ability to pg_dump
> > > > 2) ability to start/stop backups *and* ability to execute xlog
> related
> > > > functions.
> > >
> >
> > We probably also need to define what those "xlog related functions"
> > actually arse. pg_current_xlog_location() is definitely an xlog related
> > function, but does it need the privilege? pg_switch_xlog()?
> > pg_start_backup()? pg_xlog_replay_pause()?
>
> I had defined them when I started the thread:
>

Apologies - I had missed that part.

pg_start_backup
> pg_stop_backup
> pg_switch_xlog
> pg_create_restore_point
>

Ok. That makes sense.

Later I added:
>
> pg_xlog_replay_pause
> pg_xlog_replay_resume
>
> Though I'd be find if the xlog_replay ones were their own privilege (eg:
> REPLAY or something).
>

I think it makes more sense to have those replay functions to be a separate
privilege, yes. They have nothing to actually do with taking the backups -
they are for restoring them. And to restore the backups, you clearly
already have superuser level privileges on the system outside the db (at
least as long as we only allow full cluster restores).

> That sounds reasonable to me (and is what was initially proposed, though
> > > I've come around to the thinking that this BACKUP role attribute should
> > > also allow pg_xlog_replay_pause/resume(), as those can be useful on
> > > replicas).
> > >
> >
> > If it's for replicas, then why are we not using the REPLICATION privilege
> > which is extremely similar to this?
>
> I don't actually see REPLICATION as being the same.
>
> The point is to have a role which can log into the replica, pause
> the stream to be able to run whatever queries they're permitted to
> (which might not include all of the data) and then resume it when done.
> Perhaps that needs to be independent of the EXCLUSIVEBACKUP role, but
> it's definitely different from the REPLICATION privilege.
>

Agreed per above, now that I realize what we mean. I do think it needs to
be at least as independent from EXCLUSIVEBACKUP as it does froM REPLICATION
though.

> > The read-all vs. ability-to-pg_dump distinction doesn't really exist for
> > > role attributes, as I see it (see my comments above). That said,
> having
> > > DUMP or read-all is different from read-*only*, which would probably be
> > > good to have independently. I can imagine a use-case for a read-only
> > > account which only has read ability for those tables, schemas, etc,
> > > explicitly granted to it.
> >
> > You mean something that restricts the user to read even *if* write
> > permissions has been granted on an individual table? Yeah, that would
> > actually be quite useful, I think - sort of a "reverse privilege".
>
> Yes. My thinking on how to approach this was to forcibly set all of
> their transactions to read-only.
>

Agreed that this would be very useful.

> There is one issue that occurs to me, however. We're talking about
> > > pg_dump, but what about pg_dumpall? In particular, I don't think the
> > > DUMP privilege should provide access to pg_authid, as that would allow
> > > the user to bypass the privilege system in some environments by using
> > > the hash to log in as a superuser. Now, I don't encourage using
> > > password based authentication, especially for superuser accounts, but
> > > lots of people do. The idea with these privileges is to allow certain
> > > operations to be performed by a non-superuser while preventing trivial
> > > access to superuser. Perhaps it's pie-in-the-sky, but my hope is to
> > > achieve that.
> >
> > Well, from an actual security perspective that would make it equivalent
> to
> > superuser in the case of anybody using password auth. I'm not sure we
> cant
> > to grant that out to DUMP by default - perhaps we need a separate one for
> > DUMPAUTH or DUMPPASSWORDS.
>
> That makes sense to me- DUMPAUTH or maybe just DUMPALL (to go with
> pg_dumpall).
>

I'd prefer DUMPAUTH specifically because it makes it glaringly obvious that
you're going to be dumping authentication data.

> (We could dump all the users *without* passwords with just the DUMP
> > privilege)
>
> Agreed. That's actually something that I think would be *really* nice-
> an option to dump the necessary globals for whatever database you're
> running pg_dump against. We have existing problems in this area
> (database-level GUCs aren't considered database-specific and therefore
> aren't picked up by pg_dump, for example..), but being able to also dump
> the roles which are used in a given database with pg_dump would be
> *really* nice..
>

Yes. Shouldn't be *that* hard, completely independent of this privilege
work. Famous last words, I'm sure...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

* Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> On Tue, Dec 30, 2014 at 4:16 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > The approach I was thinking was to document and implement this as
> > impliciting granting exactly USAGE and SELECT rights, no more (not
> > BYPASSRLS) and no less (yes, the role could execute functions). I agree
>
> If the role could execute functions, it's *not* "exactly USAGE and SELECT
> rights", it's now "USAGE and SELECT and EXECUTE" rights... Just to be
> nitpicking, but see below.

We seem to be coming at it from two different directions, so I'll try to
clarify. What I'm suggesting is that this role attribute be strictly
*additive*. Any other privileges granted to the role with this role
attribute would still be applicable, including grants made to PUBLIC.

This role attribute would *not* include EXECUTE rights, by that logic.
However, if PUBLIC was granted EXECUTE rights for the function, then
this role could execute that function.

What it sounds like is you're imagining this role attribute to mean the
role has *only* USAGE and SELECT (or COPY or whatever) rights across the
board and that any grants done explicitly to this role or to public
wouldn't be respected. In my view, that moves this away from a role
*attribute* to being a pre-defined *role*, as such a role would not be
usable for anything else.

> > that doing so would be strictly more than what pg_dump actually requires
> > but it's also what we actually have support for in our privilege system.
>
> Yeah, but would it also be what people would actually *want*?

I can certainly see reasons why you'd want such a role to be able to
execute functions- in particular, consider xlog_pause anx xlog_resume.
If this role can't execute those functions then they probably can't
successfully run pg_dump against a replica.

> I think having it do exactly what pg_dump needs, and not things like
> execute functions etc, would be the thing people want for a 'DUMP'
> privilege.

What if we want pg_dump in 9.6 to have an option to execute xlog_pause
and xlog_resume for you? You wouldn't be able to run that against a 9.5
database (or at least, that option wouldn't work).

> We might *also* want a USAGEANDSELECTANDEXECUTEANDWHATNOTBUTNOBYPASSURL
> (crap, it has to fit within NAMEDATALEN?) privilege, but I think that's a
> different thing.

Our privilege system doesn't currently allow for negative grants (deny
user X the ability to run functions, even if EXECUTE is granted to
PUBLIC). I'm not against that idea, but I don't see it as the same as
this.

> > it would only give you COPY access. (And also
> > > COPY != SELECT in the way that the rule system applies, I think? And this
> > > one could be for COPY only)
> >
> > COPY certainly does equal SELECT rights.. We haven't got an independent
> > COPY privilege and I don't think it makes sense to invent one. It
>
> We don't, but I'm saying it might make sense to implement this. Not as a
> regular privilige, but as a role attribute. I'm not sure it's the right
> thing, but it might actually be interesting to entertain.

We've discussed having a role attribute for COPY-from-filesystem, but
pg_dump doesn't use that ever, it only uses COPY TO STDOUT. I'm not
a fan of making a COPY_TO_STDOUT-vs-SELECT distinction just for this..

> > sounds like you're suggesting that we add a hack directly into COPY for
> > this privilege, but my thinking is that the right place to change for
> > this is in the privilege system and not hack it into individual
> > commands.. I'm also a bit nervous that we'll end up painting ourselves
> > into a corner if we hack this to mean exactly what pg_dump needs today.
>
> One point would be that if we define it as "exactly what pg_dump needs",
> that definition can change in a future major version.

Sure, but that then gets a bit ugly because we encourage running the
latest version of pg_dump against the prior-major-version.

> > > I think BYPASSRLS would have to be implicitly granted by the DUMP
> > > privilege. Without that, the DUMP privilege is more or less meaningless
> > > (for anybody who uses RLS - but if they don't use RLS, then having it
> > > include BYPASSRLS makes no difference). Worse than that, you may end up
> > > with a dump that you cannot restore.
> >
> > The dump would fail, as I mentioned before. I don't see why BYPASSRLS
> > has to be implicitly granted by the DUMP privilege and can absolutely
> > see use-cases for it to *not* be. Those use-cases would require passing
> > pg_dump the --enable-row-security option, but that's why it's there.
>
> So you're basically saying that if RLS is in use anywhere, this priviliege
> alone would be useless, correct?

No, it would still be *extremely* useful. We have an option to pg_dump
to say "please dump with RLS enabled". What that means is that you'd be
able to dump the entire database for all data you're allowed to see
through RLS policies. How is that useless? I certainly think it'd be
very handy and a good way to get *segregated* dumps according to policy.

> And you would get a hard failure at *dump
> time*, not at reload time?

If you attempt to pg_dump a relation which has RLS enabled, and you
don't enable RLS with pg_dump, then you'll get a failure at dump time,
yes. That's far better than a reload-time failure.

> That would at least make it acceptable, as you
> would know it's wrong. and we could make the error messages shown
> specifically hint that you need to grant both privileges to make it useful.

Agreed, having such a hint makes sense.

> We could/should also throw a WARNING if DUMP Is granted to a role without
> BYPASSRLS in case row level security is enabled in the system, I think. But
> that's more of an implementation detail.

That's a bit ugly and RLS could be added to a relation after the DUMP
privilege is granted.

What I think this part of the discussion is getting at is that there's a
lot of people who view pg_dump as explicitly for "dump the ENTIRE
database". While that's one use-case it is certainly not the only one.
I find "pg_dump schema X" to be very common and often find that pg_dump
against an entire database isn't practical because of the size of the
database.

> Implementations which don't use RLS are not impacted either way, so we
> > don't need to consider them. Implementations which *do* use RLS, in my
> > experience, would certainly understand and expect BYPASSRLS to be
> > required if they want this role to be able to dump all data out
> > regardless of the RLS policies. What does implicitly including
> > BYPASSRLS in this privilege gain us? A slightly less irritated DBA who
> > forgot to include BYPASSRLS and ended up getting a pg_dump error because
> > of it. On the other hand, it walls off any possibility of using this
> > privilege for roles who shouldn't be allowed to bypass RLS or for
> > pg_dumps to be done across the entire system for specific RLS policies.
> >
>
> Yeah, I think that's acceptable as long as we get the error at dump, and
> not at reload (when it's too late to fix it).

Right, that's the same decision we came to in the general RLS
discussion.

> > Similar concerns would exist for the existing REPLICATION role for example
> > > - that one clearly lets you bypass RLS as well, just not with a SQL
> > > statement.
> >
> > I'm not sure that I see the point you're making here. Yes, REPLICATION
> > allows you to do a filesystem copy of the entire database and that
> > clearly bypasses RLS and *all* of our privilege system. I'm suggesting
> > that this role attribute work as an implicit grant of privileges we
> > already have. That strikes me as easy to document and very clear for
> > users.
>
> It does - though documenting that it implicitly gives you a different
> privilege as well is also easy :)
>
> But it does potentially limit us to what we can actually do with the
> priviliges. One reason to use them is exactly because we *cannot* express
> this with our regular permissions (such as BYPASSRLS or REPLICATION).

Ok, I see the point you're making that we could make this into a
capability which isn't something which can be expressed through our
existing GRANT system. That strikes me as a solution trying to find a
problem though. There's no need to invent such an oddity for this
particular use-case, I don't think.

> For regular permissions, we could just pre-populate the system with
> predefined roles and use regular GRANTs to those roles, instead of relying
> on role attributes, which might in that case make it even more clear?

The reason for this approach is to address exactly the nightmare that is
trying to maintain those regular permissions across all the objects in
the system. Today, people simply give the role trying to do the pg_dump
superuser, which is the best option we have. Saying 'grant SELECT on
all the tables and USAGE on all the schemas' isn't suggested because
it's a huge pain. This role attribute provides a middle ground where
the pg_dump'ing role isn't a superuser, but you don't have to ensure
usage and select are granted to it for every relation.

> > should work 'sanely' with any combination of the two options.
> > > > Similairly, DUMP shouldn't imply BACKUP or visa-versa. In particular,
> > > > I'd like to see roles which have only the BACKUP privilege be unable to
> > > > directly read any data (modulo things granted to PUBLIC). This would
> > > > allow an unprivileged user to manage backups, kick off ad-hoc ones,
> > etc,
> > > > without being able to actually access any of the data (this would
> > > > require the actual backup system to have a similar control, but that's
> > > > entirely possible with more advanced SANs and enterprise backup
> > > > solutions).
> > >
> > > So you're saying a privilege that would allow you to do
> > > pg_start_backup()/pg_stop_backup() but *not* actually use pg_basebackup?
> >
> > Yes.
>
> What's really the usecase for that? I'd say that pg_start/stop backup is a
> superset and a higher privilige than using pg_basebackup, since you can for
> example destroy other peoples backups using it (by running pg_stop_backup
> while they are running).

One use-case is to allow unprivileged users to run adhoc backups.
Another is simply that you don't want the cronjob that runs the backup
to be able to read any of the data directly (why would you?). I agree
that the individuals who have this capability would still need to
coordinate or at lesat not step on each other or they could end up with
bad backups.

Another way to address that would be to require that the role calling
stop_backup be a member of the role which called start_backup (that also
address the superuser case, as superusers are considered members of all
roles). That seems like a pretty reasonable sanity check requirement.

> > Personalyl I think using the DUMP name makes that a lot more clear. Maybe
> > > we need to avoid using BACKUP alone as well, to make sure it doesn't go
> > the
> > > other way - using BASEBACKUP and EXCLUSIVEBACKUP for those two different
> > > ones perhaps?
> >
> > DUMP - implicitly grants existing rights, to facilitate pg_dump and
> > perhaps some other use-cases
> > BASEBACKUP - allows pg_basebackup, which means bypassing all in-DB
> > privilege systems
>
> That's equivalent of REPLICATION, yes?

Ah, yeah, seems like it would be. Got ahead of myself. :)

> > EXCLUSIVEBACKUP - just allows pg_start/stop_backup and friends
>
> I'd say there is no "just" there really - that's a higher level privilege,
> but I see your point :)

Well, see above, but ok.

> I'm still not entirely happy with the naming, but I feel like we're
> > getting there. One thought I had was using ARCHIVE somewhere, but I
> > kind of like BASEBACKUP meaning what's needed to run pg_basebackup, and,
> > well, we say 'backup' in the pg_start/stop_backup function names, so
> > it's kind of hard to avoid that. EXCLUSIVEBACKUP seems really long tho.
> >
>
> I don't like ARCHIVE in that it sounds like something to do with log
> archiving. If anything, that would affect pg_receivexlog, but that's
> definitely REPLICATION. I'd rather keep that one for a future time where we
> might let users actually do something about log archiving directly, which
> would then use the name.

Ok.

> The reason I like EXCLUSIVEBACKUP is that it's a term we already use. It's
> only 4 chars longer than REPLICATION for example - does that really matter?

Eh, I guess not. :)

> As long as we don't store the name, it should have no effect at all, and
> even if we do, I'm not sure it matters. If you want it shorter, we can have
> EXCLBACKUP :)

Eh.

> > I had defined them when I started the thread:
>
> Apologies - I had missed that part.

No prob.

> pg_start_backup
> > pg_stop_backup
> > pg_switch_xlog
> > pg_create_restore_point
> >
>
> Ok. That makes sense.
>
>
> Later I added:
> >
> > pg_xlog_replay_pause
> > pg_xlog_replay_resume
> >
> > Though I'd be find if the xlog_replay ones were their own privilege (eg:
> > REPLAY or something).
> >
>
> I think it makes more sense to have those replay functions to be a separate
> privilege, yes. They have nothing to actually do with taking the backups -
> they are for restoring them. And to restore the backups, you clearly
> already have superuser level privileges on the system outside the db (at
> least as long as we only allow full cluster restores).

Not quite- remember that reply_pause/resume can be done on a replica, so
they are useful to be able to grant independent from superuser. Perhaps
we call such a role attribute XLOGREPLAY ?

> > I don't actually see REPLICATION as being the same.
> >
> > The point is to have a role which can log into the replica, pause
> > the stream to be able to run whatever queries they're permitted to
> > (which might not include all of the data) and then resume it when done.
> > Perhaps that needs to be independent of the EXCLUSIVEBACKUP role, but
> > it's definitely different from the REPLICATION privilege.
> >
>
> Agreed per above, now that I realize what we mean. I do think it needs to
> be at least as independent from EXCLUSIVEBACKUP as it does froM REPLICATION
> though.

Ok.

> > > You mean something that restricts the user to read even *if* write
> > > permissions has been granted on an individual table? Yeah, that would
> > > actually be quite useful, I think - sort of a "reverse privilege".
> >
> > Yes. My thinking on how to approach this was to forcibly set all of
> > their transactions to read-only.
>
> Agreed that this would be very useful.

Glad we agree.. Any thoughts on implementation? :)

> > > Well, from an actual security perspective that would make it equivalent
> > to
> > > superuser in the case of anybody using password auth. I'm not sure we
> > cant
> > > to grant that out to DUMP by default - perhaps we need a separate one for
> > > DUMPAUTH or DUMPPASSWORDS.
> >
> > That makes sense to me- DUMPAUTH or maybe just DUMPALL (to go with
> > pg_dumpall).
> >
>
> I'd prefer DUMPAUTH specifically because it makes it glaringly obvious that
> you're going to be dumping authentication data.

Ok.

> > (We could dump all the users *without* passwords with just the DUMP
> > > privilege)
> >
> > Agreed. That's actually something that I think would be *really* nice-
> > an option to dump the necessary globals for whatever database you're
> > running pg_dump against. We have existing problems in this area
> > (database-level GUCs aren't considered database-specific and therefore
> > aren't picked up by pg_dump, for example..), but being able to also dump
> > the roles which are used in a given database with pg_dump would be
> > *really* nice..
> >
>
> Yes. Shouldn't be *that* hard, completely independent of this privilege
> work. Famous last words, I'm sure...

Indeed.

Thanks!

Stephen

Amit,

* Amit Kapila (amit(dot)kapila16(at)gmail(dot)com) wrote:
> On Tue, Dec 30, 2014 at 6:35 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > I'm pretty sure we've agreed that having a catch-all role attribute like
> > 'DBA' is a bad idea because we'd likely be adding privileges to it later
> > which would expand the rights of users with that attribute, possibly
> > beyond what is intended.
>
> Yes, that could happen but do you want to say that is the only reason
> to consider server level roles (such as DBA) a bad idea.

No, the other reason is that having more granular permissions is better
than catch-all's. 'superuser' is that catch-all today.

> Can't we make
> users aware of such things via documentation and then there will be
> some onus on user's to give such a privilege with care.

Perhaps, but if we're going to go in that direction, I'd rather have a
hierarchy which is built upon more granular options rather than assuming
we know exactly what the 'DBA' role should have for every environment.

> On looking around, it seems many of the databases provides such
> roles
> https://dbbulletin.wordpress.com/2013/05/29/backup-privileges-needed-to-backup-databases/
>
> In the link, though they are talking about physical (file level) backup,
> there is mention about such roles in other databases.

Most of this discussion is about non-physical-level-backups. We have
the REPLICATION role attribute for physical-level-backups and I don't
think anyone wants to get rid of that in favor of pulling it into some
'DBA' role attribute.

> The other way as discussed on this thread is to use something like
> DUMPALL (or DUMPFULL) privilege which also sounds to be a good
> way, apart from the fact that the same privilege can be used for
> non-dump purposes to extract the information from database/cluster.

I think we're probably going to go with DUMPAUTH as the specific role
attribute privilege, to make it clear that it includes authentication
information. That's really the only distinction between DUMP (or
whatever) and a privilege to support pg_dumpall.

This does make me think that we need to consider if this role attribute
implicitly grants CONNECT to all databases also.

> > > How about PHYSICAL BACKUP (for basebackup) and LOGICAL BACKUP
> > > for pg_dump?
> >
> > Again, this makes it look like the read-all right would be specific to
> > users executing pg_dump, which is incorrect and misleading. "PHYSICAL"
> > might also imply the ability to do pg_basebackup.
>
> That's right, however having unrelated privileges for doing physical
> backup via pg_basebackup and pg_start*/pg_stop* method also
> doesn't sound to be the best way (can be slightly difficult for
> users to correlate after reading docs).

We should definitely segregate the privilege to run start/stop backup
and run pg_basebackup. One allows you to read the database files off
the filesystem more-or-less directly and the other doesn't. That's a
big difference.

> Don't you think in this case
> we should have some form of hierarchy for privileges (like user
> having privilege to do pg_basebackup can also perform the
> backup via pg_start*/pg_stop* method or some other way to relate
> both forms of physical backup)?

If we had a specific privilege for pg_basebackup then I would agree that
it would allow call pg_start/stop_backup. The same is not true in
reverse though.

Thanks,

Stephen

On Wed, Dec 31, 2014 at 3:08 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> * Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> > On Tue, Dec 30, 2014 at 4:16 PM, Stephen Frost <sfrost(at)snowman(dot)net>
> wrote:
> > > The approach I was thinking was to document and implement this as
> > > impliciting granting exactly USAGE and SELECT rights, no more (not
> > > BYPASSRLS) and no less (yes, the role could execute functions). I
> agree
> >
> > If the role could execute functions, it's *not* "exactly USAGE and SELECT
> > rights", it's now "USAGE and SELECT and EXECUTE" rights... Just to be
> > nitpicking, but see below.
>
> We seem to be coming at it from two different directions, so I'll try to
> clarify. What I'm suggesting is that this role attribute be strictly
> *additive*. Any other privileges granted to the role with this role
> attribute would still be applicable, including grants made to PUBLIC.
>
> This role attribute would *not* include EXECUTE rights, by that logic.
> However, if PUBLIC was granted EXECUTE rights for the function, then
> this role could execute that function.
>

Ah, ok, mistunderstood that part.

What it sounds like is you're imagining this role attribute to mean the
> role has *only* USAGE and SELECT (or COPY or whatever) rights across the
> board and that any grants done explicitly to this role or to public
> wouldn't be respected. In my view, that moves this away from a role
> *attribute* to being a pre-defined *role*, as such a role would not be
> usable for anything else.
>

No, i meant additive as well. I misread you.

> > that doing so would be strictly more than what pg_dump actually requires
> > > but it's also what we actually have support for in our privilege
> system.
> >
> > Yeah, but would it also be what people would actually *want*?
>
> I can certainly see reasons why you'd want such a role to be able to
> execute functions- in particular, consider xlog_pause anx xlog_resume.

If this role can't execute those functions then they probably can't
> successfully run pg_dump against a replica.
>

uh, pg_dump doesn't run those commands :P I don't see why that's a
requirement at all. And you can still always grant those things on top of
whatever the privilege gives you.

> I think having it do exactly what pg_dump needs, and not things like
> > execute functions etc, would be the thing people want for a 'DUMP'
> > privilege.
>
> What if we want pg_dump in 9.6 to have an option to execute xlog_pause
> and xlog_resume for you? You wouldn't be able to run that against a 9.5
> database (or at least, that option wouldn't work).
>

It would if you added an explicit grant for it, which would have to be
documented.

I don't see how that's different if the definition is "allows select on all
tables". That wouldn't automatically give it the ability to do xlog_pause
in an old version either.

> We might *also* want a USAGEANDSELECTANDEXECUTEANDWHATNOTBUTNOBYPASSURL
> > (crap, it has to fit within NAMEDATALEN?) privilege, but I think that's a
> > different thing.
>
> Our privilege system doesn't currently allow for negative grants (deny
> user X the ability to run functions, even if EXECUTE is granted to
> PUBLIC). I'm not against that idea, but I don't see it as the same as
> this.
>

Agreed. That's what I said - different thing :)

> > it would only give you COPY access. (And also
> > > > COPY != SELECT in the way that the rule system applies, I think? And
> this
> > > > one could be for COPY only)
> > >
> > > COPY certainly does equal SELECT rights.. We haven't got an
> independent
> > > COPY privilege and I don't think it makes sense to invent one. It
> >
> > We don't, but I'm saying it might make sense to implement this. Not as a
> > regular privilige, but as a role attribute. I'm not sure it's the right
> > thing, but it might actually be interesting to entertain.
>
> We've discussed having a role attribute for COPY-from-filesystem, but
> pg_dump doesn't use that ever, it only uses COPY TO STDOUT. I'm not
> a fan of making a COPY_TO_STDOUT-vs-SELECT distinction just for this..
>

Yeah, it's probably going overboard with it, since AFAICT the only thing
that would actually be affected is RULEs on SELECT, which I bet most people
don't use on their tables.

> > sounds like you're suggesting that we add a hack directly into COPY for
> > > this privilege, but my thinking is that the right place to change for
> > > this is in the privilege system and not hack it into individual
> > > commands.. I'm also a bit nervous that we'll end up painting ourselves
> > > into a corner if we hack this to mean exactly what pg_dump needs today.
> >
> > One point would be that if we define it as "exactly what pg_dump needs",
> > that definition can change in a future major version.
>
> Sure, but that then gets a bit ugly because we encourage running the
> latest version of pg_dump against the prior-major-version.
>

But the latest version of pg_dump *knows* how the prior major version
behaved with these things, and can either adapt, or give the user a message
about what they need to do to adapt.

> > > I think BYPASSRLS would have to be implicitly granted by the DUMP
> > > > privilege. Without that, the DUMP privilege is more or less
> meaningless
> > > > (for anybody who uses RLS - but if they don't use RLS, then having it
> > > > include BYPASSRLS makes no difference). Worse than that, you may end
> up
> > > > with a dump that you cannot restore.
> > >
> > > The dump would fail, as I mentioned before. I don't see why BYPASSRLS
> > > has to be implicitly granted by the DUMP privilege and can absolutely
> > > see use-cases for it to *not* be. Those use-cases would require
> passing
> > > pg_dump the --enable-row-security option, but that's why it's there.
> >
> > So you're basically saying that if RLS is in use anywhere, this
> priviliege
> > alone would be useless, correct?
>
> No, it would still be *extremely* useful. We have an option to pg_dump
> to say "please dump with RLS enabled". What that means is that you'd be
> able to dump the entire database for all data you're allowed to see
> through RLS policies. How is that useless? I certainly think it'd be
> very handy and a good way to get *segregated* dumps according to policy.
>

Hmm. Yeah, I guess - my mindset was int he "database backup" mode, where a
"silently partial" backup is a really scary thing rather than a feature :)
I guess as long as you still dump all the parts, RLS itself ensures that
foreign keys etc will actually be valid upon a reaload?

> And you would get a hard failure at *dump
> > time*, not at reload time?
>
> If you attempt to pg_dump a relation which has RLS enabled, and you
> don't enable RLS with pg_dump, then you'll get a failure at dump time,
> yes. That's far better than a reload-time failure.
>

Good.

> That would at least make it acceptable, as you
> > would know it's wrong. and we could make the error messages shown
> > specifically hint that you need to grant both privileges to make it
> useful.
>
> Agreed, having such a hint makes sense.
>
> > We could/should also throw a WARNING if DUMP Is granted to a role without
> > BYPASSRLS in case row level security is enabled in the system, I think.
> But
> > that's more of an implementation detail.
>
> That's a bit ugly and RLS could be added to a relation after the DUMP
> privilege is granted.
>

Yes, it's not going to be all-covering, but it can still be a useful
hint/warning in the cases where it *does* that. We obviously still need
pg_dump to give the error in both scenarios.

What I think this part of the discussion is getting at is that there's a
> lot of people who view pg_dump as explicitly for "dump the ENTIRE
> database". While that's one use-case it is certainly not the only one.
> I find "pg_dump schema X" to be very common and often find that pg_dump
> against an entire database isn't practical because of the size of the
> database.
>

Agreed. I was in that mindset since we were talking about other backups.
And FWIW, one reason I like to call it "DUMP" and not anything to do with
BACKUP is exactly that. pg_dump is often not very useful for backups
anymore, due to the size of databases, but it's very useful for other
things.

> > Similar concerns would exist for the existing REPLICATION role for
> example
> > > > - that one clearly lets you bypass RLS as well, just not with a SQL
> > > > statement.
> > >
> > > I'm not sure that I see the point you're making here. Yes, REPLICATION
> > > allows you to do a filesystem copy of the entire database and that
> > > clearly bypasses RLS and *all* of our privilege system. I'm suggesting
> > > that this role attribute work as an implicit grant of privileges we
> > > already have. That strikes me as easy to document and very clear for
> > > users.
> >
> > It does - though documenting that it implicitly gives you a different
> > privilege as well is also easy :)
> >
> > But it does potentially limit us to what we can actually do with the
> > priviliges. One reason to use them is exactly because we *cannot* express
> > this with our regular permissions (such as BYPASSRLS or REPLICATION).
>
> Ok, I see the point you're making that we could make this into a
> capability which isn't something which can be expressed through our
> existing GRANT system. That strikes me as a solution trying to find a
> problem though. There's no need to invent such an oddity for this
> particular use-case, I don't think.
>

Maybe not, but we should make sure we don't paint ourselves into a corner
where we cannot do it in the future either.

> For regular permissions, we could just pre-populate the system with
> > predefined roles and use regular GRANTs to those roles, instead of
> relying
> > on role attributes, which might in that case make it even more clear?
>
> The reason for this approach is to address exactly the nightmare that is
> trying to maintain those regular permissions across all the objects in
> the system. Today, people simply give the role trying to do the pg_dump
> superuser, which is the best option we have. Saying 'grant SELECT on
> all the tables and USAGE on all the schemas' isn't suggested because
> it's a huge pain. This role attribute provides a middle ground where
> the pg_dump'ing role isn't a superuser, but you don't have to ensure
> usage and select are granted to it for every relation.
>

No, what I'm saying is we could have *predefined role* that allows "select
on all the tables and usage on all the schemas". And you are unable to
actually remove that. It's not stored on the object, so you cannot REVOKE
the permission on the *object*. Since it's not store on the object it will
also automatically apply to all new objects, regardless of what you've done
with DEFAULT PRIVILEGES etc.

But you can grant users and other roles access to this role, and it's dealt
with like other roles from the "who has it" perspective, instead of being
special-cased.

Instead of "ALTER USER foo WITH DUMP" (or whatever), you'd just do a "GRANT
pgdump TO foo" (which would then also work through things like group
membership as well)

> > should work 'sanely' with any combination of the two options.
> > > > > Similairly, DUMP shouldn't imply BACKUP or visa-versa. In
> particular,
> > > > > I'd like to see roles which have only the BACKUP privilege be
> unable to
> > > > > directly read any data (modulo things granted to PUBLIC). This
> would
> > > > > allow an unprivileged user to manage backups, kick off ad-hoc ones,
> > > etc,
> > > > > without being able to actually access any of the data (this would
> > > > > require the actual backup system to have a similar control, but
> that's
> > > > > entirely possible with more advanced SANs and enterprise backup
> > > > > solutions).
> > > >
> > > > So you're saying a privilege that would allow you to do
> > > > pg_start_backup()/pg_stop_backup() but *not* actually use
> pg_basebackup?
> > >
> > > Yes.
> >
> > What's really the usecase for that? I'd say that pg_start/stop backup is
> a
> > superset and a higher privilige than using pg_basebackup, since you can
> for
> > example destroy other peoples backups using it (by running pg_stop_backup
> > while they are running).
>
> One use-case is to allow unprivileged users to run adhoc backups.

Another is simply that you don't want the cronjob that runs the backup
> to be able to read any of the data directly (why would you?). I agree
> that the individuals who have this capability would still need to
> coordinate or at lesat not step on each other or they could end up with
> bad backups.
>
> Another way to address that would be to require that the role calling
> stop_backup be a member of the role which called start_backup (that also
> address the superuser case, as superusers are considered members of all
> roles). That seems like a pretty reasonable sanity check requirement.
>

This diverges a bit from the actual role attribute discussion here, but I
think a better solution to this is to actually have a separate interface
rather than pg_start/pg_stop backup. One of the main problems with
pg_start/pg_stop vs pg_basebackup is that you can easily leave the system
in a broken state (for example by forgetting to pg_stop_backup, or by
accidentally pg_stop_backup:ing in the middle of someone elses backup).
pg_basebackup gets around this by automatically executing do_pg_stop_backup
if the client disconnects. This also lets us allow parallel base backups.
We could have an equivalent functionality exposed through a SQL function,
or argument to pg_start_backup - it would require the backup software to
keep the connection running as it works and then disconnect when it's done,
but for anything beyond the most trivial shellscript that's not exactly
hard, and it would make the backups safer.

If we did that, perhaps we don't even need a separate privilege for
pg_start_backup() as it is today, btu can leave that as superuser?

> > Personalyl I think using the DUMP name makes that a lot more clear.
> Maybe
> > > > we need to avoid using BACKUP alone as well, to make sure it doesn't
> go
> > > the
> > > > other way - using BASEBACKUP and EXCLUSIVEBACKUP for those two
> different
> > > > ones perhaps?
> > >
> > > DUMP - implicitly grants existing rights, to facilitate pg_dump and
> > > perhaps some other use-cases
> > > BASEBACKUP - allows pg_basebackup, which means bypassing all in-DB
> > > privilege systems
> >
> > That's equivalent of REPLICATION, yes?
>
> Ah, yeah, seems like it would be. Got ahead of myself. :)
>
> > > EXCLUSIVEBACKUP - just allows pg_start/stop_backup and friends
> >
> > I'd say there is no "just" there really - that's a higher level
> privilege,
> > but I see your point :)
>
> Well, see above, but ok.
>
> > Later I added:
> > >
> > > pg_xlog_replay_pause
> > > pg_xlog_replay_resume
> > >
> > > Though I'd be find if the xlog_replay ones were their own privilege
> (eg:
> > > REPLAY or something).
> > >
> >
> > I think it makes more sense to have those replay functions to be a
> separate
> > privilege, yes. They have nothing to actually do with taking the backups
> -
> > they are for restoring them. And to restore the backups, you clearly
> > already have superuser level privileges on the system outside the db (at
> > least as long as we only allow full cluster restores).
>
> Not quite- remember that reply_pause/resume can be done on a replica, so
> they are useful to be able to grant independent from superuser. Perhaps
> we call such a role attribute XLOGREPLAY ?
>

Yes, that's what I meant - they are useful, but they're not interesting for
the bacukp itself. So yes, something like XLOGREPLAY makes sense.

> > > You mean something that restricts the user to read even *if* write
> > > > permissions has been granted on an individual table? Yeah, that would
> > > > actually be quite useful, I think - sort of a "reverse privilege".
> > >
> > > Yes. My thinking on how to approach this was to forcibly set all of
> > > their transactions to read-only.
> >
> > Agreed that this would be very useful.
>
> Glad we agree.. Any thoughts on implementation? :)
>

Where's your patch? :)

In theory we'd just have to trap any attempt to set the value for
transaction_read_only, no? Same as hot_standby does?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

José,

* José Luis Tallón (jltallon(at)adv-solutions(dot)net) wrote:
> On 12/30/2014 04:16 PM, Stephen Frost wrote:
> >The approach I was thinking was to document and implement this as
> >impliciting granting exactly USAGE and SELECT rights, no more (not
> >BYPASSRLS) and no less (yes, the role could execute functions). I agree
> >that doing so would be strictly more than what pg_dump actually requires
> >but it's also what we actually have support for in our privilege system.
>
> Hmmm.... coupled with your comments below, I'd say some tweaking of
> the existing privileges is actually needed if we want to add these
> new "capabilities".

Not sure I see how..? Can you clarify what you think needs to be
changed in the existing privilege system?

> BTW, and since this is getting a bit bigger than originally
> considered: would it be interesting to support some
> extension-defined capabilities, too?
> (for things can't be easily controlled by the existing USAGE /
> SELECT / ... rights, I mean)

Not entirely following what you mean here either, but to the extent that
it's independent from the current discussion, perhaps it deserves to be
on its own thread?

> >>it would only give you COPY access. (And also
> >>COPY != SELECT in the way that the rule system applies, I think? And this
> >>one could be for COPY only)
> >COPY certainly does equal SELECT rights.. We haven't got an independent
> >COPY privilege and I don't think it makes sense to invent one.
>
> FWIW, a COPY(DUMP?) privilege different from SELECT would make sense.
> Considering your below comments it would be better that it not imply
> BYPASSRLS, though.

How would a COPY-to-STDOUT privilege be different from SELECT?

> >Lastly, I've been considering other use-cases for this privilege beyond
> >the pg_dump one (thinking about auditing, for example).
>
> ISTR there was something upthread on an AUDIT role, right?
> This might be the moment to add it....

One of the challenges to adding such a role is defining what it means.
What privileges do you think such a role would have? I can see that
perhaps it would include read-only access to everything, but I'd want it
to also have read access to the logs..

> >>That would be "EXCLUSIVEBACKUP" or something like that, to be consistent
> >>with existing terminology though.
> >Ok. I agree that working out the specific naming is difficult and would
> >like to focus on that, but we probably need to agree on the specific
> >capabilities first. :)
>
> Yes :)
> For the record, LOGBACKUP vs PHYSBACKUP was suggested a couple days
> ago. I'd favor DUMP(logical) and BACKUP(physical) --- for lack of a
> better name.

I think I'm coming around to the EXCLUSIVEBACKUP option, per the
discussion with Magnus. I don't particularly like LOGBACKUP and don't
think it really makes sense, while PHYSBACKUP seems like it'd cover what
REPLICATION already does.

> The above reasoning would have pg_basebackup requiring REPLICATION,
> which is a logical consequence of the implementation but strikes me
> as a bit surprising in the light of these other privileges.

I see what you mean that it's a bit strange for pg_basebackup to require
the REPLICATION role attribute, but, well, that's already the case, no?
Don't think we're going to change that..

> ARCHIVE, though completely appropriate for the "exclusivebackup"
> case above (so this would become DUMP/BASEBACKUP/ARCHIVE
> +REPLICATION) might end up causing quite some confusion ... ("what?
> WAL archiving is NOT granted by the "archive" privilege, but
> requires a superuser to turn it on(via ALTER SYSTEM)?

Yeah, Magnus didn't like ARCHIVE either and I understand his reasoning.

> >pg_xlog_replay_pause
> >pg_xlog_replay_resume
> >
> >Though I'd be find if the xlog_replay ones were their own privilege (eg:
> >REPLAY or something).
> +1

Yeah, Magnus was also agreeing that they should be independent.

> >>I think just calling them "xlog related functions" is doing us a disservice
> >>there. Definitely once we have an actual documentation to write for it, but
> >>also in this discussion.
> >[snip]
> >>If it's for replicas, then why are we not using the REPLICATION privilege
> >>which is extremely similar to this?
> >I don't actually see REPLICATION as being the same.
>
> REPLICATION (ability to replicate) vs REPLICAOPERATOR (ability to
> control *the replica* or the R/O behaviour of the server, for that
> matter...)

Right, think Magnus and I clarified what was meant there.

> >Yes. My thinking on how to approach this was to forcibly set all of
> >their transactions to read-only.
>
> So "READONLY" ?

Right.

> >Agreed. That's actually something that I think would be *really* nice-
> >an option to dump the necessary globals for whatever database you're
> >running pg_dump against. We have existing problems in this area
> >(database-level GUCs aren't considered database-specific and therefore
> >aren't picked up by pg_dump, for example..), but being able to also dump
> >the roles which are used in a given database with pg_dump would be
> >*really* nice..
>
> Ok, so this would imply modifying pg_dump to include database-level
> configs. I would heartily welcome this.

I think a lot of people would like to see that, though I do think it's
at least somewhat independent from this particular proposal.

> So, it seems to me that we are actually evolving towards a set of
> "low-level" "capabilities" and some "high-level" (use case-focused)
> "privileges".

Well, I would argue that we're making a good set of 'low-level'
capabilities which users are then able to pull together as they see fit
into specific 'roles' for their environment. One environment might see
a DBA role as having X, Y, Z, while another only wants X and Y.

> I am hereby volunteering to compile this thread into some wiki page.
> I'm thinking "Privileges" as the title for starters. Suggestions?

A wiki page would certainly be useful, especially if it's done in such a
way that we can take and make it into the documentation to go along with
these role attributes easily.

Thanks!

Stephen

* Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> On Wed, Dec 31, 2014 at 3:08 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > * Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> > > that doing so would be strictly more than what pg_dump actually requires
> > > > but it's also what we actually have support for in our privilege
> > system.
> > >
> > > Yeah, but would it also be what people would actually *want*?
> >
> > I can certainly see reasons why you'd want such a role to be able to
> > execute functions- in particular, consider xlog_pause anx xlog_resume.
>
> If this role can't execute those functions then they probably can't
> > successfully run pg_dump against a replica.
>
> uh, pg_dump doesn't run those commands :P I don't see why that's a
> requirement at all. And you can still always grant those things on top of
> whatever the privilege gives you.

Ok, so, first off, this is all about the discussion around "is this
additive or not".. Since we just agreed that it's additive, I'm not
sure I see the need to discuss the EXECUTE privileges..

Having said that though..

If you can't pause/resume then you can end up with your pg_dump
transaction getting killed. I'm aware of folks who already do this
today, by hand with shell scripts.. I agree that pg_dump doesn't do it,
but I do think it'd be nice to have and I can certainly see the use-case
for them..

> > I think having it do exactly what pg_dump needs, and not things like
> > > execute functions etc, would be the thing people want for a 'DUMP'
> > > privilege.
> >
> > What if we want pg_dump in 9.6 to have an option to execute xlog_pause
> > and xlog_resume for you? You wouldn't be able to run that against a 9.5
> > database (or at least, that option wouldn't work).
>
> It would if you added an explicit grant for it, which would have to be
> documented.

Huh? An explicit grant for xlog_pause/xlog_resume won't work as we
check role attributes rights inside the function..

> I don't see how that's different if the definition is "allows select on all
> tables". That wouldn't automatically give it the ability to do xlog_pause
> in an old version either.

Ok, that I agree with- you don't automatically get xlog_pause rights if
you have the 'select on all tables' right.

> > We've discussed having a role attribute for COPY-from-filesystem, but
> > pg_dump doesn't use that ever, it only uses COPY TO STDOUT. I'm not
> > a fan of making a COPY_TO_STDOUT-vs-SELECT distinction just for this..
>
> Yeah, it's probably going overboard with it, since AFAICT the only thing
> that would actually be affected is RULEs on SELECT, which I bet most people
> don't use on their tables.

Well, we could make SELECT not work, but if you've got COPY then you can
still get all the data, so, yeah, not much different. I seriously doubt
many people are using rules..

> > > One point would be that if we define it as "exactly what pg_dump needs",
> > > that definition can change in a future major version.
> >
> > Sure, but that then gets a bit ugly because we encourage running the
> > latest version of pg_dump against the prior-major-version.
>
> But the latest version of pg_dump *knows* how the prior major version
> behaved with these things, and can either adapt, or give the user a message
> about what they need to do to adapt.

Yes, that's true.

> > No, it would still be *extremely* useful. We have an option to pg_dump
> > to say "please dump with RLS enabled". What that means is that you'd be
> > able to dump the entire database for all data you're allowed to see
> > through RLS policies. How is that useless? I certainly think it'd be
> > very handy and a good way to get *segregated* dumps according to policy.
>
> Hmm. Yeah, I guess - my mindset was int he "database backup" mode, where a
> "silently partial" backup is a really scary thing rather than a feature :)

Agreed, we don't ever want that.

> I guess as long as you still dump all the parts, RLS itself ensures that
> foreign keys etc will actually be valid upon a reaload?

If you set up your policies correctly, yes. RLS is flexible enough that
you could create policies which fail, but you have the same problem
today to some extent (consider tables you don't have SELECT rights on).

> > > We could/should also throw a WARNING if DUMP Is granted to a role without
> > > BYPASSRLS in case row level security is enabled in the system, I think.
> > But
> > > that's more of an implementation detail.
> >
> > That's a bit ugly and RLS could be added to a relation after the DUMP
> > privilege is granted.
>
> Yes, it's not going to be all-covering, but it can still be a useful
> hint/warning in the cases where it *does* that. We obviously still need
> pg_dump to give the error in both scenarios.

I'm not against doing it, personally, but I suspect others won't like it
(or at least, that's been the case in the past with other things..).

> What I think this part of the discussion is getting at is that there's a
> > lot of people who view pg_dump as explicitly for "dump the ENTIRE
> > database". While that's one use-case it is certainly not the only one.
> > I find "pg_dump schema X" to be very common and often find that pg_dump
> > against an entire database isn't practical because of the size of the
> > database.
>
> Agreed. I was in that mindset since we were talking about other backups.
> And FWIW, one reason I like to call it "DUMP" and not anything to do with
> BACKUP is exactly that. pg_dump is often not very useful for backups
> anymore, due to the size of databases, but it's very useful for other
> things.

Ok.

> > Ok, I see the point you're making that we could make this into a
> > capability which isn't something which can be expressed through our
> > existing GRANT system. That strikes me as a solution trying to find a
> > problem though. There's no need to invent such an oddity for this
> > particular use-case, I don't think.
>
> Maybe not, but we should make sure we don't paint ourselves into a corner
> where we cannot do it in the future either.

Agreed. Do you see a risk here of that?

> > For regular permissions, we could just pre-populate the system with
> > > predefined roles and use regular GRANTs to those roles, instead of
> > relying
> > > on role attributes, which might in that case make it even more clear?
> >
> > The reason for this approach is to address exactly the nightmare that is
> > trying to maintain those regular permissions across all the objects in
> > the system. Today, people simply give the role trying to do the pg_dump
> > superuser, which is the best option we have. Saying 'grant SELECT on
> > all the tables and USAGE on all the schemas' isn't suggested because
> > it's a huge pain. This role attribute provides a middle ground where
> > the pg_dump'ing role isn't a superuser, but you don't have to ensure
> > usage and select are granted to it for every relation.
>
> No, what I'm saying is we could have *predefined role* that allows "select
> on all the tables and usage on all the schemas". And you are unable to
> actually remove that. It's not stored on the object, so you cannot REVOKE
> the permission on the *object*. Since it's not store on the object it will
> also automatically apply to all new objects, regardless of what you've done
> with DEFAULT PRIVILEGES etc.
>
> But you can grant users and other roles access to this role, and it's dealt
> with like other roles from the "who has it" perspective, instead of being
> special-cased.
>
> Instead of "ALTER USER foo WITH DUMP" (or whatever), you'd just do a "GRANT
> pgdump TO foo" (which would then also work through things like group
> membership as well)

There's a definite backwards-compatibility concern with this, of course,
but I see where you're coming from. This only really applies with this
particular pg_dump-related role-attribute discussion though, right?

This role wouldn't be able to be logged in with, I presume? Would it
show up when you run \du? What about in pg_authid? I feel like it
would have to and I worry that users might not care for it- and what
happens if they want to remove it?

The question about role-attribute vs. inheirited-right is one that I've
been wondering about also though.

> Another is simply that you don't want the cronjob that runs the backup
> > to be able to read any of the data directly (why would you?). I agree
> > that the individuals who have this capability would still need to
> > coordinate or at lesat not step on each other or they could end up with
> > bad backups.
> >
> > Another way to address that would be to require that the role calling
> > stop_backup be a member of the role which called start_backup (that also
> > address the superuser case, as superusers are considered members of all
> > roles). That seems like a pretty reasonable sanity check requirement.
>
> This diverges a bit from the actual role attribute discussion here, but I
> think a better solution to this is to actually have a separate interface
> rather than pg_start/pg_stop backup. One of the main problems with
> pg_start/pg_stop vs pg_basebackup is that you can easily leave the system
> in a broken state (for example by forgetting to pg_stop_backup, or by
> accidentally pg_stop_backup:ing in the middle of someone elses backup).

I agree. I've had this happen to me a number of times and it really
stinks to have your *next* backup fail because the last one failed
half-way and didn't run pg_stop_backup.

> pg_basebackup gets around this by automatically executing do_pg_stop_backup
> if the client disconnects. This also lets us allow parallel base backups.
> We could have an equivalent functionality exposed through a SQL function,
> or argument to pg_start_backup - it would require the backup software to
> keep the connection running as it works and then disconnect when it's done,
> but for anything beyond the most trivial shellscript that's not exactly
> hard, and it would make the backups safer.

Agreed, I do like that idea.

> If we did that, perhaps we don't even need a separate privilege for
> pg_start_backup() as it is today, btu can leave that as superuser?

I don't see how this follows though.. We are not talking about only
pg_basebackup or only about where the user running pg_start/stop
has filesystem-level access to the database.

> > Not quite- remember that reply_pause/resume can be done on a replica, so
> > they are useful to be able to grant independent from superuser. Perhaps
> > we call such a role attribute XLOGREPLAY ?
>
> Yes, that's what I meant - they are useful, but they're not interesting for
> the bacukp itself. So yes, something like XLOGREPLAY makes sense.

Ok, that works for me.

> > > > You mean something that restricts the user to read even *if* write
> > > > > permissions has been granted on an individual table? Yeah, that would
> > > > > actually be quite useful, I think - sort of a "reverse privilege".
> > > >
> > > > Yes. My thinking on how to approach this was to forcibly set all of
> > > > their transactions to read-only.
> > >
> > > Agreed that this would be very useful.
> >
> > Glad we agree.. Any thoughts on implementation? :)
>
> Where's your patch? :)

Haha. :P

> In theory we'd just have to trap any attempt to set the value for
> transaction_read_only, no? Same as hot_standby does?

I like the idea.. Will have to give it a shot and see what happens. :)

Thanks!

Stephen

On Wed, Dec 31, 2014 at 4:23 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> * Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> > On Wed, Dec 31, 2014 at 3:08 PM, Stephen Frost <sfrost(at)snowman(dot)net>
> wrote:
> > > * Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> > > I think having it do exactly what pg_dump needs, and not things like
> > > > execute functions etc, would be the thing people want for a 'DUMP'
> > > > privilege.
> > >
> > > What if we want pg_dump in 9.6 to have an option to execute xlog_pause
> > > and xlog_resume for you? You wouldn't be able to run that against a
> 9.5
> > > database (or at least, that option wouldn't work).
> >
> > It would if you added an explicit grant for it, which would have to be
> > documented.
>
> Huh? An explicit grant for xlog_pause/xlog_resume won't work as we
> check role attributes rights inside the function..
>

Correct, of course. I was confusing myself.

> > We've discussed having a role attribute for COPY-from-filesystem, but
> > > pg_dump doesn't use that ever, it only uses COPY TO STDOUT. I'm not
> > > a fan of making a COPY_TO_STDOUT-vs-SELECT distinction just for this..
> >
> > Yeah, it's probably going overboard with it, since AFAICT the only thing
> > that would actually be affected is RULEs on SELECT, which I bet most
> people
> > don't use on their tables.
>
> Well, we could make SELECT not work, but if you've got COPY then you can
> still get all the data, so, yeah, not much different. I seriously doubt
> many people are using rules..
>

Yeah.

> > > We could/should also throw a WARNING if DUMP Is granted to a role
> without
> > > > BYPASSRLS in case row level security is enabled in the system, I
> think.
> > > But
> > > > that's more of an implementation detail.
> > >
> > > That's a bit ugly and RLS could be added to a relation after the DUMP
> > > privilege is granted.
> >
> > Yes, it's not going to be all-covering, but it can still be a useful
> > hint/warning in the cases where it *does* that. We obviously still need
> > pg_dump to give the error in both scenarios.
>
> I'm not against doing it, personally, but I suspect others won't like it
> (or at least, that's been the case in the past with other things..).
>

Heh, let's defer to a third party then :)

> > Ok, I see the point you're making that we could make this into a
> > > capability which isn't something which can be expressed through our
> > > existing GRANT system. That strikes me as a solution trying to find a
> > > problem though. There's no need to invent such an oddity for this
> > > particular use-case, I don't think.
> >
> > Maybe not, but we should make sure we don't paint ourselves into a corner
> > where we cannot do it in the future either.
>
> Agreed. Do you see a risk here of that?
>

Not really, anymore, i think :)

> > For regular permissions, we could just pre-populate the system with
> > > > predefined roles and use regular GRANTs to those roles, instead of
> > > relying
> > > > on role attributes, which might in that case make it even more clear?
> > >
> > > The reason for this approach is to address exactly the nightmare that
> is
> > > trying to maintain those regular permissions across all the objects in
> > > the system. Today, people simply give the role trying to do the
> pg_dump
> > > superuser, which is the best option we have. Saying 'grant SELECT on
> > > all the tables and USAGE on all the schemas' isn't suggested because
> > > it's a huge pain. This role attribute provides a middle ground where
> > > the pg_dump'ing role isn't a superuser, but you don't have to ensure
> > > usage and select are granted to it for every relation.
> >
> > No, what I'm saying is we could have *predefined role* that allows
> "select
> > on all the tables and usage on all the schemas". And you are unable to
> > actually remove that. It's not stored on the object, so you cannot REVOKE
> > the permission on the *object*. Since it's not store on the object it
> will
> > also automatically apply to all new objects, regardless of what you've
> done
> > with DEFAULT PRIVILEGES etc.
> >
> > But you can grant users and other roles access to this role, and it's
> dealt
> > with like other roles from the "who has it" perspective, instead of being
> > special-cased.
> >
> > Instead of "ALTER USER foo WITH DUMP" (or whatever), you'd just do a
> "GRANT
> > pgdump TO foo" (which would then also work through things like group
> > membership as well)
>
> There's a definite backwards-compatibility concern with this, of course,
> but I see where you're coming from. This only really applies with this
> particular pg_dump-related role-attribute discussion though, right?
>

Yes, I believe so. Because it's so similar to the regular permissions,
where as something like "being able to take a base backup" is more of a
boolean - it doesn't apply to actual objects in the database cluster, it's
more global.

This role wouldn't be able to be logged in with, I presume?

Definitely not.

> Would it
> show up when you run \du? What about in pg_authid? I feel like it
> would have to and I worry that users might not care for it- and what
> happens if they want to remove it?
>

Well, going by experience from other systems that have such a role, I'd say
yes it should show up, and it should throw an error when you try to remove
it.

The question about role-attribute vs. inheirited-right is one that I've
> been wondering about also though.
>
> > This diverges a bit from the actual role attribute discussion here, but I
> > think a better solution to this is to actually have a separate interface
> > rather than pg_start/pg_stop backup. One of the main problems with
> > pg_start/pg_stop vs pg_basebackup is that you can easily leave the system
> > in a broken state (for example by forgetting to pg_stop_backup, or by
> > accidentally pg_stop_backup:ing in the middle of someone elses backup).
>
> I agree. I've had this happen to me a number of times and it really
> stinks to have your *next* backup fail because the last one failed
> half-way and didn't run pg_stop_backup.
>

I've seen things so much worse than just that :)

> pg_basebackup gets around this by automatically executing
> do_pg_stop_backup
> > if the client disconnects. This also lets us allow parallel base backups.
> > We could have an equivalent functionality exposed through a SQL function,
> > or argument to pg_start_backup - it would require the backup software to
> > keep the connection running as it works and then disconnect when it's
> done,
> > but for anything beyond the most trivial shellscript that's not exactly
> > hard, and it would make the backups safer.
>
> Agreed, I do like that idea.
>
> > If we did that, perhaps we don't even need a separate privilege for
> > pg_start_backup() as it is today, btu can leave that as superuser?
>
> I don't see how this follows though.. We are not talking about only
> pg_basebackup or only about where the user running pg_start/stop
> has filesystem-level access to the database.
>

Hmm, yeah, i guess you're right. While from a security perspective they can
already read all the data, they can't read it safely without that. And the
fact that you *also* need a local account with file system access is a
second layer of security or something like that.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Wed, Dec 24, 2014 at 12:48 PM, Adam Brightwell
<adam(dot)brightwell(at)crunchydatasolutions(dot)com> wrote:
> * BACKUP - allows role to perform backup operations
> * LOGROTATE - allows role to rotate log files
> * MONITOR - allows role to view pg_stat_* details
> * PROCSIGNAL - allows role to signal backend processes

How about just "SIGNAL" instead of "PROCSIGNAL"?

Generally, I think we'll be happier if these capabilities have names
that are actual words - or combinations of words - rather than partial
words, so I'd suggest avoiding things like PROC for PROCESS and AUTH
for AUTHORIZATION.

In this particular case, it seems like the name of the capability is
based off the name of an internal system data structure. That's the
sort of thing that we do not want to expose to users. As far as we
can, we should try to describe what the capability allows, not the
details of how that is (currently) implemented.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, Jan 5, 2015 at 11:49 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Wed, Dec 24, 2014 at 12:48 PM, Adam Brightwell
> <adam(dot)brightwell(at)crunchydatasolutions(dot)com> wrote:
> > * BACKUP - allows role to perform backup operations
> > * LOGROTATE - allows role to rotate log files
> > * MONITOR - allows role to view pg_stat_* details
> > * PROCSIGNAL - allows role to signal backend processes
>
> How about just "SIGNAL" instead of "PROCSIGNAL"?
>

Sure.

> Generally, I think we'll be happier if these capabilities have names
> that are actual words - or combinations of words - rather than partial
> words, so I'd suggest avoiding things like PROC for PROCESS and AUTH
> for AUTHORIZATION.
>

Agreed.

> In this particular case, it seems like the name of the capability is
> based off the name of an internal system data structure. That's the
> sort of thing that we do not want to expose to users. As far as we
> can, we should try to describe what the capability allows, not the
> details of how that is (currently) implemented.

Agreed.

If others are also in agreement on this point then I'll update the patch
accordingly.

Thanks,
Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

Adam, all,

* Adam Brightwell (adam(dot)brightwell(at)crunchydatasolutions(dot)com) wrote:
> If others are also in agreement on this point then I'll update the patch
> accordingly.

Works for me.

Thanks!

Stephen

All,

Attached is a patch that proposes the following additional role attributes
for review:

* ONLINE_BACKUP - allows role to perform backup operations
- originally proposed as BACKUP - due to concern for the use of that term
in relation to other potential backup related permissions this form is in
line with the documentation as it describes the affected backup operations
as being 'online backups'.
- applies only to the originally proposed backup functions.
* XLOG_REPLAY - allows role to perform pause and resume on xlog_replay
operations ('pg_xlog_replay_pause' and 'pg_xlog_replay_resume')
- following the recommendation from Stephen and Magnus.
* LOG - allows role to rotate log files - remains broad enough to consider
future log related operations
* MONITOR - allows role to view pg_stat_* details (as originally proposed)
* SIGNAL - allows role to signal backend processes (as originally proposed)

The documentation still needs to be updated. If this these attributes and
the capabilities they provide are acceptable, then I'll begin moving
forward on making those updates as well.

Regarding the discussion on a DUMP/READONLY permission. I believe that
topic needs much further discussion and decided it is probably best to keep
it as a separate patch/effort. I'd certainly be willing to continue that
discussion and assist in moving any related effort forward, therefore,
please let me know if there is anything I can do to help.

Thanks,
Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

On Thu, Jan 15, 2015 at 6:03 PM, Adam Brightwell
<adam(dot)brightwell(at)crunchydatasolutions(dot)com> wrote:
> * ONLINE_BACKUP - allows role to perform backup operations
> - originally proposed as BACKUP - due to concern for the use of that term
> in relation to other potential backup related permissions this form is in
> line with the documentation as it describes the affected backup operations
> as being 'online backups'.
> - applies only to the originally proposed backup functions.

I'm slightly mystified as to how including the word "online" helps
here. It's unlikely that there will be an offline_backup permission,
because if the system is off-line, SQL-level permissions are
irrelevant.

> * LOG - allows role to rotate log files - remains broad enough to consider
> future log related operations

Maybe LOGFILE? Only because some confusion with the LOG message level
seems possible; or confusion about whether this is a permission that
lets you log things.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert,

Thanks for the feedback.

I'm slightly mystified as to how including the word "online" helps
> here. It's unlikely that there will be an offline_backup permission,
> because if the system is off-line, SQL-level permissions are
> irrelevant.

I'm certainly open to recommendations on this one. Initially, BACKUP was
proposed, but based on the discussion, it is unacceptable. As mentioned,
the documentation for the affected functions refer to starting/stopping an
'on-line backup', hence the current proposal. I feel like it is obviously
more in line with the documentation and removes the ambiguity in what
'type' of backup it allows, as that seemed to be one of the major concerns
of just using BACKUP. However, I could certainly understand if there was a
confusion on the terminology of 'online' vs 'offline' if those are not
regularly used terms or concepts. At any rate, I'll certainly continue to
give this one thought, but I wouldn't mind any recommendations/suggestions
anyone was willing to throw my way.

> * LOG - allows role to rotate log files - remains broad enough to consider
> > future log related operations
>
> Maybe LOGFILE? Only because some confusion with the LOG message level
> seems possible; or confusion about whether this is a permission that
> lets you log things.

That's a good point. I'll change this one up.

Thanks,
Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Thu, Jan 15, 2015 at 6:03 PM, Adam Brightwell
> <adam(dot)brightwell(at)crunchydatasolutions(dot)com> wrote:
> > * ONLINE_BACKUP - allows role to perform backup operations
> > - originally proposed as BACKUP - due to concern for the use of that term
> > in relation to other potential backup related permissions this form is in
> > line with the documentation as it describes the affected backup operations
> > as being 'online backups'.
> > - applies only to the originally proposed backup functions.
>
> I'm slightly mystified as to how including the word "online" helps
> here. It's unlikely that there will be an offline_backup permission,
> because if the system is off-line, SQL-level permissions are
> irrelevant.

ONLINE does match up with what we call the pg_start/stop_backup based
backups in the documentation, at least. Also, it's intended to contrast
against pg_dump-based backups, not offline backups (which we don't
discuss at all in the docs that I can see).

Looking over the docs again a bit though, what about BACKUP_CONTROL,
following the title of 9.26.3?

Suggestions certainly welcome.

> > * LOG - allows role to rotate log files - remains broad enough to consider
> > future log related operations
>
> Maybe LOGFILE? Only because some confusion with the LOG message level
> seems possible; or confusion about whether this is a permission that
> lets you log things.

LOGFILE works for me.

Thanks!

Stephen

On 3 November 2014 at 17:08, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> role attributes don't act like
> GRANTs anyway (there's no ADMIN option and they aren't inheirited).

I'm happy with us *not* doing this using GRANTs, as long as we spend
some love on the docs to show there is a very clear distinction
between the two.

Users get confused between privs, role attributes and SETs that apply to roles.

Introducing the new word "capability" needs to also have some clarity.
Is that the same thing as "role attribute", or is that a 4th kind of
thang?

Make things make sense and they're easy to agree.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, RemoteDBA, Training & Services

* Simon Riggs (simon(at)2ndQuadrant(dot)com) wrote:
> On 3 November 2014 at 17:08, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > role attributes don't act like
> > GRANTs anyway (there's no ADMIN option and they aren't inheirited).
>
> I'm happy with us *not* doing this using GRANTs, as long as we spend
> some love on the docs to show there is a very clear distinction
> between the two.

The distinction already exists. I agree that the documentation should
be improved to clarify how GRANT'd privileges are different from role
attributes (which is what our existing superuser, createrole, etc
options are).

> Users get confused between privs, role attributes and SETs that apply to roles.

Agreed.

> Introducing the new word "capability" needs to also have some clarity.
> Is that the same thing as "role attribute", or is that a 4th kind of
> thang?

At present, it's exactly the same as 'role attribute' and, for my part
at least, I was thinking it would remain the same. I believe the idea
was to migrate the terminology from 'role attribute' to 'capability' as
the latter better represents both the existing options and the new ones.

Thanks!

Stephen

All,

>
> I'm slightly mystified as to how including the word "online" helps
> here. It's unlikely that there will be an offline_backup permission,
> because if the system is off-line, SQL-level permissions are
> irrelevant.

After re-reading through this thread is seems like EXCLUSIVEBACKUP
(proposed by Magnus) seemed to be a potentially acceptable alternative.

----
Relevant post:
http://www.postgresql.org/message-id/CABUevEz7BZ0r85VUt4RVXX0JkpiH8hP8ToqzGVpuFL0KvcvBNg@mail.gmail.com
----
We need to separate the logical backups (pg_dump) from the physical ones
(start/stop+filesystem and pg_basebackup). We might also need to separate
the two different ways of doing physical backups.

Personalyl I think using the DUMP name makes that a lot more clear. Maybe
we need to avoid using BACKUP alone as well, to make sure it doesn't go the
other way - using BASEBACKUP and EXCLUSIVEBACKUP for those two different
ones perhaps?
----

----
Relevant post:
http://www.postgresql.org/message-id/20141231144610.GS3062@tamriel.snowman.net
----
I think I'm coming around to the EXCLUSIVEBACKUP option, per the
discussion with Magnus. I don't particularly like LOGBACKUP and don't
think it really makes sense, while PHYSBACKUP seems like it'd cover what
REPLICATION already does.
----

Thoughts?

-Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

Adam, all,

* Adam Brightwell (adam(dot)brightwell(at)crunchydatasolutions(dot)com) wrote:
> After re-reading through this thread is seems like EXCLUSIVEBACKUP
> (proposed by Magnus) seemed to be a potentially acceptable alternative.

I just chatted a bit on IRC w/ Magnus about this and I'm on-board with
his proposal to use EXCLUSIVEBACKUP, but there's a couple additional
things which need doing as part of that:

We need to actually define what an 'exclusive backup' is in the
documentation- it's not there today.

pg_start_backup/pg_stop_backup docs should be updated to refer to those
operations as relating to "on-line exclusive backup," same as
pg_is_in_backup() and pg_backup_start_time() do.

The docs for the EXCLUSIVEBACKUP attribute should, of course, refer to
where EXCLUSIVE BACKUP is defined.

Hopefully that wraps up the last of the debate around the naming of
these options and we can move forward, once the patch is updated to
reflect this and docs are written. Would be great to hear from anyone
who feels otherwise.

Regarding the pg_dump/read-only/etc discussion- that wasn't intended to
be part of this and I continue to feel it'd be best addressed on a new
thread specifically for that. Once we have this initial round of role
attribute additions settled, I'd be more than happy to support that
discussion and see what we can do to provide such a role.

Thanks!

Stephen

On Wed, Jan 21, 2015 at 11:27 AM, Adam Brightwell
<adam(dot)brightwell(at)crunchydatasolutions(dot)com> wrote:
> After re-reading through this thread is seems like EXCLUSIVEBACKUP (proposed
> by Magnus) seemed to be a potentially acceptable alternative.

So this would let you do pg_start_backup() and pg_stop_backup(), but
it wouldn't let you run pg_basebackup against the server?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Wed, Jan 21, 2015 at 11:27 AM, Adam Brightwell
> <adam(dot)brightwell(at)crunchydatasolutions(dot)com> wrote:
> > After re-reading through this thread is seems like EXCLUSIVEBACKUP (proposed
> > by Magnus) seemed to be a potentially acceptable alternative.
>
> So this would let you do pg_start_backup() and pg_stop_backup(), but
> it wouldn't let you run pg_basebackup against the server?

Right. We already have a role attribute which allows pg_basebackup
(replication). Also, with pg_basebackup / rolreplication, your role
is able to read the entire data directory from the server, that's not
the case with only rights to run pg_start/stop_backup.

In conjunction with enterprise backup solutions and SANs, which offer
similar controls where a generally unprivileged user can have a snapshot
of the system taken through the SAN interface, you can give users the
ability to run ad-hoc backups of the cluster without giving them
superuser-level access or replication-level access.

Even with simpler solutions, it means that the backup user doesn't
have to be able to run some superuser-level script against the database
to run the backup.

As for pg_basebackup itself, I agree that it's not exactly intuitive
that 'replication' is what grants you the right to run pg_basebackup..
Perhaps we could rename it or make an alias for it, or something along
those lines? I wasn't looking to do anything with it at this time, but
it would probably be good to improve it somehow, if you (or anyone) have
suggestions on how best to do so.

Thanks!

Stephen

On 2015-01-26 13:47:02 -0500, Stephen Frost wrote:
> * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> > On Wed, Jan 21, 2015 at 11:27 AM, Adam Brightwell
> > <adam(dot)brightwell(at)crunchydatasolutions(dot)com> wrote:
> > > After re-reading through this thread is seems like EXCLUSIVEBACKUP (proposed
> > > by Magnus) seemed to be a potentially acceptable alternative.
> >
> > So this would let you do pg_start_backup() and pg_stop_backup(), but
> > it wouldn't let you run pg_basebackup against the server?
>
> Right. We already have a role attribute which allows pg_basebackup
> (replication). Also, with pg_basebackup / rolreplication, your role
> is able to read the entire data directory from the server, that's not
> the case with only rights to run pg_start/stop_backup.
>
> In conjunction with enterprise backup solutions and SANs, which offer
> similar controls where a generally unprivileged user can have a snapshot
> of the system taken through the SAN interface, you can give users the
> ability to run ad-hoc backups of the cluster without giving them
> superuser-level access or replication-level access.

I'm sorry if this has already been discussed, but the thread is awfully
long already. But what's actually the point of having a separate
EXCLUSIVEBACKUP permission? Using it still requires full file system
access to the data directory, so the additional permissions granted by
replication aren't really relevant.

I don't think the comparison with the SAN snapshot functionality is apt:
The SAN solution itself will still run with full data access. Just
pressing the button for the snapshot requires less. You're comparing
that button to pg_start/stop_backup() - but that doesn't make sense,
because it's only useful if somebody actually takes the backup during
that time.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Mon, Jan 26, 2015 at 1:59 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2015-01-26 13:47:02 -0500, Stephen Frost wrote:
>> * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
>> > On Wed, Jan 21, 2015 at 11:27 AM, Adam Brightwell
>> > <adam(dot)brightwell(at)crunchydatasolutions(dot)com> wrote:
>> > > After re-reading through this thread is seems like EXCLUSIVEBACKUP (proposed
>> > > by Magnus) seemed to be a potentially acceptable alternative.
>> >
>> > So this would let you do pg_start_backup() and pg_stop_backup(), but
>> > it wouldn't let you run pg_basebackup against the server?
>>
>> Right. We already have a role attribute which allows pg_basebackup
>> (replication). Also, with pg_basebackup / rolreplication, your role
>> is able to read the entire data directory from the server, that's not
>> the case with only rights to run pg_start/stop_backup.
>>
>> In conjunction with enterprise backup solutions and SANs, which offer
>> similar controls where a generally unprivileged user can have a snapshot
>> of the system taken through the SAN interface, you can give users the
>> ability to run ad-hoc backups of the cluster without giving them
>> superuser-level access or replication-level access.
>
> I'm sorry if this has already been discussed, but the thread is awfully
> long already. But what's actually the point of having a separate
> EXCLUSIVEBACKUP permission? Using it still requires full file system
> access to the data directory, so the additional permissions granted by
> replication aren't really relevant.

That's not necessarily true. You could be able to run a command like
"san_snapshot $PGDATA" without necessarily having the permissions to
inspect the contents of the resulting snapshot. Of course somebody
should be doing that, but in accord with the principle of least
privilege, there's no reason that the account running the unattended
backup needs to have those rights.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

* Andres Freund (andres(at)2ndquadrant(dot)com) wrote:
> On 2015-01-26 13:47:02 -0500, Stephen Frost wrote:
> > Right. We already have a role attribute which allows pg_basebackup
> > (replication). Also, with pg_basebackup / rolreplication, your role
> > is able to read the entire data directory from the server, that's not
> > the case with only rights to run pg_start/stop_backup.
> >
> > In conjunction with enterprise backup solutions and SANs, which offer
> > similar controls where a generally unprivileged user can have a snapshot
> > of the system taken through the SAN interface, you can give users the
> > ability to run ad-hoc backups of the cluster without giving them
> > superuser-level access or replication-level access.
>
> I'm sorry if this has already been discussed, but the thread is awfully
> long already. But what's actually the point of having a separate
> EXCLUSIVEBACKUP permission? Using it still requires full file system
> access to the data directory, so the additional permissions granted by
> replication aren't really relevant.

I agree that it's a pretty long thread for what amount to a few
relatively straight-forward role attributes (at least, in my view).

> I don't think the comparison with the SAN snapshot functionality is apt:
> The SAN solution itself will still run with full data access. Just
> pressing the button for the snapshot requires less. You're comparing
> that button to pg_start/stop_backup() - but that doesn't make sense,
> because it's only useful if somebody actually takes the backup during
> that time.

I'm not following your logic here.. You're right- just pressing the
button to take a snapshot can be granted out to a lower-level user using
the SAN solution. That snapshot's useless unless the user can first run
pg_start_backup though (and subsequently run pg_stop_backup afterwards).
Clearly, XLOG archiving has to be set up already, but that would be set
up when the system is initially brought online.

This capability would be used in conjunction with the SAN snapshot
capability, it's not intended to be a comparison to what SANs offer.

Thanks!

Stephen

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Mon, Jan 26, 2015 at 1:59 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > On 2015-01-26 13:47:02 -0500, Stephen Frost wrote:
> >> Right. We already have a role attribute which allows pg_basebackup
> >> (replication). Also, with pg_basebackup / rolreplication, your role
> >> is able to read the entire data directory from the server, that's not
> >> the case with only rights to run pg_start/stop_backup.
> >>
> >> In conjunction with enterprise backup solutions and SANs, which offer
> >> similar controls where a generally unprivileged user can have a snapshot
> >> of the system taken through the SAN interface, you can give users the
> >> ability to run ad-hoc backups of the cluster without giving them
> >> superuser-level access or replication-level access.
> >
> > I'm sorry if this has already been discussed, but the thread is awfully
> > long already. But what's actually the point of having a separate
> > EXCLUSIVEBACKUP permission? Using it still requires full file system
> > access to the data directory, so the additional permissions granted by
> > replication aren't really relevant.
>
> That's not necessarily true. You could be able to run a command like
> "san_snapshot $PGDATA" without necessarily having the permissions to
> inspect the contents of the resulting snapshot. Of course somebody
> should be doing that, but in accord with the principle of least
> privilege, there's no reason that the account running the unattended
> backup needs to have those rights.

Right! You explained it more clearly than I did.

Thanks!

Stephen

On 2015-01-26 14:05:03 -0500, Stephen Frost wrote:
> This capability would be used in conjunction with the SAN snapshot
> capability, it's not intended to be a comparison to what SANs offer.

Oh, on a reread that's now clear. Many of those actually allow hooks to
be run when taking a snapshot, that'd probably be a better approach. But
I can now see the point.

Thanks,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

All,

I have attached and updated patch for review.

Thanks,
Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

Adam,

* Adam Brightwell (adam(dot)brightwell(at)crunchydatasolutions(dot)com) wrote:
> I have attached and updated patch for review.

Thanks! I've gone over this and made quite a few documentation and
comment updates, but not too much else, so I'm pretty happy with how
this is coming along. As mentioned elsewhere, this conflicts with the
GetUserId() to has_privs_of_role() cleanup, but as I anticipate handling
both this patch and that one, I'll find some way to manage. :)

Updated patch attached. Barring objections, I'll be moving forward with
this soonish. Would certainly appreciate any additional testing or
review that you (or anyone!) has time to provide.

Thanks again!

Stephen

Stephen Frost wrote:

> Thanks! I've gone over this and made quite a few documentation and
> comment updates, but not too much else, so I'm pretty happy with how
> this is coming along. As mentioned elsewhere, this conflicts with the
> GetUserId() to has_privs_of_role() cleanup, but as I anticipate handling
> both this patch and that one, I'll find some way to manage. :)
>
> Updated patch attached. Barring objections, I'll be moving forward with
> this soonish. Would certainly appreciate any additional testing or
> review that you (or anyone!) has time to provide.

I thought I saw a comment about using underscore to separate words in
privilege names, such as EXCLUSIVE_BACKUP rather than running it all
together. Was that idea discarded?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

* Alvaro Herrera (alvherre(at)2ndquadrant(dot)com) wrote:
> Stephen Frost wrote:
>
> > Thanks! I've gone over this and made quite a few documentation and
> > comment updates, but not too much else, so I'm pretty happy with how
> > this is coming along. As mentioned elsewhere, this conflicts with the
> > GetUserId() to has_privs_of_role() cleanup, but as I anticipate handling
> > both this patch and that one, I'll find some way to manage. :)
> >
> > Updated patch attached. Barring objections, I'll be moving forward with
> > this soonish. Would certainly appreciate any additional testing or
> > review that you (or anyone!) has time to provide.
>
> I thought I saw a comment about using underscore to separate words in
> privilege names, such as EXCLUSIVE_BACKUP rather than running it all
> together. Was that idea discarded?

Hrm, I'm not finding that on a quick look through the archives. Looking
at the other role options, we don't have any with underscores but we do
have a couple of cases where we uses spaces, eg:

CONNECTION LIMIT
VALID UNTIL

So, using EXCLUSIVE BACKUP would be similar to those, but we'd then need
to have the 'NO' version as 'NO EXCLUSIVE BACKUP', I'd think, and I'm
not sure we really want to go there.

On the other hand, we do run together the 'CREATE' options (CREATEDB,
CREATEROLE), and I've not heard anyone suggesting to change those.

I guess for my part, at least, having it run together as
'EXCLUSIVEBACKUP' seems fine, but this does make me realize that I need
to go add tab-completion for these new options. :)

Thanks!

Stephen

Alvaro,

I thought I saw a comment about using underscore to separate words in
> privilege names, such as EXCLUSIVE_BACKUP rather than running it all
> together. Was that idea discarded?
>

I'm not sure there was an actual discussion on the topic. Though, at one
point I had proposed it as one of the forms of this attribute. Personally,
I think it is easier to read with the underscore. But, ultimately, I
defaulted to no underscore to remain consistent with the other attributes,
such as CREATEDB and CREATEROLE.

Thanks,
Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

Adam Brightwell wrote:
> Alvaro,
>
> > I thought I saw a comment about using underscore to separate words
> > in privilege names, such as EXCLUSIVE_BACKUP rather than running it
> > all together. Was that idea discarded?
>
> I'm not sure there was an actual discussion on the topic. Though, at one
> point I had proposed it as one of the forms of this attribute. Personally,
> I think it is easier to read with the underscore. But, ultimately, I
> defaulted to no underscore to remain consistent with the other attributes,
> such as CREATEDB and CREATEROLE.

If we were choosing those names nowadays, would we choose CREATEDB at
all in the first place? I think we'd go for something more verbose,
probably CREATE_DATABASE. (CREATEROLE is not as old as CREATEDB, but my
bet is that it was modelled after CREATEUSER without considering the
whole readability topic too much.)