Thread: crypt auth

crypt auth

From
Magnus Hagander
Date:
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
especiallyfor
 


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.

//Magnus


Re: crypt auth

From
Peter Eisentraut
Date:
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.

AFAICT, removing an authentication method requires a protocol version 
bump.  If you think it is worth dealing with those complications, then 
go for it.  I think it might be worth it.


Re: crypt auth

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> AFAICT, removing an authentication method requires a protocol version 
> bump.

Why would it require that?  There would just be some auth method codes
that remain reserved but aren't used anymore.
        regards, tom lane


Re: crypt auth

From
Peter Eisentraut
Date:
Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> AFAICT, removing an authentication method requires a protocol version 
>> bump.
> 
> Why would it require that?  There would just be some auth method codes
> that remain reserved but aren't used anymore.

Yeah, I was mistaken.  AuthenticationCryptPassword would remain in the 
protocol definition, but the server would just never send it.


Re: crypt auth

From
Magnus Hagander
Date:
Peter Eisentraut wrote:
> Tom Lane wrote:
>> Peter Eisentraut <peter_e@gmx.net> writes:
>>> AFAICT, removing an authentication method requires a protocol version
>>> bump.
>>
>> Why would it require that?  There would just be some auth method codes
>> that remain reserved but aren't used anymore.
> 
> Yeah, I was mistaken.  AuthenticationCryptPassword would remain in the
> protocol definition, but the server would just never send it.

Since I've seen no actual objections to this happening, I will go ahead
and do it unless someone objects explicitly :-)

//Magnus


Re: crypt auth

From
Magnus Hagander
Date:
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 ----