From b0cb6b55e919718401a3fea9a777c5ba737ccb7d Mon Sep 17 00:00:00 2001 From: Marcin Lis Date: Tue, 15 Jul 2014 18:48:14 +0200 Subject: [PATCH 01/16] Logging: Remove the log tag from logs messages The log tag "SECURITY_MANAGER" and its client's version that were used in dlog messages are not needed in systemd journal logs, this is redundant information. It is easy to maintain the source of logs using journalctl. Change-Id: Ia987cb3e401f46fe15eea210a0c2a9406caa7882 Signed-off-by: Marcin Lis --- src/dpl/log/src/sd_journal_provider.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dpl/log/src/sd_journal_provider.cpp b/src/dpl/log/src/sd_journal_provider.cpp index 6c22929..bd570ca 100644 --- a/src/dpl/log/src/sd_journal_provider.cpp +++ b/src/dpl/log/src/sd_journal_provider.cpp @@ -60,7 +60,7 @@ void SdJournalProvider::Debug(const char *message, const char *function) { // sd-journal imports LOG priorities from the syslog, see syslog(3) for details - sd_journal_print(LOG_DEBUG, "%s: %s", m_tag.c_str(), + sd_journal_print(LOG_DEBUG, "%s", (FormatMessage(message, filename, line, function)).c_str()); } @@ -69,7 +69,7 @@ void SdJournalProvider::Info(const char *message, int line, const char *function) { - sd_journal_print(LOG_INFO, "%s: %s", m_tag.c_str(), + sd_journal_print(LOG_INFO, "%s", (FormatMessage(message, filename, line, function)).c_str()); } @@ -78,7 +78,7 @@ void SdJournalProvider::Warning(const char *message, int line, const char *function) { - sd_journal_print(LOG_WARNING, "%s: %s", m_tag.c_str(), + sd_journal_print(LOG_WARNING, "%s", (FormatMessage(message, filename, line, function)).c_str()); } @@ -87,7 +87,7 @@ void SdJournalProvider::Error(const char *message, int line, const char *function) { - sd_journal_print(LOG_ERR, "%s: %s", m_tag.c_str(), + sd_journal_print(LOG_ERR, "%s", (FormatMessage(message, filename, line, function)).c_str()); } -- 2.7.4 From fa749491cb36afa924364e703b7d097546d17f36 Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Fri, 11 Jul 2014 17:50:43 +0200 Subject: [PATCH 02/16] Cynara: implement method for setting policies Change-Id: I65a1c54c6307a60fba383b9e376c8541908ded59 Signed-off-by: Rafal Krypa --- src/server/service/cynara.cpp | 71 +++++++++++++++++++++++++++++++++++++ src/server/service/include/cynara.h | 29 +++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/src/server/service/cynara.cpp b/src/server/service/cynara.cpp index ab760ef..7b1dbb8 100644 --- a/src/server/service/cynara.cpp +++ b/src/server/service/cynara.cpp @@ -21,11 +21,68 @@ * @brief Wrapper class for Cynara interface */ +#include #include +#include #include "cynara.h" namespace SecurityManager { + +CynaraAdminPolicy::CynaraAdminPolicy(const std::string &client, const std::string &user, + const std::string &privilege, Operation operation, + const std::string &bucket) +{ + this->client = strdup(client.c_str()); + this->user = strdup(user.c_str()); + this->privilege = strdup(privilege.c_str()); + this->bucket = strdup(bucket.c_str()); + + if (this->bucket == nullptr || this->client == nullptr || + this->user == nullptr || this->privilege == nullptr) { + free(this->bucket); + free(this->client); + free(this->user); + free(this->privilege); + throw std::bad_alloc(); + } + + this->result = static_cast(operation); + this->result_extra = nullptr; +} + +CynaraAdminPolicy::CynaraAdminPolicy(const std::string &client, const std::string &user, + const std::string &privilege, const std::string &goToBucket, + const std::string &bucket) +{ + this->bucket = strdup(bucket.c_str()); + this->client = strdup(client.c_str()); + this->user = strdup(user.c_str()); + this->privilege = strdup(privilege.c_str()); + this->result_extra = strdup(goToBucket.c_str()); + this->result = CYNARA_ADMIN_BUCKET; + + if (this->bucket == nullptr || this->client == nullptr || + this->user == nullptr || this->privilege == nullptr || + this->result_extra == nullptr) { + free(this->bucket); + free(this->client); + free(this->user); + free(this->privilege); + free(this->result_extra); + throw std::bad_alloc(); + } +} + +CynaraAdminPolicy::~CynaraAdminPolicy() +{ + free(this->bucket); + free(this->client); + free(this->user); + free(this->privilege); + free(this->result_extra); +} + static void checkCynaraAdminError(int result, const std::string &msg) { switch (result) { @@ -54,4 +111,18 @@ CynaraAdmin::~CynaraAdmin() cynara_admin_finish(m_CynaraAdmin); } +void CynaraAdmin::SetPolicies(const std::vector &policies) +{ + std::vector pp_policies(policies.size() + 1); + + for (std::size_t i = 0; i < policies.size(); ++i) + pp_policies[i] = static_cast(&policies[i]); + + pp_policies[policies.size()] = nullptr; + + checkCynaraAdminError( + cynara_admin_set_policies(m_CynaraAdmin, pp_policies.data()), + "Error while updating Cynara policy."); +} + } // namespace SecurityManager diff --git a/src/server/service/include/cynara.h b/src/server/service/include/cynara.h index e11b133..b9ef6ed 100644 --- a/src/server/service/include/cynara.h +++ b/src/server/service/include/cynara.h @@ -26,6 +26,7 @@ #include #include +#include namespace SecurityManager { @@ -39,12 +40,40 @@ public: DECLARE_EXCEPTION_TYPE(Base, UnknownError) }; +struct CynaraAdminPolicy : cynara_admin_policy +{ + enum class Operation { + Deny = CYNARA_ADMIN_DENY, + Allow = CYNARA_ADMIN_ALLOW, + Delete = CYNARA_ADMIN_DELETE, + Bucket = CYNARA_ADMIN_BUCKET, + }; + + CynaraAdminPolicy(const std::string &client, const std::string &user, + const std::string &privilege, Operation operation, + const std::string &bucket = std::string(CYNARA_ADMIN_DEFAULT_BUCKET)); + + CynaraAdminPolicy(const std::string &client, const std::string &user, + const std::string &privilege, const std::string &goToBucket, + const std::string &bucket = std::string(CYNARA_ADMIN_DEFAULT_BUCKET)); + + ~CynaraAdminPolicy(); +}; + class CynaraAdmin { public: CynaraAdmin(); virtual ~CynaraAdmin(); + /** + * Update Cynara policies. + * Caller must have permission to access Cynara administrative socket. + * + * @param policies vector of CynaraAdminPolicy objects to send to Cynara + */ + void SetPolicies(const std::vector &policies); + private: struct cynara_admin *m_CynaraAdmin; }; -- 2.7.4 From 3385812d18cb304f997434fb877fb3a8477068e2 Mon Sep 17 00:00:00 2001 From: Marcin Lis Date: Wed, 16 Jul 2014 16:54:59 +0200 Subject: [PATCH 03/16] Cynara: Change the type of exception in CynaraAdminPolicy constructors It is better to keep exception types unified. That would minimize the number of "catch" statements. Change-Id: Id9e5bafef70c7ffb126a60c595505b644d596729 Signed-off-by: Marcin Lis --- src/server/service/cynara.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/server/service/cynara.cpp b/src/server/service/cynara.cpp index 7b1dbb8..2cf3f19 100644 --- a/src/server/service/cynara.cpp +++ b/src/server/service/cynara.cpp @@ -44,7 +44,8 @@ CynaraAdminPolicy::CynaraAdminPolicy(const std::string &client, const std::strin free(this->client); free(this->user); free(this->privilege); - throw std::bad_alloc(); + ThrowMsg(CynaraException::OutOfMemory, + std::string("Error in CynaraAdminPolicy allocation.")); } this->result = static_cast(operation); @@ -70,7 +71,8 @@ CynaraAdminPolicy::CynaraAdminPolicy(const std::string &client, const std::strin free(this->user); free(this->privilege); free(this->result_extra); - throw std::bad_alloc(); + ThrowMsg(CynaraException::OutOfMemory, + std::string("Error in CynaraAdminPolicy allocation.")); } } -- 2.7.4 From 536102ce612c9d61053e1dca4e87f04d30e00c28 Mon Sep 17 00:00:00 2001 From: Jan Cybulski Date: Fri, 18 Jul 2014 17:35:41 +0200 Subject: [PATCH 04/16] Add possibility of installing apps for different users Uid of installing user will be obtained from peer's socket and will be stored in database. Change-Id: I0a0edf726b54fc7b28e5f2063186a97eb29479a9 Signed-off-by: Jan Cybulski --- src/server/db/db.sql | 11 +++--- src/server/db/include/privilege_db.h | 25 +++++++------ src/server/db/privilege_db.cpp | 19 ++++++---- src/server/service/include/installer.h | 8 +++-- src/server/service/installer.cpp | 65 +++++++++++++++++++++++++++------- 5 files changed, 92 insertions(+), 36 deletions(-) diff --git a/src/server/db/db.sql b/src/server/db/db.sql index 680c2ed..513a664 100644 --- a/src/server/db/db.sql +++ b/src/server/db/db.sql @@ -15,8 +15,9 @@ UNIQUE (name) CREATE TABLE IF NOT EXISTS app ( app_id INTEGER PRIMARY KEY, pkg_id INTEGER NOT NULL, +uid INTEGER NOT NULL, name VARCHAR NOT NULL , -UNIQUE (name), +UNIQUE (name, uid), FOREIGN KEY (pkg_id) REFERENCES pkg (pkg_id) ); @@ -46,6 +47,7 @@ CREATE VIEW app_privilege_view AS SELECT app_privilege.app_id as app_id, app.name as app_name, + app.uid as uid, app.pkg_id as pkg_id, pkg.name as pkg_name, app_privilege.privilege_id as privilege_id, @@ -61,6 +63,7 @@ SELECT app.app_id, app.name as app_name, app.pkg_id, + app.uid, pkg.name as pkg_name FROM app LEFT JOIN pkg USING (pkg_id); @@ -71,7 +74,7 @@ INSTEAD OF INSERT ON app_privilege_view BEGIN INSERT OR IGNORE INTO privilege(name) VALUES (NEW.privilege_name); INSERT OR IGNORE INTO app_privilege(app_id, privilege_id) VALUES - ((SELECT app_id FROM app WHERE name=NEW.app_name), + ((SELECT app_id FROM app WHERE name=NEW.app_name AND uid=NEW.uid), (SELECT privilege_id FROM privilege WHERE name=NEW.privilege_name)); END; @@ -87,14 +90,14 @@ CREATE TRIGGER app_pkg_view_insert_trigger INSTEAD OF INSERT ON app_pkg_view BEGIN INSERT OR IGNORE INTO pkg(name) VALUES (NEW.pkg_name); - INSERT OR IGNORE INTO app(pkg_id, name) VALUES ((SELECT pkg_id FROM pkg WHERE name=NEW.pkg_name), NEW.app_name); + INSERT OR IGNORE INTO app(pkg_id, name, uid) VALUES ((SELECT pkg_id FROM pkg WHERE name=NEW.pkg_name), NEW.app_name, NEW.uid); END; DROP TRIGGER IF EXISTS app_pkg_view_delete_trigger; CREATE TRIGGER app_pkg_view_delete_trigger INSTEAD OF DELETE ON app_pkg_view BEGIN - DELETE FROM app WHERE app_id=OLD.app_id; + DELETE FROM app WHERE app_id=OLD.app_id AND uid=OLD.uid; DELETE FROM pkg WHERE pkg_id NOT IN (SELECT DISTINCT pkg_id from app); END; diff --git a/src/server/db/include/privilege_db.h b/src/server/db/include/privilege_db.h index b90c1c0..7890022 100644 --- a/src/server/db/include/privilege_db.h +++ b/src/server/db/include/privilege_db.h @@ -61,11 +61,11 @@ class PrivilegeDb { private: SecurityManager::DB::SqlConnection *mSqlConnection; const std::map Queries = { - { QueryType::EGetPkgPrivileges, "SELECT privilege_name FROM app_privilege_view WHERE pkg_name=?"}, - { QueryType::EAddApplication, "INSERT INTO app_pkg_view (app_name, pkg_name) VALUES (?, ?)" }, - { QueryType::ERemoveApplication, "DELETE FROM app_pkg_view WHERE app_name=?" }, - { QueryType::EAddAppPrivileges, "INSERT INTO app_privilege_view (app_name, privilege_name) VALUES (?, ?)" }, - { QueryType::ERemoveAppPrivileges, "DELETE FROM app_privilege_view WHERE app_name=?" }, + { QueryType::EGetPkgPrivileges, "SELECT privilege_name FROM app_privilege_view WHERE pkg_name=? AND uid=?"}, + { QueryType::EAddApplication, "INSERT INTO app_pkg_view (app_name, pkg_name, uid) VALUES (?, ?, ?)" }, + { QueryType::ERemoveApplication, "DELETE FROM app_pkg_view WHERE app_name=? AND uid=?" }, + { QueryType::EAddAppPrivileges, "INSERT INTO app_privilege_view (app_name, uid, privilege_name) VALUES (?, ?, ?)" }, + { QueryType::ERemoveAppPrivileges, "DELETE FROM app_privilege_view WHERE app_name=? AND uid=?" }, { QueryType::EPkgIdExists, "SELECT * FROM pkg WHERE name=?" }, { QueryType::EGetPkgId, " SELECT pkg_name FROM app_pkg_view WHERE app_name = ?" }, }; @@ -133,10 +133,11 @@ public: * Retrieve list of privileges assigned to a pkgId * * @param pkgId - package identifier + * @param uid - user identifier for whom privileges will be retrieved * @param[out] currentPrivileges - list of current privileges assigned to pkgId * @exception DB::SqlConnection::Exception::InternalError on internal error */ - void GetPkgPrivileges(const std::string &pkgId, + void GetPkgPrivileges(const std::string &pkgId, uid_t uid, std::vector ¤tPrivilege); /** @@ -144,38 +145,42 @@ public: * * @param appId - application identifier * @param pkgId - package identifier + * @param uid - user identifier for whom application is going to be installed * @param[out] pkgIdIsNew - return info if pkgId is new to the database * @exception DB::SqlConnection::Exception::InternalError on internal error */ void AddApplication(const std::string &appId, const std::string &pkgId, - bool &pkgIdIsNew); + uid_t uid, bool &pkgIdIsNew); /** * Remove an application from the database * * @param appId - application identifier + * @param uid - user identifier whose application is going to be uninstalled * @param[out] pkgIdIsNoMore - return info if pkgId is in the database * @exception DB::SqlConnection::Exception::InternalError on internal error */ - void RemoveApplication(const std::string &appId, bool &pkgIdIsNoMore); + void RemoveApplication(const std::string &appId, uid_t uid, bool &pkgIdIsNoMore); /** * Remove privileges assigned to application * * @param appId - application identifier + * @param uid - user identifier for whom privileges will be removed * @exception DB::SqlConnection::Exception::InternalError on internal error */ - void RemoveAppPrivileges(const std::string &appId); + void RemoveAppPrivileges(const std::string &appId, uid_t uid); /** * Update privileges assigned to application * To assure data integrity this method must be called inside db transaction. * * @param appId - application identifier + * @param uid - user identifier for whom privileges will be updated * @param privileges - list of privileges to assign * @exception DB::SqlConnection::Exception::InternalError on internal error */ - void UpdateAppPrivileges(const std::string &appId, + void UpdateAppPrivileges(const std::string &appId, uid_t uid, const std::vector &privileges); }; diff --git a/src/server/db/privilege_db.cpp b/src/server/db/privilege_db.cpp index 5eafde6..32e18b4 100644 --- a/src/server/db/privilege_db.cpp +++ b/src/server/db/privilege_db.cpp @@ -132,7 +132,7 @@ bool PrivilegeDb::GetAppPkgId(const std::string &appId, std::string &pkgId) } void PrivilegeDb::AddApplication(const std::string &appId, - const std::string &pkgId, bool &pkgIdIsNew) + const std::string &pkgId, uid_t uid, bool &pkgIdIsNew) { pkgIdIsNew = !(this->PkgIdExists(pkgId)); @@ -143,6 +143,7 @@ void PrivilegeDb::AddApplication(const std::string &appId, command->BindString(1, appId.c_str()); command->BindString(2, pkgId.c_str()); + command->BindInteger(3, static_cast(uid)); if (command->Step()) { LogDebug("Unexpected SQLITE_ROW answer to query: " << @@ -154,7 +155,7 @@ void PrivilegeDb::AddApplication(const std::string &appId, }); } -void PrivilegeDb::RemoveApplication(const std::string &appId, +void PrivilegeDb::RemoveApplication(const std::string &appId, uid_t uid, bool &pkgIdIsNoMore) { try_catch([&] { @@ -169,6 +170,7 @@ void PrivilegeDb::RemoveApplication(const std::string &appId, Queries.at(QueryType::ERemoveApplication)); command->BindString(1, appId.c_str()); + command->BindInteger(2, static_cast(uid)); if (command->Step()) { LogDebug("Unexpected SQLITE_ROW answer to query: " << @@ -182,7 +184,7 @@ void PrivilegeDb::RemoveApplication(const std::string &appId, }); } -void PrivilegeDb::GetPkgPrivileges(const std::string &pkgId, +void PrivilegeDb::GetPkgPrivileges(const std::string &pkgId, uid_t uid, std::vector ¤tPrivileges) { try_catch([&] { @@ -190,6 +192,7 @@ void PrivilegeDb::GetPkgPrivileges(const std::string &pkgId, mSqlConnection->PrepareDataCommand( Queries.at(QueryType::EGetPkgPrivileges)); command->BindString(1, pkgId.c_str()); + command->BindInteger(2, static_cast(uid)); while (command->Step()) { std::string privilege = command->GetColumnString(0); @@ -199,13 +202,14 @@ void PrivilegeDb::GetPkgPrivileges(const std::string &pkgId, }); } -void PrivilegeDb::RemoveAppPrivileges(const std::string &appId) +void PrivilegeDb::RemoveAppPrivileges(const std::string &appId, uid_t uid) { try_catch([&] { DB::SqlConnection::DataCommandAutoPtr command = mSqlConnection->PrepareDataCommand(Queries.at(QueryType::ERemoveAppPrivileges)); command->BindString(1, appId.c_str()); + command->BindInteger(2, static_cast(uid)); if (command->Step()) { LogDebug("Unexpected SQLITE_ROW answer to query: " << Queries.at(QueryType::ERemoveAppPrivileges)); @@ -215,18 +219,19 @@ void PrivilegeDb::RemoveAppPrivileges(const std::string &appId) }); } -void PrivilegeDb::UpdateAppPrivileges(const std::string &appId, +void PrivilegeDb::UpdateAppPrivileges(const std::string &appId, uid_t uid, const std::vector &privileges) { try_catch([&] { DB::SqlConnection::DataCommandAutoPtr command = mSqlConnection->PrepareDataCommand(Queries.at(QueryType::EAddAppPrivileges)); command->BindString(1, appId.c_str()); + command->BindInteger(2, static_cast(uid)); - RemoveAppPrivileges(appId); + RemoveAppPrivileges(appId, uid); for (const auto &privilege : privileges) { - command->BindString(2, privilege.c_str()); + command->BindString(3, privilege.c_str()); command->Step(); command->Reset(); LogDebug("Added privilege: " << privilege << " to appId: " << appId); diff --git a/src/server/service/include/installer.h b/src/server/service/include/installer.h index c1f8d42..1a7caf2 100644 --- a/src/server/service/include/installer.h +++ b/src/server/service/include/installer.h @@ -77,18 +77,20 @@ private: * * @param buffer Raw received data buffer * @param send Raw data buffer to be sent + * @param uid User's identifier for whom application will be installed * @return true on success */ - bool processAppInstall(MessageBuffer &buffer, MessageBuffer &send); + bool processAppInstall(MessageBuffer &buffer, MessageBuffer &send, uid_t uid); /** - * Process libprivilege-control action and store result in a bufer + * Process application uninstallation * * @param buffer Raw received data buffer * @param send Raw data buffer to be sent + * @param uid User's identifier for whom application will be uninstalled * @return true on success */ - bool processAppUninstall(MessageBuffer &buffer, MessageBuffer &send); + bool processAppUninstall(MessageBuffer &buffer, MessageBuffer &send, uid_t uid); /** * Process getting package id from app id diff --git a/src/server/service/installer.cpp b/src/server/service/installer.cpp index 8f7bb0c..6451dc8 100644 --- a/src/server/service/installer.cpp +++ b/src/server/service/installer.cpp @@ -27,6 +27,8 @@ #include #include +#include +#include #include "installer.h" #include "protocols.h" @@ -89,6 +91,19 @@ void InstallerService::close(const CloseEvent &event) m_connectionInfoMap.erase(event.connectionID.counter); } +static bool getPeerUserID(int sock, uid_t *uid) { + struct ucred cr; + socklen_t len = sizeof (cr); + if (!uid) { + return false; + } + if (!getsockopt(sock, SOL_SOCKET, SO_PEERCRED, &cr, &len)) { + *uid = cr.uid; + return true; + } + return false; +} + bool InstallerService::processOne(const ConnectionID &conn, MessageBuffer &buffer, InterfaceID interfaceID) { @@ -102,6 +117,14 @@ bool InstallerService::processOne(const ConnectionID &conn, MessageBuffer &buffe MessageBuffer send; bool retval = false; + uid_t uid; + + if(!getPeerUserID(conn.sock, &uid)) { + LogError("Closing socket because of error: unable to get peer's uid"); + m_serviceManager->Close(conn); + return false; + } + if (INSTALLER_IFACE == interfaceID) { Try { // deserialize API call type @@ -112,11 +135,11 @@ bool InstallerService::processOne(const ConnectionID &conn, MessageBuffer &buffe switch (call_type) { case SecurityModuleCall::APP_INSTALL: LogDebug("call_type: SecurityModuleCall::APP_INSTALL"); - processAppInstall(buffer, send); + processAppInstall(buffer, send, uid); break; case SecurityModuleCall::APP_UNINSTALL: LogDebug("call_type: SecurityModuleCall::APP_UNINSTALL"); - processAppUninstall(buffer, send); + processAppUninstall(buffer, send, uid); break; case SecurityModuleCall::APP_GET_PKGID: processGetPkgId(buffer, send); @@ -152,7 +175,19 @@ bool InstallerService::processOne(const ConnectionID &conn, MessageBuffer &buffe return retval; } -bool InstallerService::processAppInstall(MessageBuffer &buffer, MessageBuffer &send) +static inline bool installRequestAuthCheck(const app_inst_req &req, uid_t uid) +{ + for (const auto &appPath : req.appPaths) { + app_install_path_type pathType = static_cast(appPath.second); + if (pathType == SECURITY_MANAGER_PATH_PUBLIC && uid != 0) { + LogDebug("Only root can register SECURITY_MANAGER_PATH_PUBLIC path"); + return false; + } + } + return true; +} + +bool InstallerService::processAppInstall(MessageBuffer &buffer, MessageBuffer &send, uid_t uid) { bool pkgIdIsNew = false; std::vector addedPermissions; @@ -165,6 +200,12 @@ bool InstallerService::processAppInstall(MessageBuffer &buffer, MessageBuffer &s Deserialization::Deserialize(buffer, req.privileges); Deserialization::Deserialize(buffer, req.appPaths); + if(!installRequestAuthCheck(req, uid)) { + LogError("Request from uid " << uid << " for app installation denied"); + Serialization::Serialize(send, SECURITY_MANAGER_API_ERROR_AUTHENTICATION_FAILED); + return false; + } + std::string smackLabel; if (!generateAppLabel(req.pkgId, smackLabel)) { LogError("Cannot generate Smack label for package: " << req.pkgId); @@ -211,10 +252,10 @@ bool InstallerService::processAppInstall(MessageBuffer &buffer, MessageBuffer &s std::vector oldPkgPrivileges, newPkgPrivileges; m_privilegeDb.BeginTransaction(); - m_privilegeDb.GetPkgPrivileges(req.pkgId, oldPkgPrivileges); - m_privilegeDb.AddApplication(req.appId, req.pkgId, pkgIdIsNew); - m_privilegeDb.UpdateAppPrivileges(req.appId, req.privileges); - m_privilegeDb.GetPkgPrivileges(req.pkgId, newPkgPrivileges); + m_privilegeDb.GetPkgPrivileges(req.pkgId, uid, oldPkgPrivileges); + m_privilegeDb.AddApplication(req.appId, req.pkgId, uid, pkgIdIsNew); + m_privilegeDb.UpdateAppPrivileges(req.appId, uid, req.privileges); + m_privilegeDb.GetPkgPrivileges(req.pkgId, uid, newPkgPrivileges); // TODO: configure Cynara rules based on oldPkgPrivileges and newPkgPrivileges m_privilegeDb.CommitTransaction(); LogDebug("Application installation commited to database"); @@ -270,7 +311,7 @@ error_label: return false; } -bool InstallerService::processAppUninstall(MessageBuffer &buffer, MessageBuffer &send) +bool InstallerService::processAppUninstall(MessageBuffer &buffer, MessageBuffer &send, uid_t uid) { // deserialize request data std::string appId; @@ -307,10 +348,10 @@ bool InstallerService::processAppUninstall(MessageBuffer &buffer, MessageBuffer LogDebug("Unnstall parameters: appId: " << appId << ", pkgId: " << pkgId << ", generated smack label: " << smackLabel); - m_privilegeDb.GetPkgPrivileges(pkgId, oldPkgPrivileges); - m_privilegeDb.UpdateAppPrivileges(appId, std::vector()); - m_privilegeDb.RemoveApplication(appId, removePkg); - m_privilegeDb.GetPkgPrivileges(pkgId, newPkgPrivileges); + m_privilegeDb.GetPkgPrivileges(pkgId, uid, oldPkgPrivileges); + m_privilegeDb.UpdateAppPrivileges(appId, uid, std::vector()); + m_privilegeDb.RemoveApplication(appId, uid, removePkg); + m_privilegeDb.GetPkgPrivileges(pkgId, uid, newPkgPrivileges); // TODO: configure Cynara rules based on oldPkgPrivileges and newPkgPrivileges m_privilegeDb.CommitTransaction(); LogDebug("Application uninstallation commited to database"); -- 2.7.4 From f0da65c408fa2c8a776d91733fde94f10c63147f Mon Sep 17 00:00:00 2001 From: Jan Cybulski Date: Fri, 18 Jul 2014 10:56:11 +0200 Subject: [PATCH 05/16] Register only directories inside user's HOME Change-Id: I546ba542dea481db2efebb24bbe03e5cd87d7220 Signed-off-by: Jan Cybulski --- src/server/service/installer.cpp | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/src/server/service/installer.cpp b/src/server/service/installer.cpp index 6451dc8..286fce6 100644 --- a/src/server/service/installer.cpp +++ b/src/server/service/installer.cpp @@ -29,6 +29,9 @@ #include #include #include +#include +#include +#include #include "installer.h" #include "protocols.h" @@ -177,12 +180,39 @@ bool InstallerService::processOne(const ConnectionID &conn, MessageBuffer &buffe static inline bool installRequestAuthCheck(const app_inst_req &req, uid_t uid) { - for (const auto &appPath : req.appPaths) { - app_install_path_type pathType = static_cast(appPath.second); - if (pathType == SECURITY_MANAGER_PATH_PUBLIC && uid != 0) { - LogDebug("Only root can register SECURITY_MANAGER_PATH_PUBLIC path"); + struct passwd *pwd; + char buffer[PATH_MAX]; + do { + errno = 0; + pwd = getpwuid(uid); + if (!pwd && errno != EINTR) { + LogError("getpwuid failed with '" << uid << "' as paramter: " << strerror(errno)); return false; } + } while (!pwd); + + for (const auto &appPath : req.appPaths) { + + if (uid != 0) { + char *real_path = realpath(appPath.first.c_str(), buffer); + if (!real_path) { + LogError("realpath failed with '" << appPath.first.c_str() + << "' as paramter: " << strerror(errno)); + return false; + } + LogDebug("Requested path is '" << appPath.first.c_str() + << "'. User's HOME is '" << pwd->pw_dir << "'"); + if (strncmp(pwd->pw_dir, real_path, strlen(pwd->pw_dir))!=0) { + LogWarning("User's apps may have registered folders only in user's home dir"); + return false; + } + + app_install_path_type pathType = static_cast(appPath.second); + if (pathType == SECURITY_MANAGER_PATH_PUBLIC) { + LogWarning("Only root can register SECURITY_MANAGER_PATH_PUBLIC path"); + return false; + } + } } return true; } -- 2.7.4 From 3fcb46fea965ce537c9c8308906a071385748157 Mon Sep 17 00:00:00 2001 From: Jan Cybulski Date: Fri, 18 Jul 2014 14:42:25 +0200 Subject: [PATCH 06/16] Change security_manager_app_install return code So far, security_manager_app_install returned only SECURITY_MANAGER_SUCCESS or SECURITY_MANAGER_ERROR_UNKNOWN, which is not enough now. Now, there is possibility, that security manager would reject installation of some applciations on the basis of uid and users home directory. This function will return information about that now as return code. Change-Id: I53b23b8318a756a8fbf4b804e49046cfa5acd4e0 Signed-off-by: Jan Cybulski --- src/client/client-security-manager.cpp | 11 ++++++++--- src/include/security-manager.h | 9 +++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/client/client-security-manager.cpp b/src/client/client-security-manager.cpp index 9de0fa9..5f84f16 100644 --- a/src/client/client-security-manager.cpp +++ b/src/client/client-security-manager.cpp @@ -136,10 +136,15 @@ int security_manager_app_install(const app_inst_req *p_req) //receive response from server Deserialization::Deserialize(recv, retval); - if (retval != SECURITY_MANAGER_API_SUCCESS) - return SECURITY_MANAGER_ERROR_UNKNOWN; + switch(retval) { + case SECURITY_MANAGER_API_SUCCESS: + return SECURITY_MANAGER_SUCCESS; + case SECURITY_MANAGER_API_ERROR_AUTHENTICATION_FAILED: + return SECURITY_MANAGER_ERROR_AUTHENTICATION_FAILED; + default: + return SECURITY_MANAGER_ERROR_UNKNOWN; + } - return SECURITY_MANAGER_SUCCESS;; }); } diff --git a/src/include/security-manager.h b/src/include/security-manager.h index 6bc2441..760ee3f 100644 --- a/src/include/security-manager.h +++ b/src/include/security-manager.h @@ -108,7 +108,8 @@ enum lib_retcode { SECURITY_MANAGER_ERROR_UNKNOWN, SECURITY_MANAGER_ERROR_INPUT_PARAM, SECURITY_MANAGER_ERROR_MEMORY, - SECURITY_MANAGER_ERROR_REQ_NOT_COMPLETE + SECURITY_MANAGER_ERROR_REQ_NOT_COMPLETE, + SECURITY_MANAGER_ERROR_AUTHENTICATION_FAILED }; /*! \brief accesses types for application installation paths*/ @@ -188,7 +189,11 @@ int security_manager_app_inst_req_add_path(app_inst_req *p_req, const char *path * using filled up app_inst_req data structure * * \param[in] Pointer handling app_inst_req structure - * \return API return code or error code + * \return API return code or error code: it would be + * - SECURITY_MANAGER_SUCCESS on success, + * - SECURITY_MANAGER_ERROR_AUTHENTICATION_FAILED when user does not + * have rights to install requested directories, + * - SECURITY_MANAGER_ERROR_UNKNOWN on other errors. */ int security_manager_app_install(const app_inst_req *p_req); -- 2.7.4 From 6d04432eca534a589ba946d5c83ebe84992af9c7 Mon Sep 17 00:00:00 2001 From: Jan Cybulski Date: Fri, 18 Jul 2014 15:35:29 +0200 Subject: [PATCH 07/16] Move return codes sent by server to protocols.h Those codes are not part of security-manager's API but are used only in communication between client and server part. Return codes of libsecurity-manager's functions are defined in enum lib_retcode, so there is no need in placing additional macros in header file security-manager.h Also: fix problems with documentation in those macros Change-Id: Iaa2f489f2b0a3e9dc3d2aaf74f522451e1b65057 Signed-off-by: Jan Cybulski --- src/client/client-common.cpp | 2 +- src/common/include/protocols.h | 72 ++++++++++++++++++++++++++++++++++++++++++ src/include/security-manager.h | 69 ---------------------------------------- 3 files changed, 73 insertions(+), 70 deletions(-) diff --git a/src/client/client-common.cpp b/src/client/client-common.cpp index d43aa94..a58f7b3 100644 --- a/src/client/client-common.cpp +++ b/src/client/client-common.cpp @@ -40,7 +40,7 @@ #include #include -#include +#include IMPLEMENT_SAFE_SINGLETON(SecurityManager::Log::LogSystem); diff --git a/src/common/include/protocols.h b/src/common/include/protocols.h index d6ba8bb..fadc46c 100644 --- a/src/common/include/protocols.h +++ b/src/common/include/protocols.h @@ -28,6 +28,78 @@ #include #include +/** + * \name Return Codes + * exported by the foundation API. + * result codes begin with the start error code and extend into negative direction. + * @{ +*/ + +/*! \brief indicating the result of the one specific API is successful */ +#define SECURITY_MANAGER_API_SUCCESS 0 + +/*! \brief indicating the socket between client and Security Manager has been failed */ +#define SECURITY_MANAGER_API_ERROR_SOCKET -1 + +/*! \brief indicating the request to Security Manager is malformed */ +#define SECURITY_MANAGER_API_ERROR_BAD_REQUEST -2 + +/*! \brief indicating the response from Security Manager is malformed */ +#define SECURITY_MANAGER_API_ERROR_BAD_RESPONSE -3 + +/*! \brief indicating the requested service does not exist */ +#define SECURITY_MANAGER_API_ERROR_NO_SUCH_SERVICE -4 + +/*! \brief indicating requesting object is not exist */ +#define SECURITY_MANAGER_API_ERROR_NO_SUCH_OBJECT -6 + +/*! \brief indicating the authentication between client and server has been failed */ +#define SECURITY_MANAGER_API_ERROR_AUTHENTICATION_FAILED -7 + +/*! \brief indicating the API's input parameter is malformed */ +#define SECURITY_MANAGER_API_ERROR_INPUT_PARAM -8 + +/*! \brief indicating the output buffer size which is passed as parameter is too small */ +#define SECURITY_MANAGER_API_ERROR_BUFFER_TOO_SMALL -9 + +/*! \brief indicating system is running out of memory state */ +#define SECURITY_MANAGER_API_ERROR_OUT_OF_MEMORY -10 + +/*! \brief indicating the access has been denied by Security Manager */ +#define SECURITY_MANAGER_API_ERROR_ACCESS_DENIED -11 + +/*! \brief indicating Security Manager has been failed for some reason */ +#define SECURITY_MANAGER_API_ERROR_SERVER_ERROR -12 + +/*! \brief indicating getting smack label from socket failed */ +#define SECURITY_MANAGER_API_ERROR_GETTING_SOCKET_LABEL_FAILED -21 + +/*! \brief indicating getting smack label from file failed */ +#define SECURITY_MANAGER_API_ERROR_GETTING_FILE_LABEL_FAILED -22 + +/*! \brief indicating setting smack label for file failed */ +#define SECURITY_MANAGER_API_ERROR_SETTING_FILE_LABEL_FAILED -23 + +/*! \brief indicating file already exists */ +#define SECURITY_MANAGER_API_ERROR_FILE_EXIST -24 + +/*! \brief indicating file does not exist */ +#define SECURITY_MANAGER_API_ERROR_FILE_NOT_EXIST -25 + +/*! \brief indicating file open error */ +#define SECURITY_MANAGER_API_ERROR_FILE_OPEN_FAILED -26 + +/*! \brief indicating file creation error */ +#define SECURITY_MANAGER_API_ERROR_FILE_CREATION_FAILED -27 + +/*! \brief indicating file deletion error */ +#define SECURITY_MANAGER_API_ERROR_FILE_DELETION_FAILED -28 + +/*! \brief indicating the error with unknown reason */ +#define SECURITY_MANAGER_API_ERROR_UNKNOWN -255 +/** @}*/ + + struct app_inst_req { std::string appId; std::string pkgId; diff --git a/src/include/security-manager.h b/src/include/security-manager.h index 760ee3f..81ddade 100644 --- a/src/include/security-manager.h +++ b/src/include/security-manager.h @@ -29,75 +29,6 @@ #include -/** - * \name Return Codes - * exported by the foundation API. - * result codes begin with the start error code and extend into negative direction. - * @{ -*/ -#define SECURITY_MANAGER_API_SUCCESS 0 -/*! \brief indicating the result of the one specific API is successful */ -#define SECURITY_MANAGER_API_ERROR_SOCKET -1 - -/*! \brief indicating the socket between client and Security Manager has been failed */ -#define SECURITY_MANAGER_API_ERROR_BAD_REQUEST -2 - -/*! \brief indicating the response from Security Manager is malformed */ -#define SECURITY_MANAGER_API_ERROR_BAD_RESPONSE -3 - -/*! \brief indicating the requested service does not exist */ -#define SECURITY_MANAGER_API_ERROR_NO_SUCH_SERVICE -4 - -/*! \brief indicating requesting object is not exist */ -#define SECURITY_MANAGER_API_ERROR_NO_SUCH_OBJECT -6 - -/*! \brief indicating the authentication between client and server has been failed */ -#define SECURITY_MANAGER_API_ERROR_AUTHENTICATION_FAILED -7 - -/*! \brief indicating the API's input parameter is malformed */ -#define SECURITY_MANAGER_API_ERROR_INPUT_PARAM -8 - -/*! \brief indicating the output buffer size which is passed as parameter is too small */ -#define SECURITY_MANAGER_API_ERROR_BUFFER_TOO_SMALL -9 - -/*! \brief indicating system is running out of memory state */ -#define SECURITY_MANAGER_API_ERROR_OUT_OF_MEMORY -10 - -/*! \brief indicating the access has been denied by Security Manager */ -#define SECURITY_MANAGER_API_ERROR_ACCESS_DENIED -11 - -/*! \brief indicating Security Manager has been failed for some reason */ -#define SECURITY_MANAGER_API_ERROR_SERVER_ERROR -12 - -/*! \brief indicating getting smack label from socket failed */ -#define SECURITY_MANAGER_API_ERROR_GETTING_SOCKET_LABEL_FAILED -21 - -/*! \brief indicating getting smack label from file failed */ -#define SECURITY_MANAGER_API_ERROR_GETTING_FILE_LABEL_FAILED -22 - -/*! \brief indicating setting smack label for file failed */ -#define SECURITY_MANAGER_API_ERROR_SETTING_FILE_LABEL_FAILED -23 - -/*! \brief indicating file already exists */ -#define SECURITY_MANAGER_API_ERROR_FILE_EXIST -24 - -/*! \brief indicating file does not exist */ -#define SECURITY_MANAGER_API_ERROR_FILE_NOT_EXIST -25 - -/*! \brief indicating file open error */ -#define SECURITY_MANAGER_API_ERROR_FILE_OPEN_FAILED -26 - -/*! \brief indicating file creation error */ -#define SECURITY_MANAGER_API_ERROR_FILE_CREATION_FAILED -27 - -/*! \brief indicating file deletion error */ -#define SECURITY_MANAGER_API_ERROR_FILE_DELETION_FAILED -28 - -/*! \brief indicating the error with unknown reason */ -#define SECURITY_MANAGER_API_ERROR_UNKNOWN -255 -/** @}*/ - - #ifdef __cplusplus extern "C" { #endif -- 2.7.4 From be9f67e0554d4cf7f7008dd1731d7dcef2341424 Mon Sep 17 00:00:00 2001 From: Stephane Desneux Date: Fri, 25 Jul 2014 13:21:10 +0200 Subject: [PATCH 08/16] Add missing gcc option -pthread to build correctly Bug-Tizen: TC-1446 Change-Id: I5d2c560a01f867722c3918daa912048f098e3ab6 Signed-off-by: Stephane Desneux --- src/server/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/server/CMakeLists.txt b/src/server/CMakeLists.txt index 89b21a6..0abbfda 100644 --- a/src/server/CMakeLists.txt +++ b/src/server/CMakeLists.txt @@ -53,7 +53,9 @@ ADD_EXECUTABLE(${TARGET_SERVER} ${SERVER_SOURCES}) SET_TARGET_PROPERTIES(${TARGET_SERVER} PROPERTIES - COMPILE_FLAGS "-D_GNU_SOURCE -fvisibility=hidden") + COMPILE_FLAGS "-D_GNU_SOURCE -fvisibility=hidden -pthread" + LINK_FLAGS "-pthread" +) TARGET_LINK_LIBRARIES(${TARGET_SERVER} ${TARGET_COMMON} -- 2.7.4 From d46fdd154e14438a344490bc2b3732a1a7ec80c6 Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Fri, 1 Aug 2014 14:17:38 +0200 Subject: [PATCH 09/16] Change pthread flag settings in CMake to a more generic construct Modify the previous commit using proper CMake module for thread library support. Change-Id: I1eaf2f8bc3b6ac542e5c81deeba14f68e47af381 Signed-off-by: Rafal Krypa --- src/server/CMakeLists.txt | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/server/CMakeLists.txt b/src/server/CMakeLists.txt index 0abbfda..3da8ddf 100644 --- a/src/server/CMakeLists.txt +++ b/src/server/CMakeLists.txt @@ -10,10 +10,8 @@ PKG_CHECK_MODULES(SERVER_DEP cynara-admin ) -FIND_PACKAGE( - Boost - REQUIRED - ) +FIND_PACKAGE(Boost REQUIRED) +FIND_PACKAGE(Threads REQUIRED) INCLUDE_DIRECTORIES(SYSTEM ${SERVER_DEP_INCLUDE_DIRS} @@ -53,12 +51,11 @@ ADD_EXECUTABLE(${TARGET_SERVER} ${SERVER_SOURCES}) SET_TARGET_PROPERTIES(${TARGET_SERVER} PROPERTIES - COMPILE_FLAGS "-D_GNU_SOURCE -fvisibility=hidden -pthread" - LINK_FLAGS "-pthread" -) + COMPILE_FLAGS "-D_GNU_SOURCE -fvisibility=hidden") TARGET_LINK_LIBRARIES(${TARGET_SERVER} ${TARGET_COMMON} + ${CMAKE_THREAD_LIBS_INIT} ${SERVER_DEP_LIBRARIES} ) -- 2.7.4 From 0055de15bda0f8f559a7dac9d7fac677a5280b8a Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Fri, 1 Aug 2014 14:13:41 +0200 Subject: [PATCH 10/16] Provide move constructor instead of copy constructor for CynaraAdminPolicy The class stores pointers and owns the memory they point to. Memory is allocated in constructor and freed in destructor. But copying these pointers between objects causes double free in destructor. The poiners should not be copied, only moved. Now CynaraAdminPolicy will provide custom move constructor. It will be used by default, since default copy constructor is now deleted. Change-Id: If6c49184318c54574caff8af74b336dd1c8ddd2f Signed-off-by: Rafal Krypa --- src/server/service/cynara.cpp | 16 ++++++++++++++++ src/server/service/include/cynara.h | 6 ++++++ 2 files changed, 22 insertions(+) diff --git a/src/server/service/cynara.cpp b/src/server/service/cynara.cpp index 2cf3f19..4925984 100644 --- a/src/server/service/cynara.cpp +++ b/src/server/service/cynara.cpp @@ -76,6 +76,22 @@ CynaraAdminPolicy::CynaraAdminPolicy(const std::string &client, const std::strin } } +CynaraAdminPolicy::CynaraAdminPolicy(CynaraAdminPolicy &&that) +{ + bucket = that.bucket; + client = that.client; + user = that.user; + privilege = that.privilege; + result_extra = that.result_extra; + result = that.result; + + that.bucket = nullptr; + that.client = nullptr; + that.user = nullptr; + that.privilege = nullptr; + that.result_extra = nullptr; +} + CynaraAdminPolicy::~CynaraAdminPolicy() { free(this->bucket); diff --git a/src/server/service/include/cynara.h b/src/server/service/include/cynara.h index b9ef6ed..df28fdc 100644 --- a/src/server/service/include/cynara.h +++ b/src/server/service/include/cynara.h @@ -57,6 +57,12 @@ struct CynaraAdminPolicy : cynara_admin_policy const std::string &privilege, const std::string &goToBucket, const std::string &bucket = std::string(CYNARA_ADMIN_DEFAULT_BUCKET)); + /* Don't provide copy constructor, it would cause pointer trouble. */ + CynaraAdminPolicy(const CynaraAdminPolicy &that) = delete; + + /* Move constructor is the way to go. */ + CynaraAdminPolicy(CynaraAdminPolicy &&that); + ~CynaraAdminPolicy(); }; -- 2.7.4 From eb01bad83e70424dc8196bdbbd969d815dcee035 Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Mon, 14 Jul 2014 00:21:26 +0200 Subject: [PATCH 11/16] Set Cynara policies during application installation and uninstallation Applied policies will have a wildcard in "user" field. Security-manager will handle app installation per user soon, so this will also be changed. Change-Id: I41606fb94b7385426debbcf47a57ba1593dbfc5a Signed-off-by: Rafal Krypa Signed-off-by: Jan Cybulski --- src/server/db/include/privilege_db.h | 2 +- src/server/service/cynara.cpp | 68 +++++++++++++++++++++++++++++++++++- src/server/service/include/cynara.h | 22 ++++++++++++ src/server/service/installer.cpp | 25 +++++++++++-- 4 files changed, 113 insertions(+), 4 deletions(-) diff --git a/src/server/db/include/privilege_db.h b/src/server/db/include/privilege_db.h index 7890022..b508576 100644 --- a/src/server/db/include/privilege_db.h +++ b/src/server/db/include/privilege_db.h @@ -61,7 +61,7 @@ class PrivilegeDb { private: SecurityManager::DB::SqlConnection *mSqlConnection; const std::map Queries = { - { QueryType::EGetPkgPrivileges, "SELECT privilege_name FROM app_privilege_view WHERE pkg_name=? AND uid=?"}, + { QueryType::EGetPkgPrivileges, "SELECT DISTINCT privilege_name FROM app_privilege_view WHERE pkg_name=? AND uid=? ORDER BY privilege_name"}, { QueryType::EAddApplication, "INSERT INTO app_pkg_view (app_name, pkg_name, uid) VALUES (?, ?, ?)" }, { QueryType::ERemoveApplication, "DELETE FROM app_pkg_view WHERE app_name=? AND uid=?" }, { QueryType::EAddAppPrivileges, "INSERT INTO app_privilege_view (app_name, uid, privilege_name) VALUES (?, ?, ?)" }, diff --git a/src/server/service/cynara.cpp b/src/server/service/cynara.cpp index 4925984..ab9dbf3 100644 --- a/src/server/service/cynara.cpp +++ b/src/server/service/cynara.cpp @@ -26,6 +26,8 @@ #include #include "cynara.h" +#include + namespace SecurityManager { @@ -133,8 +135,17 @@ void CynaraAdmin::SetPolicies(const std::vector &policies) { std::vector pp_policies(policies.size() + 1); - for (std::size_t i = 0; i < policies.size(); ++i) + LogDebug("Sending " << policies.size() << " policies to Cynara"); + for (std::size_t i = 0; i < policies.size(); ++i) { pp_policies[i] = static_cast(&policies[i]); + LogDebug("policies[" << i << "] = {" << + ".bucket = " << pp_policies[i]->bucket << ", " << + ".client = " << pp_policies[i]->client << ", " << + ".user = " << pp_policies[i]->user << ", " << + ".privilege = " << pp_policies[i]->privilege << ", " << + ".result = " << pp_policies[i]->result << ", " << + ".result_extra = " << pp_policies[i]->result_extra << "}"); + } pp_policies[policies.size()] = nullptr; @@ -143,4 +154,59 @@ void CynaraAdmin::SetPolicies(const std::vector &policies) "Error while updating Cynara policy."); } +void CynaraAdmin::UpdatePackagePolicy( + const std::string &pkg, + const std::string &user, + const std::vector &oldPrivileges, + const std::vector &newPrivileges) +{ + CynaraAdmin cynaraAdmin; + std::vector policies; + + // Perform sort-merge join on oldPrivileges and newPrivileges. + // Assume that they are already sorted and without duplicates. + auto oldIter = oldPrivileges.begin(); + auto newIter = newPrivileges.begin(); + + while (oldIter != oldPrivileges.end() && newIter != newPrivileges.end()) { + int compare = oldIter->compare(*newIter); + if (compare == 0) { + LogDebug("(user = " << user << " pkg = " << pkg << ") " << + "keeping privilege " << *newIter); + ++oldIter; + ++newIter; + continue; + } else if (compare < 0) { + LogDebug("(user = " << user << " pkg = " << pkg << ") " << + "removing privilege " << *oldIter); + policies.push_back(CynaraAdminPolicy(pkg, user, *oldIter, + CynaraAdminPolicy::Operation::Delete)); + ++oldIter; + } else { + LogDebug("(user = " << user << " pkg = " << pkg << ") " << + "adding privilege " << *newIter); + policies.push_back(CynaraAdminPolicy(pkg, user, *newIter, + CynaraAdminPolicy::Operation::Allow)); + ++newIter; + } + } + + for (; oldIter != oldPrivileges.end(); ++oldIter) { + LogDebug("(user = " << user << " pkg = " << pkg << ") " << + "removing privilege " << *oldIter); + policies.push_back(CynaraAdminPolicy(pkg, user, *oldIter, + CynaraAdminPolicy::Operation::Delete)); + } + + for (; newIter != newPrivileges.end(); ++newIter) { + LogDebug("(user = " << user << " pkg = " << pkg << ") " << + "adding privilege " << *newIter); + policies.push_back(CynaraAdminPolicy(pkg, user, *newIter, + CynaraAdminPolicy::Operation::Allow)); + } + + cynaraAdmin.SetPolicies(policies); +} + + } // namespace SecurityManager diff --git a/src/server/service/include/cynara.h b/src/server/service/include/cynara.h index df28fdc..0daa1e5 100644 --- a/src/server/service/include/cynara.h +++ b/src/server/service/include/cynara.h @@ -80,6 +80,28 @@ public: */ void SetPolicies(const std::vector &policies); + /** + * Update Cynara policies for the package and the user, using two vectors + * of privileges: privileges set before (and already enabled in Cynara) + * and new privileges, to be set in Cynara. + * Difference will be calculated, removing old unneeded privileges and + * adding new, previously not enabled privileges. + * Caller must have permission to access Cynara administrative socket. + * + * @param pkg package identifier + * @param user user identifier + * @param oldPrivileges previously enabled privileges for the package. + * Must be sorted and without duplicates. + * @param newPrivileges currently enabled privileges for the package. + * Must be sorted and without duplicates. + * + * TODO: drop oldPrivileges argument and get them directly from Cynara. + * Appropriate Cynara interface is needed first. + */ + static void UpdatePackagePolicy(const std::string &pkg, const std::string &user, + const std::vector &oldPrivileges, + const std::vector &newPrivileges); + private: struct cynara_admin *m_CynaraAdmin; }; diff --git a/src/server/service/installer.cpp b/src/server/service/installer.cpp index 286fce6..0e5ec01 100644 --- a/src/server/service/installer.cpp +++ b/src/server/service/installer.cpp @@ -40,6 +40,7 @@ #include "smack-rules.h" #include "smack-labels.h" #include "privilege_db.h" +#include "cynara.h" namespace SecurityManager { @@ -286,13 +287,23 @@ bool InstallerService::processAppInstall(MessageBuffer &buffer, MessageBuffer &s m_privilegeDb.AddApplication(req.appId, req.pkgId, uid, pkgIdIsNew); m_privilegeDb.UpdateAppPrivileges(req.appId, uid, req.privileges); m_privilegeDb.GetPkgPrivileges(req.pkgId, uid, newPkgPrivileges); - // TODO: configure Cynara rules based on oldPkgPrivileges and newPkgPrivileges + CynaraAdmin::UpdatePackagePolicy(req.pkgId, + CYNARA_ADMIN_WILDCARD, /* TODO: pass proper user identifier */ + oldPkgPrivileges, newPkgPrivileges); m_privilegeDb.CommitTransaction(); LogDebug("Application installation commited to database"); } catch (const PrivilegeDb::Exception::InternalError &e) { m_privilegeDb.RollbackTransaction(); LogError("Error while saving application info to database: " << e.DumpToString()); goto error_label; + } catch (const CynaraException::Base &e) { + m_privilegeDb.RollbackTransaction(); + LogError("Error while setting Cynara rules for application: " << e.DumpToString()); + goto error_label; + } catch (const std::bad_alloc &e) { + m_privilegeDb.RollbackTransaction(); + LogError("Memory allocation while setting Cynara rules for application: " << e.what()); + goto error_label; } // register paths @@ -382,7 +393,9 @@ bool InstallerService::processAppUninstall(MessageBuffer &buffer, MessageBuffer m_privilegeDb.UpdateAppPrivileges(appId, uid, std::vector()); m_privilegeDb.RemoveApplication(appId, uid, removePkg); m_privilegeDb.GetPkgPrivileges(pkgId, uid, newPkgPrivileges); - // TODO: configure Cynara rules based on oldPkgPrivileges and newPkgPrivileges + CynaraAdmin::UpdatePackagePolicy(pkgId, + CYNARA_ADMIN_WILDCARD, /* TODO: pass proper user identifier */ + oldPkgPrivileges, newPkgPrivileges); m_privilegeDb.CommitTransaction(); LogDebug("Application uninstallation commited to database"); } @@ -390,6 +403,14 @@ bool InstallerService::processAppUninstall(MessageBuffer &buffer, MessageBuffer m_privilegeDb.RollbackTransaction(); LogError("Error while removing application info from database: " << e.DumpToString()); goto error_label; + } catch (const CynaraException::Base &e) { + m_privilegeDb.RollbackTransaction(); + LogError("Error while setting Cynara rules for application: " << e.DumpToString()); + goto error_label; + } catch (const std::bad_alloc &e) { + m_privilegeDb.RollbackTransaction(); + LogError("Memory allocation while setting Cynara rules for application: " << e.what()); + goto error_label; } if (appExists) { -- 2.7.4 From eb9170d7a736830cc100243a3bfec2bb0036de60 Mon Sep 17 00:00:00 2001 From: Sebastian Grabowski Date: Thu, 24 Jul 2014 12:14:26 +0200 Subject: [PATCH 12/16] Changed CYNARA_ADMIN_WILDCARD to proper uid string. Change-Id: Ic4e9b4d26c3c41a983a4db61bbd557c84ff7c542 Signed-off-by: Sebastian Grabowski --- src/server/service/installer.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/server/service/installer.cpp b/src/server/service/installer.cpp index 0e5ec01..54619f4 100644 --- a/src/server/service/installer.cpp +++ b/src/server/service/installer.cpp @@ -281,15 +281,19 @@ bool InstallerService::processAppInstall(MessageBuffer &buffer, MessageBuffer &s try { std::vector oldPkgPrivileges, newPkgPrivileges; + std::string uidstr = uid ? std::to_string(static_cast(uid)) + : CYNARA_ADMIN_WILDCARD; + + LogDebug("Install parameters: appId: " << req.appId << ", pkgId: " << req.pkgId + << ", uidstr " << uidstr << ", generated smack label: " << smackLabel); m_privilegeDb.BeginTransaction(); m_privilegeDb.GetPkgPrivileges(req.pkgId, uid, oldPkgPrivileges); m_privilegeDb.AddApplication(req.appId, req.pkgId, uid, pkgIdIsNew); m_privilegeDb.UpdateAppPrivileges(req.appId, uid, req.privileges); m_privilegeDb.GetPkgPrivileges(req.pkgId, uid, newPkgPrivileges); - CynaraAdmin::UpdatePackagePolicy(req.pkgId, - CYNARA_ADMIN_WILDCARD, /* TODO: pass proper user identifier */ - oldPkgPrivileges, newPkgPrivileges); + CynaraAdmin::UpdatePackagePolicy(req.pkgId, uidstr, oldPkgPrivileges, + newPkgPrivileges); m_privilegeDb.CommitTransaction(); LogDebug("Application installation commited to database"); } catch (const PrivilegeDb::Exception::InternalError &e) { @@ -384,18 +388,21 @@ bool InstallerService::processAppUninstall(MessageBuffer &buffer, MessageBuffer if (!generateAppLabel(pkgId, smackLabel)) { LogError("Cannot generate Smack label for package: " << pkgId); goto error_label; + } - LogDebug("Unnstall parameters: appId: " << appId << ", pkgId: " << pkgId - << ", generated smack label: " << smackLabel); + std::string uidstr = uid ? std::to_string(static_cast(uid)) + : CYNARA_ADMIN_WILDCARD; + + LogDebug("Uninstall parameters: appId: " << appId << ", pkgId: " << pkgId + << ", uidstr " << uidstr << ", generated smack label: " << smackLabel); m_privilegeDb.GetPkgPrivileges(pkgId, uid, oldPkgPrivileges); m_privilegeDb.UpdateAppPrivileges(appId, uid, std::vector()); m_privilegeDb.RemoveApplication(appId, uid, removePkg); m_privilegeDb.GetPkgPrivileges(pkgId, uid, newPkgPrivileges); - CynaraAdmin::UpdatePackagePolicy(pkgId, - CYNARA_ADMIN_WILDCARD, /* TODO: pass proper user identifier */ - oldPkgPrivileges, newPkgPrivileges); + CynaraAdmin::UpdatePackagePolicy(pkgId, uidstr, oldPkgPrivileges, + newPkgPrivileges); m_privilegeDb.CommitTransaction(); LogDebug("Application uninstallation commited to database"); } -- 2.7.4 From 22edb0bde71d451577eb09332ae075e90e17d299 Mon Sep 17 00:00:00 2001 From: Jan Cybulski Date: Thu, 7 Aug 2014 15:32:45 +0200 Subject: [PATCH 13/16] Drop libprivilege-control Change-Id: Ifff71e53ad15d644d50b978bcb979bb492c09f92 Signed-off-by: Jan Cybulski --- packaging/security-manager.spec | 1 - src/server/CMakeLists.txt | 1 - src/server/service/installer.cpp | 71 +--------------------------------------- 3 files changed, 1 insertion(+), 72 deletions(-) diff --git a/packaging/security-manager.spec b/packaging/security-manager.spec index bf84cfc..4110dda 100644 --- a/packaging/security-manager.spec +++ b/packaging/security-manager.spec @@ -14,7 +14,6 @@ BuildRequires: zip BuildRequires: libattr-devel BuildRequires: libcap-devel BuildRequires: pkgconfig(libsmack) -BuildRequires: pkgconfig(libprivilege-control) BuildRequires: pkgconfig(libsystemd-daemon) BuildRequires: pkgconfig(libsystemd-journal) BuildRequires: pkgconfig(libtzplatform-config) diff --git a/src/server/CMakeLists.txt b/src/server/CMakeLists.txt index 3da8ddf..65cc36a 100644 --- a/src/server/CMakeLists.txt +++ b/src/server/CMakeLists.txt @@ -2,7 +2,6 @@ PKG_CHECK_MODULES(SERVER_DEP REQUIRED libcap libsmack - libprivilege-control libsystemd-daemon libtzplatform-config sqlite3 diff --git a/src/server/service/installer.cpp b/src/server/service/installer.cpp index 54619f4..4caf454 100644 --- a/src/server/service/installer.cpp +++ b/src/server/service/installer.cpp @@ -26,7 +26,6 @@ #include #include -#include #include #include #include @@ -255,30 +254,6 @@ bool InstallerService::processAppInstall(MessageBuffer &buffer, MessageBuffer &s } pp_permissions[req.privileges.size()] = nullptr; - // start database transaction - int result = perm_begin(); - if (PC_OPERATION_SUCCESS != result) { - // libprivilege is locked - LogError("perm_begin() returned " << result); - Serialization::Serialize(send, SECURITY_MANAGER_API_ERROR_SERVER_ERROR); - return false; - } - - result = perm_app_install(smackLabel.c_str()); - if (PC_OPERATION_SUCCESS != result) { - // libprivilege error - LogError("perm_app_install() returned " << result); - goto error_label; - } - - result = perm_app_enable_permissions(smackLabel.c_str(), APP_TYPE_WGT, - pp_permissions.get(), true); - if (PC_OPERATION_SUCCESS != result) { - // libprivilege error - LogError("perm_app_enable_permissions() returned " << result); - goto error_label; - } - try { std::vector oldPkgPrivileges, newPkgPrivileges; std::string uidstr = uid ? std::to_string(static_cast(uid)) @@ -314,7 +289,7 @@ bool InstallerService::processAppInstall(MessageBuffer &buffer, MessageBuffer &s for (const auto &appPath : req.appPaths) { const std::string &path = appPath.first; app_install_path_type pathType = static_cast(appPath.second); - result = setupPath(req.pkgId, path, pathType); + int result = setupPath(req.pkgId, path, pathType); if (!result) { LogError("setupPath() failed"); @@ -330,28 +305,11 @@ bool InstallerService::processAppInstall(MessageBuffer &buffer, MessageBuffer &s } } - // finish database transaction - result = perm_end(); - if (PC_OPERATION_SUCCESS != result) { - LogError("perm_end() returned " << result); - if (pkgIdIsNew) - if (!SmackRules::uninstallPackageRules(req.pkgId)) - LogWarning("Failed to rollback package-specific smack rules"); - - // error in libprivilege-control - Serialization::Serialize(send, SECURITY_MANAGER_API_ERROR_SERVER_ERROR); - return false; - } - // success Serialization::Serialize(send, SECURITY_MANAGER_API_SUCCESS); return true; error_label: - // rollback failed transaction before exiting - result = perm_rollback(); - if (PC_OPERATION_SUCCESS != result) - LogError("perm_rollback() returned " << result); Serialization::Serialize(send, SECURITY_MANAGER_API_ERROR_SERVER_ERROR); return false; } @@ -367,14 +325,6 @@ bool InstallerService::processAppUninstall(MessageBuffer &buffer, MessageBuffer Deserialization::Deserialize(buffer, appId); - int result = perm_begin(); - if (PC_OPERATION_SUCCESS != result) { - // libprivilege is locked - LogError("perm_begin() returned " << result); - Serialization::Serialize(send, SECURITY_MANAGER_API_ERROR_SERVER_ERROR); - return false; - } - try { std::vector oldPkgPrivileges, newPkgPrivileges; @@ -421,12 +371,6 @@ bool InstallerService::processAppUninstall(MessageBuffer &buffer, MessageBuffer } if (appExists) { - result = perm_app_uninstall(smackLabel.c_str()); - if (PC_OPERATION_SUCCESS != result) { - // error in libprivilege-control - LogError("perm_app_uninstall() returned " << result); - goto error_label; - } if (removePkg) { LogDebug("Removing Smack rules for deleted pkgId " << pkgId); @@ -437,24 +381,11 @@ bool InstallerService::processAppUninstall(MessageBuffer &buffer, MessageBuffer } } - // finish database transaction - result = perm_end(); - if (PC_OPERATION_SUCCESS != result) { - // error in libprivilege-control - LogError("perm_end() returned " << result); - Serialization::Serialize(send, SECURITY_MANAGER_API_ERROR_SERVER_ERROR); - return false; - } - // success Serialization::Serialize(send, SECURITY_MANAGER_API_SUCCESS); return true; error_label: - // rollback failed transaction before exiting - result = perm_rollback(); - if (PC_OPERATION_SUCCESS != result) - LogError("perm_rollback() returned " << result); Serialization::Serialize(send, SECURITY_MANAGER_API_ERROR_SERVER_ERROR); return false; } -- 2.7.4 From afa17b7a57716977f8c1c2bc1168a72eaa18a7f2 Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Wed, 27 Aug 2014 18:11:53 +0200 Subject: [PATCH 14/16] Fix checking whether application path is inside user's home directory. Internal function installRequestAuthCheck() making this check contained few bugs. It didn't canonicalize the home directory. It simply checked for substring instead of subdirectory ("/home/useruser" shouldn't be considered as subdirectory of "/home/user"). It relied on PATH_MAX for realpath() calls, which is broken by design according to function manual. All of the above issues are now corrected. Change-Id: I446c50e642b38ecbd1b4997ec5e6f7c9b5032291 Signed-off-by: Rafal Krypa --- src/server/service/installer.cpp | 60 ++++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/src/server/service/installer.cpp b/src/server/service/installer.cpp index 4caf454..3e9f124 100644 --- a/src/server/service/installer.cpp +++ b/src/server/service/installer.cpp @@ -178,40 +178,58 @@ bool InstallerService::processOne(const ConnectionID &conn, MessageBuffer &buffe return retval; } +static inline bool isSubDir(const char *parent, const char *subdir) +{ + while (*parent && *subdir) + if (*parent++ != *subdir++) + return false; + + return (*subdir == '/'); +} + static inline bool installRequestAuthCheck(const app_inst_req &req, uid_t uid) { + if (uid == 0) + return true; + struct passwd *pwd; - char buffer[PATH_MAX]; do { errno = 0; pwd = getpwuid(uid); if (!pwd && errno != EINTR) { - LogError("getpwuid failed with '" << uid << "' as paramter: " << strerror(errno)); + LogError("getpwuid failed with '" << uid + << "' as paramter: " << strerror(errno)); return false; } } while (!pwd); - for (const auto &appPath : req.appPaths) { + std::unique_ptr> home( + realpath(pwd->pw_dir, NULL), free); + if (!home.get()) { + LogError("realpath failed with '" << pwd->pw_dir + << "' as paramter: " << strerror(errno)); + return false; + } - if (uid != 0) { - char *real_path = realpath(appPath.first.c_str(), buffer); - if (!real_path) { - LogError("realpath failed with '" << appPath.first.c_str() - << "' as paramter: " << strerror(errno)); - return false; - } - LogDebug("Requested path is '" << appPath.first.c_str() - << "'. User's HOME is '" << pwd->pw_dir << "'"); - if (strncmp(pwd->pw_dir, real_path, strlen(pwd->pw_dir))!=0) { - LogWarning("User's apps may have registered folders only in user's home dir"); - return false; - } + for (const auto &appPath : req.appPaths) { + std::unique_ptr> real_path( + realpath(appPath.first.c_str(), NULL), free); + if (!real_path.get()) { + LogError("realpath failed with '" << appPath.first.c_str() + << "' as paramter: " << strerror(errno)); + return false; + } + LogDebug("Requested path is '" << appPath.first.c_str() + << "'. User's HOME is '" << pwd->pw_dir << "'"); + if (!isSubDir(home.get(), real_path.get())) { + LogWarning("User's apps may have registered folders only in user's home dir"); + return false; + } - app_install_path_type pathType = static_cast(appPath.second); - if (pathType == SECURITY_MANAGER_PATH_PUBLIC) { - LogWarning("Only root can register SECURITY_MANAGER_PATH_PUBLIC path"); - return false; - } + app_install_path_type pathType = static_cast(appPath.second); + if (pathType == SECURITY_MANAGER_PATH_PUBLIC) { + LogWarning("Only root can register SECURITY_MANAGER_PATH_PUBLIC path"); + return false; } } return true; -- 2.7.4 From 8af9d44cf707942fd107a48d91ee1e30b94e37d7 Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Fri, 29 Aug 2014 18:34:49 +0200 Subject: [PATCH 15/16] Refactoring: there will be only one service Security-manager started with installer service implementation. It was created in a way supporting future creation of other services, working in separate threads and listening on separate sockets. Such design is however not planned for this project. The installer service recently began to implement methods not related to installation, which begged for some refactoring. Hereby the installer service is renamed as just "service". There will be a single socket and single service for all security-manager functions. Change-Id: I40e939ded1b0e20c4e92c86738fb62ea4acd4a50 Signed-off-by: Rafal Krypa --- packaging/security-manager.spec | 6 ++-- src/client/client-security-manager.cpp | 6 ++-- src/common/include/protocols.h | 2 +- src/common/protocols.cpp | 5 ++- src/server/CMakeLists.txt | 2 +- src/server/main/server-main.cpp | 4 +-- .../service/include/{installer.h => service.h} | 18 +++++------ src/server/service/{installer.cpp => service.cpp} | 36 +++++++++++----------- systemd/CMakeLists.txt | 2 +- systemd/security-manager.service.in | 2 +- ...er-installer.socket => security-manager.socket} | 2 +- 11 files changed, 42 insertions(+), 43 deletions(-) rename src/server/service/include/{installer.h => service.h} (90%) rename src/server/service/{installer.cpp => service.cpp} (92%) rename systemd/{security-manager-installer.socket => security-manager.socket} (77%) diff --git a/packaging/security-manager.spec b/packaging/security-manager.spec index 4110dda..c4552c9 100644 --- a/packaging/security-manager.spec +++ b/packaging/security-manager.spec @@ -78,7 +78,7 @@ cp app-rules-template.smack %{buildroot}/%{TZ_SYS_SMACK} mkdir -p %{buildroot}/%{_unitdir}/multi-user.target.wants mkdir -p %{buildroot}/%{_unitdir}/sockets.target.wants ln -s ../security-manager.service %{buildroot}/%{_unitdir}/multi-user.target.wants/security-manager.service -ln -s ../security-manager-installer.socket %{buildroot}/%{_unitdir}/sockets.target.wants/security-manager-installer.socket +ln -s ../security-manager.socket %{buildroot}/%{_unitdir}/sockets.target.wants/security-manager.socket %clean rm -rf %{buildroot} @@ -123,8 +123,8 @@ fi %attr(-,root,root) %{_unitdir}/multi-user.target.wants/security-manager.service %attr(-,root,root) %{_unitdir}/security-manager.service %attr(-,root,root) %{_unitdir}/security-manager.target -%attr(-,root,root) %{_unitdir}/sockets.target.wants/security-manager-installer.socket -%attr(-,root,root) %{_unitdir}/security-manager-installer.socket +%attr(-,root,root) %{_unitdir}/sockets.target.wants/security-manager.socket +%attr(-,root,root) %{_unitdir}/security-manager.socket %attr(-,root,root) %{TZ_SYS_SMACK}/app-rules-template.smack %config(noreplace) %attr(0600,root,root) %{TZ_SYS_DB}/.security-manager.db %config(noreplace) %attr(0600,root,root) %{TZ_SYS_DB}/.security-manager.db-journal diff --git a/src/client/client-security-manager.cpp b/src/client/client-security-manager.cpp index 5f84f16..8ddd408 100644 --- a/src/client/client-security-manager.cpp +++ b/src/client/client-security-manager.cpp @@ -128,7 +128,7 @@ int security_manager_app_install(const app_inst_req *p_req) Serialization::Serialize(send, p_req->appPaths); //send buffer to server - int retval = sendToServer(SERVICE_SOCKET_INSTALLER, send.Pop(), recv); + int retval = sendToServer(SERVICE_SOCKET, send.Pop(), recv); if (retval != SECURITY_MANAGER_API_SUCCESS) { LogError("Error in sendToServer. Error code: " << retval); return SECURITY_MANAGER_ERROR_UNKNOWN; @@ -166,7 +166,7 @@ int security_manager_app_uninstall(const app_inst_req *p_req) Serialization::Serialize(send, p_req->appId); //send buffer to server - int retval = sendToServer(SERVICE_SOCKET_INSTALLER, send.Pop(), recv); + int retval = sendToServer(SERVICE_SOCKET, send.Pop(), recv); if (retval != SECURITY_MANAGER_API_SUCCESS) { LogError("Error in sendToServer. Error code: " << retval); return SECURITY_MANAGER_ERROR_UNKNOWN; @@ -207,7 +207,7 @@ int security_manager_get_app_pkgid(char **pkg_id, const char *app_id) Serialization::Serialize(send, std::string(app_id)); //send buffer to server - int retval = sendToServer(SERVICE_SOCKET_INSTALLER, send.Pop(), recv); + int retval = sendToServer(SERVICE_SOCKET, send.Pop(), recv); if (retval != SECURITY_MANAGER_API_SUCCESS) { LogDebug("Error in sendToServer. Error code: " << retval); return SECURITY_MANAGER_ERROR_UNKNOWN; diff --git a/src/common/include/protocols.h b/src/common/include/protocols.h index fadc46c..c60f3b1 100644 --- a/src/common/include/protocols.h +++ b/src/common/include/protocols.h @@ -109,7 +109,7 @@ struct app_inst_req { namespace SecurityManager { -extern char const * const SERVICE_SOCKET_INSTALLER; +extern char const * const SERVICE_SOCKET; enum class SecurityModuleCall { diff --git a/src/common/protocols.cpp b/src/common/protocols.cpp index 59bf302..798b9d6 100644 --- a/src/common/protocols.cpp +++ b/src/common/protocols.cpp @@ -29,10 +29,9 @@ namespace SecurityManager { #define SOCKET_PATH_PREFIX "/run/" -#define SOCKET_PATH_PREFIX_SECURITY_MANAGER SOCKET_PATH_PREFIX "security-manager/" -char const * const SERVICE_SOCKET_INSTALLER = - SOCKET_PATH_PREFIX_SECURITY_MANAGER "security-manager-installer.socket"; +char const * const SERVICE_SOCKET = + SOCKET_PATH_PREFIX "security-manager.socket"; } // namespace SecurityManager diff --git a/src/server/CMakeLists.txt b/src/server/CMakeLists.txt index 65cc36a..8595111 100644 --- a/src/server/CMakeLists.txt +++ b/src/server/CMakeLists.txt @@ -35,9 +35,9 @@ SET(SERVER_SOURCES ${SERVER_PATH}/main/generic-socket-manager.cpp ${SERVER_PATH}/main/socket-manager.cpp ${SERVER_PATH}/main/server-main.cpp + ${SERVER_PATH}/service/service.cpp ${SERVER_PATH}/service/smack-rules.cpp ${SERVER_PATH}/service/smack-labels.cpp - ${SERVER_PATH}/service/installer.cpp ${SERVER_PATH}/service/cynara.cpp ${SERVER_PATH}/db/privilege_db.cpp ${DPL_PATH}/core/src/errno_string.cpp diff --git a/src/server/main/server-main.cpp b/src/server/main/server-main.cpp index 6f67efa..16f03ba 100644 --- a/src/server/main/server-main.cpp +++ b/src/server/main/server-main.cpp @@ -30,7 +30,7 @@ #include -#include +#include IMPLEMENT_SAFE_SINGLETON(SecurityManager::Log::LogSystem); @@ -78,7 +78,7 @@ int main(void) { LogInfo("Start!"); SecurityManager::SocketManager manager; - REGISTER_SOCKET_SERVICE(manager, SecurityManager::InstallerService); + REGISTER_SOCKET_SERVICE(manager, SecurityManager::Service); manager.MainLoop(); } diff --git a/src/server/service/include/installer.h b/src/server/service/include/service.h similarity index 90% rename from src/server/service/include/installer.h rename to src/server/service/include/service.h index 1a7caf2..13e40ee 100644 --- a/src/server/service/include/installer.h +++ b/src/server/service/include/service.h @@ -16,14 +16,14 @@ * limitations under the License */ /* - * @file installer.h + * @file service.h * @author Michal Witanowski * @author Rafal Krypa - * @brief Implementation of installer service + * @brief Implementation of security-manager service */ -#ifndef _SECURITY_MANAGER_INSTALLER_ -#define _SECURITY_MANAGER_INSTALLER_ +#ifndef _SECURITY_MANAGER_SERVICE_ +#define _SECURITY_MANAGER_SERVICE_ #include #include @@ -33,19 +33,19 @@ namespace SecurityManager { -class InstallerException +class ServiceException { public: DECLARE_EXCEPTION_TYPE(SecurityManager::Exception, Base) DECLARE_EXCEPTION_TYPE(Base, InvalidAction) }; -class InstallerService : +class Service : public SecurityManager::GenericSocketService, - public SecurityManager::ServiceThread + public SecurityManager::ServiceThread { public: - InstallerService(); + Service(); ServiceDescriptionVector GetServiceDescription(); DECLARE_THREAD_EVENT(AcceptEvent, accept) @@ -104,4 +104,4 @@ private: } // namespace SecurityManager -#endif // _SECURITY_MANAGER_INSTALLER_ +#endif // _SECURITY_MANAGER_SERVICE_ diff --git a/src/server/service/installer.cpp b/src/server/service/service.cpp similarity index 92% rename from src/server/service/installer.cpp rename to src/server/service/service.cpp index 3e9f124..664235f 100644 --- a/src/server/service/installer.cpp +++ b/src/server/service/service.cpp @@ -16,11 +16,11 @@ * limitations under the License */ /* - * @file installer.cpp + * @file service.cpp * @author Michal Witanowski * @author Jacek Bukarewicz * @author Rafal Krypa - * @brief Implementation of installer service. + * @brief Implementation of security-manager service. */ #include @@ -32,7 +32,7 @@ #include #include -#include "installer.h" +#include "service.h" #include "protocols.h" #include "security-manager.h" #include "smack-common.h" @@ -43,21 +43,21 @@ namespace SecurityManager { -const InterfaceID INSTALLER_IFACE = 0; +const InterfaceID IFACE = 1; -InstallerService::InstallerService() +Service::Service() { } -GenericSocketService::ServiceDescriptionVector InstallerService::GetServiceDescription() +GenericSocketService::ServiceDescriptionVector Service::GetServiceDescription() { return ServiceDescriptionVector { - {SERVICE_SOCKET_INSTALLER, "security-manager::installer", INSTALLER_IFACE}, + {SERVICE_SOCKET, "security-manager", IFACE}, }; } -void InstallerService::accept(const AcceptEvent &event) +void Service::accept(const AcceptEvent &event) { LogDebug("Accept event. ConnectionID.sock: " << event.connectionID.sock << " ConnectionID.counter: " << event.connectionID.counter << @@ -67,7 +67,7 @@ void InstallerService::accept(const AcceptEvent &event) info.interfaceID = event.interfaceID; } -void InstallerService::write(const WriteEvent &event) +void Service::write(const WriteEvent &event) { LogDebug("WriteEvent. ConnectionID: " << event.connectionID.sock << " Size: " << event.size << @@ -77,7 +77,7 @@ void InstallerService::write(const WriteEvent &event) m_serviceManager->Close(event.connectionID); } -void InstallerService::process(const ReadEvent &event) +void Service::process(const ReadEvent &event) { LogDebug("Read event for counter: " << event.connectionID.counter); auto &info = m_connectionInfoMap[event.connectionID.counter]; @@ -88,7 +88,7 @@ void InstallerService::process(const ReadEvent &event) while (processOne(event.connectionID, info.buffer, info.interfaceID)); } -void InstallerService::close(const CloseEvent &event) +void Service::close(const CloseEvent &event) { LogDebug("CloseEvent. ConnectionID: " << event.connectionID.sock); m_connectionInfoMap.erase(event.connectionID.counter); @@ -107,7 +107,7 @@ static bool getPeerUserID(int sock, uid_t *uid) { return false; } -bool InstallerService::processOne(const ConnectionID &conn, MessageBuffer &buffer, +bool Service::processOne(const ConnectionID &conn, MessageBuffer &buffer, InterfaceID interfaceID) { LogDebug("Iteration begin. Interface = " << interfaceID); @@ -128,7 +128,7 @@ bool InstallerService::processOne(const ConnectionID &conn, MessageBuffer &buffe return false; } - if (INSTALLER_IFACE == interfaceID) { + if (IFACE == interfaceID) { Try { // deserialize API call type int call_type_int; @@ -149,13 +149,13 @@ bool InstallerService::processOne(const ConnectionID &conn, MessageBuffer &buffe break; default: LogError("Invalid call: " << call_type_int); - Throw(InstallerException::InvalidAction); + Throw(ServiceException::InvalidAction); } // if we reach this point, the protocol is OK retval = true; } Catch (MessageBuffer::Exception::Base) { LogError("Broken protocol."); - } Catch (InstallerException::Base) { + } Catch (ServiceException::Base) { LogError("Broken protocol."); } catch (std::exception &e) { LogError("STD exception " << e.what()); @@ -235,7 +235,7 @@ static inline bool installRequestAuthCheck(const app_inst_req &req, uid_t uid) return true; } -bool InstallerService::processAppInstall(MessageBuffer &buffer, MessageBuffer &send, uid_t uid) +bool Service::processAppInstall(MessageBuffer &buffer, MessageBuffer &send, uid_t uid) { bool pkgIdIsNew = false; std::vector addedPermissions; @@ -332,7 +332,7 @@ error_label: return false; } -bool InstallerService::processAppUninstall(MessageBuffer &buffer, MessageBuffer &send, uid_t uid) +bool Service::processAppUninstall(MessageBuffer &buffer, MessageBuffer &send, uid_t uid) { // deserialize request data std::string appId; @@ -408,7 +408,7 @@ error_label: return false; } -bool InstallerService::processGetPkgId(MessageBuffer &buffer, MessageBuffer &send) +bool Service::processGetPkgId(MessageBuffer &buffer, MessageBuffer &send) { // deserialize request data std::string appId; diff --git a/systemd/CMakeLists.txt b/systemd/CMakeLists.txt index 9c2ac54..66af5dd 100644 --- a/systemd/CMakeLists.txt +++ b/systemd/CMakeLists.txt @@ -3,7 +3,7 @@ CONFIGURE_FILE(security-manager.service.in security-manager.service @ONLY) INSTALL(FILES security-manager.service security-manager.target - security-manager-installer.socket + security-manager.socket DESTINATION ${SYSTEMD_INSTALL_DIR} ) diff --git a/systemd/security-manager.service.in b/systemd/security-manager.service.in index df0b077..de58043 100644 --- a/systemd/security-manager.service.in +++ b/systemd/security-manager.service.in @@ -5,7 +5,7 @@ Description=Start the security manager Type=notify ExecStart=@BIN_INSTALL_DIR@/security-manager -Sockets=security-manager-installer.socket +Sockets=security-manager.socket [Install] WantedBy=multi-user.target diff --git a/systemd/security-manager-installer.socket b/systemd/security-manager.socket similarity index 77% rename from systemd/security-manager-installer.socket rename to systemd/security-manager.socket index e851c79..c0590ad 100644 --- a/systemd/security-manager-installer.socket +++ b/systemd/security-manager.socket @@ -1,5 +1,5 @@ [Socket] -ListenStream=/run/security-manager/security-manager-installer.socket +ListenStream=/run/security-manager.socket SocketMode=0777 SmackLabelIPIn=* SmackLabelIPOut=@ -- 2.7.4 From a017fe6aa8867e652b1b92a86357e296ba11b496 Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Tue, 2 Sep 2014 11:29:00 +0200 Subject: [PATCH 16/16] Fix Cynara policy setting, use Smack label as app identifier In Tizen Cynara policies should use application Smack label as application identifier. Services using Cynara will be based on that assumption. Previously security-manager incorrectly used pkgId as app identifier. Change-Id: I31f59e3c6a037cc3730936963b10a1e7bcb008e0 Signed-off-by: Rafal Krypa --- src/server/service/cynara.cpp | 20 ++++++++++---------- src/server/service/include/cynara.h | 4 ++-- src/server/service/service.cpp | 5 ++--- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/server/service/cynara.cpp b/src/server/service/cynara.cpp index ab9dbf3..6050444 100644 --- a/src/server/service/cynara.cpp +++ b/src/server/service/cynara.cpp @@ -155,7 +155,7 @@ void CynaraAdmin::SetPolicies(const std::vector &policies) } void CynaraAdmin::UpdatePackagePolicy( - const std::string &pkg, + const std::string &label, const std::string &user, const std::vector &oldPrivileges, const std::vector &newPrivileges) @@ -171,37 +171,37 @@ void CynaraAdmin::UpdatePackagePolicy( while (oldIter != oldPrivileges.end() && newIter != newPrivileges.end()) { int compare = oldIter->compare(*newIter); if (compare == 0) { - LogDebug("(user = " << user << " pkg = " << pkg << ") " << + LogDebug("(user = " << user << " label = " << label << ") " << "keeping privilege " << *newIter); ++oldIter; ++newIter; continue; } else if (compare < 0) { - LogDebug("(user = " << user << " pkg = " << pkg << ") " << + LogDebug("(user = " << user << " label = " << label << ") " << "removing privilege " << *oldIter); - policies.push_back(CynaraAdminPolicy(pkg, user, *oldIter, + policies.push_back(CynaraAdminPolicy(label, user, *oldIter, CynaraAdminPolicy::Operation::Delete)); ++oldIter; } else { - LogDebug("(user = " << user << " pkg = " << pkg << ") " << + LogDebug("(user = " << user << " label = " << label << ") " << "adding privilege " << *newIter); - policies.push_back(CynaraAdminPolicy(pkg, user, *newIter, + policies.push_back(CynaraAdminPolicy(label, user, *newIter, CynaraAdminPolicy::Operation::Allow)); ++newIter; } } for (; oldIter != oldPrivileges.end(); ++oldIter) { - LogDebug("(user = " << user << " pkg = " << pkg << ") " << + LogDebug("(user = " << user << " label = " << label << ") " << "removing privilege " << *oldIter); - policies.push_back(CynaraAdminPolicy(pkg, user, *oldIter, + policies.push_back(CynaraAdminPolicy(label, user, *oldIter, CynaraAdminPolicy::Operation::Delete)); } for (; newIter != newPrivileges.end(); ++newIter) { - LogDebug("(user = " << user << " pkg = " << pkg << ") " << + LogDebug("(user = " << user << " label = " << label << ") " << "adding privilege " << *newIter); - policies.push_back(CynaraAdminPolicy(pkg, user, *newIter, + policies.push_back(CynaraAdminPolicy(label, user, *newIter, CynaraAdminPolicy::Operation::Allow)); } diff --git a/src/server/service/include/cynara.h b/src/server/service/include/cynara.h index 0daa1e5..187b53f 100644 --- a/src/server/service/include/cynara.h +++ b/src/server/service/include/cynara.h @@ -88,7 +88,7 @@ public: * adding new, previously not enabled privileges. * Caller must have permission to access Cynara administrative socket. * - * @param pkg package identifier + * @param label application Smack label * @param user user identifier * @param oldPrivileges previously enabled privileges for the package. * Must be sorted and without duplicates. @@ -98,7 +98,7 @@ public: * TODO: drop oldPrivileges argument and get them directly from Cynara. * Appropriate Cynara interface is needed first. */ - static void UpdatePackagePolicy(const std::string &pkg, const std::string &user, + static void UpdatePackagePolicy(const std::string &label, const std::string &user, const std::vector &oldPrivileges, const std::vector &newPrivileges); diff --git a/src/server/service/service.cpp b/src/server/service/service.cpp index 664235f..461682b 100644 --- a/src/server/service/service.cpp +++ b/src/server/service/service.cpp @@ -285,7 +285,7 @@ bool Service::processAppInstall(MessageBuffer &buffer, MessageBuffer &send, uid_ m_privilegeDb.AddApplication(req.appId, req.pkgId, uid, pkgIdIsNew); m_privilegeDb.UpdateAppPrivileges(req.appId, uid, req.privileges); m_privilegeDb.GetPkgPrivileges(req.pkgId, uid, newPkgPrivileges); - CynaraAdmin::UpdatePackagePolicy(req.pkgId, uidstr, oldPkgPrivileges, + CynaraAdmin::UpdatePackagePolicy(smackLabel, uidstr, oldPkgPrivileges, newPkgPrivileges); m_privilegeDb.CommitTransaction(); LogDebug("Application installation commited to database"); @@ -356,7 +356,6 @@ bool Service::processAppUninstall(MessageBuffer &buffer, MessageBuffer &send, ui if (!generateAppLabel(pkgId, smackLabel)) { LogError("Cannot generate Smack label for package: " << pkgId); goto error_label; - } std::string uidstr = uid ? std::to_string(static_cast(uid)) @@ -369,7 +368,7 @@ bool Service::processAppUninstall(MessageBuffer &buffer, MessageBuffer &send, ui m_privilegeDb.UpdateAppPrivileges(appId, uid, std::vector()); m_privilegeDb.RemoveApplication(appId, uid, removePkg); m_privilegeDb.GetPkgPrivileges(pkgId, uid, newPkgPrivileges); - CynaraAdmin::UpdatePackagePolicy(pkgId, uidstr, oldPkgPrivileges, + CynaraAdmin::UpdatePackagePolicy(smackLabel, uidstr, oldPkgPrivileges, newPkgPrivileges); m_privilegeDb.CommitTransaction(); LogDebug("Application uninstallation commited to database"); -- 2.7.4