Re: Add default role 'pg_access_server_files'

On Thu, Jan 18, 2018 at 02:04:45PM +0000, Ryan Murphy wrote:
> I had not tried this before with my unpatched build of postgres.  (In
> retrospect of course I should have).  I expected my superuser to be
> able to perform this task, but it seems that for security reasons we
> presently don't allow access to absolute path names (except in the
> data dir and log dir) - not even for a superuser.  Is that correct?

Correct.

> In that case the security implications of this patch would need more
> consideration.
>
> Stephen, looking at the patch, I see that in
> src/backend/utils/adt/genfile.c you've made some changes to the
> function convert_and_check_filename().  These changes, I believe,
> loosen the security checks in ways other than just adding the
> granularity of a new role which can be GRANTed to non superusers:
>
>     +   /*
>     +    * Members of the 'pg_access_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_ACCESS_SERVER_FILES))
>     +       return filename;
>     +
>     +   /* User isn't a member of the default role, so check if it's allowable */
>         if (is_absolute_path(filename))
>         {

It seems to me that this is the intention behind the patch as the
comment points out and as Stephen has stated in
https://www.postgresql.org/message-id/20171231191939.GR2416@tamriel.snowman.net.

> As you can see, you've added a short-circuit "return" statement for
> anyone who has the DEFAULT_ROLE_ACCESS_SERVER_FILES.  Prior to this
> patch, even a Superuser would be subject to the following
> is_absolute_path() logic.  But with it, the return statement
> short-circuits the is_absolute_path() check.

I agree that it is a strange concept to loosen the access while even
superusers cannot do that. By concept superusers are assumed to be able
to do anything on the server by the way.
--
Michael