Fix pg_pwd caching mechanism, which was broken by changes to fork

postmaster children before client auth step.  Postmaster now rereads
pg_pwd on receipt of SIGHUP, the same way that pg_hba.conf is handled.
No cycles need be expended to validate password cache validity during
connection startup.

4 files changed, 124 insertions, 125 deletions

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 758cf365c80..3897a5c75ec 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Header: /cvsroot/pgsql/src/backend/commands/user.c,v 1.87 2001/11/01 18:09:58 tgl Exp $
+ * $Header: /cvsroot/pgsql/src/backend/commands/user.c,v 1.88 2001/11/02 18:39:57 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -15,6 +15,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <signal.h>
 #include <unistd.h>
 
 #include "access/heapam.h"
@@ -33,14 +34,15 @@
 #include "utils/syscache.h"
 
 
-static void CheckPgUserAclNotNull(void);
 extern bool Password_encryption;
 
+static void CheckPgUserAclNotNull(void);
+
 /*---------------------------------------------------------------------
  * write_password_file / update_pg_pwd
  *
  * copy the modified contents of pg_shadow to a file used by the postmaster
- * for user authentication.  The file is stored as $PGDATA/pg_pwd.
+ * for user authentication.  The file is stored as $PGDATA/global/pg_pwd.
  *
  * This function set is both a trigger function for direct updates to pg_shadow
  * as well as being called directly from create/alter/drop user.
@@ -57,7 +59,6 @@ write_password_file(Relation rel)
 			   *tempname;
 	int			bufsize;
 	FILE	   *fp;
-	int			flagfd;
 	mode_t		oumask;
 	HeapScanDesc scan;
 	HeapTuple	tuple;
@@ -133,7 +134,7 @@ write_password_file(Relation rel)
 		/*
 		 * The extra columns we emit here are not really necessary. To remove
 		 * them, the parser in backend/libpq/crypt.c would need to be
-		 * adjusted.  Initdb might also need adjustments.
+		 * adjusted.
 		 */
 		fprintf(fp,
 				"%s"
@@ -168,6 +169,7 @@ write_password_file(Relation rel)
 
 	/*
 	 * Rename the temp file to its final name, deleting the old pg_pwd.
+	 * We expect that rename(2) is an atomic action.
 	 */
 	if (rename(tempname, filename))
 		elog(ERROR, "rename %s to %s: %m", tempname, filename);
@@ -176,19 +178,10 @@ write_password_file(Relation rel)
 	pfree((void *) filename);
 
 	/*
-	 * Create a flag file the postmaster will detect the next time it
-	 * tries to authenticate a user.  The postmaster will know to reload
-	 * the pg_pwd file contents.  Note: we used to elog(ERROR) if the file
-	 * creation failed, but it's a little silly to abort the transaction
-	 * at this point, so let's just make it a NOTICE.
+	 * Signal the postmaster to reload its password-file cache.
 	 */
-	filename = crypt_getpwdreloadfilename();
-	flagfd = BasicOpenFile(filename, O_WRONLY | O_CREAT, 0600);
-	if (flagfd < 0)
-		elog(NOTICE, "write_password_file: unable to write %s: %m", filename);
-	else
-		close(flagfd);
-	pfree((void *) filename);
+	if (IsUnderPostmaster)
+		kill(getppid(), SIGHUP);
 }
 
 
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index 8decc40d0ba..83921ee014f 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -9,7 +9,7 @@
  * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Header: /cvsroot/pgsql/src/backend/libpq/crypt.c,v 1.40 2001/11/01 18:10:48 tgl Exp $
+ * $Header: /cvsroot/pgsql/src/backend/libpq/crypt.c,v 1.41 2001/11/02 18:39:57 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -17,6 +17,9 @@
 
 #include <errno.h>
 #include <unistd.h>
+#ifdef HAVE_CRYPT_H
+#include <crypt.h>
+#endif
 
 #include "libpq/crypt.h"
 #include "libpq/libpq.h"
