openssh bugs - Nov 2019 - [Bug 3089] New: pam_close_session return value ignored in function sshpam_cleanup , file auth-pam.c

https://bugzilla.mindrot.org/show_bug.cgi?id=3089

            Bug ID: 3089
           Summary: pam_close_session return value ignored in function
                    sshpam_cleanup , file auth-pam.c
           Product: Portable OpenSSH
           Version: 8.1p1
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: PAM support
          Assignee: unassigned-bugs at mindrot.org
          Reporter: klra67 at freenet.de

(Found in 8.1, confirmed against git minutes ago)

The function in question ignores the return codes given by
pam_set_item, pam_close_session and pam_setcred.

If any of these returns an error (eg. because pam was configured to
call an external program to log session end which failed), pam_end is
called with the wrong value in sshpam_err which it does not like.
man pam_end states:

  The pam_status argument should be set to the value returned to the 
  application by the last PAM library call. 


So I changed
>>>
void
sshpam_cleanup(void)
{
        if (sshpam_handle == NULL || (use_privsep && !mm_is_monitor()))
                return;
        debug("PAM: cleanup");
        pam_set_item(sshpam_handle, PAM_CONV, (const void
*)&null_conv);
        if (sshpam_session_open) {
                debug("PAM: closing session");
                pam_close_session(sshpam_handle, PAM_SILENT);
                sshpam_session_open = 0;
        }
        if (sshpam_cred_established) {
                debug("PAM: deleting credentials");
                pam_setcred(sshpam_handle, PAM_DELETE_CRED);
                sshpam_cred_established = 0;
        }
        sshpam_authenticated = 0;
        pam_end(sshpam_handle, sshpam_err);
        sshpam_handle = NULL;
}


<<<

to 
>>>
void
sshpam_cleanup(void)
{
        if (sshpam_handle == NULL || (use_privsep && !mm_is_monitor()))
                return;
        debug("PAM: cleanup");

        sshpam_err = pam_set_item(sshpam_handle, PAM_CONV, (const void
*)&null_conv);
        if (sshpam_err != PAM_SUCCESS)
                error("PAM: failed to set PAM_CONV: %s",
                     pam_strerror(sshpam_handle, sshpam_err)
                        ) ;


        if (sshpam_session_open) {
                debug("PAM: closing session");
                sshpam_err = pam_close_session(sshpam_handle,
PAM_SILENT);
                if (sshpam_err != PAM_SUCCESS)
                        error("PAM: failed to set PAM_CONV: %s",
                                               
pam_strerror(sshpam_handle, sshpam_err)
                                ) ;
                sshpam_session_open = 0;
        }
        if (sshpam_cred_established) {
                debug("PAM: deleting credentials");
                sshpam_err = pam_setcred(sshpam_handle,
PAM_DELETE_CRED);
                if (sshpam_err != PAM_SUCCESS)
                        error("PAM: failed to delete credentials: %s",
                                               
pam_strerror(sshpam_handle, sshpam_err)
                                ) ;
                sshpam_cred_established = 0;
        }
        sshpam_authenticated = 0;
        sshpam_err = pam_end(sshpam_handle, sshpam_err);
        if (sshpam_err != PAM_SUCCESS)
                error("PAM: error ending session: %s",
                                        pam_strerror(sshpam_handle,
sshpam_err)
                        ) ;
        sshpam_handle = NULL;
}
<<<

I call error, not fatal, to make sure it gets a chance to delete
credentials.
I am not absolutely sure about the last error message, but pam_end can
return PAM_SYSTEM_ERR .

-- 
You are receiving this mail because:
You are watching the assignee of the bug.