Re: Re: Proposal for encrypting pg_shadow passwords

> Bruce Momjian  writes:
> >> Bruce Momjian  writes:
> > Also, I now need two salts, one base62 for crypt and a new one for MD5.
> >> 
> >> They're carried in two different messages, so I don't see the problem.
> 
> > But the salt is configured on startup, before you know your auth method,
> > right?  Do I need to move that?
> 
> Oh, I see what you're looking at: the salt is computed at ConnCreate
> time in the postmaster.  Hmm.  You cannot move the call into the later
> auth process, because it needs to happen before the postmaster forks.
> (Else, every forked child would start with the same random() state and
> compute the same salt ... good security eh?)

Yes, I knew we had to do it in the postmaster but I couldn't remember
why. :-)

> Yes, I think initializing two salt fields in ConnCreate is fine.  That's
> probably actually a little more secure in itself, because it ensures
> that would-be sniffers cannot see every random() result in the
> postmaster's random() sequence, only some of them.  IIRC, that makes it
> a lot harder to guess the underlying seed.

OK, here is the patch for separate salts for crypt and MD5, and allowing
null's in MD5 salt.  I haven't tested it yet.

Yes, I realized computing it every time helps.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: src/backend/commands/user.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/commands/user.c,v
retrieving revision 1.81
diff -c -r1.81 user.c
*** src/backend/commands/user.c	2001/08/15 21:08:20	1.81
--- src/backend/commands/user.c	2001/08/16 18:06:43
***************
*** 351,357 ****
  				DirectFunctionCall1(textin, CStringGetDatum(password));
  		else
  		{
! 			if (!EncryptMD5(password, stmt->user, encrypted_password))
  				elog(ERROR, "CREATE USER: password encryption failed");
  			new_record[Anum_pg_shadow_passwd - 1] =
  				DirectFunctionCall1(textin, CStringGetDatum(encrypted_password));
--- 351,358 ----
  				DirectFunctionCall1(textin, CStringGetDatum(password));
  		else
  		{
! 			if (!EncryptMD5(password, stmt->user, strlen(stmt->user),
! 				encrypted_password))
  				elog(ERROR, "CREATE USER: password encryption failed");
  			new_record[Anum_pg_shadow_passwd - 1] =
  				DirectFunctionCall1(textin, CStringGetDatum(encrypted_password));
***************
*** 583,589 ****
  				DirectFunctionCall1(textin, CStringGetDatum(password));
  		else
  		{
! 			if (!EncryptMD5(password, stmt->user, encrypted_password))
  				elog(ERROR, "CREATE USER: password encryption failed");
  			new_record[Anum_pg_shadow_passwd - 1] =
  				DirectFunctionCall1(textin, CStringGetDatum(encrypted_password));
--- 584,591 ----
  				DirectFunctionCall1(textin, CStringGetDatum(password));
  		else
  		{
! 			if (!EncryptMD5(password, stmt->user, strlen(stmt->user),
! 				encrypted_password))
  				elog(ERROR, "CREATE USER: password encryption failed");
  			new_record[Anum_pg_shadow_passwd - 1] =
  				DirectFunctionCall1(textin, CStringGetDatum(encrypted_password));
Index: src/backend/libpq/auth.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.59
diff -c -r1.59 auth.c
*** src/backend/libpq/auth.c	2001/08/16 16:24:15	1.59
--- src/backend/libpq/auth.c	2001/08/16 18:06:43
***************
*** 536,545 ****
  	pq_sendint(&buf, (int32) areq, sizeof(int32));
  
  	/* Add the salt for encrypted passwords. */
! 	if (areq == AUTH_REQ_CRYPT || areq == AUTH_REQ_MD5)
  	{
! 		pq_sendint(&buf, port->salt[0], 1);
! 		pq_sendint(&buf, port->salt[1], 1);
  	}
  
  	pq_endmessage(&buf);
--- 536,552 ----
  	pq_sendint(&buf, (int32) areq, sizeof(int32));
  
  	/* Add the salt for encrypted passwords. */
! 	if (areq == AUTH_REQ_MD5)
  	{
! 		pq_sendint(&buf, port->md5Salt[0], 1);
! 		pq_sendint(&buf, port->md5Salt[1], 1);
! 		pq_sendint(&buf, port->md5Salt[2], 1);
! 		pq_sendint(&buf, port->md5Salt[3], 1);
! 	}
! 	if (areq == AUTH_REQ_CRYPT)
! 	{
! 		pq_sendint(&buf, port->cryptSalt[0], 1);
! 		pq_sendint(&buf, port->cryptSalt[1], 1);
  	}
  
  	pq_endmessage(&buf);
Index: src/backend/libpq/crypt.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/libpq/crypt.c,v
retrieving revision 1.34
diff -c -r1.34 crypt.c
*** src/backend/libpq/crypt.c	2001/08/15 21:08:21	1.34
--- src/backend/libpq/crypt.c	2001/08/16 18:06:43
***************
*** 283,289 ****
  	 switch (port->auth_method)
  	 {
  		case uaCrypt:
! 			crypt_pwd = crypt(passwd, port->salt);
  			break;
  		case uaMD5:
  			crypt_pwd = palloc(MD5_PASSWD_LEN+1);
--- 283,289 ----
  	 switch (port->auth_method)
  	 {
  		case uaCrypt:
! 			crypt_pwd = crypt(passwd, port->cryptSalt);
  			break;
  		case uaMD5:
  			crypt_pwd = palloc(MD5_PASSWD_LEN+1);
***************
*** 291,297 ****
  			if (isMD5(passwd))
  			{
  				if (!EncryptMD5(passwd + strlen("md5"),
! 								(char *)port->salt, crypt_pwd))
  				{
  					pfree(crypt_pwd);
  					return STATUS_ERROR;
--- 291,298 ----
  			if (isMD5(passwd))
  			{
  				if (!EncryptMD5(passwd + strlen("md5"),
! 					(char *)port->md5Salt, sizeof(port->md5Salt),
! 					crypt_pwd))
  				{
  					pfree(crypt_pwd);
  					return STATUS_ERROR;
***************
*** 301,314 ****
  			{
  				char *crypt_pwd2 = palloc(MD5_PASSWD_LEN+1);
  
! 				if (!EncryptMD5(passwd, port->user, crypt_pwd2))
  				{
  					pfree(crypt_pwd);
  					pfree(crypt_pwd2);
  					return STATUS_ERROR;
  				}
! 				if (!EncryptMD5(crypt_pwd2 + strlen("md5"), port->salt,
! 								crypt_pwd))
  				{
  					pfree(crypt_pwd);
  					pfree(crypt_pwd2);
--- 302,316 ----
  			{
  				char *crypt_pwd2 = palloc(MD5_PASSWD_LEN+1);
  
! 				if (!EncryptMD5(passwd, port->user, strlen(port->user),
! 					crypt_pwd2))
  				{
  					pfree(crypt_pwd);
  					pfree(crypt_pwd2);
  					return STATUS_ERROR;
  				}
! 				if (!EncryptMD5(crypt_pwd2 + strlen("md5"), port->md5Salt,
! 								sizeof(port->md5Salt), crypt_pwd))
  				{
  					pfree(crypt_pwd);
  					pfree(crypt_pwd2);
Index: src/backend/libpq/md5.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/libpq/md5.c,v
retrieving revision 1.3
diff -c -r1.3 md5.c
*** src/backend/libpq/md5.c	2001/08/15 23:22:49	1.3
--- src/backend/libpq/md5.c	2001/08/16 18:06:44
***************
*** 295,310 ****
   * puts  md5(username+passwd)  in buf provided buflen is at least 36 bytes
   * returns 1 on success, 0 on any kind of failure and sets errno accordingly
   */
! bool EncryptMD5(const char *passwd, const char *salt, char *buf)
  {
  	char crypt_buf[128];
  
! 	if (strlen(salt) + strlen(passwd) > 127)
  		return false;
  
  	strcpy(buf, "md5");
  	memset(crypt_buf, 0, 128);
! 	sprintf(crypt_buf,"%s%s", salt, passwd);
  
! 	return md5_hash(crypt_buf, strlen(crypt_buf), buf + 3);
  }
--- 295,312 ----
   * puts  md5(username+passwd)  in buf provided buflen is at least 36 bytes
   * returns 1 on success, 0 on any kind of failure and sets errno accordingly
   */
! bool EncryptMD5(const char *passwd, const char *salt, size_t salt_len,
! 				char *buf)
  {
  	char crypt_buf[128];
  
! 	if (salt_len + strlen(passwd) > 127)
  		return false;
  
  	strcpy(buf, "md5");
  	memset(crypt_buf, 0, 128);
! 	memcpy(crypt_buf, salt, salt_len);
! 	memcpy(crypt_buf+salt_len, passwd, strlen(passwd));
  
! 	return md5_hash(crypt_buf, salt_len + strlen(passwd), buf + 3);
  }
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.235
diff -c -r1.235 postmaster.c
*** src/backend/postmaster/postmaster.c	2001/08/05 02:06:50	1.235
--- src/backend/postmaster/postmaster.c	2001/08/16 18:06:45
***************
*** 243,249 ****
  static int	initMasks(fd_set *rmask, fd_set *wmask);
  static char *canAcceptConnections(void);
  static long PostmasterRandom(void);
! static void RandomSalt(char *salt);
  static void SignalChildren(int signal);
  static int	CountChildren(void);
  static bool CreateOptsFile(int argc, char *argv[]);
--- 243,249 ----
  static int	initMasks(fd_set *rmask, fd_set *wmask);
  static char *canAcceptConnections(void);
  static long PostmasterRandom(void);
! static void RandomSalt(char *cryptSalt, char *md5Salt);
  static void SignalChildren(int signal);
  static int	CountChildren(void);
  static bool CreateOptsFile(int argc, char *argv[]);
***************
*** 1211,1217 ****
  	}
  	else
  	{
! 		RandomSalt(port->salt);
  		port->pktInfo.state = Idle;
  	}
  
--- 1211,1217 ----
  	}
  	else
  	{
! 		RandomSalt(port->cryptSalt, port->md5Salt);
  		port->pktInfo.state = Idle;
  	}
  
***************
*** 2099,2110 ****
   * RandomSalt
   */
  static void
! RandomSalt(char *salt)
  {
  	long		rand = PostmasterRandom();
  
! 	*salt = CharRemap(rand % 62);
! 	*(salt + 1) = CharRemap(rand / 62);
  }
  
  /*
--- 2099,2113 ----
   * RandomSalt
   */
  static void
! RandomSalt(char *cryptSalt, char *md5Salt)
  {
  	long		rand = PostmasterRandom();
+ 	int			i;
  
! 	cryptSalt[0] = CharRemap(rand % 62);
! 	cryptSalt[1] = CharRemap(rand / 62);
! 	for (i=0; i < 4; i++)
! 		md5Salt[i] = (rand >> (i * 4)) & 0xff;
  }
  
  /*
Index: src/include/libpq/crypt.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/libpq/crypt.h,v
retrieving revision 1.13
diff -c -r1.13 crypt.h
*** src/include/libpq/crypt.h	2001/08/15 21:08:21	1.13
--- src/include/libpq/crypt.h	2001/08/16 18:06:46
***************
*** 26,32 ****
  
  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, char *buf);
  
  #define MD5_PASSWD_LEN	35
  
--- 26,33 ----
  
  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
  
Index: src/include/libpq/libpq-be.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/libpq/libpq-be.h,v
retrieving revision 1.21
diff -c -r1.21 libpq-be.h
*** src/include/libpq/libpq-be.h	2001/01/24 19:43:24	1.21
--- src/include/libpq/libpq-be.h	2001/08/16 18:06:46
***************
*** 58,64 ****
  
  typedef struct AuthRequestPacket
  {
! 	char		data[1 + sizeof(AuthRequest) + 2];		/* 'R' + the request +
  														 * optional salt. */
  } AuthRequestPacket;
  
--- 58,64 ----
  
  typedef struct AuthRequestPacket
  {
! 	char		data[1 + sizeof(AuthRequest) + 4];		/* 'R' + the request +
  														 * optional salt. */
  } AuthRequestPacket;
  
***************
*** 119,125 ****
  	Packet		pktInfo;		/* For the packet handlers */
  	SockAddr	laddr;			/* local addr (us) */
  	SockAddr	raddr;			/* remote addr (them) */
! 	char		salt[2];		/* Password salt */
  
  	/*
  	 * Information that needs to be held during the fe/be authentication
--- 119,126 ----
  	Packet		pktInfo;		/* For the packet handlers */
  	SockAddr	laddr;			/* local addr (us) */
  	SockAddr	raddr;			/* remote addr (them) */
! 	char		md5Salt[4];		/* Password salt */
!  	char		cryptSalt[2];	/* Password salt */
  
  	/*
  	 * Information that needs to be held during the fe/be authentication
Index: src/interfaces/libpq/fe-auth.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/fe-auth.c,v
retrieving revision 1.50
diff -c -r1.50 fe-auth.c
*** src/interfaces/libpq/fe-auth.c	2001/08/15 21:08:21	1.50
--- src/interfaces/libpq/fe-auth.c	2001/08/16 18:06:47
***************
*** 443,449 ****
  	switch (areq)
  	{
  		case AUTH_REQ_CRYPT:
! 			crypt_pwd = crypt(password, conn->salt);
  			break;
  		case AUTH_REQ_MD5:
  			{
--- 443,449 ----
  	switch (areq)
  	{
  		case AUTH_REQ_CRYPT:
! 			crypt_pwd = crypt(password, conn->cryptSalt);
  			break;
  		case AUTH_REQ_MD5:
  			{
***************
*** 455,468 ****
  					perror("malloc");
  					return STATUS_ERROR;
  				}
! 				if (!EncryptMD5(password, conn->pguser, crypt_pwd2))
  				{
  					free(crypt_pwd);
  					free(crypt_pwd2);
  					return STATUS_ERROR;
  				}
! 				if (!EncryptMD5(crypt_pwd2 + strlen("md5"), conn->salt,
! 								crypt_pwd))
  				{
  					free(crypt_pwd);
  					free(crypt_pwd2);
--- 455,469 ----
  					perror("malloc");
  					return STATUS_ERROR;
  				}
! 				if (!EncryptMD5(password, conn->pguser,
! 					strlen(conn->pguser), crypt_pwd2))
  				{
  					free(crypt_pwd);
  					free(crypt_pwd2);
  					return STATUS_ERROR;
  				}
! 				if (!EncryptMD5(crypt_pwd2 + strlen("md5"), conn->md5Salt,
! 								strlen(conn->md5Salt), crypt_pwd))
  				{
  					free(crypt_pwd);
  					free(crypt_pwd2);
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.173
diff -c -r1.173 fe-connect.c
*** src/interfaces/libpq/fe-connect.c	2001/08/15 18:42:15	1.173
--- src/interfaces/libpq/fe-connect.c	2001/08/16 18:06:48
***************
*** 1341,1349 ****
  				}
  
  				/* Get the password salt if there is one. */
! 				if (areq == AUTH_REQ_CRYPT || areq == AUTH_REQ_MD5)
  				{
! 					if (pqGetnchar(conn->salt, sizeof(conn->salt), conn))
  					{
  						/* We'll come back when there are more data */
  						return PGRES_POLLING_READING;
--- 1341,1359 ----
  				}
  
  				/* Get the password salt if there is one. */
! 				if (areq == AUTH_REQ_MD5)
  				{
! 					if (pqGetnchar(conn->md5Salt,
! 						sizeof(conn->md5Salt)-1, conn))
! 					{
! 						/* We'll come back when there are more data */
! 						return PGRES_POLLING_READING;
! 					}
! 				}
! 				if (areq == AUTH_REQ_CRYPT)
! 				{
! 					if (pqGetnchar(conn->cryptSalt,
! 						sizeof(conn->cryptSalt)-1, conn))
  					{
  						/* We'll come back when there are more data */
  						return PGRES_POLLING_READING;
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.38
diff -c -r1.38 libpq-int.h
*** src/interfaces/libpq/libpq-int.h	2001/08/16 04:27:18	1.38
--- src/interfaces/libpq/libpq-int.h	2001/08/16 18:06:49
***************
*** 236,242 ****
  	/* Miscellaneous stuff */
  	int			be_pid;			/* PID of backend --- needed for cancels */
  	int			be_key;			/* key of backend --- needed for cancels */
! 	char		salt[2];		/* password salt received from backend */
  	PGlobjfuncs *lobjfuncs;		/* private state for large-object access
  								 * fns */
  
--- 236,243 ----
  	/* Miscellaneous stuff */
  	int			be_pid;			/* PID of backend --- needed for cancels */
  	int			be_key;			/* key of backend --- needed for cancels */
!  	char		md5Salt[4];		/* password salt received from backend */
!  	char		cryptSalt[2];		/* password salt received from backend */
  	PGlobjfuncs *lobjfuncs;		/* private state for large-object access
  								 * fns */
  
Index: src/interfaces/odbc/connection.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/odbc/connection.c,v
retrieving revision 1.33
diff -c -r1.33 connection.c
*** src/interfaces/odbc/connection.c	2001/08/15 18:42:16	1.33
--- src/interfaces/odbc/connection.c	2001/08/16 18:06:49
***************
*** 507,513 ****
  	int			areq = -1;
  	int			beresp;
  	char		msgbuffer[ERROR_MSG_LENGTH];
! 	char		salt[2];
  	static char *func = "CC_connect";
  
  	mylog("%s: entering...\n", func);
--- 507,513 ----
  	int			areq = -1;
  	int			beresp;
  	char		msgbuffer[ERROR_MSG_LENGTH];
! 	char		salt[5];
  	static char *func = "CC_connect";
  
  	mylog("%s: entering...\n", func);
***************
*** 677,683 ****
  						mylog("auth got 'R'\n");
  
  						areq = SOCK_get_int(sock, 4);
! 						if (areq == AUTH_REQ_CRYPT || areq == AUTH_REQ_MD5)
  							SOCK_get_n_char(sock, salt, 2);
  
  						mylog("areq = %d\n", areq);
--- 677,685 ----
  						mylog("auth got 'R'\n");
  
  						areq = SOCK_get_int(sock, 4);
! 						if (areq == AUTH_REQ_MD5)
! 							SOCK_get_n_char(sock, salt, 4);
! 						if (areq == AUTH_REQ_CRYPT)
  							SOCK_get_n_char(sock, salt, 2);
  
  						mylog("areq = %d\n", areq);