Re: Additional role attributes && superuser review

* Magnus Hagander (magnus@hagander.net) wrote:
> On Wed, Dec 31, 2014 at 3:08 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > * Magnus Hagander (magnus@hagander.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..

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

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

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

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