diff options
author | Tomasz Swierczek <t.swierczek@samsung.com> | 2023-05-30 13:46:12 +0200 |
---|---|---|
committer | Tomasz Swierczek <t.swierczek@samsung.com> | 2023-05-30 14:32:13 +0200 |
commit | 627b6768bcdf953bde0c06375dbae314af402720 (patch) | |
tree | a5a90a67526c52d5810f728f8aa56fc7eba273c3 | |
parent | 6837e99fe1ce135ced702e19713066677356be99 (diff) | |
download | security-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.cpp | 32 | ||||
-rw-r--r-- | src/client/client-security-manager.cpp | 12 |
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); |