Bug 3089 - pam_close_session return value ignored in function sshpam_cleanup , file auth-pam.c
Summary: pam_close_session return value ignored in function sshpam_cleanup , file auth...
Status: NEW
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: PAM support (show other bugs)
Version: 8.1p1
Hardware: All All
: P5 enhancement
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-06 03:54 AEDT by klra67
Modified: 2019-11-06 03:54 AEDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description klra67 2019-11-06 03:54:52 AEDT
(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 .