Bug 6253 - password expiration in pam_winbind isn't handled correctly from Active Directory
password expiration in pam_winbind isn't handled correctly from Active Directory
Status: RESOLVED FIXED
Product: Samba 3.3
Classification: Unclassified
Component: Client tools
3.3.5
x86 Linux
: P3 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
: 6452 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-04-08 07:43 UTC by Blindauer Emmanuel
Modified: 2009-07-29 07:44 UTC (History)
2 users (show)

See Also:


Attachments
fix the test to look for negative values (501 bytes, patch)
2009-04-08 08:07 UTC, Blindauer Emmanuel
no flags Details
Patch for 3-3-test (972 bytes, patch)
2009-05-05 06:08 UTC, Guenther Deschner
no flags Details
revised version of that patch (981 bytes, patch)
2009-06-05 05:24 UTC, Guenther Deschner
vl: review+
Details
3-3: final version of the patch (955 bytes, patch)
2009-06-18 18:51 UTC, Guenther Deschner
vl: review+
jra: review+
Details
3-4: final version of the patch (922 bytes, patch)
2009-06-18 18:55 UTC, Guenther Deschner
jra: review+
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Blindauer Emmanuel 2009-04-08 07:43:26 UTC
setup: a samba 3.3.2 client using winbind against a windows 2003 domain.
pam_winbind is setup in pam to authenticate users.
Debug show that password policy has expire= -1 (i.e. never expires)


Here the full log of pam with debug:

pam_winbind(login:auth): [pamh: 0x8dd7170] ENTER: pam_sm_authenticate (flags: 0x0000)
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: ITEM(PAM_SERVICE) = "login" (0x8dd7228)
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: ITEM(PAM_USER) = "blindaue" (0x8dd7238)
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: ITEM(PAM_TTY) = "tty3" (0x8dde730)
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: ITEM(PAM_USER_PROMPT) = "login: " (0x8dde740)
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: ITEM(PAM_CONV) = 0x8dd7248
pam_winbind(login:auth): getting password (0x00001281)
pam_winbind(login:auth): Verify user 'blindaue'
pam_winbind(login:auth): enabling krb5 login flag 
pam_winbind(login:auth): enabling cached login flag 
pam_winbind(login:auth): request wbcLogonUser succeeded
pam_winbind(login:auth): user 'blindaue' granted access
pam_winbind(login:auth): Password has expired (Password was last set: 1221643511, the policy says it should expire here 1221643510 (now it's: 1239188640)) 
pam_winbind(login:auth): [pamh: 0x8dd7170] LEAVE: pam_sm_authenticate returning 0 (PAM_SUCCESS)
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: ITEM(PAM_SERVICE) = "login" (0x8dd7228)
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: ITEM(PAM_USER) = "blindaue" (0x8dd7238)
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: ITEM(PAM_TTY) = "tty3" (0x8dde730)
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: ITEM(PAM_AUTHTOK) = 0x8ddfe80
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: ITEM(PAM_USER_PROMPT) = "login: " (0x8dde740)
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: ITEM(PAM_CONV) = 0x8dd7248
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: DATA(PAM_WINBIND_NEW_AUTHTOK_REQD) = "27" (0x8ddff88)
pam_winbind(login:auth): [pamh: 0x8dd7170] STATE: DATA(PAM_WINBIND_NEW_AUTHTOK_REQD_DURING_AUTH) = "1" (0x8de0a10)
pam_winbind(login:account): [pamh: 0x8dd7170] ENTER: pam_sm_acct_mgmt (flags: 0x0000)
pam_winbind(login:account): pam_sm_acct_mgmt success but PAM_WINBIND_NEW_AUTHTOK_REQD is set
pam_winbind(login:account): user 'blindaue' needs new password
pam_winbind(login:account): [pamh: 0x8dd7170] LEAVE: pam_sm_acct_mgmt returning 12 (PAM_NEW_AUTHTOK_REQD)
Comment 1 Blindauer Emmanuel 2009-04-08 08:05:58 UTC
Problem found: wbcUserPasswordPolicyInfo defines "expire" as uint64_t
"if (policy->expire <=0)" will always be true
replace the test by == -1 and the problem is solved.
Comment 2 Blindauer Emmanuel 2009-04-08 08:07:29 UTC
Created attachment 4056 [details]
fix the test to look for negative values
Comment 3 Guenther Deschner 2009-05-05 06:08:27 UTC
Created attachment 4116 [details]
Patch for 3-3-test

This is the patch that went into master/v3-4-test
Comment 4 Volker Lendecke 2009-05-05 06:16:21 UTC
Doesn't this generate a warning on 32-bit boxes? Shouldn't this be an explicit

== (uint64_t)-1

?

Volker
Comment 5 Blindauer Emmanuel 2009-05-05 07:22:58 UTC
no warning seen with gcc 4.3.2 (configure --enable-developer)
Comment 6 Volker Lendecke 2009-05-05 07:28:09 UTC
I'd still feel better with the explicit cast, but if that isn't possible and the bug is fixed on both 32 and 64 bit, then I'm certainly ok with it.

Volker
Comment 7 Guenther Deschner 2009-05-05 14:20:49 UTC
Yeah, also got not warning, I'll add the explicit typecast and repost 3-3 patches.
Comment 8 Guenther Deschner 2009-06-05 05:24:25 UTC
Created attachment 4243 [details]
revised version of that patch

This is the patch that should go into 3.3
Comment 9 Guenther Deschner 2009-06-17 05:57:42 UTC
*** Bug 6452 has been marked as a duplicate of this bug. ***
Comment 10 Guenther Deschner 2009-06-17 05:59:23 UTC
Karolin, we missed this one here for 3.3.5 unfortunately. really needs to get into 3.3 and 3.4.
Comment 11 Pietro Donatini 2009-06-17 06:25:15 UTC
In some AD account I found policy->expire to be 0
and pass_must_change_time vith value 
in the future (so the account is active)

With the suggested patch (passing from <=0
to == -1) accounts like this
are not allowed to login since the second time
_pam_send_password_expiry_message il called, 
next_change = info->pass_last_set_time + policy->expire
and so the login fail.

I don't know the real reason in AD for expire = 0 but
I suggest to consider to add to the test also 
policy->expire == 0
Comment 12 Guenther Deschner 2009-06-17 07:19:56 UTC
ok, this needs further investigation.
Comment 13 Guenther Deschner 2009-06-18 18:51:16 UTC
Created attachment 4317 [details]
3-3: final version of the patch

This is a very important fix for 3.3
Comment 14 Guenther Deschner 2009-06-18 18:55:16 UTC
Created attachment 4318 [details]
3-4: final version of the patch
Comment 15 Jeremy Allison 2009-06-18 19:06:41 UTC
Comment on attachment 4318 [details]
3-4: final version of the patch

Look good to me !
Jeremy.
Comment 16 Jeremy Allison 2009-06-18 19:07:06 UTC
Comment on attachment 4317 [details]
3-3: final version of the patch

Looks good to me !
Jeremy.
Comment 17 Guenther Deschner 2009-06-18 19:16:42 UTC
Ok, Karolin, please push to 3.3 and 3.4. Thanks.
Comment 18 Karolin Seeger 2009-06-19 01:18:43 UTC
Pushed.
Closing out bug report.

Thanks!