r7860: switch our ldb storage format to use a NDR encoded objectSid. This is

quite a large change as we had lots of code that assumed that
objectSid was a string in S- format.

metze and simo tried to convince me to use NDR format months ago, but
I didn't listen, so its fair that I have the pain of fixing all the
code now :-)

This builds on the ldb_register_samba_handlers() and ldif handlers
code I did earlier this week. There are still three parts of this
conversion I have not finished:

 - the ltdb index records need to use the string form of the objectSid
   (to keep the DNs sane). Until that it done I have disabled indexing on
   objectSid, which is a big performance hit, but allows us to pass
   all our tests while I rejig the indexing system to use a externally
   supplied conversion function

 - I haven't yet put in place the code that allows client to use the
   "S-xxx-yyy" form for objectSid in ldap search expressions. w2k3
   supports this, presumably by looking for the "S-" prefix to
   determine what type of objectSid form is being used by the client. I
   have been working on ways to handle this, but am not happy with
   them yet so they aren't part of this patch

 - I need to change pidl to generate push functions that take a
   "const void *" instead of a "void*" for the data pointer. That will
   fix the couple of new warnings this code generates.

Luckily it many places the conversion to NDR formatted records
actually simplified the code, as it means we no longer need as many
calls to dom_sid_parse_talloc(). In some places it got more complex,
but not many.
(This used to be commit d40bc2fa8ddd43560315688eebdbe98bdd02756c)

3 files changed, 118 insertions, 43 deletions

diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index 5472bed107..b5440c3cd1 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -35,7 +35,8 @@
 #include "includes.h"
 #include "lib/ldb/include/ldb.h"
 #include "lib/ldb/include/ldb_private.h"
-#include <time.h>
+#include "system/time.h"
+#include "librpc/gen_ndr/ndr_security.h"
 
 #define SAM_ACCOUNT_NAME_BASE "$000000-000000000000"
 
