diff --git a/changes/bug2792_checkdir b/changes/bug2792_checkdir
new file mode 100644
index 0000000000..10de1deb2d
--- /dev/null
+++ b/changes/bug2792_checkdir
@@ -0,0 +1,8 @@
+ o Minor features:
+ - Tor now refuses to create a ControlSocket in a directory that is
+ world-readable (or group-readable if ControlSocketsGroupWritable
+ is 0). This is necessary because some operating systems do not
+ check the permissions on an AF_UNIX socket when programs try to
+ connect to it. Checking permissions on the directory holding
+ the socket, however, seems to work everywhere.
+
diff --git a/changes/bug2972 b/changes/bug2972
new file mode 100644
index 0000000000..26afcca421
--- /dev/null
+++ b/changes/bug2972
@@ -0,0 +1,5 @@
+ o Minor features:
+ - Allow ControlSockets to be group-writable when the
+ ControlSocksGroupWritable configuration option is turned on. Patch
+ by Jérémy Bobbio; implements ticket 2972.
+
diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index 74458ab811..7d72350eb8 100644
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -167,6 +167,11 @@ Other options can be specified either on the command-line (--option
Like ControlPort, but listens on a Unix domain socket, rather than a TCP
socket. (Unix and Unix-like systems only.)
+**ControlSocketsGroupWritable** **0**|**1**::
+ If this option is set to 0, don't allow the filesystem group to read and
+ write unix sockets (e.g. ControlSocket). If the option is set to 1, make
+ the control socket readable and writable by the default GID. (Default: 0)
+
**HashedControlPassword** __hashed_password__::
Don't allow any connections on the control port except when the other
process knows the password whose one-way hash is __hashed_password__. You
diff --git a/src/common/compat.c b/src/common/compat.c
index 436d882f30..1d00e30c4d 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -1522,6 +1522,45 @@ get_user_homedir(const char *username)
}
#endif
+/** Modify fname to contain the name of the directory */
+int
+get_parent_directory(char *fname)
+{
+ char *cp;
+ int at_end = 1;
+ tor_assert(fname);
+#ifdef MS_WINDOWS
+ /* If we start with, say, c:, then don't consider that the start of the path
+ */
+ if (fname[0] && fname[1] == ':') {
+ fname += 2;
+ }
+#endif
+ /* Now we want to remove all path-separators at the end of the string,
+ * and to remove the end of the string starting with the path separator
+ * before the last non-path-separator. In perl, this would be
+ * s#[/]*$##; s#/[^/]*$##;
+ * on a unixy platform.
+ */
+ cp = fname + strlen(fname);
+ at_end = 1;
+ while (--cp > fname) {
+ int is_sep = (*cp == '/'
+#ifdef MS_WINDOWS
+ || *cp == '\\'
+#endif
+ );
+ if (is_sep) {
+ *cp = '\0';
+ if (! at_end)
+ return 0;
+ } else {
+ at_end = 0;
+ }
+ }
+ return -1;
+}
+
/** Set *addr to the IP address (in dotted-quad notation) stored in c.
* Return 1 on success, 0 if c is badly formatted. (Like inet_aton(c,addr),
* but works on Windows and Solaris.)
diff --git a/src/common/compat.h b/src/common/compat.h
index b658ed51a0..66303c4810 100644
--- a/src/common/compat.h
+++ b/src/common/compat.h
@@ -557,6 +557,8 @@ int switch_id(const char *user);
char *get_user_homedir(const char *username);
#endif
+int get_parent_directory(char *fname);
+
int spawn_func(void (*func)(void *), void *data);
void spawn_exit(void) ATTR_NORETURN;
diff --git a/src/common/util.c b/src/common/util.c
index d2140c3f32..355b3ab34b 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -31,6 +31,7 @@
#else
#include
#include
+#include
#endif
/* math.h needs this on Linux */
@@ -1665,17 +1666,25 @@ file_status(const char *fname)
return FN_ERROR;
}
-/** Check whether dirname exists and is private. If yes return 0. If
- * it does not exist, and check==CPD_CREATE is set, try to create it
+/** Check whether dirname exists and is private. If yes return 0. If
+ * it does not exist, and check&CPD_CREATE is set, try to create it
* and return 0 on success. If it does not exist, and
- * check==CPD_CHECK, and we think we can create it, return 0. Else
- * return -1. */
+ * check&CPD_CHECK, and we think we can create it, return 0. Else
+ * return -1. If CPD_GROUP_OK is set, then it's okay if the directory
+ * is group-readable, but in all cases we create the directory mode 0700.
+ * If CPD_CHECK_MODE_ONLY is set, then we don't alter the directory permissions
+ * if they are too permissive: we just return -1.
+ */
int
check_private_dir(const char *dirname, cpd_check_t check)
{
int r;
struct stat st;
char *f;
+#ifndef MS_WINDOWS
+ int mask;
+#endif
+
tor_assert(dirname);
f = tor_strdup(dirname);
clean_name_for_stat(f);
@@ -1687,10 +1696,7 @@ check_private_dir(const char *dirname, cpd_check_t check)
strerror(errno));
return -1;
}
- if (check == CPD_NONE) {
- log_warn(LD_FS, "Directory %s does not exist.", dirname);
- return -1;
- } else if (check == CPD_CREATE) {
+ if (check & CPD_CREATE) {
log_info(LD_GENERAL, "Creating directory %s", dirname);
#if defined (MS_WINDOWS) && !defined (WINCE)
r = mkdir(dirname);
@@ -1702,6 +1708,9 @@ check_private_dir(const char *dirname, cpd_check_t check)
strerror(errno));
return -1;
}
+ } else if (!(check & CPD_CHECK)) {
+ log_warn(LD_FS, "Directory %s does not exist.", dirname);
+ return -1;
}
/* XXXX In the case where check==CPD_CHECK, we should look at the
* parent directory a little harder. */
@@ -1729,9 +1738,38 @@ check_private_dir(const char *dirname, cpd_check_t check)
tor_free(process_ownername);
return -1;
}
- if (st.st_mode & 0077) {
+ if ((check & CPD_GROUP_OK) && st.st_gid != getgid()) {
+ struct group *gr;
+ char *process_groupname = NULL;
+ gr = getgrgid(getgid());
+ process_groupname = gr ? tor_strdup(gr->gr_name) : tor_strdup("");
+ gr = getgrgid(st.st_gid);
+
+ log_warn(LD_FS, "%s is not owned by this group (%s, %d) but by group "
+ "%s (%d). Are you running Tor as the wrong user?",
+ dirname, process_groupname, (int)getgid(),
+ gr ? gr->gr_name : "", (int)st.st_gid);
+
+ tor_free(process_groupname);
+ return -1;
+ }
+ if (check & CPD_GROUP_OK) {
+ mask = 0027;
+ } else {
+ mask = 0077;
+ }
+ if (st.st_mode & mask) {
+ unsigned new_mode;
+ if (check & CPD_CHECK_MODE_ONLY) {
+ log_warn(LD_FS, "Permissions on directory %s are too permissive.",
+ dirname);
+ return -1;
+ }
log_warn(LD_FS, "Fixing permissions on directory %s", dirname);
- if (chmod(dirname, 0700)) {
+ new_mode = st.st_mode;
+ new_mode |= 0700; /* Owner should have rwx */
+ new_mode &= ~mask; /* Clear the other bits that we didn't want set...*/
+ if (chmod(dirname, new_mode)) {
log_warn(LD_FS, "Could not chmod directory %s: %s", dirname,
strerror(errno));
return -1;
diff --git a/src/common/util.h b/src/common/util.h
index 974b7d6fdc..e8d5de2ef4 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -286,7 +286,12 @@ file_status_t file_status(const char *filename);
/** Possible behaviors for check_private_dir() on encountering a nonexistent
* directory; see that function's documentation for details. */
-typedef enum { CPD_NONE, CPD_CREATE, CPD_CHECK } cpd_check_t;
+typedef unsigned int cpd_check_t;
+#define CPD_NONE 0
+#define CPD_CREATE 1
+#define CPD_CHECK 2
+#define CPD_GROUP_OK 4
+#define CPD_CHECK_MODE_ONLY 8
int check_private_dir(const char *dirname, cpd_check_t check);
#define OPEN_FLAGS_REPLACE (O_WRONLY|O_CREAT|O_TRUNC)
#define OPEN_FLAGS_APPEND (O_WRONLY|O_CREAT|O_APPEND)
diff --git a/src/or/config.c b/src/or/config.c
index cc775e6105..78cc4772e9 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -214,6 +214,7 @@ static config_var_t _option_vars[] = {
V(ControlPortFileGroupReadable,BOOL, "0"),
V(ControlPortWriteToFile, FILENAME, NULL),
V(ControlSocket, LINELIST, NULL),
+ V(ControlSocketsGroupWritable, BOOL, "0"),
V(CookieAuthentication, BOOL, "0"),
V(CookieAuthFileGroupReadable, BOOL, "0"),
V(CookieAuthFile, STRING, NULL),
@@ -974,9 +975,15 @@ options_act_reversible(or_options_t *old_options, char **msg)
}
#ifndef HAVE_SYS_UN_H
- if (options->ControlSocket) {
- *msg = tor_strdup("Unix domain sockets (ControlSocket) not supported"
- " on this OS/with this build.");
+ if (options->ControlSocket || options->ControlSocketsGroupWritable) {
+ *msg = tor_strdup("Unix domain sockets (ControlSocket) not supported "
+ "on this OS/with this build.");
+ goto rollback;
+ }
+#else
+ if (options->ControlSocketsGroupWritable && !options->ControlSocket) {
+ *msg = tor_strdup("Setting ControlSocketGroupWritable without setting"
+ "a ControlSocket makes no sense.");
goto rollback;
}
#endif
diff --git a/src/or/connection.c b/src/or/connection.c
index 099482bf78..17411e01f8 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -896,6 +896,43 @@ warn_too_many_conns(void)
}
}
+#ifdef HAVE_SYS_UN_H
+/** Check whether we should be willing to open an AF_UNIX socket in
+ * path. Return 0 if we should go ahead and -1 if we shouldn't. */
+static int
+check_location_for_unix_socket(or_options_t *options, const char *path)
+{
+ int r = -1;
+ char *p = tor_strdup(path);
+ cpd_check_t flags = CPD_CHECK_MODE_ONLY;
+ if (get_parent_directory(p)<0)
+ goto done;
+
+ if (options->ControlSocketsGroupWritable)
+ flags |= CPD_GROUP_OK;
+
+ if (check_private_dir(p, flags) < 0) {
+ char *escpath, *escdir;
+ escpath = esc_for_log(path);
+ escdir = esc_for_log(p);
+ log_warn(LD_GENERAL, "Before Tor can create a control socket in %s, the "
+ "directory %s needs to exist, and to be accessible only by the "
+ "user%s account that is running Tor. (On some Unix systems, "
+ "anybody who can list a socket can conect to it, so Tor is "
+ "being careful.)", escpath, escdir,
+ options->ControlSocketsGroupWritable ? " and group" : "");
+ tor_free(escpath);
+ tor_free(escdir);
+ goto done;
+ }
+
+ r = 0;
+ done:
+ tor_free(p);
+ return r;
+}
+#endif
+
/** Bind a new non-blocking socket listening to the socket described
* by listensockaddr.
*
@@ -990,6 +1027,9 @@ connection_create_listener(const struct sockaddr *listensockaddr,
* and listeners at the same time */
tor_assert(type == CONN_TYPE_CONTROL_LISTENER);
+ if (check_location_for_unix_socket(get_options(), address) < 0)
+ goto err;
+
log_notice(LD_NET, "Opening %s on %s",
conn_type_to_string(type), address);
@@ -1009,6 +1049,15 @@ connection_create_listener(const struct sockaddr *listensockaddr,
tor_socket_strerror(tor_socket_errno(s)));
goto err;
}
+ if (get_options()->ControlSocketsGroupWritable) {
+ /* We need to use chmod; fchmod doesn't work on sockets on all
+ * platforms. */
+ if (chmod(address, 0660) < 0) {
+ log_warn(LD_FS,"Unable to make %s group-writable.", address);
+ tor_close_socket(s);
+ goto err;
+ }
+ }
if (listen(s,SOMAXCONN) < 0) {
log_warn(LD_NET, "Could not listen on %s: %s", address,
diff --git a/src/or/or.h b/src/or/or.h
index c662719565..6c7430a4e2 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2611,6 +2611,7 @@ typedef struct {
int ControlPort; /**< Port to listen on for control connections. */
config_line_t *ControlSocket; /**< List of Unix Domain Sockets to listen on
* for control connections. */
+ int ControlSocketsGroupWritable; /**< Boolean: Are control sockets g+rw? */
int DirPort; /**< Port to listen on for directory connections. */
int DNSPort; /**< Port to listen on for DNS requests. */
int AssumeReachable; /**< Whether to publish our descriptor regardless. */
diff --git a/src/test/test_util.c b/src/test/test_util.c
index ee2d455b29..d338bbac9c 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -1208,6 +1208,35 @@ test_util_listdir(void *ptr)
}
}
+static void
+test_util_parent_dir(void *ptr)
+{
+ char *cp;
+ (void)ptr;
+
+#define T(input,expect_ok,output) \
+ do { \
+ int ok; \
+ cp = tor_strdup(input); \
+ ok = get_parent_directory(cp); \
+ tt_int_op(ok, ==, expect_ok); \
+ if (ok==0) \
+ tt_str_op(cp, ==, output); \
+ tor_free(cp); \
+ } while (0);
+
+ T("/home/wombat/knish", 0, "/home/wombat");
+ T("/home/wombat/knish/", 0, "/home/wombat");
+ T("./home/wombat/knish/", 0, "./home/wombat");
+ T("./wombat", 0, ".");
+ T("", -1, "");
+ T("/", -1, "");
+ T("////", -1, "");
+
+ done:
+ tor_free(cp);
+}
+
#ifdef MS_WINDOWS
static void
test_util_load_win_lib(void *ptr)
@@ -1495,6 +1524,7 @@ struct testcase_t util_tests[] = {
UTIL_TEST(find_str_at_start_of_line, 0),
UTIL_TEST(asprintf, 0),
UTIL_TEST(listdir, 0),
+ UTIL_TEST(parent_dir, 0),
#ifdef MS_WINDOWS
UTIL_TEST(load_win_lib, 0),
#endif