'replication' keyword on .pgpass (Streaming Replication)

Fujii Masao escribi�:

On Sat, Dec 26, 2009 at 10:55 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, Dec 25, 2009 at 9:56 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

I don't see the use case for it - .pgpass is for single users, not a whole
cluster. And it does support wildcards, which takes care of the 'all' case.
In the case of pg_hba.conf we don't know in advance who will actually be
connecting. But in the case of .pgpass we do, so the extra utility of
'sameuser', 'samerole' and 'samegroup' in this case is not apparent to me.

OK, I might need to focus only on the use of replication, without
being avaricious.

The attached patch supports new keyword 'replication' on .pgpass file.
This keyword is used to specify the password for the standby server to
connect to the primary server.

Would it make more sense to have a separate password file for the
replication stuff? Say .pgreplicationpass or whatever.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Wed, Jan 6, 2010 at 15:02, Alvaro Herrera <alvherre@commandprompt.com> wrote:

Fujii Masao escribió:

On Sat, Dec 26, 2009 at 10:55 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, Dec 25, 2009 at 9:56 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

I don't see the use case for it - .pgpass is for single users, not a whole
cluster. And it does support wildcards, which takes care of the 'all' case.
In the case of pg_hba.conf we don't know in advance who will actually be
connecting. But in the case of .pgpass we do, so the extra utility of
'sameuser', 'samerole' and 'samegroup' in this case is not apparent to me.

OK, I might need to focus only on the use of replication, without
being avaricious.

The attached patch supports new keyword 'replication' on .pgpass file.
This keyword is used to specify the password for the standby server to
connect to the primary server.

Would it make more sense to have a separate password file for the
replication stuff?  Say .pgreplicationpass or whatever.

I haven't read up on the rest of the patch, but where do we put the
rest of the information about the replication master? Like which IP
and port to connect to? Perhaps it could/should go there?

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

Fujii Masao <masao.fujii@gmail.com> writes:

The attached patch supports new keyword 'replication' on .pgpass file.
This keyword is used to specify the password for the standby server to
connect to the primary server.

This strikes me as a completely bad idea. We need get no farther than
the point that it assumes nobody can have a database named "replication"
(although I notice the patch also appears to assume that libpq knows
internally that the connection is for replication --- I thought we were
going to avoid libpq changes for SR?)

I don't see any real strong reason why a .pgpass entry for this purpose
couldn't depend on having "*" in the database field. But the later
comments that the password should be in some other configuration file
altogether are probably an even better idea.

regards, tom lane

On Wed, Jan 6, 2010 at 11:11 PM, Magnus Hagander <magnus@hagander.net> wrote:

I haven't read up on the rest of the patch, but where do we put the
rest of the information about the replication master? Like which IP
and port to connect to? Perhaps it could/should go there?

Such information are supplied in the parameter 'primary_conninfo' of
recovery.conf. For example;

primary_conninfo = 'host=192.168.1.50 port=5432 user=foo'

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Thu, Jan 7, 2010 at 1:51 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

This strikes me as a completely bad idea.  We need get no farther than
the point that it assumes nobody can have a database named "replication"

Though I might misunderstand your point. My proposal would force the users
who have a database named "replication" to change their .pgpass file and
enclose the "replication" database field in double quote when they upgrade
the Postgres to v8.5. For example;

192.168.1.50:5432:"replication":foo:foopass

The same problem also exists in pg_hba.conf. It's because I introduced
new keyword "replication" in pg_hba.conf to authenticate the standby
server. This restriction is not acceptable? If so, I'd need to consider
an authentication configuration for replication again: introduce new
configuration file? just change the keyword name to "unpopular" one?...

(although I notice the patch also appears to assume that libpq knows
internally that the connection is for replication --- I thought we were
going to avoid libpq changes for SR?)

