Re: Add default role 'pg_access_server_files'

Just circling back on this.

I did have a question that came up about the behavior of the server as it is without the patch. I logged into psql with my superuser "postgres":

postgres=# select pg_read_file('/Users/postgres/temp');
ERROR: absolute path not allowed

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? 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))
{

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.

Is this an intended behavior of the patch - to allow file access to absolute paths where previously it was impossible? Or, was the intention just to add granularity via the new role? I had assumed the latter.

Best regards,
Ryan

The new status of this patch is: Waiting on Author