r16827: Factor out some code into common samdb functions:

 - creation of ForeignSecurityPrincipals
 - template duplication code

Rework much of the LSA server to pass the RPC-LSA test.  Much of the
server code was untested.  In implementing the LSA Accounts feature, I
have opted to have it only create entires when privilages are applied,
and not to delete entries, but to delete the privilages.

We skip some parts of the test, but it is much better than not testing
it at all.

Andrew Bartlett
(This used to be commit 10eeea6da465564ed9f785d06e2d2ed06cfe29a4)

5 files changed, 365 insertions, 280 deletions

diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index 2f0c6f2d17..c95fb70820 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -45,45 +45,6 @@
 int samldb_notice_sid(struct ldb_module *module, 
 		      TALLOC_CTX *mem_ctx, const struct dom_sid *sid);
 
-/* if value is not null also check for attribute to have exactly that value */
-static struct ldb_message_element *samldb_find_attribute(const struct ldb_message *msg, const char *name, const char *value)
-{
-	int j;
-	struct ldb_message_element *el = ldb_msg_find_element(msg, name);
-	if (!el) {
-		return NULL;
-	}
-
-	if (!value) {
-		return el;
-	}
-
-	for (j = 0; j < el->num_values; j++) {
-		if (strcasecmp(value, 
-			       (char *)el->values[j].data) == 0) {
-			return el;
-		}
-	}
-
-	return NULL;
-}
-
-static BOOL samldb_msg_add_string(struct ldb_module *module, struct ldb_message *msg, const char *name, const char *value)
-{
-	char *aval = talloc_strdup(msg, value);
-
-	if (aval == NULL) {
-		ldb_debug(module->ldb, LDB_DEBUG_FATAL, "samldb_msg_add_string: talloc_strdup failed!\n");
-		return False;
-	}
-
-	if (ldb_msg_add_string(msg, name, aval) != 0) {
-		return False;
-	}
-
-	return True;
-}
-
 static BOOL samldb_msg_add_sid(struct ldb_module *module, struct ldb_message *msg, const char *name, const struct dom_sid *sid)
 {
 	struct ldb_val v;
@@ -96,34 +57,6 @@ static BOOL samldb_msg_add_sid(struct ldb_module *module, struct ldb_message *ms
 	return (ldb_msg_add_value(msg, name, &v) == 0);
 }
 
-static BOOL samldb_find_or_add_value(struct ldb_module *module, struct ldb_message *msg, const char *name, const char *value, const char *set_value)
-{
-	if (msg == NULL || name == NULL || value == NULL || set_value == NULL) {
-		return False;
-	}
-
-	if (samldb_find_attribute(msg, name, value) == NULL) {
-		return samldb_msg_add_string(module, msg, name, set_value);
-	}
-	return True;
-}
-
-static BOOL samldb_find_or_add_attribute(struct ldb_module *module, struct ldb_message *msg, const char *name, const char *set_value)
-{
-	struct ldb_message_element *el;
-
-	if (msg == NULL || name == NULL || set_value == NULL) {
-		return False;
-	}
-
-       	el = ldb_msg_find_element(msg, name);
-	if (el) {
-		return True;
-	}
-		
-	return samldb_msg_add_string(module, msg, name, set_value);
-}
-
 /*
   allocate a new id, attempting to do it atomically
   return 0 on failure, the id on success
@@ -484,69 +417,6 @@ static char *samldb_generate_samAccountName(struct ldb_module *module, TALLOC_CT
 	} while (1);
 }
 
-static int samldb_copy_template(struct ldb_module *module, struct ldb_message *msg, const char *filter)
-{
-	struct ldb_result *res;
-	struct ldb_message *t;
-	int ret, i, j;
-	
-	struct ldb_dn *basedn = ldb_dn_explode(msg, "cn=Templates");
-
-	/* pull the template record */
-	ret = ldb_search(module->ldb, basedn, LDB_SCOPE_SUBTREE, filter, NULL, &res);
-	if (ret != LDB_SUCCESS) {
-		return ret;
-	}
-	if (res->count != 1) {
-		ldb_set_errstring(module->ldb, talloc_asprintf(module, "samldb_copy_template: ERROR: template '%s' matched %d records, expected 1\n", filter, 
-					  res->count));
-		return LDB_ERR_OPERATIONS_ERROR;
-	}
-	t = res->msgs[0];
-
-	for (i = 0; i < t->num_elements; i++) {
-		struct ldb_message_element *el = &t->elements[i];
-		/* some elements should not be copied from the template */
-		if (strcasecmp(el->name, "cn") == 0 ||
-		    strcasecmp(el->name, "name") == 0 ||
-		    strcasecmp(el->name, "sAMAccountName") == 0 ||
-		    strcasecmp(el->name, "objectGUID") == 0) {
-			continue;
-		}
-		for (j = 0; j < el->num_values; j++) {
-			if (strcasecmp(el->name, "objectClass") == 0) {
-				if (strcasecmp((char *)el->values[j].data, "Template") == 0 ||
-				    strcasecmp((char *)el->values[j].data, "userTemplate") == 0 ||
-				    strcasecmp((char *)el->values[j].data, "groupTemplate") == 0 ||
-				    strcasecmp((char *)el->values[j].data, "foreignSecurityPrincipalTemplate") == 0 ||
-				    strcasecmp((char *)el->values[j].data, "aliasTemplate") == 0 || 
-				    strcasecmp((char *)el->values[j].data, "trustedDomainTemplate") == 0 || 
-				    strcasecmp((char *)el->values[j].data, "secretTemplate") == 0) {
-					continue;
-				}
-				if ( ! samldb_find_or_add_value(module, msg, el->name, 
-								(char *)el->values[j].data,
-								(char *)el->values[j].data)) {
-					ldb_set_errstring(module->ldb, talloc_asprintf(module, "Adding objectClass %s failed.\n", el->values[j].data));
-					talloc_free(res);
-					return LDB_ERR_OPERATIONS_ERROR;
-				}
-			} else {
-				if ( ! samldb_find_or_add_attribute(module, msg, el->name, 
-								    (char *)el->values[j].data)) {
-					ldb_set_errstring(module->ldb, talloc_asprintf(module, "Adding attribute %s failed.\n", el->name));
-					talloc_free(res);
-					return LDB_ERR_OPERATIONS_ERROR;
-				}
-			}
-		}
-	}
-
-	talloc_free(res);
-
-	return LDB_SUCCESS;
-}
-
 static int samldb_fill_group_object(struct ldb_module *module, const struct ldb_message *msg,
 						    struct ldb_message **ret_msg)
 {
@@ -567,7 +437,7 @@ static int samldb_fill_group_object(struct ldb_module *module, const struct ldb_
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	ret = samldb_copy_template(module, msg2, "(&(CN=TemplateGroup)(objectclass=groupTemplate))");
+	ret = samdb_copy_template(module->ldb, msg2, "(&(CN=TemplateGroup)(objectclass=groupTemplate))");
 	if (ret != 0) {
 		talloc_free(mem_ctx);
 		return ret;
@@ -588,9 +458,10 @@ static int samldb_fill_group_object(struct ldb_module *module, const struct ldb_
 			talloc_free(mem_ctx);
 			return LDB_ERR_OPERATIONS_ERROR;
 		}
-		if ( ! samldb_find_or_add_attribute(module, msg2, "sAMAccountName", name)) {
+		ret = samdb_find_or_add_attribute(module->ldb, msg2, "sAMAccountName", name);
+		if (ret) {
 			talloc_free(mem_ctx);
-			return LDB_ERR_OPERATIONS_ERROR;
+			return ret;
 		}
 	}
 	
@@ -625,9 +496,9 @@ static int samldb_fill_user_or_computer_object(struct ldb_module *module, const
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	if (samldb_find_attribute(msg, "objectclass", "computer") != NULL) {
+	if (samdb_find_attribute(module->ldb, msg, "objectclass", "computer") != NULL) {
 
-		ret = samldb_copy_template(module, msg2, "(&(CN=TemplateComputer)(objectclass=userTemplate))");
+		ret = samdb_copy_template(module->ldb, msg2, "(&(CN=TemplateComputer)(objectclass=userTemplate))");
 		if (ret) {
 			ldb_debug(module->ldb, LDB_DEBUG_WARNING, "samldb_fill_user_or_computer_object: Error copying computer template!\n");
 			talloc_free(mem_ctx);
@@ -635,26 +506,29 @@ static int samldb_fill_user_or_computer_object(struct ldb_module *module, const
 		}
 
 		/* readd user and then computer objectclasses */
-		if ( ! samldb_find_or_add_value(module, msg2, "objectclass", "user", "user")) {
+		ret = samdb_find_or_add_value(module->ldb, msg2, "objectclass", "user");
+		if (ret) {
 			talloc_free(mem_ctx);
-			return LDB_ERR_OPERATIONS_ERROR;
+			return ret;
 		}
-		if ( ! samldb_find_or_add_value(module, msg2, "objectclass", "computer", "computer")) {
+		ret = samdb_find_or_add_value(module->ldb, msg2, "objectclass", "computer");
+		if (ret) {
 			talloc_free(mem_ctx);
-			return LDB_ERR_OPERATIONS_ERROR;
+			return ret;
 		}
 		
 	} else {
-		ret = samldb_copy_template(module, msg2, "(&(CN=TemplateUser)(objectclass=userTemplate))");
+		ret = samdb_copy_template(module->ldb, msg2, "(&(CN=TemplateUser)(objectclass=userTemplate))");
 		if (ret) {
 			ldb_debug(module->ldb, LDB_DEBUG_WARNING, "samldb_fill_user_or_computer_object: Error copying user template!\n");
 			talloc_free(mem_ctx);
 			return ret;
 		}
 		/* readd user objectclass */
-		if ( ! samldb_find_or_add_value(module, msg2, "objectclass", "user", "user")) {
+		ret = samdb_find_or_add_value(module->ldb, msg2, "objectclass", "user");
+		if (ret) {
 			talloc_free(mem_ctx);
-			return LDB_ERR_OPERATIONS_ERROR;
+			return ret;
 		}
 	}
 
@@ -672,9 +546,10 @@ static int samldb_fill_user_or_computer_object(struct ldb_module *module, const
 			talloc_free(mem_ctx);
 			return LDB_ERR_OPERATIONS_ERROR;
 		}
-		if ( ! samldb_find_or_add_attribute(module, msg2, "sAMAccountName", name)) {
+		ret = samdb_find_or_add_attribute(module->ldb, msg2, "sAMAccountName", name);
+		if (ret) {
 			talloc_free(mem_ctx);
-			return LDB_ERR_OPERATIONS_ERROR;
+			return ret;
 		}
 	}
 
@@ -719,7 +594,7 @@ static int samldb_fill_foreignSecurityPrincipal_object(struct ldb_module *module
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	ret = samldb_copy_template(module, msg2, "(&(CN=TemplateForeignSecurityPrincipal)(objectclass=foreignSecurityPrincipalTemplate))");
+	ret = samdb_copy_template(module->ldb, msg2, "(&(CN=TemplateForeignSecurityPrincipal)(objectclass=foreignSecurityPrincipalTemplate))");
 	if (ret != 0) {
 		ldb_debug(module->ldb, LDB_DEBUG_WARNING, "samldb_fill_foreignSecurityPrincipal_object: Error copying template!\n");
 		talloc_free(mem_ctx);
@@ -815,8 +690,8 @@ static int samldb_add(struct ldb_module *module, struct ldb_request *req)
 	}
 
 	/* is user or computer? */
-	if ((samldb_find_attribute(msg, "objectclass", "user") != NULL) ||
-	    (samldb_find_attribute(msg, "objectclass", "computer") != NULL)) {
+	if ((samdb_find_attribute(module->ldb, msg, "objectclass", "user") != NULL) ||
+	    (samdb_find_attribute(module->ldb, msg, "objectclass", "computer") != NULL)) {
 		/*  add all relevant missing objects */
 		ret = samldb_fill_user_or_computer_object(module, msg, &msg2);
 		if (ret) {
@@ -826,7 +701,7 @@ static int samldb_add(struct ldb_module *module, struct ldb_request *req)
 
 	/* is group? add all relevant missing objects */
 	if ( ! msg2 ) {
-		if (samldb_find_attribute(msg, "objectclass", "group") != NULL) {
+		if (samdb_find_attribute(module->ldb, msg, "objectclass", "group") != NULL) {
 			ret = samldb_fill_group_object(module, msg, &msg2);
 			if (ret) {
 				return ret;
@@ -836,7 +711,7 @@ static int samldb_add(struct ldb_module *module, struct ldb_request *req)
 
 	/* perhaps a foreignSecurityPrincipal? */
 	if ( ! msg2 ) {
-		if (samldb_find_attribute(msg, "objectclass", "foreignSecurityPrincipal") != NULL) {
+		if (samdb_find_attribute(module->ldb, msg, "objectclass", "foreignSecurityPrincipal") != NULL) {
 			ret = samldb_fill_foreignSecurityPrincipal_object(module, msg, &msg2);
 			if (ret) {
 				return ret;
diff --git a/source4/dsdb/samdb/samdb.c b/source4/dsdb/samdb/samdb.c
index ba73a039af..2136294661 100644
--- a/source4/dsdb/samdb/samdb.c
+++ b/source4/dsdb/samdb/samdb.c
@@ -4,7 +4,9 @@
    interface functions for the sam database
 
    Copyright (C) Andrew Tridgell 2004
-   
+   Copyright (C) Volker Lendecke 2004
+   Copyright (C) Andrew Bartlett <abartlet@samba.org> 2006
+
    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
@@ -25,6 +27,7 @@
 #include "librpc/gen_ndr/ndr_misc.h"
 #include "librpc/gen_ndr/ndr_security.h"
 #include "lib/ldb/include/ldb.h"
+#include "lib/ldb/include/ldb_private.h"
 #include "lib/ldb/include/ldb_errors.h"
 #include "libcli/security/security.h"
 #include "auth/credentials/credentials.h"
@@ -621,51 +624,121 @@ uint16_t samdb_result_acct_flags(struct ldb_message *msg, const char *attr)
 	return samdb_uf2acb(userAccountControl);
 }
 
+
+/* Find an attribute, with a particular value */
+struct ldb_message_element *samdb_find_attribute(struct ldb_context *ldb, 
+						 const struct ldb_message *msg, 
+						 const char *name, const char *value)
+{
+	int i;
+	struct ldb_message_element *el = ldb_msg_find_element(msg, name);
+	struct ldb_val v;
+
+	v.data = discard_const_p(uint8_t, value);
+	v.length = strlen(value);
+
+	if (!el) {
+		return NULL;
+	}
+
+	for (i=0;i<el->num_values;i++) {
+		if (strcasecmp(value, (char *)el->values[i].data) == 0) {
+			return el;
+		}
+	}
+
+	return NULL;
+}
+
+int samdb_find_or_add_value(struct ldb_context *ldb, struct ldb_message *msg, const char *name, const char *set_value)
+{
+	if (samdb_find_attribute(ldb, msg, name, set_value) == NULL) {
+		return samdb_msg_add_string(ldb, msg, msg, name, set_value);
+	}
+	return LDB_SUCCESS;
+}
+
+int samdb_find_or_add_attribute(struct ldb_context *ldb, struct ldb_message *msg, const char *name, const char *set_value)
+{
+	struct ldb_message_element *el;
+
+       	el = ldb_msg_find_element(msg, name);
+	if (el) {
+		return LDB_SUCCESS;
+	}
+		
+	return samdb_msg_add_string(ldb, msg, msg, name, set_value);
+}
+
+
 /*
   copy from a template record to a message
 */
-int samdb_copy_template(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, 
-			struct ldb_message *msg, const char *expression)
+int samdb_copy_template(struct ldb_context *ldb, 
+			struct ldb_message *msg, const char *filter)
 {
-	struct ldb_message **res, *t;
+	struct ldb_result *res;
+	struct ldb_message *t;
 	int ret, i, j;
 	
-	struct ldb_dn *basedn = ldb_dn_explode(msg, "cn=Templates");
+	struct ldb_dn *basedn = ldb_dn_explode(ldb, "cn=Templates");
 
 	/* pull the template record */
-	ret = gendb_search(sam_ldb, mem_ctx, basedn, &res, NULL, "%s", expression);
-	if (ret != 1) {
-		DEBUG(1,("samdb: ERROR: template '%s' matched %d records\n", 
-			 expression, ret));
-		return -1;
+	ret = ldb_search(ldb, basedn, LDB_SCOPE_SUBTREE, filter, NULL, &res);
+	talloc_free(basedn);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+	if (res->count != 1) {
+		ldb_set_errstring(ldb, talloc_asprintf(ldb, "samdb_copy_template: ERROR: template '%s' matched %d records, expected 1\n", filter, 
+						       res->count));
+		talloc_free(res);
+		return LDB_ERR_OPERATIONS_ERROR;
 	}
-	t = res[0];
+	t = res->msgs[0];
 
-	for (i=0;i<t->num_elements;i++) {
+	for (i = 0; i < t->num_elements; i++) {
 		struct ldb_message_element *el = &t->elements[i];
 		/* some elements should not be copied from the template */
 		if (strcasecmp(el->name, "cn") == 0 ||
 		    strcasecmp(el->name, "name") == 0 ||
-		    strcasecmp(el->name, "sAMAccountName") == 0) {
+		    strcasecmp(el->name, "sAMAccountName") == 0 ||
+		    strcasecmp(el->name, "objectGUID") == 0) {
 			continue;
 		}
-		for (j=0;j<el->num_values;j++) {
-			if (strcasecmp(el->name, "objectClass") == 0 &&
-			    (strcasecmp((char *)el->values[j].data, "Template") == 0 ||
-			     strcasecmp((char *)el->values[j].data, "userTemplate") == 0 ||
-			     strcasecmp((char *)el->values[j].data, "groupTemplate") == 0 ||
-			     strcasecmp((char *)el->values[j].data, "foreignSecurityTemplate") == 0 ||
-			     strcasecmp((char *)el->values[j].data, "aliasTemplate") == 0 || 
-			     strcasecmp((char *)el->values[j].data, "trustedDomainTemplate") == 0 || 
-			     strcasecmp((char *)el->values[j].data, "secretTemplate") == 0)) {
-				continue;
+		for (j = 0; j < el->num_values; j++) {
+			if (strcasecmp(el->name, "objectClass") == 0) {
+				if (strcasecmp((char *)el->values[j].data, "Template") == 0 ||
+				    strcasecmp((char *)el->values[j].data, "userTemplate") == 0 ||
+				    strcasecmp((char *)el->values[j].data, "groupTemplate") == 0 ||
+				    strcasecmp((char *)el->values[j].data, "foreignSecurityPrincipalTemplate") == 0 ||
+				    strcasecmp((char *)el->values[j].data, "aliasTemplate") == 0 || 
+				    strcasecmp((char *)el->values[j].data, "trustedDomainTemplate") == 0 || 
+				    strcasecmp((char *)el->values[j].data, "secretTemplate") == 0) {
+					continue;
+				}
+				ret = samdb_find_or_add_value(ldb, msg, el->name, 
+							      (char *)el->values[j].data);
+				if (ret) {
+					ldb_set_errstring(ldb, talloc_asprintf(ldb, "Adding objectClass %s failed.\n", el->values[j].data));
+					talloc_free(res);
+					return ret;
+				}
+			} else {
+				ret = samdb_find_or_add_attribute(ldb, msg, el->name, 
+								  (char *)el->values[j].data);
+				if (ret) {
+					ldb_set_errstring(ldb, talloc_asprintf(ldb, "Adding attribute %s failed.\n", el->name));
+					talloc_free(res);
+					return ret;
+				}
 			}
-			samdb_msg_add_string(sam_ldb, mem_ctx, msg, el->name, 
-					     (char *)el->values[j].data);
 		}
 	}
 
-	return 0;
+	talloc_free(res);
+
+	return LDB_SUCCESS;
 }
 
 
@@ -678,7 +751,7 @@ int samdb_msg_add_string(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struc
 	char *s = talloc_strdup(mem_ctx, str);
 	char *a = talloc_strdup(mem_ctx, attr_name);
 	if (s == NULL || a == NULL) {
-		return -1;
+		return LDB_ERR_OPERATIONS_ERROR;
 	}
 	return ldb_msg_add_string(msg, a, s);
 }
@@ -1446,3 +1519,58 @@ NTSTATUS security_token_create(TALLOC_CTX *mem_ctx,
 }
 
 
+NTSTATUS samdb_create_foreign_security_principal(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, 
+						 struct dom_sid *sid, struct ldb_dn **ret_dn) 
+{
+	struct ldb_message *msg;
+	struct ldb_dn *basedn;
+	const char *sidstr;
+	int ret;
+	
+	sidstr = dom_sid_string(mem_ctx, sid);
+	NT_STATUS_HAVE_NO_MEMORY(sidstr);
+	
+	/* We might have to create a ForeignSecurityPrincipal, even if this user
+	 * is in our own domain */
+	
+	msg = ldb_msg_new(mem_ctx);
+	if (msg == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+	
+	/* TODO: Hmmm. This feels wrong. How do I find the base dn to
+	 * put the ForeignSecurityPrincipals? d_state->domain_dn does
+	 * not work, this is wrong for the Builtin domain, there's no
+	 * cn=For...,cn=Builtin,dc={BASEDN}.  -- vl
+	 */
+	
+	basedn = samdb_search_dn(sam_ctx, mem_ctx, samdb_base_dn(mem_ctx),
+				 "(&(objectClass=container)(cn=ForeignSecurityPrincipals))");
+	
+	if (basedn == NULL) {
+		DEBUG(0, ("Failed to find DN for "
+			  "ForeignSecurityPrincipal container\n"));
+		return NT_STATUS_INTERNAL_DB_CORRUPTION;
+	}
+	
+	/* add core elements to the ldb_message for the alias */
+	msg->dn = ldb_dn_build_child(mem_ctx, "CN", sidstr, basedn);
+	if (msg->dn == NULL)
+		return NT_STATUS_NO_MEMORY;
+	
+	samdb_msg_add_string(sam_ctx, mem_ctx, msg,
+			     "objectClass",
+			     "foreignSecurityPrincipal");
+	
+	/* create the alias */
+	ret = samdb_add(sam_ctx, mem_ctx, msg);
+	if (ret != 0) {
+		DEBUG(0,("Failed to create foreignSecurityPrincipal "
+			 "record %s: %s\n", 
+			 ldb_dn_linearize(mem_ctx, msg->dn),
+			 ldb_errstring(sam_ctx)));
+		return NT_STATUS_INTERNAL_DB_CORRUPTION;
+	}
+	*ret_dn = msg->dn;
+	return NT_STATUS_OK;
+}
diff --git a/source4/rpc_server/lsa/dcesrv_lsa.c b/source4/rpc_server/lsa/dcesrv_lsa.c
index 17f9754c3b..9a76926375 100644
--- a/source4/rpc_server/lsa/dcesrv_lsa.c
+++ b/source4/rpc_server/lsa/dcesrv_lsa.c
@@ -27,6 +27,7 @@
 #include "auth/auth.h"
 #include "dsdb/samdb/samdb.h"
 #include "libcli/ldap/ldap.h"
+#include "lib/ldb/include/ldb_errors.h"
 #include "libcli/security/security.h"
 #include "libcli/auth/libcli_auth.h"
 #include "passdb/secrets.h"
@@ -66,7 +67,6 @@ struct lsa_account_state {
 	struct lsa_policy_state *policy;
 	uint32_t access_mask;
 	struct dom_sid *account_sid;
-	const struct ldb_dn *account_dn;
 };
 
 
@@ -90,6 +90,17 @@ struct lsa_trusted_domain_state {
 	const struct ldb_dn *trusted_domain_dn;
 };
 
+static NTSTATUS lsa_EnumAccountRights(struct dcesrv_call_state *dce_call, 
+				      TALLOC_CTX *mem_ctx,
+				      struct lsa_EnumAccountRights *r);
+
+static NTSTATUS lsa_AddRemoveAccountRights(struct dcesrv_call_state *dce_call, 
+					   TALLOC_CTX *mem_ctx,
+					   struct lsa_policy_state *state,
+					   int ldb_flag,
+					   struct dom_sid *sid,
+					   const struct lsa_RightSet *rights);
+
 /* 
   lsa_Close 
 */
@@ -139,6 +150,41 @@ static NTSTATUS lsa_Delete(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_c
 		}
 
 		return NT_STATUS_OK;
+	} else if (h->wire_handle.handle_type == LSA_HANDLE_ACCOUNT) {
+		struct lsa_RightSet *rights;
+		struct lsa_account_state *astate;
+		struct lsa_EnumAccountRights r2;
+		NTSTATUS status;
+
+		rights = talloc(mem_ctx, struct lsa_RightSet);
+
+		DCESRV_PULL_HANDLE(h, r->in.handle, LSA_HANDLE_ACCOUNT);
+		
+		astate = h->data;
+
+		r2.in.handle = &astate->policy->handle->wire_handle;
+		r2.in.sid = astate->account_sid;
+		r2.out.rights = rights;
+
+		status = lsa_EnumAccountRights(dce_call, mem_ctx, &r2);
+		if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
+			return NT_STATUS_OK;
+		}
+
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+
+		status = lsa_AddRemoveAccountRights(dce_call, mem_ctx, astate->policy, 
+						    LDB_FLAG_MOD_DELETE, astate->account_sid,
+						    r2.out.rights);
+		if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
+			return NT_STATUS_OK;
+		}
+
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
 	} 
 	
 	return NT_STATUS_INVALID_HANDLE;
@@ -458,7 +504,42 @@ static NTSTATUS lsa_ClearAuditLog(struct dcesrv_call_state *dce_call, TALLOC_CTX
 static NTSTATUS lsa_CreateAccount(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
 				  struct lsa_CreateAccount *r)
 {
-	DCESRV_FAULT(DCERPC_FAULT_OP_RNG_ERROR);
+	struct lsa_account_state *astate;
+
+	struct lsa_policy_state *state;
+	struct dcesrv_handle *h, *ah;
+
+	ZERO_STRUCTP(r->out.acct_handle);
+
+	DCESRV_PULL_HANDLE(h, r->in.handle, LSA_HANDLE_POLICY);
+
+	state = h->data;
+
+	astate = talloc(dce_call->conn, struct lsa_account_state);
+	if (astate == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	astate->account_sid = dom_sid_dup(astate, r->in.sid);
+	if (astate->account_sid == NULL) {
+		talloc_free(astate);
+		return NT_STATUS_NO_MEMORY;
+	}
+	
+	astate->policy = talloc_reference(astate, state);
+	astate->access_mask = r->in.access_mask;
+
+	ah = dcesrv_handle_new(dce_call->context, LSA_HANDLE_ACCOUNT);
+	if (!ah) {
+		talloc_free(astate);
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	ah->data = talloc_steal(ah, astate);
+
+	*r->out.acct_handle = ah->wire_handle;
+
+	return NT_STATUS_OK;
 }
 
 
@@ -479,9 +560,12 @@ static NTSTATUS lsa_EnumAccounts(struct dcesrv_call_state *dce_call, TALLOC_CTX
 
 	state = h->data;
 
-	ret = gendb_search(state->sam_ldb, mem_ctx, state->builtin_dn, &res, attrs, 
-			   "(|(privilege=*)(objectSid=*))");
-	if (ret <= 0) {
+	/* NOTE: This call must only return accounts that have at least
+	   one privilege set 
+	*/
+	ret = gendb_search(state->sam_ldb, mem_ctx, samdb_base_dn(mem_ctx), &res, attrs, 
+			   "(&(objectSid=*)(privilege=*))");
+	if (ret < 0) {
 		return NT_STATUS_NO_SUCH_USER;
 	}
 
@@ -1298,15 +1382,6 @@ static NTSTATUS lsa_OpenAccount(struct dcesrv_call_state *dce_call, TALLOC_CTX *
 		return NT_STATUS_NO_MEMORY;
 	}
 	
-	/* check it really exists */
-	astate->account_dn = samdb_search_dn(state->sam_ldb, astate,
-					     NULL, "(&(objectSid=%s)(objectClass=group))", 
-					     ldap_encode_ndr_dom_sid(mem_ctx, astate->account_sid));
-	if (astate->account_dn == NULL) {
-		talloc_free(astate);
-		return NT_STATUS_NO_SUCH_USER;
-	}
-	
 	astate->policy = talloc_reference(astate, state);
 	astate->access_mask = r->in.access_mask;
 
@@ -1337,6 +1412,7 @@ static NTSTATUS lsa_EnumPrivsAccount(struct dcesrv_call_state *dce_call,
 	struct ldb_message **res;
 	const char * const attrs[] = { "privilege", NULL};
 	struct ldb_message_element *el;
+	const char *sidstr;
 
 	DCESRV_PULL_HANDLE(h, r->in.handle, LSA_HANDLE_ACCOUNT);
 
@@ -1347,8 +1423,13 @@ static NTSTATUS lsa_EnumPrivsAccount(struct dcesrv_call_state *dce_call,
 	r->out.privs->unknown = 0;
 	r->out.privs->set = NULL;
 
-	ret = gendb_search_dn(astate->policy->sam_ldb, mem_ctx,
-			      astate->account_dn, &res, attrs);
+	sidstr = ldap_encode_ndr_dom_sid(mem_ctx, astate->account_sid);
+	if (sidstr == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	ret = gendb_search(astate->policy->sam_ldb, mem_ctx, samdb_base_dn(mem_ctx), &res, attrs, 
+			   "objectSid=%s", sidstr);
 	if (ret != 1) {
 		return NT_STATUS_OK;
 	}
@@ -1359,7 +1440,7 @@ static NTSTATUS lsa_EnumPrivsAccount(struct dcesrv_call_state *dce_call,
 	}
 
 	r->out.privs->set = talloc_array(r->out.privs, 
-					   struct lsa_LUIDAttribute, el->num_values);
+					 struct lsa_LUIDAttribute, el->num_values);
 	if (r->out.privs->set == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -1403,11 +1484,20 @@ static NTSTATUS lsa_EnumAccountRights(struct dcesrv_call_state *dce_call,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	ret = gendb_search(state->sam_ldb, mem_ctx, NULL, &res, attrs, 
-			   "objectSid=%s", sidstr);
-	if (ret != 1) {
+	ret = gendb_search(state->sam_ldb, mem_ctx, samdb_base_dn(mem_ctx), &res, attrs, 
+			   "(&(objectSid=%s)(privilege=*))", sidstr);
+	if (ret == 0) {
 		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
 	}
+	if (ret > 1) {
+		return NT_STATUS_INTERNAL_DB_CORRUPTION;
+	}
+	if (ret == -1) {
+		DEBUG(3, ("searching for account rights for SID: %s failed: %s", 
+			  dom_sid_string(mem_ctx, r->in.sid),
+			  ldb_errstring(state->sam_ldb)));
+		return NT_STATUS_INTERNAL_DB_CORRUPTION;
+	}
 
 	el = ldb_msg_find_element(res[0], "privilege");
 	if (el == NULL || el->num_values == 0) {
@@ -1442,7 +1532,7 @@ static NTSTATUS lsa_AddRemoveAccountRights(struct dcesrv_call_state *dce_call,
 {
 	const char *sidstr;
 	struct ldb_message *msg;
-	struct ldb_message_element el;
+	struct ldb_message_element *el;
 	int i, ret;
 	struct lsa_EnumAccountRights r2;
 
@@ -1456,8 +1546,18 @@ static NTSTATUS lsa_AddRemoveAccountRights(struct dcesrv_call_state *dce_call,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	msg->dn = samdb_search_dn(state->sam_ldb, mem_ctx, NULL, "objectSid=%s", sidstr);
+	msg->dn = samdb_search_dn(state->sam_ldb, mem_ctx, 
+				  samdb_base_dn(mem_ctx), "objectSid=%s", sidstr);
 	if (msg->dn == NULL) {
+		NTSTATUS status;
+		if (ldb_flag == LDB_FLAG_MOD_DELETE) {
+			return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+		}
+		status = samdb_create_foreign_security_principal(state->sam_ldb, mem_ctx, 
+								 sid, &msg->dn);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
 		return NT_STATUS_NO_SUCH_USER;
 	}
 
@@ -1478,11 +1578,6 @@ static NTSTATUS lsa_AddRemoveAccountRights(struct dcesrv_call_state *dce_call,
 		}
 	}
 
-	el.num_values = 0;
-	el.values = talloc_array(mem_ctx, struct ldb_val, rights->count);
-	if (el.values == NULL) {
-		return NT_STATUS_NO_MEMORY;
-	}
 	for (i=0;i<rights->count;i++) {
 		if (sec_privilege_id(rights->names[i].string) == -1) {
 			return NT_STATUS_NO_SUCH_PRIVILEGE;
@@ -1499,24 +1594,25 @@ static NTSTATUS lsa_AddRemoveAccountRights(struct dcesrv_call_state *dce_call,
 			if (j != r2.out.rights->count) continue;
 		}
 
-
-		el.values[el.num_values].length = strlen(rights->names[i].string);
-		el.values[el.num_values].data = (uint8_t *)talloc_strdup(mem_ctx, rights->names[i].string);
-		if (el.values[el.num_values].data == NULL) {
+		ret = ldb_msg_add_string(msg, "privilege", rights->names[i].string);
+		if (ret != LDB_SUCCESS) {
 			return NT_STATUS_NO_MEMORY;
 		}
-		el.num_values++;
 	}
 
-	if (el.num_values == 0) {
+	el = ldb_msg_find_element(msg, "privilege");
+	if (!el) {
 		return NT_STATUS_OK;
 	}
 
 	ret = samdb_modify(state->sam_ldb, mem_ctx, msg);
 	if (ret != 0) {
-		if (ldb_flag == LDB_FLAG_MOD_DELETE) {
+		if (ldb_flag == LDB_FLAG_MOD_DELETE && ret == LDB_ERR_NO_SUCH_ATTRIBUTE) {
 			return NT_STATUS_OBJECT_NAME_NOT_FOUND;
 		}
+		DEBUG(3, ("Could not %s attributes from %s: %s", 
+			  ldb_flag == LDB_FLAG_MOD_DELETE ? "delete" : "add",
+			  ldb_dn_linearize(mem_ctx, msg->dn), ldb_errstring(state->sam_ldb)));
 		return NT_STATUS_UNEXPECTED_IO_ERROR;
 	}
 
@@ -1721,9 +1817,9 @@ static NTSTATUS lsa_CreateSecret(struct dcesrv_call_state *dce_call, TALLOC_CTX
 			return NT_STATUS_OBJECT_NAME_COLLISION;
 		}
 		
-		if (ret < 0 || ret > 1) {
-			DEBUG(0,("Found %d records matching DN %s\n", ret,
-				 ldb_dn_linearize(mem_ctx, policy_state->system_dn)));
+		if (ret == -1) {
+			DEBUG(0,("Failure searching for CN=%s: %s\n", 
+				 name2, ldb_errstring(secret_state->sam_ldb)));
 			return NT_STATUS_INTERNAL_DB_CORRUPTION;
 		}
 
@@ -1753,9 +1849,9 @@ static NTSTATUS lsa_CreateSecret(struct dcesrv_call_state *dce_call, TALLOC_CTX
 			return NT_STATUS_OBJECT_NAME_COLLISION;
 		}
 		
-		if (ret < 0 || ret > 1) {
-			DEBUG(0,("Found %d records matching DN %s\n", ret,
-				 ldb_dn_linearize(mem_ctx, policy_state->system_dn)));
+		if (ret == -1) {
+			DEBUG(0,("Failure searching for CN=%s: %s\n", 
+				 name, ldb_errstring(secret_state->sam_ldb)));
 			return NT_STATUS_INTERNAL_DB_CORRUPTION;
 		}
 
@@ -1764,10 +1860,11 @@ static NTSTATUS lsa_CreateSecret(struct dcesrv_call_state *dce_call, TALLOC_CTX
 	} 
 
 	/* pull in all the template attributes.  Note this is always from the global samdb */
-	ret = samdb_copy_template(secret_state->policy->sam_ldb, mem_ctx, msg, 
-				  "(&(name=TemplateSecret)(objectclass=secretTemplate))");
+	ret = samdb_copy_template(secret_state->policy->sam_ldb, msg, 
+				  "(&(cn=TemplateSecret)(objectclass=secretTemplate))");
 	if (ret != 0) {
-		DEBUG(0,("Failed to load TemplateSecret from samdb\n"));
+		DEBUG(0,("Failed to load TemplateSecret from samdb: %s\n", 
+			 ldb_errstring(secret_state->policy->sam_ldb)));
 		return NT_STATUS_INTERNAL_DB_CORRUPTION;
 	}
 
@@ -1778,8 +1875,9 @@ static NTSTATUS lsa_CreateSecret(struct dcesrv_call_state *dce_call, TALLOC_CTX
 	/* create the secret */
 	ret = samdb_add(secret_state->sam_ldb, mem_ctx, msg);
 	if (ret != 0) {
-		DEBUG(0,("Failed to create secret record %s\n",
-			 ldb_dn_linearize(mem_ctx, msg->dn)));
+		DEBUG(0,("Failed to create secret record %s: %s\n",
+			 ldb_dn_linearize(mem_ctx, msg->dn), 
+			 ldb_errstring(secret_state->sam_ldb)));
 		return NT_STATUS_INTERNAL_DB_CORRUPTION;
 	}
 
@@ -2314,10 +2412,13 @@ static NTSTATUS lsa_EnumAccountsWithUserRight(struct dcesrv_call_state *dce_call
 		return NT_STATUS_NO_SUCH_PRIVILEGE;
 	}
 
-	ret = gendb_search(state->sam_ldb, mem_ctx, NULL, &res, attrs, 
+	ret = gendb_search(state->sam_ldb, mem_ctx, samdb_base_dn(mem_ctx), &res, attrs, 
 			   "privilege=%s", privname);
-	if (ret <= 0) {
-		return NT_STATUS_NO_SUCH_USER;
+	if (ret == -1) {
+		return NT_STATUS_INTERNAL_DB_CORRUPTION;
+	}
+	if (ret == 0) {
+		return NT_STATUS_NO_MORE_ENTRIES;
 	}
 
 	r->out.sids->sids = talloc_array(r->out.sids, struct lsa_SidPtr, ret);
diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index 40d562fc0c..e36c0e96ea 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -2527,6 +2527,7 @@ static NTSTATUS samr_AddAliasMember(struct dcesrv_call_state *dce_call, TALLOC_C
 	const char * const attrs[] = { NULL };
 	struct ldb_dn *memberdn = NULL;
 	int ret;
+	NTSTATUS status;
 
 	DCESRV_PULL_HANDLE(h, r->in.alias_handle, SAMR_HANDLE_ALIAS);
 
@@ -2544,58 +2545,13 @@ static NTSTATUS samr_AddAliasMember(struct dcesrv_call_state *dce_call, TALLOC_C
 			 ret, dom_sid_string(mem_ctx, r->in.sid)));
 		return NT_STATUS_INTERNAL_DB_CORRUPTION;
 	} else if (ret == 0) {
-		struct ldb_message *msg;
-		struct ldb_dn *basedn;
-		const char *sidstr;
-
-		sidstr = dom_sid_string(mem_ctx, r->in.sid);
-		NT_STATUS_HAVE_NO_MEMORY(sidstr);
-
-		/* We might have to create a ForeignSecurityPrincipal, even if this user
-		 * is in our own domain */
-
-		msg = ldb_msg_new(mem_ctx);
-		if (msg == NULL) {
-			return NT_STATUS_NO_MEMORY;
-		}
-
-		/* TODO: Hmmm. This feels wrong. How do I find the base dn to
-		 * put the ForeignSecurityPrincipals? d_state->domain_dn does
-		 * not work, this is wrong for the Builtin domain, there's no
-		 * cn=For...,cn=Builtin,dc={BASEDN}.  -- vl
-		 */
-
-		basedn = samdb_search_dn(d_state->sam_ctx, mem_ctx, samdb_base_dn(mem_ctx),
-					 "(&(objectClass=container)(cn=ForeignSecurityPrincipals))");
-
-		if (basedn == NULL) {
-			DEBUG(0, ("Failed to find DN for "
-				  "ForeignSecurityPrincipal container\n"));
-			return NT_STATUS_INTERNAL_DB_CORRUPTION;
-		}
-
-		/* add core elements to the ldb_message for the alias */
-		msg->dn = ldb_dn_build_child(mem_ctx, "CN", sidstr, basedn);
-		if (msg->dn == NULL)
-			return NT_STATUS_NO_MEMORY;
-
-		memberdn = msg->dn;
-
-		samdb_msg_add_string(d_state->sam_ctx, mem_ctx, msg,
-				     "objectClass",
-				     "foreignSecurityPrincipal");
-
-		/* create the alias */
-		ret = samdb_add(d_state->sam_ctx, mem_ctx, msg);
-		if (ret != 0) {
-			DEBUG(0,("Failed to create foreignSecurityPrincipal "
-				 "record %s: %s\n", 
-				 ldb_dn_linearize(mem_ctx, msg->dn),
-				 ldb_errstring(d_state->sam_ctx)));
-			return NT_STATUS_INTERNAL_DB_CORRUPTION;
+		status = samdb_create_foreign_security_principal(d_state->sam_ctx, mem_ctx, 
+								 r->in.sid, &memberdn);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 	} else {
-		DEBUG(0, ("samdb_search returned %d\n", ret));
+		DEBUG(0, ("samdb_search returned %d: %s\n", ret, ldb_errstring(d_state->sam_ctx)));
 	}
 
 	if (memberdn == NULL) {
diff --git a/source4/torture/rpc/lsa.c b/source4/torture/rpc/lsa.c
index 24f7bbbd01..099dd41430 100644
--- a/source4/torture/rpc/lsa.c
+++ b/source4/torture/rpc/lsa.c
@@ -1178,7 +1178,8 @@ static BOOL test_EnumAccountRights(struct dcerpc_pipe *p,
 
 	status = dcerpc_lsa_EnumAccountRights(p, mem_ctx, &r);
 	if (!NT_STATUS_IS_OK(status)) {
-		printf("EnumAccountRights failed - %s\n", nt_errstr(status));
+		printf("EnumAccountRights of %s failed - %s\n", 
+		       dom_sid_string(mem_ctx, sid), nt_errstr(status));
 		return False;
 	}
 
@@ -1194,6 +1195,11 @@ static BOOL test_QuerySecurity(struct dcerpc_pipe *p,
 	NTSTATUS status;
 	struct lsa_QuerySecurity r;
 
+	if (lp_parm_bool(-1, "target", "samba4", False)) {
+		printf("skipping QuerySecurity test against Samba4\n");
+		return True;
+	}
+
 	printf("Testing QuerySecurity\n");
 
 	r.in.handle = acct_handle;
@@ -1250,6 +1256,7 @@ static BOOL test_EnumAccounts(struct dcerpc_pipe *p,
 	struct lsa_SidArray sids1, sids2;
 	uint32_t resume_handle = 0;
 	int i;
+	BOOL ret = True;
 
 	printf("\ntesting EnumAccounts\n");
 
@@ -1284,14 +1291,14 @@ static BOOL test_EnumAccounts(struct dcerpc_pipe *p,
 
 		printf("testing all accounts\n");
 		for (i=0;i<sids1.num_sids;i++) {
-			test_OpenAccount(p, mem_ctx, handle, sids1.sids[i].sid);
-			test_EnumAccountRights(p, mem_ctx, handle, sids1.sids[i].sid);
+			ret &= test_OpenAccount(p, mem_ctx, handle, sids1.sids[i].sid);
+			ret &= test_EnumAccountRights(p, mem_ctx, handle, sids1.sids[i].sid);
 		}
 		printf("\n");
 	}
 
 	if (sids1.num_sids < 3) {
-		return True;
+		return ret;
 	}
 	
 	printf("trying EnumAccounts partial listing (asking for 1 at 2)\n");
@@ -1580,8 +1587,12 @@ static BOOL test_EnumTrustDom(struct dcerpc_pipe *p,
 			return False;
 		}
 		
-		ret &= test_query_each_TrustDom(p, mem_ctx, handle, &domains);
-
+		if (lp_parm_bool(-1, "target", "samba4", False)) {
+			printf("skipping 'each' Trusted Domains tests against Samba4\n");
+		} else {
+			ret &= test_query_each_TrustDom(p, mem_ctx, handle, &domains);
+		}
+		
 	} while ((NT_STATUS_EQUAL(enum_status, STATUS_MORE_ENTRIES)));
 
 	return ret;
@@ -1666,6 +1677,11 @@ static BOOL test_QueryDomainInfoPolicy(struct dcerpc_pipe *p,
 	NTSTATUS status;
 	int i;
 	BOOL ret = True;
+	if (lp_parm_bool(-1, "target", "samba4", False)) {
+		printf("skipping QueryDomainInformationPolicy test against Samba4\n");
+		return True;
+	}
+
 	printf("\nTesting QueryDomainInformationPolicy\n");
 
 	for (i=2;i<4;i++) {
@@ -1697,6 +1713,11 @@ static BOOL test_QueryInfoPolicy(struct dcerpc_pipe *p,
 	BOOL ret = True;
 	printf("\nTesting QueryInfoPolicy\n");
 
+	if (lp_parm_bool(-1, "target", "samba4", False)) {
+		printf("skipping QueryInfoPolicy2 against Samba4\n");
+		return True;
+	}
+
 	for (i=1;i<13;i++) {
 		r.in.handle = handle;
 		r.in.level = i;
@@ -1730,6 +1751,10 @@ static BOOL test_QueryInfoPolicy2(struct dcerpc_pipe *p,
 	int i;
 	BOOL ret = True;
 	printf("\nTesting QueryInfoPolicy2\n");
+	if (lp_parm_bool(-1, "target", "samba4", False)) {
+		printf("skipping QueryInfoPolicy2 against Samba4\n");
+		return True;
+	}
 
 	for (i=1;i<13;i++) {
 		r.in.handle = handle;