From 12c4b09fd511eaa0671ccf0b05d8407f97167105 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Matthias=20Dieter=20Walln=C3=B6fer?= Date: Fri, 23 Oct 2009 12:51:47 +0200 Subject: [PATCH] s4:password_hash - Rework unique value checks Windows Server performs the constraint checks in a different way than we do. All testing has been done using "passwords.py". --- source4/dsdb/samdb/ldb_modules/password_hash.c | 120 +++++++++++++++---------- 1 file changed, 71 insertions(+), 49 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c index 52c7a434263..c7f6465e97d 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -1748,23 +1748,6 @@ static int password_hash_add(struct ldb_module *module, struct ldb_request *req) return ldb_next_request(module, req); } - if (userPasswordAttr && (userPasswordAttr->num_values != 1)) { - ldb_set_errstring(ldb, "'userPassword' must have exactly one value!"); - return LDB_ERR_CONSTRAINT_VIOLATION; - } - if (clearTextPasswordAttr && (clearTextPasswordAttr->num_values != 1)) { - ldb_set_errstring(ldb, "'clearTextPassword' must have exactly one value!"); - return LDB_ERR_CONSTRAINT_VIOLATION; - } - if (ntAttr && (ntAttr->num_values != 1)) { - ldb_set_errstring(ldb, "'unicodePwd' must have exactly one value!"); - return LDB_ERR_CONSTRAINT_VIOLATION; - } - if (lmAttr && (lmAttr->num_values != 1)) { - ldb_set_errstring(ldb, "'dBCSPwd' must have exactly one value!"); - return LDB_ERR_CONSTRAINT_VIOLATION; - } - /* Make sure we are performing the password set action on a (for us) * valid object. Those are instances of either "user" and/or * "inetOrgPerson". Otherwise continue with the submodules. */ @@ -1890,8 +1873,10 @@ static int password_hash_modify(struct ldb_module *module, struct ldb_request *r { struct ldb_context *ldb; struct ph_context *ac; - struct ldb_message_element *userPasswordAttr, *clearTextPasswordAttr, - *ntAttr, *lmAttr; + const char *passwordAttrs[] = { "userPassword", "clearTextPassword", + "unicodePwd", "dBCSPwd", NULL }, **l; + unsigned int attr_cnt, del_attr_cnt, add_attr_cnt, rep_attr_cnt; + struct ldb_message_element *passwordAttr; struct ldb_message *msg; struct ldb_request *down_req; int ret; @@ -1925,31 +1910,14 @@ static int password_hash_modify(struct ldb_module *module, struct ldb_request *r * OR 'unicodePwd' OR 'dBCSPwd' we don't need to make any changes. * For password changes/set there should be a 'delete' or a 'modify' * on these attributes. */ - - userPasswordAttr = ldb_msg_find_element(req->op.mod.message, "userPassword"); - clearTextPasswordAttr = ldb_msg_find_element(req->op.mod.message, "clearTextPassword"); - ntAttr = ldb_msg_find_element(req->op.mod.message, "unicodePwd"); - lmAttr = ldb_msg_find_element(req->op.mod.message, "dBCSPwd"); - - if ((!userPasswordAttr) && (!clearTextPasswordAttr) && (!ntAttr) && (!lmAttr)) { - return ldb_next_request(module, req); - } - - if (userPasswordAttr && (userPasswordAttr->num_values != 1)) { - ldb_set_errstring(ldb, "'userPassword' must have exactly one value!"); - return LDB_ERR_CONSTRAINT_VIOLATION; - } - if (clearTextPasswordAttr && (clearTextPasswordAttr->num_values != 1)) { - ldb_set_errstring(ldb, "'clearTextPassword' must have exactly one value!"); - return LDB_ERR_CONSTRAINT_VIOLATION; - } - if (ntAttr && (ntAttr->num_values != 1)) { - ldb_set_errstring(ldb, "'unicodePwd' must have exactly one value!"); - return LDB_ERR_CONSTRAINT_VIOLATION; + attr_cnt = 0; + for (l = passwordAttrs; *l != NULL; l++) { + if (ldb_msg_find_element(req->op.mod.message, *l) != NULL) { + ++attr_cnt; + } } - if (lmAttr && (lmAttr->num_values != 1)) { - ldb_set_errstring(ldb, "'dBCSPwd' must have exactly one value!"); - return LDB_ERR_CONSTRAINT_VIOLATION; + if (attr_cnt == 0) { + return ldb_next_request(module, req); } ac = ph_init_context(module, req); @@ -1965,12 +1933,66 @@ static int password_hash_modify(struct ldb_module *module, struct ldb_request *r return LDB_ERR_OPERATIONS_ERROR; } - /* - remove any modification to the password from the first commit - * we will make the real modification later */ - if (userPasswordAttr) ldb_msg_remove_attr(msg, "userPassword"); - if (clearTextPasswordAttr) ldb_msg_remove_attr(msg, "clearTextPassword"); - if (ntAttr) ldb_msg_remove_attr(msg, "unicodePwd"); - if (lmAttr) ldb_msg_remove_attr(msg, "dBCSPwd"); + /* - check for single-valued password attributes + * (if not return "CONSTRAINT_VIOLATION") + * - check that for a password change operation one add and one delete + * operation exists + * (if not return "CONSTRAINT_VIOLATION" or "UNWILLING_TO_PERFORM") + * - check that a password change and a password set operation cannot + * be mixed + * (if not return "UNWILLING_TO_PERFORM") + * - remove all password attributes modifications from the first change + * operation (anything without the passwords) - we will make the real + * modification later */ + del_attr_cnt = 0; + add_attr_cnt = 0; + rep_attr_cnt = 0; + for (l = passwordAttrs; *l != NULL; l++) { + while ((passwordAttr = ldb_msg_find_element(msg, *l)) != NULL) { + if (passwordAttr->flags == LDB_FLAG_MOD_DELETE) { + ++del_attr_cnt; + } + if (passwordAttr->flags == LDB_FLAG_MOD_ADD) { + ++add_attr_cnt; + } + if (passwordAttr->flags == LDB_FLAG_MOD_REPLACE) { + ++rep_attr_cnt; + } + if ((passwordAttr->num_values != 1) && + (passwordAttr->flags != LDB_FLAG_MOD_REPLACE)) { + talloc_free(ac); + ldb_asprintf_errstring(ldb, + "'%s' attributes must have exactly one value!", + *l); + return LDB_ERR_CONSTRAINT_VIOLATION; + } + ldb_msg_remove_attr(msg, *l); + } + } + if ((del_attr_cnt > 0) && (add_attr_cnt == 0)) { + talloc_free(ac); + ldb_set_errstring(ldb, + "Only the delete action for a password change specified!"); + return LDB_ERR_CONSTRAINT_VIOLATION; + } + if ((del_attr_cnt == 0) && (add_attr_cnt > 0)) { + talloc_free(ac); + ldb_set_errstring(ldb, + "Only the add action for a password change specified!"); + return LDB_ERR_UNWILLING_TO_PERFORM; + } + if ((del_attr_cnt > 1) || (add_attr_cnt > 1)) { + talloc_free(ac); + ldb_set_errstring(ldb, + "Only one delete and one add action for a password change allowed!"); + return LDB_ERR_UNWILLING_TO_PERFORM; + } + if ((rep_attr_cnt > 0) && ((del_attr_cnt > 0) || (add_attr_cnt > 0))) { + talloc_free(ac); + ldb_set_errstring(ldb, + "Either a password change or a password set operation is allowed!"); + return LDB_ERR_UNWILLING_TO_PERFORM; + } /* if there was nothing else to be modified skip to next step */ if (msg->num_elements == 0) { -- 2.11.4.GIT