Fix pg_hba.conf matching so that replication connections only match records

with database = replication.  The previous coding would allow them to match
ordinary records too, but that seems like a recipe for security breaches.
Improve the messages associated with no-such-pg_hba.conf entry to report
replication connections as such, since that's now a critical aspect of
whether the connection matches.  Make some cursory improvements in the related
documentation, too.

6 files changed, 146 insertions, 88 deletions

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 38b13954ea5..d2f30a47c4e 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/client-auth.sgml,v 1.136 2010/04/03 07:22:53 petere Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/client-auth.sgml,v 1.137 2010/04/21 03:32:53 tgl Exp $ -->
 
 <chapter id="client-authentication">
  <title>Client Authentication</title>
@@ -75,13 +75,14 @@
   <para>
    The general format of the <filename>pg_hba.conf</filename> file is
    a set of records, one per line. Blank lines are ignored, as is any
-   text after the <literal>#</literal> comment character. A record is made
+   text after the <literal>#</literal> comment character.
+   Records cannot be continued across lines.
+   A record is made
    up of a number of fields which are separated by spaces and/or tabs.
    Fields can contain white space if the field value is quoted.
-   Quoting one of the keywords in database or username field (e.g "all"
-   or "replication") makes the name lose its special character, and just
-   match a database or username with that name. Records cannot be
-   continued across lines.
+   Quoting one of the keywords in a database or username field (e.g.,
+   <literal>all</> or <literal>replication</>) makes the word lose its special
+   character, and just match a database or user with that name.
   </para>
 
   <para>
@@ -185,7 +186,8 @@ hostnossl  <replaceable>database</replaceable>  <replaceable>user</replaceable>
        name as the requested database.  (<literal>samegroup</> is an
        obsolete but still accepted spelling of <literal>samerole</>.)
        The value <literal>replication</> specifies that the record
-       matches if streaming replication is requested.
+       matches if a replication connection is requested (note that
+       replication connections do not specify any particular database).
        Otherwise, this is the name of
        a specific <productname>PostgreSQL</productname> database.
        Multiple database names can be supplied by separating them with
@@ -241,7 +243,7 @@ hostnossl  <replaceable>database</replaceable>  <replaceable>user</replaceable>
        Typical examples of a <replaceable>CIDR-address</replaceable> are
        <literal>172.20.143.89/32</literal> for a single host, or
        <literal>172.20.143.0/24</literal> for a small network, or
-       <literal>10.6.0.0/16</literal> for a larger one.  
+       <literal>10.6.0.0/16</literal> for a larger one.
        <literal>0.0.0.0/0</literal> (<quote>all balls</>) represents all addresses.
        To specify a single host, use a CIDR mask of 32 for IPv4 or
        128 for IPv6.  In a network address, do not omit trailing zeroes.
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index d2a8ea77998..766d223bd2b 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/high-availability.sgml,v 1.61 2010/04/20 11:15:06 rhaas Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/high-availability.sgml,v 1.62 2010/04/21 03:32:53 tgl Exp $ -->
 
 <chapter id="high-availability">
  <title>High Availability, Load Balancing, and Replication</title>
@@ -617,7 +617,7 @@ protocol to make nodes agree on a serializable transactional order.
   </sect2>
 
   <sect2 id="preparing-master-for-standby">
-   <title>Preparing Master for Standby Servers</title>
+   <title>Preparing the Master for Standby Servers</title>
 
    <para>
     Set up continuous archiving on the primary to an archive directory
@@ -629,9 +629,13 @@ protocol to make nodes agree on a serializable transactional order.
    </para>
 
    <para>
-    If you want to use streaming replication, set up authentication to allow
-    streaming replication connections and set <varname>max_wal_senders</> in
-    the configuration file of the primary server.
+    If you want to use streaming replication, set up authentication on the
+    primary server to allow replication connections from the standby
+    server(s); that is, provide a suitable entry or entries in
+    <filename>pg_hba.conf</> with the database field set to
+    <literal>replication</>.  Also ensure <varname>max_wal_senders</> is set
+    to a sufficiently large value in the configuration file of the primary
+    server.
    </para>
 
    <para>
@@ -641,7 +645,7 @@ protocol to make nodes agree on a serializable transactional order.
   </sect2>
 
   <sect2 id="standby-server-setup">
-   <title>Setting up the standby server</title>
+   <title>Setting Up a Standby Server</title>
 
    <para>
     To set up the standby server, restore the base backup taken from primary
@@ -656,7 +660,7 @@ protocol to make nodes agree on a serializable transactional order.
      <para>
      Do not use pg_standby or similar tools with the built-in standby mode
      described here. <varname>restore_command</> should return immediately
