Re: PAM patches, iteration 4

From: David J. MacKenzie (djm@web.us.uu.net)
Date: 01/24/01


To: "Jacques A. Vidrine" <n@nectar.com>, "David J. MacKenzie" <djm@web.us.uu.net>, freebsd-security@freebsd.org
Date: Tue, 23 Jan 2001 18:24:15 -0500
From: "David J. MacKenzie" <djm@web.us.uu.net>


> 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



Relevant Pages