Move permissions check from do_pg_start_backup to pg_start_backup

And the same for do_pg_stop_backup. The code in do_pg_* is not allowed
to access the catalogs. For manual base backups, the permissions
check can be handled in the calling function, and for streaming
base backups only users with the required permissions can get past
the authentication step in the first place.

Reported by Antonin Houska, diagnosed by Andres Freund

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 190d86b752990ab6b1f5d1af3992c6cb20e9989b..1e8bb8bbc69c9bec7225be25a99be1f70765cac0 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9412,6 +9412,9 @@ issue_xlog_fsync(int fd, uint32 log, uint32 seg)
  *
  * 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, char **labelfile)
@@ -9431,11 +9434,6 @@ do_pg_start_backup(const char *backupidstr, bool fast, char **labelfile)
 
        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.
         */
@@ -9731,6 +9729,9 @@ pg_start_backup_callback(int code, Datum arg)
 
  * If labelfile is NULL, this stops an exclusive backup. Otherwise this stops
  * the non-exclusive backup specified by 'labelfile'.
+ *
+ * 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)
@@ -9761,11 +9762,6 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive)
 
        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 f3c8a09c2aa4826549a3e06cfc29e495d72c2ceb..b27bf76a22e99df52716d1e4c5c736022b27f866 100644 (file)
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -54,6 +54,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);
 
        snprintf(startxlogstr, sizeof(startxlogstr), "%X/%X",
@@ -80,6 +85,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);
 
        snprintf(stopxlogstr, sizeof(stopxlogstr), "%X/%X",