From a2c3931a244b67115a0eac1ee5fde9eb7cb4e42c Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Wed, 21 Apr 2010 03:32:53 +0000 Subject: [PATCH] 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. --- doc/src/sgml/client-auth.sgml | 18 +-- doc/src/sgml/high-availability.sgml | 45 ++++---- doc/src/sgml/recovery-config.sgml | 45 +++++--- src/backend/libpq/auth.c | 105 +++++++++++++----- src/backend/libpq/hba.c | 17 ++- .../libpqwalreceiver/libpqwalreceiver.c | 4 +- 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)))); /* -- GitLab