Skip to content

Commit 9b75844

Browse files
committed
Fix dblink panic when connecting as a non-superuser
1, QD to QD's connection user is environment variable PGUSER, we need to set it to session user in dblink. 2, QD to QD's unix domain socket connection doesn't require any authentication, request non-superuser to provide host to use TCP/UDP connections. Signed-off-by: Adam Lee <ali@pivotal.io> Signed-off-by: Xiaoran Wang <xiwang@pivotal.io>
1 parent 0ed9333 commit 9b75844

2 files changed

Lines changed: 46 additions & 7 deletions

File tree

contrib/dblink/Makefile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
MODULE_big = dblink
44
PG_CPPFLAGS = -I$(libpq_srcdir)
55
OBJS = dblink.o
6-
SHLIB_LINK = $(libpq)
76

87
DATA_built = dblink.sql
98
DATA = uninstall_dblink.sql

contrib/dblink/dblink.c

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ static int get_attnum_pk_pos(int *pkattnums, int pknumatts, int key);
9494
static HeapTuple get_tuple_of_interest(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals);
9595
static Relation get_rel_from_relname(text *relname_text, LOCKMODE lockmode, AclMode aclmode);
9696
static char *generate_relation_name(Relation rel);
97-
static void dblink_connstr_check(const char *connstr);
97+
static char *dblink_connstr_check(const char *connstr);
9898
static void dblink_security_check(PGconn *conn, remoteConn *rconn);
9999
static void dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail);
100100
static void dblink_security_check(PGconn *conn, remoteConn *rconn);
@@ -192,8 +192,7 @@ typedef struct remoteConnHashEnt
192192
} \
193193
else \
194194
{ \
195-
connstr = conname_or_str; \
196-
dblink_connstr_check(connstr); \
195+
connstr = dblink_connstr_check(conname_or_str); \
197196
conn = PQconnectdb(connstr); \
198197
if (PQstatus(conn) == CONNECTION_BAD) \
199198
{ \
@@ -258,7 +257,7 @@ dblink_connect(PG_FUNCTION_ARGS)
258257
sizeof(remoteConn));
259258

260259
/* check password in connection string if not superuser */
261-
dblink_connstr_check(connstr);
260+
connstr = dblink_connstr_check(connstr);
262261
conn = PQconnectdb(connstr);
263262

264263
if (PQstatus(conn) == CONNECTION_BAD)
@@ -2271,39 +2270,80 @@ dblink_security_check(PGconn *conn, remoteConn *rconn)
22712270
* prevents a password from being picked up from .pgpass, a service file,
22722271
* the environment, etc. We don't want the postgres user's passwords
22732272
* to be accessible to non-superusers.
2273+
*
2274+
* For Greenplum, dblink uses built libpq to construct conninfo, whose user is
2275+
* environment variable PGUSER, which is wrong, modifies this function to add
2276+
* the session's username into connstr.
2277+
*
22742278
*/
2275-
static void
2279+
static char *
22762280
dblink_connstr_check(const char *connstr)
22772281
{
2282+
char *connstr_modified = (char *) connstr;
2283+
22782284
if (!superuser())
22792285
{
22802286
PQconninfoOption *options;
22812287
PQconninfoOption *option;
22822288
bool connstr_gives_password = false;
2289+
bool username_is_set = false;
2290+
bool host_is_set = false;
22832291

22842292
options = PQconninfoParse(connstr, NULL);
22852293
if (options)
22862294
{
22872295
for (option = options; option->keyword != NULL; option++)
22882296
{
2297+
if (strcmp(option->keyword, "host") == 0)
2298+
{
2299+
if (option->val != NULL && option->val[0] != '\0')
2300+
{
2301+
host_is_set = true;
2302+
}
2303+
}
2304+
2305+
if (strcmp(option->keyword, "user") == 0)
2306+
{
2307+
if (option->val == NULL || option->val[0] == '\0')
2308+
{
2309+
char *username = GetUserNameFromId(GetUserId());
2310+
2311+
/* 7 is strlen("user= ") + length of '\0' */
2312+
connstr_modified = palloc0(7 + strlen(username) + strlen(connstr));
2313+
sprintf(connstr_modified, "user=%s %s", username, connstr);
2314+
}
2315+
2316+
username_is_set = true;
2317+
}
2318+
22892319
if (strcmp(option->keyword, "password") == 0)
22902320
{
22912321
if (option->val != NULL && option->val[0] != '\0')
22922322
{
22932323
connstr_gives_password = true;
2294-
break;
22952324
}
22962325
}
2326+
2327+
if (host_is_set && username_is_set && connstr_gives_password)
2328+
break;
22972329
}
22982330
PQconninfoFree(options);
22992331
}
23002332

2333+
if (!host_is_set)
2334+
ereport(ERROR,
2335+
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
2336+
errmsg("host is required"),
2337+
errdetail("Non-superusers must provide a host in the connection string.")));
2338+
23012339
if (!connstr_gives_password)
23022340
ereport(ERROR,
23032341
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
23042342
errmsg("password is required"),
23052343
errdetail("Non-superusers must provide a password in the connection string.")));
23062344
}
2345+
2346+
return connstr_modified;
23072347
}
23082348

23092349
static void

0 commit comments

Comments
 (0)