From 9057e603cf58b2fac5473df2999d6d0a704686b1 Mon Sep 17 00:00:00 2001 From: Matthias Dieter Wallnöfer Date: Mon, 1 Nov 2010 17:51:36 +0100 Subject: s4:descriptor LDB module - make the "nTSecurityDescriptor" attribute fully behave as in AD - fix crash when provided "nTSecurityDescriptor" attribute is empty - print out the correct error codes if it's provided multi-valued - simplify the "recalculate_sd" control handling --- source4/dsdb/samdb/ldb_modules/descriptor.c | 108 ++++++++++++++++------------ source4/dsdb/tests/python/ldap.py | 61 ++++++++++++++-- 2 files changed, 117 insertions(+), 52 deletions(-) (limited to 'source4/dsdb') diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c index 731b7bcc2a..0515dfe592 100644 --- a/source4/dsdb/samdb/ldb_modules/descriptor.c +++ b/source4/dsdb/samdb/ldb_modules/descriptor.c @@ -55,6 +55,7 @@ struct descriptor_context { struct ldb_reply *search_res; struct ldb_reply *search_oc_res; struct ldb_val *parentsd_val; + struct ldb_message_element *sd_element; struct ldb_val *sd_val; int (*step_fn)(struct descriptor_context *); }; @@ -553,14 +554,13 @@ static int descriptor_do_mod(struct descriptor_context *ac) struct ldb_context *ldb; const struct dsdb_schema *schema; struct ldb_request *mod_req; - struct ldb_message_element *objectclass_element, *tmp_element, *oldsd_el; + struct ldb_message_element *objectclass_element, *oldsd_el; struct ldb_val *oldsd_val = NULL; int ret; DATA_BLOB *sd; const struct dsdb_class *objectclass; struct ldb_control *sd_control; struct ldb_control *sd_control2; - int flags = 0; uint32_t sd_flags = 0; ldb = ldb_module_get_ctx(ac->module); @@ -578,7 +578,8 @@ static int descriptor_do_mod(struct descriptor_context *ac) } sd_control = ldb_request_get_control(ac->req, LDB_CONTROL_SD_FLAGS_OID); - sd_control2 = ldb_request_get_control(ac->req, LDB_CONTROL_RECALCULATE_SD_OID); + sd_control2 = ldb_request_get_control(ac->req, + LDB_CONTROL_RECALCULATE_SD_OID); if (sd_control) { struct ldb_sd_flags_control *sdctr = (struct ldb_sd_flags_control *)sd_control->data; sd_flags = sdctr->secinfo_flags; @@ -586,34 +587,44 @@ static int descriptor_do_mod(struct descriptor_context *ac) sd_flags = sd_flags & 0x0000000F; } if (sd_flags != 0) { - oldsd_el = ldb_msg_find_element(ac->search_oc_res->message, "nTSecurityDescriptor"); + oldsd_el = ldb_msg_find_element(ac->search_oc_res->message, + "nTSecurityDescriptor"); if (oldsd_el) { oldsd_val = oldsd_el->values; } } - sd = get_new_descriptor(ac->module, ac->msg->dn, ac, objectclass, - ac->parentsd_val, ac->sd_val, oldsd_val, sd_flags); - if (ac->sd_val) { - tmp_element = ldb_msg_find_element(ac->msg, - "nTSecurityDescriptor"); - flags = tmp_element->flags; - ldb_msg_remove_attr(ac->msg, "nTSecurityDescriptor"); - } - if (sd) { - ret = ldb_msg_add_steal_value(ac->msg, - "nTSecurityDescriptor", sd); - if (ret != LDB_SUCCESS) { - return ret; - } - tmp_element = ldb_msg_find_element(ac->msg, - "nTSecurityDescriptor"); - if (sd_control2) { - tmp_element->flags = LDB_FLAG_MOD_REPLACE; - } else { - tmp_element->flags = flags; + sd = get_new_descriptor(ac->module, ac->msg->dn, ac, + objectclass, ac->parentsd_val, + ac->sd_val, oldsd_val, sd_flags); + if (sd != NULL) { + if (ac->sd_val != NULL) { + ac->sd_element->values[0] = *sd; + } else if (sd_control2 != NULL) { + /* In this branche we really do force the recalculation + * of the SD */ + ldb_msg_remove_attr(ac->msg, "nTSecurityDescriptor"); + + ret = ldb_msg_add_steal_value(ac->msg, + "nTSecurityDescriptor", + sd); + if (ret != LDB_SUCCESS) { + return ret; + } + ac->sd_element = ldb_msg_find_element(ac->msg, + "nTSecurityDescriptor"); + ac->sd_element->flags = LDB_FLAG_MOD_REPLACE; } } + + /* mark the controls as non-critical since we've handled them */ + if (sd_control != NULL) { + sd_control->critical = 0; + } + if (sd_control2 != NULL) { + sd_control2->critical = 0; + } + ret = ldb_build_mod_req(&mod_req, ldb, ac, ac->msg, ac->req->controls, @@ -623,11 +634,6 @@ static int descriptor_do_mod(struct descriptor_context *ac) if (ret != LDB_SUCCESS) { return ret; } - /* mark it non-critical, so we don't get an error from the - backend, but mark that we've handled it */ - if (sd_control) { - sd_control->critical = 0; - } return ldb_next_request(ac->module, mod_req); } @@ -637,7 +643,7 @@ static int descriptor_do_add(struct descriptor_context *ac) struct ldb_context *ldb; const struct dsdb_schema *schema; struct ldb_request *add_req; - struct ldb_message_element *objectclass_element, *sd_element = NULL; + struct ldb_message_element *objectclass_element; int ret; DATA_BLOB *sd; const struct dsdb_class *objectclass; @@ -676,12 +682,16 @@ static int descriptor_do_add(struct descriptor_context *ac) return ldb_operr(ldb); } - - /* get the security descriptor values*/ - sd_element = ldb_msg_find_element(ac->msg, "nTSecurityDescriptor"); - if (sd_element) { - ac->sd_val = talloc_memdup(ac, &sd_element->values[0], sizeof(struct ldb_val)); + /* Check if there is a valid security descriptor provided */ + ac->sd_element = dsdb_get_single_valued_attr(ac->msg, + "nTSecurityDescriptor", + ac->req->operation); + if ((ac->sd_element != NULL) && (ac->sd_element->num_values == 1)) { + ac->sd_val = talloc_memdup(ac, + &ac->sd_element->values[0], + sizeof(struct ldb_val)); } + /* NC's have no parent */ /* FIXME: this has to be made dynamic at some point */ if ((ldb_dn_compare(ac->msg->dn, (ldb_get_schema_basedn(ldb))) == 0) || @@ -702,15 +712,16 @@ static int descriptor_do_add(struct descriptor_context *ac) sd = get_new_descriptor(ac->module, ac->msg->dn, ac, objectclass, ac->parentsd_val, ac->sd_val, NULL, 0); - if (ac->sd_val) { - ldb_msg_remove_attr(ac->msg, "nTSecurityDescriptor"); - } - - if (sd) { - ret = ldb_msg_add_steal_value(ac->msg, - "nTSecurityDescriptor", sd); - if (ret != LDB_SUCCESS) { - return ret; + if (sd != NULL) { + if (ac->sd_val != NULL) { + ac->sd_element->values[0] = *sd; + } else if (ac->sd_element == NULL) { + ret = ldb_msg_add_steal_value(ac->msg, + "nTSecurityDescriptor", + sd); + if (ret != LDB_SUCCESS) { + return ret; + } } } @@ -759,9 +770,12 @@ static int descriptor_change(struct ldb_module *module, struct ldb_request *req) break; case LDB_MODIFY: dn = req->op.mod.message->dn; - sd_element = ldb_msg_find_element(req->op.mod.message, "nTSecurityDescriptor"); - /* This control allow forcing the recalculation of the SD */ - sd_control = ldb_request_get_control(req, LDB_CONTROL_RECALCULATE_SD_OID); + sd_element = ldb_msg_find_element(req->op.mod.message, + "nTSecurityDescriptor"); + /* This control forces the recalculation of the SD also when + * no modification is performed. */ + sd_control = ldb_request_get_control(req, + LDB_CONTROL_RECALCULATE_SD_OID); if (!sd_element && !sd_control) { return ldb_next_request(module, req); } diff --git a/source4/dsdb/tests/python/ldap.py b/source4/dsdb/tests/python/ldap.py index b4fe8cd2ee..18af214fd7 100755 --- a/source4/dsdb/tests/python/ldap.py +++ b/source4/dsdb/tests/python/ldap.py @@ -1437,7 +1437,7 @@ objectClass: container res = self.ldb.search(base=("" % self.base_dn), scope=SCOPE_BASE, attrs=[]) self.assertEquals(len(res), 1) - + res2 = self.ldb.search(scope=SCOPE_BASE, attrs=["wellKnownObjects"], expression=("wellKnownObjects=B:32:ab1d30f3768811d1aded00c04fd8d5cd:%s" % res[0].dn)) self.assertEquals(len(res2), 1) @@ -2343,9 +2343,22 @@ objectClass: posixAccount"""% (self.base_dn)) user_name = "testdescriptoruser1" user_dn = "CN=%s,CN=Users,%s" % (user_name, self.base_dn) # - # Test add_ldif() with SDDL security descriptor input + # Test an empty security descriptor (naturally this shouldn't work) # self.delete_force(self.ldb, user_dn) + try: + self.ldb.add({ "dn": user_dn, + "objectClass": "user", + "sAMAccountName": user_name, + "nTSecurityDescriptor": [] }) + self.fail() + except LdbError, (num, _): + self.assertEquals(num, ERR_CONSTRAINT_VIOLATION) + finally: + self.delete_force(self.ldb, user_dn) + # + # Test add_ldif() with SDDL security descriptor input + # try: sddl = "O:DUG:DUD:PAI(A;;RPWP;;;AU)S:PAI" self.ldb.add_ldif(""" @@ -2407,11 +2420,49 @@ nTSecurityDescriptor:: """ + desc_base64) user_name = "testdescriptoruser2" user_dn = "CN=%s,CN=Users,%s" % (user_name, self.base_dn) # - # Delete user object and test modify_ldif() with SDDL security descriptor input + # Test an empty security descriptor (naturally this shouldn't work) + # + self.delete_force(self.ldb, user_dn) + self.ldb.add({ "dn": user_dn, + "objectClass": "user", + "sAMAccountName": user_name }) + + m = Message() + m.dn = Dn(ldb, user_dn) + m["nTSecurityDescriptor"] = MessageElement([], FLAG_MOD_ADD, + "nTSecurityDescriptor") + try: + self.ldb.modify(m) + self.fail() + except LdbError, (num, _): + self.assertEquals(num, ERR_CONSTRAINT_VIOLATION) + + m = Message() + m.dn = Dn(ldb, user_dn) + m["nTSecurityDescriptor"] = MessageElement([], FLAG_MOD_REPLACE, + "nTSecurityDescriptor") + try: + self.ldb.modify(m) + self.fail() + except LdbError, (num, _): + self.assertEquals(num, ERR_UNWILLING_TO_PERFORM) + + m = Message() + m.dn = Dn(ldb, user_dn) + m["nTSecurityDescriptor"] = MessageElement([], FLAG_MOD_DELETE, + "nTSecurityDescriptor") + try: + self.ldb.modify(m) + self.fail() + except LdbError, (num, _): + self.assertEquals(num, ERR_UNWILLING_TO_PERFORM) + + self.delete_force(self.ldb, user_dn) + # + # Test modify_ldif() with SDDL security descriptor input # Add ACE to the original descriptor test # try: - self.delete_force(self.ldb, user_dn) self.ldb.add_ldif(""" dn: """ + user_dn + """ objectclass: user @@ -2585,7 +2636,7 @@ class BaseDnTests(unittest.TestCase): res = self.ldb.search("", scope=SCOPE_BASE, attrs=["namingContexts", "defaultNamingContext", "schemaNamingContext", "configurationNamingContext"]) self.assertEquals(len(res), 1) - + ncs = set([]) for nc in res[0]["namingContexts"]: self.assertTrue(nc not in ncs) -- cgit