Add new GUC createrole_self_grant.
authorRobert Haas <rhaas@postgresql.org>
Tue, 10 Jan 2023 17:44:49 +0000 (12:44 -0500)
committerRobert Haas <rhaas@postgresql.org>
Tue, 10 Jan 2023 17:44:49 +0000 (12:44 -0500)
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.

Discussion: https://postgr.es/m/CA+TgmobN59ct+Emmz6ig1Nua2Q-_o=r6DSD98KfU53kctq_kQw@mail.gmail.com

doc/src/sgml/config.sgml
doc/src/sgml/ref/create_role.sgml
doc/src/sgml/ref/createuser.sgml
src/backend/commands/user.c
src/backend/utils/misc/guc_tables.c
src/backend/utils/misc/postgresql.conf.sample
src/include/commands/user.h
src/test/regress/expected/create_role.out
src/test/regress/sql/create_role.sql

index 2fec613484af57be32c11f7b53511f65feefbc6e..77574e2d4ec1f735acbd4cacc3d9e3a61c72af60 100644 (file)
@@ -9447,6 +9447,39 @@ SET XML OPTION { DOCUMENT | CONTENT };
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-createrole-self-grant" xreflabel="createrole_self_grant">
+      <term><varname>createrole_self_grant</varname> (<type>string</type>)
+      <indexterm>
+       <primary><varname>createrole_self_grant</varname></primary>
+       <secondary>configuration parameter</secondary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        If a user who has <literal>CREATEROLE</literal> but not
+        <literal>SUPERUSER</literal> 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
+        <literal>set</literal>, <literal>inherit</literal>, or a
+        comma-separated list of these.
+       </para>
+       <para>
+        The purpose of this option is to allow a <literal>CREATEROLE</literal>
+        user who is not a superuser to automatically inherit, or automatically
+        gain the ability to <literal>SET ROLE</literal> to, any created users.
+        Since a <literal>CREATEROLE</literal> user is always implicitly granted
+        <literal>ADMIN OPTION</literal> on created roles, that user could
+        always execute a <literal>GRANT</literal> 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
+        <literal>SET ROLE</literal> to any role, and this setting can be used
+        to produce a similar behavior for <literal>CREATEROLE</literal> users
+        for users which they create.
+       </para>
+      </listitem>
+     </varlistentry>
+
      </variablelist>
     </sect2>
      <sect2 id="runtime-config-client-format">
index 0863acbcac4e1f7055b8a316f98320c54146e4ce..7ce4e38b458928dba2d5dbd3b12d97d543c8d996 100644 (file)
@@ -506,6 +506,7 @@ CREATE ROLE <replaceable class="parameter">name</replaceable> [ WITH ADMIN <repl
    <member><xref linkend="sql-grant"/></member>
    <member><xref linkend="sql-revoke"/></member>
    <member><xref linkend="app-createuser"/></member>
+   <member><xref linkend="guc-createrole-self-grant"/></member>
   </simplelist>
  </refsect1>
 </refentry>
index f91dc500a4083eb792108961cbf7222a61070e8f..9a1c3d01f48b356cbd7bfc8804981780fb61ba47 100644 (file)
@@ -555,6 +555,7 @@ PostgreSQL documentation
   <simplelist type="inline">
    <member><xref linkend="app-dropuser"/></member>
    <member><xref linkend="sql-createrole"/></member>
+   <member><xref linkend="guc-createrole-self-grant"/></member>
   </simplelist>
  </refsect1>
 
index 1ae2d0a66fb191653b7d4be46324dbd39862eefe..4d193a6f9a4e77e07cd44eb95ce29a2acc996aed 100644 (file)
@@ -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;
+}
index 92545b495875373f70827a62c3aad38c7427b4f3..5025e80f89d32f80232ea5ba3b16923a9f844a64 100644 (file)
@@ -3949,6 +3949,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."),
index c2ada920549d4d096d9fa1d518ff02eb5f5c51d2..4cceda416222692b8230e57a63a4d863b97b9849 100644 (file)
 #xmlbinary = 'base64'
 #xmloption = 'content'
 #gin_pending_list_limit = 4MB
+#createrole_self_grant = ''        # set and/or inherit
 
 # - Locale and Formatting -
 
index 54c720d88017628407c9a39b9741cfab82d6010e..97dcb93791b89c0f676369b78b44c26003f3eb67 100644 (file)
 #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 */
