Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 23 Jan 2001 18:24:15 -0500
From:      "David J. MacKenzie" <djm@web.us.uu.net>
To:        "Jacques A. Vidrine" <n@nectar.com>, "David J. MacKenzie" <djm@web.us.uu.net>, freebsd-security@freebsd.org
Subject:   Re: PAM patches, iteration 4 
Message-ID:  <20010123232415.6AC8C3E5B@catapult.web.us.uu.net>
In-Reply-To: Message from "Jacques A. Vidrine" <n@nectar.com>  of "Tue, 23 Jan 2001 16:13:18 CST." <20010123161318.A95429@hamlet.nectar.com> 

next in thread | previous in thread | raw e-mail | index | archive | help

> These patches look like good to me.   The pam_setcred workaround is no
> worse than what we have now [1], and it is useful.
> 
> [1] All the PAM modules in the base system just return PAM_SUCCESS, so
>     this will have no effect unless a third-party module is installed,
>     such as ports/security/krb5.

The pam_krb5 port does the right thing, I believe.

One unresolved question I have is, when should pam_end() be called.
Probably in the same places that login_close() and endpwent() are.
I think I called it in places where it's unnecessary and omitted
it in a place where it's important.  Here's a patch to my last patch set
to make pam_end() calls more consistent.

It's still not perfect yet, though; anytime after
pam_setcred(PAM_ESTABLISH_CRED) has been called, before exiting,
pam_setcred(PAM_DELETE_CRED) should be called to clean up.  Perhaps
the same for pam_open_session() and pam_close_session().  I haven't
done that so far.

--- su.c	2001/01/23 18:10:00	1.16
+++ su.c	2001/01/23 23:15:02
@@ -235,7 +235,6 @@
 	retcode = pam_set_item(pamh, PAM_RHOST, myhost);
 	if (retcode != PAM_SUCCESS) {
 		syslog(LOG_ERR, "pam_set_item(PAM_RHOST): %s", pam_strerror(pamh, retcode));
-		pam_end(pamh, retcode);
 		errx(1, "pam_set_item(PAM_RHOST): %s", pam_strerror(pamh, retcode));
 	}
 
@@ -245,7 +244,6 @@
 	retcode = pam_set_item(pamh, PAM_TTY, mytty);
 	if (retcode != PAM_SUCCESS) {
 		syslog(LOG_ERR, "pam_set_item(PAM_TTY): %s", pam_strerror(pamh, retcode));
-		pam_end(pamh, retcode);
 		errx(1, "pam_set_item(PAM_TTY): %s", pam_strerror(pamh, retcode));
 	}
 
@@ -253,7 +251,6 @@
 		retcode = pam_authenticate(pamh, 0);
 		if (retcode != PAM_SUCCESS) {
 			syslog(LOG_ERR, "pam_authenticate: %s", pam_strerror(pamh, retcode));
-			pam_end(pamh, retcode);
 			errx(1, "Sorry");
 		}
 
@@ -268,13 +265,11 @@
 			retcode = pam_chauthtok(pamh, PAM_CHANGE_EXPIRED_AUTHTOK);
 			if (retcode != PAM_SUCCESS) {
 				syslog(LOG_ERR, "pam_chauthtok: %s", pam_strerror(pamh, retcode));
-				pam_end(pamh, retcode);
 				errx(1, "Sorry");
 			}
 		}
 		if (retcode != PAM_SUCCESS) {
 			syslog(LOG_ERR, "pam_acct_mgmt: %s", pam_strerror(pamh, retcode));
-			pam_end(pamh, retcode);
 			errx(1, "Sorry");
 		}
 	}
@@ -485,6 +480,10 @@
 	if (env != NULL) {
 		for (i=0; env[i]; i++)
 			putenv(env[i]);
+	}
+	retcode = pam_end(pamh, PAM_DATA_SILENT);
+	if (retcode != PAM_SUCCESS) {
+		syslog(LOG_ERR, "pam_end: %s", pam_strerror(pamh, retcode));
 	}
 #endif /* USE_PAM */
 
