Magnus Hagander wrote:
> I notice our docs have:
> 
>     If you are at all concerned about password
>     <quote>sniffing</> attacks then <literal>md5</> is preferred, with
>     <literal>crypt</> to be used only if you must support pre-7.2
>     clients. Plain <literal>password</> should be avoided especially for
> 
> 
> At what point do we just remove the support and say that people need to
> upgrade their clients? Sure, it's up to ppl not to configure it that
> way, but security-wise it's a foot-gun that I think is completely
> unnecessary.

Here's a patch that does this. Will apply unless there are objections.

//Magnus

*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
***************
*** 316,339 **** hostnossl  <replaceable>database</replaceable>  <replaceable>user</replaceable>
         </varlistentry>
  
         <varlistentry>
-         <term><literal>crypt</></term>
-         <listitem>
-          <note>
-          <para>
-           This option is recommended only for communicating with pre-7.2
-           clients.
-          </para>
-          </note>
-          <para>
-           Require the client to supply a <function>crypt()</>-encrypted
-           password for authentication.
-           <literal>md5</literal> is now recommended over <literal>crypt</>.
-           See <xref linkend="auth-password"> for details.
-          </para>
-         </listitem>
-        </varlistentry>
- 
-        <varlistentry>
          <term><literal>password</></term>
          <listitem>
           <para>
--- 316,321 ----
***************
*** 705,734 **** omicron       bryanh            guest1
      <primary>MD5</>
     </indexterm>
     <indexterm>
-     <primary>crypt</>
-    </indexterm>
-    <indexterm>
      <primary>password</primary>
      <secondary>authentication</secondary>
     </indexterm>
  
     <para>
      The password-based authentication methods are <literal>md5</>,
!     <literal>crypt</>, and <literal>password</>. These methods operate
      similarly except for the way that the password is sent across the
!     connection: respectively, MD5-hashed, crypt-encrypted, and clear-text.
!     A limitation is that the <literal>crypt</> method does not work with
!     passwords that have been encrypted in <structname>pg_authid</structname>.
     </para>
  
     <para>
      If you are at all concerned about password
!     <quote>sniffing</> attacks then <literal>md5</> is preferred, with
!     <literal>crypt</> to be used only if you must support pre-7.2
!     clients. Plain <literal>password</> should be avoided especially for
!     connections over the open Internet (unless you use <acronym>SSL</acronym>,
!     <acronym>SSH</>, or another
!     communications security wrapper around the connection).
     </para>
  
     <para>
--- 687,707 ----
      <primary>MD5</>
     </indexterm>
     <indexterm>
      <primary>password</primary>
      <secondary>authentication</secondary>
     </indexterm>
  
     <para>
      The password-based authentication methods are <literal>md5</>,
!     and <literal>password</>. These methods operate
      similarly except for the way that the password is sent across the
!     connection: respectively, MD5-hashed and clear-text.
     </para>
  
     <para>
      If you are at all concerned about password
!     <quote>sniffing</> attacks then <literal>md5</> is preferred.
!     Plain <literal>password</> should always be avoided if possible.
     </para>
  
     <para>
*** a/doc/src/sgml/protocol.sgml
--- b/doc/src/sgml/protocol.sgml
***************
*** 296,314 ****
       </varlistentry>
  
       <varlistentry>
-       <term>AuthenticationCryptPassword</term>
-       <listitem>
-        <para>
-         The frontend must now send a PasswordMessage containing the
-         password encrypted via crypt(3), using the 2-character salt
-         specified in the AuthenticationCryptPassword message.  If
-         this is the correct password, the server responds with an
-         AuthenticationOk, otherwise it responds with an ErrorResponse.
-        </para>
-       </listitem>
-      </varlistentry>
- 
-      <varlistentry>
        <term>AuthenticationMD5Password</term>
        <listitem>
         <para>
--- 296,301 ----
***************
*** 1533,1593 **** AuthenticationCleartextPassword (B)
  
  <varlistentry>
  <term>
