diff options
author | Karolin Seeger <kseeger@samba.org> | 2018-08-14 12:16:21 +0200 |
---|---|---|
committer | Karolin Seeger <kseeger@samba.org> | 2018-08-14 12:16:21 +0200 |
commit | 47081d9de81339d4e940c4747f6e2d735386e651 (patch) | |
tree | 05288b1ecac67b18545e7ad7fddaa743aaf618e7 | |
parent | 6f44ef8511490f9ad9f849c09aededf0f0b38dff (diff) | |
parent | 626c489c2c879aef8b82efe9f7e832cca0183f4d (diff) | |
download | samba-47081d9de81339d4e940c4747f6e2d735386e651.tar.gz |
Merge tag 'samba-4.8.4' into v4-8-test
samba: tag release samba-4.8.4
28 files changed, 2720 insertions, 94 deletions
diff --git a/WHATSNEW.txt b/WHATSNEW.txt index 5c2d922cd90..d09297284d2 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -1,4 +1,94 @@ ============================= + Release Notes for Samba 4.8.4 + August 14, 2018 + ============================= + + +This is a security release in order to address the following defects: + +o CVE-2018-1139 (Weak authentication protocol allowed.) +o CVE-2018-1140 (Denial of Service Attack on DNS and LDAP server.) +o CVE-2018-10858 (Insufficient input validation on client directory + listing in libsmbclient.) +o CVE-2018-10918 (Denial of Service Attack on AD DC DRSUAPI server.) +o CVE-2018-10919 (Confidential attribute disclosure from the AD LDAP + server.) + + +======= +Details +======= + +o CVE-2018-1139: + Vulnerability that allows authentication via NTLMv1 even if disabled. + +o CVE-2018-1140: + Missing null pointer checks may crash the Samba AD DC, both over + DNS and LDAP. + +o CVE-2018-10858: + A malicious server could return a directory entry that could corrupt + libsmbclient memory. + +o CVE-2018-10918: + Missing null pointer checks may crash the Samba AD DC, over the + authenticated DRSUAPI RPC service. + +o CVE-2018-10919: + Missing access control checks allow discovery of confidential attribute + values via authenticated LDAP search expressions. + + +Changes since 4.8.3: +-------------------- + +o Jeremy Allison <jra@samba.org> + * BUG 13453: CVE-2018-10858: libsmb: Harden smbc_readdir_internal() against + returns from malicious servers. + +o Andrew Bartlett <abartlet@samba.org> + * BUG 13374: CVE-2018-1140: ldbsearch '(distinguishedName=abc)' and DNS query + with escapes crashes, ldb: Release LDB 1.3.5 for CVE-2018-1140 + * BUG 13552: CVE-2018-10918: cracknames: Fix DoS (NULL pointer de-ref) when + not servicePrincipalName is set on a user. + +o Tim Beale <timbeale@catalyst.net.nz> + * BUG 13434: CVE-2018-10919: acl_read: Fix unauthorized attribute access via + searches. + +o Günther Deschner <gd@samba.org> + * BUG 13360: CVE-2018-1139 libcli/auth: Do not allow ntlmv1 over SMB1 when it + is disabled via "ntlm auth". + +o Andrej Gessel <Andrej.Gessel@janztec.com> + * BUG 13374: CVE-2018-1140 Add NULL check for ldb_dn_get_casefold() in + ltdb_index_dn_attr(). + + +####################################### +Reporting bugs & Development Discussion +####################################### + +Please discuss this release on the samba-technical mailing list or by +joining the #samba-technical IRC channel on irc.freenode.net. + +If you do report problems then please try to send high quality +feedback. If you don't provide vital information to help us track down +the problem then you will probably be ignored. All bug reports should +be filed under the "Samba 4.1 and newer" product in the project's Bugzilla +database (https://bugzilla.samba.org/). + + +====================================================================== +== Our Code, Our Bugs, Our Responsibility. +== The Samba Team +====================================================================== + + +Release notes for older releases follow: +---------------------------------------- + + ============================= Release Notes for Samba 4.8.3 June 26, 2018 ============================= @@ -84,8 +174,8 @@ database (https://bugzilla.samba.org/). ====================================================================== -Release notes for older releases follow: ----------------------------------------- +---------------------------------------------------------------------- + ============================= Release Notes for Samba 4.8.2 diff --git a/lib/ldb/ABI/ldb-1.3.5.sigs b/lib/ldb/ABI/ldb-1.3.5.sigs new file mode 100644 index 00000000000..a31b84ef4b5 --- /dev/null +++ b/lib/ldb/ABI/ldb-1.3.5.sigs @@ -0,0 +1,279 @@ +ldb_add: int (struct ldb_context *, const struct ldb_message *) +ldb_any_comparison: int (struct ldb_context *, void *, ldb_attr_handler_t, const struct ldb_val *, const struct ldb_val *) +ldb_asprintf_errstring: void (struct ldb_context *, const char *, ...) +ldb_attr_casefold: char *(TALLOC_CTX *, const char *) +ldb_attr_dn: int (const char *) +ldb_attr_in_list: int (const char * const *, const char *) +ldb_attr_list_copy: const char **(TALLOC_CTX *, const char * const *) +ldb_attr_list_copy_add: const char **(TALLOC_CTX *, const char * const *, const char *) +ldb_base64_decode: int (char *) +ldb_base64_encode: char *(TALLOC_CTX *, const char *, int) +ldb_binary_decode: struct ldb_val (TALLOC_CTX *, const char *) +ldb_binary_encode: char *(TALLOC_CTX *, struct ldb_val) +ldb_binary_encode_string: char *(TALLOC_CTX *, const char *) +ldb_build_add_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *) +ldb_build_del_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *) +ldb_build_extended_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const char *, void *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *) +ldb_build_mod_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *) +ldb_build_rename_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, struct ldb_dn *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *) +ldb_build_search_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, enum ldb_scope, const char *, const char * const *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *) +ldb_build_search_req_ex: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, enum ldb_scope, struct ldb_parse_tree *, const char * const *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *) +ldb_casefold: char *(struct ldb_context *, TALLOC_CTX *, const char *, size_t) +ldb_casefold_default: char *(void *, TALLOC_CTX *, const char *, size_t) +ldb_check_critical_controls: int (struct ldb_control **) +ldb_comparison_binary: int (struct ldb_context *, void *, const struct ldb_val *, const struct ldb_val *) +ldb_comparison_fold: int (struct ldb_context *, void *, const struct ldb_val *, const struct ldb_val *) +ldb_connect: int (struct ldb_context *, const char *, unsigned int, const char **) +ldb_control_to_string: char *(TALLOC_CTX *, const struct ldb_control *) +ldb_controls_except_specified: struct ldb_control **(struct ldb_control **, TALLOC_CTX *, struct ldb_control *) +ldb_debug: void (struct ldb_context *, enum ldb_debug_level, const char *, ...) +ldb_debug_add: void (struct ldb_context *, const char *, ...) +ldb_debug_end: void (struct ldb_context *, enum ldb_debug_level) +ldb_debug_set: void (struct ldb_context *, enum ldb_debug_level, const char *, ...) +ldb_delete: int (struct ldb_context *, struct ldb_dn *) +ldb_dn_add_base: bool (struct ldb_dn *, struct ldb_dn *) +ldb_dn_add_base_fmt: bool (struct ldb_dn *, const char *, ...) +ldb_dn_add_child: bool (struct ldb_dn *, struct ldb_dn *) +ldb_dn_add_child_fmt: bool (struct ldb_dn *, const char *, ...) +ldb_dn_alloc_casefold: char *(TALLOC_CTX *, struct ldb_dn *) +ldb_dn_alloc_linearized: char *(TALLOC_CTX *, struct ldb_dn *) +ldb_dn_canonical_ex_string: char *(TALLOC_CTX *, struct ldb_dn *) +ldb_dn_canonical_string: char *(TALLOC_CTX *, struct ldb_dn *) +ldb_dn_check_local: bool (struct ldb_module *, struct ldb_dn *) +ldb_dn_check_special: bool (struct ldb_dn *, const char *) +ldb_dn_compare: int (struct ldb_dn *, struct ldb_dn *) +ldb_dn_compare_base: int (struct ldb_dn *, struct ldb_dn *) +ldb_dn_copy: struct ldb_dn *(TALLOC_CTX *, struct ldb_dn *) +ldb_dn_escape_value: char *(TALLOC_CTX *, struct ldb_val) +ldb_dn_extended_add_syntax: int (struct ldb_context *, unsigned int, const struct ldb_dn_extended_syntax *) +ldb_dn_extended_filter: void (struct ldb_dn *, const char * const *) +ldb_dn_extended_syntax_by_name: const struct ldb_dn_extended_syntax *(struct ldb_context *, const char *) +ldb_dn_from_ldb_val: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const struct ldb_val *) +ldb_dn_get_casefold: const char *(struct ldb_dn *) +ldb_dn_get_comp_num: int (struct ldb_dn *) +ldb_dn_get_component_name: const char *(struct ldb_dn *, unsigned int) +ldb_dn_get_component_val: const struct ldb_val *(struct ldb_dn *, unsigned int) +ldb_dn_get_extended_comp_num: int (struct ldb_dn *) +ldb_dn_get_extended_component: const struct ldb_val *(struct ldb_dn *, const char *) +ldb_dn_get_extended_linearized: char *(TALLOC_CTX *, struct ldb_dn *, int) +ldb_dn_get_ldb_context: struct ldb_context *(struct ldb_dn *) +ldb_dn_get_linearized: const char *(struct ldb_dn *) +ldb_dn_get_parent: struct ldb_dn *(TALLOC_CTX *, struct ldb_dn *) +ldb_dn_get_rdn_name: const char *(struct ldb_dn *) +ldb_dn_get_rdn_val: const struct ldb_val *(struct ldb_dn *) +ldb_dn_has_extended: bool (struct ldb_dn *) +ldb_dn_is_null: bool (struct ldb_dn *) +ldb_dn_is_special: bool (struct ldb_dn *) +ldb_dn_is_valid: bool (struct ldb_dn *) +ldb_dn_map_local: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *) +ldb_dn_map_rebase_remote: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *) +ldb_dn_map_remote: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *) +ldb_dn_minimise: bool (struct ldb_dn *) +ldb_dn_new: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const char *) +ldb_dn_new_fmt: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const char *, ...) +ldb_dn_remove_base_components: bool (struct ldb_dn *, unsigned int) +ldb_dn_remove_child_components: bool (struct ldb_dn *, unsigned int) +ldb_dn_remove_extended_components: void (struct ldb_dn *) +ldb_dn_replace_components: bool (struct ldb_dn *, struct ldb_dn *) +ldb_dn_set_component: int (struct ldb_dn *, int, const char *, const struct ldb_val) +ldb_dn_set_extended_component: int (struct ldb_dn *, const char *, const struct ldb_val *) +ldb_dn_update_components: int (struct ldb_dn *, const struct ldb_dn *) +ldb_dn_validate: bool (struct ldb_dn *) +ldb_dump_results: void (struct ldb_context *, struct ldb_result *, FILE *) +ldb_error_at: int (struct ldb_context *, int, const char *, const char *, int) +ldb_errstring: const char *(struct ldb_context *) +ldb_extended: int (struct ldb_context *, const char *, void *, struct ldb_result **) +ldb_extended_default_callback: int (struct ldb_request *, struct ldb_reply *) +ldb_filter_from_tree: char *(TALLOC_CTX *, const struct ldb_parse_tree *) +ldb_get_config_basedn: struct ldb_dn *(struct ldb_context *) +ldb_get_create_perms: unsigned int (struct ldb_context *) +ldb_get_default_basedn: struct ldb_dn *(struct ldb_context *) +ldb_get_event_context: struct tevent_context *(struct ldb_context *) +ldb_get_flags: unsigned int (struct ldb_context *) +ldb_get_opaque: void *(struct ldb_context *, const char *) +ldb_get_root_basedn: struct ldb_dn *(struct ldb_context *) +ldb_get_schema_basedn: struct ldb_dn *(struct ldb_context *) +ldb_global_init: int (void) +ldb_handle_get_event_context: struct tevent_context *(struct ldb_handle *) +ldb_handle_new: struct ldb_handle *(TALLOC_CTX *, struct ldb_context *) +ldb_handle_use_global_event_context: void (struct ldb_handle *) +ldb_handler_copy: int (struct ldb_context *, void *, const struct ldb_val *, struct ldb_val *) +ldb_handler_fold: int (struct ldb_context *, void *, const struct ldb_val *, struct ldb_val *) +ldb_init: struct ldb_context *(TALLOC_CTX *, struct tevent_context *) +ldb_ldif_message_redacted_string: char *(struct ldb_context *, TALLOC_CTX *, enum ldb_changetype, const struct ldb_message *) +ldb_ldif_message_string: char *(struct ldb_context *, TALLOC_CTX *, enum ldb_changetype, const struct ldb_message *) +ldb_ldif_parse_modrdn: int (struct ldb_context *, const struct ldb_ldif *, TALLOC_CTX *, struct ldb_dn **, struct ldb_dn **, bool *, struct ldb_dn **, struct ldb_dn **) +ldb_ldif_read: struct ldb_ldif *(struct ldb_context *, int (*)(void *), void *) +ldb_ldif_read_file: struct ldb_ldif *(struct ldb_context *, FILE *) +ldb_ldif_read_file_state: struct ldb_ldif *(struct ldb_context *, struct ldif_read_file_state *) +ldb_ldif_read_free: void (struct ldb_context *, struct ldb_ldif *) +ldb_ldif_read_string: struct ldb_ldif *(struct ldb_context *, const char **) +ldb_ldif_write: int (struct ldb_context *, int (*)(void *, const char *, ...), void *, const struct ldb_ldif *) +ldb_ldif_write_file: int (struct ldb_context *, FILE *, const struct ldb_ldif *) +ldb_ldif_write_redacted_trace_string: char *(struct ldb_context *, TALLOC_CTX *, const struct ldb_ldif *) +ldb_ldif_write_string: char *(struct ldb_context *, TALLOC_CTX *, const struct ldb_ldif *) +ldb_load_modules: int (struct ldb_context *, const char **) +ldb_map_add: int (struct ldb_module *, struct ldb_request *) +ldb_map_delete: int (struct ldb_module *, struct ldb_request *) +ldb_map_init: int (struct ldb_module *, const struct ldb_map_attribute *, const struct ldb_map_objectclass *, const char * const *, const char *, const char *) +ldb_map_modify: int (struct ldb_module *, struct ldb_request *) +ldb_map_rename: int (struct ldb_module *, struct ldb_request *) +ldb_map_search: int (struct ldb_module *, struct ldb_request *) +ldb_match_message: int (struct ldb_context *, const struct ldb_message *, const struct ldb_parse_tree *, enum ldb_scope, bool *) +ldb_match_msg: int (struct ldb_context *, const struct ldb_message *, const struct ldb_parse_tree *, struct ldb_dn *, enum ldb_scope) +ldb_match_msg_error: int (struct ldb_context *, const struct ldb_message *, const struct ldb_parse_tree *, struct ldb_dn *, enum ldb_scope, bool *) +ldb_match_msg_objectclass: int (const struct ldb_message *, const char *) +ldb_mod_register_control: int (struct ldb_module *, const char *) +ldb_modify: int (struct ldb_context *, const struct ldb_message *) +ldb_modify_default_callback: int (struct ldb_request *, struct ldb_reply *) +ldb_module_call_chain: char *(struct ldb_request *, TALLOC_CTX *) +ldb_module_connect_backend: int (struct ldb_context *, const char *, const char **, struct ldb_module **) +ldb_module_done: int (struct ldb_request *, struct ldb_control **, struct ldb_extended *, int) +ldb_module_flags: uint32_t (struct ldb_context *) +ldb_module_get_ctx: struct ldb_context *(struct ldb_module *) +ldb_module_get_name: const char *(struct ldb_module *) +ldb_module_get_ops: const struct ldb_module_ops *(struct ldb_module *) +ldb_module_get_private: void *(struct ldb_module *) +ldb_module_init_chain: int (struct ldb_context *, struct ldb_module *) +ldb_module_load_list: int (struct ldb_context *, const char **, struct ldb_module *, struct ldb_module **) +ldb_module_new: struct ldb_module *(TALLOC_CTX *, struct ldb_context *, const char *, const struct ldb_module_ops *) +ldb_module_next: struct ldb_module *(struct ldb_module *) +ldb_module_popt_options: struct poptOption **(struct ldb_context *) +ldb_module_send_entry: int (struct ldb_request *, struct ldb_message *, struct ldb_control **) +ldb_module_send_referral: int (struct ldb_request *, char *) +ldb_module_set_next: void (struct ldb_module *, struct ldb_module *) +ldb_module_set_private: void (struct ldb_module *, void *) +ldb_modules_hook: int (struct ldb_context *, enum ldb_module_hook_type) +ldb_modules_list_from_string: const char **(struct ldb_context *, TALLOC_CTX *, const char *) +ldb_modules_load: int (const char *, const char *) +ldb_msg_add: int (struct ldb_message *, const struct ldb_message_element *, int) +ldb_msg_add_empty: int (struct ldb_message *, const char *, int, struct ldb_message_element **) +ldb_msg_add_fmt: int (struct ldb_message *, const char *, const char *, ...) +ldb_msg_add_linearized_dn: int (struct ldb_message *, const char *, struct ldb_dn *) +ldb_msg_add_steal_string: int (struct ldb_message *, const char *, char *) +ldb_msg_add_steal_value: int (struct ldb_message *, const char *, struct ldb_val *) +ldb_msg_add_string: int (struct ldb_message *, const char *, const char *) +ldb_msg_add_value: int (struct ldb_message *, const char *, const struct ldb_val *, struct ldb_message_element **) +ldb_msg_canonicalize: struct ldb_message *(struct ldb_context *, const struct ldb_message *) +ldb_msg_check_string_attribute: int (const struct ldb_message *, const char *, const char *) +ldb_msg_copy: struct ldb_message *(TALLOC_CTX *, const struct ldb_message *) +ldb_msg_copy_attr: int (struct ldb_message *, const char *, const char *) +ldb_msg_copy_shallow: struct ldb_message *(TALLOC_CTX *, const struct ldb_message *) +ldb_msg_diff: struct ldb_message *(struct ldb_context *, struct ldb_message *, struct ldb_message *) +ldb_msg_difference: int (struct ldb_context *, TALLOC_CTX *, struct ldb_message *, struct ldb_message *, struct ldb_message **) +ldb_msg_element_compare: int (struct ldb_message_element *, struct ldb_message_element *) +ldb_msg_element_compare_name: int (struct ldb_message_element *, struct ldb_message_element *) +ldb_msg_element_equal_ordered: bool (const struct ldb_message_element *, const struct ldb_message_element *) +ldb_msg_find_attr_as_bool: int (const struct ldb_message *, const char *, int) +ldb_msg_find_attr_as_dn: struct ldb_dn *(struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, const char *) +ldb_msg_find_attr_as_double: double (const struct ldb_message *, const char *, double) +ldb_msg_find_attr_as_int: int (const struct ldb_message *, const char *, int) +ldb_msg_find_attr_as_int64: int64_t (const struct ldb_message *, const char *, int64_t) +ldb_msg_find_attr_as_string: const char *(const struct ldb_message *, const char *, const char *) +ldb_msg_find_attr_as_uint: unsigned int (const struct ldb_message *, const char *, unsigned int) +ldb_msg_find_attr_as_uint64: uint64_t (const struct ldb_message *, const char *, uint64_t) +ldb_msg_find_common_values: int (struct ldb_context *, TALLOC_CTX *, struct ldb_message_element *, struct ldb_message_element *, uint32_t) +ldb_msg_find_duplicate_val: int (struct ldb_context *, TALLOC_CTX *, const struct ldb_message_element *, struct ldb_val **, uint32_t) +ldb_msg_find_element: struct ldb_message_element *(const struct ldb_message *, const char *) +ldb_msg_find_ldb_val: const struct ldb_val *(const struct ldb_message *, const char *) +ldb_msg_find_val: struct ldb_val *(const struct ldb_message_element *, struct ldb_val *) +ldb_msg_new: struct ldb_message *(TALLOC_CTX *) +ldb_msg_normalize: int (struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_message **) +ldb_msg_remove_attr: void (struct ldb_message *, const char *) +ldb_msg_remove_element: void (struct ldb_message *, struct ldb_message_element *) +ldb_msg_rename_attr: int (struct ldb_message *, const char *, const char *) +ldb_msg_sanity_check: int (struct ldb_context *, const struct ldb_message *) +ldb_msg_sort_elements: void (struct ldb_message *) +ldb_next_del_trans: int (struct ldb_module *) +ldb_next_end_trans: int (struct ldb_module *) +ldb_next_init: int (struct ldb_module *) +ldb_next_prepare_commit: int (struct ldb_module *) +ldb_next_read_lock: int (struct ldb_module *) +ldb_next_read_unlock: int (struct ldb_module *) +ldb_next_remote_request: int (struct ldb_module *, struct ldb_request *) +ldb_next_request: int (struct ldb_module *, struct ldb_request *) +ldb_next_start_trans: int (struct ldb_module *) +ldb_op_default_callback: int (struct ldb_request *, struct ldb_reply *) +ldb_options_find: const char *(struct ldb_context *, const char **, const char *) +ldb_pack_data: int (struct ldb_context *, const struct ldb_message *, struct ldb_val *) +ldb_parse_control_from_string: struct ldb_control *(struct ldb_context *, TALLOC_CTX *, const char *) +ldb_parse_control_strings: struct ldb_control **(struct ldb_context *, TALLOC_CTX *, const char **) +ldb_parse_tree: struct ldb_parse_tree *(TALLOC_CTX *, const char *) +ldb_parse_tree_attr_replace: void (struct ldb_parse_tree *, const char *, const char *) +ldb_parse_tree_copy_shallow: struct ldb_parse_tree *(TALLOC_CTX *, const struct ldb_parse_tree *) +ldb_parse_tree_walk: int (struct ldb_parse_tree *, int (*)(struct ldb_parse_tree *, void *), void *) +ldb_qsort: void (void * const, size_t, size_t, void *, ldb_qsort_cmp_fn_t) +ldb_register_backend: int (const char *, ldb_connect_fn, bool) +ldb_register_extended_match_rule: int (struct ldb_context *, const struct ldb_extended_match_rule *) +ldb_register_hook: int (ldb_hook_fn) +ldb_register_module: int (const struct ldb_module_ops *) +ldb_rename: int (struct ldb_context *, struct ldb_dn *, struct ldb_dn *) +ldb_reply_add_control: int (struct ldb_reply *, const char *, bool, void *) +ldb_reply_get_control: struct ldb_control *(struct ldb_reply *, const char *) +ldb_req_get_custom_flags: uint32_t (struct ldb_request *) +ldb_req_is_untrusted: bool (struct ldb_request *) +ldb_req_location: const char *(struct ldb_request *) +ldb_req_mark_trusted: void (struct ldb_request *) +ldb_req_mark_untrusted: void (struct ldb_request *) +ldb_req_set_custom_flags: void (struct ldb_request *, uint32_t) +ldb_req_set_location: void (struct ldb_request *, const char *) +ldb_request: int (struct ldb_context *, struct ldb_request *) +ldb_request_add_control: int (struct ldb_request *, const char *, bool, void *) +ldb_request_done: int (struct ldb_request *, int) +ldb_request_get_control: struct ldb_control *(struct ldb_request *, const char *) +ldb_request_get_status: int (struct ldb_request *) +ldb_request_replace_control: int (struct ldb_request *, const char *, bool, void *) +ldb_request_set_state: void (struct ldb_request *, int) +ldb_reset_err_string: void (struct ldb_context *) +ldb_save_controls: int (struct ldb_control *, struct ldb_request *, struct ldb_control ***) +ldb_schema_attribute_add: int (struct ldb_context *, const char *, unsigned int, const char *) +ldb_schema_attribute_add_with_syntax: int (struct ldb_context *, const char *, unsigned int, const struct ldb_schema_syntax *) +ldb_schema_attribute_by_name: const struct ldb_schema_attribute *(struct ldb_context *, const char *) +ldb_schema_attribute_fill_with_syntax: int (struct ldb_context *, TALLOC_CTX *, const char *, unsigned int, const struct ldb_schema_syntax *, struct ldb_schema_attribute *) +ldb_schema_attribute_remove: void (struct ldb_context *, const char *) +ldb_schema_attribute_remove_flagged: void (struct ldb_context *, unsigned int) +ldb_schema_attribute_set_override_handler: void (struct ldb_context *, ldb_attribute_handler_override_fn_t, void *) +ldb_schema_set_override_GUID_index: void (struct ldb_context *, const char *, const char *) +ldb_schema_set_override_indexlist: void (struct ldb_context *, bool) +ldb_search: int (struct ldb_context *, TALLOC_CTX *, struct ldb_result **, struct ldb_dn *, enum ldb_scope, const char * const *, const char *, ...) +ldb_search_default_callback: int (struct ldb_request *, struct ldb_reply *) +ldb_sequence_number: int (struct ldb_context *, enum ldb_sequence_type, uint64_t *) +ldb_set_create_perms: void (struct ldb_context *, unsigned int) +ldb_set_debug: int (struct ldb_context *, void (*)(void *, enum ldb_debug_level, const char *, va_list), void *) +ldb_set_debug_stderr: int (struct ldb_context *) +ldb_set_default_dns: void (struct ldb_context *) +ldb_set_errstring: void (struct ldb_context *, const char *) +ldb_set_event_context: void (struct ldb_context *, struct tevent_context *) +ldb_set_flags: void (struct ldb_context *, unsigned int) +ldb_set_modules_dir: void (struct ldb_context *, const char *) +ldb_set_opaque: int (struct ldb_context *, const char *, void *) +ldb_set_require_private_event_context: void (struct ldb_context *) +ldb_set_timeout: int (struct ldb_context *, struct ldb_request *, int) +ldb_set_timeout_from_prev_req: int (struct ldb_context *, struct ldb_request *, struct ldb_request *) +ldb_set_utf8_default: void (struct ldb_context *) +ldb_set_utf8_fns: void (struct ldb_context *, void *, char *(*)(void *, void *, const char *, size_t)) +ldb_setup_wellknown_attributes: int (struct ldb_context *) +ldb_should_b64_encode: int (struct ldb_context *, const struct ldb_val *) +ldb_standard_syntax_by_name: const struct ldb_schema_syntax *(struct ldb_context *, const char *) +ldb_strerror: const char *(int) +ldb_string_to_time: time_t (const char *) +ldb_string_utc_to_time: time_t (const char *) +ldb_timestring: char *(TALLOC_CTX *, time_t) +ldb_timestring_utc: char *(TALLOC_CTX *, time_t) +ldb_transaction_cancel: int (struct ldb_context *) +ldb_transaction_cancel_noerr: int (struct ldb_context *) +ldb_transaction_commit: int (struct ldb_context *) +ldb_transaction_prepare_commit: int (struct ldb_context *) +ldb_transaction_start: int (struct ldb_context *) +ldb_unpack_data: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *) +ldb_unpack_data_only_attr_list: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *, const char * const *, unsigned int, unsigned int *) +ldb_unpack_data_only_attr_list_flags: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *, const char * const *, unsigned int, unsigned int, unsigned int *) +ldb_val_dup: struct ldb_val (TALLOC_CTX *, const struct ldb_val *) +ldb_val_equal_exact: int (const struct ldb_val *, const struct ldb_val *) +ldb_val_map_local: struct ldb_val (struct ldb_module *, void *, const struct ldb_map_attribute *, const struct ldb_val *) +ldb_val_map_remote: struct ldb_val (struct ldb_module *, void *, const struct ldb_map_attribute *, const struct ldb_val *) +ldb_val_string_cmp: int (const struct ldb_val *, const char *) +ldb_val_to_time: int (const struct ldb_val *, time_t *) +ldb_valid_attr_name: int (const char *) +ldb_vdebug: void (struct ldb_context *, enum ldb_debug_level, const char *, va_list) +ldb_wait: int (struct ldb_handle *, enum ldb_wait_type) diff --git a/lib/ldb/ABI/pyldb-util-1.3.5.sigs b/lib/ldb/ABI/pyldb-util-1.3.5.sigs new file mode 100644 index 00000000000..74d6719d2bc --- /dev/null +++ b/lib/ldb/ABI/pyldb-util-1.3.5.sigs @@ -0,0 +1,2 @@ +pyldb_Dn_FromDn: PyObject *(struct ldb_dn *) +pyldb_Object_AsDn: bool (TALLOC_CTX *, PyObject *, struct ldb_context *, struct ldb_dn **) diff --git a/lib/ldb/ABI/pyldb-util.py3-1.3.5.sigs b/lib/ldb/ABI/pyldb-util.py3-1.3.5.sigs new file mode 100644 index 00000000000..74d6719d2bc --- /dev/null +++ b/lib/ldb/ABI/pyldb-util.py3-1.3.5.sigs @@ -0,0 +1,2 @@ +pyldb_Dn_FromDn: PyObject *(struct ldb_dn *) +pyldb_Object_AsDn: bool (TALLOC_CTX *, PyObject *, struct ldb_context *, struct ldb_dn **) diff --git a/lib/ldb/ldb_sqlite3/ldb_sqlite3.c b/lib/ldb/ldb_sqlite3/ldb_sqlite3.c index f94dc993904..0f5abf87547 100644 --- a/lib/ldb/ldb_sqlite3/ldb_sqlite3.c +++ b/lib/ldb/ldb_sqlite3/ldb_sqlite3.c @@ -323,6 +323,9 @@ static char *parsetree_to_sql(struct ldb_module *module, const char *cdn = ldb_dn_get_casefold( ldb_dn_new(mem_ctx, ldb, (const char *)value.data)); + if (cdn == NULL) { + return NULL; + } return lsqlite3_tprintf(mem_ctx, "SELECT eid FROM ldb_entry " diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c index 40baeea5c2b..429c8f5aa24 100644 --- a/lib/ldb/ldb_tdb/ldb_index.c +++ b/lib/ldb/ldb_tdb/ldb_index.c @@ -970,6 +970,7 @@ static int ltdb_index_dn_leaf(struct ldb_module *module, return LDB_SUCCESS; } if (ldb_attr_dn(tree->u.equality.attr) == 0) { + bool valid_dn = false; struct ldb_dn *dn = ldb_dn_from_ldb_val(list, ldb_module_get_ctx(module), @@ -981,6 +982,14 @@ static int ltdb_index_dn_leaf(struct ldb_module *module, return LDB_SUCCESS; } + valid_dn = ldb_dn_validate(dn); + if (valid_dn == false) { + /* If we can't parse it, no match */ + list->dn = NULL; + list->count = 0; + return LDB_SUCCESS; + } + /* * Re-use the same code we use for a SCOPE_BASE * search @@ -1405,6 +1414,15 @@ static int ltdb_index_dn_attr(struct ldb_module *module, /* work out the index key from the parent DN */ val.data = (uint8_t *)((uintptr_t)ldb_dn_get_casefold(dn)); + if (val.data == NULL) { + const char *dn_str = ldb_dn_get_linearized(dn); + ldb_asprintf_errstring(ldb_module_get_ctx(module), + __location__ + ": Failed to get casefold DN " + "from: %s", + dn_str); + return LDB_ERR_OPERATIONS_ERROR; + } val.length = strlen((char *)val.data); key = ltdb_index_key(ldb, ltdb, attr, &val, NULL); if (!key) { diff --git a/lib/ldb/ldb_tdb/ldb_search.c b/lib/ldb/ldb_tdb/ldb_search.c index 02890862cf7..d14be0febd4 100644 --- a/lib/ldb/ldb_tdb/ldb_search.c +++ b/lib/ldb/ldb_tdb/ldb_search.c @@ -295,6 +295,14 @@ int ltdb_search_dn1(struct ldb_module *module, struct ldb_dn *dn, struct ldb_mes }; TALLOC_CTX *tdb_key_ctx = NULL; + bool valid_dn = ldb_dn_validate(dn); + if (valid_dn == false) { + ldb_asprintf_errstring(ldb_module_get_ctx(module), + "Invalid Base DN: %s", + ldb_dn_get_linearized(dn)); + return LDB_ERR_INVALID_DN_SYNTAX; + } + if (ltdb->cache->GUID_index_attribute == NULL) { tdb_key_ctx = talloc_new(msg); if (!tdb_key_ctx) { @@ -803,6 +811,14 @@ int ltdb_search(struct ltdb_context *ctx) ldb_dn_get_linearized(req->op.search.base)); } + } else if (ldb_dn_validate(req->op.search.base) == false) { + + /* We don't want invalid base DNs here */ + ldb_asprintf_errstring(ldb, + "Invalid Base DN: %s", + ldb_dn_get_linearized(req->op.search.base)); + ret = LDB_ERR_INVALID_DN_SYNTAX; + } else { /* If we are not checking the base DN life is easy */ ret = LDB_SUCCESS; diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c index 701427609e9..c7bf865de58 100644 --- a/lib/ldb/ldb_tdb/ldb_tdb.c +++ b/lib/ldb/ldb_tdb/ldb_tdb.c @@ -515,6 +515,16 @@ static int ltdb_add_internal(struct ldb_module *module, struct ldb_context *ldb = ldb_module_get_ctx(module); int ret = LDB_SUCCESS; unsigned int i; + bool valid_dn = false; + + /* Check the new DN is reasonable */ + valid_dn = ldb_dn_validate(msg->dn); + if (valid_dn == false) { + ldb_asprintf_errstring(ldb_module_get_ctx(module), + "Invalid DN in ADD: %s", + ldb_dn_get_linearized(msg->dn)); + return LDB_ERR_INVALID_DN_SYNTAX; + } for (i=0;i<msg->num_elements;i++) { struct ldb_message_element *el = &msg->elements[i]; @@ -1292,6 +1302,7 @@ static int ltdb_rename(struct ltdb_context *ctx) int ret = LDB_SUCCESS; TDB_DATA tdb_key, tdb_key_old; struct ldb_dn *db_dn; + bool valid_dn = false; ldb_request_set_state(req, LDB_ASYNC_PENDING); @@ -1304,10 +1315,24 @@ static int ltdb_rename(struct ltdb_context *ctx) return LDB_ERR_OPERATIONS_ERROR; } + /* Check the new DN is reasonable */ + valid_dn = ldb_dn_validate(req->op.rename.newdn); + if (valid_dn == false) { + ldb_asprintf_errstring(ldb_module_get_ctx(module), + "Invalid New DN: %s", + ldb_dn_get_linearized(req->op.rename.newdn)); + return LDB_ERR_INVALID_DN_SYNTAX; + } + /* we need to fetch the old record to re-add under the new name */ ret = ltdb_search_dn1(module, req->op.rename.olddn, msg, LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC); - if (ret != LDB_SUCCESS) { + if (ret == LDB_ERR_INVALID_DN_SYNTAX) { + ldb_asprintf_errstring(ldb_module_get_ctx(module), + "Invalid Old DN: %s", + ldb_dn_get_linearized(req->op.rename.newdn)); + return ret; + } else if (ret != LDB_SUCCESS) { /* not finding the old record is an error */ return ret; } diff --git a/lib/ldb/tests/python/api.py b/lib/ldb/tests/python/api.py index a62b241444b..48fac887f1c 100755 --- a/lib/ldb/tests/python/api.py +++ b/lib/ldb/tests/python/api.py @@ -401,6 +401,19 @@ class SimpleLdb(LdbBaseTest): finally: l.delete(ldb.Dn(l, "dc=bar")) + def test_rename_bad_string_dns(self): + l = ldb.Ldb(self.url(), flags=self.flags()) + m = ldb.Message() + m.dn = ldb.Dn(l, "dc=foo8") + m["bla"] = b"bla" + m["objectUUID"] = b"0123456789abcdef" + self.assertEqual(len(l.search()), 0) + l.add(m) + self.assertEqual(len(l.search()), 1) + self.assertRaises(ldb.LdbError,lambda: l.rename("dcXfoo8", "dc=bar")) + self.assertRaises(ldb.LdbError,lambda: l.rename("dc=foo8", "dcXbar")) + l.delete(ldb.Dn(l, "dc=foo8")) + def test_empty_dn(self): l = ldb.Ldb(self.url(), flags=self.flags()) self.assertEqual(0, len(l.search())) @@ -1143,6 +1156,110 @@ class SearchTests(LdbBaseTest): # At some point we should fix this, but it isn't trivial self.assertEqual(len(res11), 1) + def test_distinguishedName_filter_one(self): + """Testing that a distinguishedName= filter succeeds + when the scope is SCOPE_ONELEVEL. + + This should be made more consistent, but for now lock in + the behaviour + + """ + + res11 = self.l.search(base="DC=SAMBA,DC=ORG", + scope=ldb.SCOPE_ONELEVEL, + expression="(distinguishedName=OU=OU1,DC=SAMBA,DC=ORG)") + self.assertEqual(len(res11), 1) + + def test_distinguishedName_filter_subtree(self): + """Testing that a distinguishedName= filter succeeds + when the scope is SCOPE_SUBTREE""" + + res11 = self.l.search(base="DC=SAMBA,DC=ORG", + scope=ldb.SCOPE_SUBTREE, + expression="(distinguishedName=OU=OU1,DC=SAMBA,DC=ORG)") + self.assertEqual(len(res11), 1) + + def test_distinguishedName_filter_base(self): + """Testing that (incorrectly) a distinguishedName= filter works + when the scope is SCOPE_BASE""" + + res11 = self.l.search(base="OU=OU1,DC=SAMBA,DC=ORG", + scope=ldb.SCOPE_BASE, + expression="(distinguishedName=OU=OU1,DC=SAMBA,DC=ORG)") + + # At some point we should fix this, but it isn't trivial + self.assertEqual(len(res11), 1) + + def test_bad_dn_filter_base(self): + """Testing that a dn= filter on an invalid DN works + when the scope is SCOPE_BASE but + returns zero results""" + + res11 = self.l.search(base="OU=OU1,DC=SAMBA,DC=ORG", + scope=ldb.SCOPE_BASE, + expression="(dn=OU=OU1,DC=SAMBA,DCXXXX)") + + # At some point we should fix this, but it isn't trivial + self.assertEqual(len(res11), 0) + + + def test_bad_dn_filter_one(self): + """Testing that a dn= filter succeeds but returns zero + results when the DN is not valid on a SCOPE_ONELEVEL search + + """ + + res11 = self.l.search(base="DC=SAMBA,DC=ORG", + scope=ldb.SCOPE_ONELEVEL, + expression="(dn=OU=OU1,DC=SAMBA,DCXXXX)") + self.assertEqual(len(res11), 0) + + def test_bad_dn_filter_subtree(self): + """Testing that a dn= filter succeeds but returns zero + results when the DN is not valid on a SCOPE_SUBTREE search + + """ + + res11 = self.l.search(base="DC=SAMBA,DC=ORG", + scope=ldb.SCOPE_SUBTREE, + expression="(dn=OU=OU1,DC=SAMBA,DCXXXX)") + self.assertEqual(len(res11), 0) + + def test_bad_distinguishedName_filter_base(self): + """Testing that a distinguishedName= filter on an invalid DN works + when the scope is SCOPE_BASE but + returns zero results""" + + res11 = self.l.search(base="OU=OU1,DC=SAMBA,DC=ORG", + scope=ldb.SCOPE_BASE, + expression="(distinguishedName=OU=OU1,DC=SAMBA,DCXXXX)") + + # At some point we should fix this, but it isn't trivial + self.assertEqual(len(res11), 0) + + + def test_bad_distinguishedName_filter_one(self): + """Testing that a distinguishedName= filter succeeds but returns zero + results when the DN is not valid on a SCOPE_ONELEVEL search + + """ + + res11 = self.l.search(base="DC=SAMBA,DC=ORG", + scope=ldb.SCOPE_ONELEVEL, + expression="(distinguishedName=OU=OU1,DC=SAMBA,DCXXXX)") + self.assertEqual(len(res11), 0) + + def test_bad_distinguishedName_filter_subtree(self): + """Testing that a distinguishedName= filter succeeds but returns zero + results when the DN is not valid on a SCOPE_SUBTREE search + + """ + + res11 = self.l.search(base="DC=SAMBA,DC=ORG", + scope=ldb.SCOPE_SUBTREE, + expression="(distinguishedName=OU=OU1,DC=SAMBA,DCXXXX)") + self.assertEqual(len(res11), 0) + class IndexedSearchTests(SearchTests): """Test searches using the index, to ensure the index doesn't @@ -1291,6 +1408,17 @@ class AddModifyTests(LdbBaseTest): enum = err.args[0] self.assertEqual(enum, ldb.ERR_ENTRY_ALREADY_EXISTS) + def test_add_bad(self): + try: + self.l.add({"dn": "BAD,DC=SAMBA,DC=ORG", + "name": b"Admins", + "x": "z", "y": "a", + "objectUUID": b"0123456789abcde1"}) + self.fail("Should have failed adding entry with invalid DN") + except ldb.LdbError as err: + enum = err.args[0] + self.assertEqual(enum, ldb.ERR_INVALID_DN_SYNTAX) + def test_add_del_add(self): self.l.add({"dn": "OU=DUP,DC=SAMBA,DC=ORG", "name": b"Admins", @@ -1372,6 +1500,34 @@ class AddModifyTests(LdbBaseTest): enum = err.args[0] self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT) + def test_move_bad(self): + self.l.add({"dn": "OU=DUP2,DC=SAMBA,DC=ORG", + "name": b"Admins", + "x": "z", "y": "a", + "objectUUID": b"0123456789abcde2"}) + + try: + self.l.rename("OUXDUP,DC=SAMBA,DC=ORG", + "OU=DUP2,DC=SAMBA,DC=ORG") + self.fail("Should have failed on invalid DN") + except ldb.LdbError as err: + enum = err.args[0] + self.assertEqual(enum, ldb.ERR_INVALID_DN_SYNTAX) + + def test_move_bad2(self): + self.l.add({"dn": "OU=DUP2,DC=SAMBA,DC=ORG", + "name": b"Admins", + "x": "z", "y": "a", + "objectUUID": b"0123456789abcde2"}) + + try: + self.l.rename("OU=DUP,DC=SAMBA,DC=ORG", + "OUXDUP2,DC=SAMBA,DC=ORG") + self.fail("Should have failed on missing") + except ldb.LdbError as err: + enum = err.args[0] + self.assertEqual(enum, ldb.ERR_INVALID_DN_SYNTAX) + def test_move_fail_move_add(self): self.l.add({"dn": "OU=DUP,DC=SAMBA,DC=ORG", "name": b"Admins", diff --git a/lib/ldb/wscript b/lib/ldb/wscript index 15a7caebbf6..27b4df120a0 100644 --- a/lib/ldb/wscript +++ b/lib/ldb/wscript @@ -1,7 +1,7 @@ #!/usr/bin/env python APPNAME = 'ldb' -VERSION = '1.3.4' +VERSION = '1.3.5' blddir = 'bin' diff --git a/libcli/auth/ntlm_check.c b/libcli/auth/ntlm_check.c index 3b02adc1d48..b68e9c87888 100644 --- a/libcli/auth/ntlm_check.c +++ b/libcli/auth/ntlm_check.c @@ -224,7 +224,7 @@ NTSTATUS hash_password_check(TALLOC_CTX *mem_ctx, const struct samr_Password *stored_nt) { if (stored_nt == NULL) { - DEBUG(3,("ntlm_password_check: NO NT password stored for user %s.\n", + DEBUG(3,("hash_password_check: NO NT password stored for user %s.\n", username)); } @@ -232,14 +232,14 @@ NTSTATUS hash_password_check(TALLOC_CTX *mem_ctx, if (memcmp(client_nt->hash, stored_nt->hash, sizeof(stored_nt->hash)) == 0) { return NT_STATUS_OK; } else { - DEBUG(3,("ntlm_password_check: Interactive logon: NT password check failed for user %s\n", + DEBUG(3,("hash_password_check: Interactive logon: NT password check failed for user %s\n", username)); return NT_STATUS_WRONG_PASSWORD; } } else if (client_lanman && stored_lanman) { if (!lanman_auth) { - DEBUG(3,("ntlm_password_check: Interactive logon: only LANMAN password supplied for user %s, and LM passwords are disabled!\n", + DEBUG(3,("hash_password_check: Interactive logon: only LANMAN password supplied for user %s, and LM passwords are disabled!\n", username)); return NT_STATUS_WRONG_PASSWORD; } @@ -250,7 +250,7 @@ NTSTATUS hash_password_check(TALLOC_CTX *mem_ctx, if (memcmp(client_lanman->hash, stored_lanman->hash, sizeof(stored_lanman->hash)) == 0) { return NT_STATUS_OK; } else { - DEBUG(3,("ntlm_password_check: Interactive logon: LANMAN password check failed for user %s\n", + DEBUG(3,("hash_password_check: Interactive logon: LANMAN password check failed for user %s\n", username)); return NT_STATUS_WRONG_PASSWORD; } @@ -572,7 +572,7 @@ NTSTATUS ntlm_password_check(TALLOC_CTX *mem_ctx, - I think this is related to Win9X pass-though authentication */ DEBUG(4,("ntlm_password_check: Checking NT MD4 password in LM field\n")); - if (ntlm_auth) { + if (ntlm_auth == NTLM_AUTH_ON) { if (smb_pwd_check_ntlmv1(mem_ctx, lm_response, stored_nt->hash, challenge, diff --git a/libcli/auth/tests/ntlm_check.c b/libcli/auth/tests/ntlm_check.c new file mode 100644 index 00000000000..e87a0a276d4 --- /dev/null +++ b/libcli/auth/tests/ntlm_check.c @@ -0,0 +1,413 @@ +/* + * Unit tests for the ntlm_check password hash check library. + * + * Copyright (C) Andrew Bartlett <abartlet@samba.org> 2018 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +/* + * from cmocka.c: + * These headers or their equivalents should be included prior to + * including + * this header file. + * + * #include <stdarg.h> + * #include <stddef.h> + * #include <setjmp.h> + * + * This allows test applications to use custom definitions of C standard + * library functions and types. + * + */ + +/* + * Note that the messaging routines (audit_message_send and get_event_server) + * are not tested by these unit tests. Currently they are for integration + * test support, and as such are exercised by the integration tests. + */ +#include <stdarg.h> +#include <stddef.h> +#include <setjmp.h> +#include <cmocka.h> + +#include "includes.h" +#include "../lib/crypto/crypto.h" +#include "librpc/gen_ndr/netlogon.h" +#include "libcli/auth/libcli_auth.h" +#include "auth/credentials/credentials.h" + +struct ntlm_state { + const char *username; + const char *domain; + DATA_BLOB challenge; + DATA_BLOB ntlm; + DATA_BLOB lm; + DATA_BLOB ntlm_key; + DATA_BLOB lm_key; + const struct samr_Password *nt_hash; +}; + +static int test_ntlm_setup_with_options(void **state, + int flags, bool upn) +{ + NTSTATUS status; + DATA_BLOB challenge = { + .data = discard_const_p(uint8_t, "I am a teapot"), + .length = 8 + }; + struct ntlm_state *ntlm_state = talloc(NULL, struct ntlm_state); + DATA_BLOB target_info = NTLMv2_generate_names_blob(ntlm_state, + NULL, + "serverdom"); + struct cli_credentials *creds = cli_credentials_init(ntlm_state); + cli_credentials_set_username(creds, + "testuser", + CRED_SPECIFIED); + cli_credentials_set_domain(creds, + "testdom", + CRED_SPECIFIED); + cli_credentials_set_workstation(creds, + "testwksta", + CRED_SPECIFIED); + cli_credentials_set_password(creds, + "testpass", + CRED_SPECIFIED); + + if (upn) { + cli_credentials_set_principal(creds, + "testuser@samba.org", + CRED_SPECIFIED); + } + + cli_credentials_get_ntlm_username_domain(creds, + ntlm_state, + &ntlm_state->username, + &ntlm_state->domain); + + status = cli_credentials_get_ntlm_response(creds, + ntlm_state, + &flags, + challenge, + NULL, + target_info, + &ntlm_state->lm, + &ntlm_state->ntlm, + &ntlm_state->lm_key, + &ntlm_state->ntlm_key); + ntlm_state->challenge = challenge; + + ntlm_state->nt_hash = cli_credentials_get_nt_hash(creds, + ntlm_state); + + if (!NT_STATUS_IS_OK(status)) { + return -1; + } + + *state = ntlm_state; + return 0; +} + +static int test_ntlm_setup(void **state) { + return test_ntlm_setup_with_options(state, 0, false); +} + +static int test_ntlm_and_lm_setup(void **state) { + return test_ntlm_setup_with_options(state, + CLI_CRED_LANMAN_AUTH, + false); +} + +static int test_ntlm2_setup(void **state) { + return test_ntlm_setup_with_options(state, + CLI_CRED_NTLM2, + false); +} + +static int test_ntlmv2_setup(void **state) { + return test_ntlm_setup_with_options(state, + CLI_CRED_NTLMv2_AUTH, + false); +} + +static int test_ntlm_teardown(void **state) +{ + struct ntlm_state *ntlm_state + = talloc_get_type_abort(*state, + struct ntlm_state); + TALLOC_FREE(ntlm_state); + *state = NULL; + return 0; +} + +static void test_ntlm_allowed(void **state) +{ + DATA_BLOB user_sess_key, lm_sess_key; + struct ntlm_state *ntlm_state + = talloc_get_type_abort(*state, + struct ntlm_state); + NTSTATUS status; + status = ntlm_password_check(ntlm_state, + false, + NTLM_AUTH_ON, + 0, + &ntlm_state->challenge, + &ntlm_state->lm, + &ntlm_state->ntlm, + ntlm_state->username, + ntlm_state->username, + ntlm_state->domain, + NULL, + ntlm_state->nt_hash, + &user_sess_key, + &lm_sess_key); + + assert_int_equal(NT_STATUS_V(status), NT_STATUS_V(NT_STATUS_OK)); +} + +static void test_ntlm_allowed_lm_supplied(void **state) +{ + return test_ntlm_allowed(state); +} + +static void test_ntlm_disabled(void **state) +{ + DATA_BLOB user_sess_key, lm_sess_key; + struct ntlm_state *ntlm_state + = talloc_get_type_abort(*state, + struct ntlm_state); + NTSTATUS status; + status = ntlm_password_check(ntlm_state, + false, + NTLM_AUTH_DISABLED, + 0, + &ntlm_state->challenge, + &ntlm_state->lm, + &ntlm_state->ntlm, + ntlm_state->username, + ntlm_state->username, + ntlm_state->domain, + NULL, + ntlm_state->nt_hash, + &user_sess_key, + &lm_sess_key); + + assert_int_equal(NT_STATUS_V(status), NT_STATUS_V(NT_STATUS_NTLM_BLOCKED)); +} + +static void test_ntlm2(void **state) +{ + DATA_BLOB user_sess_key, lm_sess_key; + struct ntlm_state *ntlm_state + = talloc_get_type_abort(*state, + struct ntlm_state); + NTSTATUS status; + status = ntlm_password_check(ntlm_state, + false, + NTLM_AUTH_ON, + 0, + &ntlm_state->challenge, + &ntlm_state->lm, + &ntlm_state->ntlm, + ntlm_state->username, + ntlm_state->username, + ntlm_state->domain, + NULL, + ntlm_state->nt_hash, + &user_sess_key, + &lm_sess_key); + + /* + * NTLM2 session security (where the real challenge is the + * MD5(challenge, client-challenge) (in the first 8 bytes of + * the lm) isn't decoded by ntlm_password_check(), it must + * first be converted back into normal NTLM by the NTLMSSP + * layer + */ + assert_int_equal(NT_STATUS_V(status), + NT_STATUS_V(NT_STATUS_WRONG_PASSWORD)); +} + +static void test_ntlm_mschapv2_only_allowed(void **state) +{ + DATA_BLOB user_sess_key, lm_sess_key; + struct ntlm_state *ntlm_state + = talloc_get_type_abort(*state, + struct ntlm_state); + NTSTATUS status; + status = ntlm_password_check(ntlm_state, + false, + NTLM_AUTH_MSCHAPv2_NTLMV2_ONLY, + MSV1_0_ALLOW_MSVCHAPV2, + &ntlm_state->challenge, + &ntlm_state->lm, + &ntlm_state->ntlm, + ntlm_state->username, + ntlm_state->username, + ntlm_state->domain, + NULL, + ntlm_state->nt_hash, + &user_sess_key, + &lm_sess_key); + + assert_int_equal(NT_STATUS_V(status), NT_STATUS_V(NT_STATUS_OK)); +} + +static void test_ntlm_mschapv2_only_denied(void **state) +{ + DATA_BLOB user_sess_key, lm_sess_key; + struct ntlm_state *ntlm_state + = talloc_get_type_abort(*state, + struct ntlm_state); + NTSTATUS status; + status = ntlm_password_check(ntlm_state, + false, + NTLM_AUTH_MSCHAPv2_NTLMV2_ONLY, + 0, + &ntlm_state->challenge, + &ntlm_state->lm, + &ntlm_state->ntlm, + ntlm_state->username, + ntlm_state->username, + ntlm_state->domain, + NULL, + ntlm_state->nt_hash, + &user_sess_key, + &lm_sess_key); + + assert_int_equal(NT_STATUS_V(status), + NT_STATUS_V(NT_STATUS_WRONG_PASSWORD)); +} + +static void test_ntlmv2_only_ntlmv2(void **state) +{ + DATA_BLOB user_sess_key, lm_sess_key; + struct ntlm_state *ntlm_state + = talloc_get_type_abort(*state, + struct ntlm_state); + NTSTATUS status; + status = ntlm_password_check(ntlm_state, + false, + NTLM_AUTH_NTLMV2_ONLY, + 0, + &ntlm_state->challenge, + &ntlm_state->lm, + &ntlm_state->ntlm, + ntlm_state->username, + ntlm_state->username, + ntlm_state->domain, + NULL, + ntlm_state->nt_hash, + &user_sess_key, + &lm_sess_key); + + assert_int_equal(NT_STATUS_V(status), NT_STATUS_V(NT_STATUS_OK)); +} + +static void test_ntlmv2_only_ntlm(void **state) +{ + DATA_BLOB user_sess_key, lm_sess_key; + struct ntlm_state *ntlm_state + = talloc_get_type_abort(*state, + struct ntlm_state); + NTSTATUS status; + status = ntlm_password_check(ntlm_state, + false, + NTLM_AUTH_NTLMV2_ONLY, + 0, + &ntlm_state->challenge, + &ntlm_state->lm, + &ntlm_state->ntlm, + ntlm_state->username, + ntlm_state->username, + ntlm_state->domain, + NULL, + ntlm_state->nt_hash, + &user_sess_key, + &lm_sess_key); + + assert_int_equal(NT_STATUS_V(status), + NT_STATUS_V(NT_STATUS_WRONG_PASSWORD)); +} + +static void test_ntlmv2_only_ntlm_and_lanman(void **state) +{ + return test_ntlmv2_only_ntlm(state); +} + +static void test_ntlmv2_only_ntlm_once(void **state) +{ + DATA_BLOB user_sess_key, lm_sess_key; + struct ntlm_state *ntlm_state + = talloc_get_type_abort(*state, + struct ntlm_state); + NTSTATUS status; + status = ntlm_password_check(ntlm_state, + false, + NTLM_AUTH_NTLMV2_ONLY, + 0, + &ntlm_state->challenge, + &data_blob_null, + &ntlm_state->ntlm, + ntlm_state->username, + ntlm_state->username, + ntlm_state->domain, + NULL, + ntlm_state->nt_hash, + &user_sess_key, + &lm_sess_key); + + assert_int_equal(NT_STATUS_V(status), + NT_STATUS_V(NT_STATUS_WRONG_PASSWORD)); +} + +int main(int argc, const char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test_setup_teardown(test_ntlm_allowed, + test_ntlm_setup, + test_ntlm_teardown), + cmocka_unit_test_setup_teardown(test_ntlm_allowed_lm_supplied, + test_ntlm_and_lm_setup, + test_ntlm_teardown), + cmocka_unit_test_setup_teardown(test_ntlm_disabled, + test_ntlm_setup, + test_ntlm_teardown), + cmocka_unit_test_setup_teardown(test_ntlm2, + test_ntlm2_setup, + test_ntlm_teardown), + cmocka_unit_test_setup_teardown(test_ntlm_mschapv2_only_allowed, + test_ntlm_setup, + test_ntlm_teardown), + cmocka_unit_test_setup_teardown(test_ntlm_mschapv2_only_denied, + test_ntlm_setup, + test_ntlm_teardown), + cmocka_unit_test_setup_teardown(test_ntlmv2_only_ntlm, + test_ntlm_setup, + test_ntlm_teardown), + cmocka_unit_test_setup_teardown(test_ntlmv2_only_ntlm_and_lanman, + test_ntlm_and_lm_setup, + test_ntlm_teardown), + cmocka_unit_test_setup_teardown(test_ntlmv2_only_ntlm_once, + test_ntlm_setup, + test_ntlm_teardown), + cmocka_unit_test_setup_teardown(test_ntlmv2_only_ntlmv2, + test_ntlmv2_setup, + test_ntlm_teardown) + }; + + cmocka_set_message_output(CM_OUTPUT_SUBUNIT); + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/libcli/auth/wscript_build b/libcli/auth/wscript_build index 475b7d69406..d319d9b879e 100644 --- a/libcli/auth/wscript_build +++ b/libcli/auth/wscript_build @@ -41,3 +41,16 @@ bld.SAMBA_SUBSYSTEM('PAM_ERRORS', bld.SAMBA_SUBSYSTEM('SPNEGO_PARSE', source='spnego_parse.c', deps='asn1util') + +bld.SAMBA_BINARY( + 'test_ntlm_check', + source='tests/ntlm_check.c', + deps=''' + NTLM_CHECK + CREDENTIALS_NTLM + samba-credentials + cmocka + talloc + ''', + install=False + ) diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c index b4c850b613e..03a7dca4adf 100644 --- a/libcli/security/access_check.c +++ b/libcli/security/access_check.c @@ -375,6 +375,81 @@ static const struct GUID *get_ace_object_type(struct security_ace *ace) } /** + * Evaluates access rights specified in a object-specific ACE for an AD object. + * This logic corresponds to MS-ADTS 5.1.3.3.3 Checking Object-Specific Access. + * @param[in] ace - the ACE being processed + * @param[in/out] tree - remaining_access gets updated for the tree + * @param[out] grant_access - set to true if the ACE grants sufficient access + * rights to the object/attribute + * @returns NT_STATUS_OK, unless access was denied + */ +static NTSTATUS check_object_specific_access(struct security_ace *ace, + struct object_tree *tree, + bool *grant_access) +{ + struct object_tree *node = NULL; + const struct GUID *type = NULL; + + *grant_access = false; + + /* if no tree was supplied, we can't do object-specific access checks */ + if (!tree) { + return NT_STATUS_OK; + } + + /* Get the ObjectType GUID this ACE applies to */ + type = get_ace_object_type(ace); + + /* + * If the ACE doesn't have a type, then apply it to the whole tree, i.e. + * treat 'OA' ACEs as 'A' and 'OD' as 'D' + */ + if (!type) { + node = tree; + } else { + + /* skip it if the ACE's ObjectType GUID is not in the tree */ + node = get_object_tree_by_GUID(tree, type); + if (!node) { + return NT_STATUS_OK; + } + } + + if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) { + + /* apply the access rights to this node, and any children */ + object_tree_modify_access(node, ace->access_mask); + + /* + * Currently all nodes in the tree request the same access mask, + * so we can use any node to check if processing this ACE now + * means the requested access has been granted + */ + if (node->remaining_access == 0) { + *grant_access = true; + return NT_STATUS_OK; + } + + /* + * As per 5.1.3.3.4 Checking Control Access Right-Based Access, + * if the CONTROL_ACCESS right is present, then we can grant + * access and stop any further access checks + */ + if (ace->access_mask & SEC_ADS_CONTROL_ACCESS) { + *grant_access = true; + return NT_STATUS_OK; + } + } else { + + /* this ACE denies access to the requested object/attribute */ + if (node->remaining_access & ace->access_mask){ + return NT_STATUS_ACCESS_DENIED; + } + } + return NT_STATUS_OK; +} + +/** * @brief Perform directoryservice (DS) related access checks for a given user * * Perform DS access checks for the user represented by its security_token, on @@ -405,8 +480,6 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, { uint32_t i; uint32_t bits_remaining; - struct object_tree *node; - const struct GUID *type; struct dom_sid self_sid; dom_sid_parse(SID_NT_SELF, &self_sid); @@ -456,6 +529,8 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, for (i=0; bits_remaining && i < sd->dacl->num_aces; i++) { struct dom_sid *trustee; struct security_ace *ace = &sd->dacl->aces[i]; + NTSTATUS status; + bool grant_access = false; if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) { continue; @@ -486,34 +561,15 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd, 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; - } + status = check_object_specific_access(ace, tree, + &grant_access); - if (!type) { - node = tree; - } else { - if (!(node = get_object_tree_by_GUID(tree, type))) { - continue; - } + if (!NT_STATUS_IS_OK(status)) { + return status; } - if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) { - object_tree_modify_access(node, ace->access_mask); - if (node->remaining_access == 0) { - return NT_STATUS_OK; - } - } else { - if (node->remaining_access & ace->access_mask){ - return NT_STATUS_ACCESS_DENIED; - } + if (grant_access) { + return NT_STATUS_OK; } break; default: /* Other ACE types not handled/supported */ diff --git a/python/samba/tests/dns_invalid.py b/python/samba/tests/dns_invalid.py new file mode 100644 index 00000000000..9f87cd56084 --- /dev/null +++ b/python/samba/tests/dns_invalid.py @@ -0,0 +1,87 @@ +# Unix SMB/CIFS implementation. +# Copyright (C) Kai Blin <kai@samba.org> 2018 +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +import os +import sys +import struct +import random +import socket +import samba.ndr as ndr +from samba import credentials, param +from samba.dcerpc import dns, dnsp, dnsserver +from samba.netcmd.dns import TXTRecord, dns_record_match, data_to_dns_record +from samba.tests.subunitrun import SubunitOptions, TestProgram +from samba import werror, WERRORError +from samba.tests.dns_base import DNSTest +import samba.getopt as options +import optparse + +parser = optparse.OptionParser("dns_invalid.py <server ip> [options]") +sambaopts = options.SambaOptions(parser) +parser.add_option_group(sambaopts) + +# This timeout only has relevance when testing against Windows +# Format errors tend to return patchy responses, so a timeout is needed. +parser.add_option("--timeout", type="int", dest="timeout", + help="Specify timeout for DNS requests") + +# use command line creds if available +credopts = options.CredentialsOptions(parser) +parser.add_option_group(credopts) +subunitopts = SubunitOptions(parser) +parser.add_option_group(subunitopts) + +opts, args = parser.parse_args() + +lp = sambaopts.get_loadparm() +creds = credopts.get_credentials(lp) + +timeout = opts.timeout + +if len(args) < 1: + parser.print_usage() + sys.exit(1) + +server_ip = args[0] +creds.set_krb_forwardable(credentials.NO_KRB_FORWARDABLE) + + +class TestBrokenQueries(DNSTest): + def setUp(self): + super(TestBrokenQueries, self).setUp() + global server, server_ip, lp, creds, timeout + self.server_ip = server_ip + self.lp = lp + self.creds = creds + self.timeout = timeout + + def test_invalid_chars_in_name(self): + """Check the server refuses invalid characters in the query name""" + p = self.make_name_packet(dns.DNS_OPCODE_QUERY) + questions = [] + + name = "\x10\x11\x05\xa8.%s" % self.get_dns_domain() + q = self.make_name_question(name, dns.DNS_QTYPE_A, dns.DNS_QCLASS_IN) + print "asking for ", q.name + questions.append(q) + + self.finish_name_packet(p, questions) + (response, response_packet) = self.dns_transaction_udp(p, host=server_ip) + self.assert_dns_rcode_equals(response, dns.DNS_RCODE_NXDOMAIN) + + +TestProgram(module=__name__, opts=subunitopts) diff --git a/selftest/knownfail b/selftest/knownfail index ba16fd72290..84776d4f35d 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -303,8 +303,9 @@ ^samba4.smb.signing.*disabled.*signing=off.*\(ad_dc\) # fl2000dc doesn't support AES ^samba4.krb5.kdc.*as-req-aes.*fl2000dc -# nt4_member and ad_member don't support ntlmv1 +# nt4_member and ad_member don't support ntlmv1 (not even over SMB1) ^samba3.blackbox.smbclient_auth.plain.*_member.*option=clientntlmv2auth=no.member.creds.*as.user +^samba3.blackbox.smbclient_auth.plain.*_member.*option=clientntlmv2auth=no.*mNT1.member.creds.*as.user #nt-vfs server blocks read with execute access ^samba4.smb2.read.access #ntvfs server blocks copychunk with execute access on read handle diff --git a/selftest/tests.py b/selftest/tests.py index 126e1184230..dc6486c13f8 100644 --- a/selftest/tests.py +++ b/selftest/tests.py @@ -38,7 +38,6 @@ finally: f.close() have_man_pages_support = ("XSLTPROC_MANPAGES" in config_hash) -with_cmocka = ("HAVE_CMOCKA" in config_hash) with_pam = ("WITH_PAM" in config_hash) pam_wrapper_so_path=config_hash["LIBPAM_WRAPPER_SO_PATH"] @@ -168,13 +167,14 @@ if with_pam: valgrindify(python), pam_wrapper_so_path, "$DOMAIN", "alice", "Secret007"]) -if with_cmocka: - plantestsuite("samba.unittests.krb5samba", "none", - [os.path.join(bindir(), "default/testsuite/unittests/test_krb5samba")]) - plantestsuite("samba.unittests.sambafs_srv_pipe", "none", - [os.path.join(bindir(), "default/testsuite/unittests/test_sambafs_srv_pipe")]) - plantestsuite("samba.unittests.lib_util_modules", "none", - [os.path.join(bindir(), "default/testsuite/unittests/test_lib_util_modules")]) +plantestsuite("samba.unittests.krb5samba", "none", + [os.path.join(bindir(), "default/testsuite/unittests/test_krb5samba")]) +plantestsuite("samba.unittests.sambafs_srv_pipe", "none", + [os.path.join(bindir(), "default/testsuite/unittests/test_sambafs_srv_pipe")]) +plantestsuite("samba.unittests.lib_util_modules", "none", + [os.path.join(bindir(), "default/testsuite/unittests/test_lib_util_modules")]) - plantestsuite("samba.unittests.smb1cli_session", "none", - [os.path.join(bindir(), "default/libcli/smb/test_smb1cli_session")]) +plantestsuite("samba.unittests.smb1cli_session", "none", + [os.path.join(bindir(), "default/libcli/smb/test_smb1cli_session")]) +plantestsuite("samba.unittests.ntlm_check", "none", + [os.path.join(bindir(), "default/libcli/auth/test_ntlm_check")]) diff --git a/source3/libsmb/libsmb_dir.c b/source3/libsmb/libsmb_dir.c index 72441c46736..54c2bcb3c73 100644 --- a/source3/libsmb/libsmb_dir.c +++ b/source3/libsmb/libsmb_dir.c @@ -943,27 +943,47 @@ SMBC_closedir_ctx(SMBCCTX *context, } -static void +static int smbc_readdir_internal(SMBCCTX * context, struct smbc_dirent *dest, struct smbc_dirent *src, int max_namebuf_len) { if (smbc_getOptionUrlEncodeReaddirEntries(context)) { + int remaining_len; /* url-encode the name. get back remaining buffer space */ - max_namebuf_len = + remaining_len = smbc_urlencode(dest->name, src->name, max_namebuf_len); + /* -1 means no null termination. */ + if (remaining_len < 0) { + return -1; + } + /* We now know the name length */ dest->namelen = strlen(dest->name); + if (dest->namelen + 1 < 1) { + /* Integer wrap. */ + return -1; + } + + if (dest->namelen + 1 >= max_namebuf_len) { + /* Out of space for comment. */ + return -1; + } + /* Save the pointer to the beginning of the comment */ dest->comment = dest->name + dest->namelen + 1; + if (remaining_len < 1) { + /* No room for comment null termination. */ + return -1; + } + /* Copy the comment */ - strncpy(dest->comment, src->comment, max_namebuf_len - 1); - dest->comment[max_namebuf_len - 1] = '\0'; + strlcpy(dest->comment, src->comment, remaining_len); /* Save other fields */ dest->smbc_type = src->smbc_type; @@ -973,10 +993,21 @@ smbc_readdir_internal(SMBCCTX * context, } else { /* No encoding. Just copy the entry as is. */ + if (src->dirlen > max_namebuf_len) { + return -1; + } memcpy(dest, src, src->dirlen); + if (src->namelen + 1 < 1) { + /* Integer wrap */ + return -1; + } + if (src->namelen + 1 >= max_namebuf_len) { + /* Comment off the end. */ + return -1; + } dest->comment = (char *)(&dest->name + src->namelen + 1); } - + return 0; } /* @@ -988,6 +1019,7 @@ SMBC_readdir_ctx(SMBCCTX *context, SMBCFILE *dir) { int maxlen; + int ret; struct smbc_dirent *dirp, *dirent; TALLOC_CTX *frame = talloc_stackframe(); @@ -1037,7 +1069,12 @@ SMBC_readdir_ctx(SMBCCTX *context, dirp = &context->internal->dirent; maxlen = sizeof(context->internal->_dirent_name); - smbc_readdir_internal(context, dirp, dirent, maxlen); + ret = smbc_readdir_internal(context, dirp, dirent, maxlen); + if (ret == -1) { + errno = EINVAL; + TALLOC_FREE(frame); + return NULL; + } dir->dir_next = dir->dir_next->next; @@ -1095,6 +1132,7 @@ SMBC_getdents_ctx(SMBCCTX *context, */ while ((dirlist = dir->dir_next)) { + int ret; struct smbc_dirent *dirent; struct smbc_dirent *currentEntry = (struct smbc_dirent *)ndir; @@ -1109,8 +1147,13 @@ SMBC_getdents_ctx(SMBCCTX *context, /* Do urlencoding of next entry, if so selected */ dirent = &context->internal->dirent; maxlen = sizeof(context->internal->_dirent_name); - smbc_readdir_internal(context, dirent, + ret = smbc_readdir_internal(context, dirent, dirlist->dirent, maxlen); + if (ret == -1) { + errno = EINVAL; + TALLOC_FREE(frame); + return -1; + } reqd = dirent->dirlen; diff --git a/source3/libsmb/libsmb_path.c b/source3/libsmb/libsmb_path.c index 01b0a61e483..5b53b386a67 100644 --- a/source3/libsmb/libsmb_path.c +++ b/source3/libsmb/libsmb_path.c @@ -173,8 +173,13 @@ smbc_urlencode(char *dest, } } - *dest++ = '\0'; - max_dest_len--; + if (max_dest_len <= 0) { + /* Ensure we return -1 if no null termination. */ + return -1; + } + + *dest++ = '\0'; + max_dest_len--; return max_dest_len; } diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index cdcb1ddbca4..68212e17258 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -190,7 +190,7 @@ for env in ["nt4_dc", "nt4_member", "ad_member", "ad_dc", "ad_dc_ntvfs", "s4memb plantestsuite("samba3.blackbox.smbclient_machine_auth.plain (%s:local)" % env, "%s:local" % env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_machine_auth.sh"), '$SERVER', smbclient3, configuration]) plantestsuite("samba3.blackbox.smbclient_ntlm.plain (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_ntlm.sh"), '$SERVER', '$DC_USERNAME', '$DC_PASSWORD', "never", smbclient3, configuration]) -for options in ["--option=clientntlmv2auth=no", "--option=clientusespnego=no --option=clientntlmv2auth=no", ""]: +for options in ["--option=clientntlmv2auth=no", "--option=clientusespnego=no --option=clientntlmv2auth=no", "--option=clientusespnego=no --option=clientntlmv2auth=no -mNT1", ""]: for env in ["nt4_member", "ad_member"]: plantestsuite("samba3.blackbox.smbclient_auth.plain (%s) %s" % (env, options), env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', '$DC_USERNAME', '$DC_PASSWORD', smbclient3, configuration, options]) plantestsuite("samba3.blackbox.smbclient_auth.plain (%s) %s member creds" % (env, options), env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', '$SERVER/$USERNAME', '$PASSWORD', smbclient3, configuration, options]) diff --git a/source3/utils/ntlm_auth.c b/source3/utils/ntlm_auth.c index 3f544902a24..8f77680416f 100644 --- a/source3/utils/ntlm_auth.c +++ b/source3/utils/ntlm_auth.c @@ -1010,7 +1010,7 @@ static NTSTATUS local_pw_check(struct auth4_context *auth4_context, *pauthoritative = 1; nt_status = ntlm_password_check(mem_ctx, - true, true, 0, + true, NTLM_AUTH_ON, 0, &auth4_context->challenge.data, &user_info->password.response.lanman, &user_info->password.response.nt, @@ -1719,7 +1719,9 @@ static void manage_ntlm_server_1_request(enum stdio_helper_mode stdio_helper_mod nt_lm_owf_gen (opt_password, nt_pw.hash, lm_pw.hash); nt_status = ntlm_password_check(mem_ctx, - true, true, 0, + true, + NTLM_AUTH_ON, + 0, &challenge, &lm_response, &nt_response, diff --git a/source4/dsdb/samdb/cracknames.c b/source4/dsdb/samdb/cracknames.c index d43f510b949..3b215ac0ec9 100644 --- a/source4/dsdb/samdb/cracknames.c +++ b/source4/dsdb/samdb/cracknames.c @@ -1253,7 +1253,13 @@ static WERROR DsCrackNameOneFilter(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ return WERR_OK; } case DRSUAPI_DS_NAME_FORMAT_SERVICE_PRINCIPAL: { - if (result->elements[0].num_values > 1) { + struct ldb_message_element *el + = ldb_msg_find_element(result, + "servicePrincipalName"); + if (el == NULL) { + info1->status = DRSUAPI_DS_NAME_STATUS_NOT_FOUND; + return WERR_OK; + } else if (el->num_values > 1) { info1->status = DRSUAPI_DS_NAME_STATUS_NOT_UNIQUE; return WERR_OK; } diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c index 3c9cf7c0672..280845a47a5 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_read.c +++ b/source4/dsdb/samdb/ldb_modules/acl_read.c @@ -38,6 +38,8 @@ #include "param/param.h" #include "dsdb/samdb/ldb_modules/util.h" +/* The attributeSecurityGuid for the Public Information Property-Set */ +#define PUBLIC_INFO_PROPERTY_SET "e48d0154-bcf8-11d1-8702-00c04fb96050" struct aclread_context { struct ldb_module *module; @@ -227,6 +229,253 @@ static int aclread_get_sd_from_ldb_message(struct aclread_context *ac, return LDB_SUCCESS; } +/* + * Returns the access mask required to read a given attribute + */ +static uint32_t get_attr_access_mask(const struct dsdb_attribute *attr, + uint32_t sd_flags) +{ + + uint32_t access_mask = 0; + bool is_sd; + + /* nTSecurityDescriptor is a special case */ + is_sd = (ldb_attr_cmp("nTSecurityDescriptor", + attr->lDAPDisplayName) == 0); + + if (is_sd) { + if (sd_flags & (SECINFO_OWNER|SECINFO_GROUP)) { + access_mask |= SEC_STD_READ_CONTROL; + } + if (sd_flags & SECINFO_DACL) { + access_mask |= SEC_STD_READ_CONTROL; + } + if (sd_flags & SECINFO_SACL) { + access_mask |= SEC_FLAG_SYSTEM_SECURITY; + } + } else { + access_mask = SEC_ADS_READ_PROP; + } + + if (attr->searchFlags & SEARCH_FLAG_CONFIDENTIAL) { + access_mask |= SEC_ADS_CONTROL_ACCESS; + } + + return access_mask; +} + +/* helper struct for traversing the attributes in the search-tree */ +struct parse_tree_aclread_ctx { + struct aclread_context *ac; + TALLOC_CTX *mem_ctx; + struct dom_sid *sid; + struct ldb_dn *dn; + struct security_descriptor *sd; + const struct dsdb_class *objectclass; + bool suppress_result; +}; + +/* + * Checks that the user has sufficient access rights to view an attribute + */ +static int check_attr_access_rights(TALLOC_CTX *mem_ctx, const char *attr_name, + struct aclread_context *ac, + struct security_descriptor *sd, + const struct dsdb_class *objectclass, + struct dom_sid *sid, struct ldb_dn *dn, + bool *is_public_info) +{ + int ret; + const struct dsdb_attribute *attr = NULL; + uint32_t access_mask; + struct ldb_context *ldb = ldb_module_get_ctx(ac->module); + + *is_public_info = false; + + attr = dsdb_attribute_by_lDAPDisplayName(ac->schema, attr_name); + if (!attr) { + ldb_debug_set(ldb, + LDB_DEBUG_TRACE, + "acl_read: %s cannot find attr[%s] in schema," + "ignoring\n", + ldb_dn_get_linearized(dn), attr_name); + return LDB_SUCCESS; + } + + /* + * If we have no Read Property (RP) rights for a child object, it should + * still appear as a visible object in 'objectClass=*' searches, + * as long as we have List Contents (LC) rights for it. + * This is needed for the acl.py tests (e.g. test_search1()). + * I couldn't find the Windows behaviour documented in the specs, so + * this is a guess, but it seems to only apply to attributes in the + * Public Information Property Set that have the systemOnly flag set to + * TRUE. (This makes sense in a way, as it's not disclosive to find out + * that a child object has a 'objectClass' or 'name' attribute, as every + * object has these attributes). + */ + if (attr->systemOnly) { + struct GUID public_info_guid; + NTSTATUS status; + + status = GUID_from_string(PUBLIC_INFO_PROPERTY_SET, + &public_info_guid); + if (!NT_STATUS_IS_OK(status)) { + ldb_set_errstring(ldb, "Public Info GUID parse error"); + return LDB_ERR_OPERATIONS_ERROR; + } + + if (GUID_compare(&attr->attributeSecurityGUID, + &public_info_guid) == 0) { + *is_public_info = true; + } + } + + access_mask = get_attr_access_mask(attr, ac->sd_flags); + + /* the access-mask should be non-zero. Skip attribute otherwise */ + if (access_mask == 0) { + DBG_ERR("Could not determine access mask for attribute %s\n", + attr_name); + return LDB_SUCCESS; + } + + ret = acl_check_access_on_attribute(ac->module, mem_ctx, sd, sid, + access_mask, attr, objectclass); + + if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) { + return ret; + } + + if (ret != LDB_SUCCESS) { + ldb_debug_set(ldb, LDB_DEBUG_FATAL, + "acl_read: %s check attr[%s] gives %s - %s\n", + ldb_dn_get_linearized(dn), attr_name, + ldb_strerror(ret), ldb_errstring(ldb)); + return ret; + } + + return LDB_SUCCESS; +} + +/* + * Returns the attribute name for this particular level of a search operation + * parse-tree. + */ +static const char * parse_tree_get_attr(struct ldb_parse_tree *tree) +{ + const char *attr = NULL; + + switch (tree->operation) { + case LDB_OP_EQUALITY: + case LDB_OP_GREATER: + case LDB_OP_LESS: + case LDB_OP_APPROX: + attr = tree->u.equality.attr; + break; + case LDB_OP_SUBSTRING: + attr = tree->u.substring.attr; + break; + case LDB_OP_PRESENT: + attr = tree->u.present.attr; + break; + case LDB_OP_EXTENDED: + attr = tree->u.extended.attr; + break; + + /* we'll check LDB_OP_AND/_OR/_NOT children later on in the walk */ + default: + break; + } + return attr; +} + +/* + * Checks a single attribute in the search parse-tree to make sure the user has + * sufficient rights to view it. + */ +static int parse_tree_check_attr_access(struct ldb_parse_tree *tree, + void *private_context) +{ + struct parse_tree_aclread_ctx *ctx = NULL; + const char *attr_name = NULL; + bool is_public_info = false; + int ret; + + ctx = (struct parse_tree_aclread_ctx *)private_context; + + /* + * we can skip any further checking if we already know that this object + * shouldn't be visible in this user's search + */ + if (ctx->suppress_result) { + return LDB_SUCCESS; + } + + /* skip this level of the search-tree if it has no attribute to check */ + attr_name = parse_tree_get_attr(tree); + if (attr_name == NULL) { + return LDB_SUCCESS; + } + + ret = check_attr_access_rights(ctx->mem_ctx, attr_name, ctx->ac, + ctx->sd, ctx->objectclass, ctx->sid, + ctx->dn, &is_public_info); + + /* + * if the user does not have the rights to view this attribute, then we + * should not return the object as a search result, i.e. act as if the + * object doesn't exist (for this particular user, at least) + */ + if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) { + + /* + * We make an exception for attribute=* searches involving the + * Public Information property-set. This allows searches like + * objectClass=* to return visible objects, even if the user + * doesn't have Read Property rights on the attribute + */ + if (tree->operation == LDB_OP_PRESENT && is_public_info) { + return LDB_SUCCESS; + } + + ctx->suppress_result = true; + return LDB_SUCCESS; + } + + return ret; +} + +/* + * Traverse the search-tree to check that the user has sufficient access rights + * to view all the attributes. + */ +static int check_search_ops_access(struct aclread_context *ac, + TALLOC_CTX *mem_ctx, + struct security_descriptor *sd, + const struct dsdb_class *objectclass, + struct dom_sid *sid, struct ldb_dn *dn, + bool *suppress_result) +{ + int ret; + struct parse_tree_aclread_ctx ctx = { 0 }; + struct ldb_parse_tree *tree = ac->req->op.search.tree; + + ctx.ac = ac; + ctx.mem_ctx = mem_ctx; + ctx.suppress_result = false; + ctx.sid = sid; + ctx.dn = dn; + ctx.sd = sd; + ctx.objectclass = objectclass; + + /* walk the search tree, checking each attribute as we go */ + ret = ldb_parse_tree_walk(tree, parse_tree_check_attr_access, &ctx); + + /* return whether this search result should be hidden to this user */ + *suppress_result = ctx.suppress_result; + return ret; +} static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) { @@ -241,6 +490,7 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) TALLOC_CTX *tmp_ctx; uint32_t instanceType; const struct dsdb_class *objectclass; + bool suppress_result = false; ac = talloc_get_type(req->context, struct aclread_context); ldb = ldb_module_get_ctx(ac->module); @@ -342,26 +592,8 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) aclread_mark_inaccesslible(&msg->elements[i]); continue; } - /* nTSecurityDescriptor is a special case */ - if (is_sd) { - access_mask = 0; - - if (ac->sd_flags & (SECINFO_OWNER|SECINFO_GROUP)) { - access_mask |= SEC_STD_READ_CONTROL; - } - if (ac->sd_flags & SECINFO_DACL) { - access_mask |= SEC_STD_READ_CONTROL; - } - if (ac->sd_flags & SECINFO_SACL) { - access_mask |= SEC_FLAG_SYSTEM_SECURITY; - } - } else { - access_mask = SEC_ADS_READ_PROP; - } - if (attr->searchFlags & SEARCH_FLAG_CONFIDENTIAL) { - access_mask |= SEC_ADS_CONTROL_ACCESS; - } + access_mask = get_attr_access_mask(attr, ac->sd_flags); if (access_mask == 0) { aclread_mark_inaccesslible(&msg->elements[i]); @@ -382,18 +614,14 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) * in anycase. */ if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) { - if (!ac->indirsync) { - /* - * do not return this entry if attribute is - * part of the search filter - */ - if (dsdb_attr_in_parse_tree(ac->req->op.search.tree, - msg->elements[i].name)) { - talloc_free(tmp_ctx); - return LDB_SUCCESS; - } - aclread_mark_inaccesslible(&msg->elements[i]); - } else { + bool in_search_filter; + + /* check if attr is part of the search filter */ + in_search_filter = dsdb_attr_in_parse_tree(ac->req->op.search.tree, + msg->elements[i].name); + + if (in_search_filter) { + /* * We are doing dirysnc answers * and the object shouldn't be returned (normally) @@ -402,13 +630,17 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) * (remove the object if it is not deleted, or return * just the objectGUID if it's deleted). */ - if (dsdb_attr_in_parse_tree(ac->req->op.search.tree, - msg->elements[i].name)) { + if (ac->indirsync) { ldb_msg_remove_attr(msg, "replPropertyMetaData"); break; } else { - aclread_mark_inaccesslible(&msg->elements[i]); + + /* do not return this entry */ + talloc_free(tmp_ctx); + return LDB_SUCCESS; } + } else { + aclread_mark_inaccesslible(&msg->elements[i]); } } else if (ret != LDB_SUCCESS) { ldb_debug_set(ldb, LDB_DEBUG_FATAL, @@ -420,6 +652,37 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) goto fail; } } + + /* + * check access rights for the search attributes, as well as the + * attribute values actually being returned + */ + ret = check_search_ops_access(ac, tmp_ctx, sd, objectclass, sid, + msg->dn, &suppress_result); + if (ret != LDB_SUCCESS) { + ldb_debug_set(ldb, LDB_DEBUG_FATAL, + "acl_read: %s check search ops %s - %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_strerror(ret), ldb_errstring(ldb)); + goto fail; + } + + if (suppress_result) { + + /* + * As per the above logic, we strip replPropertyMetaData + * out of the msg so that the dirysync module will do + * what is needed (return just the objectGUID if it's, + * deleted, or remove the object if it is not). + */ + if (ac->indirsync) { + ldb_msg_remove_attr(msg, "replPropertyMetaData"); + } else { + talloc_free(tmp_ctx); + return LDB_SUCCESS; + } + } + for (i=0; i < msg->num_elements; i++) { if (!aclread_is_inaccessible(&msg->elements[i])) { num_of_attrs++; diff --git a/source4/dsdb/tests/python/acl.py b/source4/dsdb/tests/python/acl.py index d5d069e3e6b..51e4dec8a94 100755 --- a/source4/dsdb/tests/python/acl.py +++ b/source4/dsdb/tests/python/acl.py @@ -998,6 +998,74 @@ class AclSearchTests(AclTests): res_list = res[0].keys() self.assertEquals(sorted(res_list), sorted(ok_list)) + def assert_search_on_attr(self, dn, samdb, attr, expected_list): + + expected_num = len(expected_list) + res = samdb.search(dn, expression="(%s=*)" % attr, scope=SCOPE_SUBTREE) + self.assertEquals(len(res), expected_num) + + res_list = [ x["dn"] for x in res if x["dn"] in expected_list ] + self.assertEquals(sorted(res_list), sorted(expected_list)) + + def test_search7(self): + """Checks object search visibility when users don't have full rights""" + self.create_clean_ou("OU=ou1," + self.base_dn) + mod = "(A;;LC;;;%s)(A;;LC;;;%s)" % (str(self.user_sid), + str(self.group_sid)) + self.sd_utils.dacl_add_ace("OU=ou1," + self.base_dn, mod) + tmp_desc = security.descriptor.from_sddl("D:(A;;RPWPCRCCDCLCLORCWOWDSDDTSW;;;DA)" + mod, + self.domain_sid) + self.ldb_admin.create_ou("OU=ou2,OU=ou1," + self.base_dn, sd=tmp_desc) + self.ldb_admin.create_ou("OU=ou3,OU=ou2,OU=ou1," + self.base_dn, + sd=tmp_desc) + self.ldb_admin.create_ou("OU=ou4,OU=ou2,OU=ou1," + self.base_dn, + sd=tmp_desc) + self.ldb_admin.create_ou("OU=ou5,OU=ou3,OU=ou2,OU=ou1," + self.base_dn, + sd=tmp_desc) + self.ldb_admin.create_ou("OU=ou6,OU=ou4,OU=ou2,OU=ou1," + self.base_dn, + sd=tmp_desc) + + ou2_dn = Dn(self.ldb_admin, "OU=ou2,OU=ou1," + self.base_dn) + ou1_dn = Dn(self.ldb_admin, "OU=ou1," + self.base_dn) + + # even though unprivileged users can't read these attributes for OU2, + # the object should still be visible in searches, because they have + # 'List Contents' rights still. This isn't really disclosive because + # ALL objects have these attributes + visible_attrs = ["objectClass", "distinguishedName", "name", + "objectGUID"] + two_objects = [ou2_dn, ou1_dn] + + for attr in visible_attrs: + # a regular user should just see the 2 objects + self.assert_search_on_attr(str(ou1_dn), self.ldb_user3, attr, + expected_list=two_objects) + + # whereas the following users have LC rights for all the objects, + # so they should see them all + self.assert_search_on_attr(str(ou1_dn), self.ldb_user, attr, + expected_list=self.full_list) + self.assert_search_on_attr(str(ou1_dn), self.ldb_user2, attr, + expected_list=self.full_list) + + # however when searching on the following attributes, objects will not + # be visible unless the user has Read Property rights + hidden_attrs = ["objectCategory", "instanceType", "ou", "uSNChanged", + "uSNCreated", "whenCreated"] + one_object = [ou1_dn] + + for attr in hidden_attrs: + self.assert_search_on_attr(str(ou1_dn), self.ldb_user3, attr, + expected_list=one_object) + self.assert_search_on_attr(str(ou1_dn), self.ldb_user, attr, + expected_list=one_object) + self.assert_search_on_attr(str(ou1_dn), self.ldb_user2, attr, + expected_list=one_object) + + # admin has RP rights so can still see all the objects + self.assert_search_on_attr(str(ou1_dn), self.ldb_admin, attr, + expected_list=self.full_list) + #tests on ldap delete operations class AclDeleteTests(AclTests): diff --git a/source4/dsdb/tests/python/confidential_attr.py b/source4/dsdb/tests/python/confidential_attr.py new file mode 100755 index 00000000000..1e1cf6c48f8 --- /dev/null +++ b/source4/dsdb/tests/python/confidential_attr.py @@ -0,0 +1,1025 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# +# Tests that confidential attributes (or attributes protected by a ACL that +# denies read access) cannot be guessed through wildcard DB searches. +# +# Copyright (C) Catalyst.Net Ltd +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# +import optparse +import sys +sys.path.insert(0, "bin/python") + +import samba +import os +from samba.tests.subunitrun import SubunitOptions, TestProgram +import samba.getopt as options +from ldb import SCOPE_BASE, SCOPE_SUBTREE +from samba.dsdb import SEARCH_FLAG_CONFIDENTIAL, SEARCH_FLAG_PRESERVEONDELETE +from ldb import Message, MessageElement, Dn +from ldb import FLAG_MOD_REPLACE, FLAG_MOD_ADD +from samba.auth import system_session +from samba import gensec, sd_utils +from samba.samdb import SamDB +from samba.credentials import Credentials, DONT_USE_KERBEROS +import samba.tests +from samba.tests import delete_force +import samba.dsdb + +parser = optparse.OptionParser("confidential_attr.py [options] <host>") +sambaopts = options.SambaOptions(parser) +parser.add_option_group(sambaopts) +parser.add_option_group(options.VersionOptions(parser)) + +# use command line creds if available +credopts = options.CredentialsOptions(parser) +parser.add_option_group(credopts) +subunitopts = SubunitOptions(parser) +parser.add_option_group(subunitopts) + +opts, args = parser.parse_args() + +if len(args) < 1: + parser.print_usage() + sys.exit(1) + +host = args[0] +if "://" not in host: + ldaphost = "ldap://%s" % host +else: + ldaphost = host + start = host.rindex("://") + host = host.lstrip(start + 3) + +lp = sambaopts.get_loadparm() +creds = credopts.get_credentials(lp) +creds.set_gensec_features(creds.get_gensec_features() | gensec.FEATURE_SEAL) + +# When a user does not have access rights to view the objects' attributes, +# Windows and Samba behave slightly differently. +# A windows DC will always act as if the hidden attribute doesn't exist AT ALL +# (for an unprivileged user). So, even for a user that lacks access rights, +# the inverse/'!' queries should return ALL objects. This is similar to the +# kludgeaclredacted behaviour on Samba. +# However, on Samba (for implementation simplicity) we never return a matching +# result for an unprivileged user. +# Either approach is OK, so long as it gets applied consistently and we don't +# disclose any sensitive details by varying what gets returned by the search. +DC_MODE_RETURN_NONE = 0 +DC_MODE_RETURN_ALL = 1 + +# +# Tests start here +# +class ConfidentialAttrCommon(samba.tests.TestCase): + + def setUp(self): + super(ConfidentialAttrCommon, self).setUp() + + self.ldb_admin = SamDB(ldaphost, credentials=creds, + session_info=system_session(lp), lp=lp) + self.user_pass = "samba123@" + self.base_dn = self.ldb_admin.domain_dn() + self.schema_dn = self.ldb_admin.get_schema_basedn() + self.sd_utils = sd_utils.SDUtils(self.ldb_admin) + + # the tests work by setting the 'Confidential' bit in the searchFlags + # for an existing schema attribute. This only works against Windows if + # the systemFlags does not have FLAG_SCHEMA_BASE_OBJECT set for the + # schema attribute being modified. There are only a few attributes that + # meet this criteria (most of which only apply to 'user' objects) + self.conf_attr = "homePostalAddress" + attr_cn = "CN=Address-Home" + # schemaIdGuid for homePostalAddress (used for ACE tests) + self.conf_attr_guid = "16775781-47f3-11d1-a9c3-0000f80367c1" + self.conf_attr_sec_guid = "77b5b886-944a-11d1-aebd-0000f80367c1" + self.attr_dn = "{},{}".format(attr_cn, self.schema_dn) + + userou = "OU=conf-attr-test" + self.ou = "{},{}".format(userou, self.base_dn) + self.ldb_admin.create_ou(self.ou) + + # use a common username prefix, so we can use sAMAccountName=CATC-* as + # a search filter to only return the users we're interested in + self.user_prefix = "catc-" + + # add a test object with this attribute set + self.conf_value = "abcdef" + self.conf_user = "{}conf-user".format(self.user_prefix) + self.ldb_admin.newuser(self.conf_user, self.user_pass, userou=userou) + self.conf_dn = self.get_user_dn(self.conf_user) + self.add_attr(self.conf_dn, self.conf_attr, self.conf_value) + + # add a sneaky user that will try to steal our secrets + self.user = "{}sneaky-user".format(self.user_prefix) + self.ldb_admin.newuser(self.user, self.user_pass, userou=userou) + self.ldb_user = self.get_ldb_connection(self.user, self.user_pass) + + self.all_users = [self.user, self.conf_user] + + # add some other users that also have confidential attributes, so we can + # check we don't disclose their details, particularly in '!' searches + for i in range(1, 3): + username = "{}other-user{}".format(self.user_prefix, i) + self.ldb_admin.newuser(username, self.user_pass, userou=userou) + userdn = self.get_user_dn(username) + self.add_attr(userdn, self.conf_attr, "xyz{}".format(i)) + self.all_users.append(username) + + # there are 4 users in the OU, plus the OU itself + self.test_dn = self.ou + self.total_objects = len(self.all_users) + 1 + self.objects_with_attr = 3 + + # sanity-check the flag is not already set (this'll cause problems if + # previous test run didn't clean up properly) + search_flags = self.get_attr_search_flags(self.attr_dn) + self.assertTrue(int(search_flags) & SEARCH_FLAG_CONFIDENTIAL == 0, + "{} searchFlags already {}".format(self.conf_attr, + search_flags)) + + def tearDown(self): + super(ConfidentialAttrCommon, self).tearDown() + self.ldb_admin.delete(self.ou, ["tree_delete:1"]) + + def add_attr(self, dn, attr, value): + m = Message() + m.dn = Dn(self.ldb_admin, dn) + m[attr] = MessageElement(value, FLAG_MOD_ADD, attr) + self.ldb_admin.modify(m) + + def set_schema_update_now(self): + ldif = """ +dn: +changetype: modify +add: schemaUpdateNow +schemaUpdateNow: 1 +""" + self.ldb_admin.modify_ldif(ldif) + + def set_attr_search_flags(self, attr_dn, flags): + """Modifies the searchFlags for an object in the schema""" + m = Message() + m.dn = Dn(self.ldb_admin, attr_dn) + m['searchFlags'] = MessageElement(flags, FLAG_MOD_REPLACE, + 'searchFlags') + self.ldb_admin.modify(m) + + # note we have to update the schema for this change to take effect (on + # Windows, at least) + self.set_schema_update_now() + + def get_attr_search_flags(self, attr_dn): + """Marks the attribute under test as being confidential""" + res = self.ldb_admin.search(attr_dn, scope=SCOPE_BASE, + attrs=['searchFlags']) + return res[0]['searchFlags'][0] + + def make_attr_confidential(self): + """Marks the attribute under test as being confidential""" + + # work out the original 'searchFlags' value before we overwrite it + old_value = self.get_attr_search_flags(self.attr_dn) + + self.set_attr_search_flags(self.attr_dn, str(SEARCH_FLAG_CONFIDENTIAL)) + + # reset the value after the test completes + self.addCleanup(self.set_attr_search_flags, self.attr_dn, old_value) + + # The behaviour of the DC can differ in some cases, depending on whether + # we're talking to a Windows DC or a Samba DC + def guess_dc_mode(self): + # if we're in selftest, we can be pretty sure it's a Samba DC + if os.environ.get('SAMBA_SELFTEST') == '1': + return DC_MODE_RETURN_NONE + + searches = self.get_negative_match_all_searches() + res = self.ldb_user.search(self.test_dn, expression=searches[0], + scope=SCOPE_SUBTREE) + + # we default to DC_MODE_RETURN_NONE (samba).Update this if it + # looks like we're talking to a Windows DC + if len(res) == self.total_objects: + return DC_MODE_RETURN_ALL + + # otherwise assume samba DC behaviour + return DC_MODE_RETURN_NONE + + def get_user_dn(self, name): + return "CN={},{}".format(name, self.ou) + + def get_user_sid_string(self, username): + user_dn = self.get_user_dn(username) + user_sid = self.sd_utils.get_object_sid(user_dn) + return str(user_sid) + + def get_ldb_connection(self, target_username, target_password): + creds_tmp = Credentials() + creds_tmp.set_username(target_username) + creds_tmp.set_password(target_password) + creds_tmp.set_domain(creds.get_domain()) + creds_tmp.set_realm(creds.get_realm()) + creds_tmp.set_workstation(creds.get_workstation()) + features = creds_tmp.get_gensec_features() | gensec.FEATURE_SEAL + creds_tmp.set_gensec_features(features) + creds_tmp.set_kerberos_state(DONT_USE_KERBEROS) + ldb_target = SamDB(url=ldaphost, credentials=creds_tmp, lp=lp) + return ldb_target + + def assert_not_in_result(self, res, exclude_dn): + for msg in res: + self.assertNotEqual(msg.dn, exclude_dn, + "Search revealed object {}".format(exclude_dn)) + + def assert_search_result(self, expected_num, expr, samdb): + + # try asking for different attributes back: None/all, the confidential + # attribute itself, and a random unrelated attribute + attr_filters = [None, ["*"], [self.conf_attr], ['name']] + for attr in attr_filters: + res = samdb.search(self.test_dn, expression=expr, + scope=SCOPE_SUBTREE, attrs=attr) + self.assertTrue(len(res) == expected_num, + "%u results, not %u for search %s, attr %s" % + (len(res), expected_num, expr, str(attr))) + + # return a selection of searches that match exactly against the test object + def get_exact_match_searches(self): + first_char = self.conf_value[:1] + last_char = self.conf_value[-1:] + test_attr = self.conf_attr + + searches = [ + # search for the attribute using a sub-string wildcard + # (which could reveal the attribute's actual value) + "({}={}*)".format(test_attr, first_char), + "({}=*{})".format(test_attr, last_char), + + # sanity-check equality against an exact match on value + "({}={})".format(test_attr, self.conf_value), + + # '~=' searches don't work against Samba + # sanity-check an approx search against an exact match on value + # "({}~={})".format(test_attr, self.conf_value), + + # check wildcard in an AND search... + "(&({}={}*)(objectclass=*))".format(test_attr, first_char), + + # ...an OR search (against another term that will never match) + "(|({}={}*)(objectclass=banana))".format(test_attr, first_char)] + + return searches + + # return searches that match any object with the attribute under test + def get_match_all_searches(self): + searches = [ + # check a full wildcard against the confidential attribute + # (which could reveal the attribute's presence/absence) + "({}=*)".format(self.conf_attr), + + # check wildcard in an AND search... + "(&(objectclass=*)({}=*))".format(self.conf_attr), + + # ...an OR search (against another term that will never match) + "(|(objectclass=banana)({}=*))".format(self.conf_attr), + + # check <=, and >= expressions that would normally find a match + "({}>=0)".format(self.conf_attr), + "({}<=ZZZZZZZZZZZ)".format(self.conf_attr)] + + return searches + + def assert_conf_attr_searches(self, has_rights_to=0, samdb=None): + """Check searches against the attribute under test work as expected""" + + if samdb is None: + samdb = self.ldb_user + + if has_rights_to == "all": + has_rights_to = self.objects_with_attr + + # these first few searches we just expect to match against the one + # object under test that we're trying to guess the value of + expected_num = 1 if has_rights_to > 0 else 0 + for search in self.get_exact_match_searches(): + self.assert_search_result(expected_num, search, samdb) + + # these next searches will match any objects we have rights to see + expected_num = has_rights_to + for search in self.get_match_all_searches(): + self.assert_search_result(expected_num, search, samdb) + + # The following are double negative searches (i.e. NOT non-matching- + # condition) which will therefore match ALL objects, including the test + # object(s). + def get_negative_match_all_searches(self): + first_char = self.conf_value[:1] + last_char = self.conf_value[-1:] + not_first_char = chr(ord(first_char) + 1) + not_last_char = chr(ord(last_char) + 1) + + searches = [ + "(!({}={}*))".format(self.conf_attr, not_first_char), + "(!({}=*{}))".format(self.conf_attr, not_last_char)] + return searches + + # the following searches will not match against the test object(s). So + # a user with sufficient rights will see an inverse sub-set of objects. + # (An unprivileged user would either see all objects on Windows, or no + # objects on Samba) + def get_inverse_match_searches(self): + first_char = self.conf_value[:1] + last_char = self.conf_value[-1:] + searches = [ + "(!({}={}*))".format(self.conf_attr, first_char), + "(!({}=*{}))".format(self.conf_attr, last_char)] + return searches + + def negative_searches_all_rights(self, total_objects=None): + expected_results = {} + + if total_objects is None: + total_objects = self.total_objects + + # these searches should match ALL objects (including the OU) + for search in self.get_negative_match_all_searches(): + expected_results[search] = total_objects + + # a ! wildcard should only match the objects without the attribute + search = "(!({}=*))".format(self.conf_attr) + expected_results[search] = total_objects - self.objects_with_attr + + # whereas the inverse searches should match all objects *except* the + # one under test + for search in self.get_inverse_match_searches(): + expected_results[search] = total_objects - 1 + + return expected_results + + # Returns the expected negative (i.e. '!') search behaviour when talking to + # a DC with DC_MODE_RETURN_ALL behaviour, i.e. we assert that users + # without rights always see ALL objects in '!' searches + def negative_searches_return_all(self, has_rights_to=0, + total_objects=None): + """Asserts user without rights cannot see objects in '!' searches""" + expected_results = {} + + if total_objects is None: + total_objects = self.total_objects + + # Windows 'hides' objects by always returning all of them, so negative + # searches that match all objects will simply return all objects + for search in self.get_negative_match_all_searches(): + expected_results[search] = total_objects + + # if the search is matching on an inverse subset (everything except the + # object under test), the + inverse_searches = self.get_inverse_match_searches() + inverse_searches += ["(!({}=*))".format(self.conf_attr)] + + for search in inverse_searches: + expected_results[search] = total_objects - has_rights_to + + return expected_results + + # Returns the expected negative (i.e. '!') search behaviour when talking to + # a DC with DC_MODE_RETURN_NONE behaviour, i.e. we assert that users + # without rights cannot see objects in '!' searches at all + def negative_searches_return_none(self, has_rights_to=0): + expected_results = {} + + # the 'match-all' searches should only return the objects we have + # access rights to (if any) + for search in self.get_negative_match_all_searches(): + expected_results[search] = has_rights_to + + # for inverse matches, we should NOT be told about any objects at all + inverse_searches = self.get_inverse_match_searches() + inverse_searches += ["(!({}=*))".format(self.conf_attr)] + for search in inverse_searches: + expected_results[search] = 0 + + return expected_results + + # Returns the expected negative (i.e. '!') search behaviour. This varies + # depending on what type of DC we're talking to (i.e. Windows or Samba) + # and what access rights the user has + def negative_search_expected_results(self, has_rights_to, dc_mode, + total_objects=None): + + if has_rights_to == "all": + expect_results = self.negative_searches_all_rights(total_objects) + + # if it's a Samba DC, we only expect the 'match-all' searches to return + # the objects that we have access rights to (all others are hidden). + # Whereas Windows 'hides' the objects by always returning all of them + elif dc_mode == DC_MODE_RETURN_NONE: + expect_results = self.negative_searches_return_none(has_rights_to) + else: + expect_results = self.negative_searches_return_all(has_rights_to, + total_objects) + return expect_results + + def assert_negative_searches(self, has_rights_to=0, + dc_mode=DC_MODE_RETURN_NONE, samdb=None): + """Asserts user without rights cannot see objects in '!' searches""" + + if samdb is None: + samdb = self.ldb_user + + # build a dictionary of key=search-expr, value=expected_num assertions + expected_results = self.negative_search_expected_results(has_rights_to, + dc_mode) + + for search, expected_num in expected_results.items(): + self.assert_search_result(expected_num, search, samdb) + + def assert_attr_returned(self, expect_attr, samdb, attrs): + # does a query that should always return a successful result, and + # checks whether the confidential attribute is present + res = samdb.search(self.conf_dn, expression="(objectClass=*)", + scope=SCOPE_SUBTREE, attrs=attrs) + self.assertTrue(len(res) == 1) + + attr_returned = False + for msg in res: + if self.conf_attr in msg: + attr_returned = True + self.assertEqual(expect_attr, attr_returned) + + def assert_attr_visible(self, expect_attr, samdb=None): + if samdb is None: + samdb = self.ldb_user + + # sanity-check confidential attribute is/isn't returned as expected + # based on the filter attributes we ask for + self.assert_attr_returned(expect_attr, samdb, attrs=None) + self.assert_attr_returned(expect_attr, samdb, attrs=["*"]) + self.assert_attr_returned(expect_attr, samdb, attrs=[self.conf_attr]) + + # filtering on a different attribute should never return the conf_attr + self.assert_attr_returned(expect_attr=False, samdb=samdb, + attrs=['name']) + + def assert_attr_visible_to_admin(self): + # sanity-check the admin user can always see the confidential attribute + self.assert_conf_attr_searches(has_rights_to="all", samdb=self.ldb_admin) + self.assert_negative_searches(has_rights_to="all", samdb=self.ldb_admin) + self.assert_attr_visible(expect_attr=True, samdb=self.ldb_admin) + + +class ConfidentialAttrTest(ConfidentialAttrCommon): + def test_basic_search(self): + """Basic test confidential attributes aren't disclosed via searches""" + + # check we can see a non-confidential attribute in a basic searches + self.assert_conf_attr_searches(has_rights_to="all") + self.assert_negative_searches(has_rights_to="all") + self.assert_attr_visible(expect_attr=True) + + # now make the attribute confidential. Repeat the tests and check that + # an ordinary user can't see the attribute, or indirectly match on the + # attribute via the search expression + self.make_attr_confidential() + + self.assert_conf_attr_searches(has_rights_to=0) + dc_mode = self.guess_dc_mode() + self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + self.assert_attr_visible(expect_attr=False) + + # sanity-check we haven't hidden the attribute from the admin as well + self.assert_attr_visible_to_admin() + + def _test_search_with_allow_acl(self, allow_ace): + """Checks a ACE with 'CR' rights can override a confidential attr""" + # make the test attribute confidential and check user can't see it + self.make_attr_confidential() + + self.assert_conf_attr_searches(has_rights_to=0) + dc_mode = self.guess_dc_mode() + self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + self.assert_attr_visible(expect_attr=False) + + # apply the allow ACE to the object under test + self.sd_utils.dacl_add_ace(self.conf_dn, allow_ace) + + # the user should now be able to see the attribute for the one object + # we gave it rights to + self.assert_conf_attr_searches(has_rights_to=1) + self.assert_negative_searches(has_rights_to=1, dc_mode=dc_mode) + self.assert_attr_visible(expect_attr=True) + + # sanity-check the admin can still see the attribute + self.assert_attr_visible_to_admin() + + def test_search_with_attr_acl_override(self): + """Make the confidential attr visible via an OA attr ACE""" + + # set the SEC_ADS_CONTROL_ACCESS bit ('CR') for the user for the + # attribute under test, so the user can see it once more + user_sid = self.get_user_sid_string(self.user) + ace = "(OA;;CR;{};;{})".format(self.conf_attr_guid, user_sid) + + self._test_search_with_allow_acl(ace) + + def test_search_with_propset_acl_override(self): + """Make the confidential attr visible via a Property-set ACE""" + + # set the SEC_ADS_CONTROL_ACCESS bit ('CR') for the user for the + # property-set containing the attribute under test (i.e. the + # attributeSecurityGuid), so the user can see it once more + user_sid = self.get_user_sid_string(self.user) + ace = "(OA;;CR;{};;{})".format(self.conf_attr_sec_guid, user_sid) + + self._test_search_with_allow_acl(ace) + + def test_search_with_acl_override(self): + """Make the confidential attr visible via a general 'allow' ACE""" + + # set the allow SEC_ADS_CONTROL_ACCESS bit ('CR') for the user + user_sid = self.get_user_sid_string(self.user) + ace = "(A;;CR;;;{})".format(user_sid) + + self._test_search_with_allow_acl(ace) + + def test_search_with_blanket_oa_acl(self): + """Make the confidential attr visible via a non-specific OA ACE""" + + # this just checks that an Object Access (OA) ACE without a GUID + # specified will work the same as an 'Access' (A) ACE + user_sid = self.get_user_sid_string(self.user) + ace = "(OA;;CR;;;{})".format(user_sid) + + self._test_search_with_allow_acl(ace) + + def _test_search_with_neutral_acl(self, neutral_ace): + """Checks that a user does NOT gain access via an unrelated ACE""" + + # make the test attribute confidential and check user can't see it + self.make_attr_confidential() + + self.assert_conf_attr_searches(has_rights_to=0) + dc_mode = self.guess_dc_mode() + self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + self.assert_attr_visible(expect_attr=False) + + # apply the ACE to the object under test + self.sd_utils.dacl_add_ace(self.conf_dn, neutral_ace) + + # this should make no difference to the user's ability to see the attr + self.assert_conf_attr_searches(has_rights_to=0) + self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + self.assert_attr_visible(expect_attr=False) + + # sanity-check the admin can still see the attribute + self.assert_attr_visible_to_admin() + + def test_search_with_neutral_acl(self): + """Give the user all rights *except* CR for any attributes""" + + # give the user all rights *except* CR and check it makes no difference + user_sid = self.get_user_sid_string(self.user) + ace = "(A;;RPWPCCDCLCLORCWOWDSDDTSW;;;{})".format(user_sid) + self._test_search_with_neutral_acl(ace) + + def test_search_with_neutral_attr_acl(self): + """Give the user all rights *except* CR for the attribute under test""" + + # giving user all OA rights *except* CR should make no difference + user_sid = self.get_user_sid_string(self.user) + rights = "RPWPCCDCLCLORCWOWDSDDTSW" + ace = "(OA;;{};{};;{})".format(rights, self.conf_attr_guid, user_sid) + self._test_search_with_neutral_acl(ace) + + def test_search_with_neutral_cr_acl(self): + """Give the user CR rights for *another* unrelated attribute""" + + # giving user object-access CR rights to an unrelated attribute + user_sid = self.get_user_sid_string(self.user) + # use the GUID for sAMAccountName here (for no particular reason) + unrelated_attr = "3e0abfd0-126a-11d0-a060-00aa006c33ed" + ace = "(OA;;CR;{};;{})".format(unrelated_attr, user_sid) + self._test_search_with_neutral_acl(ace) + + +# Check that a Deny ACL on an attribute doesn't reveal confidential info +class ConfidentialAttrTestDenyAcl(ConfidentialAttrCommon): + + def assert_not_in_result(self, res, exclude_dn): + for msg in res: + self.assertNotEqual(msg.dn, exclude_dn, + "Search revealed object {}".format(exclude_dn)) + + # deny ACL tests are slightly different as we are only denying access to + # the one object under test (rather than any objects with that attribute). + # Therefore we need an extra check that we don't reveal the test object + # in the search, if we're not supposed to + def assert_search_result(self, expected_num, expr, samdb, + excl_testobj=False): + + # try asking for different attributes back: None/all, the confidential + # attribute itself, and a random unrelated attribute + attr_filters = [None, ["*"], [self.conf_attr], ['name']] + for attr in attr_filters: + res = samdb.search(self.test_dn, expression=expr, + scope=SCOPE_SUBTREE, attrs=attr) + self.assertTrue(len(res) == expected_num, + "%u results, not %u for search %s, attr %s" % + (len(res), expected_num, expr, str(attr))) + + # assert we haven't revealed the hidden test-object + if excl_testobj: + self.assert_not_in_result(res, exclude_dn=self.conf_dn) + + # we make a few tweaks to the regular version of this function to cater to + # denying specifically one object via an ACE + def assert_conf_attr_searches(self, has_rights_to=0, samdb=None): + """Check searches against the attribute under test work as expected""" + + if samdb is None: + samdb = self.ldb_user + + # make sure the test object is not returned if we've been denied rights + # to it via an ACE + excl_testobj = True if has_rights_to == "deny-one" else False + + # these first few searches we just expect to match against the one + # object under test that we're trying to guess the value of + expected_num = 1 if has_rights_to == "all" else 0 + + for search in self.get_exact_match_searches(): + self.assert_search_result(expected_num, search, samdb, + excl_testobj) + + # these next searches will match any objects with the attribute that + # we have rights to see (i.e. all except the object under test) + if has_rights_to == "all": + expected_num = self.objects_with_attr + elif has_rights_to == "deny-one": + expected_num = self.objects_with_attr - 1 + + for search in self.get_match_all_searches(): + self.assert_search_result(expected_num, search, samdb, + excl_testobj) + + def negative_searches_return_none(self, has_rights_to=0): + expected_results = {} + + # on Samba we will see the objects we have rights to, but the one we + # are denied access to will be hidden + searches = self.get_negative_match_all_searches() + searches += self.get_inverse_match_searches() + for search in searches: + expected_results[search] = self.total_objects - 1 + + # The wildcard returns the objects without this attribute as normal. + search = "(!({}=*))".format(self.conf_attr) + expected_results[search] = self.total_objects - self.objects_with_attr + return expected_results + + def negative_searches_return_all(self, has_rights_to=0, + total_objects=None): + expected_results = {} + + # When a user lacks access rights to an object, Windows 'hides' it in + # '!' searches by always returning it, regardless of whether it matches + searches = self.get_negative_match_all_searches() + searches += self.get_inverse_match_searches() + for search in searches: + expected_results[search] = self.total_objects + + # in the wildcard case, the one object we don't have rights to gets + # bundled in with the objects that don't have the attribute at all + search = "(!({}=*))".format(self.conf_attr) + has_rights_to = self.objects_with_attr - 1 + expected_results[search] = self.total_objects - has_rights_to + return expected_results + + def assert_negative_searches(self, has_rights_to=0, + dc_mode=DC_MODE_RETURN_NONE, samdb=None): + """Asserts user without rights cannot see objects in '!' searches""" + + if samdb is None: + samdb = self.ldb_user + + # As the deny ACL is only denying access to one particular object, add + # an extra check that the denied object is not returned. (We can only + # assert this if the '!'/negative search behaviour is to suppress any + # objects we don't have access rights to) + excl_testobj = False + if has_rights_to != "all" and dc_mode == DC_MODE_RETURN_NONE: + excl_testobj = True + + # build a dictionary of key=search-expr, value=expected_num assertions + expected_results = self.negative_search_expected_results(has_rights_to, + dc_mode) + + for search, expected_num in expected_results.items(): + self.assert_search_result(expected_num, search, samdb, + excl_testobj=excl_testobj) + + def _test_search_with_deny_acl(self, ace): + # check the user can see the attribute initially + self.assert_conf_attr_searches(has_rights_to="all") + self.assert_negative_searches(has_rights_to="all") + self.assert_attr_visible(expect_attr=True) + + # add the ACE that denies access to the attr under test + self.sd_utils.dacl_add_ace(self.conf_dn, ace) + + # 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() + self.assert_negative_searches(has_rights_to="deny-one", + dc_mode=dc_mode) + self.assert_attr_visible(expect_attr=False) + + # sanity-check we haven't hidden the attribute from the admin as well + self.assert_attr_visible_to_admin() + + def test_search_with_deny_attr_acl(self): + """Checks a deny ACE works the same way as a confidential attribute""" + + # add an ACE that denies the user Read Property (RP) access to the attr + # (which is similar to making the attribute confidential) + user_sid = self.get_user_sid_string(self.user) + ace = "(OD;;RP;{};;{})".format(self.conf_attr_guid, user_sid) + + # check the user cannot see the attribute anymore + self._test_search_with_deny_acl(ace) + + def test_search_with_deny_acl(self): + """Checks a blanket deny ACE denies access to an object's attributes""" + + # add an blanket deny ACE for Read Property (RP) rights + user_dn = self.get_user_dn(self.user) + user_sid = self.sd_utils.get_object_sid(user_dn) + ace = "(D;;RP;;;{})".format(str(user_sid)) + + # check the user cannot see the attribute anymore + self._test_search_with_deny_acl(ace) + + def test_search_with_deny_propset_acl(self): + """Checks a deny ACE on the attribute's Property-Set""" + + # add an blanket deny ACE for Read Property (RP) rights + user_sid = self.get_user_sid_string(self.user) + ace = "(OD;;RP;{};;{})".format(self.conf_attr_sec_guid, user_sid) + + # check the user cannot see the attribute anymore + self._test_search_with_deny_acl(ace) + + def test_search_with_blanket_oa_deny_acl(self): + """Checks a non-specific 'OD' ACE works the same as a 'D' ACE""" + + # this just checks that adding a 'Object Deny' (OD) ACE without + # specifying a GUID will work the same way as a 'Deny' (D) ACE + user_sid = self.get_user_sid_string(self.user) + ace = "(OD;;RP;;;{})".format(user_sid) + + # check the user cannot see the attribute anymore + self._test_search_with_deny_acl(ace) + + +# Check that using the dirsync controls doesn't reveal confidential attributes +class ConfidentialAttrTestDirsync(ConfidentialAttrCommon): + + def setUp(self): + super(ConfidentialAttrTestDirsync, self).setUp() + self.dirsync = ["dirsync:1:1:1000"] + + # because we need to search on the base DN when using the dirsync + # controls, we need an extra filter for the inverse ('!') search, + # so we don't get thousands of objects returned + self.extra_filter = \ + "(&(samaccountname={}*)(!(isDeleted=*)))".format(self.user_prefix) + self.single_obj_filter = \ + "(&(samaccountname={})(!(isDeleted=*)))".format(self.conf_user) + + self.attr_filters = [None, ["*"], ["name"]] + + # Note dirsync behaviour is slighty different for the attribute under + # test - when you have full access rights, it only returns the objects + # that actually have this attribute (i.e. it doesn't return an empty + # message with just the DN). So we add the 'name' attribute into the + # attribute filter to avoid complicating our assertions further + self.attr_filters += [[self.conf_attr, "name"]] + + def assert_search_result(self, expected_num, expr, samdb, base_dn=None): + + # Note dirsync must always search on the partition base DN + if base_dn is None: + base_dn = self.base_dn + + # we need an extra filter for dirsync because: + # - we search on the base DN, so otherwise the '!' searches return + # thousands of unrelated results, and + # - we make the test attribute preserve-on-delete in one case, so we + # want to weed out results from any previous test runs + search = "(&{}{})".format(expr, self.extra_filter) + + for attr in self.attr_filters: + res = samdb.search(base_dn, expression=search, scope=SCOPE_SUBTREE, + attrs=attr, controls=self.dirsync) + self.assertTrue(len(res) == expected_num, + "%u results, not %u for search %s, attr %s" % + (len(res), expected_num, search, str(attr))) + + def assert_attr_returned(self, expect_attr, samdb, attrs, + no_result_ok=False): + + # When using dirsync, the base DN we search on needs to be a naming + # context. Add an extra filter to ignore all the objects we aren't + # interested in + expr = self.single_obj_filter + res = samdb.search(self.base_dn, expression=expr, scope=SCOPE_SUBTREE, + attrs=attrs, controls=self.dirsync) + self.assertTrue(len(res) == 1 or no_result_ok) + + attr_returned = False + for msg in res: + if self.conf_attr in msg and len(msg[self.conf_attr]) > 0: + attr_returned = True + self.assertEqual(expect_attr, attr_returned) + + def assert_attr_visible(self, expect_attr, samdb=None): + if samdb is None: + samdb = self.ldb_user + + # sanity-check confidential attribute is/isn't returned as expected + # based on the filter attributes we ask for + self.assert_attr_returned(expect_attr, samdb, attrs=None) + self.assert_attr_returned(expect_attr, samdb, attrs=["*"]) + + if expect_attr: + self.assert_attr_returned(expect_attr, samdb, + attrs=[self.conf_attr]) + else: + # The behaviour with dirsync when asking solely for an attribute + # that you don't have rights to is a bit strange. Samba returns + # no result rather than an empty message with just the DN. + # Presumably this is due to dirsync module behaviour. It's not + # disclosive in that the DC behaves the same way as if you asked + # for a garbage/non-existent attribute + self.assert_attr_returned(expect_attr, samdb, + attrs=[self.conf_attr], + no_result_ok=True) + self.assert_attr_returned(expect_attr, samdb, + attrs=["garbage"], no_result_ok=True) + + # filtering on a different attribute should never return the conf_attr + self.assert_attr_returned(expect_attr=False, samdb=samdb, + attrs=['name']) + + def assert_negative_searches(self, has_rights_to=0, + dc_mode=DC_MODE_RETURN_NONE, samdb=None): + """Asserts user without rights cannot see objects in '!' searches""" + + if samdb is None: + samdb = self.ldb_user + + # because dirsync uses an extra filter, the total objects we expect + # here only includes the user objects (not the parent OU) + total_objects = len(self.all_users) + expected_results = self.negative_search_expected_results(has_rights_to, + dc_mode, + total_objects) + + for search, expected_num in expected_results.items(): + self.assert_search_result(expected_num, search, samdb) + + def test_search_with_dirsync(self): + """Checks dirsync controls don't reveal confidential attributes""" + + self.assert_conf_attr_searches(has_rights_to="all") + self.assert_attr_visible(expect_attr=True) + self.assert_negative_searches(has_rights_to="all") + + # make the test attribute confidential and check user can't see it, + # even if they use the dirsync controls + self.make_attr_confidential() + + self.assert_conf_attr_searches(has_rights_to=0) + self.assert_attr_visible(expect_attr=False) + dc_mode = self.guess_dc_mode() + 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 + self.assert_conf_attr_searches(has_rights_to="all", + samdb=self.ldb_admin) + self.assert_attr_visible(expect_attr=True, samdb=self.ldb_admin) + self.assert_negative_searches(has_rights_to="all", + samdb=self.ldb_admin) + + def get_guid(self, dn): + """Returns an object's GUID (in string format)""" + res = self.ldb_admin.search(base=dn, attrs=["objectGUID"], + scope=SCOPE_BASE) + guid = res[0]['objectGUID'][0] + return self.ldb_admin.schema_format_value("objectGUID", guid) + + def make_attr_preserve_on_delete(self): + """Marks the attribute under test as being preserve on delete""" + + # work out the original 'searchFlags' value before we overwrite it + search_flags = int(self.get_attr_search_flags(self.attr_dn)) + + # check we've already set the confidential flag + self.assertTrue(search_flags & SEARCH_FLAG_CONFIDENTIAL != 0) + search_flags |= SEARCH_FLAG_PRESERVEONDELETE + + self.set_attr_search_flags(self.attr_dn, str(search_flags)) + + def change_attr_under_test(self, attr_name, attr_cn): + # change the attribute that the test code uses + self.conf_attr = attr_name + self.attr_dn = "{},{}".format(attr_cn, self.schema_dn) + + # set the new attribute for the user-under-test + self.add_attr(self.conf_dn, self.conf_attr, self.conf_value) + + # 2 other users also have the attribute-under-test set (to a randomish + # value). Set the new attribute for them now (normally this gets done + # in the setUp()) + for username in self.all_users: + if "other-user" in username: + dn = self.get_user_dn(username) + self.add_attr(dn, self.conf_attr, "xyz-blah") + + def test_search_with_dirsync_deleted_objects(self): + """Checks dirsync doesn't reveal confidential info for deleted objs""" + + # change the attribute we're testing (we'll preserve on delete for this + # test case, which means the attribute-under-test hangs around after + # the test case finishes, and would interfere with the searches for + # subsequent other test cases) + self.change_attr_under_test("carLicense", "CN=carLicense") + + # Windows dirsync behaviour is a little strange when you request + # attributes that deleted objects no longer have, so just request 'all + # attributes' to simplify the test logic + self.attr_filters = [None, ["*"]] + + # normally dirsync uses extra filters to exclude deleted objects that + # we're not interested in. Override these filters so they WILL include + # deleted objects, but only from this particular test run. We can do + # this by matching lastKnownParent against this test case's OU, which + # will match any deleted child objects. + ou_guid = self.get_guid(self.ou) + deleted_filter = "(lastKnownParent=<GUID={}>)".format(ou_guid) + + # the extra-filter will get combined via AND with the search expression + # we're testing, i.e. filter on the confidential attribute AND only + # include non-deleted objects, OR deleted objects from this test run + exclude_deleted_objs_filter = self.extra_filter + self.extra_filter = "(|{}{})".format(exclude_deleted_objs_filter, + deleted_filter) + + # for matching on a single object, the search expresseion becomes: + # match exactly by account-name AND either a non-deleted object OR a + # deleted object from this test run + match_by_name = "(samaccountname={})".format(self.conf_user) + not_deleted = "(!(isDeleted=*))" + self.single_obj_filter = "(&{}(|{}{}))".format(match_by_name, + not_deleted, + deleted_filter) + + # check that the search filters work as expected + self.assert_conf_attr_searches(has_rights_to="all") + self.assert_attr_visible(expect_attr=True) + self.assert_negative_searches(has_rights_to="all") + + # make the test attribute confidential *and* preserve on delete. + self.make_attr_confidential() + self.make_attr_preserve_on_delete() + + # 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() + self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + + # now delete the users (except for the user whose LDB connection + # we're currently using) + for user in self.all_users: + if user != self.user: + self.ldb_admin.delete(self.get_user_dn(user)) + + # check we still can't see the objects + self.assert_conf_attr_searches(has_rights_to=0) + self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + +TestProgram(module=__name__, opts=subunitopts) diff --git a/source4/dsdb/tests/python/ldap.py b/source4/dsdb/tests/python/ldap.py index 4235541fdbe..2514d0a9d72 100755 --- a/source4/dsdb/tests/python/ldap.py +++ b/source4/dsdb/tests/python/ldap.py @@ -599,6 +599,15 @@ class BasicTests(samba.tests.TestCase): except LdbError, (num, _): self.assertEquals(num, ERR_NO_SUCH_ATTRIBUTE) + # + # When searching the unknown attribute should be ignored + expr = "(|(cn=ldaptestgroup)(thisdoesnotexist=x))" + res = ldb.search(base=self.base_dn, + expression=expr, + scope=SCOPE_SUBTREE) + self.assertTrue(len(res) == 1, + "Search including unknown attribute failed") + delete_force(self.ldb, "cn=ldaptestgroup,cn=users," + self.base_dn) # attributes not in objectclasses and mandatory attributes missing test diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 226617f3b6a..ee7841a492a 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -376,6 +376,9 @@ plantestsuite_loadlist("samba.tests.dns_forwarder", "fl2003dc:local", [python, o plantestsuite_loadlist("samba.tests.dns_tkey", "fl2008r2dc", [python, os.path.join(srcdir(), "python/samba/tests/dns_tkey.py"), '$SERVER', '$SERVER_IP', '--machine-pass', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) plantestsuite_loadlist("samba.tests.dns_wildcard", "ad_dc", [python, os.path.join(srcdir(), "python/samba/tests/dns_wildcard.py"), '$SERVER', '$SERVER_IP', '--machine-pass', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) + +plantestsuite_loadlist("samba.tests.dns_invalid", "ad_dc", [python, os.path.join(srcdir(), "python/samba/tests/dns_invalid.py"), '$SERVER_IP', '--machine-pass', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) + for t in smbtorture4_testsuites("dns_internal."): plansmbtorture4testsuite(t, "ad_dc_ntvfs:local", '//$SERVER/whavever') @@ -793,6 +796,9 @@ for env in ["ad_dc_ntvfs", "fl2000dc", "fl2003dc", "fl2008r2dc"]: # therefore skip it in that configuration plantestsuite_loadlist("samba4.ldap.passwords.python(%s)" % env, env, [python, os.path.join(samba4srcdir, "dsdb/tests/python/passwords.py"), "$SERVER", '-U"$USERNAME%$PASSWORD"', "-W$DOMAIN", '$LOADLIST', '$LISTOPT']) +env = "ad_dc_ntvfs" +plantestsuite_loadlist("samba4.ldap.confidential_attr.python(%s)" % env, env, [python, os.path.join(samba4srcdir, "dsdb/tests/python/confidential_attr.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) + for env in ["ad_dc_ntvfs"]: # This test takes a lot of time, so we run it against a minimum of # environments, please only add new ones if there's really a diff --git a/source4/torture/drs/python/cracknames.py b/source4/torture/drs/python/cracknames.py index d8c8ae53d60..9bf90f9c997 100644 --- a/source4/torture/drs/python/cracknames.py +++ b/source4/torture/drs/python/cracknames.py @@ -149,6 +149,44 @@ class DrsCracknamesTestCase(drs_base.DrsBaseTestCase): self.ldb_dc1.delete(user) + def test_NoSPNAttribute(self): + """ + Verifies that, if we try and cracknames with the desired output + being an SPN, it returns + DRSUAPI_DS_NAME_STATUS_NOT_UNIQUE. + """ + username = "Cracknames_no_SPN" + user = "cn=%s,%s" % (username, self.ou) + + user_record = { + "dn": user, + "objectclass": "user", + "sAMAccountName" : username, + "userPrincipalName" : "test4@test.com", + "displayName" : "test4"} + + self.ldb_dc1.add(user_record) + + (result, ctr) = self._do_cracknames(user, + drsuapi.DRSUAPI_DS_NAME_FORMAT_FQDN_1779, + drsuapi.DRSUAPI_DS_NAME_FORMAT_GUID) + + self.assertEquals(ctr.count, 1) + self.assertEquals(ctr.array[0].status, + drsuapi.DRSUAPI_DS_NAME_STATUS_OK) + + user_guid = ctr.array[0].result_name + + (result, ctr) = self._do_cracknames(user_guid, + drsuapi.DRSUAPI_DS_NAME_FORMAT_GUID, + drsuapi.DRSUAPI_DS_NAME_FORMAT_SERVICE_PRINCIPAL) + + self.assertEquals(ctr.count, 1) + self.assertEquals(ctr.array[0].status, + drsuapi.DRSUAPI_DS_NAME_STATUS_NOT_FOUND) + + self.ldb_dc1.delete(user) + def _do_cracknames(self, name, format_offered, format_desired): req = drsuapi.DsNameRequest1() names = drsuapi.DsNameString() |