Re: dblink connection security - Mailing list pgsql-patches

Tom Lane wrote:
> Here's a straw-man proposal that we could perhaps do for 8.3:
>
> 1. Invent a libpq connection-status function
>
>     bool PQconnectionUsedPassword(const PGconn *conn);
>
> This returns true if the server had demanded a password during the
> authentication phase.  Aside from solving the immediate problem, this
> can be useful for regular clients such as psql: it could be applied to a
> failed connection object to decide whether to prompt for a password
> (replacing the current egregious hack of strcmp'ing the error message).
>
> 2. Make dblink close the connection and throw error if called by a
> non-superuser and PQconnectionUsedPassword returns false.

Attached patch implements this proposal, including documentation
changes. I'll work separately on the back-branch version.

Any comments/objections?

Joe
Index: contrib/dblink/dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.63
diff -c -r1.63 dblink.c
*** contrib/dblink/dblink.c    6 Apr 2007 04:21:41 -0000    1.63
--- contrib/dblink/dblink.c    7 Jul 2007 22:49:05 -0000
***************
*** 37,42 ****
--- 37,43 ----
  #include "libpq-fe.h"
  #include "fmgr.h"
  #include "funcapi.h"
+ #include "miscadmin.h"
  #include "access/heapam.h"
  #include "access/tupdesc.h"
  #include "catalog/namespace.h"
***************
*** 245,250 ****
--- 246,261 ----
                   errdetail("%s", msg)));
      }

+     if (!superuser())
+     {
+         if (!PQconnectionUsedPassword(conn))
+             ereport(ERROR,
+                     (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
+                      errmsg("connection without password not allowed"),
+                      errdetail("non-superuser cannot connect if server does not request password"),
+                      errhint("target server authentication method must be changed")));
+     }
+
      if (connname)
      {
          rconn->conn = conn;
Index: contrib/dblink/dblink.sql.in
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.sql.in,v
retrieving revision 1.11
diff -c -r1.11 dblink.sql.in
*** contrib/dblink/dblink.sql.in    2 Sep 2006 21:11:15 -0000    1.11
--- contrib/dblink/dblink.sql.in    8 Jul 2007 01:22:13 -0000
***************
*** 1,3 ****
--- 1,5 ----
+ -- dblink_connect now restricts non-superusers to password
+ -- authenticated connections
  CREATE OR REPLACE FUNCTION dblink_connect (text)
  RETURNS text
  AS 'MODULE_PATHNAME','dblink_connect'
***************
*** 8,13 ****
--- 10,31 ----
  AS 'MODULE_PATHNAME','dblink_connect'
  LANGUAGE C STRICT;

+ -- dblink_connect_u allows non-superusers to use
+ -- non-password authenticated connections, but initially
+ -- privileges are revoked from public
+ CREATE OR REPLACE FUNCTION dblink_connect_u (text)
+ RETURNS text
+ AS 'MODULE_PATHNAME','dblink_connect'
+ LANGUAGE C STRICT SECURITY DEFINER;
+
+ CREATE OR REPLACE FUNCTION dblink_connect_u (text, text)
+ RETURNS text
+ AS 'MODULE_PATHNAME','dblink_connect'
+ LANGUAGE C STRICT SECURITY DEFINER;
+
+ REVOKE ALL ON FUNCTION dblink_connect_u (text) FROM public;
+ REVOKE ALL ON FUNCTION dblink_connect_u (text, text) FROM public;
+
  CREATE OR REPLACE FUNCTION dblink_disconnect ()
  RETURNS text
  AS 'MODULE_PATHNAME','dblink_disconnect'
Index: contrib/dblink/doc/connection
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/doc/connection,v
retrieving revision 1.4
diff -c -r1.4 connection
*** contrib/dblink/doc/connection    11 Mar 2006 04:38:29 -0000    1.4
--- contrib/dblink/doc/connection    8 Jul 2007 01:51:07 -0000
***************
*** 27,32 ****
--- 27,38 ----

    Returns status = "OK"

+ Notes
+
+   Only superusers may use dblink_connect to create non-password
+   authenticated connections. If non-superusers need this capability,
+   use dblink_connect_u instead.
+
  Example usage

  select dblink_connect('dbname=postgres');
***************
*** 44,49 ****
--- 50,95 ----
  ==================================================================
  Name

+ dblink_connect_u -- Opens a persistent connection to a remote database
+
+ Synopsis
+
+ dblink_connect_u(text connstr)
+ dblink_connect_u(text connname, text connstr)
+
+ Inputs
+
+   connname
+     if 2 arguments are given, the first is used as a name for a persistent
+     connection
+
+   connstr
+
+     standard libpq format connection string,
+     e.g. "hostaddr=127.0.0.1 port=5432 dbname=mydb user=postgres password=mypasswd"
+
+     if only one argument is given, the connection is unnamed; only one unnamed
+     connection can exist at a time
+
+ Outputs
+
+   Returns status = "OK"
+
+ Notes
+
+   With dblink_connect_u, a non-superuser may connect to any database server
+   using any authentication method. If the authentication method specified
+   for a particular user does not require a password, impersonation and
+   therefore escalation of privileges may occur. For this reason,
+   dblink_connect_u is initially installed with all privileges revoked from
+   public. Privilege to these functions should be granted with care.
+
+ Example usage
+
+
+ ==================================================================
+ Name
+
  dblink_disconnect -- Closes a persistent connection to a remote database

  Synopsis
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /opt/src/cvs/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.235
diff -c -r1.235 libpq.sgml
*** doc/src/sgml/libpq.sgml    30 Mar 2007 03:19:02 -0000    1.235
--- doc/src/sgml/libpq.sgml    8 Jul 2007 02:06:36 -0000
***************
*** 1059,1064 ****
--- 1059,1078 ----
       </listitem>
      </varlistentry>

+     <varlistentry>
+      <term><function>PQconnectionUsedPassword</function><indexterm><primary>PQconnectionUsedPassword</></></term>
+      <listitem>
+       <para>
+        Returns true (1) if the connection authentication method
+        required a password to be supplied. Returns false (0)
+        otherwise.
+        <synopsis>
+        bool PQconnectionUsedPassword(const PGconn *conn);
+        </synopsis>
+       </para>
+      </listitem>
+     </varlistentry>
+
  </variablelist>
  </para>

Index: src/include/libpq/pqcomm.h
===================================================================
RCS file: /opt/src/cvs/pgsql/src/include/libpq/pqcomm.h,v
retrieving revision 1.102
diff -c -r1.102 pqcomm.h
*** src/include/libpq/pqcomm.h    5 Jan 2007 22:19:55 -0000    1.102
--- src/include/libpq/pqcomm.h    8 Jul 2007 01:16:29 -0000
***************
*** 156,161 ****
--- 156,162 ----
  #define AUTH_REQ_CRYPT        4    /* crypt password */
  #define AUTH_REQ_MD5        5    /* md5 password */
  #define AUTH_REQ_SCM_CREDS    6    /* transfer SCM credentials */
+ #define AUTH_REQ_UNK        7    /* User has not yet attempted to authenticate */

  typedef uint32 AuthRequest;

Index: src/interfaces/libpq/exports.txt
===================================================================
RCS file: /opt/src/cvs/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.15
diff -c -r1.15 exports.txt
*** src/interfaces/libpq/exports.txt    3 Mar 2007 19:52:46 -0000    1.15
--- src/interfaces/libpq/exports.txt    7 Jul 2007 22:07:48 -0000
***************
*** 137,139 ****
--- 137,140 ----
  PQsendDescribePrepared    135
  PQsendDescribePortal      136
  lo_truncate               137
+ PQconnectionUsedPassword  138
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /opt/src/cvs/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.345
diff -c -r1.345 fe-connect.c
*** src/interfaces/libpq/fe-connect.c    8 Mar 2007 19:27:28 -0000    1.345
--- src/interfaces/libpq/fe-connect.c    7 Jul 2007 23:07:02 -0000
***************
*** 1641,1646 ****
--- 1641,1650 ----
                      return PGRES_POLLING_READING;
                  }

