[PATCH] Improve two rpcserver3 debug messages

Volker Lendecke Volker.Lendecke at SerNet.DE
Mon Jan 15 12:23:02 UTC 2018


Hi!

Review appreciated!

Thanks, Volker

-- 
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
-------------- next part --------------
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