Tom Lane wrote:
"Marko Kreen" <[EMAIL PROTECTED]> writes:
On 9/21/08, Joe Conway <[EMAIL PROTECTED]> wrote:
Why? pg_service does not appear to support wildcards, so what is the attack
vector?
"service=foo host=custom"
The proposal to require a password = foo entry in the conn string seems
to resolve all of these, without taking away useful capability. I don't
think that forbidding use of services altogether is a good thing.
So that seems to tilt the decision towards exposing the conninfo_parse
function. Joe, do you want to have a go at it, or shall I?
Here's a first shot.
Notes:
1. I have not removed PQconnectionUsedPassword and related. It
is still needed to prevent a non-superuser from logging in
as the superuser if the server does not require authentication.
In that case, any bogus password could be added to the connection
string and be subsequently ignored, if not for this check.
2. I assume this ought to be applied as two separate commits --
one for libpq, and one for dblink.
3. I can't easily verify that I got libpq.sgml perfect; I've gotten out
of sync with the required tool chain for the docs
Comments?
Joe
Index: contrib/dblink/dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.74
diff -c -r1.74 dblink.c
*** contrib/dblink/dblink.c 3 Jul 2008 03:56:57 -0000 1.74
--- contrib/dblink/dblink.c 22 Sep 2008 00:34:17 -0000
***************
*** 93,98 ****
--- 93,99 ----
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 void dblink_connstr_check(const char *connstr);
static void dblink_security_check(PGconn *conn, remoteConn *rconn);
static void dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail);
***************
*** 165,170 ****
--- 166,172 ----
else \
{ \
connstr = conname_or_str; \
+ dblink_connstr_check(connstr); \
conn = PQconnectdb(connstr); \
if (PQstatus(conn) == CONNECTION_BAD) \
{ \
***************
*** 229,234 ****
--- 231,239 ----
if (connname)
rconn = (remoteConn *) palloc(sizeof(remoteConn));
+
+ /* check password in connection string if not superuser */
+ dblink_connstr_check(connstr);
conn = PQconnectdb(connstr);
MemoryContextSwitchTo(oldcontext);
***************
*** 246,252 ****
errdetail("%s", msg)));
}
! /* check password used if not superuser */
dblink_security_check(conn, rconn);
if (connname)
--- 251,257 ----
errdetail("%s", msg)));
}
! /* check password actually used if not superuser */
dblink_security_check(conn, rconn);
if (connname)
***************
*** 2252,2257 ****
--- 2257,2293 ----
}
static void
+ dblink_connstr_check(const char *connstr)
+ {
+ if (!superuser())
+ {
+ PQconninfoOption *options;
+ PQconninfoOption *option;
+ bool conn_used_password = false;
+
+ options = PQconninfoParse(connstr);
+ for (option = options; option->keyword != NULL; option++)
+ {
+ if (strcmp(option->keyword, "password") == 0)
+ {
+ if (option->val != NULL && option->val[0] != '\0')
+ {
+ conn_used_password = true;
+ break;
+ }
+ }
+ }
+ PQconninfoFree(options);
+
+ if (!conn_used_password)
+ ereport(ERROR,
+ (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
+ errmsg("password is required"),
+ errdetail("Non-superuser must provide a password in the connection string.")));
+ }
+ }
+
+ static void
dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail)
{
int level;
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /opt/src/cvs/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.263
diff -c -r1.263 libpq.sgml
*** doc/src/sgml/libpq.sgml 19 Sep 2008 20:06:13 -0000 1.263
--- doc/src/sgml/libpq.sgml 21 Sep 2008 23:08:27 -0000
***************
*** 625,630 ****
--- 625,658 ----
</varlistentry>
<varlistentry>
+ <term><function>PQconninfoParse</function><indexterm><primary>PQconninfoParse</></></term>
+ <listitem>
+ <para>
+ Returns parsed connection options from the provided connection string.
+
+ <synopsis>
+ PQconninfoOption *PQconninfoParse(const char *conninfo);
+ </synopsis>
+
+ <para>
+ Returns a connection options array. This can be used to determine
+ the <function>PQconnectdb</function> options in the provided
+ connection string. The return value points to an array of
+ <structname>PQconninfoOption</structname> structures, which ends
+ with an entry having a null <structfield>keyword</> pointer. The
+ null pointer is returned if memory could not be allocated.
+ </para>
+
+ <para>
+ After processing the options array, free it by passing it to
+ <function>PQconninfoFree</function>. If this is not done, a small amount of memory
+ is leaked for each call to <function>PQconndefaults</function>.
+ </para>
+
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><function>PQfinish</function><indexterm><primary>PQfinish</></></term>
<listitem>
<para>
Index: src/interfaces/libpq/exports.txt
===================================================================
RCS file: /opt/src/cvs/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.21
diff -c -r1.21 exports.txt
*** src/interfaces/libpq/exports.txt 19 Sep 2008 20:06:13 -0000 1.21
--- src/interfaces/libpq/exports.txt 21 Sep 2008 23:32:56 -0000
***************
*** 151,153 ****
--- 151,154 ----
PQresultInstanceData 149
PQresultSetInstanceData 150
PQfireResultCreateEvents 151
+ PQconninfoParse 152
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /opt/src/cvs/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.360
diff -c -r1.360 fe-connect.c
*** src/interfaces/libpq/fe-connect.c 17 Sep 2008 04:31:08 -0000 1.360
--- src/interfaces/libpq/fe-connect.c 21 Sep 2008 22:56:40 -0000
***************
*** 3101,3106 ****
--- 3101,3127 ----
return 0;
}
+ /*
+ * PQconninfoParse
+ *
+ * Parse a string like PQconnectdb() would do and return the
+ * working connection options array.
+ *
+ * NOTE: the returned array is dynamically allocated and should
+ * be freed when no longer needed via PQconninfoFree().
+ */
+ PQconninfoOption *
+ PQconninfoParse(const char *conninfo)
+ {
+ PQExpBufferData errorBuf;
+ bool password_from_string;
+ PQconninfoOption *connOptions;
+
+ initPQExpBuffer(&errorBuf);
+ connOptions = conninfo_parse(conninfo, &errorBuf, &password_from_string);
+ termPQExpBuffer(&errorBuf);
+ return connOptions;
+ }
/*
* Conninfo parser routine
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /opt/src/cvs/pgsql/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.143
diff -c -r1.143 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h 17 Sep 2008 04:31:08 -0000 1.143
--- src/interfaces/libpq/libpq-fe.h 21 Sep 2008 22:57:37 -0000
***************
*** 243,248 ****
--- 243,251 ----
/* get info about connection options known to PQconnectdb */
extern PQconninfoOption *PQconndefaults(void);
+ /* parse connection options in same way as PQconnectdb */
+ extern PQconninfoOption *PQconninfoParse(const char *conninfo);
+
/* free the data structure returned by PQconndefaults() */
extern void PQconninfoFree(PQconninfoOption *connOptions);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
