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 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


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

>From 79e32f9df48ba2ddec222e119ef1d064e345255c Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Fri, 15 Feb 2013 11:51:59 -0500
Subject: [PATCH] Fix lockout of LDAP bind.

There were several problems:

- A cut-n-paste error where the wrong value was being considered when
  an account was administratively unlocked.
- An off-by-one error where LDAP got one extra bind attempt.
- krbPwdPolicyReference wasn't being retrieved as a virtual attribute so
  only the global_policy was used.
- The lockout duration wasn't examined in the context of too many failed
  logins so wasn't being applied properly.
- Lockout duration wasn't used properly so a user was effectively unlocked
  when the failure interval expired.
- krbLastFailedAuth and krbLoginFailedCount are no longer updated past
  max failures.

https://fedorahosted.org/freeipa/ticket/3433
---
 .../ipa-slapi-plugins/ipa-lockout/ipa_lockout.c    | 226 +++++++++++++--------
 1 file changed, 144 insertions(+), 82 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c b/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c
index fea997d79825e05a6ffe4bf65e22821948794ff3..107614e4869ea0d7412af34bd174671617a5bb2b 100644
--- a/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c
+++ b/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c
@@ -226,6 +226,50 @@ done:
     return ret;
 }
 
+int ipalockout_getpolicy(Slapi_Entry *target_entry, Slapi_Entry **policy_entry,
+                         Slapi_ValueSet** values, char **actual_type_name,
+                         const char **policy_dn, int *attr_free_flags,
+                         char **errstr)
+{
+    int ldrc = 0;
+    int flags = 0;
+    int type_name_disposition = 0;
+    Slapi_DN *pdn = NULL;
+
+    /* Only continue if there is a password policy */
+    ldrc = slapi_vattr_values_get(target_entry, "krbPwdPolicyReference",
+                                values,
+                                &type_name_disposition, actual_type_name,
+                                SLAPI_VIRTUALATTRS_REQUEST_POINTERS,
+                                attr_free_flags);
+    if (ldrc == 0) {
+        Slapi_Value *sv = NULL;
+
+        slapi_valueset_first_value(*values, &sv);
+
+        if (values != NULL) {
+            *policy_dn = slapi_value_get_string(sv);
+        }
+    }
+
+    if (*policy_dn == NULL) {
+        LOG_TRACE("No kerberos password policy\n");
+        return LDAP_SUCCESS;
+    } else {
+        pdn = slapi_sdn_new_dn_byref(*policy_dn);
+        ldrc = slapi_search_internal_get_entry(pdn, NULL, policy_entry,
+                getPluginID());
+        slapi_sdn_free(&pdn);
+        if (ldrc != LDAP_SUCCESS) {
+            LOG_FATAL("Failed to retrieve entry \"%s\": %d\n", *policy_dn, ldrc);
+            *errstr = "Failed to retrieve account policy.";
+            return LDAP_OPERATIONS_ERROR;
+        }
+    }
+
+    return LDAP_SUCCESS;
+}
+
 int
 ipalockout_init(Slapi_PBlock *pb)
 {
@@ -346,7 +390,7 @@ ipalockout_close(Slapi_PBlock * pb)
 static int ipalockout_postop(Slapi_PBlock *pb)
 {
     char *dn = NULL;
-    char *policy_dn = NULL;
+    const char *policy_dn = NULL;
     Slapi_Entry *target_entry = NULL;
     Slapi_Entry *policy_entry = NULL;
     Slapi_DN *sdn = NULL;
@@ -358,8 +402,12 @@ static int ipalockout_postop(Slapi_PBlock *pb)
     int ldrc, rc = 0;
     int ret = LDAP_SUCCESS;
     unsigned long failedcount = 0;
+    long old_failedcount = -1;
     char failedcountstr[32];
+    char *failedstr = NULL;
     int failed_bind = 0;
+    unsigned int lockout_duration = 0;
+    unsigned int max_fail = 0;
     struct tm utctime;
     time_t time_now;
     char timestr[GENERALIZED_TIME_LENGTH+1];
@@ -367,6 +415,10 @@ static int ipalockout_postop(Slapi_PBlock *pb)
     char *lastfail = NULL;
     int tries = 0;
     int failure = 1;
+    int type_name_disposition = 0;
+    char *actual_type_name = NULL;
+    int attr_free_flags = 0;
+    Slapi_ValueSet *values = NULL;
 
     LOG_TRACE("--in-->\n");
 
@@ -431,29 +483,27 @@ static int ipalockout_postop(Slapi_PBlock *pb)
     }
     slapi_value_free(&objectclass);
 
-    /* Only update if there is a password policy */
-    policy_dn = slapi_entry_attr_get_charptr(target_entry, "krbPwdPolicyReference");
-    if (policy_dn == NULL) {
-        LOG_TRACE("No kerberos password policy\n");
+    ldrc = ipalockout_getpolicy(target_entry, &policy_entry,
+                                &values, &actual_type_name,
+                                &policy_dn, &attr_free_flags,
+                                &errstr);
+    if (ldrc != LDAP_SUCCESS || policy_dn == NULL) {
         goto done;
-    } else {
-        pdn = slapi_sdn_new_dn_byref(policy_dn);
-        ldrc = slapi_search_internal_get_entry(pdn, NULL, &policy_entry,
-                getPluginID());
-        slapi_sdn_free(&pdn);
-        if (ldrc != LDAP_SUCCESS) {
-            LOG_FATAL("Failed to retrieve entry \"%s\": %d\n", policy_dn, ldrc);
-            errstr = "Failed to retrieve account policy.";
-            ret = LDAP_OPERATIONS_ERROR;
-            goto done;
-        }
     }
 
+    max_fail = slapi_entry_attr_get_uint(policy_entry, "krbPwdMaxFailure");
+    lockout_duration = slapi_entry_attr_get_uint(policy_entry, "krbPwdLockoutDuration");
     failedcount = slapi_entry_attr_get_ulong(target_entry, "krbLoginFailedCount");
+
+    /* Fetch the string value for krbLoginFailedCount to see if the attribute
+     * exists in LDAP at all. We get a 0 if it doesn't exist as a long so we
+     * don't know whether to try delete the existing value later
+     */
+    failedstr = slapi_entry_attr_get_charptr(target_entry, "krbLoginFailedCount");
     failcnt_interval = slapi_entry_attr_get_uint(policy_entry, "krbPwdFailureCountInterval");
     lastfail = slapi_entry_attr_get_charptr(target_entry, "krbLastFailedAuth");
     time_now = time(NULL);
-    if (lastfail != NULL) {
+    if (lastfail != NULL && lockout_duration > 0) {
         struct tm tm;
         int res = 0;
 
@@ -467,7 +517,14 @@ static int ipalockout_postop(Slapi_PBlock *pb)
             tm.tm_year -= 1900;
             tm.tm_mon -= 1;
 
+            if ((failedcount >= max_fail) &&
+                (time_now < timegm(&tm) + lockout_duration)) {
+                /* Within lockout duration */
+                goto done;
+            }
             if (time_now > timegm(&tm) + failcnt_interval) {
+                /* Not within lockout duration, outside of fail interval */
+                old_failedcount = failedcount;
                 failedcount = 0;
             }
         }
@@ -485,22 +542,31 @@ static int ipalockout_postop(Slapi_PBlock *pb)
          * On a successful bind just do a replace and set failurecount to 0.
          */
         if (failed_bind) {
-            PR_snprintf(failedcountstr, sizeof(failedcountstr), "%lu", failedcount);
-            if (!gmtime_r(&(time_now), &utctime)) {
-                errstr = "failed to parse current date (buggy gmtime_r ?)\n";
-                LOG_FATAL("%s", errstr);
-                ret = LDAP_OPERATIONS_ERROR;
-                goto done;
+            if (failedcount < max_fail) {
+                if (old_failedcount > -1) {
+                    PR_snprintf(failedcountstr, sizeof(failedcountstr), "%lu", old_failedcount);
+                } else {
+                    PR_snprintf(failedcountstr, sizeof(failedcountstr), "%lu", failedcount);
+                }
+                if (failedstr != NULL) {
+                    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);
+                if (lastfail) {
+                    slapi_mods_add_string(smods, LDAP_MOD_DELETE, "krbLastFailedAuth", lastfail);
+                }
+                if (!gmtime_r(&(time_now), &utctime)) {
+                    errstr = "failed to parse current date (buggy gmtime_r ?)\n";
+                    LOG_FATAL("%s", errstr);
+                    ret = LDAP_OPERATIONS_ERROR;
+                    goto done;
+                }
+                strftime(timestr, GENERALIZED_TIME_LENGTH+1,
+                     "%Y%m%d%H%M%SZ", &utctime);
+                slapi_mods_add_string(smods, LDAP_MOD_ADD, "krbLastFailedAuth", timestr);
             }
-            strftime(timestr, GENERALIZED_TIME_LENGTH+1,
-                 "%Y%m%d%H%M%SZ", &utctime);
-            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);
-            if (lastfail)
-                slapi_mods_add_string(smods, LDAP_MOD_DELETE, "krbLastFailedAuth", lastfail);
-            slapi_mods_add_string(smods, LDAP_MOD_ADD, "krbLastFailedAuth", timestr);
         } else {
             PR_snprintf(failedcountstr, sizeof(failedcountstr), "%lu", 0L);
             time_now = time(NULL);
@@ -555,20 +621,22 @@ static int ipalockout_postop(Slapi_PBlock *pb)
     } /* while */
 
     if (failure) {
+        LOG_FATAL("Unable to change lockout attributes\n");
         ret = LDAP_OPERATIONS_ERROR;
     }
 
 done:
     if (!failed_bind && dn != NULL) slapi_ch_free_string(&dn);
     slapi_entry_free(target_entry);
-    if (policy_dn) {
-        slapi_ch_free_string(&policy_dn);
-        slapi_entry_free(policy_entry);
+    slapi_entry_free(policy_entry);
+    if (values != NULL) {
+        slapi_vattr_values_free(&values, &actual_type_name, attr_free_flags);
     }
     if (sdn) slapi_sdn_free(&sdn);
     if (lastfail) slapi_ch_free_string(&lastfail);
     if (pbtm) slapi_pblock_destroy(pbtm);
     if (smods) slapi_mods_free(&smods);
+    if (failedstr) slapi_ch_free_string(&failedstr);
 
     LOG("postop returning %d: %s\n", ret, errstr ? errstr : "success\n");
 
@@ -588,7 +656,7 @@ done:
 static int ipalockout_preop(Slapi_PBlock *pb)
 {
     char *dn = NULL;
-    char *policy_dn = NULL;
+    const char *policy_dn = NULL;
     Slapi_Entry *target_entry = NULL;
     Slapi_Entry *policy_entry = NULL;
     Slapi_DN *sdn = NULL;
@@ -605,6 +673,10 @@ static int ipalockout_preop(Slapi_PBlock *pb)
     time_t last_failed = 0;
     char *lastfail = NULL;
     char *unlock_time = NULL;
+    int type_name_disposition = 0;
+    char *actual_type_name = NULL;
+    int attr_free_flags = 0;
+    Slapi_ValueSet *values = NULL;
 
     LOG_TRACE("--in-->\n");
 
@@ -655,27 +727,19 @@ static int ipalockout_preop(Slapi_PBlock *pb)
     }
     slapi_value_free(&objectclass);
 
-    /* Only continue if there is a password policy */
-    policy_dn = slapi_entry_attr_get_charptr(target_entry, "krbPwdPolicyReference");
-    if (policy_dn == NULL) {
-        LOG_TRACE("No kerberos password policy\n");
+    ldrc = ipalockout_getpolicy(target_entry, &policy_entry,
+                                &values, &actual_type_name,
+                                &policy_dn, &attr_free_flags,
+                                &errstr);
+    if (ldrc != LDAP_SUCCESS || policy_dn == NULL) {
         goto done;
-    } else {
-        pdn = slapi_sdn_new_dn_byref(policy_dn);
-        ldrc = slapi_search_internal_get_entry(pdn, NULL, &policy_entry,
-                getPluginID());
-        slapi_sdn_free(&pdn);
-        if (ldrc != LDAP_SUCCESS) {
-            LOG_FATAL("Failed to retrieve entry \"%s\": %d\n", policy_dn, ldrc);
-            errstr = "Failed to retrieve account policy.";
-            ret = LDAP_OPERATIONS_ERROR;
-            goto done;
-        }
     }
 
     failedcount = slapi_entry_attr_get_ulong(target_entry, "krbLoginFailedCount");
     time_now = time(NULL);
     failcnt_interval = slapi_entry_attr_get_uint(policy_entry, "krbPwdFailureCountInterval");
+    lockout_duration = slapi_entry_attr_get_uint(policy_entry, "krbPwdLockoutDuration");
+
     lastfail = slapi_entry_attr_get_charptr(target_entry, "krbLastFailedAuth");
     unlock_time = slapi_entry_attr_get_charptr(target_entry, "krbLastAdminUnlock");
     if (lastfail != NULL) {
@@ -693,35 +757,31 @@ static int ipalockout_preop(Slapi_PBlock *pb)
             tm.tm_mon -= 1;
 
             last_failed = timegm(&tm);
-            LOG("%ld > %ld ?\n",
-                (long)time_now, (long)last_failed + failcnt_interval);
-            LOG("diff %ld\n",
-                (long)((last_failed + failcnt_interval) - time_now));
-            if (time_now > last_failed + failcnt_interval) {
-                failedcount = 0;
-            }
         }
-        if (unlock_time) {
-            time_t unlock;
+    }
 
-            memset(&tm, 0, sizeof(struct tm));
-            res = sscanf(lastfail,
-                         "%04u%02u%02u%02u%02u%02u",
-                         &tm.tm_year, &tm.tm_mon, &tm.tm_mday,
-                         &tm.tm_hour, &tm.tm_min, &tm.tm_sec);
+    if (lastfail != NULL && unlock_time != NULL) {
+        time_t unlock;
+        struct tm tm;
+        int res = 0;
 
-            if (res == 6) {
-                tm.tm_year -= 1900;
-                tm.tm_mon -= 1;
+        memset(&tm, 0, sizeof(struct tm));
+        res = sscanf(unlock_time,
+                     "%04u%02u%02u%02u%02u%02u",
+                     &tm.tm_year, &tm.tm_mon, &tm.tm_mday,
+                     &tm.tm_hour, &tm.tm_min, &tm.tm_sec);
 
-                unlock = timegm(&tm);
-                if (last_failed <= unlock) {
-                    goto done;
-                }
+        if (res == 6) {
+            tm.tm_year -= 1900;
+            tm.tm_mon -= 1;
+
+            unlock = timegm(&tm);
+            if (last_failed <= unlock) {
+                /* Administratively unlocked */
+                goto done;
             }
-            slapi_ch_free_string(&unlock_time);
         }
-        slapi_ch_free_string(&lastfail);
+        slapi_ch_free_string(&unlock_time);
     }
 
     max_fail = slapi_entry_attr_get_uint(policy_entry, "krbPwdMaxFailure");
@@ -729,14 +789,13 @@ static int ipalockout_preop(Slapi_PBlock *pb)
         goto done;
     }
 
-    lockout_duration = slapi_entry_attr_get_uint(policy_entry, "krbPwdLockoutDuration");
-    if (lockout_duration == 0) {
-        errstr = "Entry permanently locked.\n";
-        ret = LDAP_UNWILLING_TO_PERFORM;
-        goto done;
-    }
+    if (failedcount >= max_fail) {
+        if (lockout_duration == 0) {
+            errstr = "Entry permanently locked.\n";
+            ret = LDAP_UNWILLING_TO_PERFORM;
+            goto done;
+        }
 
-    if (failedcount > max_fail) {
         if (time_now < last_failed + lockout_duration) {
             /* Too many failures */
             LOG_TRACE("Too many failed logins. %lu out of %d\n", failedcount, max_fail);
@@ -746,9 +805,12 @@ static int ipalockout_preop(Slapi_PBlock *pb)
     }
 
 done:
+    if (lastfail) slapi_ch_free_string(&lastfail);
     slapi_entry_free(target_entry);
     slapi_entry_free(policy_entry);
-    if (policy_dn) slapi_ch_free_string(&policy_dn);
+    if (values != NULL) {
+        slapi_vattr_values_free(&values, &actual_type_name, attr_free_flags);
+    }
     if (sdn) slapi_sdn_free(&sdn);
 
     LOG("preop returning %d: %s\n", ret, errstr ? errstr : "success\n");
-- 
1.8.1

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to