Yes, but I changed the libpq just a bit; if the conninfo string including
"replication=1" is given to PQconnectdb(), the libpq determines that this
connection is for replication, and puts the replication-request in a startup
packet. This is for a backend to switch to walsender mode when the startup
packet arrives. Otherwise, we would have to authenticate such backend twice
on different context, i.e., a normal backend and walsender. So the settings
for each context would be required in pg_hba.conf. This is odd, I think.

I don't see any real strong reason why a .pgpass entry for this purpose
couldn't depend on having "*" in the database field.

Oh, you are right. Since a role cannot use a different password per database,
"*" in the database field seems to be enough.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Thu, Jan 7, 2010 at 07:19, Fujii Masao <masao.fujii@gmail.com> wrote:

On Wed, Jan 6, 2010 at 11:11 PM, Magnus Hagander <magnus@hagander.net> wrote:

I haven't read up on the rest of the patch, but where do we put the
rest of the information about the replication master? Like which IP
and port to connect to? Perhaps it could/should go there?

Such information are supplied in the parameter 'primary_conninfo' of
recovery.conf. For example;

   primary_conninfo = 'host=192.168.1.50 port=5432 user=foo'

So the password can just go there, no?

If an untrusted user has direct read access to your PGDATA directory
(where recovery.conf goes), you've lost already...

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

On Thu, Jan 7, 2010 at 09:26, Fujii Masao <masao.fujii@gmail.com> wrote:

The same problem also exists in pg_hba.conf. It's because I introduced
new keyword "replication" in pg_hba.conf to authenticate the standby
server. This restriction is not acceptable? If so, I'd need to consider
an authentication configuration for replication again: introduce new
configuration file? just change the keyword name to "unpopular" one?...

I certainly think there are a lot of installations out there with a
database named "replication". That doesn't mean it's unacceptable
though.

However, wouldn't it make more logical sense to replace "host/hostssl"
with "replication/replicationssl" rather than overload the database
field?

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

On Thu, Jan 7, 2010 at 5:44 PM, Magnus Hagander <magnus@hagander.net> wrote:

Such information are supplied in the parameter 'primary_conninfo' of
recovery.conf. For example;

   primary_conninfo = 'host=192.168.1.50 port=5432 user=foo'

So the password can just go there, no?

Yeah, the password can be supplied in primary_conninfo.

primary_conninfo = 'host=192.168.1.50 port=5432 user=foo password=foopass'

Also you can use the environment variable PGPASSWORD.
Is this enough? I was thinking that some people would
complain that only the password for replication cannot
be supplied in .pgpass.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Thu, Jan 7, 2010 at 5:46 PM, Magnus Hagander <magnus@hagander.net> wrote:

However, wouldn't it make more logical sense to replace "host/hostssl"
with "replication/replicationssl" rather than overload the database
field?

Seems good. How about the following formats?

replication user CIDR-address auth-method [auth-options]
replicationssl user CIDR-address auth-method [auth-options]
replication user IP-address IP-mask auth-method [auth-options]
replicationssl user IP-address IP-mask auth-method [auth-options]

Note that "database" field has been removed since it's useless
for replication.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Fujii Masao wrote:

On Thu, Jan 7, 2010 at 5:44 PM, Magnus Hagander <magnus@hagander.net> wrote:

Such information are supplied in the parameter 'primary_conninfo' of
recovery.conf. For example;

primary_conninfo = 'host=192.168.1.50 port=5432 user=foo'

So the password can just go there, no?

Yeah, the password can be supplied in primary_conninfo.

primary_conninfo = 'host=192.168.1.50 port=5432 user=foo password=foopass'

Also you can use the environment variable PGPASSWORD.
Is this enough? I was thinking that some people would
complain that only the password for replication cannot
be supplied in .pgpass.

That seems enough to me.

BTW, how do you set up authentication using an SSL certificate? ISTM
that's the way we should be encouraging people to configure
authentication between a master and standby, rather than type a password
to a file.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Tom Lane wrote:

Fujii Masao <masao.fujii@gmail.com> writes:

The attached patch supports new keyword 'replication' on .pgpass file.
This keyword is used to specify the password for the standby server to
connect to the primary server.