- AuthenticationCryptPassword (B)
- </term>
- <listitem>
- <para>
- 
- <variablelist>
- <varlistentry>
- <term>
-         Byte1('R')
- </term>
- <listitem>
- <para>
-                 Identifies the message as an authentication request.
- </para>
- </listitem>
- </varlistentry>
- <varlistentry>
- <term>
-         Int32(10)
- </term>
- <listitem>
- <para>
-                 Length of message contents in bytes, including self.
- </para>
- </listitem>
- </varlistentry>
- <varlistentry>
- <term>
-         Int32(4)
- </term>
- <listitem>
- <para>
-                 Specifies that a crypt()-encrypted password is required.
- </para>
- </listitem>
- </varlistentry>
- <varlistentry>
- <term>
-         Byte2
- </term>
- <listitem>
- <para>
-                 The salt to use when encrypting the password.
- </para>
- </listitem>
- </varlistentry>
- </variablelist>
- 
- </para>
- </listitem>
- </varlistentry>
- 
- 
- <varlistentry>
- <term>
  AuthenticationMD5Password (B)
  </term>
  <listitem>
--- 1520,1525 ----
*** a/doc/src/sgml/user-manag.sgml
--- b/doc/src/sgml/user-manag.sgml
***************
*** 215,222 **** CREATE USER <replaceable>name</replaceable>;
         <para>
          A password is only significant if the client authentication
          method requires the user to supply a password when connecting
!         to the database. The <option>password</>,
!         <option>md5</>, and <option>crypt</> authentication methods
          make use of passwords. Database passwords are separate from
          operating system passwords. Specify a password upon role
          creation with <literal>CREATE ROLE
--- 215,222 ----
         <para>
          A password is only significant if the client authentication
          method requires the user to supply a password when connecting
!         to the database. The <option>password</> and
!         <option>md5</> authentication methods
          make use of passwords. Database passwords are separate from
          operating system passwords. Specify a password upon role
          creation with <literal>CREATE ROLE
*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
***************
*** 230,236 **** auth_failed(Port *port, int status)
  			errstr = gettext_noop("Ident authentication failed for user \"%s\"");
  			break;
  		case uaMD5:
- 		case uaCrypt:
  		case uaPassword:
  			errstr = gettext_noop("password authentication failed for user \"%s\"");
  			break;
--- 230,235 ----
***************
*** 373,383 **** ClientAuthentication(Port *port)
  			status = recv_and_check_password_packet(port);
  			break;
  
- 		case uaCrypt:
- 			sendAuthRequest(port, AUTH_REQ_CRYPT);
- 			status = recv_and_check_password_packet(port);
- 			break;
- 
  		case uaPassword:
  			sendAuthRequest(port, AUTH_REQ_PASSWORD);
  			status = recv_and_check_password_packet(port);
--- 372,377 ----
***************
*** 426,433 **** sendAuthRequest(Port *port, AuthRequest areq)
  	/* Add the salt for encrypted passwords. */
  	if (areq == AUTH_REQ_MD5)
  		pq_sendbytes(&buf, port->md5Salt, 4);
- 	else if (areq == AUTH_REQ_CRYPT)
- 		pq_sendbytes(&buf, port->cryptSalt, 2);
  
  #if defined(ENABLE_GSS) || defined(ENABLE_SSPI)
  
--- 420,425 ----
*** a/src/backend/libpq/crypt.c
--- b/src/backend/libpq/crypt.c
***************
*** 53,66 **** md5_crypt_verify(const Port *port, const char *role, char *client_pass)
  	if (shadow_pass == NULL || *shadow_pass == '\0')
  		return STATUS_ERROR;
  
