Re: [HACKERS] pg_stat_replication security

2011-01-23 Thread Magnus Hagander
On Mon, Jan 17, 2011 at 13:14, Magnus Hagander  wrote:
> On Mon, Jan 17, 2011 at 12:11, Itagaki Takahiro
>  wrote:
>> On Mon, Jan 17, 2011 at 19:51, Magnus Hagander  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/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_stat_replication security

2011-01-17 Thread Magnus Hagander
On Mon, Jan 17, 2011 at 12:11, Itagaki Takahiro
 wrote:
> On Mon, Jan 17, 2011 at 19:51, Magnus Hagander  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/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_stat_replication security

2011-01-17 Thread Itagaki Takahiro
On Mon, Jan 17, 2011 at 19:51, Magnus Hagander  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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_stat_replication security

2011-01-17 Thread Magnus Hagander
On Sun, Jan 16, 2011 at 21:57, Josh Berkus  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/
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: user database host One row per WAL sender process, showing process 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.
  
  
 
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);
 	}

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_stat_replication security

2011-01-16 Thread Josh Berkus

>> 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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_stat_replication security

2011-01-16 Thread Magnus Hagander
On Sun, Jan 16, 2011 at 21:51, Josh Berkus  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/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_stat_replication security

2011-01-16 Thread Josh Berkus

> 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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] pg_stat_replication security

2011-01-16 Thread Magnus Hagander
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/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers