From 77eb99892190479a45a089024540b032ca8724dc Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 12 Sep 2023 12:28:49 +1200 Subject: [PATCH 1/2] CVE-2023-42670 s3-rpc_server: Strictly refuse to start RPC servers in conflict with AD DC Just as we refuse to start NETLOGON except on the DC, we must refuse to start all of the RPC services that are provided by the AD DC. Most critically of course this applies to netlogon, lsa and samr. This avoids the supression of these services being the result of a runtime epmapper lookup, as if that fails these services can disrupt service to end users by listening on the same socket as the AD DC servers. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15473 Signed-off-by: Andrew Bartlett --- source3/rpc_server/rpcd_classic.c | 45 ++++++++++++++++++++++++++---- source3/rpc_server/rpcd_epmapper.c | 33 ++++++++++++++++++++-- source3/rpc_server/rpcd_lsad.c | 21 ++++++++++++++ source3/rpc_server/rpcd_rpcecho.c | 33 ++++++++++++++++++++-- 4 files changed, 122 insertions(+), 10 deletions(-) diff --git a/source3/rpc_server/rpcd_classic.c b/source3/rpc_server/rpcd_classic.c index 4f6164c814c..8494af575ec 100644 --- a/source3/rpc_server/rpcd_classic.c +++ b/source3/rpc_server/rpcd_classic.c @@ -42,14 +42,34 @@ static size_t classic_interfaces( static const struct ndr_interface_table *ifaces[] = { &ndr_table_srvsvc, &ndr_table_netdfs, - &ndr_table_wkssvc, + &ndr_table_initshutdown, &ndr_table_svcctl, &ndr_table_ntsvcs, &ndr_table_eventlog, - &ndr_table_initshutdown, + /* + * This last item is truncated from the list by the + * num_ifaces -= 1 below. Take care when adding new + * services. + */ + &ndr_table_wkssvc, }; + size_t num_ifaces = ARRAY_SIZE(ifaces); + + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC wkssvc is provided by the 'samba' + * binary from source4/ + */ + num_ifaces -= 1; + break; + default: + break; + } + *pifaces = ifaces; - return ARRAY_SIZE(ifaces); + return num_ifaces; + } static size_t classic_servers( @@ -58,15 +78,28 @@ static size_t classic_servers( void *private_data) { static const struct dcesrv_endpoint_server *ep_servers[7] = { NULL }; + size_t num_servers = ARRAY_SIZE(ep_servers); bool ok; ep_servers[0] = srvsvc_get_ep_server(); ep_servers[1] = netdfs_get_ep_server(); - ep_servers[2] = wkssvc_get_ep_server(); + ep_servers[2] = initshutdown_get_ep_server(); ep_servers[3] = svcctl_get_ep_server(); ep_servers[4] = ntsvcs_get_ep_server(); ep_servers[5] = eventlog_get_ep_server(); - ep_servers[6] = initshutdown_get_ep_server(); + ep_servers[6] = wkssvc_get_ep_server(); + + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC wkssvc is provided by the 'samba' + * binary from source4/ + */ + num_servers -= 1; + break; + default: + break; + } ok = secrets_init(); if (!ok) { @@ -85,7 +118,7 @@ static size_t classic_servers( mangle_reset_cache(); *_ep_servers = ep_servers; - return ARRAY_SIZE(ep_servers); + return num_servers; } int main(int argc, const char *argv[]) diff --git a/source3/rpc_server/rpcd_epmapper.c b/source3/rpc_server/rpcd_epmapper.c index 950ba7ec12a..455179ccfba 100644 --- a/source3/rpc_server/rpcd_epmapper.c +++ b/source3/rpc_server/rpcd_epmapper.c @@ -19,6 +19,8 @@ #include "rpc_worker.h" #include "librpc/gen_ndr/ndr_epmapper.h" #include "librpc/gen_ndr/ndr_epmapper_scompat.h" +#include "param/loadparm.h" +#include "libds/common/roles.h" static size_t epmapper_interfaces( const struct ndr_interface_table ***pifaces, @@ -27,8 +29,22 @@ static size_t epmapper_interfaces( static const struct ndr_interface_table *ifaces[] = { &ndr_table_epmapper, }; + size_t num_ifaces = ARRAY_SIZE(ifaces); + + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC epmapper is provided by the 'samba' + * binary from source4/ + */ + num_ifaces = 0; + break; + default: + break; + } + *pifaces = ifaces; - return ARRAY_SIZE(ifaces); + return num_ifaces; } static size_t epmapper_servers( @@ -37,11 +53,24 @@ static size_t epmapper_servers( void *private_data) { static const struct dcesrv_endpoint_server *ep_servers[] = { NULL }; + size_t num_servers = ARRAY_SIZE(ep_servers); ep_servers[0] = epmapper_get_ep_server(); + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC epmapper is provided by the 'samba' + * binary from source4/ + */ + num_servers = 0; + break; + default: + break; + } + *_ep_servers = ep_servers; - return ARRAY_SIZE(ep_servers); + return num_servers; } int main(int argc, const char *argv[]) diff --git a/source3/rpc_server/rpcd_lsad.c b/source3/rpc_server/rpcd_lsad.c index 3ca0ed43fdd..b0e021493e7 100644 --- a/source3/rpc_server/rpcd_lsad.c +++ b/source3/rpc_server/rpcd_lsad.c @@ -36,6 +36,11 @@ static size_t lsad_interfaces( &ndr_table_lsarpc, &ndr_table_samr, &ndr_table_dssetup, + /* + * This last item is truncated from the list by the + * num_ifaces -= 1 below for the fileserver. Take + * care when adding new services. + */ &ndr_table_netlogon, }; size_t num_ifaces = ARRAY_SIZE(ifaces); @@ -46,6 +51,14 @@ static size_t lsad_interfaces( /* no netlogon for non-dc */ num_ifaces -= 1; break; + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * All these services are provided by the 'samba' + * binary from source4, not this code which is the + * source3 / NT4-like "classic" DC implementation + */ + num_ifaces = 0; + break; default: break; } @@ -80,6 +93,14 @@ static size_t lsad_servers( /* no netlogon for non-dc */ num_servers -= 1; break; + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * All these services are provided by the 'samba' + * binary from source4, not this code which is the + * source3 / NT4-like "classic" DC implementation + */ + num_servers = 0; + break; default: break; } diff --git a/source3/rpc_server/rpcd_rpcecho.c b/source3/rpc_server/rpcd_rpcecho.c index 9176039819f..37391f563db 100644 --- a/source3/rpc_server/rpcd_rpcecho.c +++ b/source3/rpc_server/rpcd_rpcecho.c @@ -19,6 +19,8 @@ #include "rpc_worker.h" #include "librpc/gen_ndr/ndr_echo.h" #include "librpc/gen_ndr/ndr_echo_scompat.h" +#include "param/loadparm.h" +#include "libds/common/roles.h" static size_t rpcecho_interfaces( const struct ndr_interface_table ***pifaces, @@ -27,8 +29,22 @@ static size_t rpcecho_interfaces( static const struct ndr_interface_table *ifaces[] = { &ndr_table_rpcecho, }; + size_t num_ifaces = ARRAY_SIZE(ifaces); + + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC rpcecho is provided by the 'samba' + * binary from source4/ + */ + num_ifaces = 0; + break; + default: + break; + } + *pifaces = ifaces; - return ARRAY_SIZE(ifaces); + return num_ifaces; } static size_t rpcecho_servers( @@ -37,11 +53,24 @@ static size_t rpcecho_servers( void *private_data) { static const struct dcesrv_endpoint_server *ep_servers[1] = { NULL }; + size_t num_servers = ARRAY_SIZE(ep_servers); ep_servers[0] = rpcecho_get_ep_server(); + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC rpcecho is provided by the 'samba' + * binary from source4/ + */ + num_servers = 0; + break; + default: + break; + } + *_ep_servers = ep_servers; - return ARRAY_SIZE(ep_servers); + return num_servers; } int main(int argc, const char *argv[]) -- 2.25.1 From a5658bb615a7ee39d20d9cf2bd56ced6f202ee31 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 12 Sep 2023 16:23:49 +1200 Subject: [PATCH 2/2] CVE-2023-42670 s3-rpc_server: Remove cross-check with "samba" EPM lookup We now have ensured that no conflicting services attempt to start so we do not need the runtime lookup and so avoid the risk that the lookup may fail. This means that any duplicates will be noticed early not just in a race condition. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15473 Signed-off-by: Andrew Bartlett --- source3/rpc_server/rpc_host.c | 154 +--------------------------------- 1 file changed, 4 insertions(+), 150 deletions(-) diff --git a/source3/rpc_server/rpc_host.c b/source3/rpc_server/rpc_host.c index 4a370f96e65..8ef6938de1f 100644 --- a/source3/rpc_server/rpc_host.c +++ b/source3/rpc_server/rpc_host.c @@ -212,7 +212,6 @@ struct rpc_server_get_endpoints_state { char **argl; char *ncalrpc_endpoint; enum dcerpc_transport_t only_transport; - struct dcerpc_binding **existing_bindings; struct rpc_host_iface_name *iface_names; struct rpc_host_endpoint **endpoints; @@ -233,7 +232,6 @@ static void rpc_server_get_endpoints_done(struct tevent_req *subreq); * @param[in] ev Event context to run this on * @param[in] rpc_server_exe Binary to ask with --list-interfaces * @param[in] only_transport Filter out anything but this - * @param[in] existing_bindings Filter out endpoints served by "samba" * @return The tevent_req representing this process */ @@ -241,8 +239,7 @@ static struct tevent_req *rpc_server_get_endpoints_send( TALLOC_CTX *mem_ctx, struct tevent_context *ev, const char *rpc_server_exe, - enum dcerpc_transport_t only_transport, - struct dcerpc_binding **existing_bindings) + enum dcerpc_transport_t only_transport) { struct tevent_req *req = NULL, *subreq = NULL; struct rpc_server_get_endpoints_state *state = NULL; @@ -254,7 +251,6 @@ static struct tevent_req *rpc_server_get_endpoints_send( return NULL; } state->only_transport = only_transport; - state->existing_bindings = existing_bindings; progname = strrchr(rpc_server_exe, '/'); if (progname != NULL) { @@ -415,37 +411,17 @@ static bool dcerpc_binding_same_endpoint( * In member mode, we only serve named pipes. Indicated by NCACN_NP * passed in via "only_transport". * - * In AD mode, the "samba" process already serves many endpoints, - * passed in via "existing_binding". Don't serve those from - * samba-dcerpcd. - * * @param[in] binding Which binding is in question? * @param[in] only_transport Exclusive transport to serve - * @param[in] existing_bindings Endpoints served by "samba" already * @return Do we want to serve "binding" from samba-dcerpcd? */ static bool rpc_host_serve_endpoint( struct dcerpc_binding *binding, - enum dcerpc_transport_t only_transport, - struct dcerpc_binding **existing_bindings) + enum dcerpc_transport_t only_transport) { enum dcerpc_transport_t transport = dcerpc_binding_get_transport(binding); - size_t i, num_existing_bindings; - - num_existing_bindings = talloc_array_length(existing_bindings); - - for (i=0; ibinding, state->only_transport, state->existing_bindings); + ep->binding, state->only_transport); if (!serve_this) { goto fail; } @@ -1592,7 +1568,6 @@ static struct tevent_req *rpc_server_setup_send( TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct rpc_host *host, - struct dcerpc_binding **existing_bindings, const char *rpc_server_exe) { struct tevent_req *req = NULL, *subreq = NULL; @@ -1624,8 +1599,7 @@ static struct tevent_req *rpc_server_setup_send( state, ev, rpc_server_exe, - host->np_helper ? NCACN_NP : NCA_UNKNOWN, - existing_bindings); + host->np_helper ? NCACN_NP : NCA_UNKNOWN); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } @@ -2329,7 +2303,6 @@ static struct tevent_req *rpc_host_send( TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct messaging_context *msg_ctx, - struct dcerpc_binding **existing_bindings, char *servers, int ready_signal_fd, const char *daemon_ready_progname, @@ -2450,7 +2423,6 @@ static struct tevent_req *rpc_host_send( state, ev, host, - existing_bindings, exe); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); @@ -2633,117 +2605,6 @@ static int rpc_host_pidfile_create( return EAGAIN; } -/* - * Find which interfaces are already being served by the samba AD - * DC so we know not to serve them. Some interfaces like netlogon - * are served by "samba", some like srvsvc will be served by the - * source3 based RPC servers. - */ -static NTSTATUS rpc_host_epm_lookup( - TALLOC_CTX *mem_ctx, - struct dcerpc_binding ***pbindings) -{ - struct rpc_pipe_client *cli = NULL; - struct pipe_auth_data *auth = NULL; - struct policy_handle entry_handle = { .handle_type = 0 }; - struct dcerpc_binding **bindings = NULL; - NTSTATUS status = NT_STATUS_UNSUCCESSFUL; - - status = rpc_pipe_open_ncalrpc(mem_ctx, &ndr_table_epmapper, &cli); - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("rpc_pipe_open_ncalrpc failed: %s\n", - nt_errstr(status)); - goto fail; - } - status = rpccli_ncalrpc_bind_data(cli, &auth); - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("rpccli_ncalrpc_bind_data failed: %s\n", - nt_errstr(status)); - goto fail; - } - status = rpc_pipe_bind(cli, auth); - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("rpc_pipe_bind failed: %s\n", nt_errstr(status)); - goto fail; - } - - for (;;) { - size_t num_bindings = talloc_array_length(bindings); - struct dcerpc_binding **tmp = NULL; - uint32_t num_entries = 0; - struct epm_entry_t *entry = NULL; - struct dcerpc_binding *binding = NULL; - uint32_t result; - - entry = talloc(cli, struct epm_entry_t); - if (entry == NULL) { - goto fail; - } - - status = dcerpc_epm_Lookup( - cli->binding_handle, /* binding_handle */ - cli, /* mem_ctx */ - 0, /* rpc_c_ep_all */ - NULL, /* object */ - NULL, /* interface id */ - 0, /* rpc_c_vers_all */ - &entry_handle, /* entry_handle */ - 1, /* max_ents */ - &num_entries, /* num_ents */ - entry, /* entries */ - &result); /* result */ - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("dcerpc_epm_Lookup failed: %s\n", - nt_errstr(status)); - goto fail; - } - - if (result == EPMAPPER_STATUS_NO_MORE_ENTRIES) { - break; - } - - if (result != EPMAPPER_STATUS_OK) { - DBG_DEBUG("dcerpc_epm_Lookup returned %"PRIu32"\n", - result); - break; - } - - if (num_entries != 1) { - DBG_DEBUG("epm_Lookup returned %"PRIu32" " - "entries, expected one\n", - num_entries); - break; - } - - status = dcerpc_binding_from_tower( - mem_ctx, &entry->tower->tower, &binding); - if (!NT_STATUS_IS_OK(status)) { - break; - } - - tmp = talloc_realloc( - mem_ctx, - bindings, - struct dcerpc_binding *, - num_bindings+1); - if (tmp == NULL) { - status = NT_STATUS_NO_MEMORY; - goto fail; - } - bindings = tmp; - - bindings[num_bindings] = talloc_move(bindings, &binding); - - TALLOC_FREE(entry); - } - - *pbindings = bindings; - status = NT_STATUS_OK; -fail: - TALLOC_FREE(cli); - return status; -} - static void samba_dcerpcd_stdin_handler( struct tevent_context *ev, struct tevent_fd *fde, @@ -2773,7 +2634,6 @@ int main(int argc, const char *argv[]) struct tevent_context *ev_ctx = NULL; struct messaging_context *msg_ctx = NULL; struct tevent_req *req = NULL; - struct dcerpc_binding **existing_bindings = NULL; char *servers = NULL; const char *arg = NULL; size_t num_servers; @@ -2980,11 +2840,6 @@ int main(int argc, const char *argv[]) exit(1); } - status = rpc_host_epm_lookup(frame, &existing_bindings); - DBG_DEBUG("rpc_host_epm_lookup returned %s, %zu bindings\n", - nt_errstr(status), - talloc_array_length(existing_bindings)); - ret = rpc_host_pidfile_create(msg_ctx, progname, ready_signal_fd); if (ret != 0) { DBG_DEBUG("rpc_host_pidfile_create failed: %s\n", @@ -2998,7 +2853,6 @@ int main(int argc, const char *argv[]) ev_ctx, ev_ctx, msg_ctx, - existing_bindings, servers, ready_signal_fd, cmdline_daemon_cfg->fork ? NULL : progname, -- 2.25.1