Re: Add default role 'pg_access_server_files'

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add default role 'pg_access_server_files'
Date: 2018-03-07 06:31:06
Message-ID: 20180307063106.GH1744@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 06, 2018 at 10:00:54AM -0500, Stephen Frost wrote:
> * Magnus Hagander (magnus(at)hagander(dot)net) wrote:
>> On Tue, Jan 2, 2018 at 1:08 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> > Suggestions on a name for this..? pg_server_copy_program?
>>
>> Presumably it would also be used in postgres_fdw, so that seems like a bad
>> name. Maybe pg_exec_server_command?
>
> I went with 'pg_execute_server_program', since 'program' is what we use
> in the COPY syntax, et al.

Okay.

> Attached is an updated patch which splits up the permissions as
> suggested up-thread by Magnus:
>
> The default roles added are:
>
> * pg_read_server_files
> * pg_write_server_files
> * pg_execute_server_program
>
> Reviews certainly welcome.

It seems to me that we have two different concepts here in one single
patch:
1) Replace superuser checks by execution ACLs for FS-related functions.
2) Introduce new administration roles to control COPY and file_fdw

First, could it be possible to do a split for 1) and 2)?

+ /*
+ * Members of the 'pg_read_server_files' role are allowed to access any
+ * files on the server as the PG user, so no need to do any further checks
+ * here.
+ */
+ if (is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_SERVER_FILES))
+ return filename;
Second, I don't quite understand what is the link between COPY/file_fdw
and the possibility to use absolute paths in all the functions of
genfile.c. Is the use-case the possibility to check for the existence
of a file using pg_stat_file before doing a copy? This seems rather
limited because COPY or file_fdw would complain similarly for a missing
file. So I don't quite get the use-case for authorizing that.

Could you update the documentation of pg_rewind please? It seems to me
that with your patch then superuser rights are not necessary mandatory,
as the role connecting to the source server does not need to have
superuser right, and needs just execution rights to pg_ls_dir,
pg_read_binary_files and pg_stat_file. Such a user does not need to be
granted access to pg_read_server_files either as no absolute paths are
involved in pg_rewind. Listing the functions directly would be also
nice. Personally I care a lot about 1), way less about 2).
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-03-07 06:39:06 Re: Missing break statement after transformCallStmt in transformStmt
Previous Message Tsunakawa, Takayuki 2018-03-07 06:15:24 RE: Speed up the removal of WAL files