Skip to content

Commit 256d8d8

Browse files
authored
Resolve rebase conflicts in contrib/dblink. (#1485)
In MPP/Cloudberry we historically have dblink behaviour differencies with upstream. They were initially introduced 9b75844, as bug-fix. Namely, we do not allow non-superuser to create dblink connection without explicit password/username. We also force username in connection string to match session user, if not specified (see dblink_connstr_has_pw). Two function declarations changed: dblink_connstr_check dblink_connstr_has_pw they now return modified connection string.
1 parent 36fd995 commit 256d8d8

2 files changed

Lines changed: 69 additions & 104 deletions

File tree

contrib/dblink/dblink.c

Lines changed: 69 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,9 @@ static int get_attnum_pk_pos(int *pkattnums, int pknumatts, int key);
112112
static HeapTuple get_tuple_of_interest(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals);
113113
static Relation get_rel_from_relname(text *relname_text, LOCKMODE lockmode, AclMode aclmode);
114114
static char *generate_relation_name(Relation rel);
115-
<<<<<<< HEAD
116-
static char *dblink_connstr_check(const char *connstr);
117-
static void dblink_security_check(PGconn *conn, remoteConn *rconn);
118-
=======
119-
static void dblink_connstr_check(const char *connstr);
120-
static bool dblink_connstr_has_pw(const char *connstr);
115+
static char *dblink_connstr_check(char *connstr);
116+
static char *dblink_connstr_has_pw(const char *connstr);
121117
static void dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr);
122-
>>>>>>> REL_16_9
123118
static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
124119
bool fail, const char *fmt,...) pg_attribute_printf(5, 6);
125120
static char *get_connect_string(const char *servername);
@@ -201,12 +196,12 @@ dblink_get_conn(char *conname_or_str,
201196
}
202197
else
203198
{
204-
const char *connstr;
199+
char *connstr;
205200

206201
connstr = get_connect_string(conname_or_str);
207202
if (connstr == NULL)
208203
connstr = conname_or_str;
209-
dblink_connstr_check(connstr);
204+
connstr = dblink_connstr_check(connstr);
210205

211206
/* OK to make connection */
212207
conn = libpqsrv_connect(connstr, PG_WAIT_EXTENSION);
@@ -297,7 +292,6 @@ dblink_connect(PG_FUNCTION_ARGS)
297292

298293
/* check password in connection string if not superuser */
299294
connstr = dblink_connstr_check(connstr);
300-
dblink_connstr_check(connstr);
301295

302296
/* OK to make connection */
303297
conn = libpqsrv_connect(connstr, PG_WAIT_EXTENSION);
@@ -2595,17 +2589,6 @@ deleteConnection(const char *name)
25952589
}
25962590

25972591
/*
2598-
<<<<<<< HEAD
2599-
* For non-superusers, insist that the connstr specify a password. This
2600-
* prevents a password from being picked up from .pgpass, a service file,
2601-
* the environment, etc. We don't want the postgres user's passwords
2602-
* to be accessible to non-superusers.
2603-
*
2604-
* For Cloudberry, dblink uses built libpq to construct conninfo, whose user is
2605-
* environment variable PGUSER, which is wrong, modifies this function to add
2606-
* the session's username into connstr.
2607-
*
2608-
=======
26092592
* We need to make sure that the connection made used credentials
26102593
* which were provided by the user, so check what credentials were
26112594
* used to connect and then make sure that they came from the user.
@@ -2617,8 +2600,13 @@ dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr)
26172600
if (superuser())
26182601
return;
26192602

2603+
/* Difference with upstream, return type is str not bool.
2604+
* dblink_connstr_has_pw also never returns of missing password
2605+
*/
2606+
(void) dblink_connstr_has_pw(connstr);
2607+
26202608
/* If password was used to connect, make sure it was one provided */
2621-
if (PQconnectionUsedPassword(conn) && dblink_connstr_has_pw(connstr))
2609+
if (PQconnectionUsedPassword(conn))
26222610
return;
26232611

26242612
#ifdef ENABLE_GSS
@@ -2645,31 +2633,72 @@ dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr)
26452633
* is using a provided password and not one picked up from the
26462634
* environment.
26472635
*/
2648-
static bool
2636+
static char *
26492637
dblink_connstr_has_pw(const char *connstr)
26502638
{
26512639
PQconninfoOption *options;
26522640
PQconninfoOption *option;
26532641
bool connstr_gives_password = false;
2642+
bool username_is_set = false;
2643+
bool host_is_set = false;
2644+
char *connstr_modified = (char *) connstr;
26542645

26552646
options = PQconninfoParse(connstr, NULL);
26562647
if (options)
26572648
{
26582649
for (option = options; option->keyword != NULL; option++)
26592650
{
2651+
2652+
if (strcmp(option->keyword, "host") == 0)
2653+
{
2654+
if (option->val != NULL && option->val[0] != '\0')
2655+
{
2656+
host_is_set = true;
2657+
}
2658+
}
2659+
2660+
if (strcmp(option->keyword, "user") == 0)
2661+
{
2662+
if (option->val == NULL || option->val[0] == '\0')
2663+
{
2664+
char *username = GetUserNameFromId(GetUserId(), false);
2665+
2666+
/* 7 is strlen("user= ") + length of '\0' */
2667+
connstr_modified = palloc0(7 + strlen(username) + strlen(connstr));
2668+
sprintf(connstr_modified, "user=%s %s", username, connstr);
2669+
}
2670+
2671+
username_is_set = true;
2672+
}
2673+
2674+
26602675
if (strcmp(option->keyword, "password") == 0)
26612676
{
26622677
if (option->val != NULL && option->val[0] != '\0')
26632678
{
26642679
connstr_gives_password = true;
2665-
break;
26662680
}
26672681
}
2682+
2683+
if (host_is_set && username_is_set && connstr_gives_password)
2684+
break;
26682685
}
26692686
PQconninfoFree(options);
26702687
}
26712688

2672-
return connstr_gives_password;
2689+
if (!host_is_set)
2690+
ereport(ERROR,
2691+
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
2692+
errmsg("host is required"),
2693+
errdetail("Non-superusers must provide a host in the connection string.")));
2694+
2695+
if (!connstr_gives_password)
2696+
ereport(ERROR,
2697+
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
2698+
errmsg("password is required"),
2699+
errdetail("Non-superusers must provide a password in the connection string.")));
2700+
2701+
return connstr_modified;
26732702
}
26742703

