From ad96d706732e936a6a1b560b490dceb393573b60 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 3 Jan 2013 21:31:22 +1100 Subject: [PATCH 01/44] dsdb-acl: give error string if we can not obtain the schema Reviewed-by: Stefan Metzmacher (cherry picked from commit 5812eb3c1deac51891f01338b4771b1e397dc24d) --- source4/dsdb/samdb/ldb_modules/acl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 3f09760..9056a41 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -1019,8 +1019,9 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) schema = dsdb_get_schema(ldb, tmp_ctx); if (!schema) { - ret = LDB_ERR_OPERATIONS_ERROR; - goto fail; + talloc_free(tmp_ctx); + return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR, + "acl_modify: Error obtaining schema."); } ret = dsdb_get_sd_from_ldb_message(ldb, tmp_ctx, acl_res->msgs[0], &sd); -- 1.7.11.7 From a5e675d24d3c98ac12951352538f57b11357f697 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 17 Jan 2013 08:37:12 +0100 Subject: [PATCH 02/44] dsdb-acl: don't call dsdb_user_password_support() if we don't use the result Signed-off-by: Stefan Metzmacher Reviewed-by: Matthieu Patou (cherry picked from commit 947985b259ac05e95d65be19c67f384579a797ce) --- source4/dsdb/samdb/ldb_modules/acl.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 9056a41..1927132 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -982,7 +982,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) struct security_descriptor *sd; struct dom_sid *sid = NULL; struct ldb_control *as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID); - bool userPassword = dsdb_user_password_support(module, req, req); + bool userPassword; TALLOC_CTX *tmp_ctx = talloc_new(req); static const char *acl_attrs[] = { "nTSecurityDescriptor", @@ -1017,6 +1017,8 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) goto fail; } + userPassword = dsdb_user_password_support(module, req, req); + schema = dsdb_get_schema(ldb, tmp_ctx); if (!schema) { talloc_free(tmp_ctx); @@ -1661,7 +1663,7 @@ static int acl_search(struct ldb_module *module, struct ldb_request *req) ac->allowedChildClasses = ldb_attr_in_list(req->op.search.attrs, "allowedChildClasses"); ac->allowedChildClassesEffective = ldb_attr_in_list(req->op.search.attrs, "allowedChildClassesEffective"); ac->sDRightsEffective = ldb_attr_in_list(req->op.search.attrs, "sDRightsEffective"); - ac->userPassword = dsdb_user_password_support(module, ac, req); + ac->userPassword = true; ac->schema = dsdb_get_schema(ldb, ac); ac->constructed_attrs |= ac->allowedAttributes; @@ -1681,6 +1683,10 @@ static int acl_search(struct ldb_module *module, struct ldb_request *req) return ldb_next_request(module, req); } + if (!ac->am_system) { + ac->userPassword = dsdb_user_password_support(module, ac, req); + } + ret = acl_search_update_confidential_attrs(ac, data); if (ret != LDB_SUCCESS) { return ret; -- 1.7.11.7 From 22b3909dbc3e3f282536a10d3bb427b457e82f24 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 17 Jan 2013 08:37:58 +0100 Subject: [PATCH 03/44] dsdb-acl: talloc_free the private context when we pass to the next module Signed-off-by: Stefan Metzmacher Reviewed-by: Matthieu Patou (cherry picked from commit 961a1fbbbccb7fbb14634ec230985f3fd000b050) --- source4/dsdb/samdb/ldb_modules/acl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 1927132..e559771 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -1680,6 +1680,7 @@ static int acl_search(struct ldb_module *module, struct ldb_request *req) } if (!ac->constructed_attrs && !ac->modify_search) { + talloc_free(ac); return ldb_next_request(module, req); } -- 1.7.11.7 From 47b6f0177735672a5a4d4d32d2ae1a862b89e7f6 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 30 Dec 2012 02:27:25 -0800 Subject: [PATCH 04/44] dsdb-acl: Do not apply ACL on special DNs to hide attributes that the user shouldn't see This fix frequent reindexing when using python script with a user that is not system. The reindexing is caused by ACL module hidding (removing) attributes in the search request for all attributes in dn=@ATTRIBUTES and because dsdb_schema_set_indices_and_attributes checks that the list of attributes that it just calculated from the schema is the same as the list written in @ATTRIBUTES, if not the list is replaced and a reindexing is triggered. Signed-off-by: Matthieu Patou Reviewed-by: Stefan Metzmacher (cherry picked from commit a0c59b4da1c5d8637c92e65c7cf54bb82bc8fca5) --- source4/dsdb/samdb/ldb_modules/acl.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index e559771..2504568 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -1644,6 +1644,10 @@ static int acl_search(struct ldb_module *module, struct ldb_request *req) int ret; unsigned int i; + if (ldb_dn_is_special(req->op.search.base)) { + return ldb_next_request(module, req); + } + ldb = ldb_module_get_ctx(module); ac = talloc_zero(req, struct acl_context); -- 1.7.11.7 From 2f1b27e32bc04f9f10d3058ada7450f538cb0dd4 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 17 Jan 2013 08:51:23 +0100 Subject: [PATCH 05/44] dsdb-acl: fix the order of special and system checks First we check for a special dn, then for system access. All allocations happen after this checks in order to avoid allocations we won't use. Signed-off-by: Stefan Metzmacher Reviewed-by: Matthieu Patou (cherry picked from commit 70460605c6132ffbc6be825c24f188674c0ac979) --- source4/dsdb/samdb/ldb_modules/acl.c | 83 ++++++++++++++++++++++++++---------- 1 file changed, 61 insertions(+), 22 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 2504568..2b3abce 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -751,14 +751,19 @@ static int acl_check_spn(TALLOC_CTX *mem_ctx, static int acl_add(struct ldb_module *module, struct ldb_request *req) { int ret; - struct ldb_dn *parent = ldb_dn_get_parent(req, req->op.add.message->dn); + struct ldb_dn *parent; struct ldb_context *ldb; const struct dsdb_schema *schema; struct ldb_message_element *oc_el; const struct GUID *guid; struct ldb_dn *nc_root; - struct ldb_control *as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID); + struct ldb_control *as_system; + if (ldb_dn_is_special(req->op.add.message->dn)) { + return ldb_next_request(module, req); + } + + as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID); if (as_system != NULL) { as_system->critical = 0; } @@ -766,12 +771,14 @@ static int acl_add(struct ldb_module *module, struct ldb_request *req) if (dsdb_module_am_system(module) || as_system) { return ldb_next_request(module, req); } - if (ldb_dn_is_special(req->op.add.message->dn)) { - return ldb_next_request(module, req); - } ldb = ldb_module_get_ctx(module); + parent = ldb_dn_get_parent(req, req->op.add.message->dn); + if (parent == NULL) { + return ldb_oom(ldb); + } + /* Creating an NC. There is probably something we should do here, * but we will establish that later */ @@ -981,9 +988,9 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) struct ldb_result *acl_res; struct security_descriptor *sd; struct dom_sid *sid = NULL; - struct ldb_control *as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID); + struct ldb_control *as_system; bool userPassword; - TALLOC_CTX *tmp_ctx = talloc_new(req); + TALLOC_CTX *tmp_ctx; static const char *acl_attrs[] = { "nTSecurityDescriptor", "objectClass", @@ -991,6 +998,11 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) NULL }; + if (ldb_dn_is_special(req->op.mod.message->dn)) { + return ldb_next_request(module, req); + } + + as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID); if (as_system != NULL) { as_system->critical = 0; } @@ -1003,9 +1015,12 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) if (dsdb_module_am_system(module) || as_system) { return ldb_next_request(module, req); } - if (ldb_dn_is_special(req->op.mod.message->dn)) { - return ldb_next_request(module, req); + + tmp_ctx = talloc_new(req); + if (tmp_ctx == NULL) { + return ldb_oom(ldb); } + ret = dsdb_module_search_dn(module, tmp_ctx, &acl_res, req->op.mod.message->dn, acl_attrs, DSDB_FLAG_NEXT_MODULE | @@ -1198,25 +1213,33 @@ fail: static int acl_delete(struct ldb_module *module, struct ldb_request *req) { int ret; - struct ldb_dn *parent = ldb_dn_get_parent(req, req->op.del.dn); + struct ldb_dn *parent; struct ldb_context *ldb; struct ldb_dn *nc_root; - struct ldb_control *as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID); + struct ldb_control *as_system; + + if (ldb_dn_is_special(req->op.del.dn)) { + return ldb_next_request(module, req); + } + as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID); if (as_system != NULL) { as_system->critical = 0; } - DEBUG(10, ("ldb:acl_delete: %s\n", ldb_dn_get_linearized(req->op.del.dn))); if (dsdb_module_am_system(module) || as_system) { return ldb_next_request(module, req); } - if (ldb_dn_is_special(req->op.del.dn)) { - return ldb_next_request(module, req); - } + + DEBUG(10, ("ldb:acl_delete: %s\n", ldb_dn_get_linearized(req->op.del.dn))); ldb = ldb_module_get_ctx(module); + parent = ldb_dn_get_parent(req, req->op.del.dn); + if (parent == NULL) { + return ldb_oom(ldb); + } + /* Make sure we aren't deleting a NC */ ret = dsdb_find_nc_root(ldb, req, req->op.del.dn, &nc_root); @@ -1265,8 +1288,8 @@ static int acl_delete(struct ldb_module *module, struct ldb_request *req) static int acl_rename(struct ldb_module *module, struct ldb_request *req) { int ret; - struct ldb_dn *oldparent = ldb_dn_get_parent(req, req->op.rename.olddn); - struct ldb_dn *newparent = ldb_dn_get_parent(req, req->op.rename.newdn); + struct ldb_dn *oldparent; + struct ldb_dn *newparent; const struct dsdb_schema *schema; struct ldb_context *ldb; struct security_descriptor *sd = NULL; @@ -1276,8 +1299,8 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) struct ldb_dn *nc_root; struct object_tree *root = NULL; struct object_tree *new_node = NULL; - struct ldb_control *as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID); - TALLOC_CTX *tmp_ctx = talloc_new(req); + struct ldb_control *as_system; + TALLOC_CTX *tmp_ctx; NTSTATUS status; uint32_t access_granted; const char *rdn_name; @@ -1288,6 +1311,11 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) NULL }; + if (ldb_dn_is_special(req->op.rename.olddn)) { + return ldb_next_request(module, req); + } + + as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID); if (as_system != NULL) { as_system->critical = 0; } @@ -1296,12 +1324,23 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) if (dsdb_module_am_system(module) || as_system) { return ldb_next_request(module, req); } - if (ldb_dn_is_special(req->op.rename.olddn)) { - return ldb_next_request(module, req); - } ldb = ldb_module_get_ctx(module); + tmp_ctx = talloc_new(req); + if (tmp_ctx == NULL) { + return ldb_oom(ldb); + } + + oldparent = ldb_dn_get_parent(tmp_ctx, req->op.rename.olddn); + if (oldparent == NULL) { + return ldb_oom(ldb); + } + newparent = ldb_dn_get_parent(tmp_ctx, req->op.rename.newdn); + if (newparent == NULL) { + return ldb_oom(ldb); + } + /* Make sure we aren't renaming/moving a NC */ ret = dsdb_find_nc_root(ldb, req, req->op.rename.olddn, &nc_root); -- 1.7.11.7 From 37830c588b090101fa5156a1986b671124f88d39 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 4 Jan 2013 16:03:42 +0100 Subject: [PATCH 06/44] libcli/security: don't look at the inherited type in get_ace_object_type() The inherited_type is only used to decide if aces should be inherited effectively or not (INHERIT_ONLY) for the specified object. Signed-off-by: Stefan Metzmacher Reviewed-by: Matthieu Patou (cherry picked from commit 629ce2a1ba392f2e8b632752c583843777471378) --- libcli/security/access_check.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c index 9153dad..70345f5 100644 --- a/libcli/security/access_check.c +++ b/libcli/security/access_check.c @@ -371,8 +371,6 @@ static const struct GUID *get_ace_object_type(struct security_ace *ace) if (ace->object.object.flags & SEC_ACE_OBJECT_TYPE_PRESENT) type = &ace->object.object.type.type; - else if (ace->object.object.flags & SEC_ACE_INHERITED_OBJECT_TYPE_PRESENT) - type = &ace->object.object.inherited_type.inherited_type; /* This doesn't look right. Is something wrong with the IDL? */ else type = NULL; -- 1.7.11.7 From 810b033b30c3ee777028431856e12f3fe98e34aa Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 8 Jan 2013 15:54:47 +0100 Subject: [PATCH 07/44] dsdb-acl: add helper variable 'ldb' in acl_sDRightsEffective Signed-off-by: Stefan Metzmacher Reviewed-by: Matthieu Patou (cherry picked from commit ccf577da14194f5f3377226bcdb7e69b62a94851) --- source4/dsdb/samdb/ldb_modules/acl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 2b3abce..24b6507 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -458,6 +458,7 @@ static int acl_sDRightsEffective(struct ldb_module *module, struct ldb_message *msg, struct acl_context *ac) { + struct ldb_context *ldb = ldb_module_get_ctx(module); struct ldb_message_element *rightsEffective; int ret; struct security_descriptor *sd; @@ -481,7 +482,7 @@ static int acl_sDRightsEffective(struct ldb_module *module, } else { /* Get the security descriptor from the message */ - ret = dsdb_get_sd_from_ldb_message(ldb_module_get_ctx(module), msg, sd_msg, &sd); + ret = dsdb_get_sd_from_ldb_message(ldb, msg, sd_msg, &sd); if (ret != LDB_SUCCESS) { return ret; } -- 1.7.11.7 From ccb19d481f8476823f3817949e8e1abca16459c7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 8 Jan 2013 15:55:36 +0100 Subject: [PATCH 08/44] dsdb-acl: calculate sDRightsEffective based on "nTSecurityDescriptor" acl_check_access_on_attribute should never be called with attr=NULL because we don't check access on an attribute in that case Signed-off-by: Stefan Metzmacher Reviewed-by: Matthieu Patou Autobuild-User(master): Matthieu Patou Autobuild-Date(master): Thu Jan 17 11:21:10 CET 2013 on sn-devel-104 (cherry picked from commit 6a1025551eb5b343ec996ae0c642d542162e8910) --- source4/dsdb/samdb/ldb_modules/acl.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 24b6507..539363c 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -481,6 +481,14 @@ static int acl_sDRightsEffective(struct ldb_module *module, flags = SECINFO_OWNER | SECINFO_GROUP | SECINFO_SACL | SECINFO_DACL; } else { + const struct dsdb_attribute *attr; + + attr = dsdb_attribute_by_lDAPDisplayName(ac->schema, + "nTSecurityDescriptor"); + if (attr == NULL) { + return ldb_operr(ldb); + } + /* Get the security descriptor from the message */ ret = dsdb_get_sd_from_ldb_message(ldb, msg, sd_msg, &sd); if (ret != LDB_SUCCESS) { @@ -492,7 +500,7 @@ static int acl_sDRightsEffective(struct ldb_module *module, sd, sid, SEC_STD_WRITE_OWNER, - NULL); + attr); if (ret == LDB_SUCCESS) { flags |= SECINFO_OWNER | SECINFO_GROUP; } @@ -501,7 +509,7 @@ static int acl_sDRightsEffective(struct ldb_module *module, sd, sid, SEC_STD_WRITE_DAC, - NULL); + attr); if (ret == LDB_SUCCESS) { flags |= SECINFO_DACL; } @@ -510,7 +518,7 @@ static int acl_sDRightsEffective(struct ldb_module *module, sd, sid, SEC_FLAG_SYSTEM_SECURITY, - NULL); + attr); if (ret == LDB_SUCCESS) { flags |= SECINFO_SACL; } -- 1.7.11.7 From 920807d3c9c3581ae9751a95a00fdf8f2590d2fd Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 17 Jan 2013 14:14:37 +0100 Subject: [PATCH 09/44] dsdb-schema: make schema_subclasses_order_recurse() static Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit c4b9ee255814b8121d13e33cd9b0cd7c093d736c) --- source4/dsdb/schema/schema_inferiors.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/schema/schema_inferiors.c b/source4/dsdb/schema/schema_inferiors.c index d2c134e..73bea5b 100644 --- a/source4/dsdb/schema/schema_inferiors.c +++ b/source4/dsdb/schema/schema_inferiors.c @@ -141,9 +141,9 @@ static const char **schema_subclasses_recurse(const struct dsdb_schema *schema, /* Walk down the subClass tree, setting a higher index as we go down * each level. top is 1, subclasses of top are 2, etc */ -void schema_subclasses_order_recurse(const struct dsdb_schema *schema, - struct dsdb_class *schema_class, - const int order) +static void schema_subclasses_order_recurse(const struct dsdb_schema *schema, + struct dsdb_class *schema_class, + const int order) { const char **list = schema_class->subclasses_direct; unsigned int i; -- 1.7.11.7 From caf632d6081b8ad49e198d4c579b5922aee6edf8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 17 Jan 2013 14:40:24 +0100 Subject: [PATCH 10/44] dsdb-schema: make sure use clean caches in schema_inferiors.c Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 1f673bf9209405dfa2593859bbc45d1c6dc2a960) --- source4/dsdb/schema/schema.h | 11 +++++--- source4/dsdb/schema/schema_inferiors.c | 49 +++++++++++++++++----------------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/source4/dsdb/schema/schema.h b/source4/dsdb/schema/schema.h index eb288e6..66df1c5 100644 --- a/source4/dsdb/schema/schema.h +++ b/source4/dsdb/schema/schema.h @@ -166,10 +166,6 @@ struct dsdb_class { bool isDefunct; bool systemOnly; - const char **supclasses; - const char **subclasses; - const char **subclasses_direct; - const char **posssuperiors; uint32_t subClassOf_id; uint32_t *systemAuxiliaryClass_ids; uint32_t *auxiliaryClass_ids; @@ -186,6 +182,13 @@ struct dsdb_class { * subClasses of top are 2, subclasses of those classes are * 3 */ uint32_t subClass_order; + + struct { + const char **supclasses; + const char **subclasses; + const char **subclasses_direct; + const char **posssuperiors; + } tmp; }; /** diff --git a/source4/dsdb/schema/schema_inferiors.c b/source4/dsdb/schema/schema_inferiors.c index 73bea5b..14699c7 100644 --- a/source4/dsdb/schema/schema_inferiors.c +++ b/source4/dsdb/schema/schema_inferiors.c @@ -39,8 +39,8 @@ static const char **schema_supclasses(const struct dsdb_schema *schema, { const char **list; - if (schema_class->supclasses) { - return schema_class->supclasses; + if (schema_class->tmp.supclasses) { + return schema_class->tmp.supclasses; } list = const_str_list(str_list_make_empty(schema_class)); @@ -52,7 +52,7 @@ static const char **schema_supclasses(const struct dsdb_schema *schema, /* Cope with 'top SUP top', i.e. top is subClassOf top */ if (schema_class->subClassOf && strcmp(schema_class->lDAPDisplayName, schema_class->subClassOf) == 0) { - schema_class->supclasses = list; + schema_class->tmp.supclasses = list; return list; } @@ -65,9 +65,9 @@ static const char **schema_supclasses(const struct dsdb_schema *schema, list = str_list_append_const(list, list2); } - schema_class->supclasses = str_list_unique(list); + schema_class->tmp.supclasses = str_list_unique(list); - return schema_class->supclasses; + return schema_class->tmp.supclasses; } /* @@ -87,7 +87,7 @@ static const char **schema_subclasses(const struct dsdb_schema *schema, DEBUG(0, ("ERROR: Unable to locate subClass: '%s'\n", oclist[i])); continue; } - list = str_list_append_const(list, schema_class->subclasses); + list = str_list_append_const(list, schema_class->tmp.subclasses); } return list; } @@ -99,7 +99,7 @@ static const char **schema_subclasses(const struct dsdb_schema *schema, static const char **schema_posssuperiors(const struct dsdb_schema *schema, struct dsdb_class *schema_class) { - if (schema_class->posssuperiors == NULL) { + if (schema_class->tmp.posssuperiors == NULL) { const char **list2 = const_str_list(str_list_make_empty(schema_class)); const char **list3; unsigned int i; @@ -118,16 +118,16 @@ static const char **schema_posssuperiors(const struct dsdb_schema *schema, } list2 = str_list_append_const(list2, schema_subclasses(schema, list2, list2)); - schema_class->posssuperiors = str_list_unique(list2); + schema_class->tmp.posssuperiors = str_list_unique(list2); } - return schema_class->posssuperiors; + return schema_class->tmp.posssuperiors; } static const char **schema_subclasses_recurse(const struct dsdb_schema *schema, struct dsdb_class *schema_class) { - const char **list = str_list_copy_const(schema_class, schema_class->subclasses_direct); + const char **list = str_list_copy_const(schema_class, schema_class->tmp.subclasses_direct); unsigned int i; for (i=0;list && list[i]; i++) { const struct dsdb_class *schema_class2 = dsdb_class_by_lDAPDisplayName(schema, list[i]); @@ -145,7 +145,7 @@ static void schema_subclasses_order_recurse(const struct dsdb_schema *schema, struct dsdb_class *schema_class, const int order) { - const char **list = schema_class->subclasses_direct; + const char **list = schema_class->tmp.subclasses_direct; unsigned int i; schema_class->subClass_order = order; for (i=0;list && list[i]; i++) { @@ -169,19 +169,19 @@ static int schema_create_subclasses(const struct dsdb_schema *schema) return LDB_ERR_OPERATIONS_ERROR; } if (schema_class2 && schema_class != schema_class2) { - if (schema_class2->subclasses_direct == NULL) { - schema_class2->subclasses_direct = const_str_list(str_list_make_empty(schema_class2)); - if (!schema_class2->subclasses_direct) { + if (schema_class2->tmp.subclasses_direct == NULL) { + schema_class2->tmp.subclasses_direct = const_str_list(str_list_make_empty(schema_class2)); + if (!schema_class2->tmp.subclasses_direct) { return LDB_ERR_OPERATIONS_ERROR; } } - schema_class2->subclasses_direct = str_list_add_const(schema_class2->subclasses_direct, + schema_class2->tmp.subclasses_direct = str_list_add_const(schema_class2->tmp.subclasses_direct, schema_class->lDAPDisplayName); } } for (schema_class=schema->classes; schema_class; schema_class=schema_class->next) { - schema_class->subclasses = str_list_unique(schema_subclasses_recurse(schema, schema_class)); + schema_class->tmp.subclasses = str_list_unique(schema_subclasses_recurse(schema, schema_class)); /* Initialize the subClass order, to ensure we can't have uninitialized sort on the subClass hierarchy */ schema_class->subClass_order = 0; @@ -329,6 +329,11 @@ int schema_fill_constructed(const struct dsdb_schema *schema) int ret; struct dsdb_class *schema_class; + /* make sure we start with a clean cache */ + for (schema_class=schema->classes; schema_class; schema_class=schema_class->next) { + ZERO_STRUCT(schema_class->tmp); + } + schema_fill_from_ids(schema); ret = schema_create_subclasses(schema); @@ -343,14 +348,10 @@ int schema_fill_constructed(const struct dsdb_schema *schema) /* free up our internal cache elements */ for (schema_class=schema->classes; schema_class; schema_class=schema_class->next) { - talloc_free(schema_class->supclasses); - talloc_free(schema_class->subclasses_direct); - talloc_free(schema_class->subclasses); - talloc_free(schema_class->posssuperiors); - schema_class->supclasses = NULL; - schema_class->subclasses_direct = NULL; - schema_class->subclasses = NULL; - schema_class->posssuperiors = NULL; + TALLOC_FREE(schema_class->tmp.supclasses); + TALLOC_FREE(schema_class->tmp.subclasses_direct); + TALLOC_FREE(schema_class->tmp.subclasses); + TALLOC_FREE(schema_class->tmp.posssuperiors); } return LDB_SUCCESS; -- 1.7.11.7 From 3615dbad5c88bcc22de4da358379b4d7a1ac00c7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 17 Jan 2013 14:41:39 +0100 Subject: [PATCH 11/44] dsdb-schema: make sure we build [system]PossibleInferiors completely Otherwise callers like dsdb_schema_copy_shallow() will corrupt the talloc hierarchie. Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit c2853f55fc603d4875bb1e50a1cbf409df0421ea) --- source4/dsdb/schema/schema_inferiors.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source4/dsdb/schema/schema_inferiors.c b/source4/dsdb/schema/schema_inferiors.c index 14699c7..2f7d461 100644 --- a/source4/dsdb/schema/schema_inferiors.c +++ b/source4/dsdb/schema/schema_inferiors.c @@ -202,6 +202,8 @@ static void schema_fill_possible_inferiors(const struct dsdb_schema *schema, { struct dsdb_class *c2; + schema_class->possibleInferiors = NULL; + for (c2=schema->classes; c2; c2=c2->next) { const char **superiors = schema_posssuperiors(schema, c2); if (c2->systemOnly == false @@ -223,6 +225,8 @@ static void schema_fill_system_possible_inferiors(const struct dsdb_schema *sche { struct dsdb_class *c2; + schema_class->systemPossibleInferiors = NULL; + for (c2=schema->classes; c2; c2=c2->next) { const char **superiors = schema_posssuperiors(schema, c2); if (c2->objectClassCategory != 2 -- 1.7.11.7 From bf87d1b1bb37b0a04e48f70b61e0a6982721832e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 18 Jan 2013 09:17:25 +0100 Subject: [PATCH 12/44] dsdb-acl: introduce a 'msg' helper variable to acl_modify() Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 71b856a3f08fbd095833c27c59d7ed382be70d2a) --- source4/dsdb/samdb/ldb_modules/acl.c | 40 ++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 539363c..b629c58 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -1000,6 +1000,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) struct ldb_control *as_system; bool userPassword; TALLOC_CTX *tmp_ctx; + const struct ldb_message *msg = req->op.mod.message; static const char *acl_attrs[] = { "nTSecurityDescriptor", "objectClass", @@ -1007,7 +1008,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) NULL }; - if (ldb_dn_is_special(req->op.mod.message->dn)) { + if (ldb_dn_is_special(msg->dn)) { return ldb_next_request(module, req); } @@ -1017,9 +1018,8 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) } /* Don't print this debug statement if elements[0].name is going to be NULL */ - if(req->op.mod.message->num_elements > 0) - { - DEBUG(10, ("ldb:acl_modify: %s\n", req->op.mod.message->elements[0].name)); + if (msg->num_elements > 0) { + DEBUG(10, ("ldb:acl_modify: %s\n", msg->elements[0].name)); } if (dsdb_module_am_system(module) || as_system) { return ldb_next_request(module, req); @@ -1030,7 +1030,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) return ldb_oom(ldb); } - ret = dsdb_module_search_dn(module, tmp_ctx, &acl_res, req->op.mod.message->dn, + ret = dsdb_module_search_dn(module, tmp_ctx, &acl_res, msg->dn, acl_attrs, DSDB_FLAG_NEXT_MODULE | DSDB_FLAG_AS_SYSTEM | @@ -1068,12 +1068,12 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) "acl_modify: Error retrieving object class GUID."); } sid = samdb_result_dom_sid(req, acl_res->msgs[0], "objectSid"); - for (i=0; i < req->op.mod.message->num_elements; i++){ + for (i=0; i < msg->num_elements; i++) { const struct dsdb_attribute *attr; attr = dsdb_attribute_by_lDAPDisplayName(schema, - req->op.mod.message->elements[i].name); + msg->elements[i].name); - if (ldb_attr_cmp("nTSecurityDescriptor", req->op.mod.message->elements[i].name) == 0) { + if (ldb_attr_cmp("nTSecurityDescriptor", msg->elements[i].name) == 0) { uint32_t sd_flags = dsdb_request_sd_flags(req, NULL); uint32_t access_mask = 0; @@ -1096,17 +1096,17 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) if (!NT_STATUS_IS_OK(status)) { ldb_asprintf_errstring(ldb_module_get_ctx(module), "Object %s has no write dacl access\n", - ldb_dn_get_linearized(req->op.mod.message->dn)); + ldb_dn_get_linearized(msg->dn)); dsdb_acl_debug(sd, acl_user_token(module), - req->op.mod.message->dn, + msg->dn, true, 10); ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS; goto fail; } } - else if (ldb_attr_cmp("member", req->op.mod.message->elements[i].name) == 0) { + else if (ldb_attr_cmp("member", msg->elements[i].name) == 0) { ret = acl_check_self_membership(tmp_ctx, module, req, @@ -1118,14 +1118,14 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) goto fail; } } - else if (ldb_attr_cmp("dBCSPwd", req->op.mod.message->elements[i].name) == 0) { + else if (ldb_attr_cmp("dBCSPwd", msg->elements[i].name) == 0) { /* this one is not affected by any rights, we should let it through so that passwords_hash returns the correct error */ continue; } - else if (ldb_attr_cmp("unicodePwd", req->op.mod.message->elements[i].name) == 0 || - (userPassword && ldb_attr_cmp("userPassword", req->op.mod.message->elements[i].name) == 0) || - ldb_attr_cmp("clearTextPassword", req->op.mod.message->elements[i].name) == 0) { + else if (ldb_attr_cmp("unicodePwd", msg->elements[i].name) == 0 || + (userPassword && ldb_attr_cmp("userPassword", msg->elements[i].name) == 0) || + ldb_attr_cmp("clearTextPassword", msg->elements[i].name) == 0) { ret = acl_check_password_rights(tmp_ctx, module, req, @@ -1136,7 +1136,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) if (ret != LDB_SUCCESS) { goto fail; } - } else if (ldb_attr_cmp("servicePrincipalName", req->op.mod.message->elements[i].name) == 0) { + } else if (ldb_attr_cmp("servicePrincipalName", msg->elements[i].name) == 0) { ret = acl_check_spn(tmp_ctx, module, req, @@ -1159,8 +1159,8 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) */ if (!attr) { ldb_asprintf_errstring(ldb, "acl_modify: attribute '%s' on entry '%s' was not found in the schema!", - req->op.mod.message->elements[i].name, - ldb_dn_get_linearized(req->op.mod.message->dn)); + msg->elements[i].name, + ldb_dn_get_linearized(msg->dn)); ret = LDB_ERR_NO_SUCH_ATTRIBUTE; goto fail; } @@ -1197,10 +1197,10 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) if (!NT_STATUS_IS_OK(status)) { ldb_asprintf_errstring(ldb_module_get_ctx(module), "Object %s has no write property access\n", - ldb_dn_get_linearized(req->op.mod.message->dn)); + ldb_dn_get_linearized(msg->dn)); dsdb_acl_debug(sd, acl_user_token(module), - req->op.mod.message->dn, + msg->dn, true, 10); ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS; -- 1.7.11.7 From 9ac864b7c427e3e4f5791be6846f4792c40aa162 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 18 Jan 2013 09:17:25 +0100 Subject: [PATCH 13/44] dsdb-acl: introduce a 'el' helper variable to acl_modify() Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit ddfb8fe89c493c485250d59868312614c79a9cc1) --- source4/dsdb/samdb/ldb_modules/acl.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index b629c58..ec21db3 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -1069,11 +1069,13 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) } sid = samdb_result_dom_sid(req, acl_res->msgs[0], "objectSid"); for (i=0; i < msg->num_elements; i++) { + const struct ldb_message_element *el = &msg->elements[i]; const struct dsdb_attribute *attr; + attr = dsdb_attribute_by_lDAPDisplayName(schema, - msg->elements[i].name); + el->name); - if (ldb_attr_cmp("nTSecurityDescriptor", msg->elements[i].name) == 0) { + if (ldb_attr_cmp("nTSecurityDescriptor", el->name) == 0) { uint32_t sd_flags = dsdb_request_sd_flags(req, NULL); uint32_t access_mask = 0; @@ -1105,8 +1107,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS; goto fail; } - } - else if (ldb_attr_cmp("member", msg->elements[i].name) == 0) { + } else if (ldb_attr_cmp("member", el->name) == 0) { ret = acl_check_self_membership(tmp_ctx, module, req, @@ -1117,15 +1118,13 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) if (ret != LDB_SUCCESS) { goto fail; } - } - else if (ldb_attr_cmp("dBCSPwd", msg->elements[i].name) == 0) { + } else if (ldb_attr_cmp("dBCSPwd", el->name) == 0) { /* this one is not affected by any rights, we should let it through so that passwords_hash returns the correct error */ continue; - } - else if (ldb_attr_cmp("unicodePwd", msg->elements[i].name) == 0 || - (userPassword && ldb_attr_cmp("userPassword", msg->elements[i].name) == 0) || - ldb_attr_cmp("clearTextPassword", msg->elements[i].name) == 0) { + } else if (ldb_attr_cmp("unicodePwd", el->name) == 0 || + (userPassword && ldb_attr_cmp("userPassword", el->name) == 0) || + ldb_attr_cmp("clearTextPassword", el->name) == 0) { ret = acl_check_password_rights(tmp_ctx, module, req, @@ -1136,7 +1135,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) if (ret != LDB_SUCCESS) { goto fail; } - } else if (ldb_attr_cmp("servicePrincipalName", msg->elements[i].name) == 0) { + } else if (ldb_attr_cmp("servicePrincipalName", el->name) == 0) { ret = acl_check_spn(tmp_ctx, module, req, @@ -1159,7 +1158,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) */ if (!attr) { ldb_asprintf_errstring(ldb, "acl_modify: attribute '%s' on entry '%s' was not found in the schema!", - msg->elements[i].name, + el->name, ldb_dn_get_linearized(msg->dn)); ret = LDB_ERR_NO_SUCH_ATTRIBUTE; goto fail; -- 1.7.11.7 From f56aa51b46eb6a89db5723aa4191f6fae22adef3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Jan 2013 16:39:35 +0100 Subject: [PATCH 14/44] dsdb-acl: dsdb_attribute_by_lDAPDisplayName() is needed for all attributes "clearTextPassword" is the only exception. Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit d695b8abc7a2e4f7e1853d0c61fe0c03fc786111) --- source4/dsdb/samdb/ldb_modules/acl.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index ec21db3..b8fab55 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -1072,8 +1072,24 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) const struct ldb_message_element *el = &msg->elements[i]; const struct dsdb_attribute *attr; - attr = dsdb_attribute_by_lDAPDisplayName(schema, - el->name); + /* + * This basic attribute existence check with the right errorcode + * is needed since this module is the first one which requests + * schema attribute information. + * The complete attribute checking is done in the + * "objectclass_attrs" module behind this one. + * + * NOTE: "clearTextPassword" is not defined in the schema. + */ + attr = dsdb_attribute_by_lDAPDisplayName(schema, el->name); + if (!attr && ldb_attr_cmp("clearTextPassword", el->name) != 0) { + ldb_asprintf_errstring(ldb, "acl_modify: attribute '%s' " + "on entry '%s' was not found in the schema!", + req->op.mod.message->elements[i].name, + ldb_dn_get_linearized(req->op.mod.message->dn)); + ret = LDB_ERR_NO_SUCH_ATTRIBUTE; + goto fail; + } if (ldb_attr_cmp("nTSecurityDescriptor", el->name) == 0) { uint32_t sd_flags = dsdb_request_sd_flags(req, NULL); @@ -1150,20 +1166,6 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) struct object_tree *root = NULL; struct object_tree *new_node = NULL; - /* This basic attribute existence check with the right errorcode - * is needed since this module is the first one which requests - * schema attribute information. - * The complete attribute checking is done in the - * "objectclass_attrs" module behind this one. - */ - if (!attr) { - ldb_asprintf_errstring(ldb, "acl_modify: attribute '%s' on entry '%s' was not found in the schema!", - el->name, - ldb_dn_get_linearized(msg->dn)); - ret = LDB_ERR_NO_SUCH_ATTRIBUTE; - goto fail; - } - if (!insert_in_object_tree(tmp_ctx, guid, SEC_ADS_WRITE_PROP, &root, &new_node)) { talloc_free(tmp_ctx); -- 1.7.11.7 From 17f4e3753190a871bf880c241ce1bf99518d35fd Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Jan 2013 11:45:46 +0100 Subject: [PATCH 15/44] dsdb-acl: attr is not optional to acl_check_access_on_attribute() Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 2685a4ed6681b1a20fb26087867737ecbf8fad73) --- source4/dsdb/samdb/ldb_modules/acl_util.c | 49 +++++++++++++++---------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c index fc6a55a..13d6098 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_util.c +++ b/source4/dsdb/samdb/ldb_modules/acl_util.c @@ -105,34 +105,33 @@ int acl_check_access_on_attribute(struct ldb_module *module, struct object_tree *new_node = NULL; TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx); struct security_token *token = acl_user_token(module); - if (attr) { - if (!GUID_all_zero(&attr->attributeSecurityGUID)) { - if (!insert_in_object_tree(tmp_ctx, - &attr->attributeSecurityGUID, - access_mask, &root, - &new_node)) { - DEBUG(10, ("acl_search: cannot add to object tree securityGUID\n")); - goto fail; - } - - if (!insert_in_object_tree(tmp_ctx, - &attr->schemaIDGUID, - access_mask, &new_node, - &new_node)) { - DEBUG(10, ("acl_search: cannot add to object tree attributeGUID\n")); - goto fail; - } + + if (!GUID_all_zero(&attr->attributeSecurityGUID)) { + if (!insert_in_object_tree(tmp_ctx, + &attr->attributeSecurityGUID, + access_mask, &root, + &new_node)) { + DEBUG(10, ("acl_search: cannot add to object tree securityGUID\n")); + goto fail; + } + + if (!insert_in_object_tree(tmp_ctx, + &attr->schemaIDGUID, + access_mask, &new_node, + &new_node)) { + DEBUG(10, ("acl_search: cannot add to object tree attributeGUID\n")); + goto fail; } - else { - if (!insert_in_object_tree(tmp_ctx, - &attr->schemaIDGUID, - access_mask, &root, - &new_node)) { - DEBUG(10, ("acl_search: cannot add to object tree attributeGUID\n")); - goto fail; - } + } else { + if (!insert_in_object_tree(tmp_ctx, + &attr->schemaIDGUID, + access_mask, &root, + &new_node)) { + DEBUG(10, ("acl_search: cannot add to object tree attributeGUID\n")); + goto fail; } } + status = sec_access_check_ds(sd, token, access_mask, &access_granted, -- 1.7.11.7 From f306b70e0999c36a019841cd3fcdbc8e6ed772c0 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 2 Jan 2013 14:52:21 +1100 Subject: [PATCH 16/44] dsdb-acl: Add helper function dsdb_get_structural_oc_from_msg() This will eventually replace get_oc_guid_from_message(), returning the full dsdb_class. Andrew Bartlett Signed-off-by: Stefan Metzmacher Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 74bfec026921fcfc430fb7cfaee44ed75f135a99) --- source4/dsdb/samdb/ldb_modules/util.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/source4/dsdb/samdb/ldb_modules/util.c b/source4/dsdb/samdb/ldb_modules/util.c index 253d5c1..f7803e5 100644 --- a/source4/dsdb/samdb/ldb_modules/util.c +++ b/source4/dsdb/samdb/ldb_modules/util.c @@ -1393,3 +1393,16 @@ const struct dsdb_class *dsdb_get_last_structural_class(const struct dsdb_schema return last_class; } + +const struct dsdb_class *dsdb_get_structural_oc_from_msg(const struct dsdb_schema *schema, + const struct ldb_message *msg) +{ + struct ldb_message_element *oc_el; + + oc_el = ldb_msg_find_element(msg, "objectClass"); + if (!oc_el) { + return NULL; + } + + return dsdb_get_last_structural_class(schema, oc_el); +} -- 1.7.11.7 From 365b62e9fb16b1c1bdeb59706dd7550ccaca0358 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Jan 2013 16:34:56 +0100 Subject: [PATCH 17/44] dsdb-acl: add acl_check_access_on_objectclass() helper Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 097fae2d1d6ae04a7bfc795803f200b6f703a904) --- source4/dsdb/samdb/ldb_modules/acl_util.c | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c index 13d6098..bbf8e66 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_util.c +++ b/source4/dsdb/samdb/ldb_modules/acl_util.c @@ -150,6 +150,45 @@ fail: return ldb_operr(ldb_module_get_ctx(module)); } +int acl_check_access_on_objectclass(struct ldb_module *module, + TALLOC_CTX *mem_ctx, + struct security_descriptor *sd, + struct dom_sid *rp_sid, + uint32_t access_mask, + const struct dsdb_class *objectclass) +{ + int ret; + NTSTATUS status; + uint32_t access_granted; + struct object_tree *root = NULL; + struct object_tree *new_node = NULL; + TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx); + struct security_token *token = acl_user_token(module); + + if (!insert_in_object_tree(tmp_ctx, + &objectclass->schemaIDGUID, + access_mask, &root, + &new_node)) { + DEBUG(10, ("acl_search: cannot add to object tree class schemaIDGUID\n")); + goto fail; + } + + status = sec_access_check_ds(sd, token, + access_mask, + &access_granted, + root, + rp_sid); + if (!NT_STATUS_IS_OK(status)) { + ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS; + } else { + ret = LDB_SUCCESS; + } + talloc_free(tmp_ctx); + return ret; +fail: + talloc_free(tmp_ctx); + return ldb_operr(ldb_module_get_ctx(module)); +} /* checks for validated writes */ int acl_check_extended_right(TALLOC_CTX *mem_ctx, -- 1.7.11.7 From 5ba13f223bbe7a1200c355302e9d5f51244d6549 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 2 Jan 2013 14:53:02 +1100 Subject: [PATCH 18/44] dsdb-acl: Use dsdb_get_structural_oc_from_msg() in acl_modify() Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 6d7e53aaac8c95f86e1eb8593880ae1c09d973d4) --- source4/dsdb/samdb/ldb_modules/acl.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index b8fab55..9900892 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -991,7 +991,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; unsigned int i; - const struct GUID *guid; + const struct dsdb_class *objectclass; uint32_t access_granted; NTSTATUS status; struct ldb_result *acl_res; @@ -1061,11 +1061,11 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) goto success; } - guid = get_oc_guid_from_message(schema, acl_res->msgs[0]); - if (!guid) { + objectclass = dsdb_get_structural_oc_from_msg(schema, acl_res->msgs[0]); + if (!objectclass) { talloc_free(tmp_ctx); return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR, - "acl_modify: Error retrieving object class GUID."); + "acl_modify: Error retrieving object class for GUID."); } sid = samdb_result_dom_sid(req, acl_res->msgs[0], "objectSid"); for (i=0; i < msg->num_elements; i++) { @@ -1129,7 +1129,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) req, sd, sid, - guid, + &objectclass->schemaIDGUID, attr); if (ret != LDB_SUCCESS) { goto fail; @@ -1146,7 +1146,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) req, sd, sid, - guid, + &objectclass->schemaIDGUID, userPassword); if (ret != LDB_SUCCESS) { goto fail; @@ -1157,7 +1157,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) req, sd, sid, - guid, + &objectclass->schemaIDGUID, attr); if (ret != LDB_SUCCESS) { goto fail; @@ -1166,7 +1166,9 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) struct object_tree *root = NULL; struct object_tree *new_node = NULL; - if (!insert_in_object_tree(tmp_ctx, guid, SEC_ADS_WRITE_PROP, + if (!insert_in_object_tree(tmp_ctx, + &objectclass->schemaIDGUID, + SEC_ADS_WRITE_PROP, &root, &new_node)) { talloc_free(tmp_ctx); return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR, -- 1.7.11.7 From 3d1e794a4a0d20175d2b206897dd8b73997268f6 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 2 Jan 2013 14:54:20 +1100 Subject: [PATCH 19/44] dsdb-acl: Use dsdb_get_structural_oc_from_msg() in acl_rename() Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 730433984c9f3dd30ee0b07dc22af56b4d3a062f) --- source4/dsdb/samdb/ldb_modules/acl.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 9900892..629e0c8 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -1303,6 +1303,7 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) struct ldb_dn *oldparent; struct ldb_dn *newparent; const struct dsdb_schema *schema; + const struct dsdb_class *objectclass; struct ldb_context *ldb; struct security_descriptor *sd = NULL; struct dom_sid *sid = NULL; @@ -1389,12 +1390,18 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) return ldb_operr(ldb); } - guid = get_oc_guid_from_message(schema, acl_res->msgs[0]); - if (!insert_in_object_tree(tmp_ctx, guid, SEC_ADS_WRITE_PROP, + objectclass = dsdb_get_structural_oc_from_msg(schema, acl_res->msgs[0]); + if (!objectclass) { + talloc_free(tmp_ctx); + return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR, + "acl_modify: Error retrieving object class for GUID."); + } + + if (!insert_in_object_tree(tmp_ctx, &objectclass->schemaIDGUID, SEC_ADS_WRITE_PROP, &root, &new_node)) { talloc_free(tmp_ctx); return ldb_operr(ldb); - }; + } guid = attribute_schemaid_guid_by_lDAPDisplayName(schema, "name"); @@ -1402,7 +1409,7 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) &new_node, &new_node)) { talloc_free(tmp_ctx); return ldb_operr(ldb); - }; + } rdn_name = ldb_dn_get_rdn_name(req->op.rename.olddn); if (rdn_name == NULL) { @@ -1457,15 +1464,10 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) /* new parent should have create child */ root = NULL; new_node = NULL; - guid = get_oc_guid_from_message(schema, acl_res->msgs[0]); - if (!guid) { - ldb_asprintf_errstring(ldb_module_get_ctx(module), - "acl:renamed object has no object class\n"); - talloc_free(tmp_ctx); - return ldb_module_done(req, NULL, NULL, LDB_ERR_OPERATIONS_ERROR); - } - ret = dsdb_module_check_access_on_dn(module, req, newparent, SEC_ADS_CREATE_CHILD, guid, req); + ret = dsdb_module_check_access_on_dn(module, req, newparent, + SEC_ADS_CREATE_CHILD, + &objectclass->schemaIDGUID, req); if (ret != LDB_SUCCESS) { ldb_asprintf_errstring(ldb_module_get_ctx(module), "acl:access_denied renaming %s", -- 1.7.11.7 From 5658f4bb1786b51ab0128792ca3145a8a20a140e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 2 Jan 2013 09:26:15 +1100 Subject: [PATCH 20/44] dsdb-acl: use dsdb_get_structural_oc_from_msg() rather than class_schemaid_guid_by_lDAPDisplayName This uses dsdb_get_last_structural_objectclass(), which encodes this ordering knowledge in one place in the code, rather than using this uncommented magic expression: (char *)oc_el->values[oc_el->num_values-1].data Andrew Bartlett Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 6ab41506857814d69d897471a14002d98fb4c172) --- source4/dsdb/samdb/ldb_modules/acl.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 629e0c8..2a1a853 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -763,8 +763,7 @@ static int acl_add(struct ldb_module *module, struct ldb_request *req) struct ldb_dn *parent; struct ldb_context *ldb; const struct dsdb_schema *schema; - struct ldb_message_element *oc_el; - const struct GUID *guid; + const struct dsdb_class *objectclass; struct ldb_dn *nc_root; struct ldb_control *as_system; @@ -806,17 +805,17 @@ static int acl_add(struct ldb_module *module, struct ldb_request *req) return ldb_operr(ldb); } - oc_el = ldb_msg_find_element(req->op.add.message, "objectClass"); - if (!oc_el || oc_el->num_values == 0) { + objectclass = dsdb_get_structural_oc_from_msg(schema, req->op.add.message); + if (!objectclass) { ldb_asprintf_errstring(ldb_module_get_ctx(module), - "acl: unable to find objectClass on %s\n", + "acl: unable to find or validate structrual objectClass on %s\n", ldb_dn_get_linearized(req->op.add.message->dn)); return ldb_module_done(req, NULL, NULL, LDB_ERR_OPERATIONS_ERROR); } - guid = class_schemaid_guid_by_lDAPDisplayName(schema, - (char *)oc_el->values[oc_el->num_values-1].data); - ret = dsdb_module_check_access_on_dn(module, req, parent, SEC_ADS_CREATE_CHILD, guid, req); + ret = dsdb_module_check_access_on_dn(module, req, parent, + SEC_ADS_CREATE_CHILD, + &objectclass->schemaIDGUID, req); if (ret != LDB_SUCCESS) { return ret; } -- 1.7.11.7 From cc6aa4b7a18271b04a3d19ce963a37ac386e797a Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 2 Jan 2013 15:01:00 +1100 Subject: [PATCH 21/44] dsdb-acl: ask for the objectClass attribute if it's not in the scope of the clients search This will be used later. Signed-off-by: Stefan Metzmacher Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit a1b421e8cca24a5831f4c6d77714cf54faf8c48e) --- source4/dsdb/samdb/ldb_modules/acl_read.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c index 9955451..dcabd56 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_read.c +++ b/source4/dsdb/samdb/ldb_modules/acl_read.c @@ -48,6 +48,7 @@ struct aclread_context { bool added_nTSecurityDescriptor; bool added_instanceType; bool added_objectSid; + bool added_objectClass; bool indirsync; }; @@ -123,10 +124,11 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) goto fail; } } + /* for every element in the message check RP */ for (i=0; i < msg->num_elements; i++) { const struct dsdb_attribute *attr; - bool is_sd, is_objectsid, is_instancetype; + bool is_sd, is_objectsid, is_instancetype, is_objectclass; uint32_t access_mask; attr = dsdb_attribute_by_lDAPDisplayName(ac->schema, msg->elements[i].name); @@ -144,6 +146,8 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) msg->elements[i].name) == 0; is_instancetype = ldb_attr_cmp("instanceType", msg->elements[i].name) == 0; + is_objectclass = ldb_attr_cmp("objectClass", + msg->elements[i].name) == 0; /* these attributes were added to perform access checks and must be removed */ if (is_objectsid && ac->added_objectSid) { aclread_mark_inaccesslible(&msg->elements[i]); @@ -153,6 +157,10 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) aclread_mark_inaccesslible(&msg->elements[i]); continue; } + if (is_objectclass && ac->added_objectClass) { + aclread_mark_inaccesslible(&msg->elements[i]); + continue; + } if (is_sd && ac->added_nTSecurityDescriptor) { aclread_mark_inaccesslible(&msg->elements[i]); continue; @@ -409,6 +417,13 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) } ac->added_objectSid = true; } + if (!ldb_attr_in_list(req->op.search.attrs, "objectClass")) { + attrs = ldb_attr_list_copy_add(ac, attrs, "objectClass"); + if (attrs == NULL) { + return ldb_oom(ldb); + } + ac->added_objectClass = true; + } } if (need_sd) { -- 1.7.11.7 From 5d75004720779ada6f12890f2882ce049a41d1fb Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 2 Jan 2013 14:55:36 +1100 Subject: [PATCH 22/44] dsdb-acl: Remove unused get_oc_guid_from_message() Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 93944ea90069df5379993f5c186ffd68e166f1c4) --- source4/dsdb/samdb/ldb_modules/acl.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 2a1a853..0a244f9 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -964,25 +964,6 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx, return ret; } -static const struct GUID *get_oc_guid_from_message(const struct dsdb_schema *schema, - struct ldb_message *msg) -{ - struct ldb_message_element *oc_el; - const struct dsdb_class *object_class; - - oc_el = ldb_msg_find_element(msg, "objectClass"); - if (!oc_el) { - return NULL; - } - - object_class = dsdb_get_last_structural_class(schema, oc_el); - if (object_class == NULL) { - return NULL; - } - - return &object_class->schemaIDGUID; -} - static int acl_modify(struct ldb_module *module, struct ldb_request *req) { -- 1.7.11.7 From c4ef1242619b57989ef3f52fbe544b0061938ddc Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 2 Jan 2013 15:01:00 +1100 Subject: [PATCH 23/44] dsdb-acl: Pass the structural objectClass into acl_check_access_on_attribute This will, when the GUID is entered into the object tree (not in this commit) ensure that access rights assigned to the structural objectClass are also available, as well as rights assigned to the attribute property groups. Andrew Bartlett Signed-off-by: Stefan Metzmacher Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit e8cc59eb781006c6193249128a1ffc4bcba8f28a) --- source4/dsdb/samdb/ldb_modules/acl.c | 59 +++++++++++++++++++++---------- source4/dsdb/samdb/ldb_modules/acl_read.c | 15 +++++++- source4/dsdb/samdb/ldb_modules/acl_util.c | 3 +- 3 files changed, 57 insertions(+), 20 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 0a244f9..638955d 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -191,6 +191,7 @@ static int acl_allowedAttributes(struct ldb_module *module, TALLOC_CTX *mem_ctx; const char **attr_list; int i, ret; + const struct dsdb_class *objectclass; /* If we don't have a schema yet, we can't do anything... */ if (schema == NULL) { @@ -215,6 +216,19 @@ static int acl_allowedAttributes(struct ldb_module *module, talloc_free(mem_ctx); return LDB_ERR_OPERATIONS_ERROR; } + + /* + * Get the top-most structural object class for the ACL check + */ + objectclass = dsdb_get_last_structural_class(ac->schema, + oc_el); + if (objectclass == NULL) { + ldb_asprintf_errstring(ldb, "acl_read: Failed to find a structural class for %s", + ldb_dn_get_linearized(sd_msg->dn)); + talloc_free(mem_ctx); + return LDB_ERR_OPERATIONS_ERROR; + } + if (ac->allowedAttributes) { for (i=0; attr_list && attr_list[i]; i++) { ldb_msg_add_string(msg, "allowedAttributes", attr_list[i]); @@ -262,7 +276,8 @@ static int acl_allowedAttributes(struct ldb_module *module, sd, sid, SEC_ADS_WRITE_PROP, - attr); + attr, + objectclass); if (ret == LDB_SUCCESS) { ldb_msg_add_string(msg, "allowedAttributesEffective", attr_list[i]); } @@ -479,10 +494,15 @@ static int acl_sDRightsEffective(struct ldb_module *module, } if (ac->am_system || as_system) { flags = SECINFO_OWNER | SECINFO_GROUP | SECINFO_SACL | SECINFO_DACL; - } - else { + } else { + const struct dsdb_class *objectclass; const struct dsdb_attribute *attr; + objectclass = dsdb_get_structural_oc_from_msg(ac->schema, sd_msg); + if (objectclass == NULL) { + return ldb_operr(ldb); + } + attr = dsdb_attribute_by_lDAPDisplayName(ac->schema, "nTSecurityDescriptor"); if (attr == NULL) { @@ -500,7 +520,8 @@ static int acl_sDRightsEffective(struct ldb_module *module, sd, sid, SEC_STD_WRITE_OWNER, - attr); + attr, + objectclass); if (ret == LDB_SUCCESS) { flags |= SECINFO_OWNER | SECINFO_GROUP; } @@ -509,7 +530,8 @@ static int acl_sDRightsEffective(struct ldb_module *module, sd, sid, SEC_STD_WRITE_DAC, - attr); + attr, + objectclass); if (ret == LDB_SUCCESS) { flags |= SECINFO_DACL; } @@ -518,7 +540,8 @@ static int acl_sDRightsEffective(struct ldb_module *module, sd, sid, SEC_FLAG_SYSTEM_SECURITY, - attr); + attr, + objectclass); if (ret == LDB_SUCCESS) { flags |= SECINFO_SACL; } @@ -636,8 +659,8 @@ static int acl_check_spn(TALLOC_CTX *mem_ctx, struct ldb_request *req, struct security_descriptor *sd, struct dom_sid *sid, - const struct GUID *oc_guid, - const struct dsdb_attribute *attr) + const struct dsdb_attribute *attr, + const struct dsdb_class *objectclass) { int ret; unsigned int i; @@ -671,7 +694,7 @@ static int acl_check_spn(TALLOC_CTX *mem_ctx, sd, sid, SEC_ADS_WRITE_PROP, - attr) == LDB_SUCCESS) { + attr, objectclass) == LDB_SUCCESS) { talloc_free(tmp_ctx); return LDB_SUCCESS; } @@ -828,8 +851,8 @@ static int acl_check_self_membership(TALLOC_CTX *mem_ctx, struct ldb_request *req, struct security_descriptor *sd, struct dom_sid *sid, - const struct GUID *oc_guid, - const struct dsdb_attribute *attr) + const struct dsdb_attribute *attr, + const struct dsdb_class *objectclass) { int ret; unsigned int i; @@ -842,7 +865,7 @@ static int acl_check_self_membership(TALLOC_CTX *mem_ctx, sd, sid, SEC_ADS_WRITE_PROP, - attr) == LDB_SUCCESS) { + attr, objectclass) == LDB_SUCCESS) { return LDB_SUCCESS; } /* if we are adding/deleting ourselves, check for self membership */ @@ -884,7 +907,7 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx, struct ldb_request *req, struct security_descriptor *sd, struct dom_sid *sid, - const struct GUID *oc_guid, + const struct dsdb_class *objectclass, bool userPassword) { int ret = LDB_SUCCESS; @@ -1109,8 +1132,8 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) req, sd, sid, - &objectclass->schemaIDGUID, - attr); + attr, + objectclass); if (ret != LDB_SUCCESS) { goto fail; } @@ -1126,7 +1149,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) req, sd, sid, - &objectclass->schemaIDGUID, + objectclass, userPassword); if (ret != LDB_SUCCESS) { goto fail; @@ -1137,8 +1160,8 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) req, sd, sid, - &objectclass->schemaIDGUID, - attr); + attr, + objectclass); if (ret != LDB_SUCCESS) { goto fail; } diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c index dcabd56..07b1bc4 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_read.c +++ b/source4/dsdb/samdb/ldb_modules/acl_read.c @@ -76,6 +76,7 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) struct dom_sid *sid = NULL; TALLOC_CTX *tmp_ctx; uint32_t instanceType; + const struct dsdb_class *objectclass; ac = talloc_get_type(req->context, struct aclread_context); ldb = ldb_module_get_ctx(ac->module); @@ -98,6 +99,17 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) ret = LDB_ERR_OPERATIONS_ERROR; goto fail; } + /* + * Get the most specific structural object class for the ACL check + */ + objectclass = dsdb_get_structural_oc_from_msg(ac->schema, msg); + if (objectclass == NULL) { + ldb_asprintf_errstring(ldb, "acl_read: Failed to find a structural class for %s", + ldb_dn_get_linearized(msg->dn)); + ret = LDB_ERR_OPERATIONS_ERROR; + goto fail; + } + sid = samdb_result_dom_sid(tmp_ctx, msg, "objectSid"); /* get the object instance type */ instanceType = ldb_msg_find_attr_as_uint(msg, @@ -196,7 +208,8 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) sd, sid, access_mask, - attr); + attr, + objectclass); /* * Dirsync control needs the replpropertymetadata attribute diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c index bbf8e66..95ab275 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_util.c +++ b/source4/dsdb/samdb/ldb_modules/acl_util.c @@ -96,7 +96,8 @@ int acl_check_access_on_attribute(struct ldb_module *module, struct security_descriptor *sd, struct dom_sid *rp_sid, uint32_t access_mask, - const struct dsdb_attribute *attr) + const struct dsdb_attribute *attr, + const struct dsdb_class *objectclass) { int ret; NTSTATUS status; -- 1.7.11.7 From 47a8f041df76156381d5d00f573ebd82bc001517 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 2 Jan 2013 15:01:23 +1100 Subject: [PATCH 24/44] dsdb-acl: Use the structural objectClass in acl_check_access_on_attribute() This commit enters the GUID into the object tree so that that access rights assigned to the structural objectClass are also available, as well as rights assigned to the attribute property groups. Andrew Bartlett Signed-off-by: Stefan Metzmacher Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 6a4063f30273ff184364f276c5206c3507f37644) --- source4/dsdb/samdb/ldb_modules/acl_util.c | 32 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c index 95ab275..09ca201 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_util.c +++ b/source4/dsdb/samdb/ldb_modules/acl_util.c @@ -107,30 +107,30 @@ int acl_check_access_on_attribute(struct ldb_module *module, TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx); struct security_token *token = acl_user_token(module); + if (!insert_in_object_tree(tmp_ctx, + &objectclass->schemaIDGUID, + access_mask, &root, + &new_node)) { + DEBUG(10, ("acl_search: cannot add to object tree class schemaIDGUID\n")); + goto fail; + } + if (!GUID_all_zero(&attr->attributeSecurityGUID)) { if (!insert_in_object_tree(tmp_ctx, &attr->attributeSecurityGUID, - access_mask, &root, + access_mask, &new_node, &new_node)) { DEBUG(10, ("acl_search: cannot add to object tree securityGUID\n")); goto fail; } + } - if (!insert_in_object_tree(tmp_ctx, - &attr->schemaIDGUID, - access_mask, &new_node, - &new_node)) { - DEBUG(10, ("acl_search: cannot add to object tree attributeGUID\n")); - goto fail; - } - } else { - if (!insert_in_object_tree(tmp_ctx, - &attr->schemaIDGUID, - access_mask, &root, - &new_node)) { - DEBUG(10, ("acl_search: cannot add to object tree attributeGUID\n")); - goto fail; - } + if (!insert_in_object_tree(tmp_ctx, + &attr->schemaIDGUID, + access_mask, &new_node, + &new_node)) { + DEBUG(10, ("acl_search: cannot add to object tree attributeGUID\n")); + goto fail; } status = sec_access_check_ds(sd, token, -- 1.7.11.7 From d6db12f7540de2beb0afd6902ee1cf9a2834cb02 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Jan 2013 16:35:33 +0100 Subject: [PATCH 25/44] dsdb-acl: use acl_check_access_on_objectclass() instead of acl_check_access_on_class() Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 34f1a52689f4cc64fb63118e685a4442e3fe187a) --- source4/dsdb/samdb/ldb_modules/acl.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 638955d..a3f4303 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -434,14 +434,19 @@ static int acl_childClassesEffective(struct ldb_module *module, } for (j=0; sclass->possibleInferiors && sclass->possibleInferiors[j]; j++) { - ret = acl_check_access_on_class(module, - schema, - msg, - sd, - acl_user_token(module), - sid, - SEC_ADS_CREATE_CHILD, - sclass->possibleInferiors[j]); + const struct dsdb_class *sc; + + sc = dsdb_class_by_lDAPDisplayName(schema, + sclass->possibleInferiors[j]); + if (!sc) { + /* We don't know this class? what is going on? */ + continue; + } + + ret = acl_check_access_on_objectclass(module, ac, + sd, sid, + SEC_ADS_CREATE_CHILD, + sc); if (ret == LDB_SUCCESS) { ldb_msg_add_string(msg, "allowedChildClassesEffective", sclass->possibleInferiors[j]); -- 1.7.11.7 From 57c4c70fa6b7887109b476ee2468c70f08027d0a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Jan 2013 16:36:07 +0100 Subject: [PATCH 26/44] dsdb-acl: remove unused acl_check_access_on_class() Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 8e47e64f5d73441b6eb13d59001d52ec77c1c7d5) --- source4/dsdb/samdb/ldb_modules/acl.c | 46 ------------------------------------ 1 file changed, 46 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index a3f4303..2842e58 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -340,52 +340,6 @@ static int acl_childClasses(struct ldb_module *module, return LDB_SUCCESS; } -static int acl_check_access_on_class(struct ldb_module *module, - const struct dsdb_schema *schema, - TALLOC_CTX *mem_ctx, - struct security_descriptor *sd, - struct security_token *token, - struct dom_sid *rp_sid, - uint32_t access_mask, - const char *class_name) -{ - int ret; - NTSTATUS status; - uint32_t access_granted; - struct object_tree *root = NULL; - struct object_tree *new_node = NULL; - const struct GUID *guid; - - if (class_name != NULL) { - guid = class_schemaid_guid_by_lDAPDisplayName(schema, class_name); - if (!guid) { - DEBUG(10, ("acl_search: cannot find class %s\n", - class_name)); - goto fail; - } - if (!insert_in_object_tree(mem_ctx, - guid, access_mask, - &root, &new_node)) { - DEBUG(10, ("acl_search: cannot add to object tree guid\n")); - goto fail; - } - } - - status = sec_access_check_ds(sd, token, - access_mask, - &access_granted, - root, - rp_sid); - if (!NT_STATUS_IS_OK(status)) { - ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS; - } else { - ret = LDB_SUCCESS; - } - return ret; -fail: - return ldb_operr(ldb_module_get_ctx(module)); -} - static int acl_childClassesEffective(struct ldb_module *module, const struct dsdb_schema *schema, struct ldb_message *sd_msg, -- 1.7.11.7 From 71d372121aee440884ff688c0fe4e1ad51c80a9c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Jan 2013 16:41:51 +0100 Subject: [PATCH 27/44] dsdb-acl: make use of acl_check_access_on_attribute() in acl_modify() Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 8d31e42eed71e9686b03c496eeff1ff96a6742ea) --- source4/dsdb/samdb/ldb_modules/acl.c | 60 ++++++++++-------------------------- 1 file changed, 16 insertions(+), 44 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 2842e58..b4b170f 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -954,8 +954,6 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) const struct dsdb_schema *schema; unsigned int i; const struct dsdb_class *objectclass; - uint32_t access_granted; - NTSTATUS status; struct ldb_result *acl_res; struct security_descriptor *sd; struct dom_sid *sid = NULL; @@ -1067,13 +1065,14 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) access_mask |= SEC_FLAG_SYSTEM_SECURITY; } - status = sec_access_check_ds(sd, acl_user_token(module), - access_mask, - &access_granted, - NULL, - sid); - - if (!NT_STATUS_IS_OK(status)) { + ret = acl_check_access_on_attribute(module, + tmp_ctx, + sd, + sid, + access_mask, + attr, + objectclass); + if (ret != LDB_SUCCESS) { ldb_asprintf_errstring(ldb_module_get_ctx(module), "Object %s has no write dacl access\n", ldb_dn_get_linearized(msg->dn)); @@ -1125,41 +1124,14 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) goto fail; } } else { - struct object_tree *root = NULL; - struct object_tree *new_node = NULL; - - if (!insert_in_object_tree(tmp_ctx, - &objectclass->schemaIDGUID, - SEC_ADS_WRITE_PROP, - &root, &new_node)) { - talloc_free(tmp_ctx); - return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR, - "acl_modify: Error adding new node in object tree."); - } - - if (!insert_in_object_tree(tmp_ctx, - &attr->attributeSecurityGUID, SEC_ADS_WRITE_PROP, - &new_node, &new_node)) { - ldb_asprintf_errstring(ldb_module_get_ctx(module), - "acl_modify: cannot add to object tree securityGUID\n"); - ret = LDB_ERR_OPERATIONS_ERROR; - goto fail; - } - - if (!insert_in_object_tree(tmp_ctx, - &attr->schemaIDGUID, SEC_ADS_WRITE_PROP, &new_node, &new_node)) { - ldb_asprintf_errstring(ldb_module_get_ctx(module), - "acl_modify: cannot add to object tree attributeGUID\n"); - ret = LDB_ERR_OPERATIONS_ERROR; - goto fail; - } - - status = sec_access_check_ds(sd, acl_user_token(module), - SEC_ADS_WRITE_PROP, - &access_granted, - root, - sid); - if (!NT_STATUS_IS_OK(status)) { + ret = acl_check_access_on_attribute(module, + tmp_ctx, + sd, + sid, + SEC_ADS_WRITE_PROP, + attr, + objectclass); + if (ret != LDB_SUCCESS) { ldb_asprintf_errstring(ldb_module_get_ctx(module), "Object %s has no write property access\n", ldb_dn_get_linearized(msg->dn)); -- 1.7.11.7 From 2a5941fb9e9026f0bc3fa800dd88d8c9a617ecc2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Jan 2013 16:43:14 +0100 Subject: [PATCH 28/44] dsdb-acl: make use of acl_check_access_on_{attribute,objectclass} in acl_rename() Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 8aa855573067418c84f71aa3a20e5f472343851d) --- source4/dsdb/samdb/ldb_modules/acl.c | 90 +++++++++++++++++------------------- 1 file changed, 43 insertions(+), 47 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index b4b170f..0ab9670 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -1238,18 +1238,14 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) struct ldb_dn *newparent; const struct dsdb_schema *schema; const struct dsdb_class *objectclass; + const struct dsdb_attribute *attr = NULL; struct ldb_context *ldb; struct security_descriptor *sd = NULL; struct dom_sid *sid = NULL; struct ldb_result *acl_res; - const struct GUID *guid; struct ldb_dn *nc_root; - struct object_tree *root = NULL; - struct object_tree *new_node = NULL; struct ldb_control *as_system; TALLOC_CTX *tmp_ctx; - NTSTATUS status; - uint32_t access_granted; const char *rdn_name; static const char *acl_attrs[] = { "nTSecurityDescriptor", @@ -1318,12 +1314,24 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) return ret; } + ret = dsdb_get_sd_from_ldb_message(ldb, req, acl_res->msgs[0], &sd); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ldb_operr(ldb); + } + if (!sd) { + talloc_free(tmp_ctx); + return ldb_operr(ldb); + } + schema = dsdb_get_schema(ldb, acl_res); if (!schema) { talloc_free(tmp_ctx); return ldb_operr(ldb); } + sid = samdb_result_dom_sid(req, acl_res->msgs[0], "objectSid"); + objectclass = dsdb_get_structural_oc_from_msg(schema, acl_res->msgs[0]); if (!objectclass) { talloc_free(tmp_ctx); @@ -1331,18 +1339,27 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) "acl_modify: Error retrieving object class for GUID."); } - if (!insert_in_object_tree(tmp_ctx, &objectclass->schemaIDGUID, SEC_ADS_WRITE_PROP, - &root, &new_node)) { + attr = dsdb_attribute_by_lDAPDisplayName(schema, "name"); + if (attr == NULL) { talloc_free(tmp_ctx); return ldb_operr(ldb); } - guid = attribute_schemaid_guid_by_lDAPDisplayName(schema, - "name"); - if (!insert_in_object_tree(tmp_ctx, guid, SEC_ADS_WRITE_PROP, - &new_node, &new_node)) { + ret = acl_check_access_on_attribute(module, tmp_ctx, sd, sid, + SEC_ADS_WRITE_PROP, + attr, objectclass); + if (ret != LDB_SUCCESS) { + ldb_asprintf_errstring(ldb_module_get_ctx(module), + "Object %s has no wp on %s\n", + ldb_dn_get_linearized(req->op.rename.olddn), + attr->lDAPDisplayName); + dsdb_acl_debug(sd, + acl_user_token(module), + req->op.rename.olddn, + true, + 10); talloc_free(tmp_ctx); - return ldb_operr(ldb); + return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS; } rdn_name = ldb_dn_get_rdn_name(req->op.rename.olddn); @@ -1350,36 +1367,21 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) talloc_free(tmp_ctx); return ldb_operr(ldb); } - guid = attribute_schemaid_guid_by_lDAPDisplayName(schema, - rdn_name); - if (!insert_in_object_tree(tmp_ctx, guid, SEC_ADS_WRITE_PROP, - &new_node, &new_node)) { - talloc_free(tmp_ctx); - return ldb_operr(ldb); - }; - - ret = dsdb_get_sd_from_ldb_message(ldb, req, acl_res->msgs[0], &sd); - if (ret != LDB_SUCCESS) { + attr = dsdb_attribute_by_lDAPDisplayName(schema, rdn_name); + if (attr == NULL) { talloc_free(tmp_ctx); return ldb_operr(ldb); } - /* Theoretically we pass the check if the object has no sd */ - if (!sd) { - talloc_free(tmp_ctx); - return LDB_SUCCESS; - } - sid = samdb_result_dom_sid(req, acl_res->msgs[0], "objectSid"); - status = sec_access_check_ds(sd, acl_user_token(module), - SEC_ADS_WRITE_PROP, - &access_granted, - root, - sid); - if (!NT_STATUS_IS_OK(status)) { + ret = acl_check_access_on_attribute(module, tmp_ctx, sd, sid, + SEC_ADS_WRITE_PROP, + attr, objectclass); + if (ret != LDB_SUCCESS) { ldb_asprintf_errstring(ldb_module_get_ctx(module), - "Object %s has no wp on name\n", - ldb_dn_get_linearized(req->op.rename.olddn)); + "Object %s has no wp on %s\n", + ldb_dn_get_linearized(req->op.rename.olddn), + attr->lDAPDisplayName); dsdb_acl_debug(sd, acl_user_token(module), req->op.rename.olddn, @@ -1396,9 +1398,6 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) } /* new parent should have create child */ - root = NULL; - new_node = NULL; - ret = dsdb_module_check_access_on_dn(module, req, newparent, SEC_ADS_CREATE_CHILD, &objectclass->schemaIDGUID, req); @@ -1409,15 +1408,12 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) talloc_free(tmp_ctx); return ret; } - /* do we have delete object on the object? */ - - status = sec_access_check_ds(sd, acl_user_token(module), - SEC_STD_DELETE, - &access_granted, - NULL, - sid); - if (NT_STATUS_IS_OK(status)) { + /* do we have delete object on the object? */ + ret = acl_check_access_on_objectclass(module, tmp_ctx, sd, sid, + SEC_STD_DELETE, + objectclass); + if (ret == LDB_SUCCESS) { talloc_free(tmp_ctx); return ldb_next_request(module, req); } -- 1.7.11.7 From 6bbdc6306096d89b61c2c626d37dc3cd909ed789 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 17 Jan 2013 16:21:10 +0100 Subject: [PATCH 29/44] dsdb-acl: make use of acl_check_access_on_objectclass() for the object in acl_delete() We should only use dsdb_module_check_access_on_dn() on the parent. Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 8f8d97f9fe05b2de1403676a148ab7b90a83812b) --- source4/dsdb/samdb/ldb_modules/acl.c | 56 ++++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 0ab9670..41c257b 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -1163,6 +1163,17 @@ static int acl_delete(struct ldb_module *module, struct ldb_request *req) struct ldb_context *ldb; struct ldb_dn *nc_root; struct ldb_control *as_system; + const struct dsdb_schema *schema; + const struct dsdb_class *objectclass; + struct security_descriptor *sd = NULL; + struct dom_sid *sid = NULL; + struct ldb_result *acl_res; + static const char *acl_attrs[] = { + "nTSecurityDescriptor", + "objectClass", + "objectSid", + NULL + }; if (ldb_dn_is_special(req->op.del.dn)) { return ldb_next_request(module, req); @@ -1201,11 +1212,43 @@ static int acl_delete(struct ldb_module *module, struct ldb_request *req) } talloc_free(nc_root); + ret = dsdb_module_search_dn(module, req, &acl_res, + req->op.del.dn, acl_attrs, + DSDB_FLAG_NEXT_MODULE | + DSDB_FLAG_AS_SYSTEM | + DSDB_SEARCH_SHOW_RECYCLED, req); + /* we sould be able to find the parent */ + if (ret != LDB_SUCCESS) { + DEBUG(10,("acl: failed to find object %s\n", + ldb_dn_get_linearized(req->op.rename.olddn))); + return ret; + } + + ret = dsdb_get_sd_from_ldb_message(ldb, req, acl_res->msgs[0], &sd); + if (ret != LDB_SUCCESS) { + return ldb_operr(ldb); + } + if (!sd) { + return ldb_operr(ldb); + } + + schema = dsdb_get_schema(ldb, req); + if (!schema) { + return ldb_operr(ldb); + } + + sid = samdb_result_dom_sid(req, acl_res->msgs[0], "objectSid"); + + objectclass = dsdb_get_structural_oc_from_msg(schema, acl_res->msgs[0]); + if (!objectclass) { + return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR, + "acl_modify: Error retrieving object class for GUID."); + } + if (ldb_request_get_control(req, LDB_CONTROL_TREE_DELETE_OID)) { - ret = dsdb_module_check_access_on_dn(module, req, - req->op.del.dn, - SEC_ADS_DELETE_TREE, NULL, - req); + ret = acl_check_access_on_objectclass(module, req, sd, sid, + SEC_ADS_DELETE_TREE, + objectclass); if (ret != LDB_SUCCESS) { return ret; } @@ -1214,8 +1257,9 @@ static int acl_delete(struct ldb_module *module, struct ldb_request *req) } /* First check if we have delete object right */ - ret = dsdb_module_check_access_on_dn(module, req, req->op.del.dn, - SEC_STD_DELETE, NULL, req); + ret = acl_check_access_on_objectclass(module, req, sd, sid, + SEC_STD_DELETE, + objectclass); if (ret == LDB_SUCCESS) { return ldb_next_request(module, req); } -- 1.7.11.7 From 003900df977fc415a263b9b31746deebf773a146 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 17 Jan 2013 16:22:09 +0100 Subject: [PATCH 30/44] dsdb-acl: the SEC_ADS_DELETE_CHILD checks need objectclass->schemaIDGUID Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 0ebb93708eb377e29eaaf4400c65399d18c229b6) --- source4/dsdb/samdb/ldb_modules/acl.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 41c257b..75b871f 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -1267,7 +1267,9 @@ static int acl_delete(struct ldb_module *module, struct ldb_request *req) /* Nope, we don't have delete object. Lets check if we have delete * child on the parent */ ret = dsdb_module_check_access_on_dn(module, req, parent, - SEC_ADS_DELETE_CHILD, NULL, req); + SEC_ADS_DELETE_CHILD, + &objectclass->schemaIDGUID, + req); if (ret != LDB_SUCCESS) { return ret; } @@ -1462,7 +1464,10 @@ static int acl_rename(struct ldb_module *module, struct ldb_request *req) return ldb_next_request(module, req); } /* what about delete child on the current parent */ - ret = dsdb_module_check_access_on_dn(module, req, oldparent, SEC_ADS_DELETE_CHILD, NULL, req); + ret = dsdb_module_check_access_on_dn(module, req, oldparent, + SEC_ADS_DELETE_CHILD, + &objectclass->schemaIDGUID, + req); if (ret != LDB_SUCCESS) { ldb_asprintf_errstring(ldb_module_get_ctx(module), "acl:access_denied renaming %s", ldb_dn_get_linearized(req->op.rename.olddn)); -- 1.7.11.7 From 94e81e03ed4f0cad5bc7706d000067f6c9c865a0 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Jan 2013 09:43:44 +0100 Subject: [PATCH 31/44] libcli/security: fix whitespaces in access_check.c Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 10a90ce8422ac4ff4461b13a3dd03bbcd9bd2258) --- libcli/security/access_check.c | 198 +++++++++++++++++++++-------------------- 1 file changed, 100 insertions(+), 98 deletions(-) diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c index 70345f5..0a8d0a4 100644 --- a/libcli/security/access_check.c +++ b/libcli/security/access_check.c @@ -367,14 +367,14 @@ NTSTATUS se_file_access_check(const struct security_descriptor *sd, static const struct GUID *get_ace_object_type(struct security_ace *ace) { - struct GUID *type; + struct GUID *type; - if (ace->object.object.flags & SEC_ACE_OBJECT_TYPE_PRESENT) - type = &ace->object.object.type.type; - else - type = NULL; + if (ace->object.object.flags & SEC_ACE_OBJECT_TYPE_PRESENT) + type = &ace->object.object.type.type; + else + type = NULL; - return type; + return type; } @@ -389,31 +389,31 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, struct object_tree *tree, struct dom_sid *replace_sid) { - uint32_t i; - uint32_t bits_remaining; - struct object_tree *node; - const struct GUID *type; - struct dom_sid *ps_sid = dom_sid_parse_talloc(NULL, SID_NT_SELF); - - *access_granted = access_desired; - bits_remaining = access_desired; - - /* handle the maximum allowed flag */ - if (access_desired & SEC_FLAG_MAXIMUM_ALLOWED) { - access_desired |= access_check_max_allowed(sd, token); - access_desired &= ~SEC_FLAG_MAXIMUM_ALLOWED; - *access_granted = access_desired; + uint32_t i; + uint32_t bits_remaining; + struct object_tree *node; + const struct GUID *type; + struct dom_sid *ps_sid = dom_sid_parse_talloc(NULL, SID_NT_SELF); + + *access_granted = access_desired; + bits_remaining = access_desired; + + /* handle the maximum allowed flag */ + if (access_desired & SEC_FLAG_MAXIMUM_ALLOWED) { + access_desired |= access_check_max_allowed(sd, token); + access_desired &= ~SEC_FLAG_MAXIMUM_ALLOWED; + *access_granted = access_desired; bits_remaining = access_desired; - } + } - if (access_desired & SEC_FLAG_SYSTEM_SECURITY) { - if (security_token_has_privilege(token, SEC_PRIV_SECURITY)) { - bits_remaining &= ~SEC_FLAG_SYSTEM_SECURITY; - } else { - talloc_free(ps_sid); - return NT_STATUS_PRIVILEGE_NOT_HELD; - } - } + if (access_desired & SEC_FLAG_SYSTEM_SECURITY) { + if (security_token_has_privilege(token, SEC_PRIV_SECURITY)) { + bits_remaining &= ~SEC_FLAG_SYSTEM_SECURITY; + } else { + talloc_free(ps_sid); + return NT_STATUS_PRIVILEGE_NOT_HELD; + } + } /* the owner always gets SEC_STD_WRITE_DAC and SEC_STD_READ_CONTROL */ if ((bits_remaining & (SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL)) && @@ -431,25 +431,25 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, bits_remaining &= ~(SEC_RIGHTS_PRIV_BACKUP); } - /* a NULL dacl allows access */ - if ((sd->type & SEC_DESC_DACL_PRESENT) && sd->dacl == NULL) { - *access_granted = access_desired; - talloc_free(ps_sid); - return NT_STATUS_OK; - } + /* a NULL dacl allows access */ + if ((sd->type & SEC_DESC_DACL_PRESENT) && sd->dacl == NULL) { + *access_granted = access_desired; + talloc_free(ps_sid); + return NT_STATUS_OK; + } - if (sd->dacl == NULL) { - goto done; - } + if (sd->dacl == NULL) { + goto done; + } - /* check each ace in turn. */ - for (i=0; bits_remaining && i < sd->dacl->num_aces; i++) { + /* check each ace in turn. */ + for (i=0; bits_remaining && i < sd->dacl->num_aces; i++) { struct dom_sid *trustee; struct security_ace *ace = &sd->dacl->aces[i]; - if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) { - continue; - } + if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) { + continue; + } if (dom_sid_equal(&ace->trustee, ps_sid) && replace_sid) { trustee = replace_sid; } @@ -457,62 +457,64 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, { trustee = &ace->trustee; } - if (!security_token_has_sid(token, trustee)) { - continue; - } - - switch (ace->type) { - case SEC_ACE_TYPE_ACCESS_ALLOWED: - if (tree) - object_tree_modify_access(tree, ace->access_mask); - - bits_remaining &= ~ace->access_mask; - break; - case SEC_ACE_TYPE_ACCESS_DENIED: - if (bits_remaining & ace->access_mask) { - talloc_free(ps_sid); - return NT_STATUS_ACCESS_DENIED; - } - break; - case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT: - case SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT: - /* check only in case we have provided a tree, - * the ACE has an object type and that type - * is in the tree */ - type = get_ace_object_type(ace); - - if (!tree) - continue; - - if (!type) - node = tree; - else - if (!(node = get_object_tree_by_GUID(tree, type))) - continue; - - if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) { - object_tree_modify_access(node, ace->access_mask); - if (node->remaining_access == 0) { - talloc_free(ps_sid); - return NT_STATUS_OK; - } - } else { - if (node->remaining_access & ace->access_mask){ - talloc_free(ps_sid); - return NT_STATUS_ACCESS_DENIED; - } - } - break; - default: /* Other ACE types not handled/supported */ - break; - } - } + if (!security_token_has_sid(token, trustee)) { + continue; + } + + switch (ace->type) { + case SEC_ACE_TYPE_ACCESS_ALLOWED: + if (tree) + object_tree_modify_access(tree, ace->access_mask); + + bits_remaining &= ~ace->access_mask; + break; + case SEC_ACE_TYPE_ACCESS_DENIED: + if (bits_remaining & ace->access_mask) { + talloc_free(ps_sid); + return NT_STATUS_ACCESS_DENIED; + } + break; + case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT: + case SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT: + /* + * check only in case we have provided a tree, + * the ACE has an object type and that type + * is in the tree + */ + type = get_ace_object_type(ace); + + if (!tree) + continue; + + if (!type) + node = tree; + else + if (!(node = get_object_tree_by_GUID(tree, type))) + continue; + + if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) { + object_tree_modify_access(node, ace->access_mask); + if (node->remaining_access == 0) { + talloc_free(ps_sid); + return NT_STATUS_OK; + } + } else { + if (node->remaining_access & ace->access_mask){ + talloc_free(ps_sid); + return NT_STATUS_ACCESS_DENIED; + } + } + break; + default: /* Other ACE types not handled/supported */ + break; + } + } done: - talloc_free(ps_sid); - if (bits_remaining != 0) { - return NT_STATUS_ACCESS_DENIED; - } + talloc_free(ps_sid); + if (bits_remaining != 0) { + return NT_STATUS_ACCESS_DENIED; + } - return NT_STATUS_OK; + return NT_STATUS_OK; } -- 1.7.11.7 From 0fe46ce45d73f1398aa0aef3d0f3ede040748081 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Jan 2013 09:46:48 +0100 Subject: [PATCH 32/44] libcli/security: fix formating in access_check.c Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit b0f731fc3b96edf91216829bd0dc63bb4269f458) --- libcli/security/access_check.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c index 0a8d0a4..3be322e 100644 --- a/libcli/security/access_check.c +++ b/libcli/security/access_check.c @@ -450,21 +450,22 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) { continue; } + if (dom_sid_equal(&ace->trustee, ps_sid) && replace_sid) { trustee = replace_sid; - } - else - { + } else { trustee = &ace->trustee; } + if (!security_token_has_sid(token, trustee)) { continue; } switch (ace->type) { case SEC_ACE_TYPE_ACCESS_ALLOWED: - if (tree) + if (tree) { object_tree_modify_access(tree, ace->access_mask); + } bits_remaining &= ~ace->access_mask; break; @@ -483,14 +484,17 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, */ type = get_ace_object_type(ace); - if (!tree) + if (!tree) { continue; + } - if (!type) + if (!type) { node = tree; - else - if (!(node = get_object_tree_by_GUID(tree, type))) + } else { + if (!(node = get_object_tree_by_GUID(tree, type))) { continue; + } + } if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) { object_tree_modify_access(node, ace->access_mask); -- 1.7.11.7 From 014ab82df9667a432840c6892ff8e12f3dfd5803 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Jan 2013 10:05:56 +0100 Subject: [PATCH 33/44] libcli/security: simplify get_ace_object_type() Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit a3fffde368fa0c6594f7fd5309e0b20d3fa7c68e) --- libcli/security/access_check.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c index 3be322e..83b7f9b 100644 --- a/libcli/security/access_check.c +++ b/libcli/security/access_check.c @@ -367,15 +367,11 @@ NTSTATUS se_file_access_check(const struct security_descriptor *sd, static const struct GUID *get_ace_object_type(struct security_ace *ace) { - struct GUID *type; - - if (ace->object.object.flags & SEC_ACE_OBJECT_TYPE_PRESENT) - type = &ace->object.object.type.type; - else - type = NULL; - - return type; + if (ace->object.object.flags & SEC_ACE_OBJECT_TYPE_PRESENT) { + return &ace->object.object.type.type; + } + return NULL; } /* modified access check for the purposes of DS security -- 1.7.11.7 From c2c79aedfcf162b1242d087dbc7302af0e34ca40 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Jan 2013 09:49:20 +0100 Subject: [PATCH 34/44] libcli/security: avoid usage of dom_sid_parse_talloc() in sec_access_check_ds() Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit a359aef0837781c42bf9dbcdd069796c72cc94c7) --- libcli/security/access_check.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c index 83b7f9b..f0a7b66 100644 --- a/libcli/security/access_check.c +++ b/libcli/security/access_check.c @@ -389,7 +389,9 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, uint32_t bits_remaining; struct object_tree *node; const struct GUID *type; - struct dom_sid *ps_sid = dom_sid_parse_talloc(NULL, SID_NT_SELF); + struct dom_sid self_sid; + + dom_sid_parse(SID_NT_SELF, &self_sid); *access_granted = access_desired; bits_remaining = access_desired; @@ -406,7 +408,6 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, if (security_token_has_privilege(token, SEC_PRIV_SECURITY)) { bits_remaining &= ~SEC_FLAG_SYSTEM_SECURITY; } else { - talloc_free(ps_sid); return NT_STATUS_PRIVILEGE_NOT_HELD; } } @@ -430,7 +431,6 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, /* a NULL dacl allows access */ if ((sd->type & SEC_DESC_DACL_PRESENT) && sd->dacl == NULL) { *access_granted = access_desired; - talloc_free(ps_sid); return NT_STATUS_OK; } @@ -447,7 +447,7 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, continue; } - if (dom_sid_equal(&ace->trustee, ps_sid) && replace_sid) { + if (dom_sid_equal(&ace->trustee, &self_sid) && replace_sid) { trustee = replace_sid; } else { trustee = &ace->trustee; @@ -467,7 +467,6 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, break; case SEC_ACE_TYPE_ACCESS_DENIED: if (bits_remaining & ace->access_mask) { - talloc_free(ps_sid); return NT_STATUS_ACCESS_DENIED; } break; @@ -495,12 +494,10 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) { object_tree_modify_access(node, ace->access_mask); if (node->remaining_access == 0) { - talloc_free(ps_sid); return NT_STATUS_OK; } } else { if (node->remaining_access & ace->access_mask){ - talloc_free(ps_sid); return NT_STATUS_ACCESS_DENIED; } } @@ -511,7 +508,6 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, } done: - talloc_free(ps_sid); if (bits_remaining != 0) { return NT_STATUS_ACCESS_DENIED; } -- 1.7.11.7 From 13cb4ddf8cdff9b0a17463758b33599b1ac769c1 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 3 Jan 2013 20:40:32 +1100 Subject: [PATCH 35/44] libcli/security: handle node initialisation in one spot in insert_in_object_tree() This removes special-case for initalising the children array in insert_in_object_tree(). talloc_realloc() handles the intial allocate case perfectly well, so there is no need to have this duplicated. This also restores having just one place were the rest of the elements are intialised, to ensure uniform behaviour. To do this, we have to rework insert_in_object_tree to have only one output variable, both because having both root and new_node as output variables was too confusing, and because otherwise the two pointers were being allowed to point at the same memory. Andrew Bartlett Signed-off-by: Stefan Metzmacher Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 5b4e3de2bb25eeb85d72a886386c853cea3e9468) --- libcli/security/access_check.h | 8 ++-- libcli/security/object_tree.c | 67 +++++++++++++++---------------- source4/dsdb/common/dsdb_access.c | 5 +-- source4/dsdb/samdb/ldb_modules/acl_util.c | 16 ++++---- 4 files changed, 47 insertions(+), 49 deletions(-) diff --git a/libcli/security/access_check.h b/libcli/security/access_check.h index 84b2e5f..952589d 100644 --- a/libcli/security/access_check.h +++ b/libcli/security/access_check.h @@ -77,10 +77,10 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, struct dom_sid *replace_sid); bool insert_in_object_tree(TALLOC_CTX *mem_ctx, - const struct GUID *guid, - uint32_t init_access, - struct object_tree **root, - struct object_tree **new_node); + const struct GUID *guid, + uint32_t init_access, + struct object_tree *root, + struct object_tree **new_node_out); /* search by GUID */ struct object_tree *get_object_tree_by_GUID(struct object_tree *root, diff --git a/libcli/security/object_tree.c b/libcli/security/object_tree.c index dcbd310..a629177 100644 --- a/libcli/security/object_tree.c +++ b/libcli/security/object_tree.c @@ -38,52 +38,51 @@ */ bool insert_in_object_tree(TALLOC_CTX *mem_ctx, - const struct GUID *guid, - uint32_t init_access, - struct object_tree **root, - struct object_tree **new_node) + const struct GUID *guid, + uint32_t init_access, + struct object_tree *root, + struct object_tree **new_node_out) { + struct object_tree *new_node; + if (!guid || GUID_all_zero(guid)){ return true; } - if (!*root){ - *root = talloc_zero(mem_ctx, struct object_tree); - if (!*root) { + if (!root) { + root = talloc_zero(mem_ctx, struct object_tree); + if (!root) { return false; } - (*root)->guid = *guid; - (*root)->remaining_access = init_access; - *new_node = *root; - return true; - } - - if (!(*root)->children) { - (*root)->children = talloc_array(mem_ctx, struct object_tree, 1); - (*root)->children[0].guid = *guid; - (*root)->children[0].num_of_children = 0; - (*root)->children[0].children = NULL; - (*root)->num_of_children++; - (*root)->children[0].remaining_access = init_access; - *new_node = &((*root)->children[0]); - return true; - } - else { + new_node = root; + } else { int i; - for (i = 0; i < (*root)->num_of_children; i++) { - if (GUID_equal(&((*root)->children[i].guid), guid)) { - *new_node = &((*root)->children[i]); + + for (i = 0; i < root->num_of_children; i++) { + if (GUID_equal(&root->children[i].guid, guid)) { + new_node = &root->children[i]; + *new_node_out = new_node; return true; } } - (*root)->children = talloc_realloc(mem_ctx, (*root)->children, struct object_tree, - (*root)->num_of_children +1); - (*root)->children[(*root)->num_of_children].guid = *guid; - (*root)->children[(*root)->num_of_children].remaining_access = init_access; - *new_node = &((*root)->children[(*root)->num_of_children]); - (*root)->num_of_children++; - return true; + + root->children = talloc_realloc(mem_ctx, root->children, + struct object_tree, + root->num_of_children + 1); + if (!root->children) { + return false; + } + new_node = &root->children[root->num_of_children]; + root->num_of_children++; } + + new_node->children = NULL; + new_node->guid = *guid; + new_node->remaining_access = init_access; + new_node->num_of_children = 0; + + *new_node_out = new_node; + return true; } /* search by GUID */ diff --git a/source4/dsdb/common/dsdb_access.c b/source4/dsdb/common/dsdb_access.c index fd75e77..6af5c3a 100644 --- a/source4/dsdb/common/dsdb_access.c +++ b/source4/dsdb/common/dsdb_access.c @@ -93,7 +93,6 @@ int dsdb_check_access_on_dn_internal(struct ldb_context *ldb, struct security_descriptor *sd = NULL; struct dom_sid *sid = NULL; struct object_tree *root = NULL; - struct object_tree *new_node = NULL; NTSTATUS status; uint32_t access_granted; int ret; @@ -108,8 +107,8 @@ int dsdb_check_access_on_dn_internal(struct ldb_context *ldb, } sid = samdb_result_dom_sid(mem_ctx, acl_res->msgs[0], "objectSid"); if (guid) { - if (!insert_in_object_tree(mem_ctx, guid, access_mask, &root, - &new_node)) { + if (!insert_in_object_tree(mem_ctx, guid, access_mask, NULL, + &root)) { return ldb_operr(ldb); } } diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c index 09ca201..795a39c 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_util.c +++ b/source4/dsdb/samdb/ldb_modules/acl_util.c @@ -109,16 +109,17 @@ int acl_check_access_on_attribute(struct ldb_module *module, if (!insert_in_object_tree(tmp_ctx, &objectclass->schemaIDGUID, - access_mask, &root, - &new_node)) { + access_mask, NULL, + &root)) { DEBUG(10, ("acl_search: cannot add to object tree class schemaIDGUID\n")); goto fail; } + new_node = root; if (!GUID_all_zero(&attr->attributeSecurityGUID)) { if (!insert_in_object_tree(tmp_ctx, &attr->attributeSecurityGUID, - access_mask, &new_node, + access_mask, new_node, &new_node)) { DEBUG(10, ("acl_search: cannot add to object tree securityGUID\n")); goto fail; @@ -127,7 +128,7 @@ int acl_check_access_on_attribute(struct ldb_module *module, if (!insert_in_object_tree(tmp_ctx, &attr->schemaIDGUID, - access_mask, &new_node, + access_mask, new_node, &new_node)) { DEBUG(10, ("acl_search: cannot add to object tree attributeGUID\n")); goto fail; @@ -162,14 +163,13 @@ int acl_check_access_on_objectclass(struct ldb_module *module, NTSTATUS status; uint32_t access_granted; struct object_tree *root = NULL; - struct object_tree *new_node = NULL; TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx); struct security_token *token = acl_user_token(module); if (!insert_in_object_tree(tmp_ctx, &objectclass->schemaIDGUID, - access_mask, &root, - &new_node)) { + access_mask, NULL, + &root)) { DEBUG(10, ("acl_search: cannot add to object tree class schemaIDGUID\n")); goto fail; } @@ -209,7 +209,7 @@ int acl_check_extended_right(TALLOC_CTX *mem_ctx, GUID_from_string(ext_right, &right); if (!insert_in_object_tree(tmp_ctx, &right, right_type, - &root, &new_node)) { + NULL, &root)) { DEBUG(10, ("acl_ext_right: cannot add to object tree\n")); talloc_free(tmp_ctx); return LDB_ERR_OPERATIONS_ERROR; -- 1.7.11.7 From 3b076ccc80168f6613dacc23be06e8147bd5ac2c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 15 Jan 2013 19:03:00 +0100 Subject: [PATCH 36/44] libcli/security: add init_mask to existing children in insert_in_object_tree Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 853ecd418afe15973d3e8844ad0e01d3d54536d5) --- libcli/security/object_tree.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libcli/security/object_tree.c b/libcli/security/object_tree.c index a629177..fb523be 100644 --- a/libcli/security/object_tree.c +++ b/libcli/security/object_tree.c @@ -61,6 +61,7 @@ bool insert_in_object_tree(TALLOC_CTX *mem_ctx, for (i = 0; i < root->num_of_children; i++) { if (GUID_equal(&root->children[i].guid, guid)) { new_node = &root->children[i]; + new_node->remaining_access |= init_access; *new_node_out = new_node; return true; } -- 1.7.11.7 From 2df6b342b7cb1d4c7b2e4384f1f27ed230dca127 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 3 Jan 2013 21:30:12 +1100 Subject: [PATCH 37/44] libcli/security: remove useless if (root->num_of_children > 0) statements The for loop does this implicitly when comparing for (i = 0; i < root->num_of_children; i++) Andrew Bartlett Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit d36c03056fb85dfedbafd3a59497e35db63ade17) --- libcli/security/object_tree.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/libcli/security/object_tree.c b/libcli/security/object_tree.c index fb523be..3e5ee10 100644 --- a/libcli/security/object_tree.c +++ b/libcli/security/object_tree.c @@ -97,11 +97,9 @@ struct object_tree *get_object_tree_by_GUID(struct object_tree *root, result = root; return result; } - else if (root->num_of_children > 0) { - for (i = 0; i < root->num_of_children; i++) { + for (i = 0; i < root->num_of_children; i++) { if ((result = get_object_tree_by_GUID(&root->children[i], guid))) break; - } } return result; } @@ -111,11 +109,9 @@ struct object_tree *get_object_tree_by_GUID(struct object_tree *root, void object_tree_modify_access(struct object_tree *root, uint32_t access_mask) { + int i; root->remaining_access &= ~access_mask; - if (root->num_of_children > 0) { - int i; - for (i = 0; i < root->num_of_children; i++) { - object_tree_modify_access(&root->children[i], access_mask); - } + for (i = 0; i < root->num_of_children; i++) { + object_tree_modify_access(&root->children[i], access_mask); } } -- 1.7.11.7 From 7854c26ae3f8476b37c4aa3cfd4b3f7a0b488a9c Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Sat, 29 Dec 2012 15:13:54 +1100 Subject: [PATCH 38/44] dsdb: Ensure "authenticated users" is processed for group memberships This change moves the addition of "Authenticated Users" from the very end of the token processing to the start. The reason is that we need to see if "Authenticated Users" is a member of other builtin groups, just as we would for any other SID. This picks up the "Pre-Windows 2000 Compatible Access" group, which is in turn often used in ACLs on LDAP objects. Without this change, the eventual token does not contain S-1-5-32-554 and users other than "Administrator" are unable to read uidNumber (in particular). Andrew Bartlett Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 8f078cdf247476fad511bb6d7e00c8654fd26e85) --- source4/auth/session.c | 44 ++++++++++++++++++++++++++++++++++----- source4/dsdb/common/util_groups.c | 25 ++++++++++++++++++++++ source4/dsdb/samdb/samdb.c | 31 --------------------------- 3 files changed, 64 insertions(+), 36 deletions(-) diff --git a/source4/auth/session.c b/source4/auth/session.c index de417cc..bb0b5bc 100644 --- a/source4/auth/session.c +++ b/source4/auth/session.c @@ -102,22 +102,56 @@ _PUBLIC_ NTSTATUS auth_generate_session_info(TALLOC_CTX *mem_ctx, sids[i] = user_info_dc->sids[i]; } - if (user_info_dc->num_sids > PRIMARY_USER_SID_INDEX && dom_sid_equal(anonymous_sid, &user_info_dc->sids[PRIMARY_USER_SID_INDEX])) { + /* + * Finally add the "standard" sids. + * The only difference between guest and "anonymous" + * is the addition of Authenticated_Users. + */ + + if (session_info_flags & AUTH_SESSION_INFO_DEFAULT_GROUPS) { + sids = talloc_realloc(tmp_ctx, sids, struct dom_sid, num_sids + 2); + NT_STATUS_HAVE_NO_MEMORY(sids); + + if (!dom_sid_parse(SID_WORLD, &sids[num_sids])) { + return NT_STATUS_INTERNAL_ERROR; + } + num_sids++; + + if (!dom_sid_parse(SID_NT_NETWORK, &sids[num_sids])) { + return NT_STATUS_INTERNAL_ERROR; + } + num_sids++; + } + + if (session_info_flags & AUTH_SESSION_INFO_AUTHENTICATED) { + sids = talloc_realloc(tmp_ctx, sids, struct dom_sid, num_sids + 1); + NT_STATUS_HAVE_NO_MEMORY(sids); + + if (!dom_sid_parse(SID_NT_AUTHENTICATED_USERS, &sids[num_sids])) { + return NT_STATUS_INTERNAL_ERROR; + } + num_sids++; + } + + + + if (num_sids > PRIMARY_USER_SID_INDEX && dom_sid_equal(anonymous_sid, &sids[PRIMARY_USER_SID_INDEX])) { /* Don't expand nested groups of system, anonymous etc*/ - } else if (user_info_dc->num_sids > PRIMARY_USER_SID_INDEX && dom_sid_equal(system_sid, &user_info_dc->sids[PRIMARY_USER_SID_INDEX])) { + } else if (num_sids > PRIMARY_USER_SID_INDEX && dom_sid_equal(system_sid, &sids[PRIMARY_USER_SID_INDEX])) { /* Don't expand nested groups of system, anonymous etc*/ } else if (sam_ctx) { filter = talloc_asprintf(tmp_ctx, "(&(objectClass=group)(groupType:1.2.840.113556.1.4.803:=%u))", GROUP_TYPE_BUILTIN_LOCAL_GROUP); /* Search for each group in the token */ - for (i = 0; i < user_info_dc->num_sids; i++) { + for (i = 0; i < num_sids; i++) { char *sid_string; const char *sid_dn; DATA_BLOB sid_blob; - + int ret; + sid_string = dom_sid_string(tmp_ctx, - &user_info_dc->sids[i]); + &sids[i]); NT_STATUS_HAVE_NO_MEMORY_AND_FREE(sid_string, user_info_dc); sid_dn = talloc_asprintf(tmp_ctx, "", sid_string); diff --git a/source4/dsdb/common/util_groups.c b/source4/dsdb/common/util_groups.c index b5aecba..6a96ce8 100644 --- a/source4/dsdb/common/util_groups.c +++ b/source4/dsdb/common/util_groups.c @@ -126,6 +126,31 @@ NTSTATUS dsdb_expand_nested_groups(struct ldb_context *sam_ctx, filter); } + /* + * We have the problem with the caller creating a + * DN for ForeignSecurityPrincipals as they also have + * duplicate objects with the SAME SID under CN=Configuration. + * This causes a SID= DN to fail with NO_SUCH_OBJECT on Samba + * and on Windows. So, we allow this to fail, and + * double-check if we can find it with a search in the main + * domain partition. + */ + if (ret == LDB_ERR_NO_SUCH_OBJECT && only_childs) { + char *sid_string = dom_sid_string(tmp_ctx, + &sid); + if (!sid_string) { + talloc_free(tmp_ctx); + return NT_STATUS_OK; + } + + ret = dsdb_search(sam_ctx, tmp_ctx, &res, + ldb_get_default_basedn(sam_ctx), + LDB_SCOPE_SUBTREE, + attrs, DSDB_SEARCH_SHOW_EXTENDED_DN, + "(&(objectClass=foreignSecurityPrincipal)(objectSID=%s))", + sid_string); + } + if (ret == LDB_ERR_NO_SUCH_OBJECT) { talloc_free(tmp_ctx); return NT_STATUS_OK; diff --git a/source4/dsdb/samdb/samdb.c b/source4/dsdb/samdb/samdb.c index 713448c..361ece7 100644 --- a/source4/dsdb/samdb/samdb.c +++ b/source4/dsdb/samdb/samdb.c @@ -143,37 +143,6 @@ NTSTATUS security_token_create(TALLOC_CTX *mem_ctx, } } - /* - * Finally add the "standard" sids. - * The only difference between guest and "anonymous" - * is the addition of Authenticated_Users. - */ - - if (session_info_flags & AUTH_SESSION_INFO_DEFAULT_GROUPS) { - ptoken->sids = talloc_realloc(ptoken, ptoken->sids, struct dom_sid, ptoken->num_sids + 2); - NT_STATUS_HAVE_NO_MEMORY(ptoken->sids); - - if (!dom_sid_parse(SID_WORLD, &ptoken->sids[ptoken->num_sids])) { - return NT_STATUS_INTERNAL_ERROR; - } - ptoken->num_sids++; - - if (!dom_sid_parse(SID_NT_NETWORK, &ptoken->sids[ptoken->num_sids])) { - return NT_STATUS_INTERNAL_ERROR; - } - ptoken->num_sids++; - } - - if (session_info_flags & AUTH_SESSION_INFO_AUTHENTICATED) { - ptoken->sids = talloc_realloc(ptoken, ptoken->sids, struct dom_sid, ptoken->num_sids + 1); - NT_STATUS_HAVE_NO_MEMORY(ptoken->sids); - - if (!dom_sid_parse(SID_NT_AUTHENTICATED_USERS, &ptoken->sids[ptoken->num_sids])) { - return NT_STATUS_INTERNAL_ERROR; - } - ptoken->num_sids++; - } - /* The caller may have requested simple privilages, for example if there isn't a local DB */ if (session_info_flags & AUTH_SESSION_INFO_SIMPLE_PRIVILEGES) { /* Shortcuts to prevent recursion and avoid lookups */ -- 1.7.11.7 From c05da2ce5a7c46abdb2aff232a811a87bc3e7ce0 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 2 Jan 2013 09:27:51 +1100 Subject: [PATCH 39/44] dsdb: Explain ordering constraints on the ACL module as well. Andrew Bartlett Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit c52408f461fb3515cde17eebb458b566fd0a049c) --- source4/dsdb/samdb/ldb_modules/samba_dsdb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/samba_dsdb.c b/source4/dsdb/samdb/ldb_modules/samba_dsdb.c index 361059f..d10d1bb 100644 --- a/source4/dsdb/samdb/ldb_modules/samba_dsdb.c +++ b/source4/dsdb/samdb/ldb_modules/samba_dsdb.c @@ -150,8 +150,8 @@ static int samba_dsdb_init(struct ldb_module *module) - extended_dn_in must be before objectclass.c, as it resolves the DN - objectclass must be before password_hash and samldb since these LDB modules require the expanded "objectClass" list - - objectclass must be before descriptor, as descriptor assumes that - objectClass values are sorted + - objectclass must be before descriptor and acl, as both assume that + objectClass values are sorted - objectclass_attrs must be behind operational in order to see all attributes (the operational module protects and therefore suppresses per default some important ones) -- 1.7.11.7 From 2f5d9bfc996dd4bbfca9aaa6370ccc38cf6468bd Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sat, 29 Dec 2012 16:43:44 -0800 Subject: [PATCH 40/44] dsdb: Fix warning about unused var Reviewed-by: Stefan Metzmacher Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Mon Jan 21 17:51:16 CET 2013 on sn-devel-104 (cherry picked from commit abc0030f780b775bf7656b572ee754ebd8079b5d) --- source4/dsdb/samdb/ldb_modules/extended_dn_out.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_out.c b/source4/dsdb/samdb/ldb_modules/extended_dn_out.c index 8e28ec7..b1eacf5 100644 --- a/source4/dsdb/samdb/ldb_modules/extended_dn_out.c +++ b/source4/dsdb/samdb/ldb_modules/extended_dn_out.c @@ -707,7 +707,6 @@ static int extended_dn_out_search(struct ldb_module *module, struct ldb_request const char * const *const_attrs; struct ldb_context *ldb = ldb_module_get_ctx(module); int ret; - bool critical; struct extended_dn_out_private *p = talloc_get_type(ldb_module_get_private(module), struct extended_dn_out_private); @@ -806,7 +805,6 @@ static int extended_dn_out_search(struct ldb_module *module, struct ldb_request /* mark extended DN and storage format controls as done */ if (control) { - critical = control->critical; control->critical = 0; } -- 1.7.11.7 From e94a2444870f283f0803f7adcb09309526aca16d Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sat, 13 Oct 2012 15:28:08 -0700 Subject: [PATCH 41/44] libcli-security: Add documentation for object_tree_modify_access Reviewed-by: Andrew Bartlett (cherry picked from commit c0638dae6cbf8915e6a436d575562fc131ba772a) --- libcli/security/object_tree.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/libcli/security/object_tree.c b/libcli/security/object_tree.c index 3e5ee10..fd00068 100644 --- a/libcli/security/object_tree.c +++ b/libcli/security/object_tree.c @@ -104,8 +104,18 @@ struct object_tree *get_object_tree_by_GUID(struct object_tree *root, return result; } -/* Change the granted access per each ACE */ - +/** + * @brief Modify the tree to mark specified access rights as granted + * + * This function will modify the root and the child of the tree pointed by + * root, so that for each tree element the bits set in access_mask are + * marked as granted. + * + * @param[in] root An object_tree structure that we want to modify + * + * @param[in] access_mask A bitfield of access right that we want to mark as + * granted in the whole tree. + */ void object_tree_modify_access(struct object_tree *root, uint32_t access_mask) { -- 1.7.11.7 From e5915eedfd5a13ac3a687cea8800bd2c3bf4f689 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sat, 13 Oct 2012 15:02:57 -0700 Subject: [PATCH 42/44] security: Add documentation Names seems to be a bit cryptic and misleading (at least for me). So documenting them should remove at least partially this problem. Reviewed-by: Andrew Bartlett (cherry picked from commit 7822952a11707ff8aaa415adef62082c158c2398) --- libcli/security/security.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libcli/security/security.h b/libcli/security/security.h index 659d341..6e4b172 100644 --- a/libcli/security/security.h +++ b/libcli/security/security.h @@ -89,6 +89,15 @@ #define SHARE_ALL_ACCESS FILE_GENERIC_ALL #define SHARE_READ_ONLY (FILE_GENERIC_READ|FILE_EXECUTE) +/** + * Remaining access is a bit mask of remaining access rights (bits) that have + * to be granted in order to fulfill the requested access. + * + * The GUID is optional, if specified it restricts this object tree and its + * childs to object/attributes that inherits from this GUID. + * For DS access an object inherits from a GUID if one of its class has this GUID + * in the schemaIDGUID attribute. + */ struct object_tree { uint32_t remaining_access; struct GUID guid; -- 1.7.11.7 From e4e0f2d5b173308807f6e03a9fba4eb268419306 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 14 Oct 2012 01:04:51 -0700 Subject: [PATCH 43/44] drsuapi: Add documentation Reviewed-by: Andrew Bartlett (cherry picked from commit 65396adaad18821568f727a223c38c36a2b16291) --- source4/rpc_server/drsuapi/updaterefs.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/source4/rpc_server/drsuapi/updaterefs.c b/source4/rpc_server/drsuapi/updaterefs.c index 2d62718..1b52cba 100644 --- a/source4/rpc_server/drsuapi/updaterefs.c +++ b/source4/rpc_server/drsuapi/updaterefs.c @@ -120,9 +120,24 @@ static WERROR uref_del_dest(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, return WERR_OK; } -/* - drsuapi_DsReplicaUpdateRefs - a non RPC version callable from getncchanges -*/ +/** + * @brief Update the references for the given NC and the destination DSA object + * + * This function is callable from non RPC functions (ie. getncchanges), it + * will validate the request to update reference and then will add/del a repsTo + * to the specified server referenced by its DSA GUID in the request. + * + * @param[in] b_state A bind_state object + * + * @param[in] mem_ctx A talloc context for memory allocation + * + * @param[in] req A drsuapi_DsReplicaUpdateRefsRequest1 + * object which NC, which server and which + * action (add/delete) should be performed + * + * @return WERR_OK is success, different error + * otherwise. + */ WERROR drsuapi_UpdateRefs(struct drsuapi_bind_state *b_state, TALLOC_CTX *mem_ctx, struct drsuapi_DsReplicaUpdateRefsRequest1 *req) { -- 1.7.11.7 From a8bffa7cbdcc1cb3f6c312df39c45ff29462a3e6 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 14 Oct 2012 01:01:08 -0700 Subject: [PATCH 44/44] libcli-acl: add documentation Reviewed-by: Andrew Bartlett (cherry picked from commit b1e231384a9245a191ef5e004544d7cafe17e036) --- libcli/security/access_check.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c index f0a7b66..936ffca 100644 --- a/libcli/security/access_check.c +++ b/libcli/security/access_check.c @@ -374,7 +374,25 @@ static const struct GUID *get_ace_object_type(struct security_ace *ace) return NULL; } -/* modified access check for the purposes of DS security +/** + * @brief Perform directoryservice (DS) related access checks for a given user + * + * Perform DS access checks for the user represented by its security_token, on + * the provided security descriptor. If an tree associating GUID and access + * required is provided then object access (OA) are checked as well. * + * @param[in] sd The security descritor against which the required + * access are requested + * + * @param[in] token The security_token associated with the user to + * test + * + * @param[in] access_desired A bitfield of rights that must be granted for the + * given user in the specified SD. + * + * If one + * of the entry in the tree grants all the requested rights for the given GUID + * FIXME + * tree can be null if not null it's the * Lots of code duplication, it will ve united in just one * function eventually */ -- 1.7.11.7