Обсуждение: pgsql: Prevent running pg_basebackup as root
Prevent running pg_basebackup as root Similarly to pg_upgrade, pg_ctl and initdb, a root user is able to use --version and --help, but cannot execute the actual operation to avoid the creation of files with permissions incompatible with the postmaster. This is a behavior change, so not back-patching is done. Author: Ian Barwick Discussion: https://postgr.es/m/CABvVfJVqOdD2neLkYdygdOHvbWz_5K_iWiqY+psMfA=FeAa3qQ@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/7bae0ad9fcb76b28410571dc71edfdc3175c4a02 Modified Files -------------- src/bin/pg_basebackup/pg_basebackup.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
Greetings, * Michael Paquier (michael@paquier.xyz) wrote: > Prevent running pg_basebackup as root > > Similarly to pg_upgrade, pg_ctl and initdb, a root user is able to use > --version and --help, but cannot execute the actual operation to avoid > the creation of files with permissions incompatible with the > postmaster. > > This is a behavior change, so not back-patching is done. While it's maybe not ideal, surely there isn't an actual issue if pg_basebackup is run as root with -Ft, is there..? There's possibly something to be said about the fact that we hard-code the username/groupname in the tar file too (interestingly, we actually do pass through the uid/gid..)- perhaps we should actually be passing the username/groupname through, but if we did do something like that then having pg_basebackup running as root would be necessary if we want to preserve the file ownership. 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".. Thanks, Stephen
Вложения
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. 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. -- Michael
Вложения
On Thu, Feb 6, 2020 at 8:04 AM Michael Paquier <michael@paquier.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) And pg_basebackup is about taking backups, not restores :) > 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. 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. So *very* strong objection to makeing -Fp and -Ft behave differently in this regard. 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... -- Magnus Hagander Me: https://www.hagander.net/ Work: https://www.redpill-linpro.com/
Greetings, * Magnus Hagander (magnus@hagander.net) wrote: > On Thu, Feb 6, 2020 at 8:04 AM Michael Paquier <michael@paquier.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
Вложения
On Thu, Feb 06, 2020 at 09:44:07AM -0500, Stephen Frost wrote: > * Magnus Hagander (magnus@hagander.net) wrote: >> On Thu, Feb 6, 2020 at 8:04 AM Michael Paquier <michael@paquier.xyz> wrote: >>> 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. Hmm. I don't think that you are completely correct here either as it depends on if the OS user "postgres" exists or not. As mentioned in https://www.gnu.org/software/tar/manual/tar.html#SEC138, if the user name cannot be found in /etc/passwd, then tar switches to the user ID (if one does not have any user or group named "postgres", then the files are untar'ed with the same user and group as the one running the cluster and that's to the UID and GID set by tarCreateHeader, as you say). I think that it is a problem to not have more documentation on the matter (now there is just a small mention in the base backup restore about being sure to have the proper permissions). And it may be interesting to add into pg_basebackup options to enforce the user and/or group similarly to what tar does with --owner and --group? >> 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. OK, done that part as of dcddc3f. -- Michael
Вложения
Hi, On 2020-02-06 13:02:07 +0100, Magnus Hagander wrote: > I agree with Stephen that this seems to be misguided, and my vote is > to revert. +1. I honestly don't think we should increase the number of "root disallowed" tools unless actually necessary. Maybe that's looking too far into the future, but I'd like to see improvements to pg_basebackup that make it integrate with root requiring tooling, to do more efficient base backups. E.g. having pg_basebackup handle start/stop backup and WAL handling, but do the actual backup of the data via a snapshot mechanism (yes, one needs start/stop backup in the general case, for multiple FSs), would be nice. Btw, I think it's good form in a discussion like this to CC the original author. I'll also add a reference to this discussion from the -hackers thread. Greetings, Andres Freund
On 2020/02/07 11:07, Andres Freund wrote: > Hi, > > On 2020-02-06 13:02:07 +0100, Magnus Hagander wrote: >> I agree with Stephen that this seems to be misguided, and my vote is >> to revert. > > +1. I honestly don't think we should increase the number of "root > disallowed" tools unless actually necessary. > > Maybe that's looking too far into the future, but I'd like to see > improvements to pg_basebackup that make it integrate with root requiring > tooling, to do more efficient base backups. E.g. having pg_basebackup > handle start/stop backup and WAL handling, but do the actual backup of > the data via a snapshot mechanism (yes, one needs start/stop backup in > the general case, for multiple FSs), would be nice. > > Btw, I think it's good form in a discussion like this to CC the original > author. I'll also add a reference to this discussion from the -hackers > thread. Thanks for the notification. Points raised upthread seem reasonable enough; to be honest I was expecting this patch to hang around a bit longer anway, because (as so often) there's some aspect which wouldn't have occurred to me. Regards Ian Barwick -- Ian Barwick https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Greetings, * Michael Paquier (michael@paquier.xyz) wrote: > On Thu, Feb 06, 2020 at 09:44:07AM -0500, Stephen Frost wrote: > > 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. > > Hmm. I don't think that you are completely correct here either as it > depends on if the OS user "postgres" exists or not. Yes, I do know what happens if the named user doesn't exist, but in the general case, where the 'postgres' user does exist, they'll get owned by 'postgres'. > As mentioned in > https://www.gnu.org/software/tar/manual/tar.html#SEC138, if the user > name cannot be found in /etc/passwd, then tar switches to the user ID > (if one does not have any user or group named "postgres", then the > files are untar'ed with the same user and group as the one running the > cluster and that's to the UID and GID set by tarCreateHeader, as you > say). I think that it is a problem to not have more documentation on > the matter (now there is just a small mention in the base backup > restore about being sure to have the proper permissions). And it may > be interesting to add into pg_basebackup options to enforce the user > and/or group similarly to what tar does with --owner and --group? Yes, I agree with improving the documentation and with adding such options. > >> 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. > > OK, done that part as of dcddc3f. Great, thanks! Stephen