Re: listening addresses

Andrew, Tom:

This will be a really nice feature for those of us with PG servers that
participate in VPNs. Currently I'm blocking certain interfaces using
pg_hba.conf but would prefer a "listen" address instead.

Of course, the drawback to this is that confused DBAs will have their
pg_hba.conf conflict with their postgresql.conf, and cut off all access to
the DB. But I don't know how we can protect against that.

Might I suggest that this default to "127.0.0.1" in postgresql.conf.sample?
This is a reasonably safe default, and would allow us to use the same default
for Windows as for other OSes. It would also eliminate about 15% of the
questions I get on a weekly basis from PHP users. ("uncomment the line
tcpip_sockets ...").

If I had time, I would also love to see setting the password for the postgres
user become part of the initdb script. However, I can see that this wouldn't
work with packages.

--
Josh Berkus
Aglio Database Solutions
San Francisco

Josh Berkus wrote:
> Andrew, Tom:
>
> This will be a really nice feature for those of us with PG servers that
> participate in VPNs. Currently I'm blocking certain interfaces using
> pg_hba.conf but would prefer a "listen" address instead.
>
> Of course, the drawback to this is that confused DBAs will have their
> pg_hba.conf conflict with their postgresql.conf, and cut off all access to
> the DB. But I don't know how we can protect against that.
>
> Might I suggest that this default to "127.0.0.1" in postgresql.conf.sample?
> This is a reasonably safe default, and would allow us to use the same default
> for Windows as for other OSes. It would also eliminate about 15% of the
> questions I get on a weekly basis from PHP users. ("uncomment the line
> tcpip_sockets ...").
>
> If I had time, I would also love to see setting the password for the postgres
> user become part of the initdb script. However, I can see that this wouldn't
> work with packages.

Why couldn't we do something where we ask for a password only if stdin
is from a terminal?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> Might I suggest that this default to "127.0.0.1" in postgresql.conf.sample?

No, the default should be "localhost". Your thinking is too
IPv4-centric.

regards, tom lane

Josh Berkus wrote:

>Andrew, Tom:
>
>This will be a really nice feature for those of us with PG servers that
>participate in VPNs. Currently I'm blocking certain interfaces using
>pg_hba.conf but would prefer a "listen" address instead.
>
>

You can configure listening addresses now using the virtual_host GUC
setting. The documentation was execrable, but Tom has fixed that.

>Of course, the drawback to this is that confused DBAs will have their
>pg_hba.conf conflict with their postgresql.conf, and cut off all access to
>the DB. But I don't know how we can protect against that.
>
>

That surely can't be more than are seen now on IRC who can't contact
their DBs because they forgot to turn on tcpip. Plus this does not
intefere at all with Unix sockets, so they should still be able to use
the local psql (except on Windows, where you have to use tcpip sockets).
I am betting that 95%+ of users will either use the default (no remote
connections) or "*" (bind to all interfaces).

>Might I suggest that this default to "127.0.0.1" in postgresql.conf.sample?
>This is a reasonably safe default, and would allow us to use the same default
>for Windows as for other OSes. It would also eliminate about 15% of the
>questions I get on a weekly basis from PHP users. ("uncomment the line
>tcpip_sockets ...").
>
>
>

The intention is to make "localhost" the default. That should translate
to 127.0.0.1 and ::1 (if they have ipv6 on). Of course, if they have a
broken resolver things might get sticky, but that is true now anyway.

>If I had time, I would also love to see setting the password for the postgres
>user become part of the initdb script. However, I can see that this wouldn't
>work with packages.
>
>
>

Orthogonal problem.

cheers

andrew

Tom,

> No, the default should be "localhost". Your thinking is too
> IPv4-centric.

Good point. My clients are all years away from implementing Ipv6, so I tend
to forget about it.

--
-Josh Berkus
Aglio Database Solutions
San Francisco

I wrote:

> Josh Berkus wrote:
>
>> If I had time, I would also love to see setting the password for the
>> postgres user become part of the initdb script. However, I can see
>> that this wouldn't work with packages.
>>
>>
>>
>
> Orthogonal problem.
>

BTW, initdb is no longer a script - some idiot rewrote it in C :-)

And it does have a switch for setting the superuser password:

-W
--pwprompt

Makes initdb prompt for a password to give the database superuser.
If you don't plan on using password authentication, this is not
important. Otherwise you won't be able to use password
authentication until you have a password set up.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> The intention is to make "localhost" the default. That should translate
> to 127.0.0.1 and ::1 (if they have ipv6 on). Of course, if they have a
> broken resolver things might get sticky, but that is true now anyway.

Just to be clear: right now, if "localhost" doesn't resolve then the
stats collector will not start, but this doesn't prevent use of the
database. It might be a good idea to ensure that the same is still true
after making this change; that is, invalid entries in listen_addresses
should only provoke warnings and not refusal to start (assuming that
we are able to find at least one valid socket to listen to, of course).
I believe that right now, any bad entry in virtual_host causes the
postmaster to error out. That's defensible from one point of view but
on balance I think it's overly paranoid. Any other opinions out there?

regards, tom lane

Tom Lane wrote:

>Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>
>>The intention is to make "localhost" the default. That should translate
>>to 127.0.0.1 and ::1 (if they have ipv6 on). Of course, if they have a
>>broken resolver things might get sticky, but that is true now anyway.
>>
>>
>
>Just to be clear: right now, if "localhost" doesn't resolve then the
>stats collector will not start, but this doesn't prevent use of the
>database. It might be a good idea to ensure that the same is still true
>after making this change; that is, invalid entries in listen_addresses
>should only provoke warnings and not refusal to start (assuming that
>we are able to find at least one valid socket to listen to, of course).
>I believe that right now, any bad entry in virtual_host causes the
>postmaster to error out. That's defensible from one point of view but
>on balance I think it's overly paranoid. Any other opinions out there?
>
>
>

Makes sense - we are not giving anything away but *not* listening to
something.

I did wonder if we should treate "localhost" as a bit special and not
rely on the resolver for it.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> I did wonder if we should treate "localhost" as a bit special and not
> rely on the resolver for it.

I don't think so; we went in the other direction in 7.4 for pgstats.
(It used to try to bind to "127.0.0.1" and now tries "localhost".)
So far I've not seen any evidence that makes me think that was a bad
choice.

regards, tom lane

Tom Lane wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
> > Might I suggest that this default to "127.0.0.1" in postgresql.conf.sample?
>
> No, the default should be "localhost". Your thinking is too
> IPv4-centric.

FYI, once we default to listening on localhost, we need to warn folks
who are using socket permission to control access that they have to turn
off localhost. That needs to be mentioned in the release notes, and in
the SGML docs that talk about socket permissions.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> FYI, once we default to listening on localhost, we need to warn folks
> who are using socket permission to control access that they have to turn
> off localhost. That needs to be mentioned in the release notes, and in
> the SGML docs that talk about socket permissions.

True. I don't see this as a big problem, since use of socket
permissions requires some nondefault settings in postgresql.conf
anyway (IIRC). Adjusting listen_addresses too shouldn't be a big
deal.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > FYI, once we default to listening on localhost, we need to warn folks
> > who are using socket permission to control access that they have to turn
> > off localhost. That needs to be mentioned in the release notes, and in
> > the SGML docs that talk about socket permissions.
>
> True. I don't see this as a big problem, since use of socket
> permissions requires some nondefault settings in postgresql.conf
> anyway (IIRC). Adjusting listen_addresses too shouldn't be a big
> deal.

No, it isn't a problem. I just wanted to mention is so we remember to
address it.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Tom Lane wrote:

>Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>
>>I did wonder if we should treate "localhost" as a bit special and not
>>rely on the resolver for it.
>>
>>
>
>I don't think so; we went in the other direction in 7.4 for pgstats.
>(It used to try to bind to "127.0.0.1" and now tries "localhost".)
>So far I've not seen any evidence that makes me think that was a bad
>choice.
>
>
>

A small problem with it was reported to me a couple of days ago - user
had firewalled off all IP6 traffic. The stats collector happily bound
and connected to the socket, but all the packets fell in the bit bucket.
They found it quite hard to diagnose the problem.

Possible solutions that occurred to me:
. an initial "hello"-"yes i'm here" exchange to validate the address
. a configurable stats collector address
. fix the firewall ("Doctor, it hurts when I move like this." - "So,
stop moving like that.")

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> A small problem with it was reported to me a couple of days ago - user
> had firewalled off all IP6 traffic. The stats collector happily bound
> and connected to the socket, but all the packets fell in the bit bucket.
> They found it quite hard to diagnose the problem.

> Possible solutions that occurred to me:
> . an initial "hello"-"yes i'm here" exchange to validate the address

That one seems reasonable to me. Seems like it would take just a few
more lines of code in the loop that tries to find a working socket to
check that we can send a byte and receive it. You'd have to be careful
not to block if the socket is indeed not working ... also, is it safe to
assume that a byte sent with send() is *immediately* ready to recv()?

regards, tom lane

Tom Lane wrote:

>also, is it safe to
>assume that a byte sent with send() is *immediately* ready to recv()?
>
>
>

If not presumably you could either sleep for a very small interval
before the recv or select on the socket for a very small interval. Half
a second should be ample, I would think.

This doesn't strike me as a high priority item, but possibly worth
putting on the TODO list? Or I could just include it when I get around
to the rest of the listening address stuff we discussed earlier.

cheers

andrew

Tom Lane wrote:

>Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>
>>A small problem with it was reported to me a couple of days ago - user
>>had firewalled off all IP6 traffic. The stats collector happily bound
>>and connected to the socket, but all the packets fell in the bit bucket.
>>They found it quite hard to diagnose the problem.
>>
>>
>
>
>
>>Possible solutions that occurred to me:
>>. an initial "hello"-"yes i'm here" exchange to validate the address
>>
>>
>
>That one seems reasonable to me. Seems like it would take just a few
>more lines of code in the loop that tries to find a working socket to
>check that we can send a byte and receive it. You'd have to be careful
>not to block if the socket is indeed not working ... also, is it safe to
>assume that a byte sent with send() is *immediately* ready to recv()?
>
>
>

This patch attempts to implement the idea, with safety in case the
packet is not immediately available.

comments welcome

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> This patch attempts to implement the idea, with safety in case the
> packet is not immediately available.

Seems like you ought to be testing for failure returns from send() and
recv(). Also, what of EINTR from select()?

regards, tom lane

Tom Lane wrote:

>Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>
>>This patch attempts to implement the idea, with safety in case the
>>packet is not immediately available.
>>
>>
>
>Seems like you ought to be testing for failure returns from send() and
>recv().
>

Good point. will do.

>Also, what of EINTR from select()?
>
>
>

It will fail. Not sure what else it should do - I'm open to suggestions.
Retry?

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> Also, what of EINTR from select()?

> It will fail. Not sure what else it should do - I'm open to suggestions.
> Retry?

Yes. AFAIR, all our other select calls will just loop indefinitely on
EINTR.

regards, tom lane

Andrew Dunstan wrote:

>
>
> Tom Lane wrote:
>
>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>
>>
>>> A small problem with it was reported to me a couple of days ago -
>>> user had firewalled off all IP6 traffic. The stats collector happily
>>> bound and connected to the socket, but all the packets fell in the
>>> bit bucket. They found it quite hard to diagnose the problem.
>>>
>>
>>
>>
>>
>>> Possible solutions that occurred to me:
>>> . an initial "hello"-"yes i'm here" exchange to validate the address
>>>
>>
>>
>> That one seems reasonable to me. Seems like it would take just a few
>> more lines of code in the loop that tries to find a working socket to
>> check that we can send a byte and receive it. You'd have to be careful
>> not to block if the socket is indeed not working ... also, is it safe to
>> assume that a byte sent with send() is *immediately* ready to recv()?
>>
>>
>>
>

Revised patch attached. I think this is about as much trouble as this
problem is worth ;-)

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> A small problem with it was reported to me a couple of days ago -
>> user had firewalled off all IP6 traffic. The stats collector happily
>> bound and connected to the socket, but all the packets fell in the
>> bit bucket. They found it quite hard to diagnose the problem.

> Revised patch attached. I think this is about as much trouble as this
> problem is worth ;-)

I thought the messages were a bit sloppy, which made the patch much less
useful than it should be: we are testing for a very specific failure
mode and we can give a very specific message. Patch as-applied is
attached.

I don't have any real convenient way to set up a situation where this
failure can actually occur. Anyone want to verify that the patch
acts as intended?

regards, tom lane

*** src/backend/postmaster/pgstat.c.orig Mon Mar 15 15:01:57 2004
--- src/backend/postmaster/pgstat.c Mon Mar 22 18:55:29 2004
***************
*** 191,196 ****
--- 191,202 ----
*addr,
hints;
int ret;
+ fd_set rset;
+ struct timeval tv;
+ char test_byte;
+ int sel_res;
+
+ #define TESTBYTEVAL ((char) 199)

/*
* Force start of collector daemon if something to collect
***************
*** 303,308 ****
--- 309,393 ----
ereport(LOG,
(errcode_for_socket_access(),
errmsg("could not connect socket for statistics collector: %m")));
+ closesocket(pgStatSock);
+ pgStatSock = -1;
+ continue;
+ }
+
+ /*
+ * Try to send and receive a one-byte test message on the socket.
+ * This is to catch situations where the socket can be created but
+ * will not actually pass data (for instance, because kernel packet
+ * filtering rules prevent it).
+ */
+ test_byte = TESTBYTEVAL;
+ if (send(pgStatSock, &test_byte, 1, 0) != 1)
+ {
+ ereport(LOG,
+ (errcode_for_socket_access(),
+ errmsg("could not send test message on socket for statistics collector: %m")));
+ closesocket(pgStatSock);
+ pgStatSock = -1;
+ continue;
+ }
+
+ /*
+ * There could possibly be a little delay before the message can be
+ * received. We arbitrarily allow up to half a second before deciding
+ * it's broken.
+ */
+ for (;;) /* need a loop to handle EINTR */
+ {
+ FD_ZERO(&rset);
+ FD_SET(pgStatSock, &rset);
+ tv.tv_sec = 0;
+ tv.tv_usec = 500000;
+ sel_res = select(pgStatSock+1, &rset, NULL, NULL, &tv);
+ if (sel_res >= 0 || errno != EINTR)
+ break;
+ }
+ if (sel_res < 0)
+ {
+ ereport(LOG,
+ (errcode_for_socket_access(),
+ errmsg("select() failed in statistics collector: %m")));
+ closesocket(pgStatSock);
+ pgStatSock = -1;
+ continue;
+ }
+ if (sel_res == 0 || !FD_ISSET(pgStatSock, &rset))
+ {
+ /*
+ * This is the case we actually think is likely, so take pains to
+ * give a specific message for it.
+ *
+ * errno will not be set meaningfully here, so don't use it.
+ */
+ ereport(LOG,
+ (ERRCODE_CONNECTION_FAILURE,
+ errmsg("test message did not get through on socket for statistics collector")));
+ closesocket(pgStatSock);
+ pgStatSock = -1;
+ continue;
+ }
+
+ test_byte++; /* just make sure variable is changed */
+
+ if (recv(pgStatSock, &test_byte, 1, 0) != 1)
+ {
+ ereport(LOG,
+ (errcode_for_socket_access(),
+ errmsg("could not receive test message on socket for statistics collector: %m")));
+ closesocket(pgStatSock);
+ pgStatSock = -1;
+ continue;
+ }
+
+ if (test_byte != TESTBYTEVAL) /* strictly paranoia ... */
+ {
+ ereport(LOG,
+ (ERRCODE_INTERNAL_ERROR,
+ errmsg("incorrect test message transmission on socket for statistics collector")));
closesocket(pgStatSock);
pgStatSock = -1;
continue;

Patch applied by Tom. Thanks.

---------------------------------------------------------------------------

Andrew Dunstan wrote:
>
>
> Andrew Dunstan wrote:
>
> >
> >
> > Tom Lane wrote:
> >
> >> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> >>
> >>
> >>> A small problem with it was reported to me a couple of days ago -
> >>> user had firewalled off all IP6 traffic. The stats collector happily
> >>> bound and connected to the socket, but all the packets fell in the
> >>> bit bucket. They found it quite hard to diagnose the problem.
> >>>
> >>
> >>
> >>
> >>
> >>> Possible solutions that occurred to me:
> >>> . an initial "hello"-"yes i'm here" exchange to validate the address
> >>>
> >>
> >>
> >> That one seems reasonable to me. Seems like it would take just a few
> >> more lines of code in the loop that tries to find a working socket to
> >> check that we can send a byte and receive it. You'd have to be careful
> >> not to block if the socket is indeed not working ... also, is it safe to
> >> assume that a byte sent with send() is *immediately* ready to recv()?
> >>
> >>
> >>
> >
>
> Revised patch attached. I think this is about as much trouble as this
> problem is worth ;-)
>
> cheers
>
> andrew

> Index: src/backend/postmaster/pgstat.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/backend/postmaster/pgstat.c,v
> retrieving revision 1.61
> diff -c -r1.61 pgstat.c
> *** src/backend/postmaster/pgstat.c 15 Mar 2004 16:21:37 -0000 1.61
> --- src/backend/postmaster/pgstat.c 22 Mar 2004 12:47:54 -0000
> ***************
> *** 191,196 ****
> --- 191,200 ----
> *addr,
> hints;
> int ret;
> + fd_set rset;
> + struct timeval tv;
> + char test_byte;
> + int sel_res;
>
> /*
> * Force start of collector daemon if something to collect
> ***************
> *** 307,312 ****
> --- 311,365 ----
> pgStatSock = -1;
> continue;
> }
> +
> + #define TESTBYTEVAL 99
> +
> + /* try to send and receive a test byte on the socket */
> +
> + test_byte = TESTBYTEVAL;
> + if (send(pgStatSock,&test_byte,1,0) != 1)
> + {
> + ereport(LOG,
> + (errcode_for_socket_access(),
> + errmsg("test byte send failure socket for statistics collector: %m")));
> + closesocket(pgStatSock);
> + pgStatSock = -1;
> + continue;
> + }
> + test_byte++;
> + for(;;)
> + {
> + FD_ZERO(&rset);
> + FD_SET(pgStatSock, &rset);
> + tv.tv_sec = 0;
> + tv.tv_usec = 500000;
> + sel_res = select(pgStatSock+1,&rset,NULL,NULL,&tv);
> + if (sel_res != -1 || errno != EINTR)
> + break;
> + }
> + if( sel_res > 0 && FD_ISSET(pgStatSock,&rset)
> + && recv(pgStatSock,&test_byte,1,0) == 1)
> + {
> + if(test_byte != TESTBYTEVAL)
> + {
> + ereport(LOG,
> + (errcode_for_socket_access(),
> + errmsg("incorrect test value on send/receive on socket for statistics collector")));
> + closesocket(pgStatSock);
> + pgStatSock = -1;
> + continue;
> + }
> + }
> + else
> + {
> + ereport(LOG,
> + (errcode_for_socket_access(),
> + errmsg("could receive test byte on socket for statistics collector: %m")));
> + closesocket(pgStatSock);
> + pgStatSock = -1;
> + continue;
> + }
> +
>
> /* If we get here, we have a working socket */
> break;

>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073