Thread: Re: Re: Proposal for encrypting pg_shadow passwords

Re: Re: Proposal for encrypting pg_shadow passwords

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> Bruce Momjian <pgman@candle.pha.pa.us> 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 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.

            regards, tom lane

Re: Re: Proposal for encrypting pg_shadow passwords

From
Bruce Momjian
Date:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> Bruce Momjian <pgman@candle.pha.pa.us> 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);