-     if the file does not exist, the server will retry the command again if
+     if the file does not exist; the server will retry the command again if
      necessary. See <xref linkend="log-shipping-alternative">
      for using tools like pg_standby.
     </para>
@@ -776,22 +780,20 @@ trigger_file = '/path/to/trigger_file'
    <sect3 id="streaming-replication-authentication">
     <title>Authentication</title>
     <para>
-     It is very important that the access privilege for replication be setup
-     properly so that only trusted users can read the WAL stream, because it is
-     easy to extract privileged information from it.
+     It is very important that the access privileges for replication be set up
+     so that only trusted users can read the WAL stream, because it is
+     easy to extract privileged information from it.  Standby servers must
+     authenticate to the primary as a superuser account.
+     So a role with the <literal>SUPERUSER</> and <literal>LOGIN</>
+     privileges needs to be created on the primary.
     </para>
     <para>
-     Only the superuser is allowed to connect to the primary as the replication
-     standby. So a role with the <literal>SUPERUSER</> and <literal>LOGIN</>
-     privileges needs to be created in the primary.
-    </para>
-    <para>
-     Client authentication for replication is controlled by the
+     Client authentication for replication is controlled by a
      <filename>pg_hba.conf</> record specifying <literal>replication</> in the
      <replaceable>database</> field. For example, if the standby is running on
      host IP <literal>192.168.1.100</> and the superuser's name for replication
      is <literal>foo</>, the administrator can add the following line to the
-     <filename>pg_hba.conf</> file on the primary.
+     <filename>pg_hba.conf</> file on the primary:
 
 <programlisting>
 # Allow the user "foo" from host 192.168.1.100 to connect to the primary
@@ -809,18 +811,13 @@ host    replication     foo             192.168.1.100/32        md5
      port <literal>5432</literal>, the superuser's name for replication is
      <literal>foo</>, and the password is <literal>foopass</>, the administrator
      can add the following line to the <filename>recovery.conf</> file on the
-     standby.
+     standby:
 
 <programlisting>
 # The standby connects to the primary that is running on host 192.168.1.50
 # and port 5432 as the user "foo" whose password is "foopass".
 primary_conninfo = 'host=192.168.1.50 port=5432 user=foo password=foopass'
 </programlisting>
-
-      You do not need to specify <literal>database=replication</> in the
-      <varname>primary_conninfo</varname>. The required option will be added
-      automatically. If you mention the database parameter at all within
-      <varname>primary_conninfo</varname> then a FATAL error will be raised.
     </para>
    </sect3>
 
@@ -1959,7 +1956,7 @@ mv /tmp/pg_control /path/to/backup/data/global
       <filename>pg_control</> contains the location where WAL replay will
       begin after restoring from the backup; backing it up first ensures
       that it points to the last restartpoint when the backup started, not
-      some later restartpoint that happened while files were copied to the 
+      some later restartpoint that happened while files were copied to the
       backup.
      </para>
     </listitem>
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 85d3b7eb3fb..58fa5b0b107 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/recovery-config.sgml,v 2.4 2010/04/07 10:58:49 heikki Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/recovery-config.sgml,v 2.5 2010/04/21 03:32:53 tgl Exp $ -->
 
 <chapter Id="recovery-config">
   <title>Recovery Configuration</title>
@@ -10,12 +10,10 @@
   </indexterm>
 
    <para>
