sin/samba-mirror
1
0
Fork 0
mirror of https://github.com/samba-team/samba.git synced 2025-01-05 09:18:06 +03:00
Code

CVE-2023-0614 ldb: Prevent disclosure of confidential attributes

Browse Source 
Add a hook, acl_redact_msg_for_filter(), in the aclread module, that
marks inaccessible any message elements used by an LDAP search filter
that the user has no right to access. Make the various ldb_match_*()
functions check whether message elements are accessible, and refuse to
match any that are not. Remaining message elements, not mentioned in the
search filter, are checked in aclread_callback(), and any inaccessible
elements are removed at this point.

Certain attributes, namely objectClass, distinguishedName, name, and
objectGUID, are always present, and hence the presence of said
attributes is always allowed to be checked in a search filter. This
corresponds with the behaviour of Windows.

Further, we unconditionally allow the attributes isDeleted and
isRecycled in a check for presence or equality. Windows is not known to
make this special exception, but it seems mostly harmless, and should
mitigate the performance impact on searches made by the show_deleted
module.

As a result of all these changes, our behaviour regarding confidential
attributes happens to match Windows more closely. For the test in
confidential_attr.py, we can now model our attribute handling with
DC_MODE_RETURN_ALL, which corresponds to the behaviour exhibited by
Windows.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270

Pair-Programmed-With: Andrew Bartlett <abartlet@samba.org>

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

[abartlet@samba.org adapted due to Samba 4.17 and lower
not having the patches for CVE-2020-25720]
This commit is contained in:
Joseph Sutton 2023-03-03 17:34:29 +13:00 committed by Jule Anger
parent 2e3ed6cfd2
commit c1921f5ae0
12 changed files with 672 additions and 455 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
lib/ldb-samba/ldb_matching_rules.c
View File

