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 3d7d3bc9627771e86d88d1e24decf109d6a1e70e..d256524347e01ecc9788485a0defab075725be75 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8444,6 +8444,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,
@@ -8464,11 +8467,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.
         */
@@ -8770,6 +8768,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)
@@ -8802,11 +8803,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 b7950f77a6587dcfda2e58ff2e455afbdb261b18..f47840fa35733cba04aa59d85385ceb31dc6c070 100644 (file)
--- 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",