-      This chapter describes the settings available in
-      <filename>recovery.conf</> file. They apply only for the duration of
-      the recovery.  (A sample file, <filename>share/recovery.conf.sample</>,
-      exists in the installation's <filename>share/</> directory.)  They must
-      be reset for any subsequent recovery you wish to perform. They cannot
-      be changed once recovery has begun.
+    This chapter describes the settings available in the
+    <filename>recovery.conf</> file. They apply only for the duration of the
+    recovery.  They must be reset for any subsequent recovery you wish to
+    perform.  They cannot be changed once recovery has begun.
    </para>
 
    <para>
@@ -26,6 +24,11 @@
      value, write two quotes (<literal>''</>).
    </para>
 
+   <para>
+    A sample file, <filename>share/recovery.conf.sample</>,
+    is provided in the installation's <filename>share/</> directory.
+   </para>
+
   <sect1 id="archive-recovery-settings">
 
     <title>Archive recovery settings</title>
@@ -208,9 +211,10 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
          <para>
           Specifies whether to start the <productname>PostgreSQL</> server as
           a standby. If this parameter is <literal>on</>, the server will
-          not end recovery when the end of archived WAL is reached, but
-          will keep trying to continue recovery using <varname>restore_command</>
-          and by connecting to the primary server as specified by the
+          not stop recovery when the end of archived WAL is reached, but
+          will keep trying to continue recovery by fetching new WAL segments
+          using <varname>restore_command</>
+          and/or by connecting to the primary server as specified by the
           <varname>primary_conninfo</> setting.
          </para>
         </listitem>
@@ -229,14 +233,19 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
           defaults are used.
          </para>
          <para>
-          The built-in replication requires that a host name (or host address)
-          or port number which the primary server listens on be
-          specified in this string. Also ensure that a role with
-          the <literal>SUPERUSER</> and <literal>LOGIN</> privileges on the
-          primary is set (see
-          <xref linkend="streaming-replication-authentication">). Note that
-          the password needs to be set if the primary demands password
-          authentication.
+          The connection string should specify the host name (or address)
+          of the primary server, as well as the port number if it is not
+          the same as the standby server's default.
+          Also specify a user name corresponding to a role that has the
+          <literal>SUPERUSER</> and <literal>LOGIN</> privileges on the
+          primary (see
+          <xref linkend="streaming-replication-authentication">).
+          A password needs to be provided too, if the primary demands password
+          authentication.  (The password can be provided either in
+          the <varname>primary_conninfo</varname> string or in a separate
+          <filename>~/.pgpass</> file on the standby server.)
+          Do not specify a database name in the
+          <varname>primary_conninfo</varname> string.
          </para>
          <para>
           This setting has no effect if <varname>standby_mode</> is <literal>off</>.
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index ac26317d264..c4e234186e6 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/libpq/auth.c,v 1.199 2010/04/19 19:02:18 sriggs Exp $
+ *	  $PostgreSQL: pgsql/src/backend/libpq/auth.c,v 1.200 2010/04/21 03:32:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -36,6 +36,7 @@
 #include "libpq/pqformat.h"
 #include "libpq/md5.h"
 #include "miscadmin.h"
+#include "replication/walsender.h"
 #include "storage/ipc.h"
 
 
@@ -250,6 +251,7 @@ auth_failed(Port *port, int status)
 	switch (port->hba->auth_method)
 	{
 		case uaReject:
+		case uaImplicitReject:
 			errstr = gettext_noop("authentication failed for user \"%s\": host rejected");
 			break;
 		case uaKrb5:
@@ -363,12 +365,14 @@ ClientAuthentication(Port *port)
 		case uaReject:
 
 			/*
-			 * An explicit "reject" entry in pg_hba.conf. Take pity on the poor
-			 * user and issue a helpful error message.
-			 * NOTE: this is not a security breach, because all the info
-			 * reported here is known at the frontend and must be assumed
-			 * known to bad guys. We're merely helping out the less clueful
-			 * good guys.
+			 * An explicit "reject" entry in pg_hba.conf.  This report exposes
+			 * the fact that there's an explicit reject entry, which is perhaps
+			 * not so desirable from a security standpoint; but the message
+			 * for an implicit reject could confuse the DBA a lot when the
+			 * true situation is a match to an explicit reject.  And we don't
+			 * want to change the message for an implicit reject.  As noted
+			 * below, the additional information shown here doesn't expose
+			 * anything not known to an attacker.
 			 */
 			{
 				char		hostinfo[NI_MAXHOST];
@@ -378,29 +382,50 @@ ClientAuthentication(Port *port)
 								   NULL, 0,
 								   NI_NUMERICHOST);
 
+				if (am_walsender)
+				{
 #ifdef USE_SSL
-				ereport(FATAL,
-						(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
-						 errmsg("pg_hba.conf rejects host \"%s\", user \"%s\", database \"%s\", %s",
-							  hostinfo, port->user_name, port->database_name,
-								port->ssl ? _("SSL on") : _("SSL off"))));
+					ereport(FATAL,
+							(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+							 errmsg("pg_hba.conf rejects replication connection for host \"%s\", user \"%s\", %s",
+									hostinfo, port->user_name,
+									port->ssl ? _("SSL on") : _("SSL off"))));
 #else
-				ereport(FATAL,
-						(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
-						 errmsg("pg_hba.conf rejects host \"%s\", user \"%s\", database \"%s\"",
-						   hostinfo, port->user_name, port->database_name)));
+					ereport(FATAL,
+							(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+							 errmsg("pg_hba.conf rejects replication connection for host \"%s\", user \"%s\"",
+									hostinfo, port->user_name)));
 #endif
+				}
+				else
+				{
+#ifdef USE_SSL
+					ereport(FATAL,
+							(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+							 errmsg("pg_hba.conf rejects connection for host \"%s\", user \"%s\", database \"%s\", %s",
+									hostinfo, port->user_name,
+									port->database_name,
+									port->ssl ? _("SSL on") : _("SSL off"))));
+#else
+					ereport(FATAL,
+							(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+							 errmsg("pg_hba.conf rejects connection for host \"%s\", user \"%s\", database \"%s\"",
+									hostinfo, port->user_name,
+									port->database_name)));
+#endif
+				}
 				break;
 			}
 
 		case uaImplicitReject:
 
 			/*
-			 * No matching entry so tell the user we fell through.
-			 * NOTE: this is not a security breach, because all the info
-			 * reported here is known at the frontend and must be assumed
-			 * known to bad guys. We're merely helping out the less clueful
-			 * good guys.
+			 * No matching entry, so tell the user we fell through.
+			 *
+			 * NOTE: the extra info reported here is not a security breach,
+			 * because all that info is known at the frontend and must be
+			 * assumed known to bad guys.  We're merely helping out the less
+			 * clueful good guys.
 			 */
 			{
 				char		hostinfo[NI_MAXHOST];
@@ -410,18 +435,38 @@ ClientAuthentication(Port *port)
 								   NULL, 0,
 								   NI_NUMERICHOST);
 
+				if (am_walsender)
+				{
 #ifdef USE_SSL
-				ereport(FATAL,
-						(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
-						 errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\", %s",
-							  hostinfo, port->user_name, port->database_name,
-								port->ssl ? _("SSL on") : _("SSL off"))));
+					ereport(FATAL,
+							(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+							 errmsg("no pg_hba.conf entry for replication connection from host \"%s\", user \"%s\", %s",
+									hostinfo, port->user_name,
+									port->ssl ? _("SSL on") : _("SSL off"))));
 #else
-				ereport(FATAL,
-						(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
-						 errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\"",
-						   hostinfo, port->user_name, port->database_name)));
+					ereport(FATAL,
+							(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+							 errmsg("no pg_hba.conf entry for replication connection from host \"%s\", user \"%s\"",
+									hostinfo, port->user_name)));
+#endif
+				}
+				else
+				{
+#ifdef USE_SSL
+					ereport(FATAL,
+							(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+							 errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\", %s",
+									hostinfo, port->user_name,
+									port->database_name,
+									port->ssl ? _("SSL on") : _("SSL off"))));
+#else
+					ereport(FATAL,
+							(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+							 errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\"",
+									hostinfo, port->user_name,
+									port->database_name)));
 #endif
