summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRafal Krypa <r.krypa@samsung.com>2016-01-14 16:48:19 +0100
committerTomasz Swierczek <t.swierczek@samsung.com>2016-06-16 05:16:37 -0700
commitb1e0fb389d0a8c2230985b3ea7aab7d363a7e403 (patch)
tree1688407a4a836ee0bf16fab93b38b72c6f415109
parent6eef187ab9b81e83744e96bedc9c667598e588f6 (diff)
downloadsecurity-manager-b1e0fb389d0a8c2230985b3ea7aab7d363a7e403.tar.gz
security-manager-b1e0fb389d0a8c2230985b3ea7aab7d363a7e403.tar.bz2
security-manager-b1e0fb389d0a8c2230985b3ea7aab7d363a7e403.zip
Add check if privileges were properly dropped
Check if every thread in process has same stats as thread calling security_manager_prepare_app() and exit from process if they do not. Change-Id: I008c2b8e442edb6a5f9f1d74bf13f95465b6bdca Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
-rw-r--r--packaging/security-manager.spec1
-rw-r--r--src/client/CMakeLists.txt2
-rw-r--r--src/client/check-proper-drop.cpp129
-rw-r--r--src/client/client-security-manager.cpp41
-rw-r--r--src/client/include/check-proper-drop.h76
5 files changed, 236 insertions, 13 deletions
diff --git a/packaging/security-manager.spec b/packaging/security-manager.spec
index 20a84bda..82c27f24 100644
--- a/packaging/security-manager.spec
+++ b/packaging/security-manager.spec
@@ -15,6 +15,7 @@ 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 f934617d..44d898f3 100644
--- a/src/client/CMakeLists.txt
+++ b/src/client/CMakeLists.txt
@@ -2,6 +2,7 @@ PKG_CHECK_MODULES(CLIENT_DEP
REQUIRED
libsmack
libcap
+ libprocps
)
SET(CLIENT_VERSION_MAJOR 1)
@@ -24,6 +25,7 @@ SET(CLIENT_SOURCES
${CLIENT_PATH}/client-common.cpp
${CLIENT_PATH}/client-offline.cpp
${CLIENT_PATH}/client-label-monitor.cpp
+ ${CLIENT_PATH}/check-proper-drop.cpp
)
ADD_LIBRARY(${TARGET_CLIENT} SHARED ${CLIENT_SOURCES})
diff --git a/src/client/check-proper-drop.cpp b/src/client/check-proper-drop.cpp
new file mode 100644
index 00000000..4d136c5e
--- /dev/null
+++ b/src/client/check-proper-drop.cpp
@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) 2015 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.cpp
+ * @author Rafal Krypa <r.krypa@samsung.com>
+ * @version 1.0
+ * @brief Implementation of proper privilege dropping check utilities
+ */
+
+#include "check-proper-drop.h"
+#include "smack-labels.h"
+#include <dpl/log/log.h>
+
+#include <sys/capability.h>
+
+#include <memory>
+#include <string>
+
+namespace SecurityManager {
+
+using proctabPtr = std::unique_ptr<PROCTAB, void (*)(PROCTAB*)>;
+using capPtr = std::unique_ptr<_cap_struct, int (*)(void*)>;
+using capStrPtr = std::unique_ptr<char, int(*)(void*)>;
+
+CheckProperDrop::~CheckProperDrop()
+{
+ for (const auto &thread : m_threads)
+ freeproc(thread);
+ freeproc(m_proc);
+}
+
+void CheckProperDrop::getThreads()
+{
+ pid_t pid[2] = {m_pid, 0};
+ proctabPtr PT(openproc(PROC_FILLSTATUS | PROC_PID, pid), closeproc);
+ if (!PT)
+ ThrowMsg(Exception::ProcError,
+ "Unable to open proc interface");
+
+ m_proc = readproc(PT.get(), nullptr);
+ if (!m_proc)
+ ThrowMsg(Exception::ProcError,
+ "Unable read process information for " << pid);
+
+ proc_t *thread;
+ while ((thread = readtask(PT.get(), m_proc, nullptr)))
+ if (thread->tid != m_pid)
+ m_threads.push_back(thread);
+}
+
+bool CheckProperDrop::checkThreads()
+{
+#define REPORT_THREAD_ERROR(TID, NAME, VAL1, VAL2) { \
+ LogError("Invalid value of " << (NAME) << " for thread " << (TID) << "." \
+ << ". Process has " << (VAL1) << ", thread has " << (VAL2) << "."); \
+ return false; \
+}
+
+#define CHECK_THREAD_CRED_FIELD(P, T, FIELD) { \
+ int pval = (P)->FIELD, tval = (T)->FIELD; \
+ if (pval != tval) \
+ REPORT_THREAD_ERROR((T)->tid, #FIELD, pval, tval); \
+}
+
+ std::string smackProc = SmackLabels::getSmackLabelFromPid(m_pid);
+
+ capPtr capProc(cap_get_pid(m_pid), cap_free);
+ if (!capProc)
+ ThrowMsg(Exception::CapError,
+ "Unable to get capabilities for " << m_pid);
+
+ capStrPtr capStrProc(cap_to_text(capProc.get(), nullptr), cap_free);
+ if (!capStrProc)
+ ThrowMsg(Exception::CapError,
+ "Unable to get capabilities for " << m_pid);
+
+ for (const auto &thread : m_threads) {
+ capPtr capThread(cap_get_pid(thread->tid), cap_free);
+ if (!capThread)
+ ThrowMsg(Exception::CapError,
+ "Unable to get capabilities for " << thread->tid);
+
+ if (cap_compare(capProc.get(), capThread.get())) {
+ capStrPtr capStrThread(cap_to_text(capThread.get(), nullptr), cap_free);
+ if (!capStrThread)
+ ThrowMsg(Exception::CapError, "Unable to get capabilities for " << thread->tid);
+
+ REPORT_THREAD_ERROR(thread->tid, "capabilities",
+ capStrProc.get(), capStrThread.get());
+ }
+
+ std::string smackThread = SmackLabels::getSmackLabelFromPid(thread->tid);
+ if (smackProc != smackThread)
+ REPORT_THREAD_ERROR(thread->tid, "Smack label",
+ smackProc, smackThread);
+
+ if (strcmp(m_proc->supgid, thread->supgid))
+ REPORT_THREAD_ERROR(thread->tid, "Supplementary groups",
+ m_proc->supgid, thread->supgid);
+
+ CHECK_THREAD_CRED_FIELD(m_proc, thread, euid);
+ CHECK_THREAD_CRED_FIELD(m_proc, thread, egid);
+ CHECK_THREAD_CRED_FIELD(m_proc, thread, ruid);
+ CHECK_THREAD_CRED_FIELD(m_proc, thread, rgid);
+ CHECK_THREAD_CRED_FIELD(m_proc, thread, suid);
+ CHECK_THREAD_CRED_FIELD(m_proc, thread, sgid);
+ CHECK_THREAD_CRED_FIELD(m_proc, thread, fuid);
+ CHECK_THREAD_CRED_FIELD(m_proc, thread, fgid);
+ }
+
+ return true;
+}
+
+} // namespace SecurityManager
diff --git a/src/client/client-security-manager.cpp b/src/client/client-security-manager.cpp
index 40b037f2..55109fc0 100644
--- a/src/client/client-security-manager.cpp
+++ b/src/client/client-security-manager.cpp
@@ -26,6 +26,7 @@
*/
#include <cstdio>
+#include <cstdlib>
#include <functional>
#include <memory>
#include <unordered_set>
@@ -52,6 +53,7 @@
#include <protocols.h>
#include <service_impl.h>
#include <connection.h>
+#include <check-proper-drop.h>
#include <security-manager.h>
#include <client-offline.h>
@@ -694,22 +696,35 @@ int security_manager_drop_process_privileges(void)
SECURITY_MANAGER_API
int security_manager_prepare_app(const char *app_name)
{
- LogDebug("security_manager_prepare_app() called");
- int ret;
+ return try_catch([&] {
+ LogDebug("security_manager_prepare_app() called");
+ int ret;
- ret = security_manager_set_process_groups_from_appid(app_name);
- if (ret != SECURITY_MANAGER_SUCCESS) {
- LogWarning("Unable to setup process groups for application. Privileges with direct access to resources will not work.");
- ret = SECURITY_MANAGER_SUCCESS;
- }
+ ret = security_manager_set_process_groups_from_appid(app_name);
+ if (ret != SECURITY_MANAGER_SUCCESS) {
+ LogWarning("Unable to setup process groups for application. Privileges with direct access to resources will not work.");
+ }
- ret = security_manager_sync_threads_internal(app_name);
- if (ret != SECURITY_MANAGER_SUCCESS) {
- LogError("Can't properly setup application threads (Smack label & capabilities)");
- return ret;
- }
+ ret = security_manager_sync_threads_internal(app_name);
+ if (ret != SECURITY_MANAGER_SUCCESS) {
+ LogError("Can't properly setup application threads (Smack label & capabilities)");
+ exit(EXIT_FAILURE);
+ }
- return ret;
+ try {
+ CheckProperDrop cpd;
+ cpd.getThreads();
+ if (!cpd.checkThreads()) {
+ LogError("Privileges haven't been properly dropped for the whole process");
+ exit(EXIT_FAILURE);
+ }
+ } catch (const SecurityManager::Exception &e) {
+ LogError("Error while checking privileges of the process: " << e.DumpToString());
+ exit(EXIT_FAILURE);
+ }
+
+ return ret;
+ });
}
SECURITY_MANAGER_API
diff --git a/src/client/include/check-proper-drop.h b/src/client/include/check-proper-drop.h
new file mode 100644
index 00000000..ad1df5b5
--- /dev/null
+++ b/src/client/include/check-proper-drop.h
@@ -0,0 +1,76 @@
+/*
+ * Copyright (c) 2015 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
+ */
+
+#ifndef SECURITY_MANAGER_CHECK_PROPER_DROP_
+#define SECURITY_MANAGER_CHECK_PROPER_DROP_
+
+#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().
+ */
+ void getThreads();
+
+ /**
+ * Check whether all threads of the process has properly aligned
+ * credentials:
+ * - uids
+ * - gids
+ * - capabilities
+ * - Smack labels
+ *
+ * It will terminate the calling process if any thread has different
+ * value than the other threads. This prevents security risks associated
+ * with improperly dropped privileges during application launch.
+ */
+ bool checkThreads();
+
+private:
+ pid_t m_pid;
+ proc_t *m_proc = nullptr;
+ std::vector<proc_t*> m_threads;
+};
+
+} // namespace SecurityManager
+#endif /* SECURITY_MANAGER_CHECK_PROPER_DROP_H_ */