- 	/* We can't do crypt with MD5 passwords */
- 	if (isMD5(shadow_pass) && port->hba->auth_method == uaCrypt)
- 	{
- 		ereport(LOG,
- 				(errmsg("cannot use authentication method \"crypt\" because password is MD5-encrypted")));
- 		return STATUS_ERROR;
- 	}
- 
  	/*
  	 * Compare with the encrypted or plain password depending on the
  	 * authentication method being used for this connection.
--- 53,58 ----
***************
*** 106,119 **** md5_crypt_verify(const Port *port, const char *role, char *client_pass)
  				pfree(crypt_pwd2);
  			}
  			break;
- 		case uaCrypt:
- 			{
- 				char		salt[3];
- 
- 				strlcpy(salt, port->cryptSalt, sizeof(salt));
- 				crypt_pwd = crypt(shadow_pass, salt);
- 				break;
- 			}
  		default:
  			if (isMD5(shadow_pass))
  			{
--- 98,103 ----
*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
***************
*** 791,798 **** parse_hba_line(List *line, int line_num, HbaLine *parsedline)
  		parsedline->auth_method = uaReject;
  	else if (strcmp(token, "md5") == 0)
  		parsedline->auth_method = uaMD5;
- 	else if (strcmp(token, "crypt") == 0)
- 		parsedline->auth_method = uaCrypt;
  	else if (strcmp(token, "pam") == 0)
  #ifdef USE_PAM
  		parsedline->auth_method = uaPAM;
--- 791,796 ----
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
***************
*** 323,329 **** static int	initMasks(fd_set *rmask);
  static void report_fork_failure_to_client(Port *port, int errnum);
  static enum CAC_state canAcceptConnections(void);
  static long PostmasterRandom(void);
! static void RandomSalt(char *cryptSalt, char *md5Salt);
  static void signal_child(pid_t pid, int signal);
  static void SignalSomeChildren(int signal, bool only_autovac);
  
--- 323,329 ----
  static void report_fork_failure_to_client(Port *port, int errnum);
  static enum CAC_state canAcceptConnections(void);
  static long PostmasterRandom(void);
! static void RandomSalt(char *md5Salt);
  static void signal_child(pid_t pid, int signal);
  static void SignalSomeChildren(int signal, bool only_autovac);
  
***************
*** 1808,1814 **** ConnCreate(int serverFd)
  		 * fork, not after.  Else the postmaster's random sequence won't get
  		 * advanced, and all backends would end up using the same salt...
  		 */
! 		RandomSalt(port->cryptSalt, port->md5Salt);
  	}
  
  	/*
--- 1808,1814 ----
  		 * fork, not after.  Else the postmaster's random sequence won't get
  		 * advanced, and all backends would end up using the same salt...
  		 */
! 		RandomSalt(port->md5Salt);
  	}
  
  	/*
***************
*** 3910,3958 **** dummy_handler(SIGNAL_ARGS)
  {
  }
  
- 
- /*
-  * CharRemap: given an int in range 0..61, produce textual encoding of it
-  * per crypt(3) conventions.
-  */
- static char
- CharRemap(long ch)
- {
- 	if (ch < 0)
- 		ch = -ch;
- 	ch = ch % 62;
- 
- 	if (ch < 26)
- 		return 'A' + ch;
- 
- 	ch -= 26;
- 	if (ch < 26)
- 		return 'a' + ch;
- 
- 	ch -= 26;
- 	return '0' + ch;
- }
- 
  /*
   * RandomSalt
   */
  static void
