summaryrefslogtreecommitdiff
path: root/libpam
diff options
context:
space:
mode:
authorAndrew G. Morgan <morgan@kernel.org>2001-10-11 04:52:25 +0000
committerAndrew G. Morgan <morgan@kernel.org>2001-10-11 04:52:25 +0000
commit64ab317e86dc8e798b4f4ed603227206c0fc001b (patch)
tree724590bf0513412074404694ffcfb0f4c8432999 /libpam
parent345044121bc4e8977a22d6235d31df4b2114a240 (diff)
Relevant BUGIDs: 468724
Purpose of commit: bugfix Commit summary: --------------- Legacy behavior for pam_close_session and pam_setcred was not sufficient. Basically, it appears to be common practice for some applications to call these functions without first calling pam_authenticate and pam_open_session which would have frozen the auth and session module stacks. The new behavior is to treat the returns of these secondary functions as authoritative when navigating the stack in the absence of a chain-freezing first set of calls. pam_chauthtok should not benefit from this behavior, and there does not appear to be a justification for using an event like this to freeze the stack outright - legacy behavior did not do that.
Diffstat (limited to 'libpam')
-rw-r--r--libpam/pam_dispatch.c68
-rw-r--r--libpam/pam_handlers.c4
-rw-r--r--libpam/pam_private.h2
3 files changed, 52 insertions, 22 deletions
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);