The branch, master has been updated via 60aa7b3634e group_audit: error handling in group change via 942567afda8 group_audit: Tests for error handling in group change via f4b3229f5b8 s4/py_dsdb: catch/handle alloc failures in py_dsdb_normalise_attributes() via 7fc60ea55ca python/kcc lib: cope with differently formed repsToFrom via 011ee2713f0 python/uptodateness: cope with unknown invocation ID via fb049827569 python: dns_hub: do not crash if a socket fails from 448d67bae72 s4:kdc: Fix size type for num_bind in kdc-heimdal
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 60aa7b3634e3312e92955779f3c9d339456c4a95 Author: Gary Lockyer <g...@catalyst.net.nz> Date: Tue Jan 8 14:24:06 2019 +1300 group_audit: error handling in group change Generate an appropriate log message in the event of an error log_group_membership_changes. As the changes have not been applied to the database, there is no easy way to determine the intended changes. This information is available in the "dsdbChange" audit messages, to avoid replicating this logic for what should be a very rare occurrence we simply log it as a "Failure" Signed-off-by: Gary Lockyer <g...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Sat Jan 19 22:32:05 CET 2019 on sn-devel-144 commit 942567afda8129425083228062c5d0f5b2a08eda Author: Gary Lockyer <g...@catalyst.net.nz> Date: Tue Jan 8 14:23:38 2019 +1300 group_audit: Tests for error handling in group change Add tests to exercise the error handling in log_group_membership_changes. Signed-off-by: Gary Lockyer <g...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit f4b3229f5b817a919b5ccede27215f283ce2734b Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu May 3 09:47:18 2018 +1200 s4/py_dsdb: catch/handle alloc failures in py_dsdb_normalise_attributes() Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Noel Power <noel.po...@suse.com> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 7fc60ea55ca56da1a9a37f642bfa03f4da2b45fa Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Dec 20 16:01:24 2018 +1300 python/kcc lib: cope with differently formed repsToFrom samba-tool visualise reuses these libraries to parse reps from other DCs, and Windows sometimes sends more data than we are expecting Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 011ee2713f0b5b0236865bc21c171d6e4ce3d108 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Dec 20 15:57:35 2018 +1300 python/uptodateness: cope with unknown invocation ID This can happen if a server has been replaced Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit fb0498275690c9477a5c0a7c2da20a3bb29556af Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sat Jan 19 09:14:28 2019 +1300 python: dns_hub: do not crash if a socket fails We print the error and keep going. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Jeremy Allison <j...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> ----------------------------------------------------------------------- Summary of changes: python/samba/kcc/kcc_utils.py | 21 +- .../samba/tests/dns_forwarder_helpers/dns_hub.py | 4 +- python/samba/uptodateness.py | 9 +- source4/dsdb/pydsdb.c | 7 + source4/dsdb/samdb/ldb_modules/group_audit.c | 31 +- .../samdb/ldb_modules/tests/test_group_audit.c | 591 ++++++++++++++++++++- 6 files changed, 636 insertions(+), 27 deletions(-) Changeset truncated at 500 lines: diff --git a/python/samba/kcc/kcc_utils.py b/python/samba/kcc/kcc_utils.py index 81d381abd99..3e9a988b778 100644 --- a/python/samba/kcc/kcc_utils.py +++ b/python/samba/kcc/kcc_utils.py @@ -19,7 +19,8 @@ # # 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 __future__ import print_function +import sys import ldb import uuid @@ -314,8 +315,13 @@ class NCReplica(NamingContext): # Possibly no repsFrom if this is a singleton DC if "repsFrom" in msg: for value in msg["repsFrom"]: - rep = RepsFromTo(self.nc_dnstr, - ndr_unpack(drsblobs.repsFromToBlob, value)) + try: + unpacked = ndr_unpack(drsblobs.repsFromToBlob, value) + except RuntimeError as e: + print("bad repsFrom NDR: %r" % (value), + file=sys.stderr) + continue + rep = RepsFromTo(self.nc_dnstr, unpacked) self.rep_repsFrom.append(rep) def commit_repsFrom(self, samdb, ro=False): @@ -468,8 +474,13 @@ class NCReplica(NamingContext): # Possibly no repsTo if this is a singleton DC if "repsTo" in msg: for value in msg["repsTo"]: - rep = RepsFromTo(self.nc_dnstr, - ndr_unpack(drsblobs.repsFromToBlob, value)) + try: + unpacked = ndr_unpack(drsblobs.repsFromToBlob, value) + except RuntimeError as e: + print("bad repsTo NDR: %r" % (value), + file=sys.stderr) + continue + rep = RepsFromTo(self.nc_dnstr, unpacked) self.rep_repsTo.append(rep) def commit_repsTo(self, samdb, ro=False): diff --git a/python/samba/tests/dns_forwarder_helpers/dns_hub.py b/python/samba/tests/dns_forwarder_helpers/dns_hub.py index 067f32d0bf6..2ac675361e0 100755 --- a/python/samba/tests/dns_forwarder_helpers/dns_hub.py +++ b/python/samba/tests/dns_forwarder_helpers/dns_hub.py @@ -122,8 +122,8 @@ class DnsHandler(sserver.BaseRequestHandler): sock.sendto(send_packet, self.client_address) except socket.error as err: print("Error sending %s to address %s for name %s: %s\n" % - (forwarder, self.client_address, name, err.errno)) - raise + (forwarder, self.client_address, name, err)) + class server_thread(threading.Thread): def __init__(self, server): diff --git a/python/samba/uptodateness.py b/python/samba/uptodateness.py index 3964bd7d6db..711407e5641 100644 --- a/python/samba/uptodateness.py +++ b/python/samba/uptodateness.py @@ -83,8 +83,13 @@ def get_utdv(samdb, dn): expression=("(&(invocationId=%s)" "(objectClass=nTDSDSA))" % inv_id), attrs=["distinguishedName", "invocationId"]) - settings_dn = str(res[0]["distinguishedName"][0]) - prefix, dsa_dn = settings_dn.split(',', 1) + try: + settings_dn = str(res[0]["distinguishedName"][0]) + prefix, dsa_dn = settings_dn.split(',', 1) + except IndexError as e: + print("Unknown invocation ID %s" % inv_id, + file=sys.stderr) + continue if prefix != 'CN=NTDS Settings': raise CommandError("Expected NTDS Settings DN, got %s" % settings_dn) diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c index 297943b1a54..3a1e97cf47f 100644 --- a/source4/dsdb/pydsdb.c +++ b/source4/dsdb/pydsdb.c @@ -744,10 +744,17 @@ static PyObject *py_dsdb_normalise_attributes(PyObject *self, PyObject *args) return NULL; } py_ret = py_type->tp_alloc(py_type, 0); + if (py_ret == NULL) { + Py_DECREF(py_type); + PyErr_NoMemory(); + return NULL; + } ret = (PyLdbMessageElementObject *)py_ret; ret->mem_ctx = talloc_new(NULL); if (talloc_reference(ret->mem_ctx, new_el) == NULL) { + Py_DECREF(py_type); + Py_DECREF(py_ret); PyErr_NoMemory(); return NULL; } diff --git a/source4/dsdb/samdb/ldb_modules/group_audit.c b/source4/dsdb/samdb/ldb_modules/group_audit.c index 4356046f675..dd991bfbb07 100644 --- a/source4/dsdb/samdb/ldb_modules/group_audit.c +++ b/source4/dsdb/samdb/ldb_modules/group_audit.c @@ -1012,14 +1012,33 @@ static void log_group_membership_changes( new_val = ldb_msg_find_element(res->msgs[0], "member"); group_type = ldb_msg_find_attr_as_uint( res->msgs[0], "groupType", 0); + log_membership_changes(acc->module, + acc->request, + new_val, + acc->members, + group_type, + status); + TALLOC_FREE(ctx); + return; } } - log_membership_changes(acc->module, - acc->request, - new_val, - acc->members, - group_type, - status); + /* + * If we get here either + * one of the lower level modules failed and the group record did + * not get updated + * or + * the updated group record could not be read. + * + * In both cases it does not make sense to log individual membership + * changes so we log a group membership change "Failure" message. + * + */ + log_membership_change(acc->module, + acc->request, + "Failure", + "", + EVT_ID_NONE, + status); TALLOC_FREE(ctx); } diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c index a6b9d603977..0bbde9f3e3b 100644 --- a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c +++ b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c @@ -39,6 +39,7 @@ uint32_t g_dsdb_flags; const char *g_exp_fmt; const char *g_dn = NULL; int g_status = LDB_SUCCESS; +struct ldb_result *g_result = NULL; int dsdb_search_one(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, @@ -63,6 +64,24 @@ int dsdb_search_one(struct ldb_context *ldb, return g_status; } +int dsdb_module_search_dn( + struct ldb_module *module, + TALLOC_CTX *mem_ctx, + struct ldb_result **res, + struct ldb_dn *basedn, + const char * const *attrs, + uint32_t dsdb_flags, + struct ldb_request *parent) +{ + + g_basedn = basedn; + g_attrs = attrs; + g_dsdb_flags = dsdb_flags; + + *res = g_result; + + return g_status; +} /* * Mock version of audit_log_json */ @@ -158,7 +177,8 @@ static void _check_group_change_message(const int message, /* * Validate the groupChange element */ - if (json_object_size(audit) != 11) { + if ((event_id == EVT_ID_NONE && json_object_size(audit) != 10) || + (event_id != EVT_ID_NONE && json_object_size(audit) != 11)) { cm_print_error("Unexpected number of elements in groupChange " "%zu != %d\n", json_object_size(audit), @@ -206,17 +226,28 @@ static void _check_group_change_message(const int message, * Validate the eventId element */ v = json_object_get(audit, "eventId"); - if (v == NULL) { - cm_print_error("No eventId element\n"); - _fail(file, line); + if (event_id == EVT_ID_NONE) { + if (v != NULL) { + int_value = json_integer_value(v); + cm_print_error("Unexpected eventId \"%d\", it should " + "NOT be present", + int_value); + _fail(file, line); + } } - - int_value = json_integer_value(v); - if (int_value != event_id) { - cm_print_error("Unexpected eventId \"%d\" != \"%d\"\n", - int_value, - event_id); - _fail(file, line); + else { + if (v == NULL) { + cm_print_error("No eventId element\n"); + _fail(file, line); + } + + int_value = json_integer_value(v); + if (int_value != event_id) { + cm_print_error("Unexpected eventId \"%d\" != \"%d\"\n", + int_value, + event_id); + _fail(file, line); + } } } @@ -802,7 +833,7 @@ static void test_audit_group_json(void **state) "the-user-name", "the-group-name", event_id, - LDB_ERR_OPERATIONS_ERROR); + LDB_SUCCESS); assert_int_equal(3, json_object_size(json.root)); v = json_object_get(json.root, "type"); @@ -829,6 +860,111 @@ static void test_audit_group_json(void **state) assert_int_equal(EVT_ID_USER_ADDED_TO_GLOBAL_SEC_GROUP, json_integer_value(v)); + v = json_object_get(audit, "statusCode"); + assert_non_null(v); + assert_true(json_is_integer(v)); + assert_int_equal(LDB_SUCCESS, json_integer_value(v)); + + v = json_object_get(audit, "status"); + assert_non_null(v); + assert_true(json_is_string(v)); + assert_string_equal("Success", json_string_value(v)); + + v = json_object_get(audit, "user"); + assert_non_null(v); + assert_true(json_is_string(v)); + assert_string_equal("the-user-name", json_string_value(v)); + + v = json_object_get(audit, "group"); + assert_non_null(v); + assert_true(json_is_string(v)); + assert_string_equal("the-group-name", json_string_value(v)); + + v = json_object_get(audit, "action"); + assert_non_null(v); + assert_true(json_is_string(v)); + assert_string_equal("the-action", json_string_value(v)); + + json_free(&json); + TALLOC_FREE(ctx); +} + +static void test_audit_group_json_error(void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_module *module = NULL; + struct ldb_request *req = NULL; + + struct tsocket_address *ts = NULL; + + const char *const SID = "S-1-5-21-2470180966-3899876309-2637894779"; + const char * const SESSION = "7130cb06-2062-6a1b-409e-3514c26b1773"; + + struct GUID transaction_id; + const char *const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773"; + + enum event_id_type event_id = EVT_ID_USER_ADDED_TO_GLOBAL_SEC_GROUP; + + struct json_object json; + json_t *audit = NULL; + json_t *v = NULL; + json_t *o = NULL; + time_t before; + + + TALLOC_CTX *ctx = talloc_new(NULL); + + ldb = ldb_init(ctx, NULL); + + GUID_from_string(TRANSACTION, &transaction_id); + + module = talloc_zero(ctx, struct ldb_module); + module->ldb = ldb; + + tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts); + ldb_set_opaque(ldb, "remoteAddress", ts); + + add_session_data(ctx, ldb, SESSION, SID); + + req = talloc_zero(ctx, struct ldb_request); + req->operation = LDB_ADD; + add_transaction_id(req, TRANSACTION); + + before = time(NULL); + json = audit_group_json(module, + req, + "the-action", + "the-user-name", + "the-group-name", + event_id, + LDB_ERR_OPERATIONS_ERROR); + assert_int_equal(3, json_object_size(json.root)); + + v = json_object_get(json.root, "type"); + assert_non_null(v); + assert_string_equal("groupChange", json_string_value(v)); + + v = json_object_get(json.root, "timestamp"); + assert_non_null(v); + assert_true(json_is_string(v)); + check_timestamp(before, json_string_value(v)); + + audit = json_object_get(json.root, "groupChange"); + assert_non_null(audit); + assert_true(json_is_object(audit)); + assert_int_equal(11, json_object_size(audit)); + + o = json_object_get(audit, "version"); + assert_non_null(o); + check_version(o, AUDIT_MAJOR, AUDIT_MINOR); + + v = json_object_get(audit, "eventId"); + assert_non_null(v); + assert_true(json_is_integer(v)); + assert_int_equal( + EVT_ID_USER_ADDED_TO_GLOBAL_SEC_GROUP, + json_integer_value(v)); + v = json_object_get(audit, "statusCode"); assert_non_null(v); assert_true(json_is_integer(v)); @@ -858,6 +994,106 @@ static void test_audit_group_json(void **state) TALLOC_FREE(ctx); } +static void test_audit_group_json_no_event(void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_module *module = NULL; + struct ldb_request *req = NULL; + + struct tsocket_address *ts = NULL; + + const char *const SID = "S-1-5-21-2470180966-3899876309-2637894779"; + const char * const SESSION = "7130cb06-2062-6a1b-409e-3514c26b1773"; + + struct GUID transaction_id; + const char *const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773"; + + enum event_id_type event_id = EVT_ID_NONE; + + struct json_object json; + json_t *audit = NULL; + json_t *v = NULL; + json_t *o = NULL; + time_t before; + + + TALLOC_CTX *ctx = talloc_new(NULL); + + ldb = ldb_init(ctx, NULL); + + GUID_from_string(TRANSACTION, &transaction_id); + + module = talloc_zero(ctx, struct ldb_module); + module->ldb = ldb; + + tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts); + ldb_set_opaque(ldb, "remoteAddress", ts); + + add_session_data(ctx, ldb, SESSION, SID); + + req = talloc_zero(ctx, struct ldb_request); + req->operation = LDB_ADD; + add_transaction_id(req, TRANSACTION); + + before = time(NULL); + json = audit_group_json(module, + req, + "the-action", + "the-user-name", + "the-group-name", + event_id, + LDB_SUCCESS); + assert_int_equal(3, json_object_size(json.root)); + + v = json_object_get(json.root, "type"); + assert_non_null(v); + assert_string_equal("groupChange", json_string_value(v)); + + v = json_object_get(json.root, "timestamp"); + assert_non_null(v); + assert_true(json_is_string(v)); + check_timestamp(before, json_string_value(v)); + + audit = json_object_get(json.root, "groupChange"); + assert_non_null(audit); + assert_true(json_is_object(audit)); + assert_int_equal(10, json_object_size(audit)); + + o = json_object_get(audit, "version"); + assert_non_null(o); + check_version(o, AUDIT_MAJOR, AUDIT_MINOR); + + v = json_object_get(audit, "eventId"); + assert_null(v); + + v = json_object_get(audit, "statusCode"); + assert_non_null(v); + assert_true(json_is_integer(v)); + assert_int_equal(LDB_SUCCESS, json_integer_value(v)); + + v = json_object_get(audit, "status"); + assert_non_null(v); + assert_true(json_is_string(v)); + assert_string_equal("Success", json_string_value(v)); + + v = json_object_get(audit, "user"); + assert_non_null(v); + assert_true(json_is_string(v)); + assert_string_equal("the-user-name", json_string_value(v)); + + v = json_object_get(audit, "group"); + assert_non_null(v); + assert_true(json_is_string(v)); + assert_string_equal("the-group-name", json_string_value(v)); + + v = json_object_get(audit, "action"); + assert_non_null(v); + assert_true(json_is_string(v)); + assert_string_equal("the-action", json_string_value(v)); + + json_free(&json); + TALLOC_FREE(ctx); +} static void setup_ldb( TALLOC_CTX *ctx, struct ldb_context **ldb, @@ -1411,6 +1647,332 @@ static void test_get_remove_member_event(void **state) assert_int_equal(EVT_ID_NONE, get_remove_member_event(UINT32_MAX)); } + +/* test log_group_membership_changes + * + * Happy path test case + * + */ +static void test_log_group_membership_changes(void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_module *module = NULL; + const char * const SID = "S-1-5-21-2470180966-3899876309-2637894779"; + const char * const SESSION = "7130cb06-2062-6a1b-409e-3514c26b1773"; + const char * const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773"; + const char * const IP = "127.0.0.1"; + struct ldb_request *req = NULL; + struct ldb_message *msg = NULL; + struct ldb_message_element *el = NULL; + struct audit_callback_context *acc = NULL; + struct ldb_result *res = NULL; + struct ldb_message *new_msg = NULL; + struct ldb_message_element *group_type = NULL; + const char *group_type_str = NULL; + struct ldb_message_element *new_el = NULL; + struct ldb_message_element *old_el = NULL; + int status = 0; + TALLOC_CTX *ctx = talloc_new(NULL); + + setup_ldb(ctx, &ldb, &module, IP, SESSION, SID); + + /* + * Build the ldb message + */ + msg = talloc_zero(ctx, struct ldb_message); + + /* -- Samba Shared Repository