[Freeipa-devel] [PATCH] 1092 Fix LDAP lockout plugin
Rob Crittenden
rcritten at redhat.com
Wed Mar 20 22:20:43 UTC 2013
Martin Kosek wrote:
> 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.
We need to consider the interval because all the failures have to occur
within it.
I reordered a bit of the evaluation code and added consideration for
lockout period so it is properly enforced now both for duration == 0
(permanent lockout) and for a duration.
I confirmed that administrative unlock works as well, also both in the
case of a duration lockout and a permanent lockout.
Hopefully I tested better this go-around.
rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-1092-6-lockout.patch
Type: text/x-patch
Size: 15514 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130320/e9e85d74/attachment.bin>
More information about the Freeipa-devel
mailing list