On Tue, Dec 24, 2013 at 1:24 PM, Andres Freund <[email protected]>wrote:
> On 2013-12-23 18:28:51 +0100, Magnus Hagander wrote:
> > On Dec 19, 2013 12:06 AM, "Andres Freund" <[email protected]>
> wrote:
> > >
> > > Hi Magnus,
> > >
> > > It looks to me like the path to do_pg_start_backup() outside of a
> > > transaction context comes from your initial commit of the base backup
> > > facility.
> > > The problem is that you're not allowed to do anything leading to a
> > > syscache/catcache lookup in those contexts.
> >
> > I think that may have come with the addition of the replication privilege
> > actually but that doesn't change the fact that yes, it appears broken..
>
> There was a if (!superuser()) check there before as well, that has the
> same dangers.
>
>
I think the correct fix is to move the security check outside of
do_pg_start_backup() and leave it to the caller. That means
pg_start_backup() for a manual backup. And for a streaming base backup the
check has already been made - you can't get through the authentication step
if you don't have the required permissions.
Does the attached seem right to you?
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 43a0416..bf1174e 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9727,6 +9727,9 @@ XLogFileNameP(TimeLineID tli, XLogSegNo segno)
*
* Every successfully started non-exclusive backup must be stopped by calling
* do_pg_stop_backup() or do_pg_abort_backup().
+ *
+ * It is the responsibility of the caller of this function to verify the
+ * permissions of the calling user!
*/
XLogRecPtr
do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
@@ -9747,11 +9750,6 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
backup_started_in_recovery = RecoveryInProgress();
- if (!superuser() && !has_rolreplication(GetUserId()))
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("must be superuser or replication role to run a backup")));
-
/*
* Currently only non-exclusive backup can be taken during recovery.
*/
@@ -10053,6 +10051,9 @@ pg_start_backup_callback(int code, Datum arg)
*
* Returns the last WAL position that must be present to restore from this
* backup, and the corresponding timeline ID in *stoptli_p.
+ *
+ * It is the responsibility of the caller of this function to verify the
+ * permissions of the calling user!
*/
XLogRecPtr
do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
@@ -10085,11 +10086,6 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
backup_started_in_recovery = RecoveryInProgress();
- if (!superuser() && !has_rolreplication(GetUserId()))
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser or replication role to run a backup"))));
-
/*
* Currently only non-exclusive backup can be taken during recovery.
*/
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index c421a59..4be31b4 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -56,6 +56,11 @@ pg_start_backup(PG_FUNCTION_ARGS)
backupidstr = text_to_cstring(backupid);
+ if (!superuser() && !has_rolreplication(GetUserId()))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser or replication role to run a backup")));
+
startpoint = do_pg_start_backup(backupidstr, fast, NULL, NULL);
snprintf(startxlogstr, sizeof(startxlogstr), "%X/%X",
@@ -82,6 +87,11 @@ pg_stop_backup(PG_FUNCTION_ARGS)
XLogRecPtr stoppoint;
char stopxlogstr[MAXFNAMELEN];
+ if (!superuser() && !has_rolreplication(GetUserId()))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("must be superuser or replication role to run a backup"))));
+
stoppoint = do_pg_stop_backup(NULL, true, NULL);
snprintf(stopxlogstr, sizeof(stopxlogstr), "%X/%X",
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
