From 36505e0ea64b11a5e743ea6ff73515a13d1038db Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Thu, 14 Dec 2017 15:20:18 +0100 Subject: [PATCH] Monitor mount/umount events on the system and update app mount namespaces It is possible that file system path that has access guarded by a privilege is not available when application starts, but becomes available later. The reason for this is because a parent directory containing such path may be a mount point that is not yet mounted at the time when application starts. If the application doesn't hold privilege to the directory in question, it should have a dummy, empty directory mounted over that path. But this cannot be done properly when application starts and the privileged directory is not yet available. Later, while application is running, the parent mount point may be mounted. This mount will be propagated to mount namespaces of all running applications. Then the applications that do not hold the required privilege will be able to access privileged directory in that mount points, because dummy bind mount wasn't done. This patch implements a watcher keeping track of mount/unmount events in the system. When such event is detected, mount namespaces of all running applications will be reevaluated. If a privileged directory shows up in mount namespace of an already running application and the application doesn't hold required privilege, the directory will be hidden from the app. Change-Id: Idb7044d764a620b64666bfa5e6b1724b504866f0 Signed-off-by: Rafal Krypa --- packaging/security-manager.spec | 1 + src/client/CMakeLists.txt | 1 + src/common/CMakeLists.txt | 2 + src/common/include/mount-monitor.h | 113 ++++++++++++++++++++++ src/common/include/service_impl.h | 2 + src/common/mount-monitor.cpp | 193 +++++++++++++++++++++++++++++++++++++ src/common/service_impl.cpp | 15 ++- src/server/CMakeLists.txt | 1 + 8 files changed, 327 insertions(+), 1 deletion(-) create mode 100644 src/common/include/mount-monitor.h create mode 100644 src/common/mount-monitor.cpp diff --git a/packaging/security-manager.spec b/packaging/security-manager.spec index 1893fe9..057504f 100644 --- a/packaging/security-manager.spec +++ b/packaging/security-manager.spec @@ -33,6 +33,7 @@ BuildRequires: pkgconfig(cynara-admin) BuildRequires: pkgconfig(cynara-client-async) BuildRequires: pkgconfig(security-privilege-manager) BuildRequires: pkgconfig(openssl) +BuildRequires: pkgconfig(mount) BuildRequires: boost-devel %{?systemd_requires} diff --git a/src/client/CMakeLists.txt b/src/client/CMakeLists.txt index ca97d16..4bad7bc 100644 --- a/src/client/CMakeLists.txt +++ b/src/client/CMakeLists.txt @@ -4,6 +4,7 @@ PKG_CHECK_MODULES(CLIENT_DEP libsmack libcap libprocps + mount ) SET(CLIENT_VERSION_MAJOR 2) diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 145cb82..0f8f431 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -10,6 +10,7 @@ PKG_CHECK_MODULES(COMMON_DEP cynara-client-async libtzplatform-config security-privilege-manager + mount ) IF(DPL_WITH_DLOG) @@ -70,6 +71,7 @@ SET(COMMON_SOURCES ${COMMON_PATH}/privilege-gids.cpp ${COMMON_PATH}/group2gid.cpp ${COMMON_PATH}/mount-namespace.cpp + ${COMMON_PATH}/mount-monitor.cpp ${COMMON_PATH}/service-file-locker.cpp ) diff --git a/src/common/include/mount-monitor.h b/src/common/include/mount-monitor.h new file mode 100644 index 0000000..c298bcb --- /dev/null +++ b/src/common/include/mount-monitor.h @@ -0,0 +1,113 @@ +/* + * Copyright (c) 2017 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Rafal Krypa + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ +/* + * @file mount-monitor.h + * @author Rafal Krypa + * @version 1.0 + * @brief Monitor mount/umount events + */ + +#pragma once + +#include + +#include +#include +#include +#include +#include +#include + +namespace SecurityManager { + +class MntFS { +public: + MntFS(const libmnt_fs *fs); + ~MntFS(); + + MntFS(const MntFS &) = delete; + MntFS(MntFS &&other); + MntFS & operator=(const MntFS &) = delete; + MntFS && operator=(MntFS &&other); + + std::string getSource() const; + std::string getTarget() const; + +private: + struct libmnt_fs *m_fs = nullptr; +}; + +class MntDiffEntry +{ +public: + + enum class Type { + MOUNT = MNT_TABDIFF_MOUNT, + UMOUNT = MNT_TABDIFF_UMOUNT, + MOVE = MNT_TABDIFF_MOVE, + REMOUNT = MNT_TABDIFF_REMOUNT + }; + + Type m_type; + MntFS m_oldFS, m_newFS; + + MntDiffEntry(int type, const libmnt_fs *oldFS, const libmnt_fs *newFS) : + m_type(static_cast(type)), m_oldFS(oldFS), m_newFS(newFS) + {} +}; + +class MntTable { +public: + MntTable(const std::string &file = "/proc/self/mountinfo"); + ~MntTable(); + + MntTable(const MntTable &) = delete; + MntTable(MntTable &&other); + MntTable & operator=(const MntTable &) = delete; + MntTable & operator=(MntTable &&other); + + std::vector diff(const MntTable &other); + +private: + libmnt_table *m_table = nullptr; +}; + +class MntMonitor +{ +public: + MntMonitor(const std::function &callback); + ~MntMonitor(); + +private: + void threadNotifyPut(); + void threadNotifyGet(); + void run(); + + MntTable m_table; + struct libmnt_monitor *m_monitor; + int m_monitorFd; + + std::function m_callback; + + std::mutex m_monitorMut; + std::thread m_thread; + std::atomic m_terminate; + const int m_eventFd; +}; + +} // namespace SecurityManager diff --git a/src/common/include/service_impl.h b/src/common/include/service_impl.h index fa9c684..fc2e360 100644 --- a/src/common/include/service_impl.h +++ b/src/common/include/service_impl.h @@ -31,6 +31,7 @@ #include "channel.h" #include "credentials.h" #include "cynara.h" +#include "mount-monitor.h" #include "security-manager.h" #include "smack-rules.h" #include "protocols.h" @@ -374,6 +375,7 @@ private: CynaraAdmin m_cynaraAdmin; PrivilegeGids m_privilegeGids; Channel m_channel; + MntMonitor *m_mntMonitor; }; } /* namespace SecurityManager */ diff --git a/src/common/mount-monitor.cpp b/src/common/mount-monitor.cpp new file mode 100644 index 0000000..0d424fc --- /dev/null +++ b/src/common/mount-monitor.cpp @@ -0,0 +1,193 @@ +/* + * Copyright (c) 2017 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Rafal Krypa + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ +/* + * @file mount-monitor.cpp + * @author Rafal Krypa + * @version 1.0 + * @brief Monitor mount/umount events + */ + +#include + +#include +#include +#include + +#include +#include + +namespace SecurityManager { + +MntFS::MntFS(const libmnt_fs *fs) +{ + if (fs != nullptr) { + m_fs = mnt_copy_fs(nullptr, fs); + if (m_fs == nullptr) + throw std::bad_alloc(); + } else { + m_fs = nullptr; + } +} + +MntFS::MntFS(MntFS &&other) +{ + if (m_fs != nullptr) + mnt_unref_fs(m_fs); + m_fs = other.m_fs; + other.m_fs = nullptr; +} + +MntFS::~MntFS() +{ + if (m_fs != nullptr) + mnt_unref_fs(m_fs); +} + +std::string MntFS::getSource() const +{ + return mnt_fs_get_source(m_fs); +} + +std::string MntFS::getTarget() const +{ + return mnt_fs_get_target(m_fs); +} + +MntTable::MntTable(const std::string &file) +{ + m_table = mnt_new_table_from_file(file.c_str()); + if (m_table == nullptr) + throw std::bad_alloc(); +} + +MntTable::~MntTable() +{ + if (m_table != nullptr) + mnt_free_table(m_table); +} + +MntTable& MntTable::operator=(MntTable &&other) +{ + if (m_table != nullptr) + mnt_free_table(m_table); + m_table = other.m_table; + other.m_table = nullptr; + + return *this; +} + +std::vector MntTable::diff(const MntTable &other) +{ + std::vector entries; + + struct libmnt_tabdiff *diff = mnt_new_tabdiff(); + if (diff == nullptr) + throw std::bad_alloc(); + auto diffPtr = makeUnique(diff, mnt_free_tabdiff); + + if (mnt_diff_tables(diff, m_table, other.m_table) < 0) + throw std::bad_alloc(); + + struct libmnt_iter *itr = mnt_new_iter(MNT_ITER_FORWARD); + if (itr == nullptr) + throw std::bad_alloc(); + auto itrPtr = makeUnique(itr, mnt_free_iter); + + int type; + struct libmnt_fs *fs_old, *fs_new; + while (mnt_tabdiff_next_change(diff, itr, &fs_old, &fs_new, &type) == 0) + entries.emplace_back(type, fs_old, fs_new); + + return entries; +} + +MntMonitor::MntMonitor(const std::function &callback) : + m_callback(callback), m_terminate(false), m_eventFd(eventfd(0, 0)) +{ + m_monitor = mnt_new_monitor(); + if (m_monitor == nullptr) + throw std::bad_alloc(); + mnt_monitor_enable_kernel(m_monitor, true); + m_monitorFd = mnt_monitor_get_fd(m_monitor); + + m_thread = std::thread(&MntMonitor::run, this); +} + +MntMonitor::~MntMonitor() +{ + m_terminate = true; + threadNotifyPut(); + m_thread.join(); + + // Critical section + std::lock_guard guard(m_monitorMut); + mnt_unref_monitor(m_monitor); +} + +void MntMonitor::threadNotifyPut() +{ + int ret = eventfd_write(m_eventFd, 1); + if (ret == -1) + LogError("Unexpected error while writing to eventfd: " << GetErrnoString(errno)); +} + +void MntMonitor::threadNotifyGet() +{ + eventfd_t value; + int ret = eventfd_read(m_eventFd, &value); + if (ret == -1) + LogError("Unexpected error while reading from eventfd: " << GetErrnoString(errno)); +} + +void MntMonitor::run() +{ + while (true) { + struct pollfd pollFds[2] = {{m_eventFd, POLLIN, 0}, {m_monitorFd, POLLIN, 0}}; + int ret = poll(pollFds, 2, -1); + + if (ret == -1) { + if (errno != EINTR) + //LogError("Unexpected error returned by poll: " << GetErrnoString(errno)); + continue; + } + + // Check eventfd for termination signal + if (pollFds[0].revents) { + threadNotifyGet(); + if (m_terminate) { + //LogInfo("MntMonitor thread terminated"); + return; + } + } + + pollFds[1].revents = 0; + // Critical section + std::lock_guard guard(m_monitorMut); + + while (mnt_monitor_next_change(m_monitor, nullptr, nullptr) == 0) { + MntTable tableNew; + std::vector diff = m_table.diff(tableNew); + m_table = std::move(tableNew); + + for (auto &e : diff) + m_callback(e); + } + } +} + +} /* namespace SecurityManager */ diff --git a/src/common/service_impl.cpp b/src/common/service_impl.cpp index 9ff2ba4..7142d68 100644 --- a/src/common/service_impl.cpp +++ b/src/common/service_impl.cpp @@ -161,10 +161,23 @@ ServiceImpl::ServiceImpl() PrivilegeGids::GroupPrivileges group_privileges; m_privilegeDb.GetGroupsRelatedPrivileges(group_privileges); m_privilegeGids.init(group_privileges); + + if (MountNS::isMountNamespaceEnabled()) + m_mntMonitor = new MntMonitor([this](MntDiffEntry &e) { + // TODO: react only on mount events on directories related to a privilege + // TODO: filter out mount events that are generated by security-manager itself + if (e.m_type == MntDiffEntry::Type::MOUNT || e.m_type == MntDiffEntry::Type::UMOUNT) + NSMountLogic(m_cynara, m_channel).check(); + }); + else + m_mntMonitor = nullptr; } ServiceImpl::~ServiceImpl() -{} +{ + if (m_mntMonitor != nullptr) + delete m_mntMonitor; +} int ServiceImpl::validatePolicy(const Credentials &creds, policy_entry &policyEntry, CynaraAdminPolicy &cyap) { diff --git a/src/server/CMakeLists.txt b/src/server/CMakeLists.txt index a38d5eb..4e56516 100644 --- a/src/server/CMakeLists.txt +++ b/src/server/CMakeLists.txt @@ -2,6 +2,7 @@ PKG_CHECK_MODULES(SERVER_DEP REQUIRED libsystemd cynara-client + mount ) FIND_PACKAGE(Threads REQUIRED) -- 2.7.4