Re: Additional role attributes && superuser review

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/