Re: Add default role 'pg_access_server_files'

From: Ryan Murphy <ryanfmurphy(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Stephen Frost <sfrost(at)snowman(dot)net>
Subject: Re: Add default role 'pg_access_server_files'
Date: 2018-01-06 20:01:41
Message-ID: 151526890110.1766.16625038855701720951.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Stephen, so far I've read thru your patch and familiarized myself with some of the auth functionality in pg_authid.h and src/backend/utils/adt/acl.c

The only question I have so far about your patch is the last several hunks of the diff, which remove superuser checks without adding anything immediately obvious in their place:

...
@@ -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);
...

I wanted to ask if you have reason to believe that these checks were not necessary (and therefore can be deleted instead of replaced by is_member_of_role() checks like you did elsewhere). I still have limited understanding of the overall code, so really just asking because it's the first thing that jumped out.

Best,
Ryan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2018-01-06 20:29:07 Re: [HACKERS] [PROPOSAL] Temporal query processing with range types
Previous Message Ryan Murphy 2018-01-06 19:10:43 Re: Challenges preventing us moving to 64 bit transaction id (XID)?