From: Volker Lendecke <vl@samba.org>
Date: Mon, 14 Jun 2021 05:54:55 +0000 (+0200)
Subject: s3:services: Disable rcinit-based service control code
X-Git-Tag: tdb-1.4.6~370
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a60c7b4ff29bc59c0d5a42f14dbe0ae4dbe26192;p=thirdparty%2Fsamba.git

s3:services: Disable rcinit-based service control code

This is a become_root user callout that I have never seen in use in
more than 20 years of Samba. Why disable now? In the next commit I
need to make a change to initializing the registry values for
services, the svcctl service won't be able to do registry transactions
anymore. I'm not sure that going without transactions is 100% safe in
all failure cases, so I decided to propose disabling the problematic
code that might lead to security issues.

One fix might be to add a lot more validation code to
_svcctl_OpenServiceW() to see whether the registry values underlying
the service are sane.

Yes, this is technical debt, but I would question that starting unix
daemons via DCERPC used at all out there.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Samuel Cabrero <scabrero@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---

diff --git a/source3/services/svc_rcinit.c b/source3/services/svc_rcinit.c
index 95442ce2efb..7a8ad6516c2 100644
--- a/source3/services/svc_rcinit.c
+++ b/source3/services/svc_rcinit.c
@@ -25,8 +25,15 @@
 
 static WERROR rcinit_stop( const char *service, struct SERVICE_STATUS *status )
 {
+	int ret = -1;
+
+	/*
+	 * Disabled due to security concerns and unknown use in the
+	 * field -- vl@samba.org
+	 */
+#if 0
 	char *command = NULL;
-	int ret, fd;
+	int fd;
 
 	if (asprintf(&command, "%s/%s/%s stop",
 				get_dyn_MODULESDIR(), SVCCTL_SCRIPT_DIR, service) < 0) {
@@ -50,7 +57,7 @@ static WERROR rcinit_stop( const char *service, struct SERVICE_STATUS *status )
 	status->state			= (ret == 0 ) ? SVCCTL_STOPPED : SVCCTL_RUNNING;
 	status->controls_accepted	= SVCCTL_ACCEPT_STOP |
 					  SVCCTL_ACCEPT_SHUTDOWN;
-
+#endif
 	return ( ret == 0 ) ? WERR_OK : WERR_ACCESS_DENIED;
 }
 
@@ -59,8 +66,14 @@ static WERROR rcinit_stop( const char *service, struct SERVICE_STATUS *status )
 
 static WERROR rcinit_start( const char *service )
 {
+	int ret = -1;
+	/*
+	 * Disabled due to security concerns and unknown use in the
+	 * field -- vl@samba.org
+	 */
+#if 0
 	char *command = NULL;
-	int ret, fd;
+	int fd;
 
 	if (asprintf(&command, "%s/%s/%s start",
 				get_dyn_MODULESDIR(), SVCCTL_SCRIPT_DIR, service) < 0) {
@@ -77,7 +90,7 @@ static WERROR rcinit_start( const char *service )
 	close(fd);
 
 	SAFE_FREE(command);
-
+#endif
 	return ( ret == 0 ) ? WERR_OK : WERR_ACCESS_DENIED;
 }
 
@@ -86,6 +99,11 @@ static WERROR rcinit_start( const char *service )
 
 static WERROR rcinit_status( const char *service, struct SERVICE_STATUS *status )
 {
+	/*
+	 * Disabled due to security concerns and unknown use in the
+	 * field -- vl@samba.org
+	 */
+#if 0
 	char *command = NULL;
 	int ret, fd;
 
@@ -115,6 +133,9 @@ static WERROR rcinit_status( const char *service, struct SERVICE_STATUS *status
 					  SVCCTL_ACCEPT_SHUTDOWN;
 
 	return WERR_OK;
+#else
+	return WERR_ACCESS_DENIED;
+#endif
 }
 
 /*********************************************************************