You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2016/01/16 19:56:56 UTC
trafficserver git commit: TS-4134: Fix privilege elevation issues.
Repository: trafficserver
Updated Branches:
refs/heads/master fd08c6efc -> 2c1bdddc5
TS-4134: Fix privilege elevation issues.
This closes #425.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/2c1bdddc
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/2c1bdddc
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/2c1bdddc
Branch: refs/heads/master
Commit: 2c1bdddc5e0946ef2fca0d821603e03f3d19cef2
Parents: fd08c6e
Author: Alan M. Carroll <am...@apache.org>
Authored: Thu Jan 14 19:28:44 2016 -0600
Committer: James Peach <jp...@apache.org>
Committed: Sat Jan 16 10:56:36 2016 -0800
----------------------------------------------------------------------
iocore/net/SSLUtils.cc | 4 +-
lib/ts/BaseLogFile.cc | 7 +--
lib/ts/Diags.cc | 7 +--
lib/ts/ink_cap.cc | 97 +++++++++++++++++++++++++-----------
lib/ts/ink_cap.h | 23 +++++++--
mgmt/LocalManager.cc | 3 +-
mgmt/Rollback.cc | 4 +-
proxy/Main.cc | 18 -------
proxy/Plugin.cc | 4 +-
proxy/http/remap/RemapConfig.cc | 4 +-
10 files changed, 100 insertions(+), 71 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2c1bdddc/iocore/net/SSLUtils.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
index 03d1b4e..91d5486 100644
--- a/iocore/net/SSLUtils.cc
+++ b/iocore/net/SSLUtils.cc
@@ -1874,12 +1874,10 @@ SSLParseCertificateConfiguration(const SSLConfigParams *params, SSLCertLookup *l
return false;
}
-#if TS_USE_POSIX_CAP
// elevate/allow file access to root read only files/certs
uint32_t elevate_setting = 0;
REC_ReadConfigInteger(elevate_setting, "proxy.config.ssl.cert.load_elevated");
- ElevateAccess elevate_access(elevate_setting != 0); // destructor will demote for us
-#endif /* TS_USE_POSIX_CAP */
+ ElevateAccess elevate_access(elevate_setting ? ElevateAccess::FILE_PRIVILEGE : 0); // destructor will demote for us
line = tokLine(file_buf, &tok_state);
while (line != NULL) {
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2c1bdddc/lib/ts/BaseLogFile.cc
----------------------------------------------------------------------
diff --git a/lib/ts/BaseLogFile.cc b/lib/ts/BaseLogFile.cc
index f134269..349da82 100644
--- a/lib/ts/BaseLogFile.cc
+++ b/lib/ts/BaseLogFile.cc
@@ -323,7 +323,8 @@ BaseLogFile::open_file(int perm)
// open actual log file (not metainfo)
log_log_trace("BaseLogFile: attempting to open %s\n", m_name.get());
- m_fp = fopen(m_name.get(), "a+");
+
+ m_fp = elevating_fopen(m_name.get(), "a+");
// error check
if (m_fp) {
@@ -478,7 +479,7 @@ void
BaseMetaInfo::_read_from_file()
{
_flags |= DATA_FROM_METAFILE; // mark attempt
- int fd = open(_filename, O_RDONLY);
+ int fd = elevating_open(_filename, O_RDONLY);
if (fd < 0) {
log_log_error("Could not open metafile %s for reading: %s\n", _filename, strerror(errno));
} else {
@@ -522,7 +523,7 @@ BaseMetaInfo::_read_from_file()
void
BaseMetaInfo::_write_to_file()
{
- int fd = open(_filename, O_WRONLY | O_CREAT | O_TRUNC, LOGFILE_DEFAULT_PERMS);
+ int fd = elevating_open(_filename, O_WRONLY | O_CREAT | O_TRUNC, LOGFILE_DEFAULT_PERMS);
if (fd < 0) {
log_log_error("Could not open metafile %s for writing: %s\n", _filename, strerror(errno));
return;
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2c1bdddc/lib/ts/Diags.cc
----------------------------------------------------------------------
diff --git a/lib/ts/Diags.cc b/lib/ts/Diags.cc
index 7e5747b..cda43b0 100644
--- a/lib/ts/Diags.cc
+++ b/lib/ts/Diags.cc
@@ -821,9 +821,6 @@ Diags::set_stdout_output(const char *_bind_stdout)
stdout_log = NULL;
}
- // get root
- ElevateAccess elevate;
-
// create backing BaseLogFile for stdout
stdout_log = new BaseLogFile(_bind_stdout);
@@ -860,9 +857,7 @@ Diags::set_stderr_output(const char *_bind_stderr)
delete stderr_log;
stderr_log = NULL;
}
- // get root
- ElevateAccess elevate;
-
+
// create backing BaseLogFile for stdout
stderr_log = new BaseLogFile(_bind_stderr);
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2c1bdddc/lib/ts/ink_cap.cc
----------------------------------------------------------------------
diff --git a/lib/ts/ink_cap.cc b/lib/ts/ink_cap.cc
index 01e5d5e..625903d 100644
--- a/lib/ts/ink_cap.cc
+++ b/lib/ts/ink_cap.cc
@@ -307,37 +307,74 @@ EnableDeathSignal(int signum)
#endif
}
+int
+elevating_open(char const *path, unsigned int flags, unsigned int fperms)
+{
+ int fd = open(path, flags, fperms);
+ if (fd < 0 && (EPERM == errno || EACCES == errno)) {
+ ElevateAccess access(ElevateAccess::FILE_PRIVILEGE);
+ fd = open(path, flags, fperms);
+ }
+ return fd;
+}
+
+int
+elevating_open(char const *path, unsigned int flags)
+{
+ int fd = open(path, flags);
+ if (fd < 0 && (EPERM == errno || EACCES == errno)) {
+ ElevateAccess access(ElevateAccess::FILE_PRIVILEGE);
+ fd = open(path, flags);
+ }
+ return fd;
+}
+
+FILE*
+elevating_fopen(char const *path, const char* mode)
+{
+ FILE* f = fopen(path, mode);
+ if (NULL == f && (EPERM == errno || EACCES == errno)) {
+ ElevateAccess access(ElevateAccess::FILE_PRIVILEGE);
+ f = fopen(path, mode);
+ }
+ return f;
+}
+
#if TS_USE_POSIX_CAP
/** Acquire file access privileges to bypass DAC.
@a level is a mask of the specific file access capabilities to acquire.
*/
void
-ElevateAccess::acquireFileAccessCap(unsigned level)
+ElevateAccess::acquirePrivilege(unsigned priv_mask)
{
unsigned cap_count = 0;
cap_value_t cap_list[2];
cap_t new_cap_state;
- Debug("privileges", "[acquireFileAccessCap] level= %x\n", level);
+ Debug("privileges", "[acquirePrivilege] level= %x\n", level);
ink_assert(NULL == cap_state);
- if (level) {
- this->cap_state = cap_get_proc(); // save current capabilities
- new_cap_state = cap_get_proc(); // and another instance to modify.
+ // Some privs aren't checked or used here because they are kept permanently in the
+ // the capability list. See @a eff_list in @c RestrictCapabilities
+ // It simplifies things elsewhere to be able to specify them so that the cases for
+ // POSIX capabilities and user impersonation have the same interface.
- if (level & ElevateAccess::FILE_PRIVILEGE) {
- cap_list[cap_count] = CAP_DAC_OVERRIDE;
- ++cap_count;
- }
+ if (priv_mask & ElevateAccess::FILE_PRIVILEGE) {
+ cap_list[cap_count] = CAP_DAC_OVERRIDE;
+ ++cap_count;
+ }
- if (level & ElevateAccess::TRACE_PRIVILEGE) {
- cap_list[cap_count] = CAP_SYS_PTRACE;
- ++cap_count;
- }
+ if (priv_mask & ElevateAccess::TRACE_PRIVILEGE) {
+ cap_list[cap_count] = CAP_SYS_PTRACE;
+ ++cap_count;
+ }
- ink_release_assert(cap_count <= sizeof(cap_list));
+ ink_release_assert(cap_count <= sizeof(cap_list));
+ if (cap_count > 0) {
+ this->cap_state = cap_get_proc(); // save current capabilities
+ new_cap_state = cap_get_proc(); // and another instance to modify.
cap_set_flag(new_cap_state, CAP_EFFECTIVE, cap_count, cap_list, CAP_SET);
if (cap_set_proc(new_cap_state) != 0) {
@@ -345,12 +382,13 @@ ElevateAccess::acquireFileAccessCap(unsigned level)
}
cap_free(new_cap_state);
+ elevated = true;
}
}
/** Restore previous capabilities.
*/
void
-ElevateAccess::releaseFileAccessCap()
+ElevateAccess::releasePrivilege()
{
Debug("privileges", "[releaseFileAccessCap]");
@@ -379,7 +417,7 @@ ElevateAccess::ElevateAccess(unsigned lvl)
ElevateAccess::~ElevateAccess()
{
- if (elevated == true) {
+ if (elevated) {
demote();
#if !TS_USE_POSIX_CAP
DEBUG_CREDENTIALS("privileges");
@@ -389,28 +427,31 @@ ElevateAccess::~ElevateAccess()
}
void
-ElevateAccess::elevate(unsigned level)
+ElevateAccess::elevate(unsigned priv_mask)
{
#if TS_USE_POSIX_CAP
- acquireFileAccessCap(level);
+ acquirePrivilege(priv_mask);
#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);
- ImpersonateUserID(0, IMPERSONATE_EFFECTIVE);
+ if (priv_mask) {
+ // 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);
+ ImpersonateUserID(0, IMPERSONATE_EFFECTIVE);
+ elevated = true;
+ }
#endif
- elevated = true;
}
void
ElevateAccess::demote()
{
+ if (elevated) {
#if TS_USE_POSIX_CAP
- releaseFileAccessCap();
+ releasePrivilege();
#else
- ImpersonateUserID(saved_uid, IMPERSONATE_EFFECTIVE);
- ink_mutex_release(&lock);
+ ImpersonateUserID(saved_uid, IMPERSONATE_EFFECTIVE);
+ ink_mutex_release(&lock);
#endif
- elevated = false;
+ elevated = false;
+ }
}
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2c1bdddc/lib/ts/ink_cap.h
----------------------------------------------------------------------
diff --git a/lib/ts/ink_cap.h b/lib/ts/ink_cap.h
index 19e94a4..0978965 100644
--- a/lib/ts/ink_cap.h
+++ b/lib/ts/ink_cap.h
@@ -38,6 +38,18 @@ extern bool PreserveCapabilities();
/// Initialize and restrict the capabilities of a thread.
/// @return true on success
extern bool RestrictCapabilities();
+/** Open a file, elevating privilege only if needed.
+
+ @internal This is necessary because the CI machines run the regression tests
+ as a normal user, not as root, so attempts to get privilege fail even though
+ the @c open would succeed without elevation. So, try that first and ask for
+ elevation only on an explicit permission failure.
+*/
+extern int elevating_open(char const *path, unsigned int flags, unsigned int fperms);
+/// Open a file, elevating privilege only if needed.
+extern int elevating_open(char const *path, unsigned int flags);
+/// Open a file, elevating privilege only if needed.
+extern FILE* elevating_fopen(char const *path, const char* mode);
/** Control generate of core file on crash.
@a flag sets whether core files are enabled on crash.
@@ -60,8 +72,9 @@ class ElevateAccess
{
public:
typedef enum {
- FILE_PRIVILEGE = 0x1u, // Access filesystem objects with privilege
- TRACE_PRIVILEGE = 0x2u // Trace other processes with privilege
+ FILE_PRIVILEGE = 0x1u, ///< Access filesystem objects with privilege
+ TRACE_PRIVILEGE = 0x2u, ///< Trace other processes with privilege
+ LOW_PORT_PRIVILEGE = 0x4u ///< Bind to privilege ports.
} privilege_level;
ElevateAccess(unsigned level = FILE_PRIVILEGE);
@@ -75,8 +88,10 @@ private:
uid_t saved_uid;
unsigned level;
- void acquireFileAccessCap(unsigned level);
- void releaseFileAccessCap();
+ /// Acquire the privileges marked in @a mask for this process.
+ void acquirePrivilege(unsigned priv_mask);
+ /// Restore the privilege set to the state before acquiring them.
+ void releasePrivilege();
#if !TS_USE_POSIX_CAP
static ink_mutex lock; // only one thread at a time can elevate
#else
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2c1bdddc/mgmt/LocalManager.cc
----------------------------------------------------------------------
diff --git a/mgmt/LocalManager.cc b/mgmt/LocalManager.cc
index 44cc63b..6036a7b 100644
--- a/mgmt/LocalManager.cc
+++ b/mgmt/LocalManager.cc
@@ -1021,8 +1021,9 @@ void
LocalManager::bindProxyPort(HttpProxyPort &port)
{
int one = 1;
+ int priv = (port.m_port < 1024 && 0 != geteuid()) ? ElevateAccess::LOW_PORT_PRIVILEGE : 0;
- ElevateAccess access(port.m_port < 1024 && geteuid() != 0);
+ ElevateAccess access(priv);
/* Setup reliable connection, for large config changes */
if ((port.m_fd = socket(port.m_family, SOCK_STREAM, 0)) < 0) {
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2c1bdddc/mgmt/Rollback.cc
----------------------------------------------------------------------
diff --git a/mgmt/Rollback.cc b/mgmt/Rollback.cc
index 217902c..f1361f4 100644
--- a/mgmt/Rollback.cc
+++ b/mgmt/Rollback.cc
@@ -260,7 +260,7 @@ Rollback::statFile(version_t version, struct stat *buf)
}
ats_scoped_str filePath(createPathStr(version));
- ElevateAccess access(root_access_needed);
+ ElevateAccess access(root_access_needed ? ElevateAccess::FILE_PRIVILEGE : 0);
statResult = stat(filePath, buf);
@@ -278,7 +278,7 @@ Rollback::openFile(version_t version, int oflags, int *errnoPtr)
int fd;
ats_scoped_str filePath(createPathStr(version));
- ElevateAccess access(root_access_needed);
+ ElevateAccess access(root_access_needed ? ElevateAccess::FILE_PRIVILEGE : 0);
// TODO: Use the original permissions
// Anyhow the _1 files should not be created inside Syconfdir.
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2c1bdddc/proxy/Main.cc
----------------------------------------------------------------------
diff --git a/proxy/Main.cc b/proxy/Main.cc
index 5fc8f86..45881e3 100644
--- a/proxy/Main.cc
+++ b/proxy/Main.cc
@@ -1422,24 +1422,6 @@ change_uid_gid(const char *user)
#endif
}
-/** Open a file, elevating privilege only if needed.
-
- @internal This is necessary because the CI machines run the regression tests
- as a normal user, not as root, so attempts to get privilege fail even though
- the @c open would succeed without elevation. So, try that first and ask for
- elevation only on an explicit permission failure.
-*/
-static int
-elevating_open(char const *path, unsigned int flags, unsigned int fperms)
-{
- int fd = open(path, flags, fperms);
- if (fd < 0 && (EPERM == errno || EACCES == errno)) {
- ElevateAccess access;
- fd = open(path, flags, fperms);
- }
- return fd;
-}
-
/*
* Binds stdout and stderr to files specified by the parameters
*
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2c1bdddc/proxy/Plugin.cc
----------------------------------------------------------------------
diff --git a/proxy/Plugin.cc b/proxy/Plugin.cc
index a621b92..b7b450c 100644
--- a/proxy/Plugin.cc
+++ b/proxy/Plugin.cc
@@ -94,11 +94,9 @@ plugin_load(int argc, char *argv[], bool validateOnly)
// elevate the access to read files as root if compiled with capabilities, if not
// change the effective user to root
{
-#if TS_USE_POSIX_CAP
uint32_t elevate_access = 0;
REC_ReadConfigInteger(elevate_access, "proxy.config.plugin.load_elevated");
- ElevateAccess access(elevate_access != 0);
-#endif /* TS_USE_POSIX_CAP */
+ ElevateAccess access(elevate_access ? ElevateAccess::FILE_PRIVILEGE : 0);
void *handle = dlopen(path, RTLD_NOW);
if (!handle) {
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2c1bdddc/proxy/http/remap/RemapConfig.cc
----------------------------------------------------------------------
diff --git a/proxy/http/remap/RemapConfig.cc b/proxy/http/remap/RemapConfig.cc
index 4a0e850..8b5f58e 100644
--- a/proxy/http/remap/RemapConfig.cc
+++ b/proxy/http/remap/RemapConfig.cc
@@ -760,11 +760,9 @@ remap_load_plugin(const char **argv, int argc, url_mapping *mp, char *errbuf, in
Debug("remap_plugin", "New remap plugin info created for \"%s\"", c);
{
-#if TS_USE_POSIX_CAP
uint32_t elevate_access = 0;
REC_ReadConfigInteger(elevate_access, "proxy.config.plugin.load_elevated");
- ElevateAccess access(elevate_access != 0);
-#endif /* TS_USE_POSIX_CAP */
+ ElevateAccess access(elevate_access ? ElevateAccess::FILE_PRIVILEGE : 0);
if ((pi->dlh = dlopen(c, RTLD_NOW)) == NULL) {
#if defined(freebsd) || defined(openbsd)