Greetings, This patch adds a new default role called 'pg_access_server_files' which allows an administrator to GRANT to a non-superuser role the ability to access server-side files through PostgreSQL (as the user the database is running as). By itself, having this role allows a non-superuser to use server-side COPY and to use file_fdw (if installed by a superuser and GRANT'd USAGE on it).
Further, this patch moves the privilege check for the remaining misc file functions from explicit superuser checks to the GRANT system, similar to what's done for pg_ls_logdir() and others. Lastly, these functions are changed to allow a user with the 'pg_access_server_files' role to be able to access files outside of the PG data directory. This follows on and continues what was recently done with the lo_import/export functions. There's other superuser checks to replace with grant'able default roles, but those probably make more sense as independent patches. I continue to be of the opinion that it'd be nice to have more fine-grained control over these functions to limit the access granted, but nothing here prevents that from being done and this at least allows some movement away from having to have roles with superuser access. Thanks! Stephen
From eb8be8ffbadcc37418dc12d59c6767e073028e35 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfr...@snowman.net>
Date: Sun, 31 Dec 2017 14:01:12 -0500
Subject: [PATCH] Add default role pg_access_server_files
This patch adds a new default role called 'pg_access_server_files' which
allows an administrator to GRANT to a non-superuser role the ability to
access server-side files through PostgreSQL (as the user the database is
running as). By itself, having this role allows a non-superuser to use
server-side COPY and to use file_fdw (if installed by a superuser and
GRANT'd USAGE on it).
Further, this patch moves the privilege check for the remaining misc
file functions from explicit superuser checks to the GRANT system,
similar to what's done for pg_ls_logdir() and others. Lastly, these
functions are changed to allow a user with the 'pg_access_server_files'
role to be able to access files outside of the PG data directory.
---
contrib/file_fdw/file_fdw.c | 13 ++++++++-----
doc/src/sgml/file-fdw.sgml | 9 +++++----
doc/src/sgml/func.sgml | 16 ++++++++--------
doc/src/sgml/ref/copy.sgml | 5 +++--
src/backend/catalog/system_views.sql | 14 ++++++++++++++
src/backend/commands/copy.c | 16 ++++++++++------
src/backend/utils/adt/genfile.c | 30 ++++++++++--------------------
src/include/catalog/pg_authid.h | 2 ++
8 files changed, 60 insertions(+), 45 deletions(-)
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 370cc365d6..08d8d61f10 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -18,6 +18,7 @@
#include "access/htup_details.h"
#include "access/reloptions.h"
#include "access/sysattr.h"
+#include "catalog/pg_authid.h"
#include "catalog/pg_foreign_table.h"
#include "commands/copy.h"
#include "commands/defrem.h"
@@ -202,9 +203,10 @@ file_fdw_validator(PG_FUNCTION_ARGS)
ListCell *cell;
/*
- * Only superusers are allowed to set options of a file_fdw foreign table.
- * This is because we don't want non-superusers to be able to control
- * which file gets read or which program gets executed.
+ * Only members of the special role 'pg_access_server_files' are allowed
+ * to set options of a file_fdw foreign table. This is because we don't
+ * want regular users to be able to control which file gets read or which
+ * program gets executed.
*
* Putting this sort of permissions check in a validator is a bit of a
* crock, but there doesn't seem to be any other place that can enforce
@@ -214,10 +216,11 @@ file_fdw_validator(PG_FUNCTION_ARGS)
* program at any options level other than foreign table --- otherwise
* there'd still be a security hole.
*/
- if (catalog == ForeignTableRelationId && !superuser())
+ if (catalog == ForeignTableRelationId &&
+ !is_member_of_role(GetUserId(), DEFAULT_ROLE_ACCESS_SERVER_FILES))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("only superuser can change options of a file_fdw foreign table")));
+ errmsg("only superuser or a member of the pg_access_server_files role can change options of a file_fdw foreign table")));
/*
* Check that only options supported by file_fdw, and allowed for the
diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml
index e2598a07da..119f55add4 100644
--- a/doc/src/sgml/file-fdw.sgml
+++ b/doc/src/sgml/file-fdw.sgml
@@ -186,10 +186,11 @@
</para>
<para>
- Changing table-level options requires superuser privileges, for security
- reasons: only a superuser should be able to control which file is read
- or which program is run. In principle non-superusers could be allowed to
- change the other options, but that's not supported at present.
+ Changing table-level options requires begin a superuser or having the privileges
+ of the default role <literal>pg_access_server_files</literal>, for security
+ reasons: only certain users should be able to control which file is read or which
+ program is run. In principle regular users could be allowed to change the other
+ options, but that's not supported at present.
</para>
<para>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4dd9d029e6..29148839dc 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -19896,10 +19896,10 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
linkend="functions-admin-genfile-table"/> provide native access to
files on the machine hosting the server. Only files within the
database cluster directory and the <varname>log_directory</varname> can be
- accessed. Use a relative path for files in the cluster directory,
- and a path matching the <varname>log_directory</varname> configuration setting
- for log files. Use of these functions is restricted to superusers
- except where stated otherwise.
+ accessed unless the user is granted the role
+ <literal>pg_access_server_files</literal>. Use a relative path for files in
+ the cluster directory, and a path matching the <varname>log_directory</varname>
+ configuration setting for log files.
</para>
<table id="functions-admin-genfile-table">
@@ -19917,7 +19917,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
</entry>
<entry><type>setof text</type></entry>
<entry>
- List the contents of a directory.
+ List the contents of a directory. Restricted to superusers by default, but other users can be granted EXECUTE to run the function.
</entry>
</row>
<row>
@@ -19948,7 +19948,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
</entry>
<entry><type>text</type></entry>
<entry>
- Return the contents of a text file.
+ Return the contents of a text file. Restricted to superusers by default, but other users can be granted EXECUTE to run the function.
</entry>
</row>
<row>
@@ -19957,7 +19957,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
</entry>
<entry><type>bytea</type></entry>
<entry>
- Return the contents of a file.
+ Return the contents of a file. Restricted to superusers by default, but other users can be granted EXECUTE to run the function.
</entry>
</row>
<row>
@@ -19966,7 +19966,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
</entry>
<entry><type>record</type></entry>
<entry>
- Return information about a file.
+ Return information about a file. Restricted to superusers by default, but other users can be granted EXECUTE to run the function.
</entry>
</row>
</tbody>
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index af2a0e91b9..411fb77b1a 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -444,8 +444,9 @@ COPY <replaceable class="parameter">count</replaceable>
by the server, not by the client application, must be executable by the
<productname>PostgreSQL</productname> user.
<command>COPY</command> naming a file or command is only allowed to
- database superusers, since it allows reading or writing any file that the
- server has privileges to access.
+ database superusers or users who are granted the default role
+ <literal>pg_access_server_files</literal>, since it allows reading or
+ writing any file that the server has privileges to access.
</para>
<para>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 394aea8e0f..323741a73a 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1147,6 +1147,20 @@ REVOKE EXECUTE ON FUNCTION lo_export(oid, text) FROM public;
REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public;
REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_read_file(text) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_read_file(text,bigint,bigint) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_read_file(text,bigint,bigint,boolean) FROM public;
+
+REVOKE EXECUTE ON FUNCTION pg_read_binary_file(text) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_read_binary_file(text,bigint,bigint) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_read_binary_file(text,bigint,bigint,boolean) FROM public;
+
+REVOKE EXECUTE ON FUNCTION pg_stat_file(text) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_stat_file(text,boolean) FROM public;
+
+REVOKE EXECUTE ON FUNCTION pg_ls_dir(text) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public;
+
--
-- We also set up some things as accessible to standard roles.
--
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 254be28ae4..6d5e30f7e6 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -24,6 +24,7 @@
#include "access/xact.h"
#include "access/xlog.h"
#include "catalog/dependency.h"
+#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "commands/copy.h"
#include "commands/defrem.h"
@@ -774,8 +775,8 @@ CopyLoadRawBuf(CopyState cstate)
* input/output stream. The latter could be either stdin/stdout or a
* socket, depending on whether we're running under Postmaster control.
*
- * Do not allow a Postgres user without superuser privilege to read from
- * or write to a file.
+ * Do not allow a Postgres user without the 'pg_access_server_files' role to
+ * read from or write to a file.
*
* Do not allow the copy if user doesn't have proper permission to access
* the table or the specifically requested columns.
@@ -792,19 +793,22 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
Oid relid;
RawStmt *query = NULL;
- /* Disallow COPY to/from file or program except to superusers. */
- if (!pipe && !superuser())
+ /*
+ * Disallow COPY to/from file or program except to users with the
+ * 'pg_access_server_files' role.
+ */
+ if (!pipe && !is_member_of_role(GetUserId(), DEFAULT_ROLE_ACCESS_SERVER_FILES))
{
if (stmt->is_program)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("must be superuser to COPY to or from an external program"),
+ errmsg("must be superuser or a member of the pg_access_server_files role to COPY to or from an external program"),
errhint("Anyone can COPY to stdout or from stdin. "
"psql's \\copy command also works for anyone.")));
else
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("must be superuser to COPY to or from a file"),
+ errmsg("must be superuser or a member of the pg_access_server_files role to COPY to or from a file"),
errhint("Anyone can COPY to stdout or from stdin. "
"psql's \\copy command also works for anyone.")));
}
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 04f1efbe4b..7a2e6752af 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -22,6 +22,7 @@
#include "access/htup_details.h"
#include "access/xlog_internal.h"
+#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "funcapi.h"
#include "mb/pg_wchar.h"
@@ -54,6 +55,15 @@ convert_and_check_filename(text *arg)
filename = text_to_cstring(arg);
canonicalize_path(filename); /* filename can change length here */
+ /*
+ * 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))
{
/* Disallow '/a/b/data/..' */
@@ -195,11 +205,6 @@ pg_read_file(PG_FUNCTION_ARGS)
char *filename;
text *result;
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser to read files"))));
-
/* handle optional arguments */
if (PG_NARGS() >= 3)
{
@@ -236,11 +241,6 @@ pg_read_binary_file(PG_FUNCTION_ARGS)
char *filename;
bytea *result;
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser to read files"))));
-
/* handle optional arguments */
if (PG_NARGS() >= 3)
{
@@ -313,11 +313,6 @@ pg_stat_file(PG_FUNCTION_ARGS)
TupleDesc tupdesc;
bool missing_ok = false;
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser to get file information"))));
-
/* check the optional argument */
if (PG_NARGS() == 2)
missing_ok = PG_GETARG_BOOL(1);
@@ -399,11 +394,6 @@ pg_ls_dir(PG_FUNCTION_ARGS)
directory_fctx *fctx;
MemoryContext oldcontext;
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser to get directory listings"))));
-
if (SRF_IS_FIRSTCALL())
{
bool missing_ok = false;
diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h
index 9b6b52c9f9..168fc47f3c 100644
--- a/src/include/catalog/pg_authid.h
+++ b/src/include/catalog/pg_authid.h
@@ -108,6 +108,8 @@ DATA(insert OID = 3375 ( "pg_read_all_stats" f t f f f f f -1 _null_ _null_));
#define DEFAULT_ROLE_READ_ALL_STATS 3375
DATA(insert OID = 3377 ( "pg_stat_scan_tables" f t f f f f f -1 _null_ _null_));
#define DEFAULT_ROLE_STAT_SCAN_TABLES 3377
+DATA(insert OID = 3556 ( "pg_access_server_files" f t f f f f f -1 _null_ _null_));
+#define DEFAULT_ROLE_ACCESS_SERVER_FILES 3556
DATA(insert OID = 4200 ( "pg_signal_backend" f t f f f f f -1 _null_ _null_));
#define DEFAULT_ROLE_SIGNAL_BACKENDID 4200
--
2.14.1
signature.asc
Description: PGP signature
