Blame SOURCES/0058-Ticket-49336-SECURITY-Locked-account-provides-differ.patch

61f723
From 95b39e29361812a62f2e038c89a88d717c82794e Mon Sep 17 00:00:00 2001
61f723
From: William Brown <firstyear@redhat.com>
61f723
Date: Mon, 31 Jul 2017 14:13:49 +1000
61f723
Subject: [PATCH] Ticket 49336 - SECURITY: Locked account provides different
61f723
 return code
61f723
61f723
Bug Description:  The directory server password lockout policy prevents binds
61f723
 from operating once a threshold of failed passwords has been met. During
61f723
 this lockout, if you bind with a successful password, a different error code
61f723
 is returned. This means that an attacker has no ratelimit or penalty during
61f723
 an account lock, and can continue to attempt passwords via bruteforce, using
61f723
 the change in return code to ascertain a sucessful password auth.
61f723
61f723
Fix Description:  Move the account lock check *before* the password bind
61f723
check. If the account is locked, we do not mind disclosing this as the
61f723
attacker will either ignore it (and will not bind anyway), or they will
61f723
be forced to back off as the attack is not working preventing the
61f723
bruteforce.
61f723
61f723
https://pagure.io/389-ds-base/issue/49336
61f723
61f723
Author: wibrown
61f723
61f723
Review by: tbordaz (Thanks!)
61f723
61f723
Signed-off-by: Mark Reynolds <mreynolds@redhat.com>
61f723
---
61f723
 .../suites/password/pwd_lockout_bypass_test.py     | 55 ++++++++++++++++++++++
61f723
 ldap/servers/slapd/bind.c                          | 29 ++++++++----
61f723
 ldap/servers/slapd/pw_verify.c                     | 15 +++---
61f723
 3 files changed, 84 insertions(+), 15 deletions(-)
61f723
 create mode 100644 dirsrvtests/tests/suites/password/pwd_lockout_bypass_test.py
61f723
61f723
diff --git a/dirsrvtests/tests/suites/password/pwd_lockout_bypass_test.py b/dirsrvtests/tests/suites/password/pwd_lockout_bypass_test.py
61f723
new file mode 100644
61f723
index 0000000..e4add72
61f723
--- /dev/null
61f723
+++ b/dirsrvtests/tests/suites/password/pwd_lockout_bypass_test.py
61f723
@@ -0,0 +1,55 @@
61f723
+# --- BEGIN COPYRIGHT BLOCK ---
61f723
+# Copyright (C) 2017 Red Hat, Inc.
61f723
+# All rights reserved.
61f723
+#
61f723
+# License: GPL (version 3 or any later version).
61f723
+# See LICENSE for details.
61f723
+# --- END COPYRIGHT BLOCK ---
61f723
+#
61f723
+import pytest
61f723
+from lib389.tasks import *
61f723
+from lib389.utils import *
61f723
+from lib389.topologies import topology_st
61f723
+from lib389.idm.user import UserAccounts, TEST_USER_PROPERTIES
61f723
+import ldap
61f723
+
61f723
+# The irony of these names is not lost on me.
61f723
+GOOD_PASSWORD = 'password'
61f723
+BAD_PASSWORD = 'aontseunao'
61f723
+
61f723
+logging.getLogger(__name__).setLevel(logging.INFO)
61f723
+log = logging.getLogger(__name__)
61f723
+
61f723
+def test_lockout_bypass(topology_st):
61f723
+    inst = topology_st.standalone
61f723
+
61f723
+    # Configure the lock policy
61f723
+    inst.config.set('passwordMaxFailure', '1')
61f723
+    inst.config.set('passwordLockoutDuration', '99999')
61f723
+    inst.config.set('passwordLockout', 'on')
61f723
+
61f723
+    # Create the account
61f723
+    users = UserAccounts(inst, DEFAULT_SUFFIX)
61f723
+    testuser = users.create(properties=TEST_USER_PROPERTIES)
61f723
+    testuser.set('userPassword', GOOD_PASSWORD)
61f723
+
61f723
+    conn = testuser.bind(GOOD_PASSWORD)
61f723
+    assert conn != None
61f723
+    conn.unbind_s()
61f723
+
61f723
+    # Bind with bad creds twice
61f723
+    # This is the failure.
61f723
+    with pytest.raises(ldap.INVALID_CREDENTIALS):
61f723
+        conn = testuser.bind(BAD_PASSWORD)
61f723
+    # Now we should not be able to ATTEMPT the bind. It doesn't matter that
61f723
+    # we disclose that we have hit the rate limit here, what matters is that
61f723
+    # it exists.
61f723
+    with pytest.raises(ldap.CONSTRAINT_VIOLATION):
61f723
+        conn = testuser.bind(BAD_PASSWORD)
61f723
+
61f723
+    # now bind with good creds
61f723
+    # Should be error 19 still.
61f723
+    with pytest.raises(ldap.CONSTRAINT_VIOLATION):
61f723
+        conn = testuser.bind(GOOD_PASSWORD)
61f723
+
61f723
+
61f723
diff --git a/ldap/servers/slapd/bind.c b/ldap/servers/slapd/bind.c
61f723
index 7f4414f..064ace1 100644
61f723
--- a/ldap/servers/slapd/bind.c
61f723
+++ b/ldap/servers/slapd/bind.c
61f723
@@ -662,12 +662,14 @@ do_bind( Slapi_PBlock *pb )
61f723
         /* We could be serving multiple database backends.  Select the appropriate one */
61f723
         /* pw_verify_be_dn will select the backend we need for us. */
