From: Scott Lamb <slamb@slamb.org>
Date: Thu, 8 Nov 2007 19:18:44 +0000 (+0000)
Subject: Use a control socket directory to restrict access
X-Git-Tag: release-1.1pre1~133
X-Git-Url: http://tinc-vpn.org/git/browse?a=commitdiff_plain;h=fe2f1fceb546ca4326435cac26bcf3f513e82b43;p=tinc

Use a control socket directory to restrict access

This provides reasonable security even on Solaris. The sysadmin is
responsible for securing the control socket's ancestors from the
grandparent on.

We could add a cryptographic handshake later if desired.
---

diff --git a/src/control.c b/src/control.c
index a7958437..26733854 100644
--- a/src/control.c
+++ b/src/control.c
@@ -191,6 +191,7 @@ static void handle_new_control_socket(int fd, short events, void *data) {
 
 	memset(&greeting, 0, sizeof greeting);
 	greeting.version = TINC_CTL_VERSION_CURRENT;
+	greeting.pid = getpid();
 	if(bufferevent_write(ev, &greeting, sizeof greeting) == -1) {
 		logger(LOG_ERR,
 			   _("Cannot send greeting for new control connection: %s"),
@@ -213,10 +214,11 @@ static int control_compare(const struct event *a, const struct event *b) {
 bool init_control() {
 	int result;
 	struct sockaddr_un addr;
+	char *lastslash;
 
 	if(strlen(controlsocketname) >= sizeof addr.sun_path) {
 		logger(LOG_ERR, _("Control socket filename too long!"));
-		return false;
+		goto bail;
 	}
 
 	memset(&addr, 0, sizeof addr);
@@ -227,12 +229,43 @@ bool init_control() {
 
 	if(control_socket < 0) {
 		logger(LOG_ERR, _("Creating UNIX socket failed: %s"), strerror(errno));
-		return false;
+		goto bail;
+	}
+
+	/*
+	 * Restrict connections to our control socket by ensuring the parent
+	 * directory can be traversed only by root. Note this is not totally
+	 * race-free unless all ancestors are writable only by trusted users,
+	 * which we don't verify.
+	 */
+
+	struct stat statbuf;
+	lastslash = strrchr(controlsocketname, '/');
+	if(lastslash != NULL) {
+		*lastslash = 0; /* temporarily change controlsocketname to be dir */
+		if(mkdir(controlsocketname, 0700) < 0 && errno != EEXIST) {
+			logger(LOG_ERR, _("Unable to create control socket directory %s: %s"), controlsocketname, strerror(errno));
+			*lastslash = '/';
+			goto bail;
+		}
+
+		result = stat(controlsocketname, &statbuf);
+		*lastslash = '/';
+	} else
+		result = stat(".", &statbuf);
+
+	if(result < 0) {
+		logger(LOG_ERR, _("Examining control socket directory failed: %s"), strerror(errno));
+		goto bail;
+	}
+
+	if(statbuf.st_uid != 0 || (statbuf.st_mode & S_IXOTH) != 0 || (statbuf.st_gid != 0 && (statbuf.st_mode & S_IXGRP)) != 0) {
+		logger(LOG_ERR, _("Control socket directory ownership/permissions insecure."));
+		goto bail;
 	}
 
-	//unlink(controlsocketname);
 	result = bind(control_socket, (struct sockaddr *)&addr, sizeof addr);
-	
+
 	if(result < 0 && errno == EADDRINUSE) {
 		result = connect(control_socket, (struct sockaddr *)&addr, sizeof addr);
 		if(result < 0) {
@@ -240,33 +273,36 @@ bool init_control() {
 			unlink(controlsocketname);
 			result = bind(control_socket, (struct sockaddr *)&addr, sizeof addr);
 		} else {
-			close(control_socket);
 			if(netname)
 				logger(LOG_ERR, _("Another tincd is already running for net `%s'."), netname);
 			else
 				logger(LOG_ERR, _("Another tincd is already running."));
-			return false;
+			goto bail;
 		}
 	}
 
 	if(result < 0) {
-		logger(LOG_ERR, _("Can't bind to %s: %s\n"), controlsocketname, strerror(errno));
-		close(control_socket);
-		return false;
+		logger(LOG_ERR, _("Can't bind to %s: %s"), controlsocketname, strerror(errno));
+		goto bail;
 	}
 
 	if(listen(control_socket, 3) < 0) {
-		logger(LOG_ERR, _("Can't listen on %s: %s\n"), controlsocketname, strerror(errno));
-		close(control_socket);
-		return false;
+		logger(LOG_ERR, _("Can't listen on %s: %s"), controlsocketname, strerror(errno));
+		goto bail;
 	}
 
 	control_socket_tree = splay_alloc_tree((splay_compare_t)control_compare, (splay_action_t)bufferevent_free);
 
 	event_set(&control_event, control_socket, EV_READ | EV_PERSIST, handle_new_control_socket, NULL);
 	event_add(&control_event, NULL);
-
 	return true;
+
+bail:
+	if(control_socket != -1) {
+		close(control_socket);
+		control_socket = -1;
+	}
+	return false;
 }
 
 void exit_control() {
diff --git a/src/control_common.h b/src/control_common.h
index 09758266..6384651a 100644
--- a/src/control_common.h
+++ b/src/control_common.h
@@ -41,6 +41,7 @@ enum request_type {
 /* This greeting is sent by the server on socket open. */
 typedef struct tinc_ctl_greeting_t {
 	int version;
+	pid_t pid;
 } tinc_ctl_greeting_t;
 
 /* A single request or response header. */
diff --git a/src/tincctl.c b/src/tincctl.c
index 7e08629e..aed1c315 100644
--- a/src/tincctl.c
+++ b/src/tincctl.c
@@ -319,7 +319,7 @@ static void make_names(void) {
 #endif
 
 	if(!controlsocketname)
-		asprintf(&controlsocketname, LOCALSTATEDIR "/run/%s.control", identname);
+		asprintf(&controlsocketname, "%s/run/%s.control/socket", LOCALSTATEDIR, identname);
 
 	if(netname) {
 		if(!confbase)
@@ -439,10 +439,11 @@ static int send_ctl_request_cooked(int fd, enum request_type type,
 
 int main(int argc, char *argv[], char *envp[]) {
 	struct sockaddr_un addr;
-	int fd;
-	int len;
 	tinc_ctl_greeting_t greeting;
 	tinc_ctl_request_t req;
+	int fd;
+	int len;
+	int result;
 
 	program_name = argv[0];
 
@@ -491,7 +492,32 @@ int main(int argc, char *argv[], char *envp[]) {
 		return 1;
 	}
 
-	// Now handle commands that do involve connecting to a running tinc daemon.
+	/*
+	 * Now handle commands that do involve connecting to a running tinc daemon.
+	 * Authenticate the server by ensuring the parent directory can be
+	 * traversed only by root. Note this is not totally race-free unless all
+	 * ancestors are writable only by trusted users, which we don't verify.
+	 */
+
+	struct stat statbuf;
+	char *lastslash = strrchr(controlsocketname, '/');
+	if(lastslash != NULL) {
+		/* control socket is not in cwd; stat its parent */
+		*lastslash = 0;
+		result = stat(controlsocketname, &statbuf);
+		*lastslash = '/';
+	} else
+		result = stat(".", &statbuf);
+
+	if(result < 0) {
+		fprintf(stderr, _("Unable to check control socket directory permissions: %s\n"), strerror(errno));
+		return 1;
+	}
+
+	if(statbuf.st_uid != 0 || (statbuf.st_mode & S_IXOTH) != 0 || (statbuf.st_gid != 0 && (statbuf.st_mode & S_IXGRP)) != 0) {
+		fprintf(stderr, _("Insecure permissions on control socket directory\n"));
+		return 1;
+	}
 
 	if(strlen(controlsocketname) >= sizeof addr.sun_path) {
 		fprintf(stderr, _("Control socket filename too long!\n"));
@@ -525,16 +551,8 @@ int main(int argc, char *argv[], char *envp[]) {
 		return 1;
 	}
 
-	struct ucred cred;
-	socklen_t credlen = sizeof cred;
-
-	if(getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &cred, &credlen) < 0) {
-		fprintf(stderr, _("Could not obtain PID: %s\n"), strerror(errno));
-		return 1;
-	}
-
 	if(!strcasecmp(argv[optind], "pid")) {
-		printf("%d\n", cred.pid);
+		printf("%d\n", greeting.pid);
 		return 0;
 	}
 
diff --git a/src/tincd.c b/src/tincd.c
index c0be975e..20443101 100644
--- a/src/tincd.c
+++ b/src/tincd.c
@@ -218,7 +218,7 @@ static void make_names(void)
 #endif
 
 	if(!controlsocketname)
-		asprintf(&controlsocketname, LOCALSTATEDIR "/run/%s.control", identname);
+		asprintf(&controlsocketname, "%s/run/%s.control/socket", LOCALSTATEDIR, identname);
 
 	if(!logfilename)
 		asprintf(&logfilename, LOCALSTATEDIR "/log/%s.log", identname);