From 98e93a1fc93e9b54eb477d870ec744e9e1669f34 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 11 Jan 2022 13:46:12 -0500 Subject: [PATCH] Clean up messy API for src/port/thread.c. The point of this patch is to reduce inclusion spam by not needing to #include or in port.h (which is read by every compile in our tree). To do that, we must remove port.h's declarations of pqGetpwuid and pqGethostbyname. pqGethostbyname is only used, and is only ever likely to be used, in src/port/getaddrinfo.c --- which isn't even built on most platforms, making pqGethostbyname dead code for most people. Hence, deal with that by just moving it into getaddrinfo.c. To clean up pqGetpwuid, invent a couple of simple wrapper functions with less-messy APIs. This allows removing some duplicate error-handling code, too. In passing, remove thread.c from the MSVC build, since it contains nothing we use on Windows. Noted while working on 376ce3e40. Discussion: https://postgr.es/m/1634252654444.90107@mit.edu --- src/backend/libpq/auth.c | 1 + src/bin/psql/common.c | 1 + src/include/port.h | 14 +---- src/interfaces/libpq/fe-auth.c | 54 ++++++++-------- src/interfaces/libpq/fe-auth.h | 1 + src/interfaces/libpq/fe-connect.c | 40 +++--------- src/port/Makefile | 4 ++ src/port/getaddrinfo.c | 44 +++++++++++++ src/port/path.c | 11 +--- src/port/thread.c | 100 +++++++++++++++++++----------- src/tools/msvc/Mkvcbuild.pm | 2 +- 11 files changed, 160 insertions(+), 112 deletions(-) diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 9a266beb5d..efc53f3135 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #ifdef HAVE_SYS_SELECT_H #include diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index f210ccbde8..3503605a7d 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #ifndef WIN32 #include /* for write() */ diff --git a/src/include/port.h b/src/include/port.h index 56e3721f6a..3d103a2b31 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -14,8 +14,6 @@ #define PG_PORT_H #include -#include -#include /* * Windows has enough specialized port stuff that we push most of it off @@ -484,18 +482,12 @@ extern char *dlerror(void); #define RTLD_GLOBAL 0 #endif -/* thread.h */ +/* thread.c */ #ifndef WIN32 -extern int pqGetpwuid(uid_t uid, struct passwd *resultbuf, char *buffer, - size_t buflen, struct passwd **result); +extern bool pg_get_user_name(uid_t user_id, char *buffer, size_t buflen); +extern bool pg_get_user_home_dir(uid_t user_id, char *buffer, size_t buflen); #endif -extern int pqGethostbyname(const char *name, - struct hostent *resultbuf, - char *buffer, size_t buflen, - struct hostent **result, - int *herrno); - extern void pg_qsort(void *base, size_t nel, size_t elsize, int (*cmp) (const void *, const void *)); extern int pg_qsort_strcmp(const void *a, const void *b); diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c index 82fc7cdb98..2edc3f48e2 100644 --- a/src/interfaces/libpq/fe-auth.c +++ b/src/interfaces/libpq/fe-auth.c @@ -35,7 +35,6 @@ #ifndef MAXHOSTNAMELEN #include /* for MAXHOSTNAMELEN on some */ #endif -#include #endif #include "common/md5.h" @@ -1099,14 +1098,17 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn) /* - * pg_fe_getauthname + * pg_fe_getusername * - * Returns a pointer to malloc'd space containing whatever name the user - * has authenticated to the system. If there is an error, return NULL, - * and append a suitable error message to *errorMessage if that's not NULL. + * Returns a pointer to malloc'd space containing the name of the + * specified user_id. If there is an error, return NULL, and append + * a suitable error message to *errorMessage if that's not NULL. + * + * Caution: on Windows, the user_id argument is ignored, and we always + * fetch the current user's name. */ char * -pg_fe_getauthname(PQExpBuffer errorMessage) +pg_fe_getusername(uid_t user_id, PQExpBuffer errorMessage) { char *result = NULL; const char *name = NULL; @@ -1116,17 +1118,13 @@ pg_fe_getauthname(PQExpBuffer errorMessage) char username[256 + 1]; DWORD namesize = sizeof(username); #else - uid_t user_id = geteuid(); char pwdbuf[BUFSIZ]; - struct passwd pwdstr; - struct passwd *pw = NULL; - int pwerr; #endif /* * Some users are using configure --enable-thread-safety-force, so we * might as well do the locking within our library to protect - * pqGetpwuid(). In fact, application developers can use getpwuid() in + * getpwuid(). In fact, application developers can use getpwuid() in * their application if they use the locking call we provide, or install * their own locking function using PQregisterThreadLock(). */ @@ -1140,21 +1138,10 @@ pg_fe_getauthname(PQExpBuffer errorMessage) libpq_gettext("user name lookup failure: error code %lu\n"), GetLastError()); #else - pwerr = pqGetpwuid(user_id, &pwdstr, pwdbuf, sizeof(pwdbuf), &pw); - if (pw != NULL) - name = pw->pw_name; + if (pg_get_user_name(user_id, pwdbuf, sizeof(pwdbuf))) + name = pwdbuf; else if (errorMessage) - { - if (pwerr != 0) - appendPQExpBuffer(errorMessage, - libpq_gettext("could not look up local user ID %d: %s\n"), - (int) user_id, - strerror_r(pwerr, pwdbuf, sizeof(pwdbuf))); - else - appendPQExpBuffer(errorMessage, - libpq_gettext("local user with ID %d does not exist\n"), - (int) user_id); - } + appendPQExpBuffer(errorMessage, "%s\n", pwdbuf); #endif if (name) @@ -1170,6 +1157,23 @@ pg_fe_getauthname(PQExpBuffer errorMessage) return result; } +/* + * pg_fe_getauthname + * + * Returns a pointer to malloc'd space containing whatever name the user + * has authenticated to the system. If there is an error, return NULL, + * and append a suitable error message to *errorMessage if that's not NULL. + */ +char * +pg_fe_getauthname(PQExpBuffer errorMessage) +{ +#ifdef WIN32 + return pg_fe_getusername(0, errorMessage); +#else + return pg_fe_getusername(geteuid(), errorMessage); +#endif +} + /* * PQencryptPassword -- exported routine to encrypt a password with MD5 diff --git a/src/interfaces/libpq/fe-auth.h b/src/interfaces/libpq/fe-auth.h index 16d5e1da0f..f22b3fe648 100644 --- a/src/interfaces/libpq/fe-auth.h +++ b/src/interfaces/libpq/fe-auth.h @@ -20,6 +20,7 @@ /* Prototypes for functions in fe-auth.c */ extern int pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn); +extern char *pg_fe_getusername(uid_t user_id, PQExpBuffer errorMessage); extern char *pg_fe_getauthname(PQExpBuffer errorMessage); /* Mechanisms in fe-auth-scram.c */ diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index a12e0180fd..5fc16be849 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -2813,10 +2813,7 @@ keep_going: /* We will come back to here until there is IS_AF_UNIX(conn->raddr.addr.ss_family)) { #ifndef WIN32 - char pwdbuf[BUFSIZ]; - struct passwd pass_buf; - struct passwd *pass; - int passerr; + char *remote_username; #endif uid_t uid; gid_t gid; @@ -2839,28 +2836,20 @@ keep_going: /* We will come back to here until there is } #ifndef WIN32 - passerr = pqGetpwuid(uid, &pass_buf, pwdbuf, sizeof(pwdbuf), &pass); - if (pass == NULL) - { - if (passerr != 0) - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not look up local user ID %d: %s\n"), - (int) uid, - strerror_r(passerr, sebuf, sizeof(sebuf))); - else - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("local user with ID %d does not exist\n"), - (int) uid); - goto error_return; - } + remote_username = pg_fe_getusername(uid, + &conn->errorMessage); + if (remote_username == NULL) + goto error_return; /* message already logged */ - if (strcmp(pass->pw_name, conn->requirepeer) != 0) + if (strcmp(remote_username, conn->requirepeer) != 0) { appendPQExpBuffer(&conn->errorMessage, libpq_gettext("requirepeer specifies \"%s\", but actual peer user name is \"%s\"\n"), - conn->requirepeer, pass->pw_name); + conn->requirepeer, remote_username); + free(remote_username); goto error_return; } + free(remote_username); #else /* WIN32 */ /* should have failed with ENOSYS above */ Assert(false); @@ -7271,16 +7260,7 @@ pqGetHomeDirectory(char *buf, int bufsize) home = getenv("HOME"); if (home == NULL || home[0] == '\0') - { - char pwdbuf[BUFSIZ]; - struct passwd pwdstr; - struct passwd *pwd = NULL; - - (void) pqGetpwuid(geteuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd); - if (pwd == NULL) - return false; - home = pwd->pw_dir; - } + return pg_get_user_home_dir(geteuid(), buf, bufsize); strlcpy(buf, home, bufsize); return true; #else diff --git a/src/port/Makefile b/src/port/Makefile index b3754d8940..bfe1feb0d4 100644 --- a/src/port/Makefile +++ b/src/port/Makefile @@ -84,6 +84,10 @@ libpgport.a: $(OBJS) rm -f $@ $(AR) $(AROPT) $@ $^ +# getaddrinfo.o and getaddrinfo_shlib.o need PTHREAD_CFLAGS (but getaddrinfo_srv.o does not) +getaddrinfo.o: CFLAGS+=$(PTHREAD_CFLAGS) +getaddrinfo_shlib.o: CFLAGS+=$(PTHREAD_CFLAGS) + # thread.o and thread_shlib.o need PTHREAD_CFLAGS (but thread_srv.o does not) thread.o: CFLAGS+=$(PTHREAD_CFLAGS) thread_shlib.o: CFLAGS+=$(PTHREAD_CFLAGS) diff --git a/src/port/getaddrinfo.c b/src/port/getaddrinfo.c index b0ca4c778e..3284c6eb52 100644 --- a/src/port/getaddrinfo.c +++ b/src/port/getaddrinfo.c @@ -34,6 +34,14 @@ #include "port/pg_bswap.h" +#ifdef FRONTEND +static int pqGethostbyname(const char *name, + struct hostent *resultbuf, + char *buffer, size_t buflen, + struct hostent **result, + int *herrno); +#endif + #ifdef WIN32 /* * The native routines may or may not exist on the Windows platform we are on, @@ -394,3 +402,39 @@ getnameinfo(const struct sockaddr *sa, int salen, return 0; } + +/* + * Wrapper around gethostbyname() or gethostbyname_r() to mimic + * POSIX gethostbyname_r() behaviour, if it is not available or required. + */ +#ifdef FRONTEND +static int +pqGethostbyname(const char *name, + struct hostent *resultbuf, + char *buffer, size_t buflen, + struct hostent **result, + int *herrno) +{ +#if defined(ENABLE_THREAD_SAFETY) && defined(HAVE_GETHOSTBYNAME_R) + + /* + * broken (well early POSIX draft) gethostbyname_r() which returns 'struct + * hostent *' + */ + *result = gethostbyname_r(name, resultbuf, buffer, buflen, herrno); + return (*result == NULL) ? -1 : 0; +#else + + /* no gethostbyname_r(), just use gethostbyname() */ + *result = gethostbyname(name); + + if (*result != NULL) + *herrno = h_errno; + + if (*result != NULL) + return 0; + else + return -1; +#endif +} +#endif /* FRONTEND */ diff --git a/src/port/path.c b/src/port/path.c index 5ac26f4bcf..69bb8fe40b 100644 --- a/src/port/path.c +++ b/src/port/path.c @@ -815,16 +815,7 @@ get_home_path(char *ret_path) home = getenv("HOME"); if (home == NULL || home[0] == '\0') - { - char pwdbuf[BUFSIZ]; - struct passwd pwdstr; - struct passwd *pwd = NULL; - - (void) pqGetpwuid(geteuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd); - if (pwd == NULL) - return false; - home = pwd->pw_dir; - } + return pg_get_user_home_dir(geteuid(), ret_path, MAXPGPATH); strlcpy(ret_path, home, MAXPGPATH); return true; #else diff --git a/src/port/thread.c b/src/port/thread.c index c1040d4e24..23c3fbdf86 100644 --- a/src/port/thread.c +++ b/src/port/thread.c @@ -45,10 +45,9 @@ * use *_r function names if they exit * (*_THREADSAFE=yes) * use non-*_r functions if they are thread-safe - * - * One thread-safe solution for gethostbyname() might be to use getaddrinfo(). */ +#ifndef WIN32 /* * Wrapper around getpwuid() or getpwuid_r() to mimic POSIX getpwuid_r() @@ -60,8 +59,7 @@ * error during lookup: returns an errno code, *result is NULL * (caller should *not* assume that the errno variable is set) */ -#ifndef WIN32 -int +static int pqGetpwuid(uid_t uid, struct passwd *resultbuf, char *buffer, size_t buflen, struct passwd **result) { @@ -75,42 +73,74 @@ pqGetpwuid(uid_t uid, struct passwd *resultbuf, char *buffer, return (*result == NULL) ? errno : 0; #endif } -#endif /* - * Wrapper around gethostbyname() or gethostbyname_r() to mimic - * POSIX gethostbyname_r() behaviour, if it is not available or required. - * This function is called _only_ by our getaddrinfo() portability function. + * pg_get_user_name - get the name of the user with the given ID + * + * On success, the user name is returned into the buffer (of size buflen), + * and "true" is returned. On failure, a localized error message is + * returned into the buffer, and "false" is returned. */ -#ifndef HAVE_GETADDRINFO -int -pqGethostbyname(const char *name, - struct hostent *resultbuf, - char *buffer, size_t buflen, - struct hostent **result, - int *herrno) +bool +pg_get_user_name(uid_t user_id, char *buffer, size_t buflen) { -#if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && defined(HAVE_GETHOSTBYNAME_R) + char pwdbuf[BUFSIZ]; + struct passwd pwdstr; + struct passwd *pw = NULL; + int pwerr; - /* - * broken (well early POSIX draft) gethostbyname_r() which returns 'struct - * hostent *' - */ - *result = gethostbyname_r(name, resultbuf, buffer, buflen, herrno); - return (*result == NULL) ? -1 : 0; -#else - - /* no gethostbyname_r(), just use gethostbyname() */ - *result = gethostbyname(name); - - if (*result != NULL) - *herrno = h_errno; - - if (*result != NULL) - return 0; + pwerr = pqGetpwuid(user_id, &pwdstr, pwdbuf, sizeof(pwdbuf), &pw); + if (pw != NULL) + { + strlcpy(buffer, pw->pw_name, buflen); + return true; + } + if (pwerr != 0) + snprintf(buffer, buflen, + _("could not look up local user ID %d: %s"), + (int) user_id, + strerror_r(pwerr, pwdbuf, sizeof(pwdbuf))); else - return -1; -#endif + snprintf(buffer, buflen, + _("local user with ID %d does not exist"), + (int) user_id); + return false; } -#endif +/* + * pg_get_user_home_dir - get the home directory of the user with the given ID + * + * On success, the directory path is returned into the buffer (of size buflen), + * and "true" is returned. On failure, a localized error message is + * returned into the buffer, and "false" is returned. + * + * Note that this does not incorporate the common behavior of checking + * $HOME first, since it's independent of which user_id is queried. + */ +bool +pg_get_user_home_dir(uid_t user_id, char *buffer, size_t buflen) +{ + char pwdbuf[BUFSIZ]; + struct passwd pwdstr; + struct passwd *pw = NULL; + int pwerr; + + pwerr = pqGetpwuid(user_id, &pwdstr, pwdbuf, sizeof(pwdbuf), &pw); + if (pw != NULL) + { + strlcpy(buffer, pw->pw_dir, buflen); + return true; + } + if (pwerr != 0) + snprintf(buffer, buflen, + _("could not look up local user ID %d: %s"), + (int) user_id, + strerror_r(pwerr, pwdbuf, sizeof(pwdbuf))); + else + snprintf(buffer, buflen, + _("local user with ID %d does not exist"), + (int) user_id); + return false; +} + +#endif /* !WIN32 */ diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index deb62c1c7b..ec3546d0c0 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -106,7 +106,7 @@ sub mkvcbuild pread.c preadv.c pwrite.c pwritev.c pg_bitutils.c pg_strong_random.c pgcheckdir.c pgmkdirp.c pgsleep.c pgstrcasecmp.c pqsignal.c mkdtemp.c qsort.c qsort_arg.c bsearch_arg.c quotes.c system.c - strerror.c tar.c thread.c + strerror.c tar.c win32env.c win32error.c win32ntdll.c win32security.c win32setlocale.c win32stat.c);