From 78c4c3f9f831d96cb15c05aee68eb957a4594410 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 23 Nov 2012 11:49:05 +0100 Subject: [PATCH 01/14] s4:dsdb/password_hash: Honor password complexity settings. Honor password complexity settings when creating new users. Without this patch, you could set simple passwords although the complexity settings were enabled. This was an issue with 'samba-tool user add' and also when adding new users via Windows' "Active Directory Users and Computers" MMC Snap-In. The following scenarios were tested successfully after applying the patch: -'samba-tool user add' against s4 -'samba-tool user add -H' against a Windows DC -Adding a new user on a s4 DC using Windows' "Active Directory Users and Computers" MMC Snap-In. Please note that this bug was caused by a mistake in the documentation. Fix bug #9414 - 'samba-tool user add' ignores password complexity settings. Pair-programmed-with: Karolin Seeger Signed-off-by: Stefan Metzmacher Signed-off-by: Karolin Seeger --- source4/dsdb/samdb/ldb_modules/password_hash.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c index 620de75..0f8920c 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -2188,8 +2188,14 @@ static int setup_io(struct ph_context *ac, & (UF_INTERDOMAIN_TRUST_ACCOUNT | UF_WORKSTATION_TRUST_ACCOUNT | UF_SERVER_TRUST_ACCOUNT)); - if ((io->u.userAccountControl & UF_PASSWD_NOTREQD) != 0) { + if (!ldb_req_is_untrusted(ac->req) && + (io->u.userAccountControl & UF_PASSWD_NOTREQD)) + { /* see [MS-ADTS] 2.2.15 */ + /* + * This seems to only happen for SAMR + * and not for LDAP clients + */ io->u.restrictions = 0; } -- 1.7.9.5 From bb2f91c678a226f03f0cf89c87c9e769f1f8b08f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 10:34:58 +0100 Subject: [PATCH 02/14] 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 40c4e3cdf9a7b952ec599495cd99dce7f4261e73 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 10:08:14 +0000 Subject: [PATCH 03/14] 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 986e308c92321228e409d270cf18608a2ccb4ad3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 11:02:49 +0100 Subject: [PATCH 04/14] 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 5707885c94626e72489cd6098fb42bb709747e31 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 13:39:31 +0100 Subject: [PATCH 05/14] 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 90ed095a51f2464c7338f54074d9251c8175a16a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 12:56:21 +0000 Subject: [PATCH 06/14] 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 e54e832388819424ff07201bc4d0b74bf42351ca Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 19:02:10 +0100 Subject: [PATCH 07/14] s4:dsdb/dirsync: fix potential talloc hierachy problems (bug #9470) Signed-off-by: Stefan Metzmacher --- 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 cd4eb437df3ea5eccfe61188f6ab3ee8d92a37e8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 18:39:29 +0100 Subject: [PATCH 08/14] s4:dsdb/acl_read: check the ldb_attr_list_copy_add() result Signed-off-by: Stefan Metzmacher --- 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 952e77bcb5fce48f1dab0c3d56d5f15e8ada0688 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 18:40:25 +0100 Subject: [PATCH 09/14] s4:dsdb/acl_read: fix the calculation of the attribute array for the sub search Signed-off-by: Stefan Metzmacher --- 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 86cafd2b368133f8925762f08c3a06c36018d534 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 6 Dec 2012 12:29:49 +0100 Subject: [PATCH 10/14] s4:dsdb/acl_read: give some variables a better name Signed-off-by: Stefan Metzmacher --- 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 8d7086411749a725e92a5c43d2cc751155da5cae Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 6 Dec 2012 12:36:09 +0100 Subject: [PATCH 11/14] 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 --- 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 458084ac8afbdfba31a37cdf512ad7608a867607 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 6 Dec 2012 15:56:26 +0100 Subject: [PATCH 12/14] 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 --- 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 294742a5f51c1151845b12645e73a0850e29b6fa Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 6 Dec 2012 14:04:47 +0100 Subject: [PATCH 13/14] 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 --- 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 074014cc5ea85ab17e3ef7e560577114ef04eb97 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Dec 2012 18:58:57 +0100 Subject: [PATCH 14/14] 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 --- 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