@@ -24,15 +27,15 @@
 #include "storage/fd.h"
 #include "utils/nabstime.h"
 
-#ifdef HAVE_CRYPT_H
-#include <crypt.h>
-#endif
 
-char	  **pwd_cache = NULL;
-int			pwd_cache_count = 0;
+#define CRYPT_PWD_FILE	"pg_pwd"
+
+
+static char	  **pwd_cache = NULL;
+static int		pwd_cache_count = 0;
 
 /*
- * crypt_getpwdfilename --- get name of password file
+ * crypt_getpwdfilename --- get full pathname of password file
  *
  * Note that result string is palloc'd, and should be freed by the caller.
  */
@@ -50,28 +53,8 @@ crypt_getpwdfilename(void)
 }
 
 /*
- * crypt_getpwdreloadfilename --- get name of password-reload-needed flag file
- *
- * Note that result string is palloc'd, and should be freed by the caller.
+ * Open the password file if possible (return NULL if not)
  */
-char *
-crypt_getpwdreloadfilename(void)
-{
-	char	   *pwdfilename;
-	int			bufsize;
-	char	   *rpfnam;
-
-	pwdfilename = crypt_getpwdfilename();
-	bufsize = strlen(pwdfilename) + strlen(CRYPT_PWD_RELOAD_SUFX) + 1;
-	rpfnam = (char *) palloc(bufsize);
-	snprintf(rpfnam, bufsize, "%s%s", pwdfilename, CRYPT_PWD_RELOAD_SUFX);
-	pfree(pwdfilename);
-
-	return rpfnam;
-}
-
-/*-------------------------------------------------------------------------*/
-
 static FILE *
 crypt_openpwdfile(void)
 {
@@ -123,107 +106,128 @@ compar_user(const void *user_a, const void *user_b)
 	return result;
 }
 
-/*-------------------------------------------------------------------------*/
-
-static void
-crypt_loadpwdfile(void)
+/*
+ * Load or reload the password-file cache
+ */
+void
+load_password_cache(void)
 {
-	char	   *filename;
-	int			result;
 	FILE	   *pwd_file;
 	char		buffer[1024];
 
-	filename = crypt_getpwdreloadfilename();
-	result = unlink(filename);
-	pfree(filename);
-
 	/*
-	 * We want to delete the flag file before reading the contents of the
-	 * pg_pwd file.  If result == 0 then the unlink of the reload file was
-	 * successful. This means that a backend performed a COPY of the
-	 * pg_shadow file to pg_pwd.  Therefore we must now do a reload.
+	 * If for some reason we fail to open the password file, preserve the
+	 * old cache contents; this seems better than dropping the cache if,
+	 * say, we are temporarily out of filetable slots.
 	 */
-	if (!pwd_cache || result == 0)
+	if (!(pwd_file = crypt_openpwdfile()))
+		return;
+
+	/* free any old data */
+	if (pwd_cache)
 	{
-		/* free the old data only if this is a reload */
-		if (pwd_cache)
-		{
-			while (pwd_cache_count--)
-				free((void *) pwd_cache[pwd_cache_count]);
-			free((void *) pwd_cache);
-			pwd_cache = NULL;
-			pwd_cache_count = 0;
-		}
+		while (--pwd_cache_count >= 0)
+			pfree(pwd_cache[pwd_cache_count]);
+		pfree(pwd_cache);
+		pwd_cache = NULL;
+		pwd_cache_count = 0;
+	}
 
-		if (!(pwd_file = crypt_openpwdfile()))
-			return;
+	/*
+	 * Read the file and store its lines in current memory context,
+	 * which we expect will be PostmasterContext.  That context will
+	 * live as long as we need the cache to live, ie, until just after
+	 * each postmaster child has completed client authentication.
+	 */
+	while (fgets(buffer, sizeof(buffer), pwd_file) != NULL)
+	{
+		int			blen;
 
 		/*
-		 * Here is where we load the data from pg_pwd.
+		 * We must remove the return char at the end of the string, as
+		 * this will affect the correct parsing of the password entry.
 		 */
-		while (fgets(buffer, sizeof(buffer), pwd_file) != NULL)
-		{
-			/*
-			 * We must remove the return char at the end of the string, as
-			 * this will affect the correct parsing of the password entry.
-			 */
-			if (buffer[(result = strlen(buffer) - 1)] == '\n')
-				buffer[result] = '\0';
+		if (buffer[(blen = strlen(buffer) - 1)] == '\n')
+			buffer[blen] = '\0';
 
+		if (pwd_cache == NULL)
 			pwd_cache = (char **)
-				realloc((void *) pwd_cache,
-						sizeof(char *) * (pwd_cache_count + 1));
-			pwd_cache[pwd_cache_count++] = strdup(buffer);
-		}
-		FreeFile(pwd_file);
-
-		/*
-		 * Now sort the entries in the cache for faster searching later.
-		 */
-		qsort((void *) pwd_cache, pwd_cache_count, sizeof(char *), compar_user);
+				palloc(sizeof(char *) * (pwd_cache_count + 1));
+		else
+			pwd_cache = (char **)
+				repalloc((void *) pwd_cache,
+						 sizeof(char *) * (pwd_cache_count + 1));
+		pwd_cache[pwd_cache_count++] = pstrdup(buffer);
 	}
-}
 
