On 03/19/2013 10:57 AM, Martin Kosek wrote: > On 03/18/2013 04:07 PM, Rob Crittenden wrote: >> Martin Kosek wrote: >>> On 03/15/2013 04:42 PM, Rob Crittenden wrote: >>>> Rob Crittenden wrote: >>>>> Martin Kosek wrote: >>>>>> On 03/11/2013 10:07 PM, Rob Crittenden wrote: >>>>>>> Fixed a number of issues applying password policy against LDAP binds. >>>>>>> See patch >>>>>>> for details. >>>>>>> >>>>>>> rob >>>>>>> >>>>>> >>>>>> I see some issues with this fix: >>>>>> >>>>>> 1) Shouldn't group password policy serve only as an override to the main >>>>>> policy? I.e. if I have this policy: >>>>>> >>>>>> # ipa pwpolicy-show test >>>>>> Group: test >>>>>> Priority: 10 >>>>>> Max failures: 2 >>>>>> >>>>>> We should still follow settings other than "Max failures" configured in >>>>>> global policy, right? At least the Kerberos seem to do it. I think we >>>>>> should be consistent in this case. Now, other values just seem to be >>>>>> zero. >>>>> >>>>> There should be only one policy. It isn't supposed to merge policies >>>>> together (there is only one krbPwdPolicyReference per principal). >>> >>> That's a good point. >>> >>>>> >>>>> How is the KDC acting differently? >>> >>> For example, if you set only maximal number of bad password guesses, it does >>> not allow any more (user fbar1 is a member of test group): >>> >>> # ipa pwpolicy-mod test --maxfail 3 >>> Group: test >>> Priority: 10 >>> Max failures: 3 >>> >>> # kinit fbar1 >>> Password for fb...@idm.lab.bos.redhat.com: >>> kinit: Password incorrect while getting initial credentials >>> # kinit fbar1 >>> Password for fb...@idm.lab.bos.redhat.com: >>> kinit: Password incorrect while getting initial credentials >>> # kinit fbar1 >>> Password for fb...@idm.lab.bos.redhat.com: >>> kinit: Password incorrect while getting initial credentials >>> # kinit fbar1 >>> kinit: Clients credentials have been revoked while getting initial >>> credentials >>> >>> But LDAP binds are still allowed >>> >>> # ldapsearch -h localhost -D >>> uid=fbar1,cn=users,cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com -x -w >>> foo >>> -s base -b "" >>> ldap_bind: Invalid credentials (49) >>> >>> I think this is just caused by different processing of >>> krbpwdfailurecountinterval in ipa-kdb and in bind preop (when it is not set, >>> max auth tries checks are pretty much disabled). >> >> We can't examine things until a successful bind is done. If it is done and we >> determine that they should be locked out we refuse to continue. > > I am not sure I follow - what do you mean by "examine things"? The bind preop > plugin can check current policy with unsuccessful login, right? I just thought > that when the custom policy has krbpwdmaxfailure set, but does not have > krbpwdfailurecountinterval set, we should still enforce the krbpwdmaxfailure > "somehow", as ipa-kdb does. > > If not, I think we should either mark those params in pwpolicy plugin as > required or at least add a note to ipa help mentioning this limitation for > LDAP > binds.... > >> >>> >>>>> >>>>>> I think we will need to fix both the pre-op and the post-op to make this >>>>>> working really consistently. >>>>>> >>>>>> 2) The lockout post-op still counts failed logins even though we are in >>>>>> lockout time, is this expected? It is another point if inconsistency >>>>>> with Kerberos auth. It leaves user's krbloginfailedcount stay on "Max >>>>>> failures". >>>>> >>>>> Ok. >>>>> >>>>>> >>>>>> 3) Sometimes, I get into a state when I lockout a new user with Kerberos >>>>>> and then wait some time until the lockout time passes (no admin unlock), >>>>>> I am able to run as many LDAP binds as I want. >>>>> >>>>> Can you clarify? Successful or unsuccessful binds? >>> >>> Unsuccessful binds. I will try to reproduce it again when you fix the >>> crash, it >>> is hard to investigate it with this crash around. >>> >>>>> >>>>>> This is all I found so far. Honza is also reviewing it, so I will let >>>>>> him post hist findings too. >>>>>> >>>>>> Martin >>>> >>>> Here is an updated patch to not increment past the max failures on LDAP >>>> binds. >>> >>> The new patch now causes 389-ds to crash with SIGSEGV if I try to bind as a >>> user with no group policy assigned (Stacktrace attached). >> >> Stupid mistake on my part. >> >> rob >> > > Fixed now :) I found one more issue. When you add a new user with a password, > wrong LDAP binds do not increase failure count until the latest failure > timestamp is set, for example via failed Kerberos login: > > # ipa user-status fbar2 > ----------------------- > Account disabled: False > ----------------------- > Server: vm-037.idm.lab.bos.redhat.com > Failed logins: 0 > Last successful authentication: N/A > Last failed authentication: N/A > Time now: 2013-03-19T09:01:35Z > ---------------------------- > Number of entries returned 1 > ---------------------------- > > # ldapsearch -h localhost -D > uid=fbar2,cn=users,cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com -x -w foo > -s base -b "" > ldap_bind: Invalid credentials (49) > > # ipa user-status fbar2 > ----------------------- > Account disabled: False > ----------------------- > Server: vm-037.idm.lab.bos.redhat.com > Failed logins: 0 > Last successful authentication: N/A > Last failed authentication: N/A > Time now: 2013-03-19T09:01:54Z > ---------------------------- > Number of entries returned 1 > ---------------------------- > > # kinit fbar2 > Password for fb...@idm.lab.bos.redhat.com: > kinit: Password incorrect while getting initial credentials > > # ipa user-status fbar2 > ----------------------- > Account disabled: False > ----------------------- > Server: vm-037.idm.lab.bos.redhat.com > Failed logins: 1 > Last successful authentication: N/A > Last failed authentication: 2013-03-19T09:02:14Z > Time now: 2013-03-19T09:02:16Z > ---------------------------- > Number of entries returned 1 > ---------------------------- > > # ldapsearch -h localhost -D > uid=fbar2,cn=users,cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com -x -w foo > -s base -b "" > ldap_bind: Invalid credentials (49) > > # ipa user-status fbar2 > ----------------------- > Account disabled: False > ----------------------- > Server: vm-037.idm.lab.bos.redhat.com > Failed logins: 2 <<<<<< It increased now > Last successful authentication: N/A > Last failed authentication: 2013-03-19T09:02:20Z > Time now: 2013-03-19T09:02:24Z > ---------------------------- > Number of entries returned 1 > ---------------------------- > > > As for the issue you could not reproduce, it is also quite difficult to > reproduce it for me, I usually have to combine same failed/successful logins, > waits for lockouts etc. I need to look at that more closely. Just for the > record, attaching an LDIF of a user in such a state when I can do as many > failing ldapsearches as I want. This is the password policy: > > # ipa pwpolicy-show > Group: global_policy > Max lifetime (days): 90 > Min lifetime (hours): 1 > History size: 0 > Character classes: 0 > Min length: 8 > Max failures: 6 > Failure reset interval: 60 > Lockout duration: 30 > > Martin >
I discovered that the issue is in the postop: + if (failedcount < max_fail) { + PR_snprintf(failedcountstr, sizeof(failedcountstr), "%lu", failedcount); + slapi_mods_add_string(smods, LDAP_MOD_DELETE, "krbLoginFailedCount", failedcountstr); + failedcount += 1; + PR_snprintf(failedcountstr, sizeof(failedcountstr), "%lu", failedcount); + slapi_mods_add_string(smods, LDAP_MOD_ADD, "krbLoginFailedCount", failedcountstr); } We try to delete failedcount and add failedcount+1, but this operation may fail if the failedcount was set to 0 previously due to exceeded krbpwdfailurecountinterval. This also makes the succeeding last failed auth timestamp update to fail too and cause this vulnerability. I was able to fix it with this change: @@ -526,11 +535,9 @@ static int ipalockout_postop(Slapi_PBlock *pb) */ if (failed_bind) { if (failedcount < max_fail) { - PR_snprintf(failedcountstr, sizeof(failedcountstr), "%lu", failedcount); - slapi_mods_add_string(smods, LDAP_MOD_DELETE, "krbLoginFailedCount", failedcountstr); failedcount += 1; PR_snprintf(failedcountstr, sizeof(failedcountstr), "%lu", failedcount); - slapi_mods_add_string(smods, LDAP_MOD_ADD, "krbLoginFailedCount", failedcountstr); + slapi_mods_add_string(smods, LDAP_MOD_REPLACE, "krbLoginFailedCount", failedcountstr); } if (lastfail) { if (!gmtime_r(&(time_now), &utctime)) { Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel