From 4664631c96a303d2066380966d31df113e1fea06 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Fri, 27 Jan 2023 14:52:57 -0800 Subject: [PATCH v6 2/2] Improve more insufficient-privileges error messages. --- contrib/file_fdw/expected/file_fdw.out | 3 ++- contrib/file_fdw/file_fdw.c | 8 ++++++-- contrib/test_decoding/expected/permissions.out | 12 ++++++++---- src/backend/backup/basebackup_server.c | 4 +++- src/backend/catalog/objectaddress.c | 16 +++++++++++----- src/backend/commands/copy.c | 12 +++++++++--- src/backend/replication/slot.c | 6 ++++-- src/backend/storage/ipc/procarray.c | 4 +++- src/backend/storage/ipc/signalfuncs.c | 16 ++++++++++++---- src/backend/tcop/utility.c | 5 ++++- src/backend/utils/init/miscinit.c | 4 ++++ src/backend/utils/init/postinit.c | 12 ++++++++---- src/backend/utils/misc/guc.c | 15 +++++++++------ .../dummy_seclabel/expected/dummy_seclabel.out | 3 ++- .../modules/unsafe_tests/expected/rolenames.out | 3 ++- src/test/regress/expected/create_role.out | 3 ++- 16 files changed, 89 insertions(+), 37 deletions(-) diff --git a/contrib/file_fdw/expected/file_fdw.out b/contrib/file_fdw/expected/file_fdw.out index 36d76ba26c..ea7afd6fe8 100644 --- a/contrib/file_fdw/expected/file_fdw.out +++ b/contrib/file_fdw/expected/file_fdw.out @@ -474,7 +474,8 @@ ALTER FOREIGN TABLE agg_text OWNER TO regress_file_fdw_user; ALTER FOREIGN TABLE agg_text OPTIONS (SET format 'text'); SET ROLE regress_file_fdw_user; ALTER FOREIGN TABLE agg_text OPTIONS (SET format 'text'); -ERROR: only superuser or a role with privileges of the pg_read_server_files role may specify the filename option of a file_fdw foreign table +ERROR: permission denied to set the "filename" option of a file_fdw foreign table +DETAIL: You must inherit privileges of the "pg_read_server_files" role to set this option. SET ROLE regress_file_fdw_superuser; -- cleanup RESET ROLE; diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c index 8ccc167548..8553d93b3f 100644 --- a/contrib/file_fdw/file_fdw.c +++ b/contrib/file_fdw/file_fdw.c @@ -278,13 +278,17 @@ file_fdw_validator(PG_FUNCTION_ARGS) !has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("only superuser or a role with privileges of the pg_read_server_files role may specify the filename option of a file_fdw foreign table"))); + errmsg("permission denied to set the \"filename\" option of a file_fdw foreign table"), + errdetail("You must inherit privileges of the \"%s\" role to set this option.", + "pg_read_server_files"))); if (strcmp(def->defname, "program") == 0 && !has_privs_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("only superuser or a role with privileges of the pg_execute_server_program role may specify the program option of a file_fdw foreign table"))); + errmsg("permission denied to set the \"program\" option of a file_fdw foreign table"), + errdetail("You must inherit privileges of the \"%s\" role to set this option.", + "pg_execute_server_program"))); filename = defGetString(def); } diff --git a/contrib/test_decoding/expected/permissions.out b/contrib/test_decoding/expected/permissions.out index ed97f81dda..b45f5cf0df 100644 --- a/contrib/test_decoding/expected/permissions.out +++ b/contrib/test_decoding/expected/permissions.out @@ -54,13 +54,16 @@ RESET ROLE; -- plain user *can't* can control replication SET ROLE regress_lr_normal; SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding'); -ERROR: must be superuser or replication role to use replication slots +ERROR: permission denied to use replication slots +DETAIL: You must have the REPLICATION attribute to use replication slots. INSERT INTO lr_test VALUES('lr_superuser_init'); ERROR: permission denied for table lr_test SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); -ERROR: must be superuser or replication role to use replication slots +ERROR: permission denied to use replication slots +DETAIL: You must have the REPLICATION attribute to use replication slots. SELECT pg_drop_replication_slot('regression_slot'); -ERROR: must be superuser or replication role to use replication slots +ERROR: permission denied to use replication slots +DETAIL: You must have the REPLICATION attribute to use replication slots. RESET ROLE; -- replication users can drop superuser created slots SET ROLE regress_lr_superuser; @@ -90,7 +93,8 @@ SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_d RESET ROLE; SET ROLE regress_lr_normal; SELECT pg_drop_replication_slot('regression_slot'); -ERROR: must be superuser or replication role to use replication slots +ERROR: permission denied to use replication slots +DETAIL: You must have the REPLICATION attribute to use replication slots. RESET ROLE; -- all users can see existing slots SET ROLE regress_lr_superuser; diff --git a/src/backend/backup/basebackup_server.c b/src/backend/backup/basebackup_server.c index 0258d7a03b..aac64408d1 100644 --- a/src/backend/backup/basebackup_server.c +++ b/src/backend/backup/basebackup_server.c @@ -72,7 +72,9 @@ bbsink_server_new(bbsink *next, char *pathname) if (!has_privs_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser or a role with privileges of the pg_write_server_files role to create backup stored on server"))); + errmsg("permission denied to create backup stored on server"), + errdetail("You must inherit privileges of the \"%s\" role.", + "pg_write_server_files"))); CommitTransactionCommand(); /* diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 2f688166e1..c1ab4ea161 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -2547,20 +2547,26 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address, if (!superuser_arg(roleid)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser"))); + errmsg("permission denied"), + errdetail("You must have the %s attribute.", + "SUPERUSER"))); } else { if (!has_createrole_privilege(roleid)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must have CREATEROLE privilege"))); + errmsg("permission denied"), + errdetail("You must have the %s attribute.", + "CREATEROLE"))); if (!is_admin_of_role(roleid, address.objectId)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must have admin option on role \"%s\"", - GetUserNameFromId(address.objectId, - true)))); + errmsg("permission denied"), + errdetail("You must have the %s option on role \"%s\".", + "ADMIN", + GetUserNameFromId(address.objectId, + true)))); } break; case OBJECT_TSPARSER: diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index e34f583ea7..388cfd7cae 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -83,7 +83,9 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, if (!has_privs_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser or have privileges of the pg_execute_server_program role to COPY to or from an external program"), + errmsg("permission denied to COPY to or from an external program"), + errdetail("You must inherit privileges of the \"%s\" role.", + "pg_execute_server_program"), errhint("Anyone can COPY to stdout or from stdin. " "psql's \\copy command also works for anyone."))); } @@ -92,14 +94,18 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, if (is_from && !has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser or have privileges of the pg_read_server_files role to COPY from a file"), + errmsg("permission denied to COPY from a file"), + errdetail("You must inherit privileges of the \"%s\" role.", + "pg_read_server_files"), errhint("Anyone can COPY to stdout or from stdin. " "psql's \\copy command also works for anyone."))); if (!is_from && !has_privs_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser or have privileges of the pg_write_server_files role to COPY to a file"), + errmsg("permission denied to COPY to a file"), + errdetail("You must inherit privileges of the \"%s\" role.", + "pg_write_server_files"), errhint("Anyone can COPY to stdout or from stdin. " "psql's \\copy command also works for anyone."))); } diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index f286918f69..51adcd8418 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -1140,10 +1140,12 @@ CheckSlotRequirements(void) void CheckSlotPermissions(void) { - if (!superuser() && !has_rolreplication(GetUserId())) + if (!has_rolreplication(GetUserId())) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser or replication role to use replication slots"))); + errmsg("permission denied to use replication slots"), + errdetail("You must have the %s attribute to use replication slots.", + "REPLICATION"))); } /* diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index a7071b2fce..09a69f2c76 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -3860,7 +3860,9 @@ TerminateOtherDBBackends(Oid databaseId) !has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_BACKEND)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be a member of the role whose process is being terminated or member of pg_signal_backend"))); + errmsg("permission denied to terminate process"), + errdetail("You must inherit privileges of the role whose process is being terminated or inherit privileges of the \"%s\" role.", + "pg_signal_backend"))); } } diff --git a/src/backend/storage/ipc/signalfuncs.c b/src/backend/storage/ipc/signalfuncs.c index bc93ab5b52..c1be67e7d0 100644 --- a/src/backend/storage/ipc/signalfuncs.c +++ b/src/backend/storage/ipc/signalfuncs.c @@ -121,12 +121,16 @@ pg_cancel_backend(PG_FUNCTION_ARGS) if (r == SIGNAL_BACKEND_NOSUPERUSER) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be a superuser to cancel superuser query"))); + errmsg("permission denied to cancel query"), + errdetail("You must have the %s attribute to cancel queries of roles with %s.", + "SUPERUSER", "SUPERUSER"))); if (r == SIGNAL_BACKEND_NOPERMISSION) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be a member of the role whose query is being canceled or member of pg_signal_backend"))); + errmsg("permission denied to cancel query"), + errdetail("You must inherit privileges of the role whose query is being canceled or inherit privileges of the \"%s\" role.", + "pg_signal_backend"))); PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS); } @@ -223,12 +227,16 @@ pg_terminate_backend(PG_FUNCTION_ARGS) if (r == SIGNAL_BACKEND_NOSUPERUSER) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be a superuser to terminate superuser process"))); + errmsg("permission denied to terminate process"), + errdetail("You must have the %s attribute to terminate processes of roles with %s.", + "SUPERUSER", "SUPERUSER"))); if (r == SIGNAL_BACKEND_NOPERMISSION) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be a member of the role whose process is being terminated or member of pg_signal_backend"))); + errmsg("permission denied to terminate process"), + errdetail("You must inherit privileges of the role whose process is being terminated or inherit privileges of the \"%s\" role.", + "pg_signal_backend"))); /* Wait only on success and if actually requested */ if (r == SIGNAL_BACKEND_SUCCESS && timeout > 0) diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index c7d9d96b45..3fd9cfb842 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -950,7 +950,10 @@ standard_ProcessUtility(PlannedStmt *pstmt, if (!has_privs_of_role(GetUserId(), ROLE_PG_CHECKPOINT)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser or have privileges of pg_checkpoint to do CHECKPOINT"))); + errmsg("permission denied to execute %s command", + "CHECKPOINT"), + errdetail("You must inherit privileges of the \"%s\" role to execute this command.", + "pg_checkpoint"))); RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_WAIT | (RecoveryInProgress() ? 0 : CHECKPOINT_FORCE)); diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index 59532bbd80..b0cfbc742b 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -701,6 +701,10 @@ has_rolreplication(Oid roleid) bool result = false; HeapTuple utup; + /* Superusers bypass all permission checking. */ + if (superuser_arg(roleid)) + return true; + utup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid)); if (HeapTupleIsValid(utup)) { diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 2f07ca7a0e..2b103586ab 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -940,12 +940,14 @@ InitPostgres(const char *in_dbname, Oid dboid, if (nfree < SuperuserReservedConnections) ereport(FATAL, (errcode(ERRCODE_TOO_MANY_CONNECTIONS), - errmsg("remaining connection slots are reserved for superusers"))); + errmsg("remaining connection slots are reserved for roles with %s", + "SUPERUSER"))); if (!has_privs_of_role(GetUserId(), ROLE_PG_USE_RESERVED_CONNECTIONS)) ereport(FATAL, (errcode(ERRCODE_TOO_MANY_CONNECTIONS), - errmsg("remaining connection slots are reserved for roles with privileges of pg_use_reserved_connections"))); + errmsg("remaining connection slots are reserved for roles that inherit privileges of the \"%s\" role", + "pg_use_reserved_connections"))); } /* Check replication permissions needed for walsender processes. */ @@ -953,10 +955,12 @@ InitPostgres(const char *in_dbname, Oid dboid, { Assert(!bootstrap); - if (!superuser() && !has_rolreplication(GetUserId())) + if (!has_rolreplication(GetUserId())) ereport(FATAL, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser or replication role to start walsender"))); + errmsg("permission denied to start WAL sender"), + errdetail("You must have the %s attribute to start a WAL sender process.", + "REPLICATION"))); } /* diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 51e07d5582..7805a6015b 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -4209,8 +4209,9 @@ GetConfigOption(const char *name, bool missing_ok, bool restrict_privileged) !ConfigOptionIsVisible(record)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser or have privileges of pg_read_all_settings to examine \"%s\"", - name))); + errmsg("permission denied to examine \"%s\"", name), + errdetail("You must inherit privileges of the \"%s\" role to examine this parameter.", + "pg_read_all_settings"))); switch (record->vartype) { @@ -4255,8 +4256,9 @@ GetConfigOptionResetString(const char *name) if (!ConfigOptionIsVisible(record)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser or have privileges of pg_read_all_settings to examine \"%s\"", - name))); + errmsg("permission denied to examine \"%s\"", name), + errdetail("You must inherit privileges of the \"%s\" role to examine this parameter.", + "pg_read_all_settings"))); switch (record->vartype) { @@ -5261,8 +5263,9 @@ GetConfigOptionByName(const char *name, const char **varname, bool missing_ok) if (!ConfigOptionIsVisible(record)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser or have privileges of pg_read_all_settings to examine \"%s\"", - name))); + errmsg("permission denied to examine \"%s\"", name), + errdetail("You must inherit privileges of the \"%s\" role to examine this parameter.", + "pg_read_all_settings"))); if (varname) *varname = record->name; diff --git a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out index c57d4fd2df..18f8db0e39 100644 --- a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out +++ b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out @@ -59,7 +59,8 @@ SECURITY LABEL ON ROLE regress_dummy_seclabel_user4 IS 'unclassified'; -- fail ( ERROR: role "regress_dummy_seclabel_user4" does not exist SET SESSION AUTHORIZATION regress_dummy_seclabel_user2; SECURITY LABEL ON ROLE regress_dummy_seclabel_user2 IS 'unclassified'; -- fail (not privileged) -ERROR: must have CREATEROLE privilege +ERROR: permission denied +DETAIL: You must have the CREATEROLE attribute. RESET SESSION AUTHORIZATION; -- -- Test for various types of object diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out index 88b1ff843b..6e55850705 100644 --- a/src/test/modules/unsafe_tests/expected/rolenames.out +++ b/src/test/modules/unsafe_tests/expected/rolenames.out @@ -1077,7 +1077,8 @@ SHOW session_preload_libraries; SET SESSION AUTHORIZATION regress_role_nopriv; -- fails with role not member of pg_read_all_settings SHOW session_preload_libraries; -ERROR: must be superuser or have privileges of pg_read_all_settings to examine "session_preload_libraries" +ERROR: permission denied to examine "session_preload_libraries" +DETAIL: You must inherit privileges of the "pg_read_all_settings" role to examine this parameter. RESET SESSION AUTHORIZATION; ERROR: current transaction is aborted, commands ignored until end of transaction block ROLLBACK; diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out index 7157d4c589..e72eb91624 100644 --- a/src/test/regress/expected/create_role.out +++ b/src/test/regress/expected/create_role.out @@ -106,7 +106,8 @@ ALTER ROLE regress_hasprivs RENAME TO regress_tenant; ALTER ROLE regress_tenant NOINHERIT NOLOGIN CONNECTION LIMIT 7; -- fail, we should be unable to modify a role we did not create COMMENT ON ROLE regress_role_normal IS 'some comment'; -ERROR: must have admin option on role "regress_role_normal" +ERROR: permission denied +DETAIL: You must have the ADMIN option on role "regress_role_normal". ALTER ROLE regress_role_normal RENAME TO regress_role_abnormal; ERROR: permission denied to rename role DETAIL: You must have the CREATEROLE attribute and the ADMIN option on role "regress_role_normal". -- 2.25.1