5 files changed, 274 insertions, 120 deletions

diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index f96d4294c5..13e71e5b65 100644
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -257,6 +257,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
 	const struct dsdb_schema *schema = dsdb_get_schema(ldb);
 	int i;
+	bool modify_sd = false;
 	const struct GUID *guid;
 	uint32_t access_granted;
 	struct object_tree *root = NULL;
@@ -315,41 +316,65 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
 			attr = dsdb_attribute_by_lDAPDisplayName(schema,
 								 req->op.mod.message->elements[i].name);
 		}
+		if (strcmp("nTSecurityDescriptor", req->op.mod.message->elements[i].name) == 0) {
+			modify_sd = true;
+		} else {
 
-		if (!attr) {
-			DEBUG(10, ("acl_modify: cannot find attribute %s\n",
-				   req->op.mod.message->elements[i].name));
-			goto fail;
-		}
-		if (!insert_in_object_tree(tmp_ctx,
-					   &attr->attributeSecurityGUID, SEC_ADS_WRITE_PROP, 
-					   &new_node, &new_node)) {
-			DEBUG(10, ("acl_modify: cannot add to object tree securityGUID\n"));
-			goto fail;
+			if (!attr) {
+				DEBUG(10, ("acl_modify: cannot find attribute %s\n",
+					   req->op.mod.message->elements[i].name));
+				goto fail;
+			}
+			if (!insert_in_object_tree(tmp_ctx,
+						   &attr->attributeSecurityGUID, SEC_ADS_WRITE_PROP,
+						   &new_node, &new_node)) {
+				DEBUG(10, ("acl_modify: cannot add to object tree securityGUID\n"));
+				goto fail;
+			}
+
+			if (!insert_in_object_tree(tmp_ctx,
+						   &attr->schemaIDGUID, SEC_ADS_WRITE_PROP, &new_node, &new_node)) {
+				DEBUG(10, ("acl_modify: cannot add to object tree attributeGUID\n"));
+				goto fail;
+			}
 		}
+	}
 
-		if (!insert_in_object_tree(tmp_ctx,
-					   &attr->schemaIDGUID, SEC_ADS_WRITE_PROP, &new_node, &new_node)) {
-			DEBUG(10, ("acl_modify: cannot add to object tree attributeGUID\n"));
-			goto fail;
+	if (root->num_of_children > 0) {
+		status = sec_access_check_ds(sd, acl_user_token(module),
+					     SEC_ADS_WRITE_PROP,
+					     &access_granted,
+					     root);
+
+		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(10, ("Object %s nas no write property access\n",
+				  ldb_dn_get_linearized(req->op.mod.message->dn)));
+			acl_debug(sd,
+				  acl_user_token(module),
+				  req->op.mod.message->dn,
+				  true,
+				  10);
+			talloc_free(tmp_ctx);
+			return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
 		}
 	}
-
-	status = sec_access_check_ds(sd, acl_user_token(module),
-				     SEC_ADS_WRITE_PROP,
+	if (modify_sd) {
+		status = sec_access_check_ds(sd, acl_user_token(module),
+				     SEC_STD_WRITE_DAC,
 				     &access_granted,
-				     root);
+				     NULL);
 
-	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(10, ("Object %s nas no write property access\n",
-			   ldb_dn_get_linearized(req->op.mod.message->dn)));
-		acl_debug(sd,
-			  acl_user_token(module),
-			  req->op.mod.message->dn,
-			  true,
-			  10);
-		talloc_free(tmp_ctx);
-		return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
+		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(10, ("Object %s nas no write dacl access\n",
+				   ldb_dn_get_linearized(req->op.mod.message->dn)));
+			acl_debug(sd,
+				  acl_user_token(module),
+				  req->op.mod.message->dn,
+				  true,
+				  10);
+			talloc_free(tmp_ctx);
+			return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
+		}
 	}
 
 	talloc_free(tmp_ctx);
diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
index f1c010c2bf..6a57560314 100644
--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
@@ -47,10 +47,13 @@ struct descriptor_data {
 };
 
 struct descriptor_context {
-		struct ldb_module *module;
-		struct ldb_request *req;
-		struct ldb_reply *search_res;
-		int (*step_fn)(struct descriptor_context *);
+	struct ldb_module *module;
+	struct ldb_request *req;
+	struct ldb_reply *search_res;
+	struct ldb_reply *search_oc_res;
+	struct ldb_val *parentsd_val;
+	struct ldb_val *sd_val;
+	int (*step_fn)(struct descriptor_context *);
 };
 
 static const struct dsdb_class * get_last_structural_class(const struct dsdb_schema *schema, struct ldb_message_element *element)
@@ -212,7 +215,7 @@ static DATA_BLOB *get_new_descriptor(struct ldb_module *module,
 
 
 	sddl_sd = sddl_encode(mem_ctx, new_sd, domain_sid);
-	DEBUG(10, ("Object %s created with desriptor %s", ldb_dn_get_linearized(dn), sddl_sd));
+	DEBUG(10, ("Object %s created with desriptor %s\n\n", ldb_dn_get_linearized(dn), sddl_sd));
 
 	linear_sd = talloc(mem_ctx, DATA_BLOB);
 	if (!linear_sd) {
@@ -272,12 +275,12 @@ static int get_search_callback(struct ldb_request *req, struct ldb_reply *ares)
 
 	switch (ares->type) {
 	case LDB_REPLY_ENTRY:
-		if (ac->search_res != NULL) {
+			if (ac->search_res != NULL) {
 			ldb_set_errstring(ldb, "Too many results");
 			talloc_free(ares);
 			return ldb_module_done(ac->req, NULL, NULL,
 						LDB_ERR_OPERATIONS_ERROR);
-		}
+						}
 
 		ac->search_res = talloc_steal(ac, ares);
 		break;
@@ -298,6 +301,58 @@ static int get_search_callback(struct ldb_request *req, struct ldb_reply *ares)
 
 	return LDB_SUCCESS;
 }
+
+static int get_search_oc_callback(struct ldb_request *req, struct ldb_reply *ares)
+{
+	struct ldb_context *ldb;
+	struct descriptor_context *ac;
+	int ret;
+
+	ac = talloc_get_type(req->context, struct descriptor_context);
+	ldb = ldb_module_get_ctx(ac->module);
+
+	if (!ares) {
+		return ldb_module_done(ac->req, NULL, NULL,
+					LDB_ERR_OPERATIONS_ERROR);
+	}
+	if (ares->error != LDB_SUCCESS &&
+	    ares->error != LDB_ERR_NO_SUCH_OBJECT) {
+		return ldb_module_done(ac->req, ares->controls,
+					ares->response, ares->error);
+	}
+
+	ldb_reset_err_string(ldb);
+
+	switch (ares->type) {
+	case LDB_REPLY_ENTRY:
+			if (ac->search_oc_res != NULL) {
+			ldb_set_errstring(ldb, "Too many results");
+			talloc_free(ares);
+			return ldb_module_done(ac->req, NULL, NULL,
+						LDB_ERR_OPERATIONS_ERROR);
+						}
+
+		ac->search_oc_res = talloc_steal(ac, ares);
+		break;
+
+	case LDB_REPLY_REFERRAL:
+		/* ignore */
+		talloc_free(ares);
+		break;
+
+	case LDB_REPLY_DONE:
+		talloc_free(ares);
+		ret = ac->step_fn(ac);
+		if (ret != LDB_SUCCESS) {
+			return ldb_module_done(ac->req, NULL, NULL, ret);
+		}
+		break;
+	}
+
+	return LDB_SUCCESS;
+}
+
+
 static int descriptor_op_callback(struct ldb_request *req, struct ldb_reply *ares)
 {
 	struct descriptor_context *ac;
@@ -323,6 +378,58 @@ static int descriptor_op_callback(struct ldb_request *req, struct ldb_reply *are
 				ares->response, ares->error);
 }
 
+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;
+	int ret;
+	DATA_BLOB *sd;
+	const struct dsdb_class *objectclass;
+	struct ldb_message *msg;
+	int flags = 0;
+
+	ldb = ldb_module_get_ctx(ac->module);
+	schema = dsdb_get_schema(ldb);
+
+	msg = ldb_msg_copy_shallow(ac, ac->req->op.mod.message);
+	objectclass_element = ldb_msg_find_element(ac->search_oc_res->message, "objectClass");
+	objectclass = get_last_structural_class(schema, objectclass_element);
+
+	if (!objectclass) {
+		ldb_asprintf_errstring(ldb, "No last structural objectclass found on %s",
+				       ldb_dn_get_linearized(ac->search_oc_res->message->dn));
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
+	sd = get_new_descriptor(ac->module, msg->dn, ac, objectclass,
+				ac->parentsd_val, ac->sd_val);
+	if (ac->sd_val) {
+		tmp_element = ldb_msg_find_element(msg, "ntSecurityDescriptor");
+		flags = tmp_element->flags;
+		ldb_msg_remove_attr(msg, "nTSecurityDescriptor");
+	}
+
+	if (sd) {
+		ret = ldb_msg_add_steal_value(msg, "nTSecurityDescriptor", sd);
+		if (ret != LDB_SUCCESS) {
+			return ret;
+		}
+		tmp_element = ldb_msg_find_element(msg, "ntSecurityDescriptor");
+		tmp_element->flags = flags;
+	}
+	ret = ldb_build_mod_req(&mod_req, ldb, ac,
+				msg,
+				ac->req->controls,
+				ac, descriptor_op_callback,
+				ac->req);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+	return ldb_next_request(ac->module, mod_req);
+}
+
 static int descriptor_do_add(struct descriptor_context *ac)
 {
 	struct ldb_context *ldb;
@@ -332,94 +439,132 @@ static int descriptor_do_add(struct descriptor_context *ac)
 	struct ldb_message *msg;
 	TALLOC_CTX *mem_ctx;
 	int ret;
-	struct ldb_val *sd_val = NULL;
-	const struct ldb_val *parentsd_val = NULL;
 	DATA_BLOB *sd;
 	const struct dsdb_class *objectclass;
+	static const char *const attrs[] = { "objectClass", NULL };
+	struct ldb_request *search_req;
 
 	ldb = ldb_module_get_ctx(ac->module);
 	schema = dsdb_get_schema(ldb);
-
 	mem_ctx = talloc_new(ac);
 	if (mem_ctx == NULL) {
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	msg = ldb_msg_copy_shallow(ac, ac->req->op.add.message);
+	switch (ac->req->operation) {
+	case LDB_ADD:
+		msg = ldb_msg_copy_shallow(ac, ac->req->op.add.message);
+		objectclass_element = ldb_msg_find_element(msg, "objectClass");
+		objectclass = get_last_structural_class(schema, objectclass_element);
 
-	/* get the security descriptor values*/
-	sd_element = ldb_msg_find_element(msg, "nTSecurityDescriptor");
-	objectclass_element = ldb_msg_find_element(msg, "objectClass");
-	objectclass = get_last_structural_class(schema, objectclass_element);
-
-	if (!objectclass) {
-		ldb_asprintf_errstring(ldb, "No last structural objectclass found on %s", ldb_dn_get_linearized(msg->dn));
+		if (!objectclass) {
+			ldb_asprintf_errstring(ldb, "No last structural objectclass found on %s", ldb_dn_get_linearized(msg->dn));
+			return LDB_ERR_OPERATIONS_ERROR;
+		}
+		break;
+	case LDB_MODIFY:
+		msg = ldb_msg_copy_shallow(ac, ac->req->op.mod.message);
+		break;
+	default:
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	if (sd_element)
-		sd_val = &sd_element->values[0];
+
+	/* get the security descriptor values*/
+	sd_element = ldb_msg_find_element(msg, "nTSecurityDescriptor");
+	if (sd_element) {
+		ac->sd_val = talloc_memdup(ac, &sd_element->values[0], sizeof(struct ldb_val));
+	}
 	/* NC's have no parent */
 	if ((ldb_dn_compare(msg->dn, (ldb_get_schema_basedn(ldb))) == 0) ||
 	    (ldb_dn_compare(msg->dn, (ldb_get_config_basedn(ldb))) == 0) ||
 	    (ldb_dn_compare(msg->dn, (ldb_get_root_basedn(ldb))) == 0)) {
-		parentsd_val = NULL;
-	} else if (ac->search_res != NULL){
-		parentsd_val = ldb_msg_find_ldb_val(ac->search_res->message, "nTSecurityDescriptor");
+		ac->parentsd_val = NULL;
+	} else if (ac->search_res != NULL) {
+		struct ldb_message_element *parent_element = ldb_msg_find_element(ac->search_res->message, "nTSecurityDescriptor");
+		if (parent_element) {
+			ac->parentsd_val = talloc_memdup(ac, &parent_element->values[0], sizeof(struct ldb_val));
+		}
 	}
 
+	if (ac->req->operation == LDB_ADD) {
 	/* get the parent descriptor and the one provided. If not provided, get the default.*/
 	/* convert to security descriptor and calculate */
-	sd = get_new_descriptor(ac->module, msg->dn, mem_ctx, objectclass,
-			parentsd_val, sd_val);
-	if (sd_val) {
-		ldb_msg_remove_attr(msg, "nTSecurityDescriptor");
-	}
+		sd = get_new_descriptor(ac->module, msg->dn, mem_ctx, objectclass,
+					ac->parentsd_val, ac->sd_val);
+		if (ac->sd_val) {
+			ldb_msg_remove_attr(msg, "nTSecurityDescriptor");
+		}
 
-	if (sd) {
-		ret = ldb_msg_add_steal_value(msg, "nTSecurityDescriptor", sd);
-		if (ret != LDB_SUCCESS) {
-			return ret;
+		if (sd) {
+			ret = ldb_msg_add_steal_value(msg, "nTSecurityDescriptor", sd);
+			if (ret != LDB_SUCCESS) {
+				return ret;
+			}
 		}
-	}
 
-	talloc_free(mem_ctx);
-	ret = ldb_msg_sanity_check(ldb, msg);
+		talloc_free(mem_ctx);
+		ret = ldb_msg_sanity_check(ldb, msg);
 
-	if (ret != LDB_SUCCESS) {
-		ldb_asprintf_errstring(ldb, "No last structural objectclass found on %s", ldb_dn_get_linearized(msg->dn));
-		return ret;
-	}
+		if (ret != LDB_SUCCESS) {
+			ldb_asprintf_errstring(ldb, "No last structural objectclass found on %s",
+					       ldb_dn_get_linearized(msg->dn));
+			return ret;
+		}
 
-	ret = ldb_build_add_req(&add_req, ldb, ac,
-				msg,
-				ac->req->controls,
-				ac, descriptor_op_callback,
-				ac->req);
-	if (ret != LDB_SUCCESS) {
-		return ret;
+		ret = ldb_build_add_req(&add_req, ldb, ac,
+					msg,
+					ac->req->controls,
+					ac, descriptor_op_callback,
+					ac->req);
+		if (ret != LDB_SUCCESS) {
+			return ret;
+		}
+		return ldb_next_request(ac->module, add_req);
+	} else {
+		ret = ldb_build_search_req(&search_req, ldb,
+				   ac, msg->dn, LDB_SCOPE_BASE,
+				   "(objectClass=*)", attrs,
+				   NULL,
+				   ac, get_search_oc_callback,
+				   ac->req);
+		if (ret != LDB_SUCCESS) {
+			return ret;
+		}
+		ac->step_fn = descriptor_do_mod;
+		return ldb_next_request(ac->module, search_req);
 	}
-
-	/* perform the add */
-	return ldb_next_request(ac->module, add_req);
 }
 
-static int descriptor_add(struct ldb_module *module, struct ldb_request *req)
+static int descriptor_change(struct ldb_module *module, struct ldb_request *req)
 {
 	struct ldb_context *ldb;
 	struct ldb_request *search_req;
 	struct descriptor_context *ac;
-	struct ldb_dn *parent_dn;
+	struct ldb_dn *parent_dn, *dn;
+	struct ldb_message_element *sd_element;
 	int ret;
-	struct descriptor_data *data;
 	static const char * const descr_attrs[] = { "nTSecurityDescriptor", NULL };
 
-	data = talloc_get_type(ldb_module_get_private(module), struct descriptor_data);
 	ldb = ldb_module_get_ctx(module);
 
-	ldb_debug(ldb, LDB_DEBUG_TRACE,"descriptor_add: %s\n", ldb_dn_get_linearized(req->op.add.message->dn));
+	switch (req->operation) {
+	case LDB_ADD:
+		dn = req->op.add.message->dn;
+		break;
+	case LDB_MODIFY:
+		dn = req->op.mod.message->dn;
+		sd_element = ldb_msg_find_element(req->op.mod.message, "nTSecurityDescriptor");
+		if (!sd_element) {
+			return ldb_next_request(module, req);
+		}
+		break;
+	default:
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+	ldb_debug(ldb, LDB_DEBUG_TRACE,"descriptor_change: %s\n", ldb_dn_get_linearized(dn));
 
-	if (ldb_dn_is_special(req->op.add.message->dn)) {
+	if (ldb_dn_is_special(dn)) {
 		return ldb_next_request(module, req);
 	}
 
@@ -429,12 +574,12 @@ static int descriptor_add(struct ldb_module *module, struct ldb_request *req)
 	}
 
 	/* If there isn't a parent, just go on to the add processing */
-	if (ldb_dn_get_comp_num(ac->req->op.add.message->dn) == 1) {
+	if (ldb_dn_get_comp_num(dn) == 1) {
 		return descriptor_do_add(ac);
 	}
 
 	/* get copy of parent DN */
-	parent_dn = ldb_dn_get_parent(ac, ac->req->op.add.message->dn);
+	parent_dn = ldb_dn_get_parent(ac, dn);
 	if (parent_dn == NULL) {
 		ldb_oom(ldb);
 		return LDB_ERR_OPERATIONS_ERROR;
@@ -455,13 +600,7 @@ static int descriptor_add(struct ldb_module *module, struct ldb_request *req)
 
 	return ldb_next_request(ac->module, search_req);
 }
-/* TODO */
-static int descriptor_modify(struct ldb_module *module, struct ldb_request *req)
-{
-	struct ldb_context *ldb = ldb_module_get_ctx(module);
-	ldb_debug(ldb, LDB_DEBUG_TRACE,"descriptor_modify: %s\n", ldb_dn_get_linearized(req->op.mod.message->dn));
-	return ldb_next_request(module, req);
-}
+
 /* TODO */
 static int descriptor_rename(struct ldb_module *module, struct ldb_request *req)
 {
@@ -472,25 +611,14 @@ static int descriptor_rename(struct ldb_module *module, struct ldb_request *req)
 
 static int descriptor_init(struct ldb_module *module)
 {
-	struct ldb_context *ldb;
-	struct descriptor_data *data;
-
-	ldb = ldb_module_get_ctx(module);
-	data = talloc(module, struct descriptor_data);
-	if (data == NULL) {
-		ldb_oom(ldb);
-		return LDB_ERR_OPERATIONS_ERROR;
-	}
-
-	ldb_module_set_private(module, data);
 	return ldb_next_init(module);
 }
 
 
 _PUBLIC_ const struct ldb_module_ops ldb_descriptor_module_ops = {
 	.name		   = "descriptor",
-	.add           = descriptor_add,
-	.modify        = descriptor_modify,
+	.add           = descriptor_change,
+	.modify        = descriptor_change,
 	.rename        = descriptor_rename,
 	.init_context  = descriptor_init
 };
diff --git a/source4/libcli/security/create_descriptor.c b/source4/libcli/security/create_descriptor.c
index 82433fc02d..d5bc7cba40 100644
--- a/source4/libcli/security/create_descriptor.c
+++ b/source4/libcli/security/create_descriptor.c
@@ -145,10 +145,8 @@ static bool postprocess_acl(struct security_acl *acl,
 	}
 	co = dom_sid_parse_talloc(tmp_ctx,  SID_CREATOR_OWNER);
 	cg = dom_sid_parse_talloc(tmp_ctx,  SID_CREATOR_GROUP);
-	for (i=0; i < acl->num_aces; i++){
+	for (i=0; i < acl->num_aces; i++) {
 		struct security_ace *ace = &acl->aces[i];
-		if (!(ace->flags == 0 || ace->flags & SEC_ACE_FLAG_INHERITED_ACE))
-			continue;
 		if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY)
 			continue;
 		if (dom_sid_equal(&ace->trustee, co)){
@@ -187,7 +185,7 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
 	for (i=0; i < acl->num_aces; i++){
 		struct security_ace *ace = &acl->aces[i];
 		if ((ace->flags & SEC_ACE_FLAG_CONTAINER_INHERIT) ||
-		    (ace->flags & SEC_ACE_FLAG_OBJECT_INHERIT)){
+		    (ace->flags & SEC_ACE_FLAG_OBJECT_INHERIT)) {
 			tmp_acl->aces = talloc_realloc(tmp_acl, tmp_acl->aces, struct security_ace,
 						       tmp_acl->num_aces+1);
 			if (tmp_acl->aces == NULL) {
@@ -212,7 +210,7 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
 		}
 	}
 
-	if (is_container){
+	if (is_container) {
 		for (i=0; i < acl->num_aces; i++){
 			struct security_ace *ace = &acl->aces[i];
 
@@ -230,13 +228,13 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
 					return NULL;
 				}
 				inh_acl->aces[inh_acl->num_aces] = *ace;
-				inh_acl->aces[inh_acl->num_aces].flags |= SEC_ACE_FLAG_INHERIT_ONLY;
+				inh_acl->aces[inh_acl->num_aces].flags &= ~SEC_ACE_FLAG_INHERIT_ONLY;
 				inh_acl->aces[inh_acl->num_aces].flags |= SEC_ACE_FLAG_INHERITED_ACE;
 				inh_acl->num_aces++;
 			}
 		}
 	}
-	new_acl = security_acl_concatenate(mem_ctx,tmp_acl, inh_acl);
+	new_acl = security_acl_concatenate(mem_ctx, inh_acl, tmp_acl);
 	if (new_acl)
 		new_acl->revision = acl->revision;
 	talloc_free(tmp_ctx);
@@ -360,6 +358,12 @@ static bool compute_acl(int acl_type,
 						       object_list);
 			if (*new_acl == NULL)
 				goto final;
+			if (acl_type == SEC_DESC_DACL_PRESENT && new_sd->dacl)
+				new_sd->type |= SEC_DESC_DACL_AUTO_INHERITED;
+
+			if (acl_type == SEC_DESC_SACL_PRESENT && new_sd->sacl)
+				new_sd->type |= SEC_DESC_SACL_AUTO_INHERITED;
+
 			if (!postprocess_acl(*new_acl, new_sd->owner_sid,
 					     new_sd->group_sid, generic_map))
 				return false;
@@ -370,7 +374,7 @@ static bool compute_acl(int acl_type,
 			}
 		}
 		if (c_acl && !(inherit_flags & SEC_DEFAULT_DESCRIPTOR)){
-			struct security_acl *pr_acl, *tmp_acl, *tpr_acl;
+			struct security_acl *pr_acl = NULL, *tmp_acl = NULL, *tpr_acl = NULL;
 			tpr_acl = preprocess_creator_acl(new_sd, c_acl);
 			tmp_acl = calculate_inherited_from_creator(new_sd,
 						      tpr_acl,
@@ -380,27 +384,26 @@ static bool compute_acl(int acl_type,
 			cr_descr_log_acl(tmp_acl, __location__"Inherited from creator", level);
 			/* Todo some refactoring here! */
 			if (acl_type == SEC_DESC_DACL_PRESENT &&
-			    !(creator_sd->type & SECINFO_PROTECTED_DACL) &&
-			    (inherit_flags & SEC_DACL_AUTO_INHERIT)){
+			    !(creator_sd->type & SEC_DESC_DACL_PROTECTED) &&
+			    (inherit_flags & SEC_DACL_AUTO_INHERIT)) {
 				pr_acl = calculate_inherited_from_parent(new_sd,
 							     p_acl,
 							     is_container,
 							     object_list);
 				cr_descr_log_acl(pr_acl, __location__"Inherited from parent", level);
-				*new_acl = security_acl_concatenate(new_sd, tmp_acl, pr_acl);
 				new_sd->type |= SEC_DESC_DACL_AUTO_INHERITED;
 			}
 			else if (acl_type == SEC_DESC_SACL_PRESENT &&
-			    !(creator_sd->type & SECINFO_PROTECTED_SACL) &&
+			    !(creator_sd->type & SEC_DESC_SACL_PROTECTED) &&
 			    (inherit_flags & SEC_SACL_AUTO_INHERIT)){
 				pr_acl = calculate_inherited_from_parent(new_sd,
 							     p_acl,
 							     is_container,
 							     object_list);
 				cr_descr_log_acl(pr_acl, __location__"Inherited from parent", level);
-				*new_acl = security_acl_concatenate(new_sd, tmp_acl, pr_acl);
 				new_sd->type |= SEC_DESC_SACL_AUTO_INHERITED;
 			}
+			*new_acl = security_acl_concatenate(new_sd, tmp_acl, pr_acl);
 		}
 		if (*new_acl == NULL)
 			goto final;
diff --git a/source4/selftest/knownfail b/source4/selftest/knownfail
index dafb57d947..527f2cd111 100644
--- a/source4/selftest/knownfail
+++ b/source4/selftest/knownfail
@@ -57,7 +57,6 @@ samba4.winbind.struct.*.LOOKUP_NAME_SID   # Not yet working in winbind
 ^samba4.*base.delaywrite.*update of write time using SET_END_OF_FILE$
 ^samba4.*base.delaywrite.*update of write time using SET_ALLOCATION_SIZE$
 ^samba4.ldap.python \(dc\).Test add_ldif\(\) with BASE64 security descriptor input using WRONG domain SID$
-^samba4.ldap.python \(dc\).Testing ldb.add_ldif\(\) for nTSecurityDescriptor
 # some operations don't work over the CIFS NTVFS backend yet (eg. root_fid)
 samba4.ntvfs.cifs.base.createx_access 
 samba4.ntvfs.cifs.base.createx_sharemodes_dir
diff --git a/source4/selftest/skip b/source4/selftest/skip
index bd3263d21e..2875acca87 100644
--- a/source4/selftest/skip
+++ b/source4/selftest/skip
@@ -65,7 +65,6 @@ samba4.rpc.countcalls # this is not useful now we have full IDL
 samba4.rap.scan # same thing here - we have docs now
 samba4.gensec.python # not finished
 bench # don't run benchmarks in our selftest
-samba4.ldap.secdesc.python # this is expected to fail until the acl module is complete
 trans2.scan # uses huge number of file descriptors
 base.scan.ioctl # bad idea in make test
 base.scan.pipe_number # bad idea in make test