The branch, master has been updated via 02991b4 rpc_server backupkey: pass remote connection data via fe6e7ce rpc_server lsa: pass remote connection data via d48b5d5 rpc_server: common routine to open ldb in system session via d06ebf6 dsdb acl: Copy dsdb_control_password_acl_validation into reply via fcbb3f3 cldap: clear remote address after cldap_dse_fill via 9f55986 auth tests: irpc remove "auth_event" name on completion from 8426c42 sambatool: heuristics to decided whether colour is wanted
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 02991b4df91605bf1cb4841238b33a64d42eed43 Author: Gary Lockyer <g...@catalyst.net.nz> Date: Wed May 30 14:45:03 2018 +1200 rpc_server backupkey: pass remote connection data Ensure that the requesting session data is passed to the audit logging module for BackupKey requests. 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): Thu May 31 12:35:15 CEST 2018 on sn-devel-144 commit fe6e7ce2a21d81ff7622b53d3ec09743bc4ed89e Author: Gary Lockyer <g...@catalyst.net.nz> Date: Wed May 30 14:44:19 2018 +1200 rpc_server lsa: pass remote connection data Ensure that the session details of the requesting user are available to the audit logging module for the CreateSecret and OpenSecret operations. Signed-off-by: Gary Lockyer <g...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit d48b5d5320807ef394d1d31c3f9e92b4955a4aea Author: Gary Lockyer <g...@catalyst.net.nz> Date: Wed May 30 14:43:25 2018 +1200 rpc_server: common routine to open ldb in system session Add a function to open an ldb connection under the system session and save the remote users session details in a ldb_opaque. This will allow the audit logging to log the original session for operations performed in the system session. Signed-off-by: Gary Lockyer <g...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit d06ebf646c2a89d2b445f8d4ce85f9151886caa5 Author: Gary Lockyer <g...@catalyst.net.nz> Date: Fri May 25 09:53:29 2018 +1200 dsdb acl: Copy dsdb_control_password_acl_validation into reply Copy the dsdb_control_password_acl_validation into the reply so that it is available to the audit_logging module. The audit logging module uses it to differentiate between password change and reset operations. We include it in the result for failed request to allow the logging of failed attempts. Signed-off-by: Gary Lockyer <g...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit fcbb3f31fea14798d8b20ae4a3ec034eef3711ba Author: Gary Lockyer <g...@catalyst.net.nz> Date: Wed Apr 4 12:38:25 2018 +1200 cldap: clear remote address after cldap_dse_fill Need to clear the remote address as the ldb handle is shared, and changes made by internal processes would be logged as coming from the last cldap requester Signed-off-by: Gary Lockyer <g...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 9f55986a59436b2194e86be24697370a33d6239f Author: Gary Lockyer <g...@catalyst.net.nz> Date: Fri May 25 15:21:33 2018 +1200 auth tests: irpc remove "auth_event" name on completion Remove the "auth_event" name on completion of tests to prevent issues with tests using messaging. Signed-off-by: Gary Lockyer <g...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> ----------------------------------------------------------------------- Summary of changes: python/samba/tests/auth_log_base.py | 1 + source4/cldap_server/rootdse.c | 7 + source4/dsdb/samdb/ldb_modules/acl.c | 187 +++++++++++++++++++-- source4/rpc_server/backupkey/dcesrv_backupkey.c | 13 +- .../backupkey/dcesrv_backupkey_heimdal.c | 14 +- source4/rpc_server/common/server_info.c | 26 +++ source4/rpc_server/lsa/dcesrv_lsa.c | 42 ++--- source4/rpc_server/wscript_build | 2 +- 8 files changed, 248 insertions(+), 44 deletions(-) Changeset truncated at 500 lines: diff --git a/python/samba/tests/auth_log_base.py b/python/samba/tests/auth_log_base.py index 5a70ce3..6edd0f6 100644 --- a/python/samba/tests/auth_log_base.py +++ b/python/samba/tests/auth_log_base.py @@ -58,6 +58,7 @@ class AuthLogTestBase(samba.tests.TestCase): if self.msg_handler_and_context: self.msg_ctx.deregister(self.msg_handler_and_context, msg_type=MSG_AUTH_LOG) + self.msg_ctx.irpc_remove_name(AUTH_EVENT_NAME) def waitForMessages(self, isLastExpectedMessage, connection=None): """Wait for all the expected messages to arrive diff --git a/source4/cldap_server/rootdse.c b/source4/cldap_server/rootdse.c index 3f389ce..a5e1c6b 100644 --- a/source4/cldap_server/rootdse.c +++ b/source4/cldap_server/rootdse.c @@ -166,6 +166,13 @@ void cldapd_rootdse_request(struct cldap_socket *cldap, cldapd_rootdse_fill(cldapd, tmp_ctx, search, &reply.response, reply.result); + /* + * We clear this after cldapd_rootdse_fill as this is shared ldb + * and if it was not cleared the audit logging would report changes + * made by internal processes as coming from the last cldap requester + */ + ldb_set_opaque(cldapd->samctx, "remoteAddress", NULL); + status = cldap_reply_send(cldap, &reply); if (!NT_STATUS_IS_OK(status)) { DEBUG(2,("cldap rootdse query failed '%s' - %s\n", diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 8b1dcbe..cd7144a 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -968,13 +968,15 @@ static int acl_check_self_membership(TALLOC_CTX *mem_ctx, return ret; } -static int acl_check_password_rights(TALLOC_CTX *mem_ctx, - struct ldb_module *module, - struct ldb_request *req, - struct security_descriptor *sd, - struct dom_sid *sid, - const struct dsdb_class *objectclass, - bool userPassword) +static int acl_check_password_rights( + TALLOC_CTX *mem_ctx, + struct ldb_module *module, + struct ldb_request *req, + struct security_descriptor *sd, + struct dom_sid *sid, + const struct dsdb_class *objectclass, + bool userPassword, + struct dsdb_control_password_acl_validation **control_for_response) { int ret = LDB_SUCCESS; unsigned int del_attr_cnt = 0, add_attr_cnt = 0, rep_attr_cnt = 0; @@ -996,6 +998,12 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx, talloc_free(tmp_ctx); return LDB_ERR_OPERATIONS_ERROR; } + /* + * Set control_for_response to pav so it can be added to the response + * and be passed up to the audit_log module which uses it to identify + * password reset attempts. + */ + *control_for_response = pav; c = ldb_request_get_control(req, DSDB_CONTROL_PASSWORD_CHANGE_OID); if (c != NULL) { @@ -1165,6 +1173,105 @@ checked: return LDB_SUCCESS; } +/* + * Context needed by acl_callback + */ +struct acl_callback_context { + struct ldb_request *request; + struct ldb_module *module; +}; + +/* + * @brief Copy the password validation control to the reply. + * + * Copy the dsdb_control_password_acl_validation control from the request, + * to the reply. The control is used by the audit_log module to identify + * password rests. + * + * @param req the ldb request. + * @param ares the result, updated with the control. + */ +static void copy_password_acl_validation_control( + struct ldb_request *req, + struct ldb_reply *ares) +{ + struct ldb_control *pav_ctrl = NULL; + struct dsdb_control_password_acl_validation *pav = NULL; + + pav_ctrl = ldb_request_get_control( + discard_const(req), + DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID); + if (pav_ctrl == NULL) { + return; + } + + pav = talloc_get_type_abort( + pav_ctrl->data, + struct dsdb_control_password_acl_validation); + if (pav == NULL) { + return; + } + ldb_reply_add_control( + ares, + DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID, + false, + pav); +} +/* + * @brief call back function for acl_modify. + * + * Calls acl_copy to copy the dsdb_control_password_acl_validation from + * the request to the reply. + * + * @param req the ldb_request. + * @param ares the operation result. + * + * @return the LDB_STATUS + */ +static int acl_callback(struct ldb_request *req, struct ldb_reply *ares) +{ + struct acl_callback_context *ac = NULL; + + ac = talloc_get_type(req->context, struct acl_callback_context); + + if (!ares) { + return ldb_module_done( + ac->request, + NULL, + NULL, + LDB_ERR_OPERATIONS_ERROR); + } + + /* pass on to the callback */ + switch (ares->type) { + case LDB_REPLY_ENTRY: + return ldb_module_send_entry( + ac->request, + ares->message, + ares->controls); + + case LDB_REPLY_REFERRAL: + return ldb_module_send_referral( + ac->request, + ares->referral); + + case LDB_REPLY_DONE: + /* + * Copy the ACL control from the request to the response + */ + copy_password_acl_validation_control(req, ares); + return ldb_module_done( + ac->request, + ares->controls, + ares->response, + ares->error); + + default: + /* Can't happen */ + return LDB_ERR_OPERATIONS_ERROR; + } +} + static int acl_modify(struct ldb_module *module, struct ldb_request *req) { int ret; @@ -1187,6 +1294,10 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) "objectSid", NULL }; + struct acl_callback_context *context = NULL; + struct ldb_request *new_req = NULL; + struct dsdb_control_password_acl_validation *pav = NULL; + struct ldb_control **controls = NULL; if (ldb_dn_is_special(msg->dn)) { return ldb_next_request(module, req); @@ -1324,6 +1435,14 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) } else if (ldb_attr_cmp("unicodePwd", el->name) == 0 || (userPassword && ldb_attr_cmp("userPassword", el->name) == 0) || ldb_attr_cmp("clearTextPassword", el->name) == 0) { + /* + * Ideally we would do the acl_check_password_rights + * before we checked the other attributes, i.e. in a + * loop before the current one. + * Have not done this as yet in order to limit the size + * of the change. To limit the possibility of breaking + * the ACL logic. + */ if (password_rights_checked) { continue; } @@ -1333,7 +1452,8 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) sd, sid, objectclass, - userPassword); + userPassword, + &pav); if (ret != LDB_SUCCESS) { goto fail; } @@ -1382,10 +1502,57 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req) success: talloc_free(tmp_ctx); - return ldb_next_request(module, req); + context = talloc_zero(req, struct acl_callback_context); + + if (context == NULL) { + return ldb_oom(ldb); + } + context->request = req; + context->module = module; + ret = ldb_build_mod_req( + &new_req, + ldb, + req, + req->op.mod.message, + req->controls, + context, + acl_callback, + req); + if (ret != LDB_SUCCESS) { + return ret; + } + return ldb_next_request(module, new_req); fail: talloc_free(tmp_ctx); - return ret; + /* + * We copy the pav into the result, so that the password reset + * logging code in audit_log can log failed password reset attempts. + */ + if (pav) { + struct ldb_control *control = NULL; + + controls = talloc_zero_array(req, struct ldb_control *, 2); + if (controls == NULL) { + return ldb_oom(ldb); + } + + control = talloc(controls, struct ldb_control); + + if (control == NULL) { + return ldb_oom(ldb); + } + + control->oid= talloc_strdup( + control, + DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID); + if (control->oid == NULL) { + return ldb_oom(ldb); + } + control->critical = false; + control->data = pav; + *controls = control; + } + return ldb_module_done(req, controls, NULL, ret); } /* similar to the modify for the time being. diff --git a/source4/rpc_server/backupkey/dcesrv_backupkey.c b/source4/rpc_server/backupkey/dcesrv_backupkey.c index d2c3f2a..fd9c101 100644 --- a/source4/rpc_server/backupkey/dcesrv_backupkey.c +++ b/source4/rpc_server/backupkey/dcesrv_backupkey.c @@ -22,6 +22,7 @@ #include "includes.h" #include "rpc_server/dcerpc_server.h" +#include "rpc_server/common/common.h" #include "librpc/gen_ndr/ndr_backupkey.h" #include "dsdb/common/util.h" #include "dsdb/samdb/samdb.h" @@ -1774,12 +1775,12 @@ static WERROR dcesrv_bkrp_BackupKey(struct dcesrv_call_state *dce_call, return WERR_NOT_SUPPORTED; } - ldb_ctx = samdb_connect(mem_ctx, - dce_call->event_ctx, - dce_call->conn->dce_ctx->lp_ctx, - system_session(dce_call->conn->dce_ctx->lp_ctx), - dce_call->conn->remote_address, - 0); + /* + * Save the current remote session details so they can used by the + * audit logging module. This allows the audit logging to report the + * remote users details, rather than the system users details. + */ + ldb_ctx = dcesrv_samdb_connect_as_system(mem_ctx, dce_call); if (samdb_rodc(ldb_ctx, &is_rodc) != LDB_SUCCESS) { talloc_unlink(mem_ctx, ldb_ctx); diff --git a/source4/rpc_server/backupkey/dcesrv_backupkey_heimdal.c b/source4/rpc_server/backupkey/dcesrv_backupkey_heimdal.c index d2fc480..198a408 100644 --- a/source4/rpc_server/backupkey/dcesrv_backupkey_heimdal.c +++ b/source4/rpc_server/backupkey/dcesrv_backupkey_heimdal.c @@ -21,6 +21,7 @@ #include "includes.h" #include "rpc_server/dcerpc_server.h" +#include "rpc_server/common/common.h" #include "librpc/gen_ndr/ndr_backupkey.h" #include "dsdb/common/util.h" #include "dsdb/samdb/samdb.h" @@ -1814,13 +1815,12 @@ static WERROR dcesrv_bkrp_BackupKey(struct dcesrv_call_state *dce_call, return WERR_NOT_SUPPORTED; } - ldb_ctx = samdb_connect(mem_ctx, - dce_call->event_ctx, - dce_call->conn->dce_ctx->lp_ctx, - system_session(dce_call->conn->dce_ctx->lp_ctx), - dce_call->conn->remote_address, - 0); - + /* + * Save the current remote session details so they can used by the + * audit logging module. This allows the audit logging to report the + * remote users details, rather than the system users details. + */ + ldb_ctx = dcesrv_samdb_connect_as_system(mem_ctx, dce_call); if (samdb_rodc(ldb_ctx, &is_rodc) != LDB_SUCCESS) { talloc_unlink(mem_ctx, ldb_ctx); return WERR_INVALID_PARAMETER; diff --git a/source4/rpc_server/common/server_info.c b/source4/rpc_server/common/server_info.c index 0aabcda..7229457 100644 --- a/source4/rpc_server/common/server_info.c +++ b/source4/rpc_server/common/server_info.c @@ -186,3 +186,29 @@ bool dcesrv_common_validate_share_name(TALLOC_CTX *mem_ctx, const char *share_na return true; } + +/* + * Open an ldb connection under the system session and save the remote users + * session details in a ldb_opaque. This will allow the audit logging to + * log the original session for operations performed in the system session. + */ +struct ldb_context *dcesrv_samdb_connect_as_system( + TALLOC_CTX *mem_ctx, + struct dcesrv_call_state *dce_call) +{ + struct ldb_context *samdb = NULL; + samdb = samdb_connect( + mem_ctx, + dce_call->event_ctx, + dce_call->conn->dce_ctx->lp_ctx, + system_session(dce_call->conn->dce_ctx->lp_ctx), + dce_call->conn->remote_address, + 0); + if (samdb) { + ldb_set_opaque( + samdb, + "networkSessionInfo", + dce_call->conn->auth_state.session_info); + } + return samdb; +} diff --git a/source4/rpc_server/lsa/dcesrv_lsa.c b/source4/rpc_server/lsa/dcesrv_lsa.c index 8c540ab..acf14f9 100644 --- a/source4/rpc_server/lsa/dcesrv_lsa.c +++ b/source4/rpc_server/lsa/dcesrv_lsa.c @@ -3167,8 +3167,6 @@ static NTSTATUS dcesrv_lsa_SetSystemAccessAccount(struct dcesrv_call_state *dce_ { DCESRV_FAULT(DCERPC_FAULT_OP_RNG_ERROR); } - - /* lsa_CreateSecret */ @@ -3231,16 +3229,17 @@ static NTSTATUS dcesrv_lsa_CreateSecret(struct dcesrv_call_state *dce_call, TALL ldb_binary_encode_string(mem_ctx, name)); NT_STATUS_HAVE_NO_MEMORY(name2); - /* We need to connect to the database as system, as this is one - * of the rare RPC calls that must read the secrets (and this - * is denied otherwise) */ - samdb = samdb_connect( - mem_ctx, - dce_call->event_ctx, - dce_call->conn->dce_ctx->lp_ctx, - system_session(dce_call->conn->dce_ctx->lp_ctx), - dce_call->conn->remote_address, - 0); + /* + * We need to connect to the database as system, as this is + * one of the rare RPC calls that must read the secrets + * (and this is denied otherwise) + * + * We also save the current remote session details so they can + * used by the audit logging module. This allows the audit + * logging to report the remote users details, rather than the + * system users details. + */ + samdb = dcesrv_samdb_connect_as_system(mem_ctx, dce_call); secret_state->sam_ldb = talloc_reference(secret_state, samdb); NT_STATUS_HAVE_NO_MEMORY(secret_state->sam_ldb); @@ -3379,14 +3378,17 @@ static NTSTATUS dcesrv_lsa_OpenSecret(struct dcesrv_call_state *dce_call, TALLOC if (strncmp("G$", r->in.name.string, 2) == 0) { name = &r->in.name.string[2]; - /* We need to connect to the database as system, as this is one of the rare RPC calls that must read the secrets (and this is denied otherwise) */ - samdb = samdb_connect( - mem_ctx, - dce_call->event_ctx, - dce_call->conn->dce_ctx->lp_ctx, - system_session(dce_call->conn->dce_ctx->lp_ctx), - dce_call->conn->remote_address, - 0); + /* + * We need to connect to the database as system, as this is + * one of the rare RPC calls that must read the secrets + * (and this is denied otherwise) + * + * We also save the current remote session details so they can + * used by the audit logging module. This allows the audit + * logging to report the remote users details, rather than the + * system users details. + */ + samdb = dcesrv_samdb_connect_as_system(mem_ctx, dce_call); secret_state->sam_ldb = talloc_reference(secret_state, samdb); secret_state->global = true; diff --git a/source4/rpc_server/wscript_build b/source4/rpc_server/wscript_build index 31a5696..8e05eb8 100644 --- a/source4/rpc_server/wscript_build +++ b/source4/rpc_server/wscript_build @@ -133,7 +133,7 @@ else: autoproto='backupkey/proto.h', subsystem='dcerpc_server', init_function='dcerpc_server_backupkey_init', - deps='samdb DCERPC_COMMON NDR_BACKUPKEY RPC_NDR_BACKUPKEY krb5 hx509 hcrypto gnutls gcrypt', + deps='samdb DCERPC_COMMON NDR_BACKUPKEY RPC_NDR_BACKUPKEY krb5 hx509 hcrypto gnutls gcrypt DCERPC_COMMON', ) -- Samba Shared Repository