+				}
 				break;
 			}
 
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 11443f76e2d..e5fb65f75d3 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/libpq/hba.c,v 1.205 2010/04/19 19:02:18 sriggs Exp $
+ *	  $PostgreSQL: pgsql/src/backend/libpq/hba.c,v 1.206 2010/04/21 03:32:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -513,7 +513,13 @@ check_db(const char *dbname, const char *role, Oid roleid, char *param_str)
 		 tok != NULL;
 		 tok = strtok(NULL, MULTI_VALUE_SEP))
 	{
-		if (strcmp(tok, "all\n") == 0)
+		if (am_walsender)
+		{
+			/* walsender connections can only match replication keyword */
+			if (strcmp(tok, "replication\n") == 0)
+				return true;
+		}
+		else if (strcmp(tok, "all\n") == 0)
 			return true;
 		else if (strcmp(tok, "sameuser\n") == 0)
 		{
@@ -526,9 +532,8 @@ check_db(const char *dbname, const char *role, Oid roleid, char *param_str)
 			if (is_member(roleid, dbname))
 				return true;
 		}
-		else if (strcmp(tok, "replication\n") == 0 &&
-				 am_walsender)
-			return true;
+		else if (strcmp(tok, "replication\n") == 0)
+			continue;			/* never match this if not walsender */
 		else if (strcmp(tok, dbname) == 0)
 			return true;
 	}
@@ -1812,7 +1817,7 @@ load_ident(void)
  *
  *	Note that STATUS_ERROR indicates a problem with the hba config file.
  *	If the file is OK but does not contain any entry matching the request,
- *	we return STATUS_OK and method = uaReject.
+ *	we return STATUS_OK and method = uaImplicitReject.
  */
 int
 hba_getauthmethod(hbaPort *port)
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index d41858e49a2..1807fde9e4b 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c,v 1.9 2010/04/19 14:10:45 mha Exp $
+ *	  $PostgreSQL: pgsql/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c,v 1.10 2010/04/21 03:32:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -91,7 +91,7 @@ libpqrcv_connect(char *conninfo, XLogRecPtr startpoint)
 	streamConn = PQconnectdb(conninfo_repl);
 	if (PQstatus(streamConn) != CONNECTION_OK)
 		ereport(ERROR,
-				(errmsg("could not connect to the primary server : %s",
+				(errmsg("could not connect to the primary server: %s",
 						PQerrorMessage(streamConn))));
 
 	/*