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