--- login.c	2001/01/23 00:57:28	1.8
+++ login.c	2001/01/23 23:10:47
@@ -593,7 +593,7 @@
 			PAM_END;
 			exit(0);
 		} else {
-			if ((e = pam_end(pamh, e)) != PAM_SUCCESS)
+			if ((e = pam_end(pamh, PAM_DATA_SILENT)) != PAM_SUCCESS)
 				syslog(LOG_ERR, "pam_end: %s", pam_strerror(pamh, e));
 		}
 	}
@@ -738,14 +738,12 @@
 	if ((e = pam_set_item(pamh, PAM_TTY, tty)) != PAM_SUCCESS) {
 		syslog(LOG_ERR, "pam_set_item(PAM_TTY): %s",
 		    pam_strerror(pamh, e));
-		pam_end(pamh, e);
 		return -1;
 	}
 	if (hostname != NULL &&
 	    (e = pam_set_item(pamh, PAM_RHOST, full_hostname)) != PAM_SUCCESS) {
 		syslog(LOG_ERR, "pam_set_item(PAM_RHOST): %s",
 		    pam_strerror(pamh, e));
-		pam_end(pamh, e);
 		return -1;
 	}
 	e = pam_authenticate(pamh, 0);
--- rshd.c	2001/01/23 18:09:49	1.14
+++ rshd.c	2001/01/23 23:14:05
@@ -382,21 +382,18 @@
 	retcode = pam_set_item (pamh, PAM_RUSER, remuser);
 	if (retcode != PAM_SUCCESS) {
 		syslog(LOG_ERR|LOG_AUTH, "pam_set_item(PAM_RUSER): %s", pam_strerror(pamh, retcode));
-		pam_end(pamh, retcode);
 		error("Login incorrect.\n");
 		exit(1);
 	}
 	retcode = pam_set_item (pamh, PAM_RHOST, fromhost);
 	if (retcode != PAM_SUCCESS) {
 		syslog(LOG_ERR|LOG_AUTH, "pam_set_item(PAM_RHOST): %s", pam_strerror(pamh, retcode));
-		pam_end(pamh, retcode);
 		error("Login incorrect.\n");
 		exit(1);
 	}
 	retcode = pam_set_item (pamh, PAM_TTY, "tty");
 	if (retcode != PAM_SUCCESS) {
 		syslog(LOG_ERR|LOG_AUTH, "pam_set_item(PAM_TTY): %s", pam_strerror(pamh, retcode));
-		pam_end(pamh, retcode);
 		error("Login incorrect.\n");
 		exit(1);
 	}
@@ -414,7 +411,6 @@
 	if (retcode != PAM_SUCCESS) {
 		syslog(LOG_INFO|LOG_AUTH, "%s@%s as %s: permission denied (%s). cmd='%.80s'",
 		       remuser, fromhost, locuser, pam_strerror(pamh, retcode), cmdbuf);
-		pam_end(pamh, retcode);
 		error("Login incorrect.\n");
 		exit(1);
 	}
@@ -535,9 +531,6 @@
 		pid = fork();
 		if (pid == -1)  {
 			error("Can't fork; try again.\n");
-#ifdef USE_PAM
-			PAM_END;
-#endif /* USE_PAM */
 			exit(1);
 		}
 		if (pid) {
@@ -681,7 +674,6 @@
 		pid = fork();
 		if (pid == -1)  {
 			error("Can't fork; try again.\n");
-			PAM_END;
 			exit(1);
 		}
 		if (pid) {
@@ -717,6 +709,8 @@
 		for (i=0; env[i]; i++)
 			putenv(env[i]);
 	}
+	if ((retcode = pam_end(pamh, PAM_DATA_SILENT)) != PAM_SUCCESS)
+		syslog(LOG_ERR|LOG_AUTH, "pam_end: %s", pam_strerror(pamh, retcode));
 #endif /* USE_PAM */
 
 	endpwent();


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-security" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010123232415.6AC8C3E5B>