This strikes me as a completely bad idea. We need get no farther than
the point that it assumes nobody can have a database named "replication"
(although I notice the patch also appears to assume that libpq knows
internally that the connection is for replication --- I thought we were
going to avoid libpq changes for SR?)

As the patch stands there's an extra startup packet, similar to
SSLRequest, in the connection handshake. But I think we should get rid
of that too, and just send an extra "replication=on" option along with
username and other options in the startup packet.

(as Fujii pointed out already, you can have a database called
"replication". But you need to quote it in pg_hba.conf, like you need to
quote "all" and "sameuser/group/role".)

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Magnus Hagander wrote:

However, wouldn't it make more logical sense to replace "host/hostssl"
with "replication/replicationssl" rather than overload the database
field?

It makes more sense to me to overload the database field. When you
connect for replication, you're not connecting to any particular
database, but to a special walsender mode.

Note that 'local' makes sense for replication too; you can connect for
replication via a Unix-domain socket. Replication isn't a connection
method like host/hostssl/local are.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Thu, Jan 7, 2010 at 10:21, Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Jan 7, 2010 at 5:46 PM, Magnus Hagander <magnus@hagander.net> wrote:

However, wouldn't it make more logical sense to replace "host/hostssl"
with "replication/replicationssl" rather than overload the database
field?

Seems good. How about the following formats?

 replication     user  CIDR-address  auth-method  [auth-options]
 replicationssl  user  CIDR-address  auth-method  [auth-options]
 replication     user  IP-address  IP-mask  auth-method  [auth-options]
 replicationssl  user  IP-address  IP-mask  auth-method  [auth-options]

Note that "database" field has been removed since it's useless
for replication.

Hm, no, I think I withdraw my comment about pg_hba.conf. It seems
better to overload the database name here. I'm not particularly keen
on yet another different set of columns, which is what happens when
you remove the database field.

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

On Thu, Jan 7, 2010 at 13:34, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

Fujii Masao wrote:

On Thu, Jan 7, 2010 at 5:44 PM, Magnus Hagander <magnus@hagander.net> wrote:

Such information are supplied in the parameter 'primary_conninfo' of
recovery.conf. For example;

   primary_conninfo = 'host=192.168.1.50 port=5432 user=foo'

So the password can just go there, no?

Yeah, the password can be supplied in primary_conninfo.

   primary_conninfo = 'host=192.168.1.50 port=5432 user=foo password=foopass'

Also you can use the environment variable PGPASSWORD.
Is this enough? I was thinking that some people would
complain that only the password for replication cannot
be supplied in .pgpass.

That seems enough to me.

BTW, how do you set up authentication using an SSL certificate? ISTM
that's the way we should be encouraging people to configure
authentication between a master and standby, rather than type a password
to a file.

Connectoin parameters: sslcert, sslkey and sslrootcert in most cases.
Or just put the key in ~/.postgresql/postgresql.key.

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

Fujii Masao <masao.fujii@gmail.com> writes:

On Thu, Jan 7, 2010 at 5:46 PM, Magnus Hagander <magnus@hagander.net> wrote:

However, wouldn't it make more logical sense to replace "host/hostssl"
with "replication/replicationssl" rather than overload the database
field?

Seems good. How about the following formats?

replication user CIDR-address auth-method [auth-options]
replicationssl user CIDR-address auth-method [auth-options]
replication user IP-address IP-mask auth-method [auth-options]
replicationssl user IP-address IP-mask auth-method [auth-options]

Note that "database" field has been removed since it's useless
for replication.

I'm getting more and more confused here. I thought we were talking
about client-side .pgpass. This seems to be talking about pg_hba.conf.

regards, tom lane

Tom Lane wrote:

I'm getting more and more confused here. I thought we were talking
about client-side .pgpass. This seems to be talking about pg_hba.conf.

Yeah, the topic was covertly changed.

It seems we have consensus to not change .pgpass, and to leave
pg_hba.conf as it is now in the patch as well.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com