summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG3
-rw-r--r--libpam/pam_dispatch.c68
-rw-r--r--libpam/pam_handlers.c4
-rw-r--r--libpam/pam_private.h2
4 files changed, 55 insertions, 22 deletions
diff --git a/CHANGELOG b/CHANGELOG
index d9c2295f..e00ff4e7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -49,6 +49,9 @@ bug report - outstanding bugs are listed here:
0.76: please submit patches for this section with actual code/doc
patches!
+* fix for legacy behavior of pam_setcred and pam_close_session in
+ the case that pam_authenticate and pam_open_session hadn't been
+ called - bug report from S Park. (Bug 468724 - agmorgan)
* some BSD updates and fixes from Mark Murray - including a slightly
more robust conversation function and some minimization of gcc
warnings. (Bugs 449203,463984 - agmorgan)
diff --git a/libpam/pam_dispatch.c b/libpam/pam_dispatch.c
index 2a6befd4..3ebdb5ba 100644
--- a/libpam/pam_dispatch.c
+++ b/libpam/pam_dispatch.c
@@ -22,6 +22,11 @@
#define _PAM_POSITIVE +1
#define _PAM_NEGATIVE -1
+/* frozen chain required codes */
+#define _PAM_PLEASE_FREEZE 0
+#define _PAM_MAY_BE_FROZEN 1
+#define _PAM_MUST_BE_FROZEN 2
+
/*
* walk a stack of modules. Interpret the administrator's instructions
* when combining the return code of each module.
@@ -99,9 +104,45 @@ 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 */
+ /*
+ * use_cached_chain is how we ensure that the setcred/close_session
+ * and chauthtok(2) modules are called in the same order as they did
+ * when they were invoked as auth/open_session/chauthtok(1). This
+ * feature was added in 0.75 to make the behavior of pam_setcred
+ * sane. It was debugged by release 0.76.
+ */
+ if (use_cached_chain != _PAM_PLEASE_FREEZE) {
+
+ /* a former stack execution should have frozen the chain */
+
cached_retval = *(h->cached_retval_p);
+ if (cached_retval == _PAM_INVALID_RETVAL) {
+
+ /* This may be a problem condition. It implies that
+ the application is running setcred, close_session,
+ chauthtok(2nd) without having first run
+ authenticate, open_session, chauthtok(1st)
+ [respectively]. */
+
+ D(("use_cached_chain is set to [%d],"
+ " but cached_retval == _PAM_INVALID_RETVAL",
+ use_cached_chain));
+
+ /* In the case of close_session and setcred there is a
+ backward compatibility reason for allowing this, in
+ the chauthtok case we have encountered a bug in
+ libpam! */
+
+ if (use_cached_chain == _PAM_MAY_BE_FROZEN) {
+ /* (not ideal) force non-frozen stack control. */
+ cached_retval = retval;
+ } else {
+ D(("BUG in libpam -"
+ " chain is required to be frozen but isn't"));
+
+ /* cached_retval is already _PAM_INVALID_RETVAL */
+ }
+ }
} else {
/* this stack execution is defining the frozen chain */
cached_retval = h->cached_retval = retval;
@@ -110,6 +151,7 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h,
/* verify that the return value is a valid one */
if ((cached_retval < PAM_SUCCESS)
|| (cached_retval >= _PAM_RETURN_VALUES)) {
+
retval = PAM_MUST_FAIL_CODE;
action = _PAM_ACTION_BAD;
} else {
@@ -133,11 +175,6 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h,
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;
@@ -145,10 +182,6 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h,
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;
@@ -178,11 +211,6 @@ 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 --
@@ -261,7 +289,7 @@ int _pam_dispatch(pam_handle_t *pamh, int flags, int choice)
return retval;
}
- use_cached_chain = 0; /* default to setting h->cached_retval */
+ use_cached_chain = _PAM_PLEASE_FREEZE;
switch (choice) {
case PAM_AUTHENTICATE:
@@ -269,7 +297,7 @@ int _pam_dispatch(pam_handle_t *pamh, int flags, int choice)
break;
case PAM_SETCRED:
h = pamh->handlers.conf.setcred;
- use_cached_chain = 1;
+ use_cached_chain = _PAM_MAY_BE_FROZEN;
break;
case PAM_ACCOUNT:
h = pamh->handlers.conf.acct_mgmt;
@@ -279,12 +307,12 @@ 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;
+ use_cached_chain = _PAM_MAY_BE_FROZEN;
break;
case PAM_CHAUTHTOK:
h = pamh->handlers.conf.chauthtok;
if (flags & PAM_UPDATE_AUTHTOK) {
- use_cached_chain = 1;
+ use_cached_chain = _PAM_MUST_BE_FROZEN;
}
break;
default:
diff --git a/libpam/pam_handlers.c b/libpam/pam_handlers.c
index 8e32f8e8..d007ed98 100644
--- a/libpam/pam_handlers.c
+++ b/libpam/pam_handlers.c
@@ -751,7 +751,7 @@ 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 = _PAM_INVALID_RETVAL;
(*handler_p)->cached_retval_p = &((*handler_p)->cached_retval);
(*handler_p)->argc = argc;
(*handler_p)->argv = argv; /* not a copy */
@@ -772,7 +772,7 @@ 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 */
+ (*handler_p2)->cached_retval = _PAM_INVALID_RETVAL; /* ignored */
/* Note, this next entry points to the handler_p value! */
(*handler_p2)->cached_retval_p = &((*handler_p)->cached_retval);
(*handler_p2)->argc = argc;
diff --git a/libpam/pam_private.h b/libpam/pam_private.h
index 52f6c5e6..7afc4fa7 100644
--- a/libpam/pam_private.h
+++ b/libpam/pam_private.h
@@ -43,6 +43,8 @@
/* components of the pam_handle structure */
+#define _PAM_INVALID_RETVAL -1 /* default value for cached_retval */
+
struct handler {
int must_fail;
int (*func)(pam_handle_t *pamh, int flags, int argc, char **argv);
generated by cgit v1.2.3 (git 2.25.1) at 2024-04-24 04:59:06 +0000