! RandomSalt(char *cryptSalt, char *md5Salt)
  {
! 	long		rand = PostmasterRandom();
! 
! 	cryptSalt[0] = CharRemap(rand % 62);
! 	cryptSalt[1] = CharRemap(rand / 62);
  
  	/*
- 	 * It's okay to reuse the first random value for one of the MD5 salt
- 	 * bytes, since only one of the two salts will be sent to the client.
- 	 * After that we need to compute more random bits.
- 	 *
  	 * We use % 255, sacrificing one possible byte value, so as to ensure that
  	 * all bits of the random() value participate in the result. While at it,
  	 * add one to avoid generating any null bytes.
  	 */
  	md5Salt[0] = (rand % 255) + 1;
  	rand = PostmasterRandom();
  	md5Salt[1] = (rand % 255) + 1;
--- 3910,3929 ----
  {
  }
  
  /*
   * RandomSalt
   */
  static void
! RandomSalt(char *md5Salt)
  {
! 	long		rand;
  
  	/*
  	 * We use % 255, sacrificing one possible byte value, so as to ensure that
  	 * all bits of the random() value participate in the result. While at it,
  	 * add one to avoid generating any null bytes.
  	 */
+ 	rand = PostmasterRandom();
  	md5Salt[0] = (rand % 255) + 1;
  	rand = PostmasterRandom();
  	md5Salt[1] = (rand % 255) + 1;
*** a/src/include/libpq/hba.h
--- b/src/include/libpq/hba.h
***************
*** 22,28 **** typedef enum UserAuth
  	uaTrust,
  	uaIdent,
  	uaPassword,
- 	uaCrypt,
  	uaMD5,
  	uaGSS,
  	uaSSPI,
--- 22,27 ----
*** a/src/include/libpq/libpq-be.h
--- b/src/include/libpq/libpq-be.h
***************
*** 123,129 **** typedef struct Port
  	 */
  	HbaLine	   *hba;
  	char		md5Salt[4];		/* Password salt */
- 	char		cryptSalt[2];	/* Password salt */
  
  	/*
  	 * Information that really has no business at all being in struct Port,
--- 123,128 ----
*** a/src/include/libpq/pqcomm.h
--- b/src/include/libpq/pqcomm.h
***************
*** 153,159 **** extern bool Db_user_namespace;
  #define AUTH_REQ_KRB4		1	/* Kerberos V4. Not supported any more. */
  #define AUTH_REQ_KRB5		2	/* Kerberos V5 */
  #define AUTH_REQ_PASSWORD	3	/* Password */
! #define AUTH_REQ_CRYPT		4	/* crypt password */
  #define AUTH_REQ_MD5		5	/* md5 password */
  #define AUTH_REQ_SCM_CREDS	6	/* transfer SCM credentials */
  #define AUTH_REQ_GSS		7	/* GSSAPI without wrap() */
--- 153,159 ----
  #define AUTH_REQ_KRB4		1	/* Kerberos V4. Not supported any more. */
  #define AUTH_REQ_KRB5		2	/* Kerberos V5 */
  #define AUTH_REQ_PASSWORD	3	/* Password */
! #define AUTH_REQ_CRYPT		4	/* crypt password. Not supported any more. */
  #define AUTH_REQ_MD5		5	/* md5 password */
  #define AUTH_REQ_SCM_CREDS	6	/* transfer SCM credentials */
  #define AUTH_REQ_GSS		7	/* GSSAPI without wrap() */
*** a/src/interfaces/libpq/fe-auth.c
--- b/src/interfaces/libpq/fe-auth.c
***************
*** 40,49 ****
  #include <pwd.h>
  #endif
  
- #ifdef HAVE_CRYPT_H
- #include <crypt.h>
- #endif
- 
  #include "libpq-fe.h"
  #include "fe-auth.h"
  #include "libpq/md5.h"
--- 40,45 ----
***************
*** 787,800 **** pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
  				}
  				break;
  			}
- 		case AUTH_REQ_CRYPT:
- 			{
- 				char		salt[3];
- 
- 				strlcpy(salt, conn->cryptSalt, sizeof(salt));
- 				crypt_pwd = crypt(password, salt);
- 				break;
- 			}
  		case AUTH_REQ_PASSWORD:
  			/* discard const so we can assign it */
  			crypt_pwd = (char *) password;
--- 783,788 ----
***************
*** 938,945 **** pg_fe_sendauth(AuthRequest areq, PGconn *conn)
  #endif
  
  
- 		case AUTH_REQ_MD5:
  		case AUTH_REQ_CRYPT:
  		case AUTH_REQ_PASSWORD:
  			conn->password_needed = true;
  			if (conn->pgpass == NULL || conn->pgpass[0] == '\0')
--- 926,937 ----
  #endif
  
  
  		case AUTH_REQ_CRYPT:
+ 			printfPQExpBuffer(&conn->errorMessage,
+ 				 libpq_gettext("Crypt authentication not supported\n"));
+ 			return STATUS_ERROR;
+ 
+ 		case AUTH_REQ_MD5:
  		case AUTH_REQ_PASSWORD:
  			conn->password_needed = true;
  			if (conn->pgpass == NULL || conn->pgpass[0] == '\0')
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 1674,1688 **** keep_going:						/* We will come back to here until there is
  						return PGRES_POLLING_READING;
  					}
  				}
- 				if (areq == AUTH_REQ_CRYPT)
- 				{
- 					if (pqGetnchar(conn->cryptSalt,
- 								   sizeof(conn->cryptSalt), conn))
- 					{
- 						/* We'll come back when there are more data */
- 						return PGRES_POLLING_READING;
- 					}
- 				}
  #if defined(ENABLE_GSS) || defined(ENABLE_SSPI)
  
  				/*
--- 1674,1679 ----
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
***************
*** 340,346 **** struct pg_conn
  	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 */
  	pgParameterStatus *pstatus; /* ParameterStatus data */
  	int			client_encoding;	/* encoding id */
  	bool		std_strings;	/* standard_conforming_strings */
--- 340,345 ----
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to