Re: [COMMITTERS] pgsql: Use GRANT system to manage access to sensitive functions - Mailing list pgsql-hackers

From Stephen Frost
Subject Re: [COMMITTERS] pgsql: Use GRANT system to manage access to sensitive functions
Date
Msg-id 20160407034331.GJ10850@tamriel.snowman.net
Whole thread Raw
List pgsql-hackers
* Fujii Masao (masao.fujii@gmail.com) wrote:
> On Thu, Apr 7, 2016 at 10:45 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > Use GRANT system to manage access to sensitive functions
> >
> > Now that pg_dump will properly dump out any ACL changes made to
> > functions which exist in pg_catalog, switch to using the GRANT system
> > to manage access to those functions.
> >
> > This means removing 'if (!superuser()) ereport()' checks from the
> > functions themselves and then REVOKEing EXECUTE right from 'public' for
> > these functions in system_views.sql.
>
> This commit revokes the execution privilege on pg_start_backup() from
> a replication role. Doesn't this affect many systems that a replication
> role is used to take a backup? This commit forces administrators of
> those systems to manually grant the privilege to a replication role
> when upgrading the system to 9.6.

That's possible, but I actually consider it pretty unlikely.  barman,
pgBackRest, and our own documentation (at least under the backup
documentation chapter; the function documentation itself does say that
replication roles could also call these functions, though provides no
guidance as to how one would go about creating a replication role) state
that the user used must be a superuser account.  There may be
hand-hacked scripts where the user has gone to the effort of setting up
an additional non-superuser account which has the replication attribute
but, frankly, those scripts are almost certainly broken in far worse
ways when you consider the complications around setting up a proper
backup solution for PG.

Further, currently, you have to have access to read the files on the
filesystem as the postgres user as we don't allow group privileges on
the data directory, so those same hand-hacked scripts are almost
certainly logging in from the postgres user account.

Additionally, just to be clear, pg_basebackup uses the replication
protocol which doesn't go through the GRANT system, and that is
therefore not impacted by this change.

There had been discussion about the replication role during this patch's
lifetime and I had figured that at least pg_start/stop_backup were
reasonably uncontroversial changes.  I plan to propose patches tomorrow
for making the same changes to the various pg_logical_* functions, which
I expect to spark additional discussion.

I do agree that this needs to be mentioned in the release notes, along
with the other changes to pg_start/stop_backup.

We could certainly hack into the pg_dump code to explicitly GRANT access
to these functions to all replication roles, but I'd really rather not
as there's no reason for a role which is issuing pg_start/stop_backup
commands to also be a replication role, with these changes.  Ultimately,
I'd like to make replication roles only able to connect using the
replication protocol.  Using it to provide access to the
pg_start/stop_backup and pg_logical_* functions really isn't a good
approach as the target user ends up with much more access than
necessary.  Propagating that into future versions doesn't strike me as a
good idea.

Thanks!

Stephen

pgsql-hackers by date:

Previous
From: "Karl O. Pinc"
Date:
Subject: Re: Patch to implement pg_current_logfile() function
Next
From: Amit Kapila
Date:
Subject: Re: Speed up Clog Access by increasing CLOG buffers