Repository: trafficserver Updated Branches: refs/heads/master 7bcc3b4f6 -> 6a5f62411
TS-306: Fix file privilege elevation to work with log rotation. This closes #322. Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/6a5f6241 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/6a5f6241 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/6a5f6241 Branch: refs/heads/master Commit: 6a5f624117c95c11cf47755a92e84fe250c19d3e Parents: 7bcc3b4 Author: Alan M. Carroll <[email protected]> Authored: Wed Nov 4 17:54:23 2015 -0600 Committer: Alan M. Carroll <[email protected]> Committed: Wed Nov 4 20:32:32 2015 -0600 ---------------------------------------------------------------------- lib/ts/BaseLogFile.cc | 4 +- lib/ts/ink_cap.cc | 99 +++++++++++++++++++++++++--------------------- lib/ts/ink_cap.h | 8 +++- proxy/Main.cc | 19 ++++----- 4 files changed, 74 insertions(+), 56 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6a5f6241/lib/ts/BaseLogFile.cc ---------------------------------------------------------------------- diff --git a/lib/ts/BaseLogFile.cc b/lib/ts/BaseLogFile.cc index 76d5644..f134269 100644 --- a/lib/ts/BaseLogFile.cc +++ b/lib/ts/BaseLogFile.cc @@ -326,7 +326,9 @@ BaseLogFile::open_file(int perm) m_fp = fopen(m_name.get(), "a+"); // error check - if (!m_fp) { + if (m_fp) { + setlinebuf(m_fp); + } else { log_log_error("Error opening log file %s: %s\n", m_name.get(), strerror(errno)); log_log_error("Actual error: %s\n", (errno == EINVAL ? "einval" : "something else")); return LOG_FILE_COULD_NOT_OPEN_FILE; http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6a5f6241/lib/ts/ink_cap.cc ---------------------------------------------------------------------- diff --git a/lib/ts/ink_cap.cc b/lib/ts/ink_cap.cc index 91647f0..01e5d5e 100644 --- a/lib/ts/ink_cap.cc +++ b/lib/ts/ink_cap.cc @@ -308,63 +308,73 @@ EnableDeathSignal(int signum) } #if TS_USE_POSIX_CAP -/** Control file access privileges to bypass DAC. - @parm state Use @c true to enable elevated privileges, - @c false to disable. - @return @c true if successful, @c false otherwise. - - @internal After some pondering I decided that the file access - privilege was worth the effort of restricting. Unlike the network - privileges this can protect a host system from programming errors - by not (usually) permitting such errors to access arbitrary - files. This is particularly true since none of the config files - current enable this feature so it's not actually called. Still, - best to program defensively and have it available. +/** Acquire file access privileges to bypass DAC. + @a level is a mask of the specific file access capabilities to acquire. */ -static void -elevateFileAccess(unsigned level, bool state) +void +ElevateAccess::acquireFileAccessCap(unsigned level) { - Debug("privileges", "[elevateFileAccess] state : %d\n", state); - - cap_t cap_state = cap_get_proc(); // current capabilities - unsigned cap_count = 0; cap_value_t cap_list[2]; + cap_t new_cap_state; - if (level & ElevateAccess::FILE_PRIVILEGE) { - cap_list[cap_count] = CAP_DAC_OVERRIDE; - ++cap_count; - } + Debug("privileges", "[acquireFileAccessCap] level= %x\n", level); - if (level & ElevateAccess::TRACE_PRIVILEGE) { - cap_list[cap_count] = CAP_SYS_PTRACE; - ++cap_count; - } + ink_assert(NULL == cap_state); - ink_release_assert(cap_count <= sizeof(cap_list)); + if (level) { + this->cap_state = cap_get_proc(); // save current capabilities + new_cap_state = cap_get_proc(); // and another instance to modify. + + if (level & ElevateAccess::FILE_PRIVILEGE) { + cap_list[cap_count] = CAP_DAC_OVERRIDE; + ++cap_count; + } - cap_set_flag(cap_state, CAP_EFFECTIVE, cap_count, cap_list, state ? CAP_SET : CAP_CLEAR); - if (cap_set_proc(cap_state) != 0) { - Fatal("failed to %s privileged capabilities: %s", state ? "acquire" : "release", strerror(errno)); + if (level & ElevateAccess::TRACE_PRIVILEGE) { + cap_list[cap_count] = CAP_SYS_PTRACE; + ++cap_count; + } + + ink_release_assert(cap_count <= sizeof(cap_list)); + + cap_set_flag(new_cap_state, CAP_EFFECTIVE, cap_count, cap_list, CAP_SET); + + if (cap_set_proc(new_cap_state) != 0) { + Fatal("failed to acquire privileged capabilities: %s", strerror(errno)); + } + + cap_free(new_cap_state); } +} +/** Restore previous capabilities. + */ +void +ElevateAccess::releaseFileAccessCap() +{ + Debug("privileges", "[releaseFileAccessCap]"); - cap_free(cap_state); + if (this->cap_state) { + if (cap_set_proc(static_cast<cap_t>(cap_state)) != 0) { + Fatal("failed to restore privileged capabilities: %s", strerror(errno)); + } + cap_state = NULL; + } } #endif -ElevateAccess::ElevateAccess(const bool state, unsigned lvl) : elevated(false), saved_uid(geteuid()), level(lvl) +ElevateAccess::ElevateAccess(unsigned lvl) + : elevated(false), saved_uid(geteuid()), level(lvl) +#if TS_USE_POSIX_CAP + , + cap_state(0) +#endif { - // XXX Squash a clang [-Wunused-private-field] warning. The right solution is probably to extract - // the capabilities into a policy class. - (void)level; - - if (state == true) { - elevate(); + elevate(level); #if !TS_USE_POSIX_CAP - DEBUG_CREDENTIALS("privileges"); + DEBUG_CREDENTIALS("privileges"); #endif - DEBUG_PRIVILEGES("privileges"); - } + DEBUG_PRIVILEGES("privileges"); } ElevateAccess::~ElevateAccess() @@ -379,11 +389,12 @@ ElevateAccess::~ElevateAccess() } void -ElevateAccess::elevate() +ElevateAccess::elevate(unsigned level) { #if TS_USE_POSIX_CAP - elevateFileAccess(level, true); + acquireFileAccessCap(level); #else + (void)level; // Since we are setting a process-wide credential, we have to block any other thread // attempting to elevate until this one demotes. ink_mutex_acquire(&lock); @@ -396,7 +407,7 @@ void ElevateAccess::demote() { #if TS_USE_POSIX_CAP - elevateFileAccess(level, false); + releaseFileAccessCap(); #else ImpersonateUserID(saved_uid, IMPERSONATE_EFFECTIVE); ink_mutex_release(&lock); http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6a5f6241/lib/ts/ink_cap.h ---------------------------------------------------------------------- diff --git a/lib/ts/ink_cap.h b/lib/ts/ink_cap.h index 8499626..19e94a4 100644 --- a/lib/ts/ink_cap.h +++ b/lib/ts/ink_cap.h @@ -64,10 +64,10 @@ public: TRACE_PRIVILEGE = 0x2u // Trace other processes with privilege } privilege_level; - ElevateAccess(const bool state, unsigned level = FILE_PRIVILEGE); + ElevateAccess(unsigned level = FILE_PRIVILEGE); ~ElevateAccess(); - void elevate(); + void elevate(unsigned level); void demote(); private: @@ -75,8 +75,12 @@ private: uid_t saved_uid; unsigned level; + void acquireFileAccessCap(unsigned level); + void releaseFileAccessCap(); #if !TS_USE_POSIX_CAP static ink_mutex lock; // only one thread at a time can elevate +#else + void *cap_state; ///< Original capabilities state to restore. #endif }; http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6a5f6241/proxy/Main.cc ---------------------------------------------------------------------- diff --git a/proxy/Main.cc b/proxy/Main.cc index 5ebb87a..992eaca 100644 --- a/proxy/Main.cc +++ b/proxy/Main.cc @@ -1419,25 +1419,26 @@ change_uid_gid(const char *user) * has elevated file access. */ void -bind_outputs(const char *_bind_stdout, const char *_bind_stderr) +bind_outputs(const char *bind_stdout, const char *bind_stderr) { int log_fd; - if (*_bind_stdout != 0) { - Debug("log", "binding stdout to %s", _bind_stdout); - log_fd = open(_bind_stdout, O_WRONLY | O_APPEND | O_CREAT, 0644); + ElevateAccess access; + if (*bind_stdout != 0) { + Debug("log", "binding stdout to %s", bind_stdout); + log_fd = open(bind_stdout, O_WRONLY | O_APPEND | O_CREAT | O_SYNC, 0644); if (log_fd < 0) { - fprintf(stdout, "[Warning]: TS unable to open log file \"%s\" [%d '%s']\n", _bind_stdout, errno, strerror(errno)); + fprintf(stdout, "[Warning]: TS unable to open log file \"%s\" [%d '%s']\n", bind_stdout, errno, strerror(errno)); } else { Debug("log", "duping stdout"); dup2(log_fd, STDOUT_FILENO); close(log_fd); } } - if (*_bind_stderr != 0) { - Debug("log", "binding stderr to %s", _bind_stderr); - log_fd = open(_bind_stderr, O_WRONLY | O_APPEND | O_CREAT, 0644); + if (*bind_stderr != 0) { + Debug("log", "binding stderr to %s", bind_stderr); + log_fd = open(bind_stderr, O_WRONLY | O_APPEND | O_CREAT | O_SYNC, 0644); if (log_fd < 0) { - fprintf(stdout, "[Warning]: TS unable to open log file \"%s\" [%d '%s']\n", _bind_stderr, errno, strerror(errno)); + fprintf(stdout, "[Warning]: TS unable to open log file \"%s\" [%d '%s']\n", bind_stderr, errno, strerror(errno)); } else { Debug("log", "duping stderr"); dup2(log_fd, STDERR_FILENO);
