From 9e20fea73b54e9a0f935b76beeac9f7530d4d853 Mon Sep 17 00:00:00 2001 From: Tomasz Swierczek Date: Wed, 14 Nov 2018 06:58:28 +0100 Subject: [PATCH] Change config.cpp variables to #define security-manager may be used in processes with many threads. Destruction of global variables may be in race condition with child thread's operation & usage of these variables. While such problem should be fixed in proper threads management, there may be problems with open-source components that we may not easily modify (and security-manager provides nss plugin that may be used in unexpected places). Change-Id: I057abc0bd2ed8a82d74f3777f6b95d386bc9b9f4 --- src/client/client-label-monitor.cpp | 2 +- src/client/client-security-manager.cpp | 2 +- src/common/config.cpp | 25 -------------- src/common/cynara.cpp | 12 +++---- src/common/include/config.h | 63 ++++++++++++++++++---------------- src/common/mount-namespace.cpp | 4 +-- src/common/permissible-set.cpp | 8 ++--- src/common/service_impl.cpp | 46 ++++++++++++------------- 8 files changed, 71 insertions(+), 91 deletions(-) diff --git a/src/client/client-label-monitor.cpp b/src/client/client-label-monitor.cpp index 9fdd11b..c5f56ac 100644 --- a/src/client/client-label-monitor.cpp +++ b/src/client/client-label-monitor.cpp @@ -164,7 +164,7 @@ void security_manager_app_labels_monitor_finish(app_labels_monitor *monitor) int ret = inotify_rm_watch(monitorPtr->inotify, monitorPtr->global_labels_file_watch); if (ret == -1) { LogError("Inotify watch removal failed on file " << - Config::APPS_LABELS_FILE << ": " << GetErrnoString(errno)); + APPS_LABELS_FILE << ": " << GetErrnoString(errno)); } } if (monitorPtr->user_labels_file_watch != -1) { diff --git a/src/client/client-security-manager.cpp b/src/client/client-security-manager.cpp index 876ea84..95c41dc 100644 --- a/src/client/client-security-manager.cpp +++ b/src/client/client-security-manager.cpp @@ -1325,7 +1325,7 @@ void security_manager_policy_levels_free(char **levels, size_t levels_count) static void loadGroups(std::vector &vgroups) { - auto groupsMapData = ConfigFile(Config::PRIVILEGE_GROUP_LIST_FILE).read(); + auto groupsMapData = ConfigFile(PRIVILEGE_GROUP_LIST_FILE).read(); for (const auto &groupsMapEntry : groupsMapData) { if (groupsMapEntry.size() != 2) continue; diff --git a/src/common/config.cpp b/src/common/config.cpp index 2189f56..17d3641 100644 --- a/src/common/config.cpp +++ b/src/common/config.cpp @@ -28,31 +28,6 @@ namespace SecurityManager { namespace Config { -const std::string SERVICE_NAME = "security-manager"; - -const std::string PRIVILEGE_APPINST_USER = "http://tizen.org/privilege/notexist"; -const std::string PRIVILEGE_APPINST_ADMIN = "http://tizen.org/privilege/notexist"; -const std::string PRIVILEGE_USER_ADMIN = "http://tizen.org/privilege/internal/usermanagement"; -const std::string PRIVILEGE_POLICY_USER = "http://tizen.org/privilege/notexist"; -const std::string PRIVILEGE_POLICY_ADMIN = "http://tizen.org/privilege/internal/usermanagement"; -const std::string PRIVILEGE_APPSHARING_ADMIN = "http://tizen.org/privilege/notexist"; -const std::string PRIVILEGE_SHM = "http://tizen.org/privilege/internal/shm"; -const std::string PRIVILEGE_APP_NAMESPACE = "http://tizen.org/privilege/notexist"; -const std::string PRIVILEGE_PERMISSION_CHECK = "http://tizen.org/privilege/permission.check"; - -const std::string APPS_LABELS_FILE = "apps-labels"; -const std::string SKEL_DIR = "/etc/skel"; - -const std::string PRIVILEGE_GROUP_LIST_FILE = POLICY_DIR "/privilege-group.list"; -const std::string PRIVILEGE_MOUNT_LIST_FILE = POLICY_DIR "/privilege-mount.list"; - -const std::string PRIVACY_POLICY_DESC = "Ask user"; -#ifdef ASKUSER_ENABLED -const bool IS_ASKUSER_ENABLED = true; -#else -const bool IS_ASKUSER_ENABLED = false; -#endif - std::string getPrivilegeDbPath() { return TizenPlatformConfig::makePath(TZ_SYS_DB, ".security-manager.db"); } diff --git a/src/common/cynara.cpp b/src/common/cynara.cpp index d5f2a90..f4a6983 100644 --- a/src/common/cynara.cpp +++ b/src/common/cynara.cpp @@ -393,13 +393,13 @@ void CynaraAdmin::updateAppPolicy( bool askUserEnabled = false; int askUserPolicy = static_cast(CynaraAdminPolicy::Operation::Allow); - if (Config::IS_ASKUSER_ENABLED) { + if (IS_ASKUSER_ENABLED) { try { - askUserPolicy = convertToPolicyType(Config::PRIVACY_POLICY_DESC); + askUserPolicy = convertToPolicyType(PRIVACY_POLICY_DESC); askUserEnabled = true; } catch (const std::out_of_range&) { // Cynara doesn't know "Ask user" - LogDebug("Unknown policy level: " << Config::PRIVACY_POLICY_DESC); + LogDebug("Unknown policy level: " << PRIVACY_POLICY_DESC); } } @@ -551,13 +551,13 @@ void CynaraAdmin::userInit(uid_t uid, security_manager_user_type userType) int askUserPolicy = static_cast(CynaraAdminPolicy::Operation::Allow); bool askUserEnabled = false; - if (Config::IS_ASKUSER_ENABLED) { + if (IS_ASKUSER_ENABLED) { try{ - askUserPolicy = convertToPolicyType(Config::PRIVACY_POLICY_DESC); + askUserPolicy = convertToPolicyType(PRIVACY_POLICY_DESC); askUserEnabled = true; } catch (const std::out_of_range&) { // Cynara doesn't know "Ask user" - LogDebug("Unknown policy level: " << Config::PRIVACY_POLICY_DESC); + LogDebug("Unknown policy level: " << PRIVACY_POLICY_DESC); } } diff --git a/src/common/include/config.h b/src/common/include/config.h index a71b9f9..230f15b 100644 --- a/src/common/include/config.h +++ b/src/common/include/config.h @@ -31,35 +31,6 @@ namespace SecurityManager { namespace Config { -/* Service name */ -extern const std::string SERVICE_NAME; - -/* Privileges required from users of our API */ -extern const std::string PRIVILEGE_APPINST_USER; -extern const std::string PRIVILEGE_APPINST_ADMIN; -extern const std::string PRIVILEGE_USER_ADMIN; -extern const std::string PRIVILEGE_POLICY_USER; -extern const std::string PRIVILEGE_POLICY_ADMIN; -extern const std::string PRIVILEGE_APPSHARING_ADMIN; -extern const std::string PRIVILEGE_SHM; -extern const std::string PRIVILEGE_APP_NAMESPACE; -extern const std::string PRIVILEGE_PERMISSION_CHECK; - -/* Files used in permitted label managment */ -extern const std::string APPS_LABELS_FILE; - -/* Policy files */ -extern const std::string PRIVILEGE_GROUP_LIST_FILE; -extern const std::string PRIVILEGE_MOUNT_LIST_FILE; - -extern const std::string SKEL_DIR; - -/* Ask-user policy description */ -extern const std::string PRIVACY_POLICY_DESC; - -/* true if privacy-related privileges should result in UI-popup question*/ -extern const bool IS_ASKUSER_ENABLED; - std::string getPrivilegeDbPath(); std::string getPrivilegeDbFallbackPath(); @@ -75,3 +46,37 @@ std::string getPrivilegeDbFallbackPath(); #define DB_JOURNAL_SUFFIX "-journal" #define DB_OK_MARKER "/tmp/.security-manager.db.ok" + + +/* Service name */ +#define SERVICE_NAME "security-manager" + +/* Privileges required from users of our API */ +#define PRIVILEGE_APPINST_USER "http://tizen.org/privilege/notexist" +#define PRIVILEGE_APPINST_ADMIN "http://tizen.org/privilege/notexist" +#define PRIVILEGE_USER_ADMIN "http://tizen.org/privilege/internal/usermanagement" +#define PRIVILEGE_POLICY_USER "http://tizen.org/privilege/notexist" +#define PRIVILEGE_POLICY_ADMIN "http://tizen.org/privilege/internal/usermanagement" +#define PRIVILEGE_APPSHARING_ADMIN "http://tizen.org/privilege/notexist" +#define PRIVILEGE_SHM "http://tizen.org/privilege/internal/shm" +#define PRIVILEGE_APP_NAMESPACE "http://tizen.org/privilege/notexist" +#define PRIVILEGE_PERMISSION_CHECK "http://tizen.org/privilege/permission.check" + +/* Files used in permitted label managment */ +#define APPS_LABELS_FILE "apps-labels" + +/* Policy files */ +#define PRIVILEGE_GROUP_LIST_FILE POLICY_DIR "/privilege-group.list" +#define PRIVILEGE_MOUNT_LIST_FILE POLICY_DIR "/privilege-mount.list" + +#define SKEL_DIR "/etc/skel" + +/* Ask-user policy description */ +#define PRIVACY_POLICY_DESC "Ask user" + +/* true if privacy-related privileges should result in UI-popup question*/ +#ifdef ASKUSER_ENABLED +#define IS_ASKUSER_ENABLED true +#else +#define IS_ASKUSER_ENABLED false +#endif diff --git a/src/common/mount-namespace.cpp b/src/common/mount-namespace.cpp index 2c38aee..d122b73 100644 --- a/src/common/mount-namespace.cpp +++ b/src/common/mount-namespace.cpp @@ -51,7 +51,7 @@ PrivilegePathsMap getPrivilegePathMap(uid_t uid) PrivilegePathsMap map; TizenPlatformConfig tpcUser(uid); - auto conf = ConfigFile(Config::PRIVILEGE_MOUNT_LIST_FILE).read(); + auto conf = ConfigFile(PRIVILEGE_MOUNT_LIST_FILE).read(); for (auto &confEntry : conf) { if (confEntry.size() != 4) continue; @@ -131,7 +131,7 @@ bool isMountNamespaceEnabled(void) return false; struct stat st; - if (stat(Config::PRIVILEGE_MOUNT_LIST_FILE.c_str(), &st) == -1) + if (stat(PRIVILEGE_MOUNT_LIST_FILE, &st) == -1) return false; if (st.st_size == 0) return false; diff --git a/src/common/permissible-set.cpp b/src/common/permissible-set.cpp index 10ebd5c..6ace2e1 100644 --- a/src/common/permissible-set.cpp +++ b/src/common/permissible-set.cpp @@ -84,11 +84,11 @@ std::string getPerrmissibleFileLocation(uid_t uid, int installationType) TizenPlatformConfig tpc(uid); if ((installationType == SM_APP_INSTALL_GLOBAL) || (installationType == SM_APP_INSTALL_PRELOADED)) - return tpc.ctxMakePath(TZ_SYS_VAR, Config::SERVICE_NAME, - Config::APPS_LABELS_FILE); + return tpc.ctxMakePath(TZ_SYS_VAR, SERVICE_NAME, + APPS_LABELS_FILE); else - return tpc.ctxMakePath(TZ_SYS_VAR, Config::SERVICE_NAME, - tpc.ctxGetEnv(TZ_USER_NAME), Config::APPS_LABELS_FILE); + return tpc.ctxMakePath(TZ_SYS_VAR, SERVICE_NAME, + tpc.ctxGetEnv(TZ_USER_NAME), APPS_LABELS_FILE); } static void markPermissibleFileValid(int fd, const std::string &nameFile, bool valid) diff --git a/src/common/service_impl.cpp b/src/common/service_impl.cpp index 943cca4..7c5372d 100644 --- a/src/common/service_impl.cpp +++ b/src/common/service_impl.cpp @@ -239,7 +239,7 @@ int ServiceImpl::validatePolicy(const Credentials &creds, policy_entry &policyEn policyEntry.privilege = CYNARA_ADMIN_WILDCARD; const std::string &privilege = (forAdmin || policyEntry.user.compare(std::to_string(creds.uid))) ? - Config::PRIVILEGE_POLICY_ADMIN : Config::PRIVILEGE_POLICY_USER; + PRIVILEGE_POLICY_ADMIN : PRIVILEGE_POLICY_USER; if (!authenticate(creds, privilege)) { LogError("Not enough privilege to enforce user policy"); return SECURITY_MANAGER_ERROR_ACCESS_DENIED; @@ -417,7 +417,7 @@ bool ServiceImpl::getSkelPkgDir(const std::string &pkgName, { std::string app = TizenPlatformConfig::getEnv(TZ_USER_APP); std::string home = TizenPlatformConfig::getEnv(TZ_USER_HOME); - std::string real_skel_dir = std::move(realPath(Config::SKEL_DIR)); + std::string real_skel_dir = std::move(realPath(SKEL_DIR)); if (real_skel_dir.empty()) { LogError("Unable to get skel pkg dir."); return false; @@ -447,11 +447,11 @@ bool ServiceImpl::authCheck(const Credentials &creds, int installationType) { if (installationType == SM_APP_INSTALL_LOCAL) { - if (!authenticate(creds, Config::PRIVILEGE_APPINST_USER)) { + if (!authenticate(creds, PRIVILEGE_APPINST_USER)) { LogError("Caller is not permitted to manage local applications"); return false; } - if (uid != creds.uid && !authenticate(creds, Config::PRIVILEGE_USER_ADMIN)) { + if (uid != creds.uid && !authenticate(creds, PRIVILEGE_USER_ADMIN)) { LogError("Caller is not permitted to manage applications for other users"); return false; } @@ -460,7 +460,7 @@ bool ServiceImpl::authCheck(const Credentials &creds, return false; } } else { - if (!authenticate(creds, Config::PRIVILEGE_APPINST_ADMIN)) { + if (!authenticate(creds, PRIVILEGE_APPINST_ADMIN)) { LogError("Caller is not permitted to manage global applications"); return false; } @@ -1227,7 +1227,7 @@ int ServiceImpl::getAppGroups(const Credentials &creds, const std::string &appPr int ServiceImpl::userAdd(const Credentials &creds, uid_t uidAdded, int userType) { - if (!authenticate(creds, Config::PRIVILEGE_USER_ADMIN)) { + if (!authenticate(creds, PRIVILEGE_USER_ADMIN)) { LogError("Caller is not permitted to manage users"); return SECURITY_MANAGER_ERROR_AUTHENTICATION_FAILED; } @@ -1259,7 +1259,7 @@ int ServiceImpl::userDelete(const Credentials &creds, uid_t uidDeleted) { int ret = SECURITY_MANAGER_SUCCESS; - if (!authenticate(creds, Config::PRIVILEGE_USER_ADMIN)) { + if (!authenticate(creds, PRIVILEGE_USER_ADMIN)) { LogError("Caller is not permitted to manage users"); return SECURITY_MANAGER_ERROR_AUTHENTICATION_FAILED; } @@ -1362,8 +1362,8 @@ int ServiceImpl::getConfiguredPolicy(const Credentials &creds, bool forAdmin, LogDebug("App: " << filter.appName << ", Label: " << appProcessLabel); if (forAdmin) { - if (!authenticate(creds, Config::PRIVILEGE_POLICY_ADMIN) - && !authenticate(creds, Config::PRIVILEGE_PERMISSION_CHECK)) { + if (!authenticate(creds, PRIVILEGE_POLICY_ADMIN) + && !authenticate(creds, PRIVILEGE_PERMISSION_CHECK)) { LogError("Not enough privilege to access admin enforced policies"); return SECURITY_MANAGER_ERROR_ACCESS_DENIED; } @@ -1378,15 +1378,15 @@ int ServiceImpl::getConfiguredPolicy(const Credentials &creds, bool forAdmin, LogDebug("ADMIN - number of policies matched: " << listOfPolicies.size()); } else { if (appProcessLabel != creds.label - && !authenticate(creds, Config::PRIVILEGE_POLICY_USER) - && !authenticate(creds, Config::PRIVILEGE_PERMISSION_CHECK)) { + && !authenticate(creds, PRIVILEGE_POLICY_USER) + && !authenticate(creds, PRIVILEGE_PERMISSION_CHECK)) { LogError("Not enough privilege to access user enforced policies"); return SECURITY_MANAGER_ERROR_ACCESS_DENIED; } if (uidStr.compare(user)) { - if (!authenticate(creds, Config::PRIVILEGE_POLICY_ADMIN) - && !authenticate(creds, Config::PRIVILEGE_PERMISSION_CHECK)) { + if (!authenticate(creds, PRIVILEGE_POLICY_ADMIN) + && !authenticate(creds, PRIVILEGE_PERMISSION_CHECK)) { LogWarning("Not enough privilege to access other user's personal policies"); return SECURITY_MANAGER_ERROR_ACCESS_DENIED; }; @@ -1481,8 +1481,8 @@ int ServiceImpl::getPolicy(const Credentials &creds, const policy_entry &filter, std::string uidStr = std::to_string(creds.uid); std::string pidStr = std::to_string(creds.pid); - if (!authenticate(creds, Config::PRIVILEGE_POLICY_USER) - && !authenticate(creds, Config::PRIVILEGE_PERMISSION_CHECK)) { + if (!authenticate(creds, PRIVILEGE_POLICY_USER) + && !authenticate(creds, PRIVILEGE_PERMISSION_CHECK)) { LogWarning("Not enough permission to call: " << __FUNCTION__); return SECURITY_MANAGER_ERROR_ACCESS_DENIED; }; @@ -1495,8 +1495,8 @@ int ServiceImpl::getPolicy(const Credentials &creds, const policy_entry &filter, std::vector listOfUsers; - if (authenticate(creds, Config::PRIVILEGE_POLICY_ADMIN) - || authenticate(creds, Config::PRIVILEGE_PERMISSION_CHECK)) { + if (authenticate(creds, PRIVILEGE_POLICY_ADMIN) + || authenticate(creds, PRIVILEGE_PERMISSION_CHECK)) { LogDebug("User is privileged"); if (filter.user.compare(SECURITY_MANAGER_ANY)) { LogDebug("Limitting Cynara query to user: " << filter.user); @@ -1775,7 +1775,7 @@ int ServiceImpl::applyPrivatePathSharing( SmackRules::Labels pkgsLabels; try { - if (!authenticate(creds, Config::PRIVILEGE_APPSHARING_ADMIN)) { + if (!authenticate(creds, PRIVILEGE_APPSHARING_ADMIN)) { LogError("Caller is not permitted to manage file sharing"); return SECURITY_MANAGER_ERROR_ACCESS_DENIED; } @@ -1870,7 +1870,7 @@ int ServiceImpl::dropPrivatePathSharing( { int errorRet; try { - if (!authenticate(creds, Config::PRIVILEGE_APPSHARING_ADMIN)) { + if (!authenticate(creds, PRIVILEGE_APPSHARING_ADMIN)) { LogError("Caller is not permitted to manage file sharing"); return SECURITY_MANAGER_ERROR_ACCESS_DENIED; } @@ -2028,7 +2028,7 @@ int ServiceImpl::shmAppName(const Credentials &creds, const std::string &shmName return SECURITY_MANAGER_ERROR_NO_SUCH_OBJECT; } - if (!authenticate(creds, Config::PRIVILEGE_SHM)) { + if (!authenticate(creds, PRIVILEGE_SHM)) { LogError("Request from uid=" << creds.uid << ", Smack=" << creds.label << " for shm denied"); return SECURITY_MANAGER_ERROR_AUTHENTICATION_FAILED; } @@ -2189,7 +2189,7 @@ int ServiceImpl::appSetupNamespace(const Credentials &creds, const std::string & std::vector> &privilegeStatusVector) { int ret; - if (!authenticate(creds, Config::PRIVILEGE_APP_NAMESPACE)) { + if (!authenticate(creds, PRIVILEGE_APP_NAMESPACE)) { LogError("Request from uid=" << creds.uid << ", Smack=" << creds.label << " for setup app namespace denied"); return SECURITY_MANAGER_ERROR_AUTHENTICATION_FAILED; } @@ -2265,7 +2265,7 @@ int ServiceImpl::getAppManifestPolicy(const Credentials &creds, const std::strin // Allow application to check the manifest if (((creds.label != cynaraClient) || (uidStr != CYNARA_ADMIN_WILDCARD && uidStr != std::to_string(creds.uid))) - && !(authenticate(creds, Config::PRIVILEGE_USER_ADMIN) || authenticate(creds, Config::PRIVILEGE_PERMISSION_CHECK))) + && !(authenticate(creds, PRIVILEGE_USER_ADMIN) || authenticate(creds, PRIVILEGE_PERMISSION_CHECK))) { LogError("Request from uid=" << creds.uid << ", Smack=" << creds.label << " for checking app manifest policy denied"); return SECURITY_MANAGER_ERROR_AUTHENTICATION_FAILED; @@ -2291,7 +2291,7 @@ int ServiceImpl::getAppManifestPolicy(const Credentials &creds, const std::strin int ServiceImpl::appCleanNamespace(const Credentials &creds, const std::string &appName, uid_t uid, pid_t pid) { - if (!authenticate(creds, Config::PRIVILEGE_APP_NAMESPACE)) { + if (!authenticate(creds, PRIVILEGE_APP_NAMESPACE)) { LogError("Request from uid=" << creds.uid << ", Smack=" << creds.label << " for cleanup app namespace denied"); return SECURITY_MANAGER_ERROR_AUTHENTICATION_FAILED; -- 2.7.4