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.gr2...@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
signature.asc
Description: PGP signature