Re: [HACKERS] New Model For Role Attributes and Fine Grained Permssions
On Thu, Aug 21, 2014 at 10:49 PM, Stephen Frost sfr...@snowman.net wrote: * Heikki Linnakangas (hlinnakan...@vmware.com) wrote: On 08/19/2014 04:27 AM, Brightwell, Adam wrote: This is a proof-of-concept patch for a new model around role attributes and fine grained permissions meant to alleviate the current over dependence on superuser. Hmm. How does this get us any closer to fine-grained permissions? This patch, by itself, does not- but it adds the structure to allow us to add more permissions without having to add more fields to pg_authid, and we could build into pg_permission the WITH ADMIN OPTION and grantor bits that the normal GRANT syntax already supports- but without having to chew up additional bits for these granted permissions which are applied to roles instead of objects in the system. As for specific examples where this could be used beyond those presented, consider things like: CREATE EXTENSION CREATE TABLESPACE COPY (server-side) I'm not opposed to this in theory, but I agree with Heikki that the syntax you've picked (as well as the internal teminology of the patch) is not sufficiently unambiguous. Permission could refer to a lot of things, and the GRANT syntax does a lot of things already. Since the patch appears to aim to supplant what we current do with ALTER ROLE .. [NO] {CREATEDB | CREATEROLE }, how about something like: ALTER ROLE role_name { GRANT | REVOKE } CAPABILITY capability_name; In terms of catalog structure, I doubt that a row-per-capability-per-user makes sense. Why not just add a new rolcapability column to pg_authid? A single int64 interpreted as a bitmask would give us room for 64 capabilities at a fraction of the storage and lookup cost of a separate catalog. If that's not enough, the column could be stored as an integer array or vector or something, but a bigger problem is that Tom's head will likely explode if you tell him you're going to have more than 64 of these things. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New Model For Role Attributes and Fine Grained Permssions
Heikki, * Heikki Linnakangas (hlinnakan...@vmware.com) wrote: On 08/19/2014 04:27 AM, Brightwell, Adam wrote: This is a proof-of-concept patch for a new model around role attributes and fine grained permissions meant to alleviate the current over dependence on superuser. Hmm. How does this get us any closer to fine-grained permissions? This patch, by itself, does not- but it adds the structure to allow us to add more permissions without having to add more fields to pg_authid, and we could build into pg_permission the WITH ADMIN OPTION and grantor bits that the normal GRANT syntax already supports- but without having to chew up additional bits for these granted permissions which are applied to roles instead of objects in the system. As for specific examples where this could be used beyond those presented, consider things like: CREATE EXTENSION CREATE TABLESPACE COPY (server-side) The question which I've been kicking around is the possible additional constraints which we may want/need for these- a list of extensions which the role is allowed to create, a set of directories which the role is allowed to create tablespaces within, similairly a set of directories the role is allowed to use server-side COPY with (and perhaps a distinction between COPY FROM and COPY TO). I guess we need this, so that you can grant/revoke the permissions, but I thought the hard part is defining what the fine-grained permissions are, in a way that you can't escalate yourself to full superuser through any of them. This is definitely a concern- which is why I mention the specific items above as needing to be constrained in some fashion. CREATE EXTENSION and CREATE TABLESPACE are, in a way, naturally constrained if you imagine an environment where the user with those permissions doesn't have direct access to modify the filesystem. COPY, on the other hand, would allow the user to gain access to pg_authid through indirect means and therefore gain access to an actual superuser role on the system, potentially. (Ok- it might be possible to do that with CREATE TABLESPACE too, but it'd be a bit more interesting to work through that than being able to just COPY to a bytea and download the result) The syntax for GRANT/REVOKE is quite complicated already. Do we want to overload it even more? Also keep in mind that the SQL standard specifies GRANT/REVOKE, so we have to be careful to not clash with the SQL standard syntax, or any conceivable future syntax the SQL committee might add to it (although we have plenty of PostgreSQL extensions in it already). For example, this is currently legal: I agree that there are concerns in this area and I've not got any great solutions. There are certainly pros and cons to using GRANT. It's familiar and natural to DBAs, but it runs the risk of conflicting with future SQL syntax, or even our own GRANT role. We can avoid the latter by keeping to reserved keywords only, but that may lead to some rather odd syntax (how would the GRANT COPY ON (dir1, dir2, dir3) work?). Is there a good alternative to consider though..? Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] New Model For Role Attributes and Fine Grained Permssions
On 08/19/2014 04:27 AM, Brightwell, Adam wrote: Hi All, This is a proof-of-concept patch for a new model around role attributes and fine grained permissions meant to alleviate the current over dependence on superuser. Hmm. How does this get us any closer to fine-grained permissions? I guess we need this, so that you can grant/revoke the permissions, but I thought the hard part is defining what the fine-grained permissions are, in a way that you can't escalate yourself to full superuser through any of them. The new syntax could be useful even if we never get around to do anything about current superuser checks, so I'm going to give this a quick review just on its own merits. Please add documentation. That's certainly required before this can be committed, but it'll make reviewing the syntax much easier too. This is not yet complete and only serves as a proof-of-concept at this point, but I wanted to share it in the hopes of receiving comments, suggestions and general feedback. The general gist of this patch is as follows: * New system catalog pg_permission that relates role id's to permissions. * New syntax. - GRANT permission TO role - REVOKE permission FROM role where, permission is one of an enumerated value, such as CREATE ROLE or CREATE DATABASE. The syntax for GRANT/REVOKE is quite complicated already. Do we want to overload it even more? Also keep in mind that the SQL standard specifies GRANT/REVOKE, so we have to be careful to not clash with the SQL standard syntax, or any conceivable future syntax the SQL committee might add to it (although we have plenty of PostgreSQL extensions in it already). For example, this is currently legal: GRANT CREATE ALL ON TABLESPACE typename TO role Is that too close to the new syntax, to cause confusion? Does the new syntax work for all the more fine-grained permissions you have in mind? I'm particularly worried of conflicts with this existing syntax: GRANT role_name [, ...] TO role_name [, ...] [ WITH ADMIN OPTION ] - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] New Model For Role Attributes and Fine Grained Permssions
Hi All, This is a proof-of-concept patch for a new model around role attributes and fine grained permissions meant to alleviate the current over dependence on superuser. This is not yet complete and only serves as a proof-of-concept at this point, but I wanted to share it in the hopes of receiving comments, suggestions and general feedback. The general gist of this patch is as follows: * New system catalog pg_permission that relates role id's to permissions. * New syntax. - GRANT permission TO role - REVOKE permission FROM role where, permission is one of an enumerated value, such as CREATE ROLE or CREATE DATABASE. * Refactor CREATEDB and NOCREATEDB role attribute to CREATE DATABASE permission set by GRANT or REVOKE. * Refactor CREATEROLE and NOCREATEROLE role attribute to CREATE ROLE permission set by GRANT or REVOKE. Again, this is meant to serve as a proof-of-concept. It is not comprehensive and only demonstrates how this might work with a few already defined permissions. I have attached the current patch based on master. Any comments or feedback would be greatly appreciated. Thanks, Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.com diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile new file mode 100644 index a974bd5..8e4ad25 *** a/src/backend/catalog/Makefile --- b/src/backend/catalog/Makefile *** POSTGRES_BKI_SRCS = $(addprefix $(top_sr *** 39,45 pg_ts_config.h pg_ts_config_map.h pg_ts_dict.h \ pg_ts_parser.h pg_ts_template.h pg_extension.h \ pg_foreign_data_wrapper.h pg_foreign_server.h pg_user_mapping.h \ ! pg_foreign_table.h \ pg_default_acl.h pg_seclabel.h pg_shseclabel.h pg_collation.h pg_range.h \ toasting.h indexing.h \ ) --- 39,45 pg_ts_config.h pg_ts_config_map.h pg_ts_dict.h \ pg_ts_parser.h pg_ts_template.h pg_extension.h \ pg_foreign_data_wrapper.h pg_foreign_server.h pg_user_mapping.h \ ! pg_foreign_table.h pg_permission.h \ pg_default_acl.h pg_seclabel.h pg_shseclabel.h pg_collation.h pg_range.h \ toasting.h indexing.h \ ) diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c new file mode 100644 index d9745ca..9f4b5df *** a/src/backend/catalog/aclchk.c --- b/src/backend/catalog/aclchk.c *** *** 42,53 --- 42,55 #include catalog/pg_opclass.h #include catalog/pg_operator.h #include catalog/pg_opfamily.h + #include catalog/pg_permission.h #include catalog/pg_proc.h #include catalog/pg_tablespace.h #include catalog/pg_type.h #include catalog/pg_ts_config.h #include catalog/pg_ts_dict.h #include commands/dbcommands.h + #include commands/permission.h #include commands/proclang.h #include commands/tablespace.h #include foreign/foreign.h *** bool *** 5065,5082 has_createrole_privilege(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)) ! { ! result = ((Form_pg_authid) GETSTRUCT(utup))-rolcreaterole; ! ReleaseSysCache(utup); ! } return result; } --- 5067,5079 has_createrole_privilege(Oid roleid) { bool result = false; /* Superusers bypass all permission checking. */ if (superuser_arg(roleid)) return true; ! result = HasPermission(roleid, PERM_CREATE_ROLE); ! return result; } diff --git a/src/backend/commands/Makefile b/src/backend/commands/Makefile new file mode 100644 index 22f116b..b98212a *** a/src/backend/commands/Makefile --- b/src/backend/commands/Makefile *** OBJS = aggregatecmds.o alter.o analyze.o *** 17,23 dbcommands.o define.o discard.o dropcmds.o \ event_trigger.o explain.o extension.o foreigncmds.o functioncmds.o \ indexcmds.o lockcmds.o matview.o operatorcmds.o opclasscmds.o \ ! portalcmds.o prepare.o proclang.o \ schemacmds.o seclabel.o sequence.o tablecmds.o tablespace.o trigger.o \ tsearchcmds.o typecmds.o user.o vacuum.o vacuumlazy.o \ variable.o view.o --- 17,23 dbcommands.o define.o discard.o dropcmds.o \ event_trigger.o explain.o extension.o foreigncmds.o functioncmds.o \ indexcmds.o lockcmds.o matview.o operatorcmds.o opclasscmds.o \ ! permission.o portalcmds.o prepare.o proclang.o \ schemacmds.o seclabel.o sequence.o tablecmds.o tablespace.o trigger.o \ tsearchcmds.o typecmds.o user.o vacuum.o vacuumlazy.o \ variable.o view.o diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c new file mode 100644 index f480be8..ecba8a5 *** a/src/backend/commands/dbcommands.c --- b/src/backend/commands/dbcommands.c *** *** 36,45 --- 36,47 #include catalog/pg_authid.h #include catalog/pg_database.h #include catalog/pg_db_role_setting.h + #include