index f5f745504c2cba767d7855001220412188de52e9..bed374988896d2cae1a48a27c09613bb5bba5596 100644 (file)
@@ -1,6 +1,7 @@
 -- ok, superuser can create users with any set of privileges
 CREATE ROLE regress_role_super SUPERUSER;
 CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS;
+GRANT CREATE ON DATABASE regression TO regress_role_admin WITH GRANT OPTION;
 CREATE ROLE regress_role_normal;
 -- fail, only superusers can create users with these privileges
 SET SESSION AUTHORIZATION regress_role_admin;
@@ -15,6 +16,7 @@ ERROR:  must be superuser to create bypassrls users
 -- ok, having CREATEROLE is enough to create users with these privileges
 CREATE ROLE regress_createdb CREATEDB;
 CREATE ROLE regress_createrole CREATEROLE NOINHERIT;
+GRANT CREATE ON DATABASE regression TO regress_createrole WITH GRANT OPTION;
 CREATE ROLE regress_login LOGIN;
 CREATE ROLE regress_inherit INHERIT;
 CREATE ROLE regress_connection_limit CONNECTION LIMIT 5;
@@ -83,9 +85,37 @@ ALTER VIEW tenant_view OWNER TO regress_role_admin;
 ERROR:  must be owner of view tenant_view
 DROP VIEW tenant_view;
 ERROR:  must be owner of view tenant_view
+-- fail, can't create objects owned as regress_tenant
+CREATE SCHEMA regress_tenant_schema AUTHORIZATION regress_tenant;
+ERROR:  must be able to SET ROLE "regress_tenant"
 -- fail, we don't inherit permissions from regress_tenant
 REASSIGN OWNED BY regress_tenant TO regress_createrole;
 ERROR:  permission denied to reassign objects
+-- ok, create a role with a value for createrole_self_grant
+SET createrole_self_grant = 'set, inherit';
+CREATE ROLE regress_tenant2;
+GRANT CREATE ON DATABASE regression TO regress_tenant2;
+-- ok, regress_tenant2 can create objects within the database
+SET SESSION AUTHORIZATION regress_tenant2;
+CREATE TABLE tenant2_table (i integer);
+REVOKE ALL PRIVILEGES ON tenant2_table FROM PUBLIC;
+-- ok, because we have SET and INHERIT on regress_tenant2
+SET SESSION AUTHORIZATION regress_createrole;
+CREATE SCHEMA regress_tenant2_schema AUTHORIZATION regress_tenant2;
+ALTER SCHEMA regress_tenant2_schema OWNER TO regress_createrole;
+ALTER TABLE tenant2_table OWNER TO regress_createrole;
+ALTER TABLE tenant2_table OWNER TO regress_tenant2;
+-- with SET but not INHERIT, we can give away objects but not take them
+REVOKE INHERIT OPTION FOR regress_tenant2 FROM regress_createrole;
+ALTER SCHEMA regress_tenant2_schema OWNER TO regress_tenant2;
+ALTER TABLE tenant2_table OWNER TO regress_createrole;
+ERROR:  must be owner of table tenant2_table
+-- with INHERIT but not SET, we can take objects but not give them away
+GRANT regress_tenant2 TO regress_createrole WITH INHERIT TRUE, SET FALSE;
+ALTER TABLE tenant2_table OWNER TO regress_createrole;
+ALTER TABLE tenant2_table OWNER TO regress_tenant2;
+ERROR:  must be able to SET ROLE "regress_tenant2"
+DROP TABLE tenant2_table;
 -- fail, CREATEROLE is not enough to create roles in privileged roles
 CREATE ROLE regress_read_all_data IN ROLE pg_read_all_data;
 ERROR:  must have admin option on role "pg_read_all_data"
@@ -131,6 +161,8 @@ ERROR:  role "regress_nosuch_recursive" does not exist
 DROP ROLE regress_nosuch_admin_recursive;
 ERROR:  role "regress_nosuch_admin_recursive" does not exist
 DROP ROLE regress_plainrole;
+-- must revoke privileges before dropping role
+REVOKE CREATE ON DATABASE regression FROM regress_createrole CASCADE;
 -- ok, should be able to drop non-superuser roles we created
 DROP ROLE regress_createdb;
 DROP ROLE regress_createrole;
@@ -149,6 +181,7 @@ DROP ROLE regress_role_admin;
 ERROR:  current user cannot be dropped
 -- ok
 RESET SESSION AUTHORIZATION;
