Re: [HACKERS] User authentication bug?

Maarten Boekhold <maartenb@dutepp2.et.tudelft.nl> writes:
> OK, I now know what's going on, at least at my home (I had this problem 
> on another server, dunno if it's caused by the same thing):
> 	I had a password longer than 8 characters in pg_shadow.
> when creating a user, postgres happily accepts more than 8 chars, and 
> also stores them. apparently libpq-fe (or psql, dunno) only sends 8 
> chars.

It's not libpq's fault (at least not with the current sources).
It's psql's.  psql.c had a hardwired limit of 8 characters on
both the username and the password.  Ick.

With the attached patch, I have verified that long (> 8char anyway)
usernames and passwords work correctly in both "password" and "crypt"
authorization mode.  NOTE: at least on my machine, it seems that the
crypt() routines ignore the part of the password beyond 8 characters,
so there's no security gain from longer passwords in crypt auth mode.
But they don't fail.

The login-related part of psql has apparently not been touched since
roughly the fall of Rome ;-).  It was going through huge pushups to
get around the lack of username/login parameters to PQsetdb.  I don't
know when PQsetdbLogin was added to libpq, but it's there now ... so
I was able to rip out quite a lot of crufty code while I was at it.

It's possible that there are still bogus length limits on username
or password in some of the other PostgreSQL user interfaces besides
psql/libpq.  I will leave it to other folks to check that code.

			regards, tom lane


*** src/bin/psql/psql.c.orig	Sat Jul 18 14:34:14 1998
--- src/bin/psql/psql.c	Sat Aug  1 20:34:47 1998
***************
*** 132,140 ****
  static int	objectDescription(PsqlSettings *pset, char *object);
  static int	rightsList(PsqlSettings *pset);
  static void prompt_for_password(char *username, char *password);
- static char *
- make_connect_string(char *host, char *port, char *dbname,
- 					char *username, char *password);
  
  static char *gets_noreadline(char *prompt, FILE *source);
  static char *gets_readline(char *prompt, FILE *source);