@@ -169,14 +170,15 @@ static char *samldb_search_domain(struct ldb_module *module, TALLOC_CTX *mem_ctx
    allocate a new RID for the domain
    return the new sid string
 */
-static char *samldb_get_new_sid(struct ldb_module *module, TALLOC_CTX *mem_ctx, const char *obj_dn)
+static struct dom_sid *samldb_get_new_sid(struct ldb_module *module, 
+					  TALLOC_CTX *mem_ctx, const char *obj_dn)
 {
 	const char * const attrs[2] = { "objectSid", NULL };
 	struct ldb_message **res = NULL;
-	const char *dom_dn, *dom_sid;
-	char *obj_sid;
+	const char *dom_dn;
 	uint32_t rid;
 	int ret, tries = 10;
+	struct dom_sid *dom_sid, *obj_sid;
 
 	/* get the domain component part of the provided dn */
 
@@ -197,11 +199,11 @@ static char *samldb_get_new_sid(struct ldb_module *module, TALLOC_CTX *mem_ctx,
 	ret = ldb_search(module->ldb, dom_dn, LDB_SCOPE_BASE, "objectSid=*", attrs, &res);
 	if (ret != 1) {
 		ldb_debug(module->ldb, LDB_DEBUG_FATAL, "samldb_get_new_sid: error retrieving domain sid!\n");
-		if (res) talloc_free(res);
+		talloc_free(res);
 		return NULL;
 	}
 
-	dom_sid = ldb_msg_find_string(res[0], "objectSid", NULL);
+	dom_sid = samdb_result_dom_sid(res, res[0], "objectSid");
 	if (dom_sid == NULL) {
 		ldb_debug(module->ldb, LDB_DEBUG_FATAL, "samldb_get_new_sid: error retrieving domain sid!\n");
 		talloc_free(res);
@@ -225,12 +227,10 @@ static char *samldb_get_new_sid(struct ldb_module *module, TALLOC_CTX *mem_ctx,
 	}
 
 	/* return the new object sid */
-
-	obj_sid = talloc_asprintf(mem_ctx, "%s-%u", dom_sid, rid);
+	obj_sid = dom_sid_add_rid(mem_ctx, dom_sid, rid);
 
 	talloc_free(res);
 
-
 	return obj_sid;
 }
 
@@ -307,6 +307,18 @@ static BOOL samldb_msg_add_string(struct ldb_module *module, struct ldb_message
 	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;
+	NTSTATUS status;
+	status = ndr_push_struct_blob(&v, msg, sid, 
+				      (ndr_push_flags_fn_t)ndr_push_dom_sid);
+	if (!NT_STATUS_IS_OK(status)) {
+		return -1;
+	}
+	return (ldb_msg_add_value(module->ldb, msg, name, &v) == 0);
+}
+
 static BOOL samldb_find_or_add_attribute(struct ldb_module *module, struct ldb_message *msg, const char *name, const char *value, const char *set_value)
 {
 	if (samldb_find_attribute(msg, name, value) == NULL) {
@@ -367,7 +379,7 @@ static struct ldb_message *samldb_fill_group_object(struct ldb_module *module, c
 {
 	struct ldb_message *msg2;
 	struct ldb_message_element *attribute;
-	char *rdn, *basedn, *sidstr;
+	char *rdn, *basedn;
 
 	if (samldb_find_attribute(msg, "objectclass", "group") == NULL) {
 		return NULL;
@@ -418,15 +430,17 @@ static struct ldb_message *samldb_fill_group_object(struct ldb_module *module, c
 	}
 
 	if ((attribute = samldb_find_attribute(msg2, "objectSid", NULL)) == NULL ) {
-
-		if ((sidstr = samldb_get_new_sid(module, msg2, msg2->dn)) == NULL) {
+		struct dom_sid *sid = samldb_get_new_sid(module, msg2, msg2->dn);
+		if (sid == NULL) {
 			ldb_debug(module->ldb, LDB_DEBUG_FATAL, "samldb_fill_group_object: internal error! Can't generate new sid\n");
 			return NULL;
 		}
 
-		if ( ! samldb_msg_add_string(module, msg2, "objectSid", sidstr)) {
+		if (!samldb_msg_add_sid(module, msg2, "objectSid", sid)) {
+			talloc_free(sid);
 			return NULL;
 		}
+		talloc_free(sid);
 	}
 
 	if ( ! samldb_find_or_add_attribute(module, msg2, "sAMAccountName", NULL, samldb_generate_samAccountName(msg2))) {
@@ -444,7 +458,7 @@ static struct ldb_message *samldb_fill_user_or_computer_object(struct ldb_module
 {
 	struct ldb_message *msg2;
 	struct ldb_message_element *attribute;
-	char *rdn, *basedn, *sidstr;
+	char *rdn, *basedn;
 
 	if ((samldb_find_attribute(msg, "objectclass", "user") == NULL) && (samldb_find_attribute(msg, "objectclass", "computer") == NULL)) {
 		return NULL;
@@ -500,15 +514,18 @@ static struct ldb_message *samldb_fill_user_or_computer_object(struct ldb_module
 	}
 
 	if ((attribute = samldb_find_attribute(msg2, "objectSid", NULL)) == NULL ) {
-
-		if ((sidstr = samldb_get_new_sid(module, msg2, msg2->dn)) == NULL) {
+		struct dom_sid *sid;
+		sid = samldb_get_new_sid(module, msg2, msg2->dn);
+		if (sid == NULL) {
 			ldb_debug(module->ldb, LDB_DEBUG_FATAL, "samldb_fill_user_or_computer_object: internal error! Can't generate new sid\n");
 			return NULL;
 		}
 
-		if ( ! samldb_msg_add_string(module, msg2, "objectSid", sidstr)) {
+		if ( ! samldb_msg_add_sid(module, msg2, "objectSid", sid)) {
+			talloc_free(sid);
 			return NULL;
 		}
+		talloc_free(sid);
 	}
 
 	if ( ! samldb_find_or_add_attribute(module, msg2, "sAMAccountName", NULL, samldb_generate_samAccountName(msg2))) {
diff --git a/source4/dsdb/samdb/samdb.c b/source4/dsdb/samdb/samdb.c
index 5f9764ce42..e2426738da 100644
--- a/source4/dsdb/samdb/samdb.c
+++ b/source4/dsdb/samdb/samdb.c
@@ -61,17 +61,17 @@ int samdb_search_domain(struct ldb_context *sam_ldb,
 	while (i<count) {
 		struct dom_sid *entry_sid;
 
-		entry_sid = samdb_result_dom_sid(mem_ctx, (*res)[i],
-						 "objectSid");
+		entry_sid = samdb_result_dom_sid(mem_ctx, (*res)[i], "objectSid");
 
 		if ((entry_sid == NULL) ||
 		    (!dom_sid_in_domain(domain_sid, entry_sid))) {
-
 			/* Delete that entry from the result set */
 			(*res)[i] = (*res)[count-1];
 			count -= 1;
+			talloc_free(entry_sid);
 			continue;
 		}
+		talloc_free(entry_sid);
 		i += 1;
 	}
 
@@ -125,6 +125,37 @@ const char *samdb_search_string(struct ldb_context *sam_ldb,
 }
 
 /*
+  search the sam for a dom_sid attribute in exactly 1 record
+*/
+struct dom_sid *samdb_search_dom_sid(struct ldb_context *sam_ldb,
+				     TALLOC_CTX *mem_ctx,
+				     const char *basedn,
+				     const char *attr_name,
+				     const char *format, ...) _PRINTF_ATTRIBUTE(5,6)
+{
+	va_list ap;
+	int count;
+	struct ldb_message **res;
+	const char * const attrs[2] = { attr_name, NULL };
+	struct dom_sid *sid;
+
+	va_start(ap, format);
+	count = gendb_search_v(sam_ldb, mem_ctx, basedn, &res, attrs, format, ap);
+	va_end(ap);
+	if (count > 1) {		
+		DEBUG(1,("samdb: search for %s %s not single valued (count=%d)\n", 
+			 attr_name, format, count));
+	}
+	if (count != 1) {
+		talloc_free(res);
+		return NULL;
+	}
+	sid = samdb_result_dom_sid(mem_ctx, res[0], attr_name);
+	talloc_free(res);
+	return sid;	
+}
+
+/*
   return the count of the number of records in the sam matching the query
 */
 int samdb_search_count(struct ldb_context *sam_ldb,
@@ -274,16 +305,18 @@ const char *samdb_result_string(struct ldb_message *msg, const char *attr,
   pull a rid from a objectSid in a result set. 
 */
 uint32_t samdb_result_rid_from_sid(TALLOC_CTX *mem_ctx, struct ldb_message *msg, 
-				 const char *attr, uint32_t default_value)
+				   const char *attr, uint32_t default_value)
 {
 	struct dom_sid *sid;
-	const char *sidstr = ldb_msg_find_string(msg, attr, NULL);
-	if (!sidstr) return default_value;
-
-	sid = dom_sid_parse_talloc(mem_ctx, sidstr);
-	if (!sid) return default_value;
+	uint32_t rid;
 
-	return sid->sub_auths[sid->num_auths-1];
+	sid = samdb_result_dom_sid(mem_ctx, msg, attr);
+	if (sid == NULL) {
+		return default_value;
+	}
+	rid = sid->sub_auths[sid->num_auths-1];
+	talloc_free(sid);
+	return rid;
 }
 
 /*
@@ -292,10 +325,24 @@ uint32_t samdb_result_rid_from_sid(TALLOC_CTX *mem_ctx, struct ldb_message *msg,
 struct dom_sid *samdb_result_dom_sid(TALLOC_CTX *mem_ctx, struct ldb_message *msg, 
 				     const char *attr)
 {
-	const char *sidstr = ldb_msg_find_string(msg, attr, NULL);
-	if (!sidstr) return NULL;
-
-	return dom_sid_parse_talloc(mem_ctx, sidstr);
+	const struct ldb_val *v;
+	struct dom_sid *sid;
+	NTSTATUS status;
+	v = ldb_msg_find_ldb_val(msg, attr);
+	if (v == NULL) {
+		return NULL;
+	}
+	sid = talloc(mem_ctx, struct dom_sid);
+	if (sid == NULL) {
+		return NULL;
+	}
+	status = ndr_pull_struct_blob(v, sid, sid, 
+				      (ndr_pull_flags_fn_t)ndr_pull_dom_sid);
+	if (!NT_STATUS_IS_OK(status)) {
+		talloc_free(sid);
+		return NULL;
+	}
+	return sid;
 }
 
 /*
@@ -324,15 +371,13 @@ struct GUID samdb_result_guid(struct ldb_message *msg, const char *attr)
   pull a sid prefix from a objectSid in a result set. 
   this is used to find the domain sid for a user
 */
-const char *samdb_result_sid_prefix(TALLOC_CTX *mem_ctx, struct ldb_message *msg, 
-				    const char *attr)
+struct dom_sid *samdb_result_sid_prefix(TALLOC_CTX *mem_ctx, struct ldb_message *msg, 
+					const char *attr)
 {
 	struct dom_sid *sid = samdb_result_dom_sid(mem_ctx, msg, attr);
 	if (!sid || sid->num_auths < 1) return NULL;
-
 	sid->num_auths--;
-
-	return dom_sid_string(mem_ctx, sid);
+	return sid;
 }
 
 /*
@@ -704,6 +749,22 @@ int samdb_msg_add_string(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struc
 }
 
 /*
+  add a dom_sid element to a message
+*/
+int samdb_msg_add_dom_sid(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct ldb_message *msg,
+			 const char *attr_name, struct dom_sid *sid)
+{
+	struct ldb_val v;
+	NTSTATUS status;
+	status = ndr_push_struct_blob(&v, mem_ctx, sid, 
+				      (ndr_push_flags_fn_t)ndr_push_dom_sid);
+	if (!NT_STATUS_IS_OK(status)) {
+		return -1;
+	}
+	return ldb_msg_add_value(sam_ldb, msg, attr_name, &v);
+}
+
+/*
   add a delete element operation to a message
 */
 int samdb_msg_add_delete(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct ldb_message *msg,
diff --git a/source4/dsdb/samdb/samdb_privilege.c b/source4/dsdb/samdb/samdb_privilege.c
index 77ddcbbdcd..bfd37f6417 100644
--- a/source4/dsdb/samdb/samdb_privilege.c
+++ b/source4/dsdb/samdb/samdb_privilege.c
@@ -31,29 +31,26 @@ static NTSTATUS samdb_privilege_setup_sid(void *samctx, TALLOC_CTX *mem_ctx,
 					  const struct dom_sid *sid, 
 					  uint64_t *mask)
 {
-	char *sidstr;
 	const char * const attrs[] = { "privilege", NULL };
 	struct ldb_message **res = NULL;
 	struct ldb_message_element *el;
 	int ret, i;
+	char *sidstr;
 	
 	*mask = 0;
 
-	sidstr = dom_sid_string(mem_ctx, sid);
-	if (sidstr == NULL) {
-		return NT_STATUS_NO_MEMORY;
-	}
+	sidstr = ldap_encode_ndr_dom_sid(mem_ctx, sid);
+	NT_STATUS_HAVE_NO_MEMORY(sidstr);
 
 	ret = gendb_search(samctx, mem_ctx, NULL, &res, attrs, "objectSid=%s", sidstr);
+	talloc_free(sidstr);
 	if (ret != 1) {
-		talloc_free(sidstr);
 		/* not an error to not match */
 		return NT_STATUS_OK;
 	}
 
 	el = ldb_msg_find_element(res[0], "privilege");
 	if (el == NULL) {
-		talloc_free(sidstr);
 		return NT_STATUS_OK;
 	}