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

Reply via email to