From a83431e659544c5ede87d2662686e36759aebe6e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 10:34:58 +0100 Subject: [PATCH 01/13] s4:dsdb/schema: fix dsdb_schema_set_el_from_ldb_msg() (bug #9470) We should always update the ts_last_change. Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit 944b6863a71efc48ccc8cd9ae8ad1a3081bc1805) --- source4/dsdb/schema/schema_set.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c index e226118..31862a4 100644 --- a/source4/dsdb/schema/schema_set.c +++ b/source4/dsdb/schema/schema_set.c @@ -676,13 +676,6 @@ WERROR dsdb_schema_set_el_from_ldb_msg(struct ldb_context *ldb, struct dsdb_sche { const char* tstring; time_t ts; - if (samdb_find_attribute(ldb, msg, - "objectclass", "attributeSchema") != NULL) { - return dsdb_set_attribute_from_ldb(ldb, schema, msg); - } else if (samdb_find_attribute(ldb, msg, - "objectclass", "classSchema") != NULL) { - return dsdb_set_class_from_ldb(schema, msg); - } tstring = ldb_msg_find_attr_as_string(msg, "whenChanged", NULL); /* keep a trace of the ts of the most recently changed object */ if (tstring) { @@ -691,6 +684,13 @@ WERROR dsdb_schema_set_el_from_ldb_msg(struct ldb_context *ldb, struct dsdb_sche schema->ts_last_change = ts; } } + if (samdb_find_attribute(ldb, msg, + "objectclass", "attributeSchema") != NULL) { + return dsdb_set_attribute_from_ldb(ldb, schema, msg); + } else if (samdb_find_attribute(ldb, msg, + "objectclass", "classSchema") != NULL) { + return dsdb_set_class_from_ldb(schema, msg); + } /* Don't fail on things not classes or attributes */ return WERR_OK; } -- 1.7.9.5 From 77e22abdd4219df037a4753536a8353f03c05aed Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 10:08:14 +0000 Subject: [PATCH 02/13] s4:dsdb/schema_data.c: correctly move the CN=Aggregate attributes to msg->elements[i].values (bug #9470) We should keep the talloc hierarchy sane. Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit 3535f8effefef6a68d2b686abe2769d797531dd9) --- source4/dsdb/samdb/ldb_modules/schema_data.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/schema_data.c b/source4/dsdb/samdb/ldb_modules/schema_data.c index bc9488b..3ce7ef9 100644 --- a/source4/dsdb/samdb/ldb_modules/schema_data.c +++ b/source4/dsdb/samdb/ldb_modules/schema_data.c @@ -405,7 +405,11 @@ static int generate_objectClasses(struct ldb_context *ldb, struct ldb_message *m int ret; for (sclass = schema->classes; sclass; sclass = sclass->next) { - ret = ldb_msg_add_string(msg, "objectClasses", schema_class_to_description(msg, sclass)); + char *v = schema_class_to_description(msg, sclass); + if (v == NULL) { + return ldb_oom(ldb); + } + ret = ldb_msg_add_steal_string(msg, "objectClasses", v); if (ret != LDB_SUCCESS) { return ret; } @@ -417,9 +421,13 @@ static int generate_attributeTypes(struct ldb_context *ldb, struct ldb_message * { const struct dsdb_attribute *attribute; int ret; - + for (attribute = schema->attributes; attribute; attribute = attribute->next) { - ret = ldb_msg_add_string(msg, "attributeTypes", schema_attribute_to_description(msg, attribute)); + char *v = schema_attribute_to_description(msg, attribute); + if (v == NULL) { + return ldb_oom(ldb); + } + ret = ldb_msg_add_steal_string(msg, "attributeTypes", v); if (ret != LDB_SUCCESS) { return ret; } @@ -461,7 +469,7 @@ static int generate_extendedAttributeInfo(struct ldb_context *ldb, return ldb_oom(ldb); } - ret = ldb_msg_add_string(msg, "extendedAttributeInfo", val); + ret = ldb_msg_add_steal_string(msg, "extendedAttributeInfo", val); if (ret != LDB_SUCCESS) { return ret; } @@ -483,7 +491,7 @@ static int generate_extendedClassInfo(struct ldb_context *ldb, return ldb_oom(ldb); } - ret = ldb_msg_add_string(msg, "extendedClassInfo", val); + ret = ldb_msg_add_steal_string(msg, "extendedClassInfo", val); if (ret != LDB_SUCCESS) { return ret; } @@ -521,7 +529,11 @@ static int generate_possibleInferiors(struct ldb_context *ldb, struct ldb_messag } for (i=0;possibleInferiors[i];i++) { - ret = ldb_msg_add_string(msg, "possibleInferiors", possibleInferiors[i]); + char *v = talloc_strdup(msg, possibleInferiors[i]); + if (v == NULL) { + return ldb_oom(ldb); + } + ret = ldb_msg_add_steal_string(msg, "possibleInferiors", v); if (ret != LDB_SUCCESS) { return ret; } -- 1.7.9.5 From 0f10b380c0fb4b1e4ff6f7f9024864daa9c33dc5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 11:02:49 +0100 Subject: [PATCH 03/13] s4:dsdb/acl_read: keep the ldb_message of the sub search (bug #9470) Some modules might not allocate values on the correct memory context. Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit 14b5b729049d92c30ba518adb82c9396fdddd09f) --- source4/dsdb/samdb/ldb_modules/acl_read.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c index 92744f2..0b0f363 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_read.c +++ b/source4/dsdb/samdb/ldb_modules/acl_read.c @@ -245,6 +245,11 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) k++; } } + /* + * This should not be needed, but some modules + * may allocate values on the wrong context... + */ + talloc_steal(ret_msg->elements, msg); } else { ret_msg->elements = NULL; } -- 1.7.9.5 From 0dbd7d60fad0cce5975808b84ff3c96d52c71307 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 13:39:31 +0100 Subject: [PATCH 04/13] s4:dsdb/acl_read: improve debugging for fatal error Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit 802124789513ef207a154ee950dc03e66a80e0b1) --- source4/dsdb/samdb/ldb_modules/acl_read.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c index 0b0f363..2b20f24 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_read.c +++ b/source4/dsdb/samdb/ldb_modules/acl_read.c @@ -91,7 +91,9 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) msg = ares->message; ret = dsdb_get_sd_from_ldb_message(ldb, tmp_ctx, msg, &sd); if (ret != LDB_SUCCESS || sd == NULL ) { - DEBUG(10, ("acl_read: cannot get descriptor\n")); + ldb_debug_set(ldb, LDB_DEBUG_FATAL, + "acl_read: cannot get descriptor of %s\n", + ldb_dn_get_linearized(msg->dn)); ret = LDB_ERR_OPERATIONS_ERROR; goto fail; } @@ -113,6 +115,11 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) talloc_free(tmp_ctx); return LDB_SUCCESS; } else if (ret != LDB_SUCCESS) { + ldb_debug_set(ldb, LDB_DEBUG_FATAL, + "acl_read: %s check parent %s - %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_strerror(ret), + ldb_errstring(ldb)); goto fail; } } @@ -124,8 +131,10 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) attr = dsdb_attribute_by_lDAPDisplayName(ac->schema, msg->elements[i].name); if (!attr) { - DEBUG(2, ("acl_read: cannot find attribute %s in schema\n", - msg->elements[i].name)); + ldb_debug_set(ldb, LDB_DEBUG_FATAL, + "acl_read: %s cannot find attr[%s] in of schema\n", + ldb_dn_get_linearized(msg->dn), + msg->elements[i].name); ret = LDB_ERR_OPERATIONS_ERROR; goto fail; } @@ -216,6 +225,12 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) } } } else if (ret != LDB_SUCCESS) { + ldb_debug_set(ldb, LDB_DEBUG_FATAL, + "acl_read: %s check attr[%s] gives %s - %s\n", + ldb_dn_get_linearized(msg->dn), + msg->elements[i].name, + ldb_strerror(ret), + ldb_errstring(ldb)); goto fail; } } -- 1.7.9.5 From c60e441bc1f1170fb415f01eb38a2fbc6beacf79 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 12:56:21 +0000 Subject: [PATCH 05/13] s4:dsdb/descriptor: fix replication of NC heads The sub NC heads maybe replicated with the parent partition, if we don't need to recalculate the nTSecurityDescriptor attribute in that case, the replication of the of the sub partition should handle that. This fixes error messages like this: descriptor_sd_propagation_recursive: DC=ForestDnsZones,DC=s40dom,DC=base not found under DC=s40dom,DC=base Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit 734d14b54834a4d03e67bcaece4f4e3cf1d10925) --- source4/dsdb/samdb/ldb_modules/descriptor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c index 95204b3..192c745 100644 --- a/source4/dsdb/samdb/ldb_modules/descriptor.c +++ b/source4/dsdb/samdb/ldb_modules/descriptor.c @@ -1192,12 +1192,12 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module, msg); if (msg == NULL) { - ldb_debug_set(ldb, LDB_DEBUG_FATAL, + ldb_debug(ldb, LDB_DEBUG_WARNING, "descriptor_sd_propagation_recursive: " "%s not found under %s", ldb_dn_get_linearized(c->dn), ldb_dn_get_linearized(change->dn)); - return LDB_ERR_OPERATIONS_ERROR; + continue; } msg->elements = (struct ldb_message_element *)c; -- 1.7.9.5 From 5f86918ea27de4a1ff8cb6ff038a1756ec95c8c3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 19:02:10 +0100 Subject: [PATCH 06/13] s4:dsdb/dirsync: fix potential talloc hierachy problems (bug #9470) Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit 6bcafceb750d5c4d24e2ddbef35b411bebccd66f) --- source4/dsdb/samdb/ldb_modules/dirsync.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c index f75ec52..e769948 100644 --- a/source4/dsdb/samdb/ldb_modules/dirsync.c +++ b/source4/dsdb/samdb/ldb_modules/dirsync.c @@ -240,7 +240,7 @@ static int dirsync_filter_entry(struct ldb_request *req, talloc_steal(newmsg->elements, el->name); talloc_steal(newmsg->elements, el->values); - talloc_free(msg); + talloc_steal(newmsg->elements, msg); return ldb_module_send_entry(dsc->req, msg, controls); } @@ -653,6 +653,7 @@ skip_link: continue; } } + talloc_steal(newmsg->elements, msg); /* * Here we run through the list of attributes returned @@ -685,10 +686,9 @@ skip_link: if (val > dsc->highestUSN) { dsc->highestUSN = val; } - talloc_free(msg); return ldb_module_send_entry(dsc->req, newmsg, controls); } else { - talloc_free(msg); + talloc_free(newmsg); return LDB_SUCCESS; } } -- 1.7.9.5 From 9e76b56b6621813ab0ad710329ea7917ad109095 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 18:39:29 +0100 Subject: [PATCH 07/13] s4:dsdb/acl_read: check the ldb_attr_list_copy_add() result Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit e2181617a00d7982e4e6ced1c51aa2ee8a40df26) --- source4/dsdb/samdb/ldb_modules/acl_read.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c index 2b20f24..c42db5f 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_read.c +++ b/source4/dsdb/samdb/ldb_modules/acl_read.c @@ -375,12 +375,18 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) if (!ldb_attr_in_list(req->op.search.attrs, "instanceType")) { ac->instance_type = true; attrs = ldb_attr_list_copy_add(ac, req->op.search.attrs, "instanceType"); + if (attrs == NULL) { + return ldb_oom(ldb); + } } else { attrs = req->op.search.attrs; } if (!ldb_attr_in_list(req->op.search.attrs, "objectSid")) { ac->object_sid = true; attrs = ldb_attr_list_copy_add(ac, attrs, "objectSid"); + if (attrs == NULL) { + return ldb_oom(ldb); + } } } @@ -389,8 +395,14 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) * if attribute list is empty */ if (!attrs) { attrs = ldb_attr_list_copy_add(ac, req->op.search.attrs, "*"); + if (attrs == NULL) { + return ldb_oom(ldb); + } } attrs = ldb_attr_list_copy_add(ac, attrs, "nTSecurityDescriptor"); + if (attrs == NULL) { + return ldb_oom(ldb); + } } ac->attrs = req->op.search.attrs; ret = ldb_build_search_req_ex(&down_req, -- 1.7.9.5 From f4c63b44bbdbb76b4c616fd6204ee7707426fd0e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 18:40:25 +0100 Subject: [PATCH 08/13] s4:dsdb/acl_read: fix the calculation of the attribute array for the sub search Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit db15fcfa899e1fe4d6994f68ceb299921b8aa6f1) --- source4/dsdb/samdb/ldb_modules/acl_read.c | 33 +++++++++++++++++------------ 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c index c42db5f..e4adcde 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_read.c +++ b/source4/dsdb/samdb/ldb_modules/acl_read.c @@ -296,6 +296,8 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) struct ldb_result *res; struct aclread_private *p; bool is_untrusted = ldb_req_is_untrusted(req); + static const char * const _all_attrs[] = { "*", NULL }; + bool all_attrs = false; const char * const *attrs = NULL; uint32_t instanceType; static const char *acl_attrs[] = { @@ -363,6 +365,18 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) if (!ac->schema) { return ldb_operr(ldb); } + + attrs = req->op.search.attrs; + if (attrs == NULL) { + all_attrs = true; + attrs = _all_attrs; + } else if (attrs[0] == NULL) { + all_attrs = true; + attrs = _all_attrs; + } else if (ldb_attr_in_list(attrs, "*")) { + all_attrs = true; + } + /* * In theory we should also check for the SD control but control verification is * expensive so we'd better had the ntsecuritydescriptor to the list of @@ -370,16 +384,15 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) */ ac->sd_flags = dsdb_request_sd_flags(ac->req, NULL); - ac->sd = !(ldb_attr_in_list(req->op.search.attrs, "nTSecurityDescriptor")); - if (req->op.search.attrs && !ldb_attr_in_list(req->op.search.attrs, "*")) { - if (!ldb_attr_in_list(req->op.search.attrs, "instanceType")) { + ac->sd = !(ldb_attr_in_list(attrs, "nTSecurityDescriptor")); + + if (!all_attrs) { + if (!ldb_attr_in_list(attrs, "instanceType")) { ac->instance_type = true; - attrs = ldb_attr_list_copy_add(ac, req->op.search.attrs, "instanceType"); + attrs = ldb_attr_list_copy_add(ac, attrs, "instanceType"); if (attrs == NULL) { return ldb_oom(ldb); } - } else { - attrs = req->op.search.attrs; } if (!ldb_attr_in_list(req->op.search.attrs, "objectSid")) { ac->object_sid = true; @@ -391,14 +404,6 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) } if (ac->sd) { - /* avoid replacing all attributes with nTSecurityDescriptor - * if attribute list is empty */ - if (!attrs) { - attrs = ldb_attr_list_copy_add(ac, req->op.search.attrs, "*"); - if (attrs == NULL) { - return ldb_oom(ldb); - } - } attrs = ldb_attr_list_copy_add(ac, attrs, "nTSecurityDescriptor"); if (attrs == NULL) { return ldb_oom(ldb); -- 1.7.9.5 From 720b87deadff04401a1f327998ff918fc4e54773 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 6 Dec 2012 12:29:49 +0100 Subject: [PATCH 09/13] s4:dsdb/acl_read: give some variables a better name Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit 4f8558ffaf4c9fb9e350ec528ec1ce60de5f2e24) --- source4/dsdb/samdb/ldb_modules/acl_read.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c index e4adcde..787e3ef 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_read.c +++ b/source4/dsdb/samdb/ldb_modules/acl_read.c @@ -45,9 +45,9 @@ struct aclread_context { const char * const *attrs; const struct dsdb_schema *schema; uint32_t sd_flags; - bool sd; - bool instance_type; - bool object_sid; + bool added_nTSecurityDescriptor; + bool added_instanceType; + bool added_objectSid; bool indirsync; }; @@ -145,15 +145,15 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) is_instancetype = ldb_attr_cmp("instanceType", msg->elements[i].name) == 0; /* these attributes were added to perform access checks and must be removed */ - if (is_objectsid && ac->object_sid) { + if (is_objectsid && ac->added_objectSid) { aclread_mark_inaccesslible(&msg->elements[i]); continue; } - if (is_instancetype && ac->instance_type) { + if (is_instancetype && ac->added_instanceType) { aclread_mark_inaccesslible(&msg->elements[i]); continue; } - if (is_sd && ac->sd) { + if (is_sd && ac->added_nTSecurityDescriptor) { aclread_mark_inaccesslible(&msg->elements[i]); continue; } @@ -295,6 +295,7 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) uint32_t flags = ldb_req_get_custom_flags(req); struct ldb_result *res; struct aclread_private *p; + bool need_sd = false; bool is_untrusted = ldb_req_is_untrusted(req); static const char * const _all_attrs[] = { "*", NULL }; bool all_attrs = false; @@ -384,31 +385,33 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) */ ac->sd_flags = dsdb_request_sd_flags(ac->req, NULL); - ac->sd = !(ldb_attr_in_list(attrs, "nTSecurityDescriptor")); + need_sd = !(ldb_attr_in_list(attrs, "nTSecurityDescriptor")); if (!all_attrs) { if (!ldb_attr_in_list(attrs, "instanceType")) { - ac->instance_type = true; attrs = ldb_attr_list_copy_add(ac, attrs, "instanceType"); if (attrs == NULL) { return ldb_oom(ldb); } + ac->added_instanceType = true; } if (!ldb_attr_in_list(req->op.search.attrs, "objectSid")) { - ac->object_sid = true; attrs = ldb_attr_list_copy_add(ac, attrs, "objectSid"); if (attrs == NULL) { return ldb_oom(ldb); } + ac->added_objectSid = true; } } - if (ac->sd) { + if (need_sd) { attrs = ldb_attr_list_copy_add(ac, attrs, "nTSecurityDescriptor"); if (attrs == NULL) { return ldb_oom(ldb); } + ac->added_nTSecurityDescriptor = true; } + ac->attrs = req->op.search.attrs; ret = ldb_build_search_req_ex(&down_req, ldb, ac, -- 1.7.9.5 From 81ad6e83e3f86e9c13bacac143f7d9ee2ae1f3f6 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 6 Dec 2012 12:36:09 +0100 Subject: [PATCH 10/13] s4:dsdb/acl_read: return the nTSecurityDescriptor attr if the sd_flags control is given (bug #9470) Not returning the nTSecurityDescriptor causes a lot of problems. Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit 22bb2fd868b8df2244b801aeaa515a8a4036bce8) --- source4/dsdb/samdb/ldb_modules/acl_read.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c index 787e3ef..9955451 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_read.c +++ b/source4/dsdb/samdb/ldb_modules/acl_read.c @@ -296,6 +296,7 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) struct ldb_result *res; struct aclread_private *p; bool need_sd = false; + bool explicit_sd_flags = false; bool is_untrusted = ldb_req_is_untrusted(req); static const char * const _all_attrs[] = { "*", NULL }; bool all_attrs = false; @@ -383,9 +384,15 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) * expensive so we'd better had the ntsecuritydescriptor to the list of * searched attribute and then remove it ! */ - ac->sd_flags = dsdb_request_sd_flags(ac->req, NULL); + ac->sd_flags = dsdb_request_sd_flags(ac->req, &explicit_sd_flags); - need_sd = !(ldb_attr_in_list(attrs, "nTSecurityDescriptor")); + if (ldb_attr_in_list(attrs, "nTSecurityDescriptor")) { + need_sd = false; + } else if (explicit_sd_flags && all_attrs) { + need_sd = false; + } else { + need_sd = true; + } if (!all_attrs) { if (!ldb_attr_in_list(attrs, "instanceType")) { -- 1.7.9.5 From 702acd8f3d097a25b2c17031ae7da1bc0fb5b840 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 6 Dec 2012 15:56:26 +0100 Subject: [PATCH 11/13] s4:dsdb/operational: fix stripping of the nTSecurityDescriptor attribute If the sd_flags control is specified, we should return nTSecurityDescriptor only if the client asked for all attributes. If there's a list of only explicit attribute names, we should ignore the sd_flags control. Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit 6bc2caed8b3f153f92af013275f39c803f886a22) --- source4/dsdb/samdb/ldb_modules/operational.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/operational.c b/source4/dsdb/samdb/ldb_modules/operational.c index 4ce8b8f..c642ad8 100644 --- a/source4/dsdb/samdb/ldb_modules/operational.c +++ b/source4/dsdb/samdb/ldb_modules/operational.c @@ -721,10 +721,20 @@ static int operational_search_post_process(struct ldb_module *module, continue; } case OPERATIONAL_SD_FLAGS: - if (controls_flags->sd || - ldb_attr_in_list(attrs_from_user, operational_remove[i].attr)) { + if (ldb_attr_in_list(attrs_from_user, operational_remove[i].attr)) { continue; } + if (controls_flags->sd) { + if (attrs_from_user == NULL) { + continue; + } + if (attrs_from_user[0] == NULL) { + continue; + } + if (ldb_attr_in_list(attrs_from_user, "*")) { + continue; + } + } ldb_msg_remove_attr(msg, operational_remove[i].attr); break; } -- 1.7.9.5 From 3dfe0a2e0655f377ded00eb431a5455119d2dc9d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 6 Dec 2012 14:04:47 +0100 Subject: [PATCH 12/13] s4:dsdb/tests/sec_descriptor: verify the nTSecurityDescriptor and sd_flags interaction This is a regression test for bug #9470. Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam (cherry picked from commit e617a3fecb797031cf5a6545d51d7e116716ab52) --- source4/dsdb/tests/python/sec_descriptor.py | 116 +++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/source4/dsdb/tests/python/sec_descriptor.py b/source4/dsdb/tests/python/sec_descriptor.py index aff6040..cf213ab 100755 --- a/source4/dsdb/tests/python/sec_descriptor.py +++ b/source4/dsdb/tests/python/sec_descriptor.py @@ -1848,6 +1848,122 @@ class SdFlagsDescriptorTests(DescriptorTests): self.assertFalse("S:" in desc_sddl) self.assertFalse("G:" in desc_sddl) + def test_311(self): + sd_flags = (SECINFO_OWNER | + SECINFO_GROUP | + SECINFO_DACL | + SECINFO_SACL) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + [], controls=None) + self.assertFalse("nTSecurityDescriptor" in res[0]) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["name"], controls=None) + self.assertFalse("nTSecurityDescriptor" in res[0]) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["name"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertFalse("nTSecurityDescriptor" in res[0]) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + [], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["*"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["nTSecurityDescriptor", "*"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["*", "nTSecurityDescriptor"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["nTSecurityDescriptor", "name"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["name", "nTSecurityDescriptor"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["nTSecurityDescriptor"], controls=None) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["name", "nTSecurityDescriptor"], controls=None) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["nTSecurityDescriptor", "name"], controls=None) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) class RightsAttributesTests(DescriptorTests): -- 1.7.9.5 From c1b9ef309187813aa2f4c55e0d532944df0d7970 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 18:58:57 +0100 Subject: [PATCH 13/13] s4:dsdb/tests/sec_descriptor: verify the search of a windows dc join keeps working This is a regression test for bug #9470. Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam Autobuild-User(master): Michael Adam Autobuild-Date(master): Mon Dec 10 15:41:12 CET 2012 on sn-devel-104 (cherry picked from commit 53b736444d55c4eed3abbc34974b655cc2607cd6) --- source4/dsdb/tests/python/sec_descriptor.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source4/dsdb/tests/python/sec_descriptor.py b/source4/dsdb/tests/python/sec_descriptor.py index cf213ab..78cd052 100755 --- a/source4/dsdb/tests/python/sec_descriptor.py +++ b/source4/dsdb/tests/python/sec_descriptor.py @@ -1965,6 +1965,13 @@ class SdFlagsDescriptorTests(DescriptorTests): self.assertTrue("D:" in sddl) self.assertTrue("S:" in sddl) + def test_312(self): + """This search is done by the windows dc join...""" + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, ["1.1"], + controls=["extended_dn:1:0", "sd_flags:1:0", "search_options:1:1"]) + self.assertFalse("nTSecurityDescriptor" in res[0]) + class RightsAttributesTests(DescriptorTests): def deleteAll(self): -- 1.7.9.5