[patch] pam_exec: use program exit code instead of PAM_SYSTEM_ERR

Jean-Sébastien Pédron dumbbell at FreeBSD.org
Fri Jan 27 17:34:25 UTC 2012


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 26.01.2012 11:07, Gleb Kurtsou wrote:
> On (24/01/2012 15:25), Jean-Sébastien Pédron wrote:
>> Attached is a patch that changes the behaviour to always return
>> the program exit code as-is.
> 
> Please consider making it optional.  It will break for generic 
> applications because pam_sm_chauthtok error codes are documented
> and standardized.

You're right, thanks. I attached a new patch with the following changes:

    o  Instead of making it optionnal, I preferred to check the exit
       code for each pam_sm_* functions supported. Here are the rules:
           - If the code is allowed, it's returned as is.
           - If the exit code is 1 (not an allowed PAM code here),
             PAM_PERM_DENIED is returned. I added this because 1 is a
             common exit code for errors.
           - For any other codes, a message is logged about this
             invalid value and PAM_SERVICE_ERR is returned.

    o  I changed return code from PAM_SYSTEM_ERR to PAM_SERVICE_ERR for
       the WIFSIGNALED(status) and !WIFEXITED(status) cases.
       PAM_SYSTEM_ERR is still returned if a syscall fails.

    o  A new environment variable is set before calling the program:
       PAM_SM_FUNC. It contains the name of the pam_sm_* function. The
       program can use it to determine which exit codes are allowed.

    o  The pam_sm_* function name is also added to messages logged from
       _pam_exec().

    o  waitpid() is now called in a loop. If it returned because of
       EINTR, do it again. Before, it would return PAM_SYSTEM_ERR
       without waiting for the child to exit.

I expanded the man page with these informations.

Again, thanks for your comments! If you have more, they're welcome :)

- -- 
Jean-Sébastien Pédron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (FreeBSD)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk8i4BsACgkQa+xGJsFYOlN/EQCg0D3uHsJC2y9jO/Sk9LHTg/xf
POcAnjUUjLwWd035bHqg4o4Ry/htfEkJ
=l4+1
-----END PGP SIGNATURE-----
-------------- next part --------------
diff --git a/lib/libpam/modules/pam_exec/pam_exec.8 b/lib/libpam/modules/pam_exec/pam_exec.8
index 311d64c..9297b1e0 100644
--- a/lib/libpam/modules/pam_exec/pam_exec.8
+++ b/lib/libpam/modules/pam_exec/pam_exec.8
@@ -32,7 +32,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd February 1, 2005
+.Dd January 27, 2012
 .Dt PAM_EXEC 8
 .Os
 .Sh NAME
@@ -56,13 +56,51 @@ variables:
 .Ev PAM_RHOST ,
 .Ev PAM_RUSER ,
 .Ev PAM_SERVICE ,
-.Ev PAM_TTY ,
+.Ev PAM_SM_FUNC ,
+.Ev PAM_TTY
 and
 .Ev PAM_USER .
+.Pp
+The
+.Ev PAM_SM_FUNC
+variable contains the name of the PAM service module function being
+called. It may be:
+.Bl -bullet -offset indent -compact
+.It
+pam_sm_acct_mgmt
+.It
+pam_sm_authenticate
+.It
+pam_sm_chauthtok
+.It
+pam_sm_close_session
+.It
+pam_sm_open_session
+.It
+pam_sm_setcred
+.El
+.Pp
+The program exit code should be
+.Er PAM_SUCCESS
+or one of the error codes allowed by the calling
+.Ev PAM_SM_FUNC
+function.
+The valid codes are documented in each function man page and their
+numerical value is defined in
+.Pa /usr/include/security/pam_constants.h
+under section "XSSO 5.2". Authentication is successful if the return code is
+.Er PAM_SUCCESS
+(0), failed otherwise.
 .Sh SEE ALSO
 .Xr pam_get_item 3 ,
 .Xr pam.conf 5 ,
-.Xr pam 8
+.Xr pam 8 ,
+.Xr pam_sm_acct_mgmt 8 ,
+.Xr pam_sm_authenticate 8 ,
+.Xr pam_sm_chauthtok 8,
+.Xr pam_sm_close_session 8 ,
+.Xr pam_sm_open_session 8 ,
+.Xr pam_sm_setcred 8 .
 .Sh AUTHORS
 The
 .Nm