-/*-------------------------------------------------------------------------*/
+	FreeFile(pwd_file);
+
+	/*
+	 * Now sort the entries in the cache for faster searching later.
+	 */
+	qsort((void *) pwd_cache, pwd_cache_count, sizeof(char *), compar_user);
+}
 
-static void
+/*
+ * Parse a line of the password file to extract password and valid-until date.
+ */
+static bool
 crypt_parsepwdentry(char *buffer, char **pwd, char **valdate)
 {
 	char	   *parse = buffer;
 	int			count,
 				i;
 
+	*pwd = NULL;
+	*valdate = NULL;
+
 	/*
 	 * skip to the password field
 	 */
 	for (i = 0; i < 6; i++)
-		parse += (strcspn(parse, CRYPT_PWD_FILE_SEPSTR) + 1);
+	{
+		parse += strcspn(parse, CRYPT_PWD_FILE_SEPSTR);
+		if (*parse == '\0')
+			return false;
+		parse++;
+	}
 
 	/*
 	 * store a copy of user password to return
 	 */
 	count = strcspn(parse, CRYPT_PWD_FILE_SEPSTR);
 	*pwd = (char *) palloc(count + 1);
-	strncpy(*pwd, parse, count);
+	memcpy(*pwd, parse, count);
 	(*pwd)[count] = '\0';
-	parse += (count + 1);
+	parse += count;
+	if (*parse == '\0')
+	{
+		pfree(*pwd);
+		*pwd = NULL;
+		return false;
+	}
+	parse++;
 
 	/*
 	 * store a copy of the date login becomes invalid
 	 */
 	count = strcspn(parse, CRYPT_PWD_FILE_SEPSTR);
 	*valdate = (char *) palloc(count + 1);
-	strncpy(*valdate, parse, count);
+	memcpy(*valdate, parse, count);
 	(*valdate)[count] = '\0';
-	parse += (count + 1);
-}
 
-/*-------------------------------------------------------------------------*/
+	return true;
+}
 
-static int
+/*
+ * Lookup a username in the password-file cache,
+ * return his password and valid-until date.
+ */
+static bool
 crypt_getloginfo(const char *user, char **passwd, char **valuntil)
 {
-	crypt_loadpwdfile();
+	*passwd = NULL;
+	*valuntil = NULL;
 
 	if (pwd_cache)
 	{
@@ -236,14 +240,12 @@ crypt_getloginfo(const char *user, char **passwd, char **valuntil)
 									  compar_user);
 		if (pwd_entry)
 		{
-			crypt_parsepwdentry(*pwd_entry, passwd, valuntil);
-			return STATUS_OK;
+			if (crypt_parsepwdentry(*pwd_entry, passwd, valuntil))
+				return true;
 		}
 	}
 