61f723
 
61f723
-        if (auto_bind) {
61f723
-            /* We have no password material. We should just check who we are binding as. */
61f723
-            rc = pw_validate_be_dn(pb, &referral);
61f723
-        } else {
61f723
-            rc = pw_verify_be_dn(pb, &referral);
61f723
-        }
61f723
+        /*
61f723
+         * WARNING: We have to validate *all* other conditions *first* before
61f723
+         * we attempt the bind!
61f723
+         *
61f723
+         * this is because ldbm_bind.c will SEND THE FAILURE.
61f723
+         */
61f723
+
61f723
+        rc = pw_validate_be_dn(pb, &referral);
61f723
 
61f723
         if (rc == SLAPI_BIND_NO_BACKEND) {
61f723
             send_nobackend_ldap_result( pb );
61f723
@@ -736,8 +738,18 @@ do_bind( Slapi_PBlock *pb )
61f723
                     myrc = 0;
61f723
                 }
61f723
                 if (!auto_bind) {
61f723
-                    /* 
61f723
-                     * There could be a race that bind_target_entry was not added 
61f723
+                    /*
61f723
+                     * Okay, we've made it here. FINALLY check if the entry really
61f723
+                     * can bind or not. THIS IS THE PASSWORD CHECK.
61f723
+                     */
61f723
+                    rc = pw_verify_be_dn(pb, &referral);
61f723
+                    if (rc != SLAPI_BIND_SUCCESS) {
61f723
+                        /* Invalid pass - lets bail ... */
61f723
+                        goto bind_failed;
61f723
+                    }
61f723
+
61f723
+                    /*
61f723
+                     * There could be a race that bind_target_entry was not added
61f723
                      * when bind_target_entry was retrieved before be_bind, but it
61f723
                      * was in be_bind.  Since be_bind returned SLAPI_BIND_SUCCESS,
61f723
                      * the entry is in the DS.  So, we need to retrieve it once more.
61f723
@@ -786,6 +798,7 @@ do_bind( Slapi_PBlock *pb )
61f723
                 }
61f723
             }
61f723
         } else { /* if auto_bind || rc == slapi_bind_success | slapi_bind_anonymous */
61f723
+        bind_failed:
61f723
             if (rc == LDAP_OPERATIONS_ERROR) {
61f723
                 send_ldap_result( pb, LDAP_UNWILLING_TO_PERFORM, NULL, "Function not implemented", 0, NULL );
61f723
                 goto free_and_return;
61f723
diff --git a/ldap/servers/slapd/pw_verify.c b/ldap/servers/slapd/pw_verify.c
61f723
index 852b027..cb182ed 100644
61f723
--- a/ldap/servers/slapd/pw_verify.c
61f723
+++ b/ldap/servers/slapd/pw_verify.c
61f723
@@ -55,7 +55,7 @@ pw_verify_root_dn(const char *dn, const Slapi_Value *cred)
61f723
 int
61f723
 pw_verify_be_dn(Slapi_PBlock *pb, Slapi_Entry **referral)
61f723
 {
61f723
-    int rc = 0;
61f723
+    int rc = SLAPI_BIND_SUCCESS;
61f723
     Slapi_Backend *be = NULL;
61f723
 
61f723
     if (slapi_mapping_tree_select(pb, &be, referral, NULL, 0) != LDAP_SUCCESS) {
61f723
@@ -109,14 +109,10 @@ pw_validate_be_dn(Slapi_PBlock *pb, Slapi_Entry **referral)
61f723
     slapi_pblock_get(pb, SLAPI_BIND_CREDENTIALS, &cred);
61f723
     slapi_pblock_get(pb, SLAPI_BIND_METHOD, &method);
61f723
 
61f723
-    if (pb_sdn != NULL || cred != NULL) {
61f723
+    if (pb_sdn == NULL) {
61f723
         return LDAP_OPERATIONS_ERROR;
61f723
     }
61f723
 
61f723
-    if (*referral) {
61f723
-        return SLAPI_BIND_REFERRAL;
61f723
-    }
61f723
-
61f723
     /* We need a slapi_sdn_isanon? */
61f723
     if (method == LDAP_AUTH_SIMPLE && cred->bv_len == 0) {
61f723
         return SLAPI_BIND_ANONYMOUS;
61f723
@@ -130,7 +126,11 @@ pw_validate_be_dn(Slapi_PBlock *pb, Slapi_Entry **referral)
61f723
     if (slapi_mapping_tree_select(pb, &be, referral, NULL, 0) != LDAP_SUCCESS) {
61f723
         return SLAPI_BIND_NO_BACKEND;
61f723
     }
61f723
-    slapi_be_Unlock(be);
61f723
+
61f723
+    if (*referral) {
61f723
+        slapi_be_Unlock(be);
61f723
+        return SLAPI_BIND_REFERRAL;
61f723
+    }
61f723
 
61f723
     slapi_pblock_set(pb, SLAPI_BACKEND, be);
61f723
     slapi_pblock_set(pb, SLAPI_PLUGIN, be->be_database);
61f723
@@ -138,6 +138,7 @@ pw_validate_be_dn(Slapi_PBlock *pb, Slapi_Entry **referral)
61f723
     set_db_default_result_handlers(pb);
61f723
 
61f723
     /* The backend associated with this identity is real. */
61f723
+    slapi_be_Unlock(be);
61f723
 
61f723
     return SLAPI_BIND_SUCCESS;
61f723
 }
61f723
-- 
61f723
2.9.4
61f723