The branch, master has been updated
via e0ab14f s4:dsdb/acl_read: make sure confidential attributes require
CONTROL_ACCESS (bug #8620)
via 21dfaef s4:dsdb/acl_read: fix whitespace formatting errors
via f6fa724 s4:dsdb/acl: only give administrators access to attributes
marked as confidential (bug #8620)
via ed8b275 s4:dsdb/acl: reorganize the logic flow in the password
filtering checks
via 54ad5c7 s4:dsdb/acl: fix search filter cleanup for password
attributes
via 94649e4 selftest: Avoid test cross-contamination in
samba.tests.posixacl
from 1d81e52 selftest: Add tests for expected behaviour on directories
as well as files
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit e0ab14f52a52c8317473b4c4cd3cf50265e1f9e4
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Nov 9 17:23:53 2012 +0100
s4:dsdb/acl_read: make sure confidential attributes require CONTROL_ACCESS
(bug #8620)
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
Autobuild-User(master): Andrew Bartlett <abart...@samba.org>
Autobuild-Date(master): Mon Nov 12 01:25:21 CET 2012 on sn-devel-104
commit 21dfaefda0e22f7ddaac62bfd8b32e6fb9fc253d
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Nov 9 17:22:44 2012 +0100
s4:dsdb/acl_read: fix whitespace formatting errors
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit f6fa7243f81891cb7703264da526fd873a9745e4
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Nov 9 17:05:44 2012 +0100
s4:dsdb/acl: only give administrators access to attributes marked as
confidential (bug #8620)
The full fix will to implement and use the code of the read_acl module,
but this is better than nothing for now.
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit ed8b27516b212b59167bb932de949a7b54dc44cb
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Nov 9 11:23:47 2012 +0100
s4:dsdb/acl: reorganize the logic flow in the password filtering checks
This avoids some nesting levels and does early returns.
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 54ad5c70e3cc731c872913841cbcd2ef29ec0e54
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Nov 9 11:25:21 2012 +0100
s4:dsdb/acl: fix search filter cleanup for password attributes
We need to this when we're *not* system.
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 94649e46b4dec528ab7e750d06a65ada3d978342
Author: Andrew Bartlett <abart...@samba.org>
Date: Mon Nov 12 07:53:40 2012 +1100
selftest: Avoid test cross-contamination in samba.tests.posixacl
This creates a new xattr.tdb per unit test, which avoids once and for all
the issue of dev/inode reuse.
For test_setposixacl_dir_getntacl_smbd the file ownership also set
specifically.
Andrew Bartlett
Reviewed-by: Jelmer Vernooij <jel...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
source4/dsdb/samdb/ldb_modules/acl.c | 231 +++++++++++++++-----
source4/dsdb/samdb/ldb_modules/acl_read.c | 256 +++++++++++-----------
source4/scripting/python/samba/tests/posixacl.py | 140 +++++-------
3 files changed, 369 insertions(+), 258 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source4/dsdb/samdb/ldb_modules/acl.c
b/source4/dsdb/samdb/ldb_modules/acl.c
index 843d17e..1a41ee2 100644
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -51,12 +51,19 @@ struct extended_access_check_attribute {
struct acl_private {
bool acl_perform;
const char **password_attrs;
+ void *cached_schema_ptr;
+ uint64_t cached_schema_metadata_usn;
+ uint64_t cached_schema_loaded_usn;
+ const char **confidential_attrs;
};
struct acl_context {
struct ldb_module *module;
struct ldb_request *req;
bool am_system;
+ bool am_administrator;
+ bool modify_search;
+ bool constructed_attrs;
bool allowedAttributes;
bool allowedAttributesEffective;
bool allowedChildClasses;
@@ -88,12 +95,11 @@ static int acl_module_init(struct ldb_module *module)
return ldb_operr(ldb);
}
- data = talloc(module, struct acl_private);
+ data = talloc_zero(module, struct acl_private);
if (data == NULL) {
return ldb_oom(ldb);
}
- data->password_attrs = NULL;
data->acl_perform = lpcfg_parm_bool(ldb_get_opaque(ldb, "loadparm"),
NULL, "acl", "perform", false);
ldb_module_set_private(module, data);
@@ -1376,6 +1382,55 @@ static int acl_rename(struct ldb_module *module, struct
ldb_request *req)
return ldb_next_request(module, req);
}
+static int acl_search_update_confidential_attrs(struct acl_context *ac,
+ struct acl_private *data)
+{
+ struct dsdb_attribute *a;
+ uint32_t n = 0;
+
+ if ((ac->schema == data->cached_schema_ptr) &&
+ (ac->schema->loaded_usn == data->cached_schema_loaded_usn) &&
+ (ac->schema->metadata_usn == data->cached_schema_metadata_usn))
+ {
+ return LDB_SUCCESS;
+ }
+
+ data->cached_schema_ptr = NULL;
+ data->cached_schema_loaded_usn = 0;
+ data->cached_schema_metadata_usn = 0;
+ TALLOC_FREE(data->confidential_attrs);
+
+ if (ac->schema == NULL) {
+ return LDB_SUCCESS;
+ }
+
+ for (a = ac->schema->attributes; a; a = a->next) {
+ const char **attrs = data->confidential_attrs;
+
+ if (!(a->searchFlags & SEARCH_FLAG_CONFIDENTIAL)) {
+ continue;
+ }
+
+ attrs = talloc_realloc(data, attrs, const char *, n + 2);
+ if (attrs == NULL) {
+ TALLOC_FREE(data->confidential_attrs);
+ return ldb_module_oom(ac->module);
+ }
+
+ attrs[n] = a->lDAPDisplayName;
+ attrs[n+1] = NULL;
+ n++;
+
+ data->confidential_attrs = attrs;
+ }
+
+ data->cached_schema_ptr = ac->schema;
+ data->cached_schema_loaded_usn = ac->schema->loaded_usn;
+ data->cached_schema_metadata_usn = ac->schema->metadata_usn;
+
+ return LDB_SUCCESS;
+}
+
static int acl_search_callback(struct ldb_request *req, struct ldb_reply *ares)
{
struct acl_context *ac;
@@ -1403,11 +1458,7 @@ static int acl_search_callback(struct ldb_request *req,
struct ldb_reply *ares)
switch (ares->type) {
case LDB_REPLY_ENTRY:
- if (ac->allowedAttributes
- || ac->allowedChildClasses
- || ac->allowedChildClassesEffective
- || ac->allowedAttributesEffective
- || ac->sDRightsEffective) {
+ if (ac->constructed_attrs) {
ret = dsdb_module_search_dn(ac->module, ac, &acl_res,
ares->message->dn,
acl_attrs,
DSDB_FLAG_NEXT_MODULE |
@@ -1415,46 +1466,85 @@ static int acl_search_callback(struct ldb_request *req,
struct ldb_reply *ares)
if (ret != LDB_SUCCESS) {
return ldb_module_done(ac->req, NULL, NULL,
ret);
}
- if (ac->allowedAttributes ||
ac->allowedAttributesEffective) {
- ret = acl_allowedAttributes(ac->module,
ac->schema, acl_res->msgs[0], ares->message, ac);
- if (ret != LDB_SUCCESS) {
- return ldb_module_done(ac->req, NULL,
NULL, ret);
- }
+ }
+
+ if (ac->allowedAttributes || ac->allowedAttributesEffective) {
+ ret = acl_allowedAttributes(ac->module, ac->schema,
+ acl_res->msgs[0],
+ ares->message, ac);
+ if (ret != LDB_SUCCESS) {
+ return ldb_module_done(ac->req, NULL, NULL,
ret);
}
- if (ac->allowedChildClasses) {
- ret = acl_childClasses(ac->module, ac->schema,
acl_res->msgs[0],
- ares->message,
"allowedChildClasses");
- if (ret != LDB_SUCCESS) {
- return ldb_module_done(ac->req, NULL,
NULL, ret);
- }
+ }
+
+ if (ac->allowedChildClasses) {
+ ret = acl_childClasses(ac->module, ac->schema,
+ acl_res->msgs[0],
+ ares->message,
+ "allowedChildClasses");
+ if (ret != LDB_SUCCESS) {
+ return ldb_module_done(ac->req, NULL, NULL,
ret);
}
- if (ac->allowedChildClassesEffective) {
- ret = acl_childClassesEffective(ac->module,
ac->schema,
-
acl_res->msgs[0], ares->message, ac);
- if (ret != LDB_SUCCESS) {
- return ldb_module_done(ac->req, NULL,
NULL, ret);
- }
+ }
+
+ if (ac->allowedChildClassesEffective) {
+ ret = acl_childClassesEffective(ac->module, ac->schema,
+ acl_res->msgs[0],
+ ares->message, ac);
+ if (ret != LDB_SUCCESS) {
+ return ldb_module_done(ac->req, NULL, NULL,
ret);
}
- if (ac->sDRightsEffective) {
- ret = acl_sDRightsEffective(ac->module,
- acl_res->msgs[0],
ares->message, ac);
- if (ret != LDB_SUCCESS) {
- return ldb_module_done(ac->req, NULL,
NULL, ret);
- }
+ }
+
+ if (ac->sDRightsEffective) {
+ ret = acl_sDRightsEffective(ac->module,
+ acl_res->msgs[0],
+ ares->message, ac);
+ if (ret != LDB_SUCCESS) {
+ return ldb_module_done(ac->req, NULL, NULL,
ret);
}
}
- if (data && data->password_attrs) {
- if (!ac->am_system) {
- for (i = 0; data->password_attrs[i]; i++) {
- if ((!ac->userPassword) &&
-
(ldb_attr_cmp(data->password_attrs[i],
- "userPassword") == 0))
- continue;
- ldb_msg_remove_attr(ares->message,
data->password_attrs[i]);
+ if (data == NULL) {
+ return ldb_module_send_entry(ac->req, ares->message,
+ ares->controls);
+ }
+
+ if (ac->am_system) {
+ return ldb_module_send_entry(ac->req, ares->message,
+ ares->controls);
+ }
+
+ if (data->password_attrs != NULL) {
+ for (i = 0; data->password_attrs[i]; i++) {
+ if ((!ac->userPassword) &&
+ (ldb_attr_cmp(data->password_attrs[i],
+ "userPassword") == 0))
+ {
+ continue;
}
+
+ ldb_msg_remove_attr(ares->message,
data->password_attrs[i]);
+ }
+ }
+
+ if (ac->am_administrator) {
+ return ldb_module_send_entry(ac->req, ares->message,
+ ares->controls);
+ }
+
+ ret = acl_search_update_confidential_attrs(ac, data);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ if (data->confidential_attrs != NULL) {
+ for (i = 0; data->confidential_attrs[i]; i++) {
+ ldb_msg_remove_attr(ares->message,
+
data->confidential_attrs[i]);
}
}
+
return ldb_module_send_entry(ac->req, ares->message,
ares->controls);
case LDB_REPLY_REFERRAL:
@@ -1472,6 +1562,7 @@ static int acl_search(struct ldb_module *module, struct
ldb_request *req)
{
struct ldb_context *ldb;
struct acl_context *ac;
+ struct ldb_parse_tree *down_tree;
struct ldb_request *down_req;
struct acl_private *data;
int ret;
@@ -1488,6 +1579,9 @@ static int acl_search(struct ldb_module *module, struct
ldb_request *req)
ac->module = module;
ac->req = req;
ac->am_system = dsdb_module_am_system(module);
+ ac->am_administrator = dsdb_module_am_administrator(module);
+ ac->constructed_attrs = false;
+ ac->modify_search = true;
ac->allowedAttributes = ldb_attr_in_list(req->op.search.attrs,
"allowedAttributes");
ac->allowedAttributesEffective = ldb_attr_in_list(req->op.search.attrs,
"allowedAttributesEffective");
ac->allowedChildClasses = ldb_attr_in_list(req->op.search.attrs,
"allowedChildClasses");
@@ -1496,30 +1590,61 @@ static int acl_search(struct ldb_module *module, struct
ldb_request *req)
ac->userPassword = dsdb_user_password_support(module, ac, req);
ac->schema = dsdb_get_schema(ldb, ac);
- /* replace any attributes in the parse tree that are private,
- so we don't allow a search for 'userPassword=penguin',
- just as we would not allow that attribute to be returned */
+ ac->constructed_attrs |= ac->allowedAttributes;
+ ac->constructed_attrs |= ac->allowedChildClasses;
+ ac->constructed_attrs |= ac->allowedChildClassesEffective;
+ ac->constructed_attrs |= ac->allowedAttributesEffective;
+ ac->constructed_attrs |= ac->sDRightsEffective;
+
+ if (data == NULL) {
+ ac->modify_search = false;
+ }
if (ac->am_system) {
- /* FIXME: We should copy the tree and keep the original
unmodified. */
- /* remove password attributes */
- if (data && data->password_attrs) {
- for (i = 0; data->password_attrs[i]; i++) {
- if ((!ac->userPassword) &&
- (ldb_attr_cmp(data->password_attrs[i],
- "userPassword") == 0))
- continue;
+ ac->modify_search = false;
+ }
+
+ if (!ac->constructed_attrs && !ac->modify_search) {
+ return ldb_next_request(module, req);
+ }
+
+ ret = acl_search_update_confidential_attrs(ac, data);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ down_tree = ldb_parse_tree_copy_shallow(ac, req->op.search.tree);
+ if (down_tree == NULL) {
+ return ldb_oom(ldb);
+ }
- ldb_parse_tree_attr_replace(req->op.search.tree,
-
data->password_attrs[i],
-
"kludgeACLredactedattribute");
+ if (!ac->am_system && data->password_attrs) {
+ for (i = 0; data->password_attrs[i]; i++) {
+ if ((!ac->userPassword) &&
+ (ldb_attr_cmp(data->password_attrs[i],
+ "userPassword") == 0))
+ {
+ continue;
}
+
+ ldb_parse_tree_attr_replace(down_tree,
+ data->password_attrs[i],
+
"kludgeACLredactedattribute");
}
}
+
+ if (!ac->am_system && !ac->am_administrator &&
data->confidential_attrs) {
+ for (i = 0; data->confidential_attrs[i]; i++) {
+ ldb_parse_tree_attr_replace(down_tree,
+ data->confidential_attrs[i],
+
"kludgeACLredactedattribute");
+ }
+ }
+
ret = ldb_build_search_req_ex(&down_req,
ldb, ac,
req->op.search.base,
req->op.search.scope,
- req->op.search.tree,
+ down_tree,
req->op.search.attrs,
req->controls,
ac, acl_search_callback,
diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c
b/source4/dsdb/samdb/ldb_modules/acl_read.c
index 35a840e..e2a2d4c 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -55,7 +55,7 @@ struct aclread_private {
};
static void aclread_mark_inaccesslible(struct ldb_message_element *el) {
- el->flags |= LDB_FLAG_INTERNAL_INACCESSIBLE_ATTRIBUTE;
+ el->flags |= LDB_FLAG_INTERNAL_INACCESSIBLE_ATTRIBUTE;
}
static bool aclread_is_inaccessible(struct ldb_message_element *el) {
@@ -64,44 +64,45 @@ static bool aclread_is_inaccessible(struct
ldb_message_element *el) {
static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
{
- struct ldb_context *ldb;
- struct aclread_context *ac;
- struct ldb_message *ret_msg;
- struct ldb_message *msg;
- int ret, num_of_attrs = 0;
- unsigned int i, k = 0;
- struct security_descriptor *sd;
- struct dom_sid *sid = NULL;
- TALLOC_CTX *tmp_ctx;
- uint32_t instanceType;
+ struct ldb_context *ldb;
+ struct aclread_context *ac;
+ struct ldb_message *ret_msg;
+ struct ldb_message *msg;
+ int ret, num_of_attrs = 0;
+ unsigned int i, k = 0;
+ struct security_descriptor *sd;
+ struct dom_sid *sid = NULL;
+ TALLOC_CTX *tmp_ctx;
+ uint32_t instanceType;
- ac = talloc_get_type(req->context, struct aclread_context);
- ldb = ldb_module_get_ctx(ac->module);
- if (!ares) {
- return ldb_module_done(ac->req, NULL, NULL,
LDB_ERR_OPERATIONS_ERROR );
- }
- if (ares->error != LDB_SUCCESS) {
- return ldb_module_done(ac->req, ares->controls,
- ares->response, ares->error);
- }
- tmp_ctx = talloc_new(ac);
- switch (ares->type) {
- case LDB_REPLY_ENTRY:
- msg = ares->message;
- ret = dsdb_get_sd_from_ldb_message(ldb, tmp_ctx, msg, &sd);
- if (ret != LDB_SUCCESS || sd == NULL ) {
- DEBUG(10, ("acl_read: cannot get descriptor\n"));
- 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,
+ ac = talloc_get_type(req->context, struct aclread_context);
+ ldb = ldb_module_get_ctx(ac->module);
+ if (!ares) {
+ return ldb_module_done(ac->req, NULL, NULL,
LDB_ERR_OPERATIONS_ERROR );
+ }
+ if (ares->error != LDB_SUCCESS) {
+ return ldb_module_done(ac->req, ares->controls,
+ ares->response, ares->error);
+ }
+ tmp_ctx = talloc_new(ac);
+ switch (ares->type) {
+ case LDB_REPLY_ENTRY:
+ msg = ares->message;
+ ret = dsdb_get_sd_from_ldb_message(ldb, tmp_ctx, msg, &sd);
+ if (ret != LDB_SUCCESS || sd == NULL ) {
+ DEBUG(10, ("acl_read: cannot get descriptor\n"));
+ 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,
"instanceType", 0);
- if (!ldb_dn_is_null(msg->dn) && !(instanceType &
INSTANCE_TYPE_IS_NC_HEAD))
- {
+ if (!ldb_dn_is_null(msg->dn) && !(instanceType &
INSTANCE_TYPE_IS_NC_HEAD))
+ {
/* the object has a parent, so we have to check for
visibility */
struct ldb_dn *parent_dn = ldb_dn_get_parent(tmp_ctx,
msg->dn);
+
ret = dsdb_module_check_access_on_dn(ac->module,
tmp_ctx,
parent_dn,
@@ -113,51 +114,56 @@ static int aclread_callback(struct ldb_request *req,
struct ldb_reply *ares)
} else if (ret != LDB_SUCCESS) {
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;
- uint32_t access_mask;
- attr = dsdb_attribute_by_lDAPDisplayName(ac->schema,
-
msg->elements[i].name);
- if (!attr) {
- DEBUG(2, ("acl_read: cannot find attribute %s
in schema\n",
+ }
+ /* 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;
+ uint32_t access_mask;
+ attr = dsdb_attribute_by_lDAPDisplayName(ac->schema,
+
msg->elements[i].name);
+ if (!attr) {
+ DEBUG(2, ("acl_read: cannot find attribute %s
in schema\n",
msg->elements[i].name));
- ret = LDB_ERR_OPERATIONS_ERROR;
- goto fail;
- }
- is_sd = ldb_attr_cmp("nTSecurityDescriptor",
+ ret = LDB_ERR_OPERATIONS_ERROR;
+ goto fail;
+ }
+ is_sd = ldb_attr_cmp("nTSecurityDescriptor",
msg->elements[i].name) == 0;
- is_objectsid = ldb_attr_cmp("objectSid",
- msg->elements[i].name) ==
0;
- is_instancetype = ldb_attr_cmp("instanceType",
- msg->elements[i].name)
== 0;
- /* these attributes were added to perform access
checks and must be removed */
- if (is_objectsid && ac->object_sid) {
- aclread_mark_inaccesslible(&msg->elements[i]);
- continue;
- }
- if (is_instancetype && ac->instance_type) {
- aclread_mark_inaccesslible(&msg->elements[i]);
- continue;
- }
- if (is_sd && ac->sd) {
- aclread_mark_inaccesslible(&msg->elements[i]);
- continue;
- }
- /* nTSecurityDescriptor is a special case */
- if (is_sd) {
- access_mask =
SEC_FLAG_SYSTEM_SECURITY|SEC_STD_READ_CONTROL;
- } else {
- access_mask = SEC_ADS_READ_PROP;
- }
- ret = acl_check_access_on_attribute(ac->module,
- tmp_ctx,
- sd,
- sid,
- access_mask,
- attr);
+ is_objectsid = ldb_attr_cmp("objectSid",
+ msg->elements[i].name) == 0;
+ is_instancetype = ldb_attr_cmp("instanceType",
+ msg->elements[i].name)
== 0;
+ /* these attributes were added to perform access checks
and must be removed */
+ if (is_objectsid && ac->object_sid) {
+ aclread_mark_inaccesslible(&msg->elements[i]);
+ continue;
+ }
+ if (is_instancetype && ac->instance_type) {
+ aclread_mark_inaccesslible(&msg->elements[i]);
+ continue;
+ }
+ if (is_sd && ac->sd) {
+ aclread_mark_inaccesslible(&msg->elements[i]);
+ continue;
+ }
+ /* nTSecurityDescriptor is a special case */
+ if (is_sd) {
+ access_mask =
SEC_FLAG_SYSTEM_SECURITY|SEC_STD_READ_CONTROL;
+ } else {
+ access_mask = SEC_ADS_READ_PROP;
+ }
+
+ if (attr->searchFlags & SEARCH_FLAG_CONFIDENTIAL) {
--
Samba Shared Repository
