dsdb: Implement password lockout on LDAP password changes
authorAndrew Bartlett <abartlet@samba.org>
Wed, 6 Nov 2013 04:11:18 +0000 (17:11 +1300)
committerStefan Metzmacher <metze@samba.org>
Wed, 2 Apr 2014 15:12:47 +0000 (17:12 +0200)
To do this, and have the badPwdCount update stick, we must abort,
open, close and reopen transactions such that the badPwdCount update
is in it's own transaction.

To ensure the tests can confirm the correct behaviour here, we must
output the Windows error code in the error message.

Andrew Bartlett

Change-Id: I5b1515b26b308301cf90ce8a3c848a3cedee85a2
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
source4/dsdb/samdb/ldb_modules/password_hash.c

index a4e663ee36dac9740e34c60c2d4d38ece7aa6e78..1dee6af8f39f118bd80fa68ff8c173989b074f5b 100644 (file)
@@ -1872,6 +1872,112 @@ static int setup_password_fields(struct setup_password_fields_io *io)
        return LDB_SUCCESS;
 }
 
+static int make_error_and_update_badPwdCount(struct setup_password_fields_io *io)
+{
+       struct ldb_context *ldb = ldb_module_get_ctx(io->ac->module);
+       struct ldb_message *mod_msg = NULL;
+       NTSTATUS status;
+       int ret;
+
+       status = dsdb_update_bad_pwd_count(io->ac, ldb,
+                                          io->ac->search_res->message,
+                                          io->ac->dom_res->message,
+                                          &mod_msg);
+       if (!NT_STATUS_IS_OK(status)) {
+               goto done;
+       }
+
+       if (mod_msg == NULL) {
+               goto done;
+       }
+
+       /*
+        * OK, horrible semantics ahead.
+        *
+        * - We need to abort any existing transaction
+        * - create a transaction arround the badPwdCount update
+        * - re-open the transaction so the upper layer
+        *   doesn't know what happened.
+        *
+        * This is needed because returning an error to the upper
+        * layer will cancel the transaction and undo the badPwdCount
+        * update.
+        */
+
+       /*
+        * Checking errors here is a bit pointless.
+        * What can we do if we can't end the transaction?
+        */
+       ret = ldb_next_del_trans(io->ac->module);
+       if (ret != LDB_SUCCESS) {
+               ldb_debug(ldb, LDB_DEBUG_FATAL,
+                         "Failed to abort transaction prior to update of badPwdCount of %s: %s",
+                         ldb_dn_get_linearized(io->ac->search_res->message->dn),
+                         ldb_errstring(ldb));
+               /*
+                * just return the original error
+                */
+               goto done;
+       }
+
+       /* Likewise, what should we do if we can't open a new transaction? */
+       ret = ldb_next_start_trans(io->ac->module);
+       if (ret != LDB_SUCCESS) {
+               ldb_debug(ldb, LDB_DEBUG_ERROR,
+                         "Failed to open transaction to update badPwdCount of %s: %s",
+                         ldb_dn_get_linearized(io->ac->search_res->message->dn),
+                         ldb_errstring(ldb));
+               /*
+                * just return the original error
+                */
+               goto done;
+       }
+
+       ret = dsdb_module_modify(io->ac->module, mod_msg,
+                                DSDB_FLAG_NEXT_MODULE,
+                                io->ac->req);
+       if (ret != LDB_SUCCESS) {
+               ldb_debug(ldb, LDB_DEBUG_ERROR,
+                         "Failed to update badPwdCount of %s: %s",
+                         ldb_dn_get_linearized(io->ac->search_res->message->dn),
+                         ldb_errstring(ldb));
+               /*
+                * We can only ignore this...
+                */
+       }
+
+       ret = ldb_next_end_trans(io->ac->module);
+       if (ret != LDB_SUCCESS) {
+               ldb_debug(ldb, LDB_DEBUG_ERROR,
+                         "Failed to close transaction to update badPwdCount of %s: %s",
+                         ldb_dn_get_linearized(io->ac->search_res->message->dn),
+                         ldb_errstring(ldb));
+               /*
+                * We can only ignore this...
+                */
+       }
+
+       ret = ldb_next_start_trans(io->ac->module);
+       if (ret != LDB_SUCCESS) {
+               ldb_debug(ldb, LDB_DEBUG_ERROR,
+                         "Failed to open transaction after update of badPwdCount of %s: %s",
+                         ldb_dn_get_linearized(io->ac->search_res->message->dn),
+                         ldb_errstring(ldb));
+               /*
+                * We can only ignore this...
+                */
+       }
+
+done:
+       ret = LDB_ERR_CONSTRAINT_VIOLATION;
+       ldb_asprintf_errstring(ldb,
+                              "%08X: %s - check_password_restrictions: "
+                              "The old password specified doesn't match!",
+                              W_ERROR_V(WERR_INVALID_PASSWORD),
+                              ldb_strerror(ret));
+       return ret;
+}
+
 static int check_password_restrictions(struct setup_password_fields_io *io)
 {
        struct ldb_context *ldb;
@@ -1896,13 +2002,7 @@ static int check_password_restrictions(struct setup_password_fields_io *io)
                   has no problems at all */
                if (io->og.nt_hash) {
                        if (!io->o.nt_hash || memcmp(io->og.nt_hash->hash, io->o.nt_hash->hash, 16) != 0) {
-                               ret = LDB_ERR_CONSTRAINT_VIOLATION;
-                               ldb_asprintf_errstring(ldb,
-                                       "%08X: %s - check_password_restrictions: "
-                                       "The old password specified doesn't match!",
-                                       W_ERROR_V(WERR_INVALID_PASSWORD),
-                                       ldb_strerror(ret));
-                               return ret;
+                               return make_error_and_update_badPwdCount(io);
                        }
 
                        nt_hash_checked = true;
@@ -1915,13 +2015,7 @@ static int check_password_restrictions(struct setup_password_fields_io *io)
                if (io->og.lm_hash) {
                        if ((!io->o.lm_hash && !nt_hash_checked)
                            || (io->o.lm_hash && memcmp(io->og.lm_hash->hash, io->o.lm_hash->hash, 16) != 0)) {
-                               ret = LDB_ERR_CONSTRAINT_VIOLATION;
-                               ldb_asprintf_errstring(ldb,
-                                       "%08X: %s - check_password_restrictions: "
-                                       "The old password specified doesn't match!",
-                                       W_ERROR_V(WERR_INVALID_PASSWORD),
-                                       ldb_strerror(ret));
-                               return ret;
+                               return make_error_and_update_badPwdCount(io);
                        }
                }
        }
@@ -2641,8 +2735,6 @@ static int get_domain_data_callback(struct ldb_request *req,
                ac->status->domain_data.store_cleartext =
                        ac->status->domain_data.pwdProperties & DOMAIN_PASSWORD_STORE_CLEARTEXT;
 
-               talloc_free(ares);
-
                /* For a domain DN, this puts things in dotted notation */
                /* For builtin domains, this will give details for the host,
                 * but that doesn't really matter, as it's just used for salt
@@ -2657,6 +2749,15 @@ static int get_domain_data_callback(struct ldb_request *req,
 
                ac->status->reject_reason = SAM_PWD_CHANGE_NO_ERROR;
 
+               if (ac->dom_res != NULL) {
+                       talloc_free(ares);
+
+                       ldb_set_errstring(ldb, "Too many results");
+                       ret = LDB_ERR_OPERATIONS_ERROR;
+                       goto done;
+               }
+
+               ac->dom_res = talloc_steal(ac, ares);
                ret = LDB_SUCCESS;
                break;
 
@@ -2724,6 +2825,8 @@ static int build_domain_data_request(struct ph_context *ac)
                                              "maxPwdAge",
                                              "minPwdAge",
                                              "minPwdLength",
+                                             "lockoutThreshold",
+                                             "lockOutObservationWindow",
                                              NULL };
        int ret;
 
@@ -3219,6 +3322,9 @@ static int password_hash_mod_search_self(struct ph_context *ac)
                                              "ntPwdHistory",
                                              "dBCSPwd",
                                              "unicodePwd",
+                                             "badPasswordTime",
+                                             "badPwdCount",
+                                             "lockoutTime",
                                              NULL };
        struct ldb_request *search_req;
        int ret;
@@ -3287,7 +3393,19 @@ static int password_hash_mod_do_mod(struct ph_context *ac)
                                                &io.o.lm_hash, &io.o.nt_hash);
        }
 
+       if (NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)) {
+               ldb_asprintf_errstring(ldb,
+                                      "%08X: check_password: "
+                                      "Password change not permitted, account locked out!",
+                                      W_ERROR_V(WERR_ACCOUNT_LOCKED_OUT));
+               return LDB_ERR_CONSTRAINT_VIOLATION;
+       }
+
        if (!NT_STATUS_IS_OK(status)) {
+               /*
+                * This only happens if the database has gone weird,
+                * not if we are just missing the passwords
+                */
                return ldb_operr(ldb);
        }