Reimplement prepare_app proper drop checking 92/231392/57
authorKonrad Lipinski <k.lipinski2@samsung.com>
Tue, 21 Apr 2020 17:32:17 +0000 (19:32 +0200)
committerKonrad Lipinski <k.lipinski2@samsung.com>
Wed, 1 Jul 2020 10:13:20 +0000 (12:13 +0200)
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

13 files changed:
packaging/security-manager.spec
src/client/CMakeLists.txt
src/client/check-proper-drop.cpp
src/client/client-security-manager.cpp
src/client/include/check-proper-drop.h [deleted file]
src/common/CMakeLists.txt
src/common/check-proper-drop.cpp [new file with mode: 0644]
src/common/include/check-proper-drop.h [new file with mode: 0644]
src/common/include/protocols.h
src/common/include/service_impl.h
src/common/service_impl.cpp
src/server/service/service.cpp
test/CMakeLists.txt

index 72886b6..69aec95 100644 (file)
@@ -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)
index 8862ca0..4d54fa7 100644 (file)
@@ -3,7 +3,6 @@ PKG_CHECK_MODULES(CLIENT_DEP
     cynara-client-async
     libsmack
     libcap
-    libprocps
     mount
     )
 
index 6566f3d..753aa64 100644 (file)
@@ -1,7 +1,7 @@
 /*
  *  Copyright (c) 2015-2020 Samsung Electronics Co., Ltd. All rights reserved
  *
- *  Contact: Rafal Krypa <r.krypa@samsung.com>
+ *  Contact: Tomasz Swierczek <t.swierczek@samsung.com>
  *
  *  Licensed under the Apache License, Version 2.0 (the "License");
  *  you may not use this file except in compliance with the License.
  *  limitations under the License
  */
 /*
- * @file        check-proper-drop.cpp
- * @author      Rafal Krypa <r.krypa@samsung.com>
- * @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 <cassert>
+#include <cctype>
+#include <fstream>
+#include <string>
+#include <sys/stat.h>
 
-#include <sys/capability.h>
-#include <errno.h>
+#include <check-proper-drop.h>
+#include <dpl/exception.h>
+#include <dpl/log/log.h>
+#include <filesystem.h>
+#include <smack-labels.h>
 
-#include <memory>
-#include <string>
-#include <cstring>
+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 <class T>
+    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<unsigned>(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 : "<NULL>",
-                                    thread->supgid ? thread->supgid : "<NULL>");
-        }
+    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
index bdd456d..aeae65a 100644 (file)
@@ -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<gid_t> &forbiddenGroups, std::vector<gid_t> &allowedGroups,
+        std::string &pkgName, PrepareAppFlags &prepareAppFlags, std::vector<gid_t> &forbiddenGroups, std::vector<gid_t> &allowedGroups,
         std::vector<bool> &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<gid_t> forbiddenGroups, allowedGroups;
         std::vector<bool> 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 (file)
index 02b5052..0000000
+++ /dev/null
@@ -1,71 +0,0 @@
-/*
- *  Copyright (c) 2015-2020 Samsung Electronics Co., Ltd All Rights Reserved
- *
- *  Contact: Rafal Krypa <r.krypa@samsung.com>
- *
- *  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 <r.krypa@samsung.com>
- * @version     1.0
- * @brief       Definition of proper privilege dropping check utilities
- */
-
-#pragma once
-
-#include <dpl/exception.h>
-
-#include <unistd.h>
-#include <proc/readproc.h>
-
-#include <vector>
-
-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<proc_t*> m_threads;
-};
-
-} // namespace SecurityManager
index e978a4f..bff849d 100644 (file)
@@ -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 (file)
index 0000000..52e495a
--- /dev/null
@@ -0,0 +1,119 @@
+/*
+ *  Copyright (c) 2020 Samsung Electronics Co., Ltd. All rights reserved
+ *
+ *  Contact: Tomasz Swierczek <t.swierczek@samsung.com>
+ *
+ *  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 <cassert>
+#include <cstring>
+#include <sys/capability.h>
+#include <thread>
+
+#include <check-proper-drop.h>
+#include <dpl/log/log.h>
+
+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 (file)
index 0000000..ec46638
--- /dev/null
@@ -0,0 +1,52 @@
+/*
+ *  Copyright (c) 2015-2020 Samsung Electronics Co., Ltd. All rights reserved
+ *
+ *  Contact: Tomasz Swierczek <t.swierczek@samsung.com>
+ *
+ *  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 <utils.h>
+
+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
index 67aa4ea..12aa322 100644 (file)
@@ -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);
 
index 761e239..c69cd16 100644 (file)
@@ -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<std::string> &privPathsVector,
-            Smack::Label &label, std::string &pkgName, bool &enabledSharedRO,
+            Smack::Label &label, std::string &pkgName, PrepareAppFlags &prepareAppFlags,
             std::vector<gid_t> &forbiddenGroups, std::vector<gid_t> &allowedGroups, std::vector<bool> &privPathsStatusVector);
 
 private:
@@ -465,6 +466,7 @@ private:
     CynaraAdmin m_cynaraAdmin;
     PrivilegeGids m_privilegeGids;
     NSMountLogic m_NSMountLogic;
+    PrepareAppFlags m_prepareAppFlags;
 };
 
 } /* namespace SecurityManager */
index e6319a6..94981ad 100644 (file)
@@ -42,6 +42,7 @@
 #include <sys/smack.h>
 
 #include <config.h>
+#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<std::string> &privPathsVector,
-        Smack::Label &label, std::string &pkgName, bool &enabledSharedRO,
+        Smack::Label &label, std::string &pkgName, PrepareAppFlags &prepareAppFlags,
         std::vector<gid_t> &forbiddenGroups, std::vector<gid_t> &allowedGroups, std::vector<bool> &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<std::string> allowedPrivileges;
index 478c2e3..cee6c0b 100644 (file)
@@ -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<std::string> privPathsVector;
     std::vector<gid_t> forbiddenGroups, allowedGroups;
     std::vector<bool> 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
index d1c645f..6dd3e40 100644 (file)
@@ -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