+REVOKE CREATE ON DATABASE regression FROM regress_role_admin CASCADE;
 DROP INDEX tenant_idx;
 DROP TABLE tenant_table;
 DROP VIEW tenant_view;
index ddc80578d9055b72ca3c711c4fd5b1113e85c3de..edaed43588bc43167706f78cc63d18257309d575 100644 (file)
@@ -1,6 +1,7 @@
 -- ok, superuser can create users with any set of privileges
 CREATE ROLE regress_role_super SUPERUSER;
 CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS;
+GRANT CREATE ON DATABASE regression TO regress_role_admin WITH GRANT OPTION;
 CREATE ROLE regress_role_normal;
 
 -- fail, only superusers can create users with these privileges
@@ -13,6 +14,7 @@ CREATE ROLE regress_nosuch_bypassrls BYPASSRLS;
 -- ok, having CREATEROLE is enough to create users with these privileges
 CREATE ROLE regress_createdb CREATEDB;
 CREATE ROLE regress_createrole CREATEROLE NOINHERIT;
+GRANT CREATE ON DATABASE regression TO regress_createrole WITH GRANT OPTION;
 CREATE ROLE regress_login LOGIN;
 CREATE ROLE regress_inherit INHERIT;
 CREATE ROLE regress_connection_limit CONNECTION LIMIT 5;
@@ -83,9 +85,40 @@ DROP TABLE tenant_table;
 ALTER VIEW tenant_view OWNER TO regress_role_admin;
 DROP VIEW tenant_view;
 
+-- fail, can't create objects owned as regress_tenant
+CREATE SCHEMA regress_tenant_schema AUTHORIZATION regress_tenant;
+
 -- fail, we don't inherit permissions from regress_tenant
 REASSIGN OWNED BY regress_tenant TO regress_createrole;
 
+-- ok, create a role with a value for createrole_self_grant
+SET createrole_self_grant = 'set, inherit';
+CREATE ROLE regress_tenant2;
+GRANT CREATE ON DATABASE regression TO regress_tenant2;
+
+-- ok, regress_tenant2 can create objects within the database
+SET SESSION AUTHORIZATION regress_tenant2;
+CREATE TABLE tenant2_table (i integer);
+REVOKE ALL PRIVILEGES ON tenant2_table FROM PUBLIC;
+
+-- ok, because we have SET and INHERIT on regress_tenant2
+SET SESSION AUTHORIZATION regress_createrole;
+CREATE SCHEMA regress_tenant2_schema AUTHORIZATION regress_tenant2;
+ALTER SCHEMA regress_tenant2_schema OWNER TO regress_createrole;
+ALTER TABLE tenant2_table OWNER TO regress_createrole;
+ALTER TABLE tenant2_table OWNER TO regress_tenant2;
+
+-- with SET but not INHERIT, we can give away objects but not take them
+REVOKE INHERIT OPTION FOR regress_tenant2 FROM regress_createrole;
+ALTER SCHEMA regress_tenant2_schema OWNER TO regress_tenant2;
+ALTER TABLE tenant2_table OWNER TO regress_createrole;
+
+-- with INHERIT but not SET, we can take objects but not give them away
+GRANT regress_tenant2 TO regress_createrole WITH INHERIT TRUE, SET FALSE;
+ALTER TABLE tenant2_table OWNER TO regress_createrole;
+ALTER TABLE tenant2_table OWNER TO regress_tenant2;
+DROP TABLE tenant2_table;
+
 -- fail, CREATEROLE is not enough to create roles in privileged roles
 CREATE ROLE regress_read_all_data IN ROLE pg_read_all_data;
 CREATE ROLE regress_write_all_data IN ROLE pg_write_all_data;
@@ -113,6 +146,9 @@ DROP ROLE regress_nosuch_recursive;
 DROP ROLE regress_nosuch_admin_recursive;
 DROP ROLE regress_plainrole;
 
+-- must revoke privileges before dropping role
+REVOKE CREATE ON DATABASE regression FROM regress_createrole CASCADE;
+
 -- ok, should be able to drop non-superuser roles we created
 DROP ROLE regress_createdb;
 DROP ROLE regress_createrole;
@@ -131,6 +167,7 @@ DROP ROLE regress_role_admin;
 
 -- ok
 RESET SESSION AUTHORIZATION;
+REVOKE CREATE ON DATABASE regression FROM regress_role_admin CASCADE;
 DROP INDEX tenant_idx;
 DROP TABLE tenant_table;
 DROP VIEW tenant_view;