This is an automated email from the ASF dual-hosted git repository. maxyang pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 7a8d28d196a49212fb0496fb7c7141dcf3344044 Author: Soumyadeep Chakraborty <[email protected]> AuthorDate: Tue Apr 16 10:14:11 2024 -0700 Retire gp_reject_internal_tcp_connection We take a number of steps in cdbconn_doConnectStart() to force QD -> entry DB QE communication to be over Unix Domain Sockets. It is more performant over TCP/IP, since it is on the same host. Also, since 4b3360292e4c332933faa554ad2d606f37d82cb7, these connections are considered as authenticated, bypassing the usual pg_hba authentication procedure (see internal_client_authentication()). However, if PGHOST/PGHOSTADDR are set in the postmaster environment, TCP/IP connections are still used instead of Unix Domain Sockets. It is actually hard to enforce UDSes here as conninfo_add_defaults() would sprinkle in the value from the environment, if present. Setting the value of PGHOST to the socket dir (/tmp by default) is also hard, as the unix_socket_directories GUC is plural, and we would need to actually read the postmaster.pid file. Too much trouble and not performant. Till now, the gp_reject_internal_tcp_connection GUC, on by default, would force authentication of these internal QD -> entry DB TCP/IP connections via regular HBA, possibly leading to issues such as: postgres=> select * from foo, pg_sleep(0); ERROR: failed to acquire resources on one or more segments DETAIL: fe_sendauth: no password supplied In this case, the current user is one for which password based authentication has been set up in $COORDINATOR_DATA_DIRECTORY/pg_hba.conf To be more friendly to these situations, mark this GUC as defunct. So, if any TCP/IP connections are created for QD -> entry DB QE, they will be considered authenticated by default, and bypass HBA. Reviewed-by: Huansong Fu <[email protected]> --- src/backend/cdb/dispatcher/cdbconn.c | 9 ++++++--- src/backend/libpq/auth.c | 32 ++++++++++++++++---------------- src/backend/utils/misc/guc_gp.c | 7 +++---- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/backend/cdb/dispatcher/cdbconn.c b/src/backend/cdb/dispatcher/cdbconn.c index 96bb5411c7..e9f2fc157b 100644 --- a/src/backend/cdb/dispatcher/cdbconn.c +++ b/src/backend/cdb/dispatcher/cdbconn.c @@ -162,12 +162,15 @@ cdbconn_doConnectStart(SegmentDatabaseDescriptor *segdbDesc, /* * For entry DB connection, we make sure both "hostaddr" and "host" are - * empty string. Or else, it will fall back to environment variables and - * won't use domain socket in function connectDBStart. Also we set the + * empty string, as we want to force Unix domain socket usage. The reason is + * that for same host communication, Unix domain sockets are more performant. + * However, if the PGHOST or PGHOSTADDR variables are set in the coordinator + * postmaster environment, TCP/IP sockets will still be used. Also, we set the * connection type for entrydb connection so that QE could change Gp_role * from DISPATCH to EXECUTE. * - * For other QE connections, we set "hostaddr". "host" is not used. + * For other QE connections, we set "hostaddr". "host" is not used, as + * hostaddr saves on the cost of hostname resolution. */ if (segdbDesc->segindex == MASTER_CONTENT_ID && IS_QUERY_DISPATCHER()) diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 6561ff0bab..29fe2e4e71 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -586,11 +586,11 @@ internal_client_authentication(Port *port) * The entry-DB (or QE at the master) case. * * The goal here is to block network connection from out of - * master to master db with magic bit packet. - * So, only when it comes from the same host, the connection - * is authenticated, if this connection is TCP/UDP. + * coordinator to coordinator db with magic bit packet. * - * If unix domain socket comes, just authenticate it. + * Internal connections originating from the same host (be it TCP/IP or + * Unix domain socket) are considered already authenticated, and receive + * a free pass. */ if (port->raddr.addr.ss_family == AF_INET #ifdef HAVE_IPV6 @@ -600,18 +600,18 @@ internal_client_authentication(Port *port) { if (check_same_host_or_net(&port->raddr, ipCmpSameHost)) { - if (gp_reject_internal_tcp_conn) - { - elog(DEBUG1, "rejecting TCP connection to master using internal" - "connection protocol, because the GUC gp_reject_internal_tcp_conn is true"); - return false; - } - else - { - elog(DEBUG1, "received same host internal TCP connection"); - FakeClientAuthentication(port); - return true; - } + /* + * Note: We do take steps to prevent TCP/IP connections from the + * coordinator to entry DB QEs (see cdbconn_doConnectStart()), + * in favor of Unix domain socket communication. However, if + * PGHOST is set in the coordinator's environment to a hostname, + * then the connection will still be TCP/IP. + */ + ereport(DEBUG1, + (errmsg("received same host internal connection over TCP/IP"), + errdetail("check if PGHOST or PGHOSTADDR is set in the coordinator environment"))); + FakeClientAuthentication(port); + return true; } /* Security violation? */ diff --git a/src/backend/utils/misc/guc_gp.c b/src/backend/utils/misc/guc_gp.c index 1995d91e75..eecfd01c70 100644 --- a/src/backend/utils/misc/guc_gp.c +++ b/src/backend/utils/misc/guc_gp.c @@ -2712,14 +2712,13 @@ struct config_bool ConfigureNamesBool_gp[] = }, { - {"gp_reject_internal_tcp_connection", PGC_POSTMASTER, - DEVELOPER_OPTIONS, - gettext_noop("Permit internal TCP connections to the master."), + {"gp_reject_internal_tcp_connection", PGC_POSTMASTER, DEFUNCT_OPTIONS, + gettext_noop("Unused. Syntax check only for GPDB compatibility."), NULL, GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE }, &gp_reject_internal_tcp_conn, - true, + false, NULL, NULL, NULL }, --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
