Commit 54d7d31c authored by Jérémy Bobbio's avatar Jérémy Bobbio Committed by Nick Mathewson
Browse files

Make ControlSocketsGroupWritable work with User.

Original message from bug3393:

check_private_dir() to ensure that ControlSocketsGroupWritable is
safe to use. Unfortunately, check_private_dir() only checks against
the currently running user… which can be root until privileges are
dropped to the user and group configured by the User config option.

The attached patch fixes the issue by adding a new effective_user
argument to check_private_dir() and updating the callers. It might
not be the best way to fix the issue, but it did in my tests.

(Code by lunar; changelog by nickm)
parent f3032744
......@@ -1677,15 +1677,20 @@ file_status(const char *fname)
* 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.
* When effective_user is not NULL, check permissions against the given user and
* its primary group.
*/
int
check_private_dir(const char *dirname, cpd_check_t check)
check_private_dir(const char *dirname, cpd_check_t check, const char *effective_user)
{
int r;
struct stat st;
char *f;
#ifndef MS_WINDOWS
int mask;
struct passwd *pw = NULL;
uid_t running_uid;
gid_t running_gid;
#endif
tor_assert(dirname);
......@@ -1724,33 +1729,47 @@ check_private_dir(const char *dirname, cpd_check_t check)
return -1;
}
#ifndef MS_WINDOWS
if (st.st_uid != getuid()) {
if (effective_user) {
/* Lookup the user and group information, if we have a problem, bail out. */
pw = getpwnam(effective_user);
if (pw == NULL) {
log_warn(LD_CONFIG, "Error setting configured user: %s not found", effective_user);
return -1;
}
running_uid = pw->pw_uid;
running_gid = pw->pw_gid;
} else {
running_uid = getuid();
running_gid = getgid();
}
if (st.st_uid != running_uid) {
struct passwd *pw = NULL;
char *process_ownername = NULL;
pw = getpwuid(getuid());
pw = getpwuid(running_uid);
process_ownername = pw ? tor_strdup(pw->pw_name) : tor_strdup("<unknown>");
pw = getpwuid(st.st_uid);
log_warn(LD_FS, "%s is not owned by this user (%s, %d) but by "
"%s (%d). Perhaps you are running Tor as the wrong user?",
dirname, process_ownername, (int)getuid(),
dirname, process_ownername, (int)running_uid,
pw ? pw->pw_name : "<unknown>", (int)st.st_uid);
tor_free(process_ownername);
return -1;
}
if ((check & CPD_GROUP_OK) && st.st_gid != getgid()) {
if ((check & CPD_GROUP_OK) && st.st_gid != running_gid) {
struct group *gr;
char *process_groupname = NULL;
gr = getgrgid(getgid());
gr = getgrgid(running_gid);
process_groupname = gr ? tor_strdup(gr->gr_name) : tor_strdup("<unknown>");
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(),
dirname, process_groupname, (int)running_gid,
gr ? gr->gr_name : "<unknown>", (int)st.st_gid);
tor_free(process_groupname);
......
......@@ -292,7 +292,8 @@ typedef unsigned int cpd_check_t;
#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);
int check_private_dir(const char *dirname, cpd_check_t check,
const char *effective_user);
#define OPEN_FLAGS_REPLACE (O_WRONLY|O_CREAT|O_TRUNC)
#define OPEN_FLAGS_APPEND (O_WRONLY|O_CREAT|O_APPEND)
typedef struct open_file_t open_file_t;
......
......@@ -1025,7 +1025,8 @@ options_act_reversible(or_options_t *old_options, char **msg)
/* Ensure data directory is private; create if possible. */
if (check_private_dir(options->DataDirectory,
running_tor ? CPD_CREATE : CPD_CHECK)<0) {
running_tor ? CPD_CREATE : CPD_CHECK,
options->User)<0) {
tor_asprintf(msg,
"Couldn't access/create private data directory \"%s\"",
options->DataDirectory);
......@@ -1038,7 +1039,8 @@ options_act_reversible(or_options_t *old_options, char **msg)
char *fn = tor_malloc(len);
tor_snprintf(fn, len, "%s"PATH_SEPARATOR"cached-status",
options->DataDirectory);
if (check_private_dir(fn, running_tor ? CPD_CREATE : CPD_CHECK) < 0) {
if (check_private_dir(fn, running_tor ? CPD_CREATE : CPD_CHECK,
options->User) < 0) {
tor_asprintf(msg,
"Couldn't access/create private data directory \"%s\"", fn);
tor_free(fn);
......
......@@ -867,7 +867,7 @@ check_location_for_unix_socket(or_options_t *options, const char *path)
if (options->ControlSocketsGroupWritable)
flags |= CPD_GROUP_OK;
if (check_private_dir(p, flags) < 0) {
if (check_private_dir(p, flags, options->User) < 0) {
char *escpath, *escdir;
escpath = esc_for_log(path);
escdir = esc_for_log(p);
......
......@@ -970,7 +970,7 @@ geoip_dirreq_stats_write(time_t now)
geoip_remove_old_clients(start_of_dirreq_stats_interval);
statsdir = get_datadir_fname("stats");
if (check_private_dir(statsdir, CPD_CREATE) < 0)
if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
goto done;
filename = get_datadir_fname2("stats", "dirreq-stats");
data_v2 = geoip_get_client_history(GEOIP_CLIENT_NETWORKSTATUS_V2);
......@@ -1209,7 +1209,7 @@ geoip_bridge_stats_write(time_t now)
/* Write it to disk. */
statsdir = get_datadir_fname("stats");
if (check_private_dir(statsdir, CPD_CREATE) < 0)
if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
goto done;
filename = get_datadir_fname2("stats", "bridge-stats");
......@@ -1304,7 +1304,7 @@ geoip_entry_stats_write(time_t now)
geoip_remove_old_clients(start_of_entry_stats_interval);
statsdir = get_datadir_fname("stats");
if (check_private_dir(statsdir, CPD_CREATE) < 0)
if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
goto done;
filename = get_datadir_fname2("stats", "entry-stats");
data = geoip_get_client_history(GEOIP_CLIENT_CONNECT);
......
......@@ -569,7 +569,7 @@ rend_service_load_keys(void)
s->directory);
/* Check/create directory */
if (check_private_dir(s->directory, CPD_CREATE) < 0)
if (check_private_dir(s->directory, CPD_CREATE, get_options()->User) < 0)
return -1;
/* Load key */
......
......@@ -2307,7 +2307,7 @@ rep_hist_exit_stats_write(time_t now)
/* Try to write to disk. */
statsdir = get_datadir_fname("stats");
if (check_private_dir(statsdir, CPD_CREATE) < 0) {
if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
log_warn(LD_HIST, "Unable to create stats/ directory!");
goto done;
}
......@@ -2497,7 +2497,7 @@ rep_hist_buffer_stats_write(time_t now)
smartlist_clear(circuits_for_buffer_stats);
/* write to file */
statsdir = get_datadir_fname("stats");
if (check_private_dir(statsdir, CPD_CREATE) < 0)
if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
goto done;
filename = get_datadir_fname2("stats", "buffer-stats");
out = start_writing_to_stdio_file(filename, OPEN_FLAGS_APPEND,
......
......@@ -533,12 +533,12 @@ init_keys(void)
return 0;
}
/* Make sure DataDirectory exists, and is private. */
if (check_private_dir(options->DataDirectory, CPD_CREATE)) {
if (check_private_dir(options->DataDirectory, CPD_CREATE, options->User)) {
return -1;
}
/* Check the key directory. */
keydir = get_datadir_fname("keys");
if (check_private_dir(keydir, CPD_CREATE)) {
if (check_private_dir(keydir, CPD_CREATE, options->User)) {
tor_free(keydir);
return -1;
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment