bin/59774: ftpd(8)/FreeBSD 5: syslog facility may be changed by PAM modules

[Nick Leuta]

>Number:         59774
>Category:       bin
>Synopsis:       ftpd(8)/FreeBSD 5: syslog facility may be changed by PAM modules
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          change-request
>Submitter-Id:   current-users
>Arrival-Date:   Fri Nov 28 15:30:21 PST 2003
>Closed-Date:
>Last-Modified:
>Originator:     Nick Leuta
>Release:        FreeBSD 4.9-RC i386
>Organization:
Lipetsk State Technical University
>Environment:
System: FreeBSD skynick.stu.lipetsk.ru 4.9-RC FreeBSD 4.9-RC #0: Sun Nov 23 19:53:55 MSK 2003 root at skynick.stu.lipetsk.ru:/usr/src/sys/compile/CORSAIR i386
>Description:
PAM module can call closelog()/openlog() for its own needs, for example, to
log a warning or an error message. After that the syslog facility may be
changed from LOG_FTP to, for example, LOG_AUTH, and following messages from
ftpd(8) will be logged into the wrong facility.

The calling of closelog()/openlog() in modules is a common way in Linux-PAM
(and there are some reasons for such way). I don't sure that this situation is
reproducable with modules from FreeBSD's base system (because it contains not
so much modules than the Linux-PAM distribution), but such problem may
occur with third-party modules, for example, from ports collection.
>How-To-Repeat:
>Fix:
diff -urN ftpd.ORI/ftpd.c ftpd/ftpd.c
--- ftpd.ORI/ftpd.c	Sat Nov 15 14:08:26 2003
+++ ftpd/ftpd.c	Wed Nov 26 01:57:12 2003
@@ -180,6 +180,7 @@
 #ifdef USE_PAM
 static int	auth_pam(struct passwd**, const char*);
 pam_handle_t *pamh = NULL;
+static void ftpd_openlog();
 #endif
 
 static struct opie opiedata;
@@ -420,11 +421,15 @@
 #endif
 	(void) freopen(_PATH_DEVNULL, "w", stderr);
 
+#ifdef USE_PAM
+	ftpd_openlog();
+#else /* Original code */
 	/*
 	 * LOG_NDELAY sets up the logging connection immediately,
 	 * necessary for anonymous ftp's that chroot and can't do it later.
 	 */
 	openlog("ftpd", LOG_PID | LOG_NDELAY, LOG_FTP);
+#endif /* USE_PAM */
 
 	if (daemon_mode) {
 		int *ctl_sock, fd, maxfd = -1, nfds, i;
@@ -1162,14 +1167,22 @@
 		       LOGIN_SETMAC);
 #endif
 #ifdef USE_PAM
-	if ((e = pam_setcred(pamh, PAM_DELETE_CRED)) != PAM_SUCCESS)
-		syslog(LOG_ERR, "pam_setcred: %s", pam_strerror(pamh, e));
-	if ((e = pam_close_session(pamh,0)) != PAM_SUCCESS)
-		syslog(LOG_ERR, "pam_close_session: %s", pam_strerror(pamh, e));
-	if ((e = pam_end(pamh, e)) != PAM_SUCCESS)
-		syslog(LOG_ERR, "pam_end: %s", pam_strerror(pamh, e));
-	pamh = NULL;
-#endif
+	if (pamh) {
+		if ((e = pam_setcred(pamh, PAM_DELETE_CRED)) != PAM_SUCCESS)
+			syslog(LOG_ERR, "pam_setcred: %s",
+			    pam_strerror(pamh, e));
+		if ((e = pam_close_session(pamh,0)) != PAM_SUCCESS)
+			syslog(LOG_ERR, "pam_close_session: %s",
+			    pam_strerror(pamh, e));
+		if ((e = pam_end(pamh, e)) != PAM_SUCCESS)
+			syslog(LOG_ERR, "pam_end: %s", pam_strerror(pamh, e));
+		pamh = NULL;
+		/* Reset the logging facility because it may be changed by PAM
+		 * modules */
+		ftpd_openlog();
+	}
+#endif /* USE_PAM */
+
 	logged_in = 0;
 	guest = 0;
 	dochroot = 0;
@@ -1353,6 +1366,10 @@
 		}
 #ifdef USE_PAM
 		rval = auth_pam(&pw, passwd);
+		/* Reset the logging facility because it may be changed by PAM
+		 * modules */
+		ftpd_openlog();
+
 		if (rval >= 0) {
 			opieunlock();
 			goto skip;
@@ -1441,6 +1458,9 @@
 		} else if ((e = pam_setcred(pamh, PAM_ESTABLISH_CRED)) != PAM_SUCCESS) {
 			syslog(LOG_ERR, "pam_setcred: %s", pam_strerror(pamh, e));
 		}
+		/* Reset the logging facility because it may be changed by PAM
+		 * modules */
+		ftpd_openlog();
 	}
 #endif
 
@@ -3212,3 +3232,17 @@
 	}
 	return(socks);
 }
+
+#ifdef USE_PAM
+static void
+ftpd_openlog()
+{
+    closelog();
+
+    /*
+     * LOG_NDELAY sets up the logging connection immediately,
+     * necessary for anonymous ftp's that chroot and can't do it later.
+     */
+    openlog("ftpd", LOG_PID | LOG_NDELAY, LOG_FTP);
+}
+#endif /* USE_PAM */
>Release-Note:
>Audit-Trail:
>Unformatted: