From 61248faddb4ec9d1bcf4e4eca3c1315b718150f0 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Fri, 27 Jan 2023 14:52:57 -0800 Subject: [PATCH v4 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 | 5 +++-- src/backend/storage/ipc/procarray.c | 4 +++- src/backend/storage/ipc/signalfuncs.c | 16 ++++++++++++---- src/backend/tcop/utility.c | 4 +++- src/backend/utils/init/miscinit.c | 4 ++++ src/backend/utils/init/postinit.c | 8 +++++--- 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, 84 insertions(+), 36 deletions(-) diff --git a/contrib/file_fdw/expected/file_fdw.out b/contrib/file_fdw/expected/file_fdw.out index 36d76ba26c..7e57ad2990 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 specify the "filename" option of a file_fdw foreign table +DETAIL: You must have privileges of the "pg_read_server_files" role. 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..696d9e5718 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 specify the \"filename\" option of a file_fdw foreign table"), + errdetail("You must have privileges of the \"%s\" role.", + "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 specify the \"program\" option of a file_fdw foreign table"), + errdetail("You must have privileges of the \"%s\" role.", + "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..d102ff066b 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 REPLICATION privilege. 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 REPLICATION privilege. 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 REPLICATION privilege. 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 REPLICATION privilege. 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..de89df190f 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 have 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 25c50d66fd..2a39ce0567 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 %s privilege.", + "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 %s privilege.", + "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 %s on role \"%s\".", + "ADMIN OPTION", + GetUserNameFromId(address.objectId, + true)))); } break; case OBJECT_TSPARSER: diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index e34f583ea7..e557eb17b6 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 have 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 have 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 have 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..e4df06a05d 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -1140,10 +1140,11 @@ 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 %s privilege.", "REPLICATION"))); } /* diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 4340bf9641..95967f8dfa 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 have privileges of the role whose process is being terminated or have 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..7e0f0aef73 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 %s privilege 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 have privileges of the role whose query is being canceled or have 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 %s privilege 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 have privileges of the role whose process is being terminated or have 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..fe77f260c5 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -950,7 +950,9 @@ 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 do CHECKPOINT"), + errdetail("You must have privileges of the \"%s\" role.", + "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 0cdc1e11a3..3a98fafaeb 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..808c24b480 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -945,7 +945,8 @@ InitPostgres(const char *in_dbname, Oid dboid, 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 with privileges of \"%s\"", + "pg_use_reserved_connections"))); } /* Check replication permissions needed for walsender processes. */ @@ -953,10 +954,11 @@ 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 walsender"), + errdetail("You must have %s privilege.", "REPLICATION"))); } /* diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 978b385568..07d97ad75e 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -4190,8 +4190,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 have privileges of the \"%s\" role.", + "pg_read_all_settings"))); switch (record->vartype) { @@ -4236,8 +4237,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 have privileges of the \"%s\" role.", + "pg_read_all_settings"))); switch (record->vartype) { @@ -5242,8 +5244,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 have privileges of the \"%s\" role.", + "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..abb0b2fa0f 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 CREATEROLE privilege. 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..6f1fc210a7 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 have privileges of the "pg_read_all_settings" role. 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 2a7d14dba9..d9620fb8d7 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 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 CREATEROLE privilege and ADMIN OPTION on role "regress_role_normal". -- 2.25.1