[Freeipa-devel] [PATCH] 1092 Fix LDAP lockout plugin
Martin Kosek
mkosek at redhat.com
Wed Mar 20 18:41:35 UTC 2013
On 03/20/2013 04:52 PM, Rob Crittenden wrote:
> 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
>
This is not what I meant, sorry for not being clear previously. This patch
still lets user to override lockout and try to login if "Failure reset
interval" already passed even though account is still in lockout phase.
Consider this 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: 3
Failure reset interval: 30 <<<<
Lockout duration: 300 <<<<
After 3 tries by Kerberos/LDAP auth, account is locked out. However, after 30
seconds, LDAP binds are allowed again due to "Failure reset interval". Kerberos
auth is not allowed until 300 seconds pass (as I would expect).
This is what I think that needs to be done when user is in lockout phase:
- do not care about "Failure reset interval" in preop/postop
- do not update failed login count/last failed login timestamp in postop. We
currently update last failed login timestamp, but Kerberos does not do it.
Otherwise, this would practically make the lockout phase longer as we count it
based on the last failed timestamp.
Martin
More information about the Freeipa-devel
mailing list