Package: linux-ftpd
Version: 0.17-34
Severity: normal
Performing a round of code audit of the PAM code
found in the present source, I believe to have
located inaccurate and missing calls.
I am not in a possession of a refined enough use case
to verify that my suggestion is correctly reacting to
accurate accounting setups, but the new code works
as expected with respect to standard setups.
The suggested ordering between pam_setcred() and
pam_{open/close}_session() and WTMP actions is what
the standard manuals pledge for. The present code
does not follow these advices, so let us evaluate
my suggestions and then decide on appropriate measures.
Best regards,
Mats Erik Andersson, co-maintainer of linux-ftpd.
Description: PAM code corrections.
Some issues on ordering of the actions
related to PAM are corrected.
.
Credentials are established before opening
the new session, and the session is conversely
closed before credentials are revoked. This
assures proper resource accounting.
.
Similarly, all WTMP and Kerberos sign off
should be undertaken before the session
is closed, of accounting reasons again.
.
The seldom event that memory allocation fails,
must return the correct status PAM_BUF_ERR.
Author: Mats Erik Andersson <[email protected]>
Forwarded: no
Last-Update: 2012-06-08
diff -Naurp linux-ftpd-0.17.debian/ftpd/ftpd.c linux-ftpd-0.17/ftpd/ftpd.c
--- linux-ftpd-0.17.debian/ftpd/ftpd.c
+++ linux-ftpd-0.17/ftpd/ftpd.c
@@ -930,9 +930,6 @@ static void end_login(void)
if (logged_in) {
#ifdef USE_PAM
int error;
- error = pam_close_session(pamh, 0);
- pam_end(pamh, error);
- pamh = 0;
#endif
ftpdlogwtmp(ttyline, "", "", NULL);
if (doutmp)
@@ -941,6 +938,12 @@ static void end_login(void)
if (!notickets && krbtkfile_env)
unlink(krbtkfile_env);
#endif
+#ifdef USE_PAM
+ error = pam_close_session(pamh, 0);
+ (void) pam_setcred(pamh, PAM_DELETE_CRED);
+ pam_end(pamh, error);
+ pamh = 0;
+#endif
}
pw = NULL;
logged_in = 0;
@@ -961,7 +964,7 @@ static int PAM_conv (int num_msg, const
#define GET_MEM \
if (!(repl = realloc(repl, size))) \
- return PAM_CONV_ERR; \
+ return PAM_BUF_ERR; \
size += sizeof(struct pam_response)
#define COPY_STRING(s) (s) ? strdup(s) : NULL
@@ -1055,10 +1058,10 @@ static int pam_doit(void)
/* Alright, we got it */
error = pam_acct_mgmt(pamh, 0);
if (error == PAM_SUCCESS)
- error = pam_open_session(pamh, 0);
- if (error == PAM_SUCCESS)
error = pam_setcred(pamh, PAM_ESTABLISH_CRED);
if (error == PAM_SUCCESS)
+ error = pam_open_session(pamh, 0);
+ if (error == PAM_SUCCESS)
error = pam_get_item(pamh, PAM_USER, &vp);
if (error == PAM_SUCCESS) {
const char *user = vp;
