[Freeipa-devel] [PATCH] 1092 Fix LDAP lockout plugin
Rob Crittenden
rcritten at redhat.com
Wed Mar 20 15:52:10 UTC 2013
Martin Kosek wrote:
> On 03/19/2013 05:09 PM, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> 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 fbar1 at IDM.LAB.BOS.REDHAT.COM:
>>>>>> kinit: Password incorrect while getting initial credentials
>>>>>> # kinit fbar1
>>>>>> Password for fbar1 at IDM.LAB.BOS.REDHAT.COM:
>>>>>> kinit: Password incorrect while getting initial credentials
>>>>>> # kinit fbar1
>>>>>> Password for fbar1 at 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 fbar2 at 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
>>>
>>
>> I cam to a similar conclusion. We want to be careful when deleting values so we
>> can handle the case where multiple bind attempts are happening at once so we
>> try to avoid using REPLACE. I changed it so we save the value of failedcount
>> when the lockout expires.
>>
>> I also added a chance to handle the case where there is no failedcount at all.
>> I was trying to delete a non-existent 0 value.
>>
>> rob
>
> Ok, the updated version works well. I think we are getting close to finish this.
>
> I now also tried krbPwdLockoutDuration=0 and it does not/cannot work properly
> as we are still reseting failedcount to zero even though the account is locked out.
>
> Shouldn't we do it only if the account is not in lockout period?
Yup. Now skip that calculation if lockout_duration == 0.
I also tested that things work if user has a strong policy and is locked
out due to duration = 0, delete that policy and then user can try to
authenticate again b/c default policy has a lockout timeout.
rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-1092-5-lockout.patch
Type: text/x-patch
Size: 13438 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130320/beca4b1e/attachment.bin>
More information about the Freeipa-devel
mailing list