Re: pgsql: Prevent running pg_basebackup as root

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-committers <pgsql-committers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgsql: Prevent running pg_basebackup as root
Date: 2020-02-06 14:44:07
Message-ID: 20200206144407.GF3195@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Greetings,

* Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> On Thu, Feb 6, 2020 at 8:04 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> >
> > On Wed, Feb 05, 2020 at 12:22:59PM -0500, Stephen Frost wrote:
> > > In any case, sorry for not responding on this sooner (was traveling for
> > > FOSDEM and such), but I'm not really convinced this is something we want
> > > and it certainly breaks at least somewhat reasonable use-cases when you
> > > think about using pg_basebackup with -Ft. In that vein, this change is
> > > kinda like saying "you can't run pg_dump as root"..
> >
> > It seems to me that this is entirely different than the case of
> > pg_dump, as it is possible to restore a dump even as root, something
> > that cannot happen with physical backups without an extra chmod -R.
>
> I don't see how that's relevant? And yes, you can restore physical
> backups this way too, if the userids match. (though see Stephens
> comment about the username, but that's independent of this issue)

Right.

> And pg_basebackup is about taking backups, not restores :)

Yes- one of the downsides of pg_basebackup is that it doesn't really do
much for you when it comes to restores, in fact.. Something that will
have to change if it starts doing incrementals of some kind. That's
mostly orthogonal to this discussion though.

> > You have a point with -Ft as untaring the tarballs from a base backup
> > taken with pg_basebackup -Ft used by root generates files owned by the
> > original user. -Fp enforces the files to be owned by the user taking
> > the backup, which makes the most sense, so for consistency with the
> > other tools preventing root to run pg_basebackup makes sense to me
> > with -Fp. Any thoughts from others to restrict the tool with -Fp but
> > not with -Ft? The argument of consistency mattered for me first for
> > both formats.

Erm- no, with -Ft + untar-as-root they get owned by "postgres", NOT the
original user. That's what I was pointing out up-thread (since it seems
to be confusing- and clearly not always well understood..) and it's an
issue imv, but it's independent of this, so probably deserves its own
thread if someone wants to do something about that.

Having -Fp run-as-root result in the files being owned by root isn't
good and I agree that's unfortunate and it would be good to fix it, but
preventing pg_basebackup from ever being run as root isn't a good
solution to that issue.

> I think having -Fp and -Ft consistent is a lot more important than
> being consistent with other tools that aren't really that closely
> related. And it's already inconsistent against probably the most
> related command, being pg_dump.

Yeah, I agree on consistency here being important too, and that pg_dump
is a closer command to be thinking about than initdb and friends.

> So *very* strong objection to makeing -Fp and -Ft behave differently
> in this regard.

What we aren't consistent about today is what happens when you do:

- Backup as root with -Ft
- Untar results as root

- Backup as root with -Fp

and that really seems less than ideal, but I don't think the answer is
"don't allow backing up as root".

> I agree with Stephen that this seems to be misguided, and my vote is
> to revert. I would've also objected had you given more than 2 days
> warning before committing, and it happened to be during FOSDEM. I saw
> the original email which clearly said it'd be in the March commitfest,
> so I figured I'd have time...

Yeah, I also agree with reverting this change. Even if we can come to
something we all agree on, I'm pretty confident it's not going to be
exactly this patch, so let's back it out for now and discuss it further
on the -hackers thread.

Thanks,

Stephen

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Fujii Masao 2020-02-06 16:11:22 pgsql: Add note about access permission checks by inherited TRUNCATE an
Previous Message Magnus Hagander 2020-02-06 12:02:07 Re: pgsql: Prevent running pg_basebackup as root

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2020-02-06 14:44:26 Re: ALTER tbl rewrite loses CLUSTER ON index
Previous Message Fujii Masao 2020-02-06 14:23:42 Re: bad logging around broken restore_command