--- 132,137 ----
***************
*** 1372,1406 ****
  	else
  	{
  		PGconn	   *olddb = pset->db;
- 		static char *userenv = NULL;
- 		char	   *old_userenv = NULL;
  		const char *dbparam;
! 
! 		if (new_user != NULL)
! 		{
! 
! 			/*
! 			 * PQsetdb() does not allow us to specify the user, so we have
! 			 * to do it via PGUSER
! 			 */
! 			if (userenv != NULL)
! 				old_userenv = userenv;
! 			userenv = malloc(strlen("PGUSER=") + strlen(new_user) + 1);
! 			sprintf(userenv, "PGUSER=%s", new_user);
! 			/* putenv() may continue to use memory as part of environment */
! 			putenv(userenv);
! 			/* can delete old memory if we malloc'ed it */
! 			if (old_userenv != NULL)
! 				free(old_userenv);
! 		}
  
  		if (strcmp(new_dbname, "-") != 0)
  			dbparam = new_dbname;
  		else
  			dbparam = PQdb(olddb);
  
! 		pset->db = PQsetdb(PQhost(olddb), PQport(olddb),
! 						   NULL, NULL, dbparam);
  		if (!pset->quiet)
  		{
  			if (!new_user)
--- 1369,1396 ----
  	else
  	{
  		PGconn	   *olddb = pset->db;
  		const char *dbparam;
! 		const char *userparam;
! 		const char *pwparam;
  
  		if (strcmp(new_dbname, "-") != 0)
  			dbparam = new_dbname;
  		else
  			dbparam = PQdb(olddb);
  
! 		if (new_user != NULL && strcmp(new_user, "-") != 0)
! 			userparam = new_user;
! 		else
! 			userparam = PQuser(olddb);
! 
! 		/* libpq doesn't provide an accessor function for the password,
! 		 * so we cheat here.
! 		 */
! 		pwparam = olddb->pgpass;
! 
! 		pset->db = PQsetdbLogin(PQhost(olddb), PQport(olddb),
! 								NULL, NULL, dbparam, userparam, pwparam);
! 
  		if (!pset->quiet)
  		{
  			if (!new_user)
***************
*** 2711,2726 ****
  
  	if (settings.getPassword)
  	{
! 		char		username[9];
! 		char		password[9];
! 		char	   *connect_string;
  
  		prompt_for_password(username, password);
  
! 		/* now use PQconnectdb so we can pass these options */
! 		connect_string = make_connect_string(host, port, dbname, username, password);
! 		settings.db = PQconnectdb(connect_string);
! 		free(connect_string);
  	}
  	else
  		settings.db = PQsetdb(host, port, NULL, NULL, dbname);
--- 2701,2713 ----
  
  	if (settings.getPassword)
  	{
! 		char		username[100];
! 		char		password[100];
  
  		prompt_for_password(username, password);
  
! 		settings.db = PQsetdbLogin(host, port, NULL, NULL, dbname,
! 								   username, password);
  	}
  	else
  		settings.db = PQsetdb(host, port, NULL, NULL, dbname);
***************
*** 2730,2736 ****
  	if (PQstatus(settings.db) == CONNECTION_BAD)
  	{
  		fprintf(stderr, "Connection to database '%s' failed.\n", dbname);
! 		fprintf(stderr, "%s", PQerrorMessage(settings.db));
  		PQfinish(settings.db);
  		exit(1);
  	}
--- 2717,2723 ----
  	if (PQstatus(settings.db) == CONNECTION_BAD)
  	{
  		fprintf(stderr, "Connection to database '%s' failed.\n", dbname);
! 		fprintf(stderr, "%s\n", PQerrorMessage(settings.db));
  		PQfinish(settings.db);
  		exit(1);
  	}
***************
*** 2964,2969 ****
--- 2951,2957 ----
  static void
  prompt_for_password(char *username, char *password)
  {
+ 	char buf[512];
  	int			length;
  
  #ifdef HAVE_TERMIOS_H
***************
*** 2973,2985 ****
  #endif
  
  	printf("Username: ");
! 	fgets(username, 9, stdin);
  	length = strlen(username);
  	/* skip rest of the line */
  	if (length > 0 && username[length - 1] != '\n')
  	{
- 		static char buf[512];
- 
  		do
  		{
  			fgets(buf, 512, stdin);
--- 2961,2971 ----
  #endif
  
  	printf("Username: ");
! 	fgets(username, 100, stdin);
  	length = strlen(username);
  	/* skip rest of the line */
  	if (length > 0 && username[length - 1] != '\n')
  	{
  		do
  		{
  			fgets(buf, 512, stdin);
***************
*** 2995,3001 ****
  	t.c_lflag &= ~ECHO;
  	tcsetattr(0, TCSADRAIN, &t);
  #endif
! 	fgets(password, 9, stdin);
  #ifdef HAVE_TERMIOS_H
  	tcsetattr(0, TCSADRAIN, &t_orig);
  #endif
--- 2981,2987 ----
  	t.c_lflag &= ~ECHO;
  	tcsetattr(0, TCSADRAIN, &t);
  #endif
! 	fgets(password, 100, stdin);
  #ifdef HAVE_TERMIOS_H
  	tcsetattr(0, TCSADRAIN, &t_orig);
  #endif
***************
*** 3004,3011 ****
  	/* skip rest of the line */
  	if (length > 0 && password[length - 1] != '\n')
  	{
- 		static char buf[512];
- 
  		do
  		{
  			fgets(buf, 512, stdin);
--- 2990,2995 ----
***************
*** 3015,3077 ****
  		password[length - 1] = '\0';
  
  	printf("\n\n");
- }
- 
- static char *
- make_connect_string(char *host, char *port, char *dbname,
- 					char *username, char *password)
- {
- 	int			connect_string_len = 0;
- 	char	   *connect_string;
- 
- 	if (host)
- 		connect_string_len += 6 + strlen(host); /* 6 == "host=" + " " */
- 	if (username)
- 		connect_string_len += 6 + strlen(username);		/* 6 == "user=" + " " */
- 	if (password)
- 		connect_string_len += 10 + strlen(password);	/* 10 == "password=" + "
- 														 * " */
- 	if (port)
- 		connect_string_len += 6 + strlen(port); /* 6 == "port=" + " " */
- 	if (dbname)
- 		connect_string_len += 8 + strlen(dbname);		/* 8 == "dbname=" + " " */
- 	connect_string_len += 18;	/* "authtype=password" + null */
- 
- 	connect_string = (char *) malloc(connect_string_len);
- 	if (!connect_string)
- 		return 0;
- 	connect_string[0] = '\0';
- 	if (host)
- 	{
- 		strcat(connect_string, "host=");
- 		strcat(connect_string, host);
- 		strcat(connect_string, " ");
- 	}
- 	if (username)
- 	{
- 		strcat(connect_string, "user=");
- 		strcat(connect_string, username);
- 		strcat(connect_string, " ");
- 	}
- 	if (password)
- 	{
- 		strcat(connect_string, "password=");
- 		strcat(connect_string, password);
- 		strcat(connect_string, " ");
- 	}
- 	if (port)
- 	{
- 		strcat(connect_string, "port=");
- 		strcat(connect_string, port);
- 		strcat(connect_string, " ");
- 	}
- 	if (dbname)
- 	{
- 		strcat(connect_string, "dbname=");
- 		strcat(connect_string, dbname);
- 		strcat(connect_string, " ");
- 	}
- 	strcat(connect_string, "authtype=password");
- 
- 	return connect_string;
  }
--- 2999,3002 ----