-	*passwd = NULL;
-	*valuntil = NULL;
-	return STATUS_ERROR;
+	return false;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -256,7 +258,7 @@ md5_crypt_verify(const Port *port, const char *user, const char *pgpass)
 			   *crypt_pwd;
 	int			retval = STATUS_ERROR;
 
-	if (crypt_getloginfo(user, &passwd, &valuntil) == STATUS_ERROR)
+	if (!crypt_getloginfo(user, &passwd, &valuntil))
 		return STATUS_ERROR;
 
 	if (passwd == NULL || *passwd == '\0')
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 1f232bfc301..2c96486cd06 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -37,7 +37,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v 1.253 2001/10/28 06:25:47 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v 1.254 2001/11/02 18:39:57 tgl Exp $
  *
  * NOTES
  *
@@ -747,6 +747,12 @@ PostmasterMain(int argc, char *argv[])
 		ExitPostmaster(1);
 
 	/*
+	 * Load cached files for client authentication.
+	 */
+	load_hba_and_ident();
+	load_password_cache();
+
+	/*
 	 * We're ready to rock and roll...
 	 */
 	StartupPID = StartupDataBase();
@@ -852,8 +858,6 @@ ServerLoop(void)
 				later;
 	struct timezone tz;
 
-	load_hba_and_ident();
-
 	gettimeofday(&now, &tz);
 
 	nSockets = initMasks(&readmask, &writemask);
@@ -925,6 +929,7 @@ ServerLoop(void)
 			got_SIGHUP = false;
 			ProcessConfigFile(PGC_SIGHUP);
 			load_hba_and_ident();
+			load_password_cache();
 		}
 
 		/*
diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.h
index 39d677e166b..76dabc89bd4 100644
--- a/src/include/libpq/crypt.h
+++ b/src/include/libpq/crypt.h
@@ -1,9 +1,13 @@
 /*-------------------------------------------------------------------------
  *
  * crypt.h
- *	  Interface to hba.c
+ *	  Interface to libpq/crypt.c
  *
+ * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
  *
+ * $Id: crypt.h,v 1.17 2001/11/02 18:39:57 tgl Exp $
+ * 
  *-------------------------------------------------------------------------
  */
 #ifndef PG_CRYPT_H
@@ -11,27 +15,22 @@
 
 #include "libpq/libpq-be.h"
 
-#define CRYPT_PWD_FILE	"pg_pwd"
-#define CRYPT_PWD_FILE_SEPCHAR	"'\\t'"
 #define CRYPT_PWD_FILE_SEPSTR	"\t"
-#define CRYPT_PWD_RELOAD_SUFX	".reload"
 
-extern char **pwd_cache;
-extern int	pwd_cache_count;
+#define MD5_PASSWD_LEN	35
+
+#define isMD5(passwd)	(strncmp((passwd),"md5",3) == 0 && \
+						 strlen(passwd) == MD5_PASSWD_LEN)
 
-extern char *crypt_getpwdfilename(void);
-extern char *crypt_getpwdreloadfilename(void);
 
-extern int	md5_crypt_verify(const Port *port, const char *user, const char *pgpass);
+extern char *crypt_getpwdfilename(void);
+extern void load_password_cache(void);
 
+extern int	md5_crypt_verify(const Port *port, const char *user,
+							 const char *pgpass);
 extern bool md5_hash(const void *buff, size_t len, char *hexsum);
 extern bool CheckMD5Pwd(char *passwd, char *storedpwd, char *seed);
 extern bool EncryptMD5(const char *passwd, const char *salt,
 		   size_t salt_len, char *buf);
 
-#define MD5_PASSWD_LEN	35
-
-#define isMD5(passwd)	(strncmp((passwd),"md5",3) == 0 && \
-						 strlen(passwd) == MD5_PASSWD_LEN)
-
 #endif