Re: BUG #5121: Segmentation Fault when using pam w/ krb5 - Mailing list pgsql-bugs

From Tom Lane
Subject Re: BUG #5121: Segmentation Fault when using pam w/ krb5
Date
Msg-id 11006.1255724122@sss.pgh.pa.us
Whole thread Raw
In response to BUG #5121: Segmentation Fault when using pam w/ krb5  ("Ryan Douglas" <rdouglas@arbinet.com>)
Responses Re: BUG #5121: Segmentation Fault when using pam w/ krb5
List pgsql-bugs
"Douglas, Ryan" <RDouglas@arbinet.com> writes:
>    You were right. According to the trace msg[0] is null.

Hah.  This must be triggered by something Active Directory does that
a KDC doesn't, because I'm still not seeing it here.  But anyway the
problem is clear now, we have to avoid referencing msg[0] when num_msg
is zero.

Please try the attached patch and see if it behaves sanely for you.
This is based on openssh's PAM callback, so it ought to be more
robust than what we had.  (This is against 8.4 branch tip, but it
should apply to 8.4.1 with maybe a few lines' offset.)

            regards, tom lane

Index: auth.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.183.2.2
diff -c -r1.183.2.2 auth.c
*** auth.c    14 Oct 2009 22:10:01 -0000    1.183.2.2
--- auth.c    16 Oct 2009 20:08:39 -0000
***************
*** 441,447 ****

          case uaPAM:
  #ifdef USE_PAM
-             pam_port_cludge = port;
              status = CheckPAMAuth(port, port->user_name, "");
  #else
              Assert(false);
--- 441,446 ----
***************
*** 1880,1940 ****
  pam_passwd_conv_proc(int num_msg, const struct pam_message ** msg,
                       struct pam_response ** resp, void *appdata_ptr)
  {
!     if (num_msg != 1 || msg[0]->msg_style != PAM_PROMPT_ECHO_OFF)
!     {
!         switch (msg[0]->msg_style)
!         {
!             case PAM_ERROR_MSG:
!                 ereport(LOG,
!                         (errmsg("error from underlying PAM layer: %s",
!                                 msg[0]->msg)));
!                 return PAM_CONV_ERR;
!             default:
!                 ereport(LOG,
!                         (errmsg("unsupported PAM conversation %d/%s",
!                                 msg[0]->msg_style, msg[0]->msg)));
!                 return PAM_CONV_ERR;
!         }
!     }

!     if (!appdata_ptr)
      {
          /*
           * Workaround for Solaris 2.6 where the PAM library is broken and does
           * not pass appdata_ptr to the conversation routine
           */
!         appdata_ptr = pam_passwd;
      }

!     /*
!      * Password wasn't passed to PAM the first time around - let's go ask the
!      * client to send a password, which we then stuff into PAM.
!      */
!     if (strlen(appdata_ptr) == 0)
!     {
!         char       *passwd;
!
!         sendAuthRequest(pam_port_cludge, AUTH_REQ_PASSWORD);
!         passwd = recv_password_packet(pam_port_cludge);

!         if (passwd == NULL)
!             return PAM_CONV_ERR;    /* client didn't want to send password */
!
!         if (strlen(passwd) == 0)
!         {
!             ereport(LOG,
!                     (errmsg("empty password returned by client")));
!             return PAM_CONV_ERR;
!         }
!         appdata_ptr = passwd;
!     }

      /*
       * Explicitly not using palloc here - PAM will free this memory in
       * pam_end()
       */
!     *resp = calloc(num_msg, sizeof(struct pam_response));
!     if (!*resp)
      {
          ereport(LOG,
                  (errcode(ERRCODE_OUT_OF_MEMORY),
--- 1879,1909 ----
  pam_passwd_conv_proc(int num_msg, const struct pam_message ** msg,
                       struct pam_response ** resp, void *appdata_ptr)
  {
!     char       *passwd;
!     struct pam_response *reply;
!     int            i;

!     if (appdata_ptr)
!         passwd = (char *) appdata_ptr;
!     else
      {
          /*
           * Workaround for Solaris 2.6 where the PAM library is broken and does
           * not pass appdata_ptr to the conversation routine
           */
!         passwd = pam_passwd;
      }

!     *resp = NULL;                /* in case of error exit */

!     if (num_msg <= 0 || num_msg > PAM_MAX_NUM_MSG)
!         return PAM_CONV_ERR;

      /*
       * Explicitly not using palloc here - PAM will free this memory in
       * pam_end()
       */
!     if ((reply = calloc(num_msg, sizeof(struct pam_response))) == NULL)
      {
          ereport(LOG,
                  (errcode(ERRCODE_OUT_OF_MEMORY),
***************
*** 1942,1951 ****
          return PAM_CONV_ERR;
      }

!     (*resp)[0].resp = strdup((char *) appdata_ptr);
!     (*resp)[0].resp_retcode = 0;

!     return ((*resp)[0].resp ? PAM_SUCCESS : PAM_CONV_ERR);
  }


--- 1911,1981 ----
          return PAM_CONV_ERR;
      }

!     for (i = 0; i < num_msg; i++)
!     {
!         switch (msg[i]->msg_style)
!         {
!             case PAM_PROMPT_ECHO_OFF:
!                 if (strlen(passwd) == 0)
!                 {
!                     /*
!                      * Password wasn't passed to PAM the first time around -
!                      * let's go ask the client to send a password, which we
!                      * then stuff into PAM.
!                      */
!                     sendAuthRequest(pam_port_cludge, AUTH_REQ_PASSWORD);
!                     passwd = recv_password_packet(pam_port_cludge);
!                     if (passwd == NULL)
!                     {
!                         /*
!                          * Client didn't want to send password.  We
!                          * intentionally do not log anything about this.
!                          */
!                         goto fail;
!                     }
!                     if (strlen(passwd) == 0)
!                     {
!                         ereport(LOG,
!                                 (errmsg("empty password returned by client")));
!                         goto fail;
!                     }
!                 }
!                 if ((reply[i].resp = strdup(passwd)) == NULL)
!                     goto fail;
!                 reply[i].resp_retcode = PAM_SUCCESS;
!                 break;
!             case PAM_ERROR_MSG:
!                 ereport(LOG,
!                         (errmsg("error from underlying PAM layer: %s",
!                                 msg[i]->msg)));
!                 /* FALL THROUGH */
!             case PAM_TEXT_INFO:
!                 /* we don't bother to log TEXT_INFO messages */
!                 if ((reply[i].resp = strdup("")) == NULL)
!                     goto fail;
!                 reply[i].resp_retcode = PAM_SUCCESS;
!                 break;
!             default:
!                 elog(LOG, "unsupported PAM conversation %d/\"%s\"",
!                      msg[i]->msg_style,
!                      msg[i]->msg ? msg[i]->msg : "(none)");
!                 goto fail;
!         }
!     }
!
!     *resp = reply;
!     return PAM_SUCCESS;
!
! fail:
!     /* free up whatever we allocated */
!     for (i = 0; i < num_msg; i++)
!     {
!         if (reply[i].resp != NULL)
!             free(reply[i].resp);
!     }
!     free(reply);

!     return PAM_CONV_ERR;
  }


***************
*** 1959,1968 ****
      pam_handle_t *pamh = NULL;

      /*
!      * Apparently, Solaris 2.6 is broken, and needs ugly static variable
!      * workaround
       */
      pam_passwd = password;

      /*
       * Set the application data portion of the conversation struct This is
--- 1989,2000 ----
      pam_handle_t *pamh = NULL;

      /*
!      * We can't entirely rely on PAM to pass through appdata --- it appears
!      * not to work on at least Solaris 2.6.  So use these ugly static
!      * variables instead.
       */
      pam_passwd = password;
+     pam_port_cludge = port;

      /*
       * Set the application data portion of the conversation struct This is

pgsql-bugs by date:

Previous
From: Robert Haas
Date:
Subject: Re: BUG #5118: start-status-insert-fatal
Next
From: David Fetter
Date:
Subject: Re: BUG #5123: bug in window function "last_value"