Re: Granting control of SUSET gucs to non-superusers

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Chapman Flack <chap(at)anastigmatix(dot)net>
Subject: Re: Granting control of SUSET gucs to non-superusers
Date: 2021-05-12 15:59:18
Message-ID: 2949B883-9FD9-437C-B6A5-F2FD2B47C8B3@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Apr 30, 2021, at 4:19 PM, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>
> Hackers,
>
> PostgreSQL defines a number of GUCs that can only be set by superusers. I would like to support granting privileges on subsets of these to non-superuser roles, inspired by Stephen Frost's recent work on pg_read_all_data and pg_write_all_data roles.
>
> The specific use case motivating this work is that of a PostgreSQL service provider. The provider guarantees certain aspects of the service, such as periodic backups, replication, uptime, availability, etc., while making no guarantees of other aspects, such as performance associated with the design of the schema or the queries executed. The provider should be able to grant to the tenant privileges to set any GUC which cannot be used to "escape the sandbox" and interfere with the handful of metrics being guaranteed. Given that the guarantees made by one provider may differ from those made by another, the exact set of GUCs which the provider allows the tenant to control may differ.
>
> By my count, there are currently 50 such GUCs, already broken down into 15 config groups. Creating a single new role pg_set_all_gucs seems much too coarse a control, but creating 50 new groups may be excessive. We could certainly debate which GUCs could be used to escape the sandbox vs. which ones could not, but I would prefer a design that allows the provider to make that determination. The patch I would like to submit would only give the provider the mechanism for controlling these things, but would not make the security choices for them.
>
> Do folks think it would make sense to create a role per config group? Adding an extra 15 default roles seems high to me, but organizing the feature this way would make the roles easier to document, because there would be a one-to-one correlation between the roles and the config groups.
>
> I have a WIP patch that I'm not attaching, but if I get any feedback, I might be able to adjust the patch before the first version posted. The basic idea is that it allows things like:
>
> GRANT pg_set_stats_monitoring TO tenant_role;
>
> And then tenant_role could, for example
>
> SET log_parser_stats TO off;

Ok, here is the first version of the patch for the list (though it is the second version I developed.) The patch is quite long, but most of it is mechanical.

Overview:

- guc.h defines a new set of privilege masks
- pg_authid.dat defines a new set of roles, with a one-to-one correlation to the privilege masks
- guc_tables.h extends struct config_generic to include a privilege mask field
- guc.c extends the structs for all variables to include a mask of privileges required to set the variable, and checks the privileges against the current user's role membership when trying to SET or ALTER SYSTEM SET
- DefineCustom*Variable functions are extended to take a privileges mask, and all calls to these functions are extended to include privileges for the custom variable being defined
- new regression tests guc_priv_admin and guc_priv_tenant are defined. The first creates a role "admin" and assigns it membership to all the new roles added in pg_authid.dat. The second creates a role "tenant" and assigns it to just the few new roles that appear reasonable for a tenant. Both tests then go on to SET SESSION AUTHORIZATION to the new role and then attempt to SET, RESET, ALTER SYSTEM SET, and ALTER SYSTEM RESET most of the variables defined in guc.c. These tests might be too verbose to be worth committing, but I thought they made an easy reference for those reviewing the patch who just want to quickly see the behavior.

One of the consequences of the design is that if a user belongs to a role with permission to SET a variable, they can also ALTER SYSTEM SET that variable, at least to the extent that ALTER SYSTEM SET would allow the superuser to do so. Not all variables can be changed via ALTER SYSTEM SET, though. This means that some variables, "data_directory" for example, cannot be changed by any of the new roles. The first version of the patch, never posted, allowed 'include' directives in postgresql.conf to be annotated with roles, such that the included file would be processed with privileges restricted to just the listed roles. This patch doesn't bother, since everything we are likely to care about can be performed using ALTER SYSTEM SET, but I can resurrect the 'include' directive logic if there is general demand for that.

Any user can still SET a PGC_USERSET variable, just as before this patch, but the default permission to do so does not translate into permission to ALTER SYSTEM SET that same variable. For that, the user needs to belong to a role with permission to set the variable, which in general for PGC_USERSET variables is the "pg_internal_settings" role. I'm not sure this is the right role for all of these, for example "password_encryption" seems like a better fit for role "pg_interface_settings", but for the first patch posted to the list I didn't fuss too much about roles assigned to PGC_USERSET variables.

I didn't bother updating the docs yet, as I doubt the set of privileges/roles in this patch will survive contact with this list. They are:

pg_internal_settings:
- changes to purely internal behavior
pg_stats_settings:
- changes to stats collection
pg_maintenance_settings
- changes to autovacuum behavior
pg_storage_settings
- changes to dealing with storage errors, such as fsync or checksum failure
pg_wal_settings
- changes to wal, recovery, and replication settings
pg_logging_settings
- changes to what gets logged
pg_interface_settings
- changes to the external interface, such as port, authentication, etc.
pg_resource_usage
- changes to memory, cpu, and disk usage
pg_filesystem_security
- changes to where files and directories are located, permissions bits on
files and directories, etc.
pg_exec_command
- changes to external commands that get executed
pg_server_configuration
- changes to the configuration of the server vis-a-vis the operating system
facilities, such as shared memory model used
pg_security_settings
- changes that relax security, such as turning off privilege checking,
changing security critical logging settings, adjusting developer options
which have security implications, or changing settings which could be
used to create a denial of services attack

Note that some GUC variables have more than one privilege bit set, meaning a user must belong to all corresponding roles before they can change the setting. For example, "log_file_mode" requires both pg_filesystem_security and pg_logging_settings.

Attachment Content-Type Size
v2-0001-Delegating-setting-of-GUC-variables.patch application/octet-stream 542.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Laurenz Albe 2021-05-12 16:08:07 Re: RFC: Logging plan of the running query
Previous Message Tom Lane 2021-05-12 15:54:06 Re: Condition pushdown: why (=) is pushed down into join, but BETWEEN or >= is not?