r12599: This new LDB module (and associated changes) allows Samba4 to operate

using pre-calculated passwords for all kerberos key types.
(Previously we could only use these for the NT# type).

The module handles all of the hash/string2key tasks for all parts of
Samba, which was previously in the rpc_server/samr/samr_password.c
code.  We also update the msDS-KeyVersionNumber, and the password
history.  This new module can be called at provision time, which
ensures we start with a database that is consistent in this respect.

By ensuring that the krb5key attribute is the only one we need to
retrieve, this also simplifies the run-time KDC logic.  (Each value of
the multi-valued attribute is encoded as a 'Key' in ASN.1, using the
definition from Heimdal's HDB.  This simplfies the KDC code.).

It is hoped that this will speed up the KDC enough that it can again
operate under valgrind.
(This used to be commit e9022743210b59f19f370d772e532e0f08bfebd9)

3 files changed, 748 insertions, 63 deletions

diff --git a/source4/dsdb/samdb/ldb_modules/config.mk b/source4/dsdb/samdb/ldb_modules/config.mk
index 3d20bff809..f9c267e2db 100644
--- a/source4/dsdb/samdb/ldb_modules/config.mk
+++ b/source4/dsdb/samdb/ldb_modules/config.mk
@@ -50,3 +50,15 @@ OBJ_FILES = \
 # End MODULE libldb_rootdse
 ################################################
 
+################################################
+# Start MODULE libldb_password_hash
+[MODULE::libldb_password_hash]
+SUBSYSTEM = LIBLDB
+OBJ_FILES = \
+		password_hash.o
+REQUIRED_SUBSYSTEMS = \
+		HEIMDAL_HDB HEIMDAL_KRB5
+#
+# End MODULE libldb_rootdse
+################################################
+
diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c
new file mode 100644
index 0000000000..bb42a0e634
--- /dev/null
+++ b/source4/dsdb/samdb/ldb_modules/password_hash.c
@@ -0,0 +1,711 @@
+/* 
+   ldb database module
+
+   Copyright (C) Simo Sorce  2004
+   Copyright (C) Andrew Bartlett <abartlet@samba.org> 2005
+   Copyright (C) Andrew Tridgell 2004
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+   
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+   
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+/*
+ *  Name: ldb
+ *
+ *  Component: ldb password_hash module
+ *
+ *  Description: correctly update hash values based on changes to unicodePwd and friends
+ *
+ *  Author: Andrew Bartlett
+ */
+
+#include "includes.h"
+#include "libcli/ldap/ldap.h"
+#include "ldb/include/ldb_errors.h"
+#include "ldb/include/ldb_private.h"
+#include "librpc/gen_ndr/ndr_misc.h"
+#include "librpc/gen_ndr/ndr_samr.h"
+#include "system/kerberos.h"
+#include "auth/kerberos/kerberos.h"
+#include "system/time.h"
+#include "dsdb/samdb/samdb.h"
+#include "ads.h"
+#include "hdb.h"
+
+/* If we have decided there is reason to work on this request, then
+ * setup all the password hash types correctly.
+ *
+ * If the administrator doesn't want the unicodePwd stored (set in the
+ * domain and per-account policies) then we must strip that out before
+ * we do the first operation.
+ *
+ * Once this is done (which could update anything at all), we
+ * calculate the password hashes.
+ *
+ * This function must not only update the ntPwdHash, lmPwdHash and
+ * krb5Key fields, it must also atomicly increment the
+ * msDS-KeyVersionNumber.  We should be in a transaction, so all this
+ * should be quite safe...
+ *
+ * Finally, if the administrator has requested that a password history
+ * be maintained, then this should also be written out.
+ *
+ */
+
+
+static int password_hash_handle(struct ldb_module *module, struct ldb_request *req, 
+			     const struct ldb_message *msg)
+{
+	int ret, old_ret = -1;
+	uint_t pwdProperties, pwdHistoryLength;
+	uint_t userAccountControl;
+	const char *dnsDomain, *realm;
+	const char *unicodePwd;
+	struct samr_Password *lmPwdHistory, *ntPwdHistory;
+	struct samr_Password *lmPwdHash, *ntPwdHash;
+	struct samr_Password *lmOldHash = NULL, *ntOldHash = NULL;
+	struct samr_Password *new_lmPwdHistory, *new_ntPwdHistory;
+	struct samr_Password local_lmNewHash, local_ntNewHash;
+	int lmPwdHistory_len, ntPwdHistory_len;
+	uint_t kvno;
+	struct dom_sid *domain_sid;
+	time_t now = time(NULL);
+	NTTIME now_nt;
+	int i;
+	krb5_error_code krb5_ret;
+
+	struct smb_krb5_context *smb_krb5_context;
+
+	struct ldb_message_element *attribute;
+	struct ldb_dn *dn = msg->dn;
+	struct ldb_message *msg2;
+
+	struct ldb_request search_request;
+	struct ldb_request modify_request;
+	struct ldb_request modified_orig_request;
+	struct ldb_result *res, *dom_res, *old_res;
+
+	struct ldb_message_element *objectclasses;
+	struct ldb_val computer_val;
+	struct ldb_val person_val;
+	BOOL is_computer;
+
+	struct ldb_message *modify_msg;
+
+	const char *domain_expression;
+	const char *old_user_attrs[] = { "lmPwdHash", "ntPwdHash", NULL };
+	const char *user_attrs[] = { "userAccountControl", "lmPwdHistory", 
+				     "ntPwdHistory", 
+				     "ntPwdHash", 
+				     "objectSid", "msDS-KeyVersionNumber", 
+				     "objectClass", "userPrincipalName",
+				     "samAccountName", 
+				     NULL };
+	const char * const domain_attrs[] = { "pwdProperties", "pwdHistoryLength", 
+					      "dnsDomain", NULL };
+
+	TALLOC_CTX *mem_ctx;
+
+	/* Do the original action */
+	
+	/* If no part of this touches the unicodePwd, then we don't
+	 * need to make any changes.  For password changes/set there should
+	 * be a 'delete' or a 'modify' on this attribute. */
+	if ((attribute = ldb_msg_find_element(msg, "unicodePwd")) == NULL ) {
+		return ldb_next_request(module, req);
+	}
+
+	mem_ctx = talloc_new(module);
+	if (!mem_ctx) {
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
+	if (req->operation == LDB_REQ_MODIFY) {
+		/* Look up the old ntPwdHash and lmPwdHash values, so
+		 * we can later place these into the password
+		 * history */
+
+		search_request.operation = LDB_REQ_SEARCH;
+		search_request.op.search.base = dn;
+		search_request.op.search.scope = LDB_SCOPE_BASE;
+		search_request.op.search.tree = ldb_parse_tree(module->ldb, NULL);
+		search_request.op.search.attrs = old_user_attrs;
+		
+		old_ret = ldb_next_request(module, &search_request);
+	}
+
+	/* we can't change things untill we copy it */
+	msg2 = ldb_msg_copy_shallow(mem_ctx, msg);
+
+	/* look again, this time at the copied attribute */
+	if (!msg2 || (attribute = ldb_msg_find_element(msg2, "unicodePwd")) == NULL ) {
+		/* Gah?  where did it go?  Oh well... */
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
+	/* Wipe out the unicodePwd attribute set, we will handle it in
+	 * the second modify.  We might not want it written to disk */
+	
+	if (req->operation == LDB_REQ_ADD) {
+		if (attribute->num_values != 1) {
+			ldb_set_errstring(module, 
+					  talloc_asprintf(mem_ctx, "unicodePwd_handle: "
+							  "attempted set of multiple unicodePwd attributes on %s rejected",
+							  ldb_dn_linearize(mem_ctx, dn)));
+			return LDB_ERR_CONSTRAINT_VIOLAION;
+		}
+	
+		unicodePwd = (const char *)attribute->values[0].data;
+		ldb_msg_remove_attr(msg2, "unicodePwd");
+	} else if (((attribute->flags & LDB_FLAG_MOD_MASK) == LDB_FLAG_MOD_ADD)
+		   || ((attribute->flags & LDB_FLAG_MOD_MASK) == LDB_FLAG_MOD_REPLACE)) {
+		if (attribute->num_values != 1) {
+			return LDB_ERR_CONSTRAINT_VIOLAION;
+		}
+		
+		unicodePwd = (const char *)attribute->values[0].data;
+		ldb_msg_remove_attr(msg2, "unicodePwd");
+	} else {
+		unicodePwd = NULL;
+	}
+
+	modified_orig_request = *req;
+	switch (modified_orig_request.operation) {
+	case LDB_REQ_ADD:
+		modified_orig_request.op.add.message = msg2;
+		break;
+	case LDB_REQ_MODIFY:
+		modified_orig_request.op.mod.message = msg2;
+		break;
+	}
+
+	/* Send the (modified) request of the original caller down to the database */
+	ret = ldb_next_request(module, &modified_orig_request);
+	if (ret) {
+		return ret;
+	}
+
+	/* While we do the search first (for the old password hashes),
+	 * we don't want to override any error that the modify may
+	 * have returned.  Now check the error */
+	if (req->operation == LDB_REQ_MODIFY) {
+		if (old_ret) {
+			talloc_free(mem_ctx);
+			return old_ret;
+		}
+
+		/* Find out the old passwords details of the user */
+		old_res = search_request.op.search.res;
+		
+		if (old_res->count != 1) {
+			ldb_set_errstring(module, 
+					  talloc_asprintf(mem_ctx, "password_hash_handle: "
+							  "(pre) search for %s found %d != 1 objects, for entry we just modified",
+							  ldb_dn_linearize(mem_ctx, dn),
+							  old_res->count));
+			/* What happend?  The above add/modify worked... */
+			talloc_free(mem_ctx);
+			return LDB_ERR_NO_SUCH_OBJECT;
+		}
+
+		lmOldHash = samdb_result_hash(mem_ctx, old_res->msgs[0],   "lmPwdHash");
+		ntOldHash = samdb_result_hash(mem_ctx, old_res->msgs[0],   "ntPwdHash");
+	}
+
+	/* Start finding out details we need for the second modify.
+	 * We do this after the first add/modify because other modules
+	 * will have filled in the templates, and we may have had
+	 * things like the username (affecting the salt) changed along
+	 * with the password. */
+
+	/* Now find out what is on the entry after the above add/modify */
+	search_request.operation       = LDB_REQ_SEARCH;
+	search_request.op.search.base  = dn;
+	search_request.op.search.scope = LDB_SCOPE_BASE;
+	search_request.op.search.tree  = ldb_parse_tree(module->ldb, NULL);
+	search_request.op.search.attrs = user_attrs;
+	
+	ret = ldb_next_request(module, &search_request);
+	if (ret) {
+		talloc_free(mem_ctx);
+		return ret;
+	}
+
+	/* Find out the full details of the user */
+	res = search_request.op.search.res;
+	if (res->count != 1) {
+		ldb_set_errstring(module, 
+				  talloc_asprintf(mem_ctx, "password_hash_handle: "
+						  "search for %s found %d != 1 objects, for entry we just added/modified",
+						  ldb_dn_linearize(mem_ctx, dn),
+						  res->count));
+		/* What happend?  The above add/modify worked... */
+		talloc_free(mem_ctx);
+		return LDB_ERR_NO_SUCH_OBJECT;
+	}
+
+	userAccountControl = samdb_result_uint(res->msgs[0],   "userAccountControl", 0);
+	lmPwdHistory_len   = samdb_result_hashes(mem_ctx, res->msgs[0], 
+						 "lmPwdHistory", &lmPwdHistory);
+	ntPwdHistory_len   = samdb_result_hashes(mem_ctx, res->msgs[0], 
+						 "ntPwdHistory", &ntPwdHistory);
+	ntPwdHash          = samdb_result_hash(mem_ctx, res->msgs[0],   "ntPwdHash");
+	kvno               = samdb_result_uint(res->msgs[0],   "msDS-KeyVersionNumber", 0);
+
+	domain_sid         = samdb_result_sid_prefix(mem_ctx, res->msgs[0], "objectSid");
+
+	
+	objectclasses = ldb_msg_find_element(res->msgs[0], "objectClass");
+	person_val = data_blob_string_const("person");
+	
+	if (!objectclasses || !ldb_msg_find_val(objectclasses, &person_val)) {
+		/* Not a 'person', so the rest of this doesn't make
+		 * sense.  How we got a unicodePwd this far I don't
+		 * know... */
+		ldb_set_errstring(module, 
+				  talloc_asprintf(mem_ctx, "password_hash_handle: "
+						  "attempted set of unicodePwd on non-'person' object %s rejected",
+						  ldb_dn_linearize(mem_ctx, dn)));
+		talloc_free(mem_ctx);
+		return LDB_ERR_CONSTRAINT_VIOLAION;
+	}
+
+	computer_val = data_blob_string_const("computer");
+	
+	if (ldb_msg_find_val(objectclasses, &computer_val)) {
+		is_computer = True;
+	} else {
+		is_computer = False;
+	}
+	
+	domain_expression  = talloc_asprintf(mem_ctx, "(&(objectSid=%s)(objectClass=domain))", 
+					     ldap_encode_ndr_dom_sid(mem_ctx, domain_sid));
+
+	/* Find the user's domain, then find out the domain password
+	 * properties */
+	ret = ldb_search(module->ldb, NULL, LDB_SCOPE_SUBTREE, domain_expression, 
+			 domain_attrs, &dom_res);
+	if (ret) {
+		talloc_free(mem_ctx);
+		return ret;
+	}
+
+	if (dom_res->count != 1) {
+		/* What happend?  The user we are modifying must be odd... */
+		ldb_set_errstring(module, 
+				  talloc_asprintf(mem_ctx, "password_hash_handle: "
+						  "search for domain %s found %d != 1 objects",
+						  dom_sid_string(mem_ctx, domain_sid),
+						  dom_res->count));
+		talloc_free(mem_ctx);
+		return LDB_ERR_NO_SUCH_OBJECT;
+	}
+
+	pwdProperties    = samdb_result_uint(dom_res->msgs[0],   "pwdProperties", 0);
+	pwdHistoryLength = samdb_result_uint(dom_res->msgs[0],   "pwdHistoryLength", 0);
+	dnsDomain        = ldb_msg_find_string(dom_res->msgs[0], "dnsDomain", NULL);
+	realm            = strupper_talloc(mem_ctx, dnsDomain);
+
+	/* Some operations below require kerberos contexts */
+	if (smb_krb5_init_context(mem_ctx, &smb_krb5_context) != 0) {
+		talloc_free(mem_ctx);
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
+	/* Prepare the modifications to set all the hash/key types */
+	modify_msg = ldb_msg_new(req);
+	modify_msg->dn = talloc_reference(modify_msg, dn);
+
+#define CHECK_RET(x) \
+	do {					\
+		int check_ret = x;		\
+		if (check_ret != LDB_SUCCESS) { \
+			talloc_free(mem_ctx);	\
+			return check_ret;	\
+		}				\
+	} while(0)
+
+	/* Setup krb5Key (we want to either delete an existing value,
+	 * or replace with a new one).  Both the unicode and NT hash
+	 * only branches append keys to this multivalued entry. */
+	CHECK_RET(ldb_msg_add_empty(modify_msg, "krb5Key", LDB_FLAG_MOD_REPLACE));
+	/* Yay, we can compute new password hashes from the unicode
+	 * password */
+	if (unicodePwd) {
+		Principal *salt_principal;
+		const char *user_principal_name = ldb_msg_find_string(res->msgs[0], "userPrincipalName", NULL);
+		
+		Key *keys;
+		size_t num_keys;
+
+		/* compute the new nt and lm hashes */
+		if (E_deshash(unicodePwd, local_lmNewHash.hash)) {
+			lmPwdHash = &local_lmNewHash;
+		} else {
+			lmPwdHash = NULL;
+		}
+		E_md4hash(unicodePwd, local_ntNewHash.hash);
+		ntPwdHash = &local_ntNewHash;
+		CHECK_RET(ldb_msg_add_empty(modify_msg, "ntPwdHash", 
+					    LDB_FLAG_MOD_REPLACE));
+		CHECK_RET(samdb_msg_add_hash(module->ldb, req, 
+					     modify_msg, "ntPwdHash", 
+					     ntPwdHash));
+		CHECK_RET(ldb_msg_add_empty(modify_msg, "lmPwdHash", 
+					    LDB_FLAG_MOD_REPLACE));
+		if (lmPwdHash) {
+			CHECK_RET(samdb_msg_add_hash(module->ldb, req, 
+						     modify_msg, "lmPwdHash", 
+						     lmPwdHash));
+		}
+
+		/* Many, many thanks to lukeh@padl.com for this
+		 * algorithm, described in his Nov 10 2004 mail to
+		 * samba-technical@samba.org */
+
+		if (is_computer) {
+			/* Determine a salting principal */
+			char *samAccountName = talloc_strdup(mem_ctx, ldb_msg_find_string(res->msgs[0], "samAccountName", NULL));
+			char *saltbody;
+			if (!samAccountName) {
+				ldb_set_errstring(module, 
+						  talloc_asprintf(mem_ctx, "password_hash_handle: "
+								  "generation of new kerberos keys failed: %s is a computer without a samAccountName",
+								  ldb_dn_linearize(mem_ctx, dn)));
+				talloc_free(mem_ctx);
+				return LDB_ERR_OPERATIONS_ERROR;
+			}
+			if (samAccountName[strlen(samAccountName)-1] == '$') {
+				samAccountName[strlen(samAccountName)-1] = '\0';
+			}
+			saltbody = talloc_asprintf(mem_ctx, "%s.%s", samAccountName, dnsDomain);
+			
+			krb5_ret = krb5_make_principal(smb_krb5_context->krb5_context, &salt_principal, realm, "host", saltbody, NULL);
+		} else if (user_principal_name) {
+			char *p;
+			user_principal_name = talloc_strdup(mem_ctx, user_principal_name);
+			if (!user_principal_name) {
+				talloc_free(mem_ctx);
+				return LDB_ERR_OPERATIONS_ERROR;
+			} else {
+				p = strchr(user_principal_name, '@');
+				if (p) {
+					p[0] = '\0';
+				}
+				krb5_ret = krb5_make_principal(smb_krb5_context->krb5_context, &salt_principal, realm, user_principal_name, NULL);
+			} 
+		} else {
+			const char *samAccountName = ldb_msg_find_string(res->msgs[0], "samAccountName", NULL);
+			if (!samAccountName) {
+				ldb_set_errstring(module, 
+						  talloc_asprintf(mem_ctx, "password_hash_handle: "
+								  "generation of new kerberos keys failed: %s has no samAccountName",
+								  ldb_dn_linearize(mem_ctx, dn)));
+				talloc_free(mem_ctx);
+				return LDB_ERR_OPERATIONS_ERROR;
+			}
+			krb5_ret = krb5_make_principal(smb_krb5_context->krb5_context, &salt_principal, realm, samAccountName, NULL);
+		}
+
+
+		if (krb5_ret) {
+			ldb_set_errstring(module, 
+					  talloc_asprintf(mem_ctx, "password_hash_handle: "
+							  "generation of a saltking principal failed: %s",
+							  smb_get_krb5_error_message(smb_krb5_context->krb5_context, 
+										     krb5_ret, mem_ctx)));
+			talloc_free(mem_ctx);
+			return LDB_ERR_OPERATIONS_ERROR;
+		}
+
+		/* TODO: We may wish to control the encryption types chosen in future */
+		krb5_ret = hdb_generate_key_set_password(smb_krb5_context->krb5_context,
+						    salt_principal, unicodePwd, &keys, &num_keys);
+		krb5_free_principal(smb_krb5_context->krb5_context, salt_principal);
+
+		if (krb5_ret) {
+			ldb_set_errstring(module, 
+					  talloc_asprintf(mem_ctx, "password_hash_handle: "
+							  "generation of new kerberos keys failed: %s",
+							  smb_get_krb5_error_message(smb_krb5_context->krb5_context, 
+										     krb5_ret, mem_ctx)));
+			talloc_free(mem_ctx);
+			return LDB_ERR_OPERATIONS_ERROR;
+		}
+
+		/* Walking 
+		 */
+		for (i=0; i < num_keys; i++) {
+			unsigned char *buf;
+			size_t buf_size;
+			size_t len;
+			struct ldb_val val;
+			
+			if (keys[i].key.keytype == ENCTYPE_ARCFOUR_HMAC) {
+				/* We might end up doing this below:
+				 * This ensures we get the unicode
+				 * conversion right.  This should also
+				 * be fixed in the Heimdal libs */
+				continue;
+			}
+			ASN1_MALLOC_ENCODE(Key, buf, buf_size, &keys[i], &len, krb5_ret);
+			
+			val.data = talloc_memdup(req, buf, len);
+			val.length = len;
+			free(buf);
+			if (!val.data || krb5_ret) {
+				hdb_free_keys (smb_krb5_context->krb5_context, num_keys, keys);
+				talloc_free(mem_ctx);
+				return LDB_ERR_OPERATIONS_ERROR;
+			}
+			ret = ldb_msg_add_value(modify_msg, "krb5Key", &val);
+			if (ret != LDB_SUCCESS) {
+				hdb_free_keys (smb_krb5_context->krb5_context, num_keys, keys);
+				talloc_free(mem_ctx);
+				return ret;
+			}
+		}
+		
+		hdb_free_keys (smb_krb5_context->krb5_context, num_keys, keys);
+	}
+
+	/* Possibly kill off the cleartext or store it */
+	CHECK_RET(ldb_msg_add_empty(modify_msg, "unicodePwd", LDB_FLAG_MOD_REPLACE));
+
+	if (unicodePwd && (pwdProperties & DOMAIN_PASSWORD_STORE_CLEARTEXT) &&
+	    (userAccountControl & UF_ENCRYPTED_TEXT_PASSWORD_ALLOWED)) {
+		CHECK_RET(ldb_msg_add_string(modify_msg, "unicodePwd", unicodePwd));
+	}
+	
+	/* Even if we didn't get a unicodePwd, we can still setup
+	 * krb5Key from the NT hash. 
+	 *
+	 * This is an append, so it works with the 'continue' in the
+	 * unicode loop above, to use Samba's NT hash function, which
+	 * is more correct than Heimdal's
+	 */
+	if (ntPwdHash) {
+		unsigned char *buf;
+		size_t buf_size;
+		size_t len;
+		struct ldb_val val;
+		Key key;
+		
+		key.mkvno = 0;
+		key.salt = NULL; /* No salt for this enc type */
+
+		krb5_ret = krb5_keyblock_init(smb_krb5_context->krb5_context,
+					 ENCTYPE_ARCFOUR_HMAC,
+					 ntPwdHash->hash, sizeof(ntPwdHash->hash), 
+					 &key.key);
+		if (krb5_ret) {
+			return LDB_ERR_OPERATIONS_ERROR;
+		}
+		ASN1_MALLOC_ENCODE(Key, buf, buf_size, &key, &len, krb5_ret);
+		krb5_free_keyblock_contents(smb_krb5_context->krb5_context,
+					    &key.key);
+		
+		val.data = talloc_memdup(req, buf, len);
+		val.length = len;
+		free(buf);
+		if (!val.data || ret) {
+			return LDB_ERR_OPERATIONS_ERROR;
+		}
+		CHECK_RET(ldb_msg_add_value(modify_msg, "krb5Key", &val));
+	}
+
+	/* If the original caller did anything with pwdLastSet then skip this.  It could be an incoming samsync */
+	if ((attribute = ldb_msg_find_element(msg, "pwdLastSet")) == NULL ) {
+		/* Update the password last set time */
+		unix_to_nt_time(&now_nt, now);
+		CHECK_RET(ldb_msg_add_empty(modify_msg, "pwdLastSet", LDB_FLAG_MOD_REPLACE));
+		CHECK_RET(samdb_msg_add_uint64(module->ldb, mem_ctx, modify_msg, "pwdLastSet", now_nt));
+	}
+
+	/* If the original caller did anything with "msDS-KeyVersionNumber" then skip this.  It could be an incoming samsync */
+	if ((attribute = ldb_msg_find_element(msg, "msDS-KeyVersionNumber")) == NULL ) {
+		if (kvno == 0) {
+			CHECK_RET(ldb_msg_add_empty(modify_msg, "msDS-KeyVersionNumber",
+						    LDB_FLAG_MOD_REPLACE));
+			CHECK_RET(samdb_msg_add_uint(module->ldb, mem_ctx, modify_msg, "msDS-KeyVersionNumber", kvno + 1));
+		} else {
+			/* While we should be in a transaction, go one extra
+			 * step in the dance for an 'atomic' increment.  This
+			 * may be of value against remote LDAP servers.  (Note
+			 * however that Mulitmaster replication stil offers no
+			 * such guarantee) */
+			
+			struct ldb_val old_kvno, new_kvno;
+			old_kvno.data = (uint8_t *)talloc_asprintf(mem_ctx, "%u", kvno);
+			if (!old_kvno.data) {
+				return -1;
+			}
+			old_kvno.length = strlen((char *)old_kvno.data);
+			
+			new_kvno.data = (uint8_t *)talloc_asprintf(mem_ctx, "%u", kvno + 1);
+			if (!new_kvno.data) {
+				return -1;
+			}
+			new_kvno.length = strlen((char *)new_kvno.data);
+			
+			CHECK_RET(ldb_msg_add_empty(modify_msg, "msDS-KeyVersionNumber",
+						    LDB_FLAG_MOD_DELETE));
+			CHECK_RET(ldb_msg_add_empty(modify_msg, "msDS-KeyVersionNumber",
+						    LDB_FLAG_MOD_ADD));
+			modify_msg->elements[modify_msg->num_elements - 2].num_values = 1;
+			modify_msg->elements[modify_msg->num_elements - 2].values = &old_kvno;
+			modify_msg->elements[modify_msg->num_elements - 1].num_values = 1;
+			modify_msg->elements[modify_msg->num_elements - 1].values = &new_kvno;
+		}
+	}
+
+	CHECK_RET(ldb_msg_add_empty(modify_msg, "lmPwdHistory",
+				    LDB_FLAG_MOD_REPLACE));
+	CHECK_RET(ldb_msg_add_empty(modify_msg, "ntPwdHistory",
+				    LDB_FLAG_MOD_REPLACE));
+
+	/* If we have something to put into the history, or an old
+	 * history element to expire, update the history */
+	if (pwdHistoryLength > 0 && 
+	    ((ntPwdHistory_len > 0) || (lmPwdHistory_len > 0) 
+	     || lmOldHash || ntOldHash)) {
+		/* store the password history */
+		new_lmPwdHistory = talloc_array(mem_ctx, struct samr_Password, 
+						pwdHistoryLength);
+		if (!new_lmPwdHistory) {
+			return LDB_ERR_OPERATIONS_ERROR;
+		}
+		new_ntPwdHistory = talloc_array(mem_ctx, struct samr_Password, 
+						pwdHistoryLength);
+		if (!new_ntPwdHistory) {
+			return LDB_ERR_OPERATIONS_ERROR;
+		}
+		for (i=0;i<MIN(pwdHistoryLength-1, lmPwdHistory_len);i++) {
+			new_lmPwdHistory[i+1] = lmPwdHistory[i];
+		}
+		for (i=0;i<MIN(pwdHistoryLength-1, ntPwdHistory_len);i++) {
+			new_ntPwdHistory[i+1] = ntPwdHistory[i];
+		}
+		
+		/* Don't store 'long' passwords in the LM history, 
+		   but make sure to 'expire' one password off the other end */
+		if (lmOldHash) {
+			new_lmPwdHistory[0] = *lmOldHash;
+		} else {
+			ZERO_STRUCT(new_lmPwdHistory[0]);
+		}
+		lmPwdHistory_len = MIN(lmPwdHistory_len + 1, pwdHistoryLength);
+		
+		/* Likewise, we might not have a new NT password (lm
+		 * only password change function) */
+		if (ntOldHash) {
+			new_ntPwdHistory[0] = *ntOldHash;
+		} else {
+			ZERO_STRUCT(new_ntPwdHistory[0]);
+		}
+		ntPwdHistory_len = MIN(ntPwdHistory_len + 1, pwdHistoryLength);
+		
+		CHECK_RET(samdb_msg_add_hashes(module->ldb, mem_ctx, modify_msg, 
+					       "lmPwdHistory", 
+					       new_lmPwdHistory, 
+					       lmPwdHistory_len));
+		
+		CHECK_RET(samdb_msg_add_hashes(module->ldb, mem_ctx, modify_msg, 
+					       "ntPwdHistory", 
+					       new_ntPwdHistory, 
+					       ntPwdHistory_len));
+	}
+
+	/* Too much code above, we should check we got it close to reasonable */
+	CHECK_RET(ldb_msg_sanity_check(modify_msg));
+
+	modify_request.operation = LDB_REQ_MODIFY;
+	modify_request.op.mod.message = modify_msg;
+
+	ret = ldb_next_request(module, &modify_request);
+	
+	talloc_free(mem_ctx);
+	return ret;
+}
+
+/* add_record: do things with the unicodePwd attribute */
+static int password_hash_add(struct ldb_module *module, struct ldb_request *req)
+{
+	const struct ldb_message *msg = req->op.add.message;
+
+	ldb_debug(module->ldb, LDB_DEBUG_TRACE, "password_hash_add_record\n");
+
+	if (ldb_dn_is_special(msg->dn)) { /* do not manipulate our control entries */
+		return ldb_next_request(module, req);
+	}
+	
+	return password_hash_handle(module, req, msg);
+}
+
+/* modify_record: do things with the unicodePwd attribute */
+static int password_hash_modify(struct ldb_module *module, struct ldb_request *req)
+{
+	const struct ldb_message *msg = req->op.mod.message;
+
+	ldb_debug(module->ldb, LDB_DEBUG_TRACE, "password_hash_modify_record\n");
+
+	if (ldb_dn_is_special(msg->dn)) { /* do not manipulate our control entries */
+		return ldb_next_request(module, req);
+	}
+	
+	return password_hash_handle(module, req, msg);
+}
+
+static int password_hash_request(struct ldb_module *module, struct ldb_request *req)
+{
+	switch (req->operation) {
+
+	case LDB_REQ_ADD:
+		return password_hash_add(module, req);
+
+	case LDB_REQ_MODIFY:
+		return password_hash_modify(module, req);
+
+	default:
+		return ldb_next_request(module, req);
+
+	}
+}
+
+static const struct ldb_module_ops password_hash_ops = {
+	.name          = "password_hash",
+	.request       = password_hash_request
+};
+
+
+/* the init function */
+#ifdef HAVE_DLOPEN_DISABLED
+ struct ldb_module *init_module(struct ldb_context *ldb, const char *options[])
+#else
+struct ldb_module *password_hash_module_init(struct ldb_context *ldb, const char *options[])
+#endif
+{
+	struct ldb_module *ctx;
+
+	ctx = talloc(ldb, struct ldb_module);
+	if (!ctx)
+		return NULL;
+
+	ctx->private_data = NULL;
+	ctx->ldb = ldb;
+	ctx->prev = ctx->next = NULL;
+	ctx->ops = &password_hash_ops;
+
+	return ctx;
+}
diff --git a/source4/dsdb/samdb/samdb.c b/source4/dsdb/samdb/samdb.c
index b6a2f9ce33..3d5535602b 100644
--- a/source4/dsdb/samdb/samdb.c
+++ b/source4/dsdb/samdb/samdb.c
@@ -510,13 +510,13 @@ NTTIME samdb_result_force_password_change(struct ldb_context *sam_ldb,
 /*
   pull a samr_Password structutre from a result set. 
 */
-struct samr_Password samdb_result_hash(struct ldb_message *msg, const char *attr)
+struct samr_Password *samdb_result_hash(TALLOC_CTX *mem_ctx, struct ldb_message *msg, const char *attr)
 {
-	struct samr_Password hash;
+	struct samr_Password *hash = NULL;
 	const struct ldb_val *val = ldb_msg_find_ldb_val(msg, attr);
-	ZERO_STRUCT(hash);
-	if (val) {
-		memcpy(hash.hash, val->data, MIN(val->length, sizeof(hash.hash)));
+	if (val && (val->length >= sizeof(hash->hash))) {
+		hash = talloc(mem_ctx, struct samr_Password);
+		memcpy(hash->hash, val->data, MIN(val->length, sizeof(hash->hash)));
 	}
 	return hash;
 }
@@ -555,62 +555,28 @@ uint_t samdb_result_hashes(TALLOC_CTX *mem_ctx, struct ldb_message *msg,
 NTSTATUS samdb_result_passwords(TALLOC_CTX *mem_ctx, struct ldb_message *msg, 
 				struct samr_Password **lm_pwd, struct samr_Password **nt_pwd) 
 {
-
-	const char *unicodePwd = samdb_result_string(msg, "unicodePwd", NULL);
-	
 	struct samr_Password *lmPwdHash, *ntPwdHash;
-	if (unicodePwd) {
-		if (nt_pwd) {
-			ntPwdHash = talloc(mem_ctx, struct samr_Password);
-			if (!ntPwdHash) {
-				return NT_STATUS_NO_MEMORY;
-			}
-			
-			E_md4hash(unicodePwd, ntPwdHash->hash);
-			*nt_pwd = ntPwdHash;
-		}
-
-		if (lm_pwd) {
-			BOOL lm_hash_ok;
-		
-			lmPwdHash = talloc(mem_ctx, struct samr_Password);
-			if (!lmPwdHash) {
-				return NT_STATUS_NO_MEMORY;
-			}
-			
-			/* compute the new nt and lm hashes */
-			lm_hash_ok = E_deshash(unicodePwd, lmPwdHash->hash);
-			
-			if (lm_hash_ok) {
-				*lm_pwd = lmPwdHash;
-			} else {
-				*lm_pwd = NULL;
-			}
+	if (nt_pwd) {
+		int num_nt;
+		num_nt = samdb_result_hashes(mem_ctx, msg, "ntPwdHash", &ntPwdHash);
+		if (num_nt == 0) {
+			*nt_pwd = NULL;
+		} else if (num_nt > 1) {
+			return NT_STATUS_INTERNAL_DB_CORRUPTION;
+		} else {
+			*nt_pwd = &ntPwdHash[0];
 		}
-	} else {
-		if (nt_pwd) {
-			int num_nt;
-			num_nt = samdb_result_hashes(mem_ctx, msg, "ntPwdHash", &ntPwdHash);
-			if (num_nt == 0) {
-				*nt_pwd = NULL;
-			} else if (num_nt > 1) {
-				return NT_STATUS_INTERNAL_DB_CORRUPTION;
-			} else {
-				*nt_pwd = &ntPwdHash[0];
-			}
-		}
-		if (lm_pwd) {
-			int num_lm;
-			num_lm = samdb_result_hashes(mem_ctx, msg, "lmPwdHash", &lmPwdHash);
-			if (num_lm == 0) {
-				*lm_pwd = NULL;
-			} else if (num_lm > 1) {
-				return NT_STATUS_INTERNAL_DB_CORRUPTION;
-			} else {
-				*lm_pwd = &lmPwdHash[0];
-			}
+	}
+	if (lm_pwd) {
+		int num_lm;
+		num_lm = samdb_result_hashes(mem_ctx, msg, "lmPwdHash", &lmPwdHash);
+		if (num_lm == 0) {
+			*lm_pwd = NULL;
+		} else if (num_lm > 1) {
+			return NT_STATUS_INTERNAL_DB_CORRUPTION;
+		} else {
+			*lm_pwd = &lmPwdHash[0];
 		}
-		
 	}
 	return NT_STATUS_OK;
 }
@@ -729,13 +695,9 @@ int samdb_msg_add_dom_sid(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, stru
 int samdb_msg_add_delete(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct ldb_message *msg,
 			 const char *attr_name)
 {
-	char *a = talloc_strdup(mem_ctx, attr_name);
-	if (a == NULL) {
-		return -1;
-	}
 	/* we use an empty replace rather than a delete, as it allows for 
 	   samdb_replace() to be used everywhere */
-	return ldb_msg_add_empty(msg, a, LDB_FLAG_MOD_REPLACE);
+	return ldb_msg_add_empty(msg, attr_name, LDB_FLAG_MOD_REPLACE);
 }
 
 /*