Обсуждение: Fix pg_rewind which can be run as root user
Hi all, I was just going through pg_rewind's code, and noticed the following pearl: /* * Don't allow pg_rewind to be run as root, to avoid overwriting the * ownership of files in the data directory. We need only check for root * -- any other user won't have sufficient permissions to modify files in * the data directory. */ #ifndef WIN32 if (geteuid() == 0) { fprintf(stderr, _("cannot be executed by \"root\"\n")); fprintf(stderr, _("You must run %s as the PostgreSQL superuser.\n"), progname); } #endif While that's nice to inform the user about the problem, that actually does not prevent pg_rewind to run as root. Attached is a patch, which needs a back-patch down to 9.5. Thanks, -- Michael
Вложения
On Mon, Apr 9, 2018 at 7:11 AM, Michael Paquier <michael@paquier.xyz> wrote:
Hi all,
I was just going through pg_rewind's code, and noticed the following
pearl:
/*
* Don't allow pg_rewind to be run as root, to avoid overwriting the
* ownership of files in the data directory. We need only check for root
* -- any other user won't have sufficient permissions to modify files in
* the data directory.
*/
#ifndef WIN32
if (geteuid() == 0)
{
fprintf(stderr, _("cannot be executed by \"root\"\n"));
fprintf(stderr, _("You must run %s as the PostgreSQL superuser.\n"),
progname);
}
#endif
While that's nice to inform the user about the problem, that actually
does not prevent pg_rewind to run as root. Attached is a patch, which
needs a back-patch down to 9.5.
Seems simple enough and the right hting to do, but I wonder if we should really backpatch it. Yes, the behaviour is not great now, but there is also a non-zero risk of breaking peoples automated failover scripts of we backpatch it, isn't it?
On Mon, Apr 9, 2018 at 12:23 PM, Magnus Hagander <magnus@hagander.net> wrote: > Seems simple enough and the right hting to do, but I wonder if we should > really backpatch it. Yes, the behaviour is not great now, but there is also > a non-zero risk of breaking peoples automated failover scripts of we > backpatch it, isn't it? +1 to committing to master as a documented behavioral change. -- Peter Geoghegan
Magnus Hagander <magnus@hagander.net> writes: > Seems simple enough and the right hting to do, but I wonder if we should > really backpatch it. Yes, the behaviour is not great now, but there is also > a non-zero risk of breaking peoples automated failover scripts of we > backpatch it, isn't it? Yeah, I'd vote against backpatching. This doesn't seem like an essential fix. regards, tom lane
On Mon, Apr 9, 2018 at 9:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
> Seems simple enough and the right hting to do, but I wonder if we should
> really backpatch it. Yes, the behaviour is not great now, but there is also
> a non-zero risk of breaking peoples automated failover scripts of we
> backpatch it, isn't it?
Yeah, I'd vote against backpatching. This doesn't seem like an essential
fix.
Applied, and pushed this way.
On Mon, Apr 09, 2018 at 09:36:40PM +0200, Magnus Hagander wrote: > Applied, and pushed this way. OK, thanks for the commit. -- Michael