From eb7cd95567407d61e11bd357d2e0e42cc01cc324 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 2 Dec 2022 08:54:18 -0500 Subject: [PATCH v3 4/4] Add new GUC createrole_self_grant. Can be set to the empty string, or to either or both of "set" or "inherit". If set to a non-empty value, a non-superuser who creates a role (necessarily by relying up the CREATEROLE privilege) will grant that role back to themselves with the specified options. This isn't a security feature, because the grant that this feature triggers can also be performed explicitly. Instead, it's a user experience feature. A superuser would necessarily inherit the privileges of any created role and be able to access all such roles via SET ROLE; with this patch, you can configure createrole_self_grant = 'set, inherit' to provide a similar experience for a user who has CREATEROLE but not SUPERUSER. FIXME: Add some regression tests. --- doc/src/sgml/config.sgml | 33 +++++++ doc/src/sgml/ref/create_role.sgml | 1 + doc/src/sgml/ref/createuser.sgml | 1 + src/backend/commands/user.c | 97 ++++++++++++++++++- src/backend/utils/misc/guc_tables.c | 12 +++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/commands/user.h | 10 +- 7 files changed, 150 insertions(+), 5 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 05b3862d09..d86385d308 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -9421,6 +9421,39 @@ SET XML OPTION { DOCUMENT | CONTENT }; + + createrole_self_grant (string) + + createrole_self_grant + configuration parameter + + + + + If a user who has CREATEROLE but not + SUPERUSER creates a role, and if this + is set to a non-empty value, the newly-created role will be granted + to the creating user with the options specified. The value must be + set, inherit, or a + comma-separated list of these. + + + The purpose of this option is to allow a CREATEROLE + user who is not a superuser to automatically inherit, or automatically + gain the ability to SET ROLE to, any created users. + Since a CREATEROLE user is always implicitly granted + ADMIN OPTION on created roles, that user could + always execute a GRANT statement that would achieve + the same effect as this setting. However, it can be convenient for + usability reasons if the grant happens automatically. A superuser + automatically inherits the privileges of every role and can always + SET ROLE to any role, and this setting can be used + to produce a similar behavior for CREATEROLE users + for users which they create. + + + + diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml index 0863acbcac..7ce4e38b45 100644 --- a/doc/src/sgml/ref/create_role.sgml +++ b/doc/src/sgml/ref/create_role.sgml @@ -506,6 +506,7 @@ CREATE ROLE name [ WITH ADMIN + diff --git a/doc/src/sgml/ref/createuser.sgml b/doc/src/sgml/ref/createuser.sgml index f91dc500a4..9a1c3d01f4 100644 --- a/doc/src/sgml/ref/createuser.sgml +++ b/doc/src/sgml/ref/createuser.sgml @@ -555,6 +555,7 @@ PostgreSQL documentation + diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 1ae2d0a66f..4d193a6f9a 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -39,6 +39,7 @@ #include "utils/fmgroids.h" #include "utils/syscache.h" #include "utils/timestamp.h" +#include "utils/varlena.h" /* * Removing a role grant - or the admin option on it - might recurse to @@ -81,8 +82,11 @@ typedef struct #define GRANT_ROLE_SPECIFIED_INHERIT 0x0002 #define GRANT_ROLE_SPECIFIED_SET 0x0004 -/* GUC parameter */ +/* GUC parameters */ int Password_encryption = PASSWORD_TYPE_SCRAM_SHA_256; +char *createrole_self_grant = ""; +bool createrole_self_grant_enabled = false; +GrantRoleOptions createrole_self_grant_options; /* Hook to check passwords in CreateRole() and AlterRole() */ check_password_hook_type check_password_hook = NULL; @@ -532,10 +536,13 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) if (!superuser()) { RoleSpec *current_role = makeNode(RoleSpec); - GrantRoleOptions poptself; + GrantRoleOptions poptself; + List *memberSpecs; + List *memberIds = list_make1_oid(currentUserId); current_role->roletype = ROLESPEC_CURRENT_ROLE; current_role->location = -1; + memberSpecs = list_make1(current_role); poptself.specified = GRANT_ROLE_SPECIFIED_ADMIN | GRANT_ROLE_SPECIFIED_INHERIT @@ -545,7 +552,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) poptself.set = false; AddRoleMems(BOOTSTRAP_SUPERUSERID, stmt->role, roleid, - list_make1(current_role), list_make1_oid(GetUserId()), + memberSpecs, memberIds, BOOTSTRAP_SUPERUSERID, &poptself); /* @@ -553,6 +560,20 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) * the additional grants will fail. */ CommandCounterIncrement(); + + /* + * Because of the implicit grant above, a CREATEROLE user who creates + * a role has the ability to grant that role back to themselves with + * the INHERIT or SET options, if they wish to inherit the role's + * privileges or be able to SET ROLE to it. The createrole_self_grant + * GUC can be used to make this happen automatically. This has no + * security implications since the same user is able to make the same + * grant using an explicit GRANT statement; it's just convenient. + */ + if (createrole_self_grant_enabled) + AddRoleMems(currentUserId, stmt->role, roleid, + memberSpecs, memberIds, + currentUserId, &createrole_self_grant_options); } /* @@ -2414,3 +2435,73 @@ InitGrantRoleOptions(GrantRoleOptions *popt) popt->inherit = false; popt->set = true; } + +/* + * GUC check_hook for createrole_self_grant + */ +bool +check_createrole_self_grant(char **newval, void **extra, GucSource source) +{ + char *rawstring; + List *elemlist; + ListCell *l; + unsigned options = 0; + unsigned *result; + + /* Need a modifiable copy of string */ + rawstring = pstrdup(*newval); + + if (!SplitIdentifierString(rawstring, ',', &elemlist)) + { + /* syntax error in list */ + GUC_check_errdetail("List syntax is invalid."); + pfree(rawstring); + list_free(elemlist); + return false; + } + + foreach(l, elemlist) + { + char *tok = (char *) lfirst(l); + + if (pg_strcasecmp(tok, "SET") == 0) + options |= GRANT_ROLE_SPECIFIED_SET; + else if (pg_strcasecmp(tok, "INHERIT") == 0) + options |= GRANT_ROLE_SPECIFIED_INHERIT; + else + { + GUC_check_errdetail("Unrecognized key word: \"%s\".", tok); + pfree(rawstring); + list_free(elemlist); + return false; + } + } + + pfree(rawstring); + list_free(elemlist); + + result = (unsigned *) guc_malloc(LOG, sizeof(unsigned)); + *result = options; + *extra = result; + + return true; +} + +/* + * GUC assign_hook for createrole_self_grant + */ +void +assign_createrole_self_grant(const char *newval, void *extra) +{ + unsigned options = * (unsigned *) extra; + + createrole_self_grant_enabled = (options != 0); + createrole_self_grant_options.specified = GRANT_ROLE_SPECIFIED_ADMIN + | GRANT_ROLE_SPECIFIED_INHERIT + | GRANT_ROLE_SPECIFIED_SET; + createrole_self_grant_options.admin = false; + createrole_self_grant_options.inherit = + (options & GRANT_ROLE_SPECIFIED_INHERIT) != 0; + createrole_self_grant_options.set = + (options & GRANT_ROLE_SPECIFIED_SET) != 0; +} diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 68328b1402..e222b087eb 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -3937,6 +3937,18 @@ struct config_string ConfigureNamesString[] = check_temp_tablespaces, assign_temp_tablespaces, NULL }, + { + {"createrole_self_grant", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Sets whether a CREATEROLE user automatically grants " + "the role to themselves, and with which options."), + NULL, + GUC_LIST_INPUT + }, + &createrole_self_grant, + "", + check_createrole_self_grant, assign_createrole_self_grant, NULL + }, + { {"dynamic_library_path", PGC_SUSET, CLIENT_CONN_OTHER, gettext_noop("Sets the path for dynamically loadable modules."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 5afdeb04de..cf8cc12164 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -702,6 +702,7 @@ #xmlbinary = 'base64' #xmloption = 'content' #gin_pending_list_limit = 4MB +#createrole_self_grant = '' # set and/or inherit # - Locale and Formatting - diff --git a/src/include/commands/user.h b/src/include/commands/user.h index 54c720d880..97dcb93791 100644 --- a/src/include/commands/user.h +++ b/src/include/commands/user.h @@ -15,9 +15,11 @@ #include "libpq/crypt.h" #include "nodes/parsenodes.h" #include "parser/parse_node.h" +#include "utils/guc.h" -/* GUC. Is actually of type PasswordType. */ -extern PGDLLIMPORT int Password_encryption; +/* GUCs */ +extern PGDLLIMPORT int Password_encryption; /* values from enum PasswordType */ +extern PGDLLIMPORT char *createrole_self_grant; /* Hook to check passwords in CreateRole() and AlterRole() */ typedef void (*check_password_hook_type) (const char *username, const char *shadow_pass, PasswordType password_type, Datum validuntil_time, bool validuntil_null); @@ -34,4 +36,8 @@ extern void DropOwnedObjects(DropOwnedStmt *stmt); extern void ReassignOwnedObjects(ReassignOwnedStmt *stmt); extern List *roleSpecsToIds(List *memberNames); +extern bool check_createrole_self_grant(char **newval, void **extra, + GucSource source); +extern void assign_createrole_self_grant(const char *newval, void *extra); + #endif /* USER_H */ -- 2.37.1 (Apple Git-137.1)