diff --git a/lib/libpam/modules/pam_exec/pam_exec.c b/lib/libpam/modules/pam_exec/pam_exec.c
index b7a870f..786319e 100644
--- a/lib/libpam/modules/pam_exec/pam_exec.c
+++ b/lib/libpam/modules/pam_exec/pam_exec.c
@@ -62,10 +62,10 @@ static struct {
 
 static int
 _pam_exec(pam_handle_t *pamh __unused, int flags __unused,
-    int argc, const char *argv[])
+    const char *func, int argc, const char *argv[])
 {
 	int envlen, i, nitems, pam_err, status;
-	char *env, **envlist, **tmp;
+	char *env, **envlist, **tmp, *envstr;
 	volatile int childerr;
 	pid_t pid;
 
@@ -79,13 +79,14 @@ _pam_exec(pam_handle_t *pamh __unused, int flags __unused,
 
 	/*
 	 * Set up the child's environment list.  It consists of the PAM
-	 * environment, plus a few hand-picked PAM items.
+	 * environment, plus a few hand-picked PAM items and the pam_sm_
+	 * function name calling it.
 	 */
 	envlist = pam_getenvlist(pamh);
 	for (envlen = 0; envlist[envlen] != NULL; ++envlen)
 		/* nothing */ ;
 	nitems = sizeof(env_items) / sizeof(*env_items);
-	tmp = realloc(envlist, (envlen + nitems + 1) * sizeof(*envlist));
+	tmp = realloc(envlist, (envlen + nitems + 1 + 1) * sizeof(*envlist));
 	if (tmp == NULL) {
 		openpam_free_envlist(envlist);
 		return (PAM_BUF_ERR);
@@ -93,7 +94,6 @@ _pam_exec(pam_handle_t *pamh __unused, int flags __unused,
 	envlist = tmp;
 	for (i = 0; i < nitems; ++i) {
 		const void *item;
-		char *envstr;
 
 		pam_err = pam_get_item(pamh, env_items[i].item, &item);
 		if (pam_err != PAM_SUCCESS || item == NULL)
@@ -107,6 +107,15 @@ _pam_exec(pam_handle_t *pamh __unused, int flags __unused,
 		envlist[envlen] = NULL;
 	}
 
+	/* Add the pam_sm_ function name to the environment. */
+	asprintf(&envstr, "%s=%s", "PAM_SM_FUNC", func);
+	if (envstr == NULL) {
+		openpam_free_envlist(envlist);
+		return (PAM_BUF_ERR);
+	}
+	envlist[envlen++] = envstr;
+	envlist[envlen] = NULL;
+
 	/*
 	 * Fork and run the command.  By using vfork() instead of fork(),
 	 * we can distinguish between an execve() failure and a non-zero
@@ -120,81 +129,259 @@ _pam_exec(pam_handle_t *pamh __unused, int flags __unused,
 	}
 	openpam_free_envlist(envlist);
 	if (pid == -1) {
-		openpam_log(PAM_LOG_ERROR, "vfork(): %m");
+		openpam_log(PAM_LOG_ERROR, "%s: vfork(): %m", func);
 		return (PAM_SYSTEM_ERR);
 	}
-	if (waitpid(pid, &status, 0) == -1) {
-		openpam_log(PAM_LOG_ERROR, "waitpid(): %m");
-		return (PAM_SYSTEM_ERR);
+	while (1) {
+		if (waitpid(pid, &status, 0) == -1) {
+			if (errno == EINTR)
+				continue;
+			openpam_log(PAM_LOG_ERROR, "%s: waitpid(): %m", func);
+			return (PAM_SYSTEM_ERR);
+		}
+
+		break;
 	}
 	if (childerr != 0) {
-		openpam_log(PAM_LOG_ERROR, "execve(): %m");
+		openpam_log(PAM_LOG_ERROR, "%s: execve(): %m", func);
 		return (PAM_SYSTEM_ERR);
 	}
 	if (WIFSIGNALED(status)) {
-		openpam_log(PAM_LOG_ERROR, "%s caught signal %d%s",
-		    argv[0], WTERMSIG(status),
+		openpam_log(PAM_LOG_ERROR, "%s: %s caught signal %d%s",
+		    func, argv[0], WTERMSIG(status),
 		    WCOREDUMP(status) ? " (core dumped)" : "");
-		return (PAM_SYSTEM_ERR);
+		return (PAM_SERVICE_ERR);
 	}
 	if (!WIFEXITED(status)) {
-		openpam_log(PAM_LOG_ERROR, "unknown status 0x%x", status);
-		return (PAM_SYSTEM_ERR);
-	}
-	if (WEXITSTATUS(status) != 0) {
-		openpam_log(PAM_LOG_ERROR, "%s returned code %d",
-		    argv[0], WEXITSTATUS(status));
-		return (PAM_SYSTEM_ERR);
+		openpam_log(PAM_LOG_ERROR, "%s: unknown status 0x%x",
+		    func, status);
+		return (PAM_SERVICE_ERR);
 	}
-	return (PAM_SUCCESS);
+	return (WEXITSTATUS(status));
 }
 
 PAM_EXTERN int
 pam_sm_authenticate(pam_handle_t *pamh, int flags,
     int argc, const char *argv[])
 {
+	int ret;
 
-	return (_pam_exec(pamh, flags, argc, argv));
+	ret = _pam_exec(pamh, flags, __func__, argc, argv);
+
+	/*
+	 * We must check that the program returned a valid code for this
+	 * function.
+	 */
+	switch (ret) {
+	case PAM_SUCCESS:
+	case PAM_ABORT:
+	case PAM_AUTHINFO_UNAVAIL:
+	case PAM_AUTH_ERR:
+	case PAM_BUF_ERR:
+	case PAM_CONV_ERR:
+	case PAM_CRED_INSUFFICIENT:
+	case PAM_IGNORE:
+	case PAM_MAXTRIES:
+	case PAM_PERM_DENIED:
+	case PAM_SERVICE_ERR:
+	case PAM_SYSTEM_ERR:
+	case PAM_USER_UNKNOWN:
+		break;
+	case 1:
+		ret = PAM_PERM_DENIED;
+		break;
+	default:
+		openpam_log(PAM_LOG_ERROR, "%s returned invalid code %d",
+		    argv[0], ret);
+		ret = PAM_SERVICE_ERR;
+	}
+
+	return (ret);
 }
 
 PAM_EXTERN int
 pam_sm_setcred(pam_handle_t *pamh, int flags,
     int argc, const char *argv[])
 {
+	int ret;
+
+	ret = _pam_exec(pamh, flags, __func__, argc, argv);
+
+	/*
+	 * We must check that the program returned a valid code for this
+	 * function.
+	 */
+	switch (ret) {
+	case PAM_SUCCESS:
+	case PAM_ABORT:
+	case PAM_BUF_ERR:
+	case PAM_CONV_ERR:
+	case PAM_CRED_ERR:
+	case PAM_CRED_EXPIRED:
+	case PAM_CRED_UNAVAIL:
+	case PAM_IGNORE:
+	case PAM_PERM_DENIED:
+	case PAM_SERVICE_ERR:
+	case PAM_SYSTEM_ERR:
+	case PAM_USER_UNKNOWN:
+		break;
+	case 1:
+		ret = PAM_PERM_DENIED;
+		break;
+	default:
+		openpam_log(PAM_LOG_ERROR, "%s returned invalid code %d",
+		    argv[0], ret);
+		ret = PAM_SERVICE_ERR;
+	}
 
-	return (_pam_exec(pamh, flags, argc, argv));
+	return (ret);
 }
 
 PAM_EXTERN int
 pam_sm_acct_mgmt(pam_handle_t *pamh, int flags,
     int argc, const char *argv[])
 {
+	int ret;
+
+	ret = _pam_exec(pamh, flags, __func__, argc, argv);
 
-	return (_pam_exec(pamh, flags, argc, argv));
+	/*
+	 * We must check that the program returned a valid code for this
+	 * function.
+	 */
+	switch (ret) {
+	case PAM_SUCCESS:
+	case PAM_ABORT:
+	case PAM_ACCT_EXPIRED:
+	case PAM_AUTH_ERR:
+	case PAM_BUF_ERR:
+	case PAM_CONV_ERR:
+	case PAM_IGNORE:
+	case PAM_NEW_AUTHTOK_REQD:
+	case PAM_PERM_DENIED:
+	case PAM_SERVICE_ERR:
+	case PAM_SYSTEM_ERR:
+	case PAM_USER_UNKNOWN:
+		break;
+	case 1:
+		ret = PAM_PERM_DENIED;
+		break;
+	default:
+		openpam_log(PAM_LOG_ERROR, "%s returned invalid code %d",
+		    argv[0], ret);
+		ret = PAM_SERVICE_ERR;
+	}
+
+	return (ret);
 }
 
 PAM_EXTERN int
 pam_sm_open_session(pam_handle_t *pamh, int flags,
     int argc, const char *argv[])
 {
+	int ret;
+
+	ret = _pam_exec(pamh, flags, __func__, argc, argv);
+
+	/*
+	 * We must check that the program returned a valid code for this
+	 * function.
+	 */
+	switch (ret) {
+	case PAM_SUCCESS:
+	case PAM_ABORT:
+	case PAM_BUF_ERR:
+	case PAM_CONV_ERR:
+	case PAM_IGNORE:
+	case PAM_PERM_DENIED:
+	case PAM_SERVICE_ERR:
+	case PAM_SESSION_ERR:
+	case PAM_SYSTEM_ERR:
+		break;
+	case 1:
+		ret = PAM_PERM_DENIED;
+		break;
+	default:
+		openpam_log(PAM_LOG_ERROR, "%s returned invalid code %d",
+		    argv[0], ret);
+		ret = PAM_SERVICE_ERR;
+	}
 
-	return (_pam_exec(pamh, flags, argc, argv));
+	return (ret);
 }
 
 PAM_EXTERN int
 pam_sm_close_session(pam_handle_t *pamh, int flags,
     int argc, const char *argv[])
 {
+	int ret;
+
+	ret = _pam_exec(pamh, flags, __func__, argc, argv);
+
+	/*
+	 * We must check that the program returned a valid code for this
+	 * function.
+	 */
+	switch (ret) {
+	case PAM_SUCCESS:
+	case PAM_ABORT:
+	case PAM_BUF_ERR:
+	case PAM_CONV_ERR:
+	case PAM_IGNORE:
+	case PAM_PERM_DENIED:
+	case PAM_SERVICE_ERR:
+	case PAM_SESSION_ERR:
+	case PAM_SYSTEM_ERR:
+		break;
+	case 1:
+		ret = PAM_PERM_DENIED;
+		break;
+	default:
+		openpam_log(PAM_LOG_ERROR, "%s returned invalid code %d",
+		    argv[0], ret);
+		ret = PAM_SERVICE_ERR;
+	}
 
-	return (_pam_exec(pamh, flags, argc, argv));
+	return (ret);
 }
 
 PAM_EXTERN int
 pam_sm_chauthtok(pam_handle_t *pamh, int flags,
     int argc, const char *argv[])
 {
+	int ret;
+
+	ret = _pam_exec(pamh, flags, __func__, argc, argv);
+
+	/*
+	 * We must check that the program returned a valid code for this
+	 * function.
+	 */
+	switch (ret) {
+	case PAM_SUCCESS:
+	case PAM_ABORT:
+	case PAM_AUTHTOK_DISABLE_AGING:
+	case PAM_AUTHTOK_ERR:
+	case PAM_AUTHTOK_LOCK_BUSY:
+	case PAM_AUTHTOK_RECOVERY_ERR:
+	case PAM_BUF_ERR:
+	case PAM_CONV_ERR:
+	case PAM_IGNORE:
+	case PAM_PERM_DENIED:
+	case PAM_SERVICE_ERR:
+	case PAM_SYSTEM_ERR:
+	case PAM_TRY_AGAIN:
+		break;
+	case 1:
+		ret = PAM_PERM_DENIED;
+		break;
+	default:
+		openpam_log(PAM_LOG_ERROR, "%s returned invalid code %d",
+		    argv[0], ret);
+		ret = PAM_SERVICE_ERR;
+	}
 
-	return (_pam_exec(pamh, flags, argc, argv));
+	return (ret);
 }
 
 PAM_MODULE_ENTRY("pam_exec");


More information about the freebsd-current mailing list