trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a..@apache.org
Subject trafficserver git commit: TS-306: Fix file privilege elevation to work with log rotation. This closes #322.
Date Thu, 05 Nov 2015 02:34:58 GMT
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 <amc@apache.org>
Authored: Wed Nov 4 17:54:23 2015 -0600
Committer: Alan M. Carroll <amc@apache.org>
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);


Mime
View raw message