From fe2f1fceb546ca4326435cac26bcf3f513e82b43 Mon Sep 17 00:00:00 2001 From: Scott Lamb Date: Thu, 8 Nov 2007 19:18:44 +0000 Subject: [PATCH] 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. --- src/control.c | 62 ++++++++++++++++++++++++++++++++++---------- src/control_common.h | 1 + src/tincctl.c | 44 +++++++++++++++++++++---------- src/tincd.c | 2 +- 4 files changed, 82 insertions(+), 27 deletions(-) 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); -- 2.20.1