@ -87,6 +87,11 @@ static int ldb_eval_transitive_filter_helper(TALLOC_CTX *mem_ctx,
return LDB_SUCCESS;
}
if (ldb_msg_element_is_inaccessible(el)) {
*matched = false;
return LDB_SUCCESS;
}
/*
* If the value to match is present in the attribute values of the
* current entry being visited, set matched to true and return OK
@ -370,6 +375,11 @@ static int dsdb_match_for_dns_to_tombstone_time(struct ldb_context *ldb,
return LDB_SUCCESS;
}
if (ldb_msg_element_is_inaccessible(el)) {
*matched = false;
return LDB_SUCCESS;
}
session_info = talloc_get_type(ldb_get_opaque(ldb, "sessionInfo"),
struct auth_session_info);
if (session_info == NULL) {
@ -489,6 +499,11 @@ static int dsdb_match_for_expunge(struct ldb_context *ldb,
return LDB_SUCCESS;
}
if (ldb_msg_element_is_inaccessible(el)) {
*matched = false;
return LDB_SUCCESS;
}
session_info
= talloc_get_type(ldb_get_opaque(ldb, DSDB_SESSION_INFO),
struct auth_session_info);

37
lib/ldb/common/ldb_match.c
View File

@ -99,6 +99,11 @@ static int ldb_match_present(struct ldb_context *ldb,
return LDB_SUCCESS;
}
if (ldb_msg_element_is_inaccessible(el)) {
*matched = false;
return LDB_SUCCESS;
}
a = ldb_schema_attribute_by_name(ldb, el->name);
if (!a) {
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
@ -140,6 +145,11 @@ static int ldb_match_comparison(struct ldb_context *ldb,
return LDB_SUCCESS;
}
if (ldb_msg_element_is_inaccessible(el)) {
*matched = false;
return LDB_SUCCESS;
}
a = ldb_schema_attribute_by_name(ldb, el->name);
if (!a) {
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
@ -210,6 +220,11 @@ static int ldb_match_equality(struct ldb_context *ldb,
return LDB_SUCCESS;
}
if (ldb_msg_element_is_inaccessible(el)) {
*matched = false;
return LDB_SUCCESS;
}
a = ldb_schema_attribute_by_name(ldb, el->name);
if (a == NULL) {
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
@ -410,6 +425,11 @@ static int ldb_match_substring(struct ldb_context *ldb,
return LDB_SUCCESS;
}
if (ldb_msg_element_is_inaccessible(el)) {
*matched = false;
return LDB_SUCCESS;
}
for (i = 0; i < el->num_values; i++) {
int ret;
ret = ldb_wildcard_compare(ldb, tree, el->values[i], matched);
@ -483,6 +503,11 @@ static int ldb_match_bitmask(struct ldb_context *ldb,
return LDB_SUCCESS;
}
if (ldb_msg_element_is_inaccessible(el)) {
*matched = false;
return LDB_SUCCESS;
}
for (i=0;i<el->num_values;i++) {
int ret;
struct ldb_val *v = &el->values[i];
@ -781,3 +806,15 @@ int ldb_register_extended_match_rule(struct ldb_context *ldb,
return LDB_SUCCESS;
}
int ldb_register_redact_callback(struct ldb_context *ldb,
ldb_redact_fn redact_fn,
struct ldb_module *module)
{
if (ldb->redact.callback != NULL) {
return LDB_ERR_ENTRY_ALREADY_EXISTS;
}
ldb->redact.callback = redact_fn;
ldb->redact.module = module;
return LDB_SUCCESS;
}

11
lib/ldb/include/ldb_module.h
View File

@ -102,6 +102,12 @@ struct ldb_module;
*/
#define LDB_FLAG_INTERNAL_SHARED_VALUES 0x200
/*
* this attribute has been access checked. We know the user has the right to
* view it. Used internally in Samba aclread module.
*/
#define LDB_FLAG_INTERNAL_ACCESS_CHECKED 0x400
/* an extended match rule that always fails to match */
#define SAMBA_LDAP_MATCH_ALWAYS_FALSE "1.3.6.1.4.1.7165.4.5.1"
@ -520,6 +526,11 @@ void ldb_msg_element_mark_inaccessible(struct ldb_message_element *el);
bool ldb_msg_element_is_inaccessible(const struct ldb_message_element *el);
void ldb_msg_remove_inaccessible(struct ldb_message *msg);
typedef int (*ldb_redact_fn)(struct ldb_module *, struct ldb_request *, struct ldb_message *);
int ldb_register_redact_callback(struct ldb_context *ldb,
ldb_redact_fn redact_fn,
struct ldb_module *module);
/*
* these pack/unpack functions are exposed in the library for use by
* ldb tools like ldbdump and for use in tests,

5
lib/ldb/include/ldb_private.h
View File

@ -119,6 +119,11 @@ struct ldb_context {
struct ldb_extended_match_entry *prev, *next;
} *extended_match_rules;
struct {
struct ldb_module *module;
ldb_redact_fn callback;
} redact;
/* custom utf8 functions */
struct ldb_utf8_fns utf8_fns;

8
lib/ldb/ldb_key_value/ldb_kv_index.c
View File

