Relevant BUGIDs: 129775

Purpose of commit: bugfix

Commit summary:
---------------
This bugfix leads to backwardly incompatable behavior with earlier
releases of Linux-PAM.

Note, this cleans up the setcred/session and chauthtok stacks in
such a way that it is no longer preferred that the setcred module
always return the same error code as the auth components of said
modules did.

This means behavior should be a great deal more sane. It also gives
meaning to the unique return codes that are available to pam_sm_setcred.
[I'm sure that when we add support for credential relevant events,
 this change will be critical.]

4 files changed, 78 insertions, 10 deletions

diff --git a/libpam/include/security/pam_modules.h b/libpam/include/security/pam_modules.h
index b555e8b2..4182ebd6 100644
--- a/libpam/include/security/pam_modules.h
+++ b/libpam/include/security/pam_modules.h
@@ -112,7 +112,7 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
 #define PAM_PRELIM_CHECK		0x4000
 
 /* The password service should update passwords Note: PAM_PRELIM_CHECK
- * and PAM_UPDATE_AUTHTOK can not both be set simultaneously! */
+ * and PAM_UPDATE_AUTHTOK cannot both be set simultaneously! */
 #define PAM_UPDATE_AUTHTOK		0x2000
 
 
diff --git a/libpam/pam_dispatch.c b/libpam/pam_dispatch.c
index 02325665..6212ac87 100644
--- a/libpam/pam_dispatch.c
+++ b/libpam/pam_dispatch.c
@@ -1,7 +1,7 @@
 /* pam_dispatch.c - handles module function dispatch */
 
 /*
- * Copyright (c) 1998 Andrew G. Morgan <morgan@linux.kernel.org>
+ * Copyright (c) 1998 Andrew G. Morgan <morgan@kernel.org>
  *
  * $Id$
  */
@@ -28,7 +28,7 @@
  */
 
 static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h,
