Re: Re: Proposal for encrypting pg_shadow passwords - Mailing list pgsql-patches

> > 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.

OK, here is an updated version of the dual salt patch.  It works and I
will apply it now.

I have to say I like working on this in an iterative way so I can get
feedback from people as I go.  This area is kind of complex and it is
good people are giving me feedback.

Tom mentioned 8-byte MD5 salt perhaps someday.  I am wondering if we
should improve the protocol negotiation code so that if the client says
they are 2.1, the server can send back 2.0 and if the client responds
with 2.0, the protocol can continue at the 2.0 level.  I think this
should be done soon because even if we don't need it now, we will in a
release or two.  How hard would this be?

I added this nice message:

+   /* If they encrypt their password, force MD5 */
+   if (isMD5(passwd) && port->auth_method != uaMD5)
+   {
+       snprintf(PQerrormsg, PQERRORMSG_LENGTH,
+           "Password is stored MD5 encrypted.  "
+           "Only pg_hba.conf's MD5 protocol can be used for this user.\n");
+       fputs(PQerrormsg, stderr);
+       pqdebug("%s", PQerrormsg);
+       return STATUS_ERROR;

Once you MD5 encrypt your password, you can't use crypt or plaintext
passwords.  Seems if you want it MD5 encrypted, you don't want it sent
over the wire plaintext.  I hope to add MD5 to ODBC for 7.2 too, unless
someone beats me to it.

Please see my other concern about non-null terminated salt() sent to
crypt!

--
  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/17 02:50:47
***************
*** 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/17 02:50:47
***************
*** 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/17 02:50:48
***************
*** 19,24 ****
--- 19,25 ----

  #include "postgres.h"
  #include "libpq/crypt.h"
+ #include "libpq/libpq.h"
  #include "miscadmin.h"
  #include "storage/fd.h"
  #include "utils/nabstime.h"
***************
*** 276,297 ****
          return STATUS_ERROR;
      }

      /*
       * Compare with the encrypted or plain password depending on the
       * authentication method being used for this connection.
       */
!      switch (port->auth_method)
!      {
          case uaCrypt:
!             crypt_pwd = crypt(passwd, port->salt);
              break;
          case uaMD5:
              crypt_pwd = palloc(MD5_PASSWD_LEN+1);
-
              if (isMD5(passwd))
              {
                  if (!EncryptMD5(passwd + strlen("md5"),
!                                 (char *)port->salt, crypt_pwd))
                  {
                      pfree(crypt_pwd);
                      return STATUS_ERROR;
--- 277,309 ----
          return STATUS_ERROR;
      }

+     /* If they encrypt their password, force MD5 */
+     if (isMD5(passwd) && port->auth_method != uaMD5)
+     {
+         snprintf(PQerrormsg, PQERRORMSG_LENGTH,
+              "Password is stored MD5 encrypted.  "
+             "Only pg_hba.conf's MD5 protocol can be used for this user.\n");
+         fputs(PQerrormsg, stderr);
+         pqdebug("%s", PQerrormsg);
+         return STATUS_ERROR;
+     }
+
      /*
       * Compare with the encrypted or plain password depending on the
       * authentication method being used for this connection.
       */
!     switch (port->auth_method)
!     {
          case uaCrypt:
!             crypt_pwd = crypt(passwd, port->cryptSalt);
              break;
          case uaMD5:
              crypt_pwd = palloc(MD5_PASSWD_LEN+1);
              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);
--- 313,327 ----
              {
                  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);
***************
*** 324,330 ****

      if (!strcmp(pgpass, crypt_pwd))
      {
-
          /*
           * check here to be sure we are not past valuntil
           */
--- 337,342 ----
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/17 02:50:51
***************
*** 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/17 02:50:52
***************
*** 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,2117 ----
   * RandomSalt
   */
  static void
! RandomSalt(char *cryptSalt, char *md5Salt)
  {
      long        rand = PostmasterRandom();

!     cryptSalt[0] = CharRemap(rand % 62);
!     cryptSalt[1] = CharRemap(rand / 62);
!     /* Grab top 16-bits of two random runs so as not to send full
!        random value over the network.  The high-order bits are more random. */
!     md5Salt[0] = rand & 0xff000000;
!     md5Salt[1] = rand & 0x00ff0000;
!     rand = PostmasterRandom();
!     md5Salt[2] = rand & 0xff000000;
!     md5Salt[3] = rand & 0x00ff0000;
  }

  /*
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/17 02:50:57
***************
*** 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/17 02:50:57
***************
*** 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/17 02:50:58
***************
*** 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,
!                                 sizeof(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/17 02:51:02
***************
*** 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), 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), 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/17 02:51:02
***************
*** 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/17 02:51:03
***************
*** 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);