From owner-freebsd-security Tue Jan 23 15:24:39 2001 Delivered-To: freebsd-security@freebsd.org Received: from catapult.web.us.uu.net (catapult.web.us.uu.net [208.211.134.20]) by hub.freebsd.org (Postfix) with ESMTP id 0EA8F37B400 for ; Tue, 23 Jan 2001 15:24:16 -0800 (PST) Received: from catapult.web.us.uu.net (localhost.web.us.uu.net [127.0.0.1]) by catapult.web.us.uu.net (Postfix) with ESMTP id 6AC8C3E5B; Tue, 23 Jan 2001 18:24:15 -0500 (EST) To: "Jacques A. Vidrine" , "David J. MacKenzie" , freebsd-security@freebsd.org Subject: Re: PAM patches, iteration 4 In-Reply-To: Message from "Jacques A. Vidrine" of "Tue, 23 Jan 2001 16:13:18 CST." <20010123161318.A95429@hamlet.nectar.com> Date: Tue, 23 Jan 2001 18:24:15 -0500 From: "David J. MacKenzie" Message-Id: <20010123232415.6AC8C3E5B@catapult.web.us.uu.net> Sender: owner-freebsd-security@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org > 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