[PATCH] Improve two rpcserver3 debug messages
Jeremy Allison
jra at samba.org
Mon Jan 15 19:07:19 UTC 2018
On Mon, Jan 15, 2018 at 01:23:02PM +0100, Volker Lendecke via samba-technical wrote:
> Hi!
>
> Review appreciated!
LGTM. Pushed, thanks !
> --
> Besuchen Sie die verinice.XP 2018 in Berlin,
> Anwenderkonferenz für Informationssicherheit
> vom 21.-23.03.2018 im Sofitel Kurfürstendamm
> Info & Anmeldung hier: http://veriniceXP.org
>
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de, mailto:kontakt at sernet.de
> From 00fd09dfcd7f35fe5981f01823131f0f894ceabe Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Mon, 15 Jan 2018 10:47:51 +0100
> Subject: [PATCH 1/2] rpc_server: Improve a debug message
>
> A client sending us a bind with an unknown interface should not spam
> syslog by default. Also, show what interface the client tried to connect
> to.
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/rpc_server/srv_pipe.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/source3/rpc_server/srv_pipe.c b/source3/rpc_server/srv_pipe.c
> index 4534200f75c..7a1c6159467 100644
> --- a/source3/rpc_server/srv_pipe.c
> +++ b/source3/rpc_server/srv_pipe.c
> @@ -738,7 +738,10 @@ static bool api_pipe_bind_req(struct pipes_struct *p,
>
> table = ndr_table_by_uuid(&id.uuid);
> if (table == NULL) {
> - DEBUG(0,("unknown interface\n"));
> + char *iface = ndr_syntax_id_to_string(talloc_tos(), &id);
> + DBG_NOTICE("unknown interface %s\n",
> + iface ? iface : "<null>");
> + TALLOC_FREE(iface);
> return false;
> }
>
> --
> 2.11.0
>
>
> From ea6677121551f71e8335e73546bdd0c649918cb9 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Mon, 15 Jan 2018 11:42:29 +0100
> Subject: [PATCH 2/2] srcctl3: Improve debug messages
>
> A customer's syslog was filled with
>
> _svcctl_OpenServiceW: Failed to get a valid security descriptor
>
> messages. This improves the messages to give info about which service failed
> with which error code. Also, it makes OpenServiceW fail with the same error
> message Windows fails with for unknown services.
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/rpc_server/svcctl/srv_svcctl_nt.c | 42 ++++++++++++++++++++-----------
> source3/services/svc_winreg_glue.c | 25 ++++++++++--------
> source3/services/svc_winreg_glue.h | 9 ++++---
> 3 files changed, 48 insertions(+), 28 deletions(-)
>
> diff --git a/source3/rpc_server/svcctl/srv_svcctl_nt.c b/source3/rpc_server/svcctl/srv_svcctl_nt.c
> index d4bf2e70e35..8eaedfb764d 100644
> --- a/source3/rpc_server/svcctl/srv_svcctl_nt.c
> +++ b/source3/rpc_server/svcctl/srv_svcctl_nt.c
> @@ -301,6 +301,7 @@ WERROR _svcctl_OpenServiceW(struct pipes_struct *p,
> uint32_t access_granted = 0;
> NTSTATUS status;
> const char *service = NULL;
> + WERROR err;
>
> service = r->in.ServiceName;
> if (!service) {
> @@ -317,14 +318,19 @@ WERROR _svcctl_OpenServiceW(struct pipes_struct *p,
> * Perform access checks. Use the system session_info in order to ensure
> * that we retrieve the security descriptor
> */
> - sec_desc = svcctl_get_secdesc(p->mem_ctx,
> - p->msg_ctx,
> - get_session_info_system(),
> - service);
> - if (sec_desc == NULL) {
> - DEBUG(0, ("_svcctl_OpenServiceW: Failed to get a valid security "
> - "descriptor"));
> - return WERR_NOT_ENOUGH_MEMORY;
> + err = svcctl_get_secdesc(p->msg_ctx,
> + get_session_info_system(),
> + service,
> + p->mem_ctx,
> + &sec_desc);
> + if (W_ERROR_EQUAL(err, WERR_FILE_NOT_FOUND)) {
> + DBG_NOTICE("service %s does not exist\n", service);
> + return WERR_SERVICE_DOES_NOT_EXIST;
> + }
> + if (!W_ERROR_IS_OK(err)) {
> + DBG_NOTICE("Failed to get a valid secdesc for %s: %s\n",
> + service, win_errstr(err));
> + return err;
> }
>
> se_map_generic( &r->in.access_mask, &svc_generic_map );
> @@ -899,6 +905,7 @@ WERROR _svcctl_QueryServiceObjectSecurity(struct pipes_struct *p,
> NTSTATUS status;
> uint8_t *buffer = NULL;
> size_t len = 0;
> + WERROR err;
>
>
> /* only support the SCM and individual services */
> @@ -917,12 +924,19 @@ WERROR _svcctl_QueryServiceObjectSecurity(struct pipes_struct *p,
> return WERR_INVALID_PARAMETER;
>
> /* Lookup the security descriptor and marshall it up for a reply */
> - sec_desc = svcctl_get_secdesc(p->mem_ctx,
> - p->msg_ctx,
> - get_session_info_system(),
> - info->name);
> - if (sec_desc == NULL) {
> - return WERR_NOT_ENOUGH_MEMORY;
> + err = svcctl_get_secdesc(p->msg_ctx,
> + get_session_info_system(),
> + info->name,
> + p->mem_ctx,
> + &sec_desc);
> + if (W_ERROR_EQUAL(err, WERR_FILE_NOT_FOUND)) {
> + DBG_NOTICE("service %s does not exist\n", info->name);
> + return WERR_SERVICE_DOES_NOT_EXIST;
> + }
> + if (!W_ERROR_IS_OK(err)) {
> + DBG_NOTICE("Failed to get a valid secdesc for %s: %s\n",
> + info->name, win_errstr(err));
> + return err;
> }
>
> *r->out.needed = ndr_size_security_descriptor(sec_desc, 0);
> diff --git a/source3/services/svc_winreg_glue.c b/source3/services/svc_winreg_glue.c
> index 7d7871d8884..50b9897acde 100644
> --- a/source3/services/svc_winreg_glue.c
> +++ b/source3/services/svc_winreg_glue.c
> @@ -75,10 +75,11 @@ struct security_descriptor* svcctl_gen_service_sd(TALLOC_CTX *mem_ctx)
> return sd;
> }
>
> -struct security_descriptor *svcctl_get_secdesc(TALLOC_CTX *mem_ctx,
> - struct messaging_context *msg_ctx,
> - const struct auth_session_info *session_info,
> - const char *name)
> +WERROR svcctl_get_secdesc(struct messaging_context *msg_ctx,
> + const struct auth_session_info *session_info,
> + const char *name,
> + TALLOC_CTX *mem_ctx,
> + struct security_descriptor **psd)
> {
> struct dcerpc_binding_handle *h = NULL;
> uint32_t access_mask = SEC_FLAG_MAXIMUM_ALLOWED;
> @@ -92,7 +93,7 @@ struct security_descriptor *svcctl_get_secdesc(TALLOC_CTX *mem_ctx,
> "%s\\%s\\Security",
> TOP_LEVEL_SERVICES_KEY, name);
> if (key == NULL) {
> - return NULL;
> + return WERR_NOT_ENOUGH_MEMORY;
> }
>
> status = dcerpc_winreg_int_hklm_openkey(mem_ctx,
> @@ -108,12 +109,12 @@ struct security_descriptor *svcctl_get_secdesc(TALLOC_CTX *mem_ctx,
> if (!NT_STATUS_IS_OK(status)) {
> DEBUG(2, ("svcctl_set_secdesc: Could not open %s - %s\n",
> key, nt_errstr(status)));
> - return NULL;
> + return WERR_INTERNAL_ERROR;
> }
> if (!W_ERROR_IS_OK(result)) {
> DEBUG(2, ("svcctl_set_secdesc: Could not open %s - %s\n",
> key, win_errstr(result)));
> - return NULL;
> + return result;
> }
>
> status = dcerpc_winreg_query_sd(mem_ctx,
> @@ -125,14 +126,14 @@ struct security_descriptor *svcctl_get_secdesc(TALLOC_CTX *mem_ctx,
> if (!NT_STATUS_IS_OK(status)) {
> DEBUG(2, ("svcctl_get_secdesc: error getting value 'Security': "
> "%s\n", nt_errstr(status)));
> - return NULL;
> + return WERR_INTERNAL_ERROR;
> }
> if (W_ERROR_EQUAL(result, WERR_FILE_NOT_FOUND)) {
> goto fallback_to_default_sd;
> } else if (!W_ERROR_IS_OK(result)) {
> DEBUG(2, ("svcctl_get_secdesc: error getting value 'Security': "
> "%s\n", win_errstr(result)));
> - return NULL;
> + return result;
> }
>
> goto done;
> @@ -141,9 +142,13 @@ fallback_to_default_sd:
> DEBUG(6, ("svcctl_get_secdesc: constructing default secdesc for "
> "service [%s]\n", name));
> sd = svcctl_gen_service_sd(mem_ctx);
> + if (sd == NULL) {
> + return WERR_NOT_ENOUGH_MEMORY;
> + }
>
> done:
> - return sd;
> + *psd = sd;
> + return WERR_OK;
> }
>
> bool svcctl_set_secdesc(struct messaging_context *msg_ctx,
> diff --git a/source3/services/svc_winreg_glue.h b/source3/services/svc_winreg_glue.h
> index c9366b377d2..e013f8deeda 100644
> --- a/source3/services/svc_winreg_glue.h
> +++ b/source3/services/svc_winreg_glue.h
> @@ -28,10 +28,11 @@ struct auth_session_info;
>
> struct security_descriptor* svcctl_gen_service_sd(TALLOC_CTX *mem_ctx);
>
> -struct security_descriptor *svcctl_get_secdesc(TALLOC_CTX *mem_ctx,
> - struct messaging_context *msg_ctx,
> - const struct auth_session_info *session_info,
> - const char *name);
> +WERROR svcctl_get_secdesc(struct messaging_context *msg_ctx,
> + const struct auth_session_info *session_info,
> + const char *name,
> + TALLOC_CTX *mem_ctx,
> + struct security_descriptor **result);
>
> bool svcctl_set_secdesc(struct messaging_context *msg_ctx,
> const struct auth_session_info *session_info,
> --
> 2.11.0
>
More information about the samba-technical
mailing list