From a69b103bdd34c1287e83439f154cced5877f3fe1 Mon Sep 17 00:00:00 2001 From: "Alan M. Carroll" Date: Wed, 4 Nov 2015 17:54:23 -0600 Subject: [PATCH] TS-306: Fix file privilege elevation to work with log rotation. --- 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(-) diff --git a/lib/ts/BaseLogFile.cc b/lib/ts/BaseLogFile.cc index 76d5644deab..f134269641d 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; diff --git a/lib/ts/ink_cap.cc b/lib/ts/ink_cap.cc index 91647f0dfbb..01e5d5ea1bb 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_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); diff --git a/lib/ts/ink_cap.h b/lib/ts/ink_cap.h index 84996263b68..19e94a473a5 100644 --- a/lib/ts/ink_cap.h +++ b/lib/ts/ink_cap.h @@ -64,10 +64,10 @@ class ElevateAccess 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 @@ class ElevateAccess 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 }; diff --git a/proxy/Main.cc b/proxy/Main.cc index 5ebb87a71f1..992eaca7f9f 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);