summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTomasz Swierczek <t.swierczek@samsung.com>2023-05-30 13:46:12 +0200
committerTomasz Swierczek <t.swierczek@samsung.com>2023-05-30 14:32:13 +0200
commit627b6768bcdf953bde0c06375dbae314af402720 (patch)
treea5a90a67526c52d5810f728f8aa56fc7eba273c3
parent6837e99fe1ce135ced702e19713066677356be99 (diff)
downloadsecurity-manager-627b6768bcdf953bde0c06375dbae314af402720.tar.gz
security-manager-627b6768bcdf953bde0c06375dbae314af402720.tar.bz2
security-manager-627b6768bcdf953bde0c06375dbae314af402720.zip
Abort app candidate process in case of wrong setup
When offending thread with higher privileges is detected, new error log is added and security-manager-client library forces entire app candidate process to abort. This will effectively block possibility of privilege escalation if a new thread was spawned ie. by Chromium during prepare_app call. Abort will also generate coredump, making it easier to debug the source of offending thread. Change-Id: I16772d0e51aa112548acb64f7b82ccf87948ded9
-rw-r--r--src/client/check-proper-drop.cpp32
-rw-r--r--src/client/client-security-manager.cpp12
2 files changed, 37 insertions, 7 deletions
diff --git a/src/client/check-proper-drop.cpp b/src/client/check-proper-drop.cpp
index 12529ef0..2d7343f7 100644
--- a/src/client/check-proper-drop.cpp
+++ b/src/client/check-proper-drop.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016-2020 Samsung Electronics Co., Ltd. All rights reserved.
+ * Copyright (c) 2016-2023 Samsung Electronics Co., Ltd. All rights reserved.
*
* This file is licensed under the terms of MIT License or the Apache License
* Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details.
@@ -118,9 +118,10 @@ class TaskData final {
}
public:
- TaskData(const ::std::string &taskId, BitFlags checkProperDropFlags)
- : m_label(SmackLabels::getSmackLabelFromPid(::std::stoul(taskId)))
- {
+ TaskData() {}
+
+ void initialize(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);
@@ -130,6 +131,13 @@ public:
checkCaps(m_capEff);
}
+
+ TaskData(const ::std::string &taskId, BitFlags checkProperDropFlags)
+ {
+ initialize(taskId, checkProperDropFlags);
+ }
+
+
void checkSameAs(const TaskData &other) const {
checkSame(m_label, other.m_label, "label");
checkSame(m_uid, other.m_uid, "uid");
@@ -150,11 +158,23 @@ void checkThreads(BitFlags checkProperDropFlags)
if (taskIds.empty())
ThrowMsg(DropError, "no tasks found");
- TaskData lastTaskData(taskIds.back(), checkProperDropFlags);
+ TaskData lastTaskData;
+ try {
+ lastTaskData.initialize(taskIds.back(), checkProperDropFlags);
+ } catch (...) {
+ LogError("Offending taskId is: " << taskIds.back());
+ throw;
+ }
+
taskIds.pop_back();
for (const auto &taskId : taskIds)
- TaskData(taskId, checkProperDropFlags).checkSameAs(lastTaskData);
+ try {
+ TaskData(taskId, checkProperDropFlags).checkSameAs(lastTaskData);
+ } catch (...) {
+ LogError("Offending taskId is: " << taskId);
+ throw;
+ }
}
} // namespace CheckProperDrop
diff --git a/src/client/client-security-manager.cpp b/src/client/client-security-manager.cpp
index c622a123..697c1e91 100644
--- a/src/client/client-security-manager.cpp
+++ b/src/client/client-security-manager.cpp
@@ -1021,7 +1021,17 @@ int security_manager_prepare_app2(const char *app_name, const char *subsession_i
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;
+ /*
+ * Not all app candidate processes behave properly and some may want to spawn
+ * new threads during this API call. This abort() below makes sure the process will:
+ *
+ * 1) not allow actual applicaton run in environment where there's a thread that still has high privileges
+ * 2) make sure a coredump will be created so that it can be analyzed later which thread caused troubles
+ *
+ * This will effectively block possible privilege escalation in application due to improper setup of
+ * the candidate process.
+ */
+ abort();
}
LogWarning("security_manager_prepare_app2() finished with return code " << ret);