Noah, * Noah Misch (n...@leadboat.com) wrote: > On Tue, Dec 29, 2015 at 08:35:50AM -0500, Stephen Frost wrote: > > * Noah Misch (n...@leadboat.com) wrote: > > > The one argument which you've put forth for adding the complexity of > > dumping catalog ACLs is that we might reduce the number of default > > roles provided to the user. > > Right. If "GRANT EXECUTE ON FUNCTION pg_rotate_logfile() TO mydba" worked as > well as it works on user-defined functions, the community would not choose to > add a pg_rotate_logfile role holding just that one permission.
I understand that's your position, but I disagree with your conclusion. If we're going to provide default roles, which I continue to feel is a good approach, then I would suggest we use them as an abstraction for the permissions which we view as senible sets of grantable rights. I dislike the idea of having default roles and then making an exception for single-permission cases. I'm approaching this largely from a 3rd-party application perspective. There are two examples off-hand which I'm considering: check_postgres pgbackrest I'd like to be able to include, in both of those, a simple set of instructions for granting the necessary rights to the user who is running those processes. A set of rights which an administrator can go look up and easily read and understand the result of those grants. For example: check_postgres: Most check_postgres sub-commands can be run without superuser privileges by granting the pg_monitor role to the monitoring user: GRANT pg_monitor TO monitor; For information regarding the pg_monitor role, see: http://www.postgresql.org/docs/current/static/roles/database-roles.html pgbackrest: To run pgbackrest as a non-superuser and not the 'postgres' system user, grant the pg_backup role to the backrest user and ensure the backrest system user has read access to the database files (eg: by having the system user be a member of the 'postgres' group): GRANT pg_backup to backrest; For information regarding the pg_backup role, see: http://www.postgresql.org/docs/current/static/roles/database-roles.html I can see similar bits of documentation being included in pgAdmin or other tools. For the pg_rotate_logfile permission, specifically, we were asked by a client about that permission with the use case being a logrotate-type of tool, which only has access to the log files but needs to be able to perform a rotation. This particular client is pretty tech savvy and I don't think they'd have a problem using GRANT EXECUTE if that was the only option, but I can see a similar use-case with logrotate or pgAdmin or even for regular non-superuser admins using psql and, to reiterate what I said above, I'd rather have one abstraction for these kinds of permissions instead of a mish-mash of instructions. The difference I can imagine being between: For backups and monitoring, you can use default roles: GRANT pg_backup,pg_monitor to new_admin; but for other regular privileges such as rotating logfiles, or sending signals to other processes, you have to explicitly GRANT permissions: GRANT EXECUTE ON FUNCTION pg_rotate_logfile() TO new_admin; GRANT EXECUTE ON FUNCTION pg_signal_backend() TO new_admin; > > I disagree that we would. Having a single > > set of default roles which provide a sensible breakdown of permissions > > is a better approach than asking every administrator and application > > developer who is building tools on top of PG to try and work through > > what makes sense themselves, even if that means we have a default role > > with a small, or even only an individual, capability. > > The proposed pg_replication role introduces abstraction that could, as you > hope, spare a DBA from studying sets of functions to grant together. The > pg_rotate_logfile role, however, does not shield the DBA from complexity. I disagree with the above statement. Having to understand only one level of abstraction (the default roles) does reduce the complexity and means that the DBA does not have to work out if the specifc GRANT requested by the end user would result in some other access or if there are any unexpected issues to encounter with issuing GRANTs directly on catalog objects- something we don't currently support, so such concern is certainly reasonable. > Being narrowly tied to a specific function, it's just a suboptimal spelling of > GRANT. The gap in GRANT has distorted the design for these predefined roles. > I do not anticipate a sound design discussion about specific predefined roles > so long as the state of GRANT clouds the matter. I'm loathe to encourage any direct modification of catalog objects, even if it's just ACLs. I've seen too many cases, as I imagine others have, of users destroying their databases or running into unexpected results when modifying the catalog. The catalog modifications supported should be explicitly provided through other means rather than direct commands against the catalog objects. I see the default roles approach as being similar to having: ALTER DATABASE db WITH CONNECTION LIMIT 5; instead of suggesting users issue: UPDATE DATABASE SET datconnlimit = 5 WHERE datname = 'db'; There is little difference between the two, technically, but I'm a whole lot more comfortable with the ALTER DATABASE than with the user issuing an UPDATE against a catalog table. With 9.5, we are adding ALLOW_CONNECTIONS and IS_TEMPLATE also and I don't recall any particular concern that those are overly redundant with the equivilant UPDATE statement. > > > To summarize, I think the right next step is to resume designing pg_dump > > > support for system object ACLs. I looked over your other two patches and > > > will > > > unshelve those reviews when their time comes. > > > > To be clear, I don't believe the two patches are particularly involved > > with each other and don't feel that one needs to wait for the other. > > Patch 2/3 could stand without patch 3/3, but not vice-versa. It's patch 2/3 > that makes pg_dumpall skip ^pg_ roles, and that must be in place no later than > the first patch that adds a predefined ^pg_ role. Apologies for not being clear on this- I was referring to pg_dump support for GRANT on catalog objects vs. the default roles patch in my statement by way of summary. > > Further, I'm not convinced that adding support for dumping ACLs or, in > > general, encouraging users to define their own ACLs on catalog objects > > is a good idea. We certainly have no mechanism in place today for those > > ACLs to be respected by SysCache and encouraging their use when we won't > > actually respect them is likely to be confusing. > > What's this problem with syscache? It sounds important. CREATE TABLE and DROP TABLE aren't going to care one bit if you have access to pg_class or not. The same goes for basically everything else. If we really want to support ACLs on the catalog, we'd have to either caveat that none of the internal lookups will respect them or revamp SysCache and any other direct catalog access to do permission checks first, which I don't think we really want to do. This entire discussion of privileges-on-catalog-objects should really also consider the ongoing discussion about providing policies for the catalog via RLS. If we start pg_dump'ing the ACLs of catalog objects then we'd, presumably, also want to pg_dump out any policies defined against catalog objects. I wonder if we may end up causing ourselves trouble going with that approach though if we start providing a set of default policies which SysCache knows how to work with but which users can change. Thanks! Stephen
signature.asc
Description: Digital signature