@ -2428,6 +2428,14 @@ static int ldb_kv_index_filter(struct ldb_kv_private *ldb_kv,
return LDB_ERR_OPERATIONS_ERROR;
}
if (ldb->redact.callback != NULL) {
ret = ldb->redact.callback(ldb->redact.module, ac->req, msg);
if (ret != LDB_SUCCESS) {
talloc_free(msg);
return ret;
}
}
/*
* We trust the index for LDB_SCOPE_ONELEVEL
* unless the index key has been truncated.

15
lib/ldb/ldb_key_value/ldb_kv_search.c
View File

@ -395,6 +395,14 @@ static int search_func(_UNUSED_ struct ldb_kv_private *ldb_kv,
}
}
if (ldb->redact.callback != NULL) {
ret = ldb->redact.callback(ldb->redact.module, ac->req, msg);
if (ret != LDB_SUCCESS) {
talloc_free(msg);
return ret;
}
}
/* see if it matches the given expression */
ret = ldb_match_msg_error(ldb, msg,
ac->tree, ac->base, ac->scope, &matched);
@ -530,6 +538,13 @@ static int ldb_kv_search_and_return_base(struct ldb_kv_private *ldb_kv,
return ret;
}
if (ldb->redact.callback != NULL) {
ret = ldb->redact.callback(ldb->redact.module, ctx->req, msg);
if (ret != LDB_SUCCESS) {
talloc_free(msg);
return ret;
}
}
/*
* We use this, not ldb_match_msg_error() as we know

1
selftest/knownfail.d/confidential-attr-timing
View File

@ -1 +0,0 @@
^samba4.ldap.confidential_attr.python\(ad_dc_slowtests\).__main__.ConfidentialAttrTestDirsync.test_timing_attack\(ad_dc_slowtests\)

183
source4/dsdb/samdb/ldb_modules/acl.c
View File

@ -46,11 +46,6 @@
#undef strcasecmp
#undef strncasecmp
struct extended_access_check_attribute {
const char *oa_name;
const uint32_t requires_rights;
};
struct acl_private {
bool acl_search;
const char **password_attrs;
@ -58,7 +53,6 @@ struct acl_private {
uint64_t cached_schema_metadata_usn;
uint64_t cached_schema_loaded_usn;
const char **confidential_attrs;
bool userPassword_support;
};
struct acl_context {
@ -66,15 +60,12 @@ struct acl_context {
struct ldb_request *req;
bool am_system;
bool am_administrator;
bool modify_search;
bool constructed_attrs;
bool allowedAttributes;
bool allowedAttributesEffective;
bool allowedChildClasses;
bool allowedChildClassesEffective;
bool sDRightsEffective;
bool userPassword;
const char * const *attrs;
struct dsdb_schema *schema;
};
@ -83,25 +74,9 @@ static int acl_module_init(struct ldb_module *module)
struct ldb_context *ldb;
struct acl_private *data;
int ret;
unsigned int i, n, j;
TALLOC_CTX *mem_ctx;
static const char * const attrs[] = { "passwordAttribute", NULL };
static const char * const secret_attrs[] = {
DSDB_SECRET_ATTRIBUTES
};
struct ldb_result *res;
struct ldb_message *msg;
struct ldb_message_element *password_attributes;
ldb = ldb_module_get_ctx(module);
ret = ldb_mod_register_control(module, LDB_CONTROL_SD_FLAGS_OID);
if (ret != LDB_SUCCESS) {
ldb_debug(ldb, LDB_DEBUG_ERROR,
"acl_module_init: Unable to register control with rootdse!\n");
return ldb_operr(ldb);
}
data = talloc_zero(module, struct acl_private);
if (data == NULL) {
return ldb_oom(ldb);
@ -111,91 +86,14 @@ static int acl_module_init(struct ldb_module *module)
NULL, "acl", "search", true);
ldb_module_set_private(module, data);
mem_ctx = talloc_new(module);
if (!mem_ctx) {
return ldb_oom(ldb);
}
ret = dsdb_module_search_dn(module, mem_ctx, &res,
ldb_dn_new(mem_ctx, ldb, "@KLUDGEACL"),
attrs,
DSDB_FLAG_NEXT_MODULE |
DSDB_FLAG_AS_SYSTEM,
NULL);
ret = ldb_mod_register_control(module, LDB_CONTROL_SD_FLAGS_OID);
if (ret != LDB_SUCCESS) {
goto done;
}
if (res->count == 0) {
goto done;
ldb_debug(ldb, LDB_DEBUG_ERROR,
"acl_module_init: Unable to register control with rootdse!\n");
return ldb_operr(ldb);
}
if (res->count > 1) {
talloc_free(mem_ctx);
return LDB_ERR_CONSTRAINT_VIOLATION;
}
msg = res->msgs[0];
password_attributes = ldb_msg_find_element(msg, "passwordAttribute");
if (!password_attributes) {
goto done;
}
data->password_attrs = talloc_array(data, const char *,
password_attributes->num_values +
ARRAY_SIZE(secret_attrs) + 1);
if (!data->password_attrs) {
talloc_free(mem_ctx);
return ldb_oom(ldb);
}
n = 0;
for (i=0; i < password_attributes->num_values; i++) {
data->password_attrs[n] = (const char *)password_attributes->values[i].data;
talloc_steal(data->password_attrs, password_attributes->values[i].data);
n++;
}
for (i=0; i < ARRAY_SIZE(secret_attrs); i++) {
bool found = false;
for (j=0; j < n; j++) {
if (strcasecmp(data->password_attrs[j], secret_attrs[i]) == 0) {
found = true;
break;
}
}
if (found) {
continue;
}
data->password_attrs[n] = talloc_strdup(data->password_attrs,
secret_attrs[i]);
if (data->password_attrs[n] == NULL) {
talloc_free(mem_ctx);
return ldb_oom(ldb);
}
n++;
}
data->password_attrs[n] = NULL;
done:
talloc_free(mem_ctx);
ret = ldb_next_init(module);
if (ret != LDB_SUCCESS) {
return ret;
}
/*
* Check this after the modules have be initialised so we
* can actually read the backend DB.
*/
data->userPassword_support
= dsdb_user_password_support(module,
module,
NULL);
return ret;
return ldb_next_init(module);
}
static int acl_allowedAttributes(struct ldb_module *module,
@ -2522,29 +2420,11 @@ static int acl_search_callback(struct ldb_request *req, struct ldb_reply *ares)
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,
@ -2569,11 +2449,12 @@ 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_parse_tree *down_tree = req->op.search.tree;
struct ldb_request *down_req;
struct acl_private *data;
int ret;
unsigned int i;
bool modify_search = true;
if (ldb_dn_is_special(req->op.search.base)) {
return ldb_next_request(module, req);
@ -2592,13 +2473,11 @@ static int acl_search(struct ldb_module *module, struct ldb_request *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");
ac->allowedChildClassesEffective = ldb_attr_in_list(req->op.search.attrs, "allowedChildClassesEffective");
ac->sDRightsEffective = ldb_attr_in_list(req->op.search.attrs, "sDRightsEffective");
ac->userPassword = true;
ac->schema = dsdb_get_schema(ldb, ac);
ac->constructed_attrs |= ac->allowedAttributes;
@ -2608,13 +2487,13 @@ static int acl_search(struct ldb_module *module, struct ldb_request *req)
ac->constructed_attrs |= ac->sDRightsEffective;
if (data == NULL) {
ac->modify_search = false;
modify_search = false;
}
if (ac->am_system) {
ac->modify_search = false;
modify_search = false;
}
if (!ac->constructed_attrs && !ac->modify_search) {
if (!ac->constructed_attrs && !modify_search) {
talloc_free(ac);
return ldb_next_request(module, req);
}
@ -2624,38 +2503,24 @@ static int acl_search(struct ldb_module *module, struct ldb_request *req)
return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR,
"acl_private data is missing");
}
ac->userPassword = data->userPassword_support;
ret = acl_search_update_confidential_attrs(ac, data);
if (ret != LDB_SUCCESS) {
return ret;
}
if (!ac->am_system && !ac->am_administrator) {
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);
}
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;
if (data->confidential_attrs != NULL) {
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(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");
for (i = 0; data->confidential_attrs[i]; i++) {
ldb_parse_tree_attr_replace(down_tree,
data->confidential_attrs[i],
"kludgeACLredactedattribute");
}
}
}

837
source4/dsdb/samdb/ldb_modules/acl_read.c
View File

File diff suppressed because it is too large Load Diff

2
source4/dsdb/samdb/samdb.h
View File

@ -232,6 +232,8 @@ struct dsdb_control_transaction_identifier {
*/
#define DSDB_CONTROL_FORCE_ALLOW_VALIDATED_DNS_HOSTNAME_SPN_WRITE_OID "1.3.6.1.4.1.7165.4.3.35"
#define DSDB_CONTROL_ACL_READ_OID "1.3.6.1.4.1.7165.4.3.37"
#define DSDB_EXTENDED_REPLICATED_OBJECTS_OID "1.3.6.1.4.1.7165.4.4.1"
struct dsdb_extended_replicated_object {
struct ldb_message *msg;

12
source4/dsdb/tests/python/confidential_attr.py
View File

@ -490,7 +490,7 @@ class ConfidentialAttrTest(ConfidentialAttrCommon):
self.make_attr_confidential()
self.assert_conf_attr_searches(has_rights_to=0)
dc_mode = self.guess_dc_mode()
dc_mode = DC_MODE_RETURN_ALL
self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
self.assert_attr_visible(expect_attr=False)
@ -503,7 +503,7 @@ class ConfidentialAttrTest(ConfidentialAttrCommon):
self.make_attr_confidential()
self.assert_conf_attr_searches(has_rights_to=0)
dc_mode = self.guess_dc_mode()
dc_mode = DC_MODE_RETURN_ALL
self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
self.assert_attr_visible(expect_attr=False)
@ -566,7 +566,7 @@ class ConfidentialAttrTest(ConfidentialAttrCommon):
self.make_attr_confidential()
self.assert_conf_attr_searches(has_rights_to=0)
dc_mode = self.guess_dc_mode()
dc_mode = DC_MODE_RETURN_ALL
self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
self.assert_attr_visible(expect_attr=False)
@ -741,7 +741,7 @@ class ConfidentialAttrTestDenyAcl(ConfidentialAttrCommon):
# the user shouldn't be able to see the attribute anymore
self.assert_conf_attr_searches(has_rights_to="deny-one")
dc_mode = self.guess_dc_mode()
dc_mode = DC_MODE_RETURN_ALL
self.assert_negative_searches(has_rights_to="deny-one",
dc_mode=dc_mode)
self.assert_attr_visible(expect_attr=False)
@ -917,7 +917,7 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
self.assert_conf_attr_searches(has_rights_to=0)
self.assert_attr_visible(expect_attr=False)
dc_mode = self.guess_dc_mode()
dc_mode = DC_MODE_RETURN_ALL
self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
# as a final sanity-check, make sure the admin can still see the attr
@ -1012,7 +1012,7 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
# check we can't see the objects now, even with using dirsync controls
self.assert_conf_attr_searches(has_rights_to=0)
self.assert_attr_visible(expect_attr=False)
dc_mode = self.guess_dc_mode()
dc_mode = DC_MODE_RETURN_ALL
self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
# now delete the users (except for the user whose LDB connection

1
source4/setup/schema_samba4.ldif
View File

@ -233,6 +233,7 @@
#Allocated: DSDB_CONTROL_TRANSACTION_IDENTIFIER_OID 1.3.6.1.4.1.7165.4.3.34
#Allocated: DSDB_CONTROL_FORCE_ALLOW_VALIDATED_DNS_HOSTNAME_SPN_WRITE_OID 1.3.6.1.4.1.7165.4.3.35
#Allocated: DSDB_CONTROL_CALCULATED_DEFAULT_SD_OID 1.3.6.1.4.1.7165.4.3.36
#Allocated: DSDB_CONTROL_ACL_READ_OID 1.3.6.1.4.1.7165.4.3.37
# Extended 1.3.6.1.4.1.7165.4.4.x