Re: dblink connection security - Mailing list pgsql-patches

Joe Conway wrote:
> I'm working on the back branch solution now.
>

Attached is for back-patching. I left in the SECURITY DEFINER functions
and dblink doc changes -- even though they may not do existing
installations much good, I think there will still be enough new 8.2.x
installations for a while to make it worthwhile. And even for older
branches, at least it gives us something handy to point to as a
workaround when people complain we broke their apps.

If there are no objections I'll commit this later today.

Thanks,

Joe
Index: contrib/dblink/dblink.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.60
diff -c -r1.60 dblink.c
*** contrib/dblink/dblink.c    19 Oct 2006 19:53:03 -0000    1.60
--- contrib/dblink/dblink.c    8 Jul 2007 17:38:00 -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"
***************
*** 89,94 ****
--- 90,96 ----
  static HeapTuple get_tuple_of_interest(Oid relid, int2vector *pkattnums, int16 pknumatts, char **src_pkattvals);
  static Oid    get_relid_from_relname(text *relname_text);
  static char *generate_relation_name(Oid relid);
+ static char *connstr_strip_password(const char *connstr);

  /* Global */
  static remoteConn *pconn = NULL;
***************
*** 228,233 ****
--- 230,257 ----

      if (connname)
          rconn = (remoteConn *) palloc(sizeof(remoteConn));
+
+     /* for non-superusers, check that server requires a password */
+     if (!superuser())
+     {
+         /* this attempt must fail */
+         conn = PQconnectdb(connstr_strip_password(connstr));
+
+         if (PQstatus(conn) == CONNECTION_OK)
+         {
+             PQfinish(conn);
+             if (rconn)
+                 pfree(rconn);
+
+             ereport(ERROR,
+                     (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
+                      errmsg("password is required"),
+                      errdetail("Non-superuser cannot connect if the server does not request a password."),
+                      errhint("Target server's authentication method must be changed.")));
+         }
+         else
+             PQfinish(conn);
+     }
      conn = PQconnectdb(connstr);

      MemoryContextSwitchTo(oldcontext);
***************
*** 2273,2275 ****
--- 2297,2430 ----
                   errmsg("undefined connection name")));

  }
+
+
+ /*
+  * Modified version of conninfo_parse() from fe-connect.c
+  * Used to remove any password from the connection string
+  * in order to test whether the server auth method will
+  * require it.
+  */
+ static char *
+ connstr_strip_password(const char *connstr)
+ {
+     char           *pname;
+     char           *pval;
+     char           *buf;
+     char           *cp;
+     char           *cp2;
+     StringInfoData    result;
+
+     /* initialize return value */
+     initStringInfo(&result);
+
+     /* Need a modifiable copy of the input string */
+     buf = pstrdup(connstr);
+     cp = buf;
+
+     while (*cp)
+     {
+         /* Skip blanks before the parameter name */
+         if (isspace((unsigned char) *cp))
+         {
+             cp++;
+             continue;
+         }
+
+         /* Get the parameter name */
+         pname = cp;
+         while (*cp)
+         {
+             if (*cp == '=')
+                 break;
+             if (isspace((unsigned char) *cp))
+             {
+                 *cp++ = '\0';
+                 while (*cp)
+                 {
+                     if (!isspace((unsigned char) *cp))
+                         break;
+                     cp++;
+                 }
+                 break;
+             }
+             cp++;
+         }
+
+         /* Check that there is a following '=' */
+         if (*cp != '=')
+             ereport(ERROR,
+                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                     errmsg("missing \"=\" after \"%s\" in connection string", pname)));
+         *cp++ = '\0';
+
+         /* Skip blanks after the '=' */
+         while (*cp)
+         {
+             if (!isspace((unsigned char) *cp))
+                 break;
+             cp++;
+         }
+
+         /* Get the parameter value */
+         pval = cp;
+
+         if (*cp != '\'')
+         {
+             cp2 = pval;
+             while (*cp)
+             {
+                 if (isspace((unsigned char) *cp))
+                 {
+                     *cp++ = '\0';
+                     break;
+                 }
+                 if (*cp == '\\')
+                 {
+                     cp++;
+                     if (*cp != '\0')
+                         *cp2++ = *cp++;
+                 }
+                 else
+                     *cp2++ = *cp++;
+             }
+             *cp2 = '\0';
+         }
+         else
+         {
+             cp2 = pval;
+             cp++;
+             for (;;)
+             {
+                 if (*cp == '\0')
+                     ereport(ERROR,
+                             (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                             errmsg("unterminated quoted string in connection string")));
+                 if (*cp == '\\')
+                 {
+                     cp++;
+                     if (*cp != '\0')
+                         *cp2++ = *cp++;
+                     continue;
+                 }
+                 if (*cp == '\'')
+                 {
+                     *cp2 = '\0';
+                     cp++;
+                     break;
+                 }
+                 *cp2++ = *cp++;
+             }
+         }
+
+         /*
+          * Now we have the name and the value. If it is not a password,
+          * append to the return connstr.
+          */
+         if (strcmp("password", pname) != 0)
+             /* append the value */
+             appendStringInfo(&result, " %s='%s'", pname, pval);
+     }
+
+     return result.data;
+ }
Index: contrib/dblink/dblink.sql.in
===================================================================
RCS file: /cvsroot/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 17:38:00 -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: /cvsroot/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 17:38:00 -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