svn rev #24061: branches/krb5-1-8/src/plugins/kdb/ldap/libkdb_ldap/

[tlyu@MIT.EDU]

http://src.mit.edu/fisheye/changelog/krb5/?cs=24061
Commit By: tlyu
Log Message:
ticket: 6718
version_fixed: 1.8.2
status: resolved

pull up r24002 from trunk

 ------------------------------------------------------------------------
 r24002 | ghudson | 2010-05-10 18:23:57 -0400 (Mon, 10 May 2010) | 14 lines

 ticket: 6718
 subject: Make KADM5_FAIL_AUTH_COUNT_INCREMENT more robust with LDAP
 target_version: 1.8.2
 tags: pullup

 In krb5_ldap_put_principal, use krb5_get_attributes_mask to determine
 whether krbLoginFailedCount existed on the entry when it was
 retrieved.  If it didn't exist, don't try to use LDAP_MOD_INCREMENT,
 and don't assert an old value when not using LDAP_MOD_INCREMENT.

 Also, create the krbLoginFailedCount attribute when creating new
 entries.  This allows us to use LDAP_MOD_INCREMENT during the first
 failed login (if the server supports it), avoiding a race condition.


Changed Files:
U   branches/krb5-1-8/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
Modified: branches/krb5-1-8/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
===================================================================
--- branches/krb5-1-8/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c	2010-05-19 18:52:43 UTC (rev 24060)
+++ branches/krb5-1-8/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c	2010-05-19 18:52:46 UTC (rev 24061)
@@ -891,6 +891,16 @@
             if (st != 0)
                 goto cleanup;
         } else if (entries->mask & KADM5_FAIL_AUTH_COUNT_INCREMENT) {
+            int attr_mask = 0;
+            krb5_boolean has_fail_count;
+
+            /* Check if the krbLoginFailedCount attribute exists.  (Through
+             * krb5 1.8.1, it wasn't set in new entries.) */
+            st = krb5_get_attributes_mask(context, entries, &attr_mask);
+            if (st != 0)
+                goto cleanup;
+            has_fail_count = ((attr_mask & KDB_FAIL_AUTH_COUNT_ATTR) != 0);
+
             /*
              * If the client library and server supports RFC 4525,
              * then use it to increment by one the value of the
@@ -898,38 +908,35 @@
              * (provided) old value by deleting it before adding.
              */
 #ifdef LDAP_MOD_INCREMENT
-            if (ldap_server_handle->server_info->modify_increment) {
+            if (ldap_server_handle->server_info->modify_increment &&
+                has_fail_count) {
                 st = krb5_add_int_mem_ldap_mod(&mods, "krbLoginFailedCount",
                                                LDAP_MOD_INCREMENT, 1);
                 if (st != 0)
                     goto cleanup;
-            } else
+            } else {
 #endif /* LDAP_MOD_INCREMENT */
-                if (entries->fail_auth_count == 0) {
-                    /*
-                     * Unfortunately we have no way of distinguishing between
-                     * an absent and a zero-valued attribute by the time we are
-                     * called here. So, although this creates a race condition,
-                     * it appears impossible to assert the old value as that
-                     * would fail were the attribute absent.
-                     */
-                    st = krb5_add_int_mem_ldap_mod(&mods, "krbLoginFailedCount",
-                                                   LDAP_MOD_REPLACE, 1);
-                    if (st != 0)
-                        goto cleanup;
-                } else {
-                    st = krb5_add_int_mem_ldap_mod(&mods, "krbLoginFailedCount",
+                if (has_fail_count) {
+                    st = krb5_add_int_mem_ldap_mod(&mods,
+                                                   "krbLoginFailedCount",
                                                    LDAP_MOD_DELETE,
                                                    entries->fail_auth_count);
                     if (st != 0)
                         goto cleanup;
-
-                    st = krb5_add_int_mem_ldap_mod(&mods, "krbLoginFailedCount",
-                                                   LDAP_MOD_ADD,
-                                                   entries->fail_auth_count + 1);
-                    if (st != 0)
-                        goto cleanup;
                 }
+                st = krb5_add_int_mem_ldap_mod(&mods, "krbLoginFailedCount",
+                                               LDAP_MOD_ADD,
+                                               entries->fail_auth_count + 1);
+                if (st != 0)
+                    goto cleanup;
+#ifdef LDAP_MOD_INCREMENT
+            }
+#endif
+        } else if (optype == ADD_PRINCIPAL) {
+            /* Initialize krbLoginFailedCount in new entries to help avoid a
+             * race during the first failed login. */
+            st = krb5_add_int_mem_ldap_mod(&mods, "krbLoginFailedCount",
+                                           LDAP_MOD_ADD, 0);
         }
 
         if (entries->mask & KADM5_MAX_LIFE) {