26752704
/*
@@ -2679,94 +2708,35 @@ dblink_connstr_has_pw(const char *connstr)
26792708
* password or GSSAPI credentials from being picked up from .pgpass, a
26802709
* service file, the environment, etc. We don't want the postgres user's
26812710
* passwords or Kerberos credentials to be accessible to non-superusers.
2682-
>>>>>>> REL_16_9
2711+
*
2712+
* For Cloudberry, dblink uses built libpq to construct conninfo, whose user is
2713+
* environment variable PGUSER, which is wrong, modifies this function to add
2714+
* the session's username into connstr.
26832715
*/
26842716
static char *
2685-
dblink_connstr_check(const char *connstr)
2717+
dblink_connstr_check(char *connstr)
26862718
{
2687-
<<<<<<< HEAD
2688-
char *connstr_modified = (char *) connstr;
2689-
2690-
if (!superuser())
2691-
{
2692-
PQconninfoOption *options;
2693-
PQconninfoOption *option;
2694-
bool connstr_gives_password = false;
2695-
bool username_is_set = false;
2696-
bool host_is_set = false;
2697-
2698-
options = PQconninfoParse(connstr, NULL);
2699-
if (options)
2700-
{
2701-
for (option = options; option->keyword != NULL; option++)
2702-
{
2703-
if (strcmp(option->keyword, "host") == 0)
2704-
{
2705-
if (option->val != NULL && option->val[0] != '\0')
2706-
{
2707-
host_is_set = true;
2708-
}
2709-
}
2710-
2711-
if (strcmp(option->keyword, "user") == 0)
2712-
{
2713-
if (option->val == NULL || option->val[0] == '\0')
2714-
{
2715-
char *username = GetUserNameFromId(GetUserId(), false);
2716-
2717-
/* 7 is strlen("user= ") + length of '\0' */
2718-
connstr_modified = palloc0(7 + strlen(username) + strlen(connstr));
2719-
sprintf(connstr_modified, "user=%s %s", username, connstr);
2720-
}
2721-
2722-
username_is_set = true;
2723-
}
2724-
2725-
if (strcmp(option->keyword, "password") == 0)
2726-
{
2727-
if (option->val != NULL && option->val[0] != '\0')
2728-
{
2729-
connstr_gives_password = true;
2730-
}
2731-
}
2732-
2733-
if (host_is_set && username_is_set && connstr_gives_password)
2734-
break;
2735-
}
2736-
PQconninfoFree(options);
2737-
}
2738-
2739-
if (!host_is_set)
2740-
ereport(ERROR,
2741-
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
2742-
errmsg("host is required"),
2743-
errdetail("Non-superusers must provide a host in the connection string.")));
2744-
2745-
if (!connstr_gives_password)
2746-
ereport(ERROR,
2747-
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
2748-
errmsg("password is required"),
2749-
errdetail("Non-superusers must provide a password in the connection string.")));
2750-
}
2751-
2752-
return connstr_modified;
2753-
=======
27542719
if (superuser())
2755-
return;
2720+
return connstr;
27562721

2757-
if (dblink_connstr_has_pw(connstr))
2758-
return;
2722+
/* Difference with upstream, return type is str not bool.
2723+
* dblink_connstr_has_pw also never returns of missing password
2724+
*/
2725+
char *res = dblink_connstr_has_pw(connstr);
27592726

27602727
#ifdef ENABLE_GSS
27612728
if (be_gssapi_get_delegation(MyProcPort))
2762-
return;
2729+
return res;
27632730
#endif
27642731

2732+
/* see `dblink_connstr_has_pw` */
2733+
return res;
2734+
#if 0
27652735
ereport(ERROR,
27662736
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
27672737
errmsg("password or GSSAPI delegated credentials required"),
27682738
errdetail("Non-superusers must provide a password in the connection string or send delegated GSSAPI credentials.")));
2769-
>>>>>>> REL_16_9
2739+
#endif
27702740
}
27712741

27722742
/*

contrib/dblink/expected/dblink.out

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -967,13 +967,8 @@ SET SESSION AUTHORIZATION regress_dblink_user;
967967
-- should fail
968968
-- GPDB: We also check for hostname in connection string which is checked first
969969
SELECT dblink_connect('myconn', 'fdtest');
970-
<<<<<<< HEAD
971970
DETAIL: Non-superusers must provide a host in the connection string.
972971
ERROR: host is required
973-
=======
974-
ERROR: password or GSSAPI delegated credentials required
975-
DETAIL: Non-superusers must provide a password in the connection string or send delegated GSSAPI credentials.
976-
>>>>>>> REL_16_9
977972
-- should succeed
978973
SELECT dblink_connect_u('myconn', 'fdtest');
979974
dblink_connect_u

0 commit comments

Comments
 (0)