-			     _pam_boolean resumed)
+			     _pam_boolean resumed, int use_cached_chain)
 {
     int depth, impression, status, skip_depth;
 
@@ -62,7 +62,7 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h,
 
     /* Loop through module logic stack */
     for (depth=0 ; h != NULL ; h = h->next, ++depth) {
-	int retval, action;
+	int retval, cached_retval, action;
 
 	/* skip leading modules if they have already returned */
 	if (depth < skip_depth) {
@@ -78,7 +78,7 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h,
 	    retval = h->func(pamh, flags, h->argc, h->argv);
 	    D(("module returned: %s", pam_strerror(pamh, retval)));
 	    if (h->must_fail) {
-		D(("module poorly listed in pam.conf; forcing failure"));
+		D(("module poorly listed in PAM config; forcing failure"));
 		retval = PAM_MUST_FAIL_CODE;
 	    }
 	}
@@ -99,23 +99,57 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h,
 	    return retval;
 	}
 
+	if (use_cached_chain) {
+	    /* a former stack execution has frozen the chain */
+	    cached_retval = *(h->cached_retval_p);
+	} else {
+	    /* this stack execution is defining the frozen chain */
+	    cached_retval = h->cached_retval = retval;
+	}
+
 	/* verify that the return value is a valid one */
-	if (retval < PAM_SUCCESS || retval >= _PAM_RETURN_VALUES) {
+	if ((cached_retval < PAM_SUCCESS)
+	    || (cached_retval >= _PAM_RETURN_VALUES)) {
 	    retval = PAM_MUST_FAIL_CODE;
 	    action = _PAM_ACTION_BAD;
 	} else {
-	    action = h->actions[retval];
+	    /* We treat the current retval with some respect. It may
+	       (for example, in the case of setcred) have a value that
+	       needs to be propagated to the user.  We want to use the
+	       cached_retval to determine the modules to be executed
+	       in the stacked chain, but we want to treat each
+	       non-ignored module in the cached chain as now being
+	       'required'. We only need to treat the,
+	       _PAM_ACTION_IGNORE, _PAM_ACTION_IS_JUMP and
+	       _PAM_ACTION_RESET actions specially. */
+
+	    action = h->actions[cached_retval];
 	}
 
+	D((stderr,
+	   "use_cached_chain=%d action=%d cached_retval=%d retval=%d\n",
+	   use_cached_chain, action, cached_retval, retval));
+
 	/* decide what to do */
 	switch (action) {
 	case _PAM_ACTION_RESET:
+
+	    /* if (use_cached_chain) {
+	           XXX - we need to consider the use_cached_chain case
+      	                 do we want to trash accumulated info here..?
+	       } */
+
 	    impression = _PAM_UNDEF;
 	    status = PAM_MUST_FAIL_CODE;
 	    break;
 
 	case _PAM_ACTION_OK:
 	case _PAM_ACTION_DONE:
+
+	    /* XXX - should we maintain cached_status and status in
+               the case of use_cached_chain? The same with BAD&DIE
+               below */
+
 	    if ( impression == _PAM_UNDEF
 		 || (impression == _PAM_POSITIVE && status == PAM_SUCCESS) ) {
 		impression = _PAM_POSITIVE;
@@ -129,7 +163,7 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h,
 	case _PAM_ACTION_BAD:
 	case _PAM_ACTION_DIE:
 #ifdef PAM_FAIL_NOW_ON
-	    if ( retval == PAM_ABORT ) {
+	    if ( cached_retval == PAM_ABORT ) {
 		impression = _PAM_NEGATIVE;
 		status = PAM_PERM_DENIED;
 		goto decision_made;
@@ -145,6 +179,11 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h,
 	    break;
 
 	case _PAM_ACTION_IGNORE:
+	    /* if (use_cached_chain) {
+	            XXX - when evaluating a cached
+	            chain, do we still want to ignore the module's
+	            return value?
+	       } */
 	    break;
 
         /* if we get here, we expect action is a positive number --
@@ -152,6 +191,20 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h,
 
 	default:
 	    if ( _PAM_ACTION_IS_JUMP(action) ) {
+
+		/* If we are evaluating a cached chain, we treat this
+		   module as required (aka _PAM_ACTION_OK) as well as
+		   executing the jump. */
+
+		if (use_cached_chain) {
+		    if (impression == _PAM_UNDEF
+			|| (impression == _PAM_POSITIVE
+			    && status == PAM_SUCCESS) ) {
+			impression = _PAM_POSITIVE;
+			status = retval;
+		    }
+		}
+		
 		/* this means that we need to skip #action stacked modules */
 		do {
  		    h = h->next;
@@ -192,7 +245,7 @@ decision_made:     /* by getting  here we have made a decision */
 int _pam_dispatch(pam_handle_t *pamh, int flags, int choice)
 {
     struct handler *h = NULL;
-    int retval;
+    int retval, use_cached_chain;
     _pam_boolean resumed;
 
     IF_NO_PAMH("_pam_dispatch", pamh, PAM_SYSTEM_ERR);
@@ -209,12 +262,15 @@ int _pam_dispatch(pam_handle_t *pamh, int flags, int choice)
 	return retval;
     }
 
+    use_cached_chain = 0;   /* default to setting h->cached_retval */
+
     switch (choice) {
     case PAM_AUTHENTICATE:
 	h = pamh->handlers.conf.authenticate;
 	break;
     case PAM_SETCRED:
 	h = pamh->handlers.conf.setcred;
+	use_cached_chain = 1;
 	break;
     case PAM_ACCOUNT:
 	h = pamh->handlers.conf.acct_mgmt;
@@ -224,9 +280,13 @@ int _pam_dispatch(pam_handle_t *pamh, int flags, int choice)
 	break;
     case PAM_CLOSE_SESSION:
 	h = pamh->handlers.conf.close_session;
+	use_cached_chain = 1;
 	break;
     case PAM_CHAUTHTOK:
 	h = pamh->handlers.conf.chauthtok;
+	if (flags & PAM_UPDATE_AUTHTOK) {
+	    use_cached_chain = 1;
+	}
 	break;
     default:
 	_pam_system_log(LOG_ERR, "undefined fn choice; %d", choice);
@@ -273,7 +333,7 @@ int _pam_dispatch(pam_handle_t *pamh, int flags, int choice)
     __PAM_TO_MODULE(pamh);
 
     /* call the list of module functions */
-    retval = _pam_dispatch_aux(pamh, flags, h, resumed);
+    retval = _pam_dispatch_aux(pamh, flags, h, resumed, use_cached_chain);
     resumed = PAM_FALSE;
 
     __PAM_TO_APP(pamh);
diff --git a/libpam/pam_handlers.c b/libpam/pam_handlers.c
index a22d66f6..b2065999 100644
--- a/libpam/pam_handlers.c
+++ b/libpam/pam_handlers.c
@@ -756,6 +756,8 @@ int _pam_add_handler(pam_handle_t *pamh
     (*handler_p)->must_fail = must_fail;        /* failure forced? */
     (*handler_p)->func = func;
     memcpy((*handler_p)->actions,actions,sizeof((*handler_p)->actions));
+    (*handler_p)->cached_retval = -1;                     /* error */
+    (*handler_p)->cached_retval_p = &((*handler_p)->cached_retval);
     (*handler_p)->argc = argc;
     (*handler_p)->argv = argv;                       /* not a copy */
     (*handler_p)->next = NULL;
@@ -775,6 +777,9 @@ int _pam_add_handler(pam_handle_t *pamh
 	(*handler_p2)->must_fail = must_fail;        /* failure forced? */
 	(*handler_p2)->func = func2;
 	memcpy((*handler_p2)->actions,actions,sizeof((*handler_p2)->actions));
+	(*handler_p2)->cached_retval = -1;            /* ignored */
+	/* Note, this next entry points to the handler_p value! */
+	(*handler_p2)->cached_retval_p = &((*handler_p)->cached_retval);
 	(*handler_p2)->argc = argc;
 	if (argv) {
 	    if (((*handler_p2)->argv = malloc(argvlen)) == NULL) {
diff --git a/libpam/pam_private.h b/libpam/pam_private.h
index 2aa90bd4..52f6c5e6 100644
--- a/libpam/pam_private.h
+++ b/libpam/pam_private.h
@@ -47,6 +47,9 @@ struct handler {
     int must_fail;
     int (*func)(pam_handle_t *pamh, int flags, int argc, char **argv);
     int actions[_PAM_RETURN_VALUES];
+    /* set by authenticate, open_session, chauthtok(1st)
+       consumed by setcred, close_session, chauthtok(2nd) */
+    int cached_retval; int *cached_retval_p;
     int argc;
     char **argv;
     struct handler *next;