From: Konrad Lipinski Date: Tue, 21 Apr 2020 17:32:17 +0000 (+0200) Subject: Reimplement prepare_app proper drop checking X-Git-Tag: submit/tizen/20200710.130420~2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=03bd5c1275d6c3ffcaab1718776f15c91fb01754;p=platform%2Fcore%2Fsecurity%2Fsecurity-manager.git Reimplement prepare_app proper drop checking Procps-ng does not reliably check for errors. They are for the most part silently ignored. The only way to approximately check for success is by checking errno. That's what we've been doing up till now. However, errno is not mentioned in the contract at all. Syscalls that succeed may zero errno and mask prior errors. Pre-3.12 kernels require CAP_SYS_PTRACE for task namespace inspection. In particular, contemporary TM1 images feature a 3.10 kernel. On such devices, PROC_FILLNS may result in errno being set to EACCES (unless overwritten as per the previous paragraph). Such is the case on TM1, making CheckProperDrop::checkThreads() fail whenever there are two or more threads. Checking for identical caps is not enough to ensure proper drop. A rogue thread may survive sync_threads_internal() (which is racy by nature), use capset() to set main thread's caps to zero, then terminate before CheckProperDrop::getThreads() starts due to a lucky interleaving. This can be guarded against by mandating capabilities to be zeroed for all threads. * Replace procps-ng usage with local code. * Assert zero caps instead of identical caps. * Refrain from checking pid and user namespaces, kernel guarantees consistency across threads (see man unshare(2)). * Compute the set of checked namespace kinds as a bitmask at manager startup, ipc the bitmask to clients in prepare_app return payload. * Set bitmask to zero for pre-3.12 kernels that require CAP_SYS_PTRACE for task namespace inspection. * Disable compilation of test_check_proper_drop.cpp. The tests were written under the assumption that caps do not have to be zeroed. This is no longer the case. Zeroing caps requires fork support, there are also new edge cases to test. This makes the needed change substantial. By review request it will be included in a future commit. Change-Id: I4814cfd92dc524c02d87926236d8beb97d633c82 --- diff --git a/packaging/security-manager.spec b/packaging/security-manager.spec index 72886b64..69aec952 100644 --- a/packaging/security-manager.spec +++ b/packaging/security-manager.spec @@ -27,7 +27,6 @@ BuildRequires: cmake BuildRequires: zip BuildRequires: pkgconfig(dlog) BuildRequires: libattr-devel -BuildRequires: pkgconfig(libprocps) BuildRequires: pkgconfig(libsmack) BuildRequires: pkgconfig(libcap) BuildRequires: pkgconfig(libsystemd-daemon) diff --git a/src/client/CMakeLists.txt b/src/client/CMakeLists.txt index 8862ca0b..4d54fa75 100644 --- a/src/client/CMakeLists.txt +++ b/src/client/CMakeLists.txt @@ -3,7 +3,6 @@ PKG_CHECK_MODULES(CLIENT_DEP cynara-client-async libsmack libcap - libprocps mount ) diff --git a/src/client/check-proper-drop.cpp b/src/client/check-proper-drop.cpp index 6566f3db..753aa642 100644 --- a/src/client/check-proper-drop.cpp +++ b/src/client/check-proper-drop.cpp @@ -1,7 +1,7 @@ /* * Copyright (c) 2015-2020 Samsung Electronics Co., Ltd. All rights reserved * - * Contact: Rafal Krypa + * Contact: Tomasz Swierczek * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,189 +16,142 @@ * limitations under the License */ /* - * @file check-proper-drop.cpp - * @author Rafal Krypa - * @version 1.0 - * @brief Implementation of proper privilege dropping check utilities + * @brief Implementation of proper privilege dropping check utilities, client side */ -#include "check-proper-drop.h" -#include "smack-labels.h" -#include "dpl/log/log.h" -#include "utils.h" +#include +#include +#include +#include +#include -#include -#include +#include +#include +#include +#include +#include -#include -#include -#include +namespace SecurityManager { +namespace CheckProperDrop { namespace { -/* This function is a simplified reimplementation of readtask() from - * procps-ng. The aformentioned function does not check for error code - * from taskreader (simple_readtask()) assuming it's always a harmless - * ENOENT. It can be anything other than that. ENOMEM for instance. Or - * in our case EACCES meaning we don't have access to the thread and - * cannot check it. This function does not ignore errors. - * - * It will return NULL with errno == 0 when there are no other tasks - * to check or NULL with errno != 0 in case of an error. - */ -proc_t* readtask_priv(PROCTAB * const PT, const proc_t * const p) -{ - char path[PROCPATHLEN]; - proc_t *ret; - proc_t *t; - - t = (proc_t*)calloc(1, sizeof(*t)); - if (t == NULL) - return NULL; - - for(;;) { - if (unlikely(!PT->taskfinder(PT,p,t,path))) - goto out; - - errno = 0; - ret = PT->taskreader(PT,p,t,path); - if (errno && errno != ENOENT) - goto out; - errno = 0; - if (ret != NULL) - return ret; +DECLARE_EXCEPTION_TYPE(SecurityManager::Exception, DropError) + +class TaskData final { + ::std::string m_label; + ::std::string m_uid, m_gid, m_groups; + ::std::string m_capInh, m_capPrm, m_capEff; + ino_t m_nsIno[N_FLAG_BITS] = {}; + + // Uid Gid Groups CapInh CapPrm CapEff + static constexpr int N_WANTED_STATUS_LINES = 6; + + static bool fillIfHasId(::std::string &lval, const ::std::string &rval, const char *id) + { + if (rval.rfind(id, 0) != 0) + return false; + if (!lval.empty()) + ThrowMsg(DropError, "ambiguous lines (" << lval << "), (" << rval << ')'); + lval = rval; + return true; } -out: - free(t); - return NULL; -} + bool fillStatusLine(const std::string &line) + { + return fillIfHasId(m_uid, line, "Uid:") + || fillIfHasId(m_gid, line, "Gid:") + || fillIfHasId(m_groups, line, "Groups:") + || fillIfHasId(m_capInh, line, "CapInh:") + || fillIfHasId(m_capPrm, line, "CapPrm:") + || fillIfHasId(m_capEff, line, "CapEff:"); + } -} + void fillStatus(const ::std::string &taskRoot) + { + ::std::string statusPath = taskRoot + "status"; + ::std::ifstream stream(statusPath); + + size_t linesFilled = 0; + for (::std::string line; ::std::getline(stream, line);) + if (fillStatusLine(line)) + linesFilled++; + if (linesFilled != N_WANTED_STATUS_LINES) + ThrowMsg(DropError, '(' << statusPath << ") missing required information"); + } -namespace SecurityManager { + void fillNs(const ::std::string &taskRoot, BitFlags checkProperDropFlags) + { + for (size_t nsNameIdx = 0; nsNameIdx < N_FLAG_BITS; nsNameIdx++) { + if (!(checkProperDropFlags & (1 << nsNameIdx))) + continue; -CheckProperDrop::~CheckProperDrop() -{ - for (const auto &thread : m_threads) - freeproc(thread); - freeproc(m_proc); -} + ::std::string nsPath = taskRoot + "ns/" + NS_NAMES[nsNameIdx]; + struct ::stat st; + if (::stat(nsPath.c_str(), &st) != 0) { + int err = errno; + ThrowMsg(DropError, "stat failed (" << nsPath << ") errno=" << err); + } -bool CheckProperDrop::getThreads() -{ - pid_t pid[2] = {m_pid, 0}; - auto proctabPtr = makeUnique(openproc(PROC_FILLSTATUS | PROC_PID | PROC_FILLNS, pid), closeproc); - if (!proctabPtr) - ThrowMsg(Exception::ProcError, "Unable to open proc interface"); - - m_proc = readproc(proctabPtr.get(), nullptr); - if (!m_proc) - ThrowMsg(Exception::ProcError, - "Unable to read process information for " << m_pid); - - proc_t *thread; - while ((thread = readtask_priv(proctabPtr.get(), m_proc))) { - if (thread->tid != m_pid) - m_threads.push_back(thread); - else - freeproc(thread); + m_nsIno[nsNameIdx] = st.st_ino; + } } - if (errno == EACCES) { - LogError("Thread data read access denied while running prepare_app for pid " << m_pid - << ". Possible race condition: the caller may have created threads during prepare_app execution."); - return false; + + static void checkCaps(const ::std::string &capsLine) + { + assert(!::isspace(capsLine.back())); + auto pos = capsLine.find_last_not_of("0"); + assert(pos != ::std::string::npos); + if (!::isspace(capsLine[pos]) && capsLine[pos] != ':') + ThrowMsg(DropError, "caps not zero (" << capsLine << ')'); } - if (errno) { - static const unsigned ERROR_MSG_LEN = 1024; - char error_msg[ERROR_MSG_LEN]; - const char *e = strerror_r(errno, error_msg, ERROR_MSG_LEN); - ThrowMsg(Exception::ProcError, - "Unable to read process information for " << m_pid << ": " << e); + template + static void checkSame(const T &a, const T &b, const ::std::string &what) + { + if (a != b) + ThrowMsg(DropError, "tasks differ [" << what << "]: (" << a << ") != (" << b << ")"); } - LogDebug("Reading proc data for " << m_threads.size() << " additional threads beside main thread"); - return true; -} +public: + TaskData(const ::std::string &taskId, BitFlags checkProperDropFlags) + : m_label(SmackLabels::getSmackLabelFromPid(::std::stoul(taskId))) + { + ::std::string taskRoot = "/proc/self/task/" + taskId + "/"; + fillStatus(taskRoot); + fillNs(taskRoot, checkProperDropFlags); + + checkCaps(m_capInh); + checkCaps(m_capPrm); + checkCaps(m_capEff); + } -bool CheckProperDrop::checkThreads() + void checkSameAs(const TaskData &other) const { + checkSame(m_label, other.m_label, "label"); + checkSame(m_uid, other.m_uid, "uid"); + checkSame(m_gid, other.m_gid, "gid"); + checkSame(m_groups, other.m_groups, "groups"); + for (size_t nsNameIdx = 0; nsNameIdx < N_FLAG_BITS; nsNameIdx++) + checkSame(m_nsIno[nsNameIdx], other.m_nsIno[nsNameIdx], NS_NAMES[nsNameIdx]); + } +}; + +} // namespace + +void checkThreads(BitFlags checkProperDropFlags) { - std::string smackProc = SmackLabels::getSmackLabelFromPid(m_pid); - - auto capProcPtr = makeUnique(cap_get_pid(m_pid), cap_free); - if (!capProcPtr) - ThrowMsg(Exception::CapError, - "Unable to get capabilities for " << m_pid); - - auto capProcStrPtr = makeUnique(cap_to_text(capProcPtr.get(), nullptr), cap_free); - if (!capProcStrPtr) - ThrowMsg(Exception::CapError, - "Unable to get capabilities for " << m_pid); - - for (const auto &thread : m_threads) { - - #define REPORT_THREAD_ERROR(NAME, VAL1, VAL2) do { \ - LogError("Invalid value of " << (NAME) << " for thread " << thread->tid << ". " \ - << "Process has " << (VAL1) << ", thread has " << (VAL2) << ". " \ - << "Application candidate process not prepared properly for launch. " \ - << (NAME) << " values should be same for all threads."); \ - return false; \ - } while (0) - - auto capThreadPtr = makeUnique(cap_get_pid(thread->tid), cap_free); - if (!capThreadPtr) - ThrowMsg(Exception::CapError, - "Unable to get capabilities for " << thread->tid); - - if (cap_compare(capProcPtr.get(), capThreadPtr.get())) { - auto capStrThreadPtr = makeUnique(cap_to_text(capThreadPtr.get(), nullptr), cap_free); - if (!capStrThreadPtr) - ThrowMsg(Exception::CapError, "Unable to get capabilities for " << thread->tid); - - REPORT_THREAD_ERROR("capabilities", capProcStrPtr.get(), capStrThreadPtr.get()); - } + LogDebug("checkProperDrop flags (" << static_cast(checkProperDropFlags) << ')'); - std::string smackThread = SmackLabels::getSmackLabelFromPid(thread->tid); - if (smackProc != smackThread) - REPORT_THREAD_ERROR("Smack label", smackProc, smackThread); - - if (m_proc->supgid && thread->supgid) { - if (strcmp(m_proc->supgid, thread->supgid)) - REPORT_THREAD_ERROR("Supplementary groups", m_proc->supgid, thread->supgid); - } else { - if (m_proc->supgid != thread->supgid) - REPORT_THREAD_ERROR("Supplementary groups", - m_proc->supgid ? m_proc->supgid : "", - thread->supgid ? thread->supgid : ""); - } + auto taskIds = FS::getSubDirectoriesFromDirectory("/proc/self/task", true); + if (taskIds.empty()) + ThrowMsg(DropError, "no tasks found"); - #define CHECK_THREAD_CRED_FIELD(FIELD) do { \ - const auto pval = m_proc->FIELD, tval = thread->FIELD; \ - if (pval != tval) \ - REPORT_THREAD_ERROR(#FIELD, pval, tval); \ - } while (0) - - CHECK_THREAD_CRED_FIELD(euid); - CHECK_THREAD_CRED_FIELD(egid); - CHECK_THREAD_CRED_FIELD(ruid); - CHECK_THREAD_CRED_FIELD(rgid); - CHECK_THREAD_CRED_FIELD(suid); - CHECK_THREAD_CRED_FIELD(sgid); - CHECK_THREAD_CRED_FIELD(fuid); - CHECK_THREAD_CRED_FIELD(fgid); - CHECK_THREAD_CRED_FIELD(ns[IPCNS]); - CHECK_THREAD_CRED_FIELD(ns[MNTNS]); - CHECK_THREAD_CRED_FIELD(ns[NETNS]); - CHECK_THREAD_CRED_FIELD(ns[PIDNS]); - CHECK_THREAD_CRED_FIELD(ns[USERNS]); - CHECK_THREAD_CRED_FIELD(ns[UTSNS]); - - #undef CHECK_THREAD_CRED_FIELD - #undef REPORT_THREAD_ERROR - } + TaskData lastTaskData(taskIds.back(), checkProperDropFlags); + taskIds.pop_back(); - return true; + for (const auto &taskId : taskIds) + TaskData(taskId, checkProperDropFlags).checkSameAs(lastTaskData); } -} // namespace SecurityManager +} // namespace CheckProperDrop +} // namespace SecurityManager diff --git a/src/client/client-security-manager.cpp b/src/client/client-security-manager.cpp index bdd456d3..aeae65a0 100644 --- a/src/client/client-security-manager.cpp +++ b/src/client/client-security-manager.cpp @@ -489,7 +489,7 @@ static int fetchForbiddenAndAllowedGroups(const std::string &appName, std::vecto } static int prepareAppInitialSetupAndFetch(const std::string &appName, const MountNS::PrivilegePathsMap &privilegePathsMap, std::string &label, - std::string &pkgName, bool &enabledSharedRO, std::vector &forbiddenGroups, std::vector &allowedGroups, + std::string &pkgName, PrepareAppFlags &prepareAppFlags, std::vector &forbiddenGroups, std::vector &allowedGroups, std::vector &privPathsStatusVector) { ClientRequest request(SecurityModuleCall::PREPARE_APP); @@ -498,7 +498,7 @@ static int prepareAppInitialSetupAndFetch(const std::string &appName, const Moun return request.getStatus(); } - request.recv(forbiddenGroups, allowedGroups, privPathsStatusVector, label, pkgName, enabledSharedRO); + request.recv(forbiddenGroups, allowedGroups, privPathsStatusVector, label, pkgName, prepareAppFlags); return SECURITY_MANAGER_SUCCESS; } @@ -949,11 +949,11 @@ int security_manager_prepare_app(const char *app_name) return try_catch([&] { std::string appLabel, pkgName; - bool enabledSharedRO; + PrepareAppFlags prepareAppFlags; std::vector forbiddenGroups, allowedGroups; std::vector privPathsStatusVector; auto privilegePathMap = MountNS::getPrivilegePathMap(getuid()); - int ret = prepareAppInitialSetupAndFetch(app_name, privilegePathMap, appLabel, pkgName, enabledSharedRO, + int ret = prepareAppInitialSetupAndFetch(app_name, privilegePathMap, appLabel, pkgName, prepareAppFlags, forbiddenGroups, allowedGroups, privPathsStatusVector); if (ret != SECURITY_MANAGER_SUCCESS) { LogError("Failed to get app info for appName: " << app_name); @@ -966,7 +966,8 @@ int security_manager_prepare_app(const char *app_name) return ret; } - ret = security_manager_setup_namespace_internal(privilegePathMap, pkgName, enabledSharedRO, privPathsStatusVector, appLabel); + ret = security_manager_setup_namespace_internal(privilegePathMap, pkgName, + prepareAppFlags & PREPARE_APP_SHARED_RO_FLAG, privPathsStatusVector, appLabel); if (ret != SECURITY_MANAGER_SUCCESS) { LogError("Unable to setup namespace for application " << app_name); return ret; @@ -979,18 +980,10 @@ int security_manager_prepare_app(const char *app_name) } try { - CheckProperDrop cpd; - if (!cpd.getThreads()) { - LogError("Privileges might not have been properly dropped for the whole process of application " << app_name); - ret = SECURITY_MANAGER_ERROR_UNKNOWN; - } - if (!cpd.checkThreads()) { - LogError("Privileges haven't been properly dropped for the whole process of application " << app_name); - ret = SECURITY_MANAGER_ERROR_UNKNOWN; - } - } catch (const SecurityManager::Exception &e) { - LogError("Error while checking privileges of the process for application " << app_name << ": " << e.DumpToString()); - ret = SECURITY_MANAGER_ERROR_UNKNOWN; + CheckProperDrop::checkThreads(prepareAppFlags >> PREPARE_APP_CPD_FLAG_SHIFT); + } catch (...) { + LogError("Privileges haven't been properly dropped for the whole process of application " << app_name); + throw; } return ret; diff --git a/src/client/include/check-proper-drop.h b/src/client/include/check-proper-drop.h deleted file mode 100644 index 02b5052a..00000000 --- a/src/client/include/check-proper-drop.h +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright (c) 2015-2020 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 check-proper-drop.h - * @author Rafal Krypa - * @version 1.0 - * @brief Definition of proper privilege dropping check utilities - */ - -#pragma once - -#include - -#include -#include - -#include - -namespace SecurityManager { - -class CheckProperDrop { -public: - class Exception { - public: - DECLARE_EXCEPTION_TYPE(SecurityManager::Exception, Base) - DECLARE_EXCEPTION_TYPE(Base, ProcError) - DECLARE_EXCEPTION_TYPE(Base, CapError) - }; - - ~CheckProperDrop(); - CheckProperDrop(pid_t pid = getpid()) : m_pid(pid) {}; - - /** - * Fetch credentials of the process and all its threads. - * Must be called before checkThreads(). - */ - bool getThreads(); - - /** - * Check whether all threads of the process has properly aligned - * credentials: - * - uids - * - gids - * - capabilities - * - Smack labels - * - Namespaces - */ - bool checkThreads(); - -private: - pid_t m_pid; - proc_t *m_proc = nullptr; - std::vector m_threads; -}; - -} // namespace SecurityManager diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index e978a4f6..bff849db 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -3,6 +3,7 @@ SET(COMMON_VERSION ${COMMON_VERSION_MAJOR}.0.2) PKG_CHECK_MODULES(COMMON_DEP REQUIRED + libcap libsystemd libsmack sqlite3 @@ -49,6 +50,7 @@ SET(COMMON_SOURCES ${DPL_PATH}/db/src/naive_synchronization_object.cpp ${DPL_PATH}/db/src/sql_connection.cpp ${COMMON_PATH}/channel.cpp + ${COMMON_PATH}/check-proper-drop.cpp ${COMMON_PATH}/config-file.cpp ${COMMON_PATH}/connection.cpp ${COMMON_PATH}/credentials.cpp diff --git a/src/common/check-proper-drop.cpp b/src/common/check-proper-drop.cpp new file mode 100644 index 00000000..52e495a6 --- /dev/null +++ b/src/common/check-proper-drop.cpp @@ -0,0 +1,119 @@ +/* + * Copyright (c) 2020 Samsung Electronics Co., Ltd. All rights reserved + * + * Contact: Tomasz Swierczek + * + * 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 + */ +/* + * @brief Implementation of proper privilege dropping check utilities, service side + */ + +#include +#include +#include +#include + +#include +#include + +namespace SecurityManager { +namespace CheckProperDrop { + +namespace { + +constexpr char MANDATORY_NS[] = "mnt"; +static_assert(!::strcmp(MANDATORY_NS, NS_NAMES[0]), + "MANDATORY_NS must be the first element in NS_NAMES - " + "it is guaranteed to exist, EACCES on it implies EACCES on others"); + +bool dropCaps() +{ + cap_t cap = ::cap_init(); + if (!cap) { + LogError("Unable to allocate capability object"); + return false; + } + int res = ::cap_set_proc(cap); + if (res != 0) + LogError("Can't drop thread capabilities"); + ::cap_free(cap); + return res == 0; +} + +} // namespace + +// Inspecting other threads' ns files requires CAP_SYS_PTRACE for kernel < 3.12. +// Since checkProperDrop() runs client-side with all caps dropped, all attempts +// to read those files would end in EACCES. Hence, namespace verification is +// skipped altogether by returning a zeroed bitmask. +// +// Namespace availability is detected by dropping caps in a child thread, then +// attempting to access main thread's ns files. The caps drop is confined to +// the child thread, thus the current thread is kept pristine. +// +// Mount namespaces are used by prepare_app via unshare(CLONE_NEWNS) prior to +// checkProperDrop(). If unshare fails, prepare_app terminates with an error +// and checkProperDrop() is never called. It is thus assumed that Kconfig var +// CONFIG_NAMESPACE is turned on. Under that assumption MANDATORY_NS is +// guaranteed to exist. EACCES on it means kernel < 3.12. +// +// NS_NAMES other than MANDATORY_NS are checked for ENOENT. A missing file +// means the respective ns support is turned off in the kernel. Said support +// is governed by the following Kconfig vars: +// "ipc": CONFIG_IPC_NS +// "net": CONFIG_NET_NS +// "uts": CONFIG_UTS_NS +int8_t computeFlags() +{ + int8_t flagsFromThread = -1; + + ::std::thread([&] { + if (!dropCaps()) + return; + + auto mandatoryNsPath = "/proc/self/ns/" + ::std::string(MANDATORY_NS); + if (::access(mandatoryNsPath.c_str(), R_OK) != 0) { + int err = errno; + if (err == EACCES) // kernel < 3.12, CAP_SYS_PTRACE required + flagsFromThread = 0; // refrain from checking namespaces at all + else + LogError("access(" << mandatoryNsPath << ") errno (" << err << ')'); + return; + } + + int8_t outFlags = 1 << 0; + + for (size_t nsNameIdx = 1; nsNameIdx < N_FLAG_BITS; nsNameIdx++) { + auto nsPath = "/proc/self/ns/" + ::std::string(NS_NAMES[nsNameIdx]); + + if (::access(nsPath.c_str(), R_OK) == 0) { + outFlags |= 1 << nsNameIdx; + continue; + } + + int err = errno; + if (err != ENOENT) { + LogError("access(" << nsPath << ") errno (" << err << ')'); + return; + } + } + + flagsFromThread = outFlags; + }).join(); + + return flagsFromThread; +} + +} // namespace CheckProperDrop +} // namespace SecurityManager diff --git a/src/common/include/check-proper-drop.h b/src/common/include/check-proper-drop.h new file mode 100644 index 00000000..ec46638b --- /dev/null +++ b/src/common/include/check-proper-drop.h @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2015-2020 Samsung Electronics Co., Ltd. All rights reserved + * + * Contact: Tomasz Swierczek + * + * 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 + */ +/* + * @brief Definition of proper privilege dropping check utilities + */ + +#pragma once + +#include + +namespace SecurityManager { +namespace CheckProperDrop { + +// SERVICE & CLIENT + +// "pid" and "user" imply consistency among threads so are not checked, +// see man unshare(2): CLONE_NEW{IPC,USER} implies CLONE_THREAD +constexpr inline const char *NS_NAMES[] = {"mnt", "ipc", "net", "uts"}; + +constexpr inline auto N_FLAG_BITS = sizeof NS_NAMES / sizeof NS_NAMES[0]; +static_assert(N_FLAG_BITS <= 7, "flags too large for int8_t"); + +using BitFlags = uint8_t; + + +// SERVICE ONLY +// Negative on unrecoverable error. +// Otherwise bit i set iff NS_NAMES[i] checked. +int8_t computeFlags(); + + +// CLIENT ONLY +// checkProperDropFlags must come from computeFlags() +void checkThreads(BitFlags flags); + +} // namespace CheckProperDrop +} // namespace SecurityManager diff --git a/src/common/include/protocols.h b/src/common/include/protocols.h index 67aa4ead..12aa3229 100644 --- a/src/common/include/protocols.h +++ b/src/common/include/protocols.h @@ -134,6 +134,12 @@ enum class SecurityModuleCall NOOP = 0x90, }; +// The least significant bit on iff shared RO is enabled. +// Subsequent bits == bitmask of namespaces to be checked in checkProperDrop(). +typedef uint8_t PrepareAppFlags; +constexpr inline PrepareAppFlags PREPARE_APP_SHARED_RO_FLAG = 1; +constexpr inline uint8_t PREPARE_APP_CPD_FLAG_SHIFT = 1; + // returns stringified name of return call type const char * SecurityModuleCallToString(SecurityModuleCall call_num); diff --git a/src/common/include/service_impl.h b/src/common/include/service_impl.h index 761e239d..c69cd16e 100644 --- a/src/common/include/service_impl.h +++ b/src/common/include/service_impl.h @@ -376,7 +376,8 @@ public: * @param[in] pathPrivVector paths related privileges to query * @param[out] label generated label * @param[out] pkgName application package name - * @param[out] enabledSharedRO placeholder for check shared_ro result + * @param[out] prepareAppFlags placeholder for check shared_ro result + * and checkProperDrop flags * @param[out] forbiddenGroups sorted vector of forbidden groups * @param[out] allowedGroups sorted vector of allowed groups * @param[out] pathPrivStatusVector results of respective paths related privilege queries @@ -384,7 +385,7 @@ public: * @return API return code, as defined in protocols.h */ int prepareApp(const Credentials &creds, const std::string &appName, const std::vector &privPathsVector, - Smack::Label &label, std::string &pkgName, bool &enabledSharedRO, + Smack::Label &label, std::string &pkgName, PrepareAppFlags &prepareAppFlags, std::vector &forbiddenGroups, std::vector &allowedGroups, std::vector &privPathsStatusVector); private: @@ -465,6 +466,7 @@ private: CynaraAdmin m_cynaraAdmin; PrivilegeGids m_privilegeGids; NSMountLogic m_NSMountLogic; + PrepareAppFlags m_prepareAppFlags; }; } /* namespace SecurityManager */ diff --git a/src/common/service_impl.cpp b/src/common/service_impl.cpp index e6319a69..94981ad4 100644 --- a/src/common/service_impl.cpp +++ b/src/common/service_impl.cpp @@ -42,6 +42,7 @@ #include #include +#include "check-proper-drop.h" #include "protocols.h" #include "privilege_db.h" #include "cynara.h" @@ -159,6 +160,18 @@ ServiceImpl::ServiceImpl(Offline offline) : PrivilegeGids::GroupPrivileges group_privileges; m_privilegeDb.GetGroupsRelatedPrivileges(group_privileges); m_privilegeGids.init(group_privileges); + + if (Offline::no == offline) { + const auto checkProperDropFlags = CheckProperDrop::computeFlags(); + if (checkProperDropFlags < 0) + ThrowMsg(FS::Exception::FileError, "Error computing CheckProperDrop flags." + " Unable to set up and verify application sandboxes." + " Mandatory functionality malfunctioning." + " The security-manager service will not run because of this."); + m_prepareAppFlags = PrepareAppFlags(checkProperDropFlags) << PREPARE_APP_CPD_FLAG_SHIFT; + static_assert(CheckProperDrop::N_FLAG_BITS + PREPARE_APP_CPD_FLAG_SHIFT <= 8 * sizeof m_prepareAppFlags, + "CheckProperDrop flags too large for prepareAppFlags"); + } } ServiceImpl::~ServiceImpl() @@ -2234,14 +2247,15 @@ Smack::Label ServiceImpl::getProcessLabel(const std::string &appName) } int ServiceImpl::prepareApp(const Credentials &creds, const std::string &appName, const std::vector &privPathsVector, - Smack::Label &label, std::string &pkgName, bool &enabledSharedRO, + Smack::Label &label, std::string &pkgName, PrepareAppFlags &prepareAppFlags, std::vector &forbiddenGroups, std::vector &allowedGroups, std::vector &privPathsStatusVector) { LogDebug("Requested prepareApp for application " << appName); - bool isHybrid; + bool isHybrid, enabledSharedRO; if (!m_privilegeDb.GetAppPkgInfo(appName, pkgName, isHybrid, enabledSharedRO)) return SECURITY_MANAGER_ERROR_UNKNOWN; + prepareAppFlags = m_prepareAppFlags | (enabledSharedRO ? PREPARE_APP_SHARED_RO_FLAG : 0); label = SmackLabels::generateProcessLabel(appName, pkgName, isHybrid); std::vector allowedPrivileges; diff --git a/src/server/service/service.cpp b/src/server/service/service.cpp index 478c2e34..cee6c0b1 100644 --- a/src/server/service/service.cpp +++ b/src/server/service/service.cpp @@ -502,16 +502,16 @@ void Service::processGetProcessLabel(MessageBuffer &buffer, MessageBuffer &send) void Service::prepareApp(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds) { std::string appName, pkgName, label; - bool enabledSharedRO; + PrepareAppFlags prepareAppFlags; std::vector privPathsVector; std::vector forbiddenGroups, allowedGroups; std::vector privPathsStatusVector; Deserialization::Deserialize(buffer, appName, privPathsVector); int ret = serviceImpl.prepareApp(creds, appName, privPathsVector, - label, pkgName, enabledSharedRO, forbiddenGroups, allowedGroups, privPathsStatusVector); + label, pkgName, prepareAppFlags, forbiddenGroups, allowedGroups, privPathsStatusVector); Serialization::Serialize(send, ret); if (ret == SECURITY_MANAGER_SUCCESS) - Serialization::Serialize(send, forbiddenGroups, allowedGroups, privPathsStatusVector, label, pkgName, enabledSharedRO); + Serialization::Serialize(send, forbiddenGroups, allowedGroups, privPathsStatusVector, label, pkgName, prepareAppFlags); } } // namespace SecurityManager diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index d1c645f7..6dd3e407 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -28,7 +28,6 @@ PKG_CHECK_MODULES(COMMON_DEP REQUIRED security-privilege-manager mount libcap - libprocps ) FIND_PACKAGE(Threads REQUIRED) @@ -67,7 +66,7 @@ SET(SM_TESTS_SOURCES ${SM_TEST_SRC}/test_service_impl_utils.cpp ${SM_TEST_SRC}/test_smack-labels.cpp ${SM_TEST_SRC}/test_smack-rules.cpp - ${SM_TEST_SRC}/test_check_proper_drop.cpp + #${SM_TEST_SRC}/test_check_proper_drop.cpp ${SM_TEST_SRC}/test_misc.cpp ${SM_TEST_SRC}/test_template-manager.cpp ${DPL_PATH}/core/src/assert.cpp