Bug 1524 - [patch] pam_winbind sends PAM_NEW_AUTHTOK_REQD at wrong time
Summary: [patch] pam_winbind sends PAM_NEW_AUTHTOK_REQD at wrong time
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: winbind (show other bugs)
Version: 3.0.4
Hardware: All All
: P3 normal
Target Milestone: none
Assignee: Gerald (Jerry) Carter (dead mail address)
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-07-13 08:47 UTC by Scott Barker
Modified: 2006-01-13 04:12 UTC (History)
1 user (show)

See Also:


Attachments
Patch to fix winbind bug (2.53 KB, patch)
2005-02-17 15:28 UTC, Scott Barker
no flags Details
Patch to solve it. (2.50 KB, patch)
2005-09-12 00:00 UTC, Gabriel Buades Rubio
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Scott Barker 2004-07-13 08:47:00 UTC
When authenticating a user for login, if a new password is required,
pam_authenticate should return PAM_SUCCESS, and pam_acct_mgmt should return
PAM_NEW_AUTHTOK_REQD.

However, pam_winbind returns PAM_NEW_AUTHTOK_REQD for pam_authenticate, making
it impossible for a user to login against a Windows DC if they must change their
password on their next login.

The following patch appears to fix this:
--- samba/source/nsswitch/pam_winbind.c 2003-06-07 11:57:34.000000000 -0600
+++ samba.mine/source/nsswitch/pam_winbind.c    2004-07-12 15:19:17.000000000 -0600
@@ -454,7 +454,24 @@
      }

      /* Now use the username to look up password */
-     return winbind_auth_request(username, password, ctrl);
+     retval = winbind_auth_request(username, password, ctrl);
+
+     static char buf[256] = {0};
+     sprintf(buf, "%d", retval);
+     if(pam_set_data(pamh, "winbind_auth_retval", buf, NULL) != PAM_SUCCESS) {
+        _pam_log(LOG_WARNING, "pam_sm_authenticate return code %d cannot be
saved for pam_sm_acct_mgmt", retval);
+        return PAM_BAD_ITEM;
+     }
+     switch (retval) {
+        case PAM_AUTHTOK_EXPIRED:
+           _pam_log(LOG_WARNING, "pam_sm_authenticate would return
PAM_AUTHTOK_EXPIRED, returning PAM_SUCCESS instead", retval);
+           return PAM_SUCCESS;
+       case PAM_NEW_AUTHTOK_REQD:
+           _pam_log(LOG_WARNING, "pam_sm_authenticate would return
PAM_NEW_AUTHTOK_REQD, returning PAM_SUCCESS instead", retval);
+           return PAM_SUCCESS;
+        default:
+           return retval;
+     }
 }

 PAM_EXTERN
@@ -503,7 +520,7 @@
        case 0:




            /* Otherwise, the authentication looked good */
            _pam_log(LOG_NOTICE, "user '%s' granted acces", username);
-           return PAM_SUCCESS;
+           break;
        default:
            /* we don't know anything about this return value */
            _pam_log(LOG_ERR, "internal module error (retval = %d, user = `%s'",
@@ -511,6 +528,24 @@
            return PAM_SERVICE_ERR;
     }

+    char *ptr = NULL;
+    if( pam_get_data(pamh, "winbind_auth_retval", (const void **)&ptr) !=
PAM_SUCCESS ) {
+       _pam_log(LOG_WARNING, "pam_sm_acct_mgmt cannot retrieve
pam_sm_authenticate return value", retval);
+       return retval;
+    }
+
+    retval = atoi( ptr );
+    switch (retval) {
+       case PAM_AUTHTOK_EXPIRED:
+          _pam_log(LOG_WARNING, "pam_sm_acct_mgmt returning
PAM_AUTHTOK_EXPIRED", retval);
+          return retval;
+       case PAM_NEW_AUTHTOK_REQD:
+          _pam_log(LOG_WARNING, "pam_sm_acct_mgmt returning
PAM_NEW_AUTHTOK_REQD", retval);
+          return retval;
+       default:
+         return PAM_SUCCESS;
+    }
+
     /* should not be reached */
     return PAM_IGNORE;
 }
Comment 1 Gerald (Jerry) Carter (dead mail address) 2005-02-17 10:01:19 UTC
any chance you could attach the patch instead of including it inline ?
Thanks.
Comment 2 Scott Barker 2005-02-17 15:28:04 UTC
Created attachment 972 [details]
Patch to fix winbind bug
Comment 3 Gerald (Jerry) Carter (dead mail address) 2005-02-17 15:52:36 UTC
thanks.  i'll get to this next week.
Comment 4 Gabriel Buades Rubio 2005-09-12 00:00:32 UTC
Created attachment 1436 [details]
Patch to solve it.

Delays PAM_AUTHTOK_EXPIRED or PAM_NEW_AUTHTOK_REQD to accounting step.
Comment 5 Scott Barker 2005-10-03 14:41:43 UTC
One thing I note in your patch: for the case where the return value should be
PAM_AUTHTOK_EXPIRED, your patch will instead return PAM_NEW_AUTHTOK_REQD. While
I'm no pam expert by any means, my reading of the various documentation I found
implied that these two differ, in that PAM_NEW_AUTHTOK_REQD means the user's
password has expired, and should be changed, while PAM_AUTHTOK_EXPIRED means the
user's password has expired, and cannot be changed by the user (the user is
effectively locked out). Thus, I think perhaps the value should be preserved.
Comment 6 Marco Marinuzzo 2005-10-20 09:52:40 UTC
I tried the 3.0.20b version (pam_winbind.c differs). I patch it by hand with 
the id=972 but logging in with an expired domain account (ads) is always 
successfull (no change password request).
So I tried the id=1436, but it calls an undefined function 
(_pam_winbind_cleanup_func).
Comment 7 Guenther Deschner 2005-12-22 04:25:04 UTC
If it is possible for you, please re-test with the pam_winbind module that is now in trunk. It should be fixed there.
Comment 8 Guenther Deschner 2006-01-13 04:12:06 UTC
Fixed in Subversion, please reopen if it is still an issue for you.