pg_stat_replication security

Started by Magnus Haganderalmost 15 years ago8 messages
#1Magnus Hagander
magnus@hagander.net

pg_stat_replication shows all replication information to all users, no
requirement to be a superuser or anything. That leaks a bunch of
information that regular pg_stat_activity doesn't - such as clients IP
addresses. And also of course all the replication info itself, which
may or may not be a problem.

I suggest pg_stat_replication do just like pg_stat_activity, which is
return NULL in most fields if the user isn't
(superuser||same_user_as_that_session).

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#2Josh Berkus
josh@agliodbs.com
In reply to: Magnus Hagander (#1)
Re: pg_stat_replication security

I suggest pg_stat_replication do just like pg_stat_activity, which is
return NULL in most fields if the user isn't
(superuser||same_user_as_that_session).

What session would that be, exactly?

I suggest instead either "superuser" or "replication" permissions.

--
-- Josh Berkus
PostgreSQL Experts Inc.
http://www.pgexperts.com

#3Magnus Hagander
magnus@hagander.net
In reply to: Josh Berkus (#2)
Re: pg_stat_replication security

On Sun, Jan 16, 2011 at 21:51, Josh Berkus <josh@agliodbs.com> wrote:

I suggest pg_stat_replication do just like pg_stat_activity, which is
return NULL in most fields if the user isn't
(superuser||same_user_as_that_session).

What session would that be, exactly?

The user doing the query to pg_stat_replication being the same as the
user running the replication.

I suggest instead either "superuser" or "replication" permissions.

That's another idea.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#4Josh Berkus
josh@agliodbs.com
In reply to: Magnus Hagander (#3)
Re: pg_stat_replication security

I suggest instead either "superuser" or "replication" permissions.

That's another idea.

Oh, wait. I take that back ... we're trying to encourage users NOT to
use the "replication" user as a login, yes?

--
-- Josh Berkus
PostgreSQL Experts Inc.
http://www.pgexperts.com

#5Magnus Hagander
magnus@hagander.net
In reply to: Josh Berkus (#4)
1 attachment(s)
Re: pg_stat_replication security

On Sun, Jan 16, 2011 at 21:57, Josh Berkus <josh@agliodbs.com> wrote:

I suggest instead either "superuser" or "replication" permissions.

That's another idea.

Oh, wait.  I take that back ... we're trying to encourage users NOT to
use the "replication" user as a login, yes?

yeah.

Here's a patch that limits it to superuser only. We can't easily match
it to the user of the session given the way the walsender data is
returned - it doesn't contain the user information. But limiting it to
superuser only seems perfectly reasonable and in line with the
encouragement not to use the replication user for login.

Objections?

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Attachments:

stat_replication_secure.patchtext/x-patch; charset=US-ASCII; name=stat_replication_secure.patchDownload
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 241131c..306af4e 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -299,7 +299,9 @@ postgres: <replaceable>user</> <replaceable>database</> <replaceable>host</> <re
       <entry>One row per WAL sender process, showing process <acronym>ID</>,
       user OID, user name, application name, client's address and port number,
       time at which the server process began execution, current WAL sender
-      state and transaction log location.
+      state and transaction log location. The columns detailing what exactly
+      the connection is doing are only visible if the user examining the view
+      is a superuser.
      </entry>
      </row>
 
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 0ad6804..dba475d 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1141,8 +1141,20 @@ pg_stat_get_wal_senders(PG_FUNCTION_ARGS)
 
 		memset(nulls, 0, sizeof(nulls));
 		values[0] = Int32GetDatum(walsnd->pid);
-		values[1] = CStringGetTextDatum(WalSndGetStateString(state));
-		values[2] = CStringGetTextDatum(sent_location);
+		if (!superuser())
+		{
+			/*
+			 * Only superusers can see details. Other users only get
+			 * the pid value to know it's a walsender, but no details.
+			 */
+			nulls[1] = true;
+			nulls[2] = true;
+		}
+		else
+		{
+			values[1] = CStringGetTextDatum(WalSndGetStateString(state));
+			values[2] = CStringGetTextDatum(sent_location);
+		}
 
 		tuplestore_putvalues(tupstore, tupdesc, values, nulls);
 	}
#6Itagaki Takahiro
itagaki.takahiro@gmail.com
In reply to: Magnus Hagander (#5)
Re: pg_stat_replication security

On Mon, Jan 17, 2011 at 19:51, Magnus Hagander <magnus@hagander.net> wrote:

Here's a patch that limits it to superuser only. We can't easily match
it to the user of the session given the way the walsender data is
returned - it doesn't contain the user information. But limiting it to
superuser only seems perfectly reasonable and in line with the
encouragement not to use the replication user for login.

Objections?

It hides all fields in pg_stat_wal_senders(). Instead, can we just
revoke usage of the function and view? Or, do we have some plans
to add fields which normal users can see?

--
Itagaki Takahiro

#7Magnus Hagander
magnus@hagander.net
In reply to: Itagaki Takahiro (#6)
Re: pg_stat_replication security

On Mon, Jan 17, 2011 at 12:11, Itagaki Takahiro
<itagaki.takahiro@gmail.com> wrote:

On Mon, Jan 17, 2011 at 19:51, Magnus Hagander <magnus@hagander.net> wrote:

Here's a patch that limits it to superuser only. We can't easily match
it to the user of the session given the way the walsender data is
returned - it doesn't contain the user information. But limiting it to
superuser only seems perfectly reasonable and in line with the
encouragement not to use the replication user for login.

Objections?

It hides all fields in pg_stat_wal_senders(). Instead, can we just
revoke usage of the function and view?  Or, do we have some plans
to add fields which normal users can see?

Yes, for consistency with pg_stat_activity. We let all users see which
other sessions are there, but not what they're doing - seems
reasonable to have the same definitions for replication sessions as
other sessions.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#8Magnus Hagander
magnus@hagander.net
In reply to: Magnus Hagander (#7)
Re: pg_stat_replication security

On Mon, Jan 17, 2011 at 13:14, Magnus Hagander <magnus@hagander.net> wrote:

On Mon, Jan 17, 2011 at 12:11, Itagaki Takahiro
<itagaki.takahiro@gmail.com> wrote:

On Mon, Jan 17, 2011 at 19:51, Magnus Hagander <magnus@hagander.net> wrote:

Here's a patch that limits it to superuser only. We can't easily match
it to the user of the session given the way the walsender data is
returned - it doesn't contain the user information. But limiting it to
superuser only seems perfectly reasonable and in line with the
encouragement not to use the replication user for login.

Objections?

It hides all fields in pg_stat_wal_senders(). Instead, can we just
revoke usage of the function and view?  Or, do we have some plans
to add fields which normal users can see?

Yes, for consistency with pg_stat_activity. We let all users see which
other sessions are there, but not what they're doing - seems
reasonable to have the same definitions for replication sessions as
other sessions.

Committed.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/