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