+                 /* save the authentication request type */
+                 if (conn->areq == AUTH_REQ_UNK)
+                     conn->areq = areq;
+
                  /* Get the password salt if there is one. */
                  if (areq == AUTH_REQ_MD5)
                  {
***************
*** 1873,1878 ****
--- 1877,1883 ----
      conn->std_strings = false;    /* unless server says differently */
      conn->verbosity = PQERRORS_DEFAULT;
      conn->sock = -1;
+     conn->areq = AUTH_REQ_UNK;
  #ifdef USE_SSL
      conn->allow_ssl_try = true;
      conn->wait_ssl_try = false;
***************
*** 3441,3446 ****
--- 3446,3462 ----
      return status;
  }

+ bool
+ PQconnectionUsedPassword(const PGconn *conn)
+ {
+     if (conn->areq == AUTH_REQ_MD5 ||
+         conn->areq == AUTH_REQ_CRYPT ||
+         conn->areq == AUTH_REQ_PASSWORD)
+         return true;
+     else
+         return false;
+ }
+
  PGVerbosity
  PQsetErrorVerbosity(PGconn *conn, PGVerbosity verbosity)
  {
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /opt/src/cvs/pgsql/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.136
diff -c -r1.136 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h    3 Mar 2007 19:52:46 -0000    1.136
--- src/interfaces/libpq/libpq-fe.h    7 Jul 2007 21:55:12 -0000
***************
*** 23,32 ****
  #include <stdio.h>

  /*
!  * postgres_ext.h defines the backend's externally visible types,
   * such as Oid.
   */
  #include "postgres_ext.h"

  /* Application-visible enum types */

--- 23,33 ----
  #include <stdio.h>

  /*
!  * defines the backend's externally visible types,
   * such as Oid.
   */
  #include "postgres_ext.h"
+ #include "postgres_fe.h"

  /* Application-visible enum types */

***************
*** 265,270 ****
--- 266,272 ----
  extern int    PQbackendPID(const PGconn *conn);
  extern int    PQclientEncoding(const PGconn *conn);
  extern int    PQsetClientEncoding(PGconn *conn, const char *encoding);
+ extern bool PQconnectionUsedPassword(const PGconn *conn);

  /* Get the OpenSSL structure associated with a connection. Returns NULL for
   * unencrypted connections or if any other TLS library is in use. */
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /opt/src/cvs/pgsql/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.119
diff -c -r1.119 libpq-int.h
*** src/interfaces/libpq/libpq-int.h    3 Mar 2007 19:52:47 -0000    1.119
--- src/interfaces/libpq/libpq-int.h    7 Jul 2007 21:09:40 -0000
***************
*** 299,304 ****
--- 299,305 ----
      SockAddr    raddr;            /* Remote address */
      ProtocolVersion pversion;    /* FE/BE protocol version in use */
      int            sversion;        /* server version, e.g. 70401 for 7.4.1 */
+     AuthRequest    areq;            /* server demanded password during auth */

      /* Transient state needed while establishing connection */
      struct addrinfo *addrlist;    /* list of possible backend addresses */