Scott Lamb
2007-Oct-18 19:26 UTC
[PATCH] Use a control socket directory to restrict access
This approach is more complex than I'd like, but it works even on Solaris,
which has neither credential passing nor permissions on the socket itself.
---
src/control.c | 82 +++++++++++++++++++++++++++++++++++++++-----------
src/control_common.h | 1 +
src/tincctl.c | 67 +++++++++++++++++++++++++++++++++--------
src/tincd.c | 2 +-
4 files changed, 120 insertions(+), 32 deletions(-)
diff --git a/src/control.c b/src/control.c
index a795843..bcf8861 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,52 +214,88 @@ static int control_compare(const struct event *a, const
struct event *b) {
bool init_control() {
int result;
struct sockaddr_un addr;
+ char *lastslash;
+ const char *controlsocketbasename = controlsocketname;
- if(strlen(controlsocketname) >= sizeof addr.sun_path) {
+ control_socket = socket(PF_UNIX, SOCK_STREAM, 0);
+
+ if(control_socket < 0) {
+ logger(LOG_ERR, _("Creating UNIX socket failed: %s"),
strerror(errno));
+ goto bail;
+ }
+
+ /*
+ * This is rather elaborate for security:
+ * - On Solaris, we need to restrict traversal into a socket's parent
+ * directory to restrict who can connect. Thus, we require
+ * uid=root, perms=0700. Create it if it doesn't exist.
+ * - We want to do the permission check in a race-free way to make sure
+ * no one switches the directories on us in the meantime, so we
+ * chdir() to the directory before checking permissions or binding.
+ */
+
+ 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;
+ }
+
+ if(chdir(controlsocketname) < 0) {
+ logger(LOG_ERR, _("Unable to enter control socket directory %s:
%s"), controlsocketname, strerror(errno));
+ *lastslash = '/';
+ goto bail;
+ }
+
+ controlsocketbasename = lastslash + 1;
+ }
+
+ if (strlen(controlsocketbasename) >= sizeof addr.sun_path) {
logger(LOG_ERR, _("Control socket filename too long!"));
- return false;
+ goto bail;
}
memset(&addr, 0, sizeof addr);
addr.sun_family = AF_UNIX;
- strncpy(addr.sun_path, controlsocketname, sizeof addr.sun_path - 1);
+ strncpy(addr.sun_path, controlsocketbasename, sizeof addr.sun_path - 1);
- control_socket = socket(PF_UNIX, SOCK_STREAM, 0);
+ struct stat statbuf;
+ if(stat(".", &statbuf) < 0) {
+ logger(LOG_ERR, _("Examining control socket directory failed: %s"),
strerror(errno));
+ goto bail;
+ }
- if(control_socket < 0) {
- logger(LOG_ERR, _("Creating UNIX socket failed: %s"),
strerror(errno));
- return false;
+ if(statbuf.st_uid != 0 || (statbuf.st_mode & 0077) != 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) {
logger(LOG_WARNING, _("Removing old control socket."));
- unlink(controlsocketname);
+ unlink(controlsocketbasename);
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);
@@ -266,7 +303,16 @@ bool init_control() {
event_set(&control_event, control_socket, EV_READ | EV_PERSIST,
handle_new_control_socket, NULL);
event_add(&control_event, NULL);
+ chdir("/");
return true;
+
+bail:
+ if(control_socket != -1) {
+ close(control_socket);
+ control_socket = -1;
+ }
+ chdir("/");
+ return false;
}
void exit_control() {
diff --git a/src/control_common.h b/src/control_common.h
index 0975826..6384651 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 7e08629..f46a643 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)
@@ -491,10 +491,49 @@ 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.
+ * Connecting is a bit tricky - we authenticate the server by ensuring
+ * the socket is in a directory only root can traverse. To do this in
+ * a race-free manner, we chdir() there temporarily, check permissions,
+ * and connect with a relative path.
+ */
+
+ int oldcwd_fd = -1;
+ char *controlsocketbasename = controlsocketname;
+ char *lastslash = strrchr(controlsocketname, '/');
+ if(lastslash != NULL) {
+ /* it's not in our current cwd; need to chdir */
+ if((oldcwd_fd = open(".", O_RDONLY)) < 0) {
+ fprintf(stderr, _("Unable to open current directory: %s\n"),
strerror(errno));
+ return 1;
+ }
+
+ *lastslash = 0;
+ if(chdir(controlsocketname) < 0) {
+ fprintf(stderr, _("Unable to enter control socket directory %s:
%s\n"), controlsocketname, strerror(errno));
+ return 1;
+ }
+ *lastslash = '/';
+ controlsocketbasename = lastslash+1;
+ }
+
+ struct stat statbuf;
+ if(stat(".", &statbuf) < 0) {
+ fprintf(stderr, _("Unable to check control socket directory permissions:
%s\n"), strerror(errno));
+ close(oldcwd_fd);
+ return 1;
+ }
+
+ if(statbuf.st_uid != 0 || (statbuf.st_mode & 0077) != 0) {
+ fprintf(stderr, _("Insecure permissions on control socket
directory\n"));
+ close(oldcwd_fd);
+ return 1;
+ }
- if(strlen(controlsocketname) >= sizeof addr.sun_path) {
+ if(strlen(controlsocketbasename) >= sizeof addr.sun_path) {
fprintf(stderr, _("Control socket filename too long!\n"));
+ close(oldcwd_fd);
return 1;
}
@@ -506,13 +545,23 @@ int main(int argc, char *argv[], char *envp[]) {
memset(&addr, 0, sizeof addr);
addr.sun_family = AF_UNIX;
- strncpy(addr.sun_path, controlsocketname, sizeof addr.sun_path - 1);
+ strncpy(addr.sun_path, controlsocketbasename, sizeof addr.sun_path - 1);
if(connect(fd, (struct sockaddr *)&addr, sizeof addr) < 0) {
fprintf(stderr, _("Cannot connect to %s: %s\n"), controlsocketname,
strerror(errno));
+ close(oldcwd_fd);
return 1;
}
+ if(oldcwd_fd >= 0) {
+ if(fchdir(oldcwd_fd) < 0) {
+ fprintf(stderr, _("Cannot restore working directory: %s\n"),
strerror(errno));
+ close(oldcwd_fd);
+ return 1;
+ }
+ close(oldcwd_fd);
+ }
+
if(fullread(fd, &greeting, sizeof greeting) == -1) {
fprintf(stderr, _("Cannot read greeting from control socket:
%s\n"),
strerror(errno));
@@ -525,16 +574,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 c0be975..2044310 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);
--
1.5.3.4.395.g85b0
--------------030901060304060800070308--
Maybe Matching Threads
Wisdom of the Ancients
