From e12af2adb0919d0de6d6ba44462d9255f63fca5b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 14 May 2014 13:50:43 -0400 Subject: Add a pair of wrapper functions: tor_getpwnam() and tor_getpwuid() We'll use these to deal with being unable to access the user DB after we install the sandbox, to fix bug 11946. --- src/common/compat.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) (limited to 'src/common/compat.c') diff --git a/src/common/compat.c b/src/common/compat.c index 7a444df106..9f31cceb09 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -1702,6 +1702,89 @@ log_credential_status(void) } #endif +#ifndef _WIN32 +static struct passwd *passwd_cached = NULL; + +static struct passwd * +tor_passwd_dup(const struct passwd *pw) +{ + struct passwd *new_pw = tor_malloc_zero(sizeof(struct passwd)); + if (pw->pw_name) + new_pw->pw_name = tor_strdup(pw->pw_name); + if (pw->pw_dir) + new_pw->pw_dir = tor_strdup(pw->pw_dir); + new_pw->pw_uid = pw->pw_uid; + new_pw->pw_gid = pw->pw_gid; + + return new_pw; +} + +static void +tor_passwd_free(struct passwd *pw) +{ + if (!pw) + return; + + tor_free(pw->pw_name); + tor_free(pw->pw_dir); + tor_free(pw); +} + +/** Wrapper around getpwnam() that caches result. Used so that we don't need + * to give the sandbox access to /etc/passwd. */ +const struct passwd * +tor_getpwnam(const char *username) +{ + struct passwd *pw; + + if (username == NULL) { + tor_passwd_free(passwd_cached); + passwd_cached = NULL; + return NULL; + } + + if ((pw = getpwnam(username))) { + tor_passwd_free(passwd_cached); + passwd_cached = tor_passwd_dup(pw); + log_notice(LD_GENERAL, "Caching new entry %s for %s", + passwd_cached->pw_name, username); + return pw; + } + + /* Lookup failed */ + if (! passwd_cached || ! passwd_cached->pw_name) + return NULL; + + if (! strcmp(username, passwd_cached->pw_name)) + return passwd_cached; + + return NULL; +} + +/** Wrapper around getpwnam() that can use cached result from + * tor_getpwnam(). Used so that we don't need to give the sandbox access to + * /etc/passwd. */ +const struct passwd * +tor_getpwuid(uid_t uid) +{ + struct passwd *pw; + + if ((pw = getpwuid(uid))) { + return pw; + } + + /* Lookup failed */ + if (! passwd_cached) + return NULL; + + if (uid == passwd_cached->pw_uid) + return passwd_cached; + + return NULL; +} +#endif + + /** Call setuid and setgid to run as user and switch to their * primary group. Return 0 on success. On failure, log and return -1. */ -- cgit v1.2.3-54-g00ecf From 9b4ac986cbe8867c24c8e77654a4b7e75f870738 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 14 May 2014 13:53:14 -0400 Subject: Use tor_getpw{nam,uid} wrappers to fix bug 11946 When running with User set, we frequently try to look up our information in the user database (e.g., /etc/passwd). The seccomp2 sandbox setup doesn't let us open /etc/passwd, and probably shouldn't. To fix this, we have a pair of wrappers for getpwnam and getpwuid. When a real call to getpwnam or getpwuid fails, they fall back to a cached value, if the uid/gid matches. (Granting access to /etc/passwd isn't possible with the way we handle opening files through the sandbox. It's not desirable either.) --- changes/bug11946 | 5 +++++ src/common/compat.c | 8 ++++---- src/common/util.c | 10 +++++----- src/or/connection.c | 4 ++-- src/or/control.c | 2 +- 5 files changed, 17 insertions(+), 12 deletions(-) create mode 100644 changes/bug11946 (limited to 'src/common/compat.c') diff --git a/changes/bug11946 b/changes/bug11946 new file mode 100644 index 0000000000..9ea48311bd --- /dev/null +++ b/changes/bug11946 @@ -0,0 +1,5 @@ + o Minor bugfixes (sandbox): + + - Handle failures in getpwnam()/getpwuid() when running with the + User option set and the Linux syscall sandbox enabled. Fixes bug + 11946; bugfix on 0.2.5.1-alpha. diff --git a/src/common/compat.c b/src/common/compat.c index 9f31cceb09..65446b530e 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -1792,7 +1792,7 @@ int switch_id(const char *user) { #ifndef _WIN32 - struct passwd *pw = NULL; + const struct passwd *pw = NULL; uid_t old_uid; gid_t old_gid; static int have_already_switched_id = 0; @@ -1813,7 +1813,7 @@ switch_id(const char *user) old_gid = getgid(); /* Lookup the user and group information, if we have a problem, bail out. */ - pw = getpwnam(user); + pw = tor_getpwnam(user); if (pw == NULL) { log_warn(LD_CONFIG, "Error setting configured user: %s not found", user); return -1; @@ -1984,10 +1984,10 @@ tor_disable_debugger_attach(void) char * get_user_homedir(const char *username) { - struct passwd *pw; + const struct passwd *pw; tor_assert(username); - if (!(pw = getpwnam(username))) { + if (!(pw = tor_getpwnam(username))) { log_err(LD_CONFIG,"User \"%s\" not found.", username); return NULL; } diff --git a/src/common/util.c b/src/common/util.c index d573b5624b..d457ba9701 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1871,7 +1871,7 @@ check_private_dir(const char *dirname, cpd_check_t check, char *f; #ifndef _WIN32 int mask; - struct passwd *pw = NULL; + const struct passwd *pw = NULL; uid_t running_uid; gid_t running_gid; #else @@ -1918,7 +1918,7 @@ check_private_dir(const char *dirname, cpd_check_t check, if (effective_user) { /* Look up the user and group information. * If we have a problem, bail out. */ - pw = getpwnam(effective_user); + pw = tor_getpwnam(effective_user); if (pw == NULL) { log_warn(LD_CONFIG, "Error setting configured user: %s not found", effective_user); @@ -1932,13 +1932,13 @@ check_private_dir(const char *dirname, cpd_check_t check, } if (st.st_uid != running_uid) { - struct passwd *pw = NULL; + const struct passwd *pw = NULL; char *process_ownername = NULL; - pw = getpwuid(running_uid); + pw = tor_getpwuid(running_uid); process_ownername = pw ? tor_strdup(pw->pw_name) : tor_strdup(""); - pw = getpwuid(st.st_uid); + pw = tor_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?", diff --git a/src/or/connection.c b/src/or/connection.c index 3cc4e09fb7..cef9172ff1 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1017,7 +1017,7 @@ connection_listener_new(const struct sockaddr *listensockaddr, tor_socket_t s = TOR_INVALID_SOCKET; /* the socket we're going to make */ or_options_t const *options = get_options(); #if defined(HAVE_PWD_H) && defined(HAVE_SYS_UN_H) - struct passwd *pw = NULL; + const struct passwd *pw = NULL; #endif uint16_t usePort = 0, gotPort = 0; int start_reading = 0; @@ -1157,7 +1157,7 @@ connection_listener_new(const struct sockaddr *listensockaddr, } #ifdef HAVE_PWD_H if (options->User) { - pw = getpwnam(options->User); + pw = tor_getpwnam(options->User); if (pw == NULL) { log_warn(LD_NET,"Unable to chown() %s socket: user %s not found.", address, options->User); diff --git a/src/or/control.c b/src/or/control.c index d571900ac3..2865d78329 100755 --- a/src/or/control.c +++ b/src/or/control.c @@ -1492,7 +1492,7 @@ getinfo_helper_misc(control_connection_t *conn, const char *question, *answer = tor_strdup(""); #else int myUid = geteuid(); - struct passwd *myPwEntry = getpwuid(myUid); + const struct passwd *myPwEntry = tor_getpwuid(myUid); if (myPwEntry) { *answer = tor_strdup(myPwEntry->pw_name); -- cgit v1.2.3-54-g00ecf From f694a443fc35e91d301470c60a7989d4fbd6de71 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 14 May 2014 22:49:38 -0400 Subject: Improved comments on bug11946 fix --- src/common/compat.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'src/common/compat.c') diff --git a/src/common/compat.c b/src/common/compat.c index 65446b530e..b95609a263 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -1703,8 +1703,14 @@ log_credential_status(void) #endif #ifndef _WIN32 +/** Cached struct from the last getpwname() call we did successfully. */ static struct passwd *passwd_cached = NULL; +/** Helper: copy a struct passwd object. + * + * We only copy the fields pw_uid, pw_gid, pw_name, pw_dir. Tor doesn't use + * any others, and I don't want to run into incompatibilities. + */ static struct passwd * tor_passwd_dup(const struct passwd *pw) { @@ -1719,6 +1725,7 @@ tor_passwd_dup(const struct passwd *pw) return new_pw; } +/** Helper: free one of our cached 'struct passwd' values. */ static void tor_passwd_free(struct passwd *pw) { @@ -1731,7 +1738,14 @@ tor_passwd_free(struct passwd *pw) } /** Wrapper around getpwnam() that caches result. Used so that we don't need - * to give the sandbox access to /etc/passwd. */ + * to give the sandbox access to /etc/passwd. + * + * The following fields alone will definitely be copied in the output: pw_uid, + * pw_gid, pw_name, pw_dir. Other fields are not present in cached values. + * + * When called with a NULL argument, this function clears storage associated + * with static variables it uses. + **/ const struct passwd * tor_getpwnam(const char *username) { @@ -1763,7 +1777,11 @@ tor_getpwnam(const char *username) /** Wrapper around getpwnam() that can use cached result from * tor_getpwnam(). Used so that we don't need to give the sandbox access to - * /etc/passwd. */ + * /etc/passwd. + * + * The following fields alone will definitely be copied in the output: pw_uid, + * pw_gid, pw_name, pw_dir. Other fields are not present in cached values. + */ const struct passwd * tor_getpwuid(uid_t uid) { -- cgit v1.2.3-54-g00ecf