From: Krzysztof Jackiewicz Date: Wed, 22 Jan 2025 15:20:55 +0000 (+0100) Subject: Setup paths using DAC X-Git-Tag: accepted/tizen/unified/20250217.155039~11 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=443db22ebef02696d78b0b1257a775d0c0fdf203;p=platform%2Fcore%2Fsecurity%2Fsecurity-manager.git Setup paths using DAC Change-Id: I7841eb8807e16190d0c1b733be498775413f8af5 --- diff --git a/src/common/include/service_impl.h b/src/common/include/service_impl.h index 46bc61d7..678c9355 100644 --- a/src/common/include/service_impl.h +++ b/src/common/include/service_impl.h @@ -441,7 +441,10 @@ private: const uid_t &uid, app_install_type installationType); - int labelPaths(path_req& req, bool isSharedRO); + int labelPaths(const Credentials &creds, + const path_req &req, + bool isSharedRO, + const std::vector &processUids); void getPkgLabels(const std::string &pkgName, Smack::Labels &pkgsLabels); diff --git a/src/common/include/smack-labels.h b/src/common/include/smack-labels.h index 24cb8b2e..d1bc9143 100644 --- a/src/common/include/smack-labels.h +++ b/src/common/include/smack-labels.h @@ -31,9 +31,11 @@ #include #include +#include #include #include #include +#include "credentials.h" namespace SecurityManager { namespace SmackLabels { @@ -54,6 +56,23 @@ void setupPath( app_install_path_type pathType, const std::string &authorHash = std::string()); +/** + * Sets ACL and DAC permissions on a directory and its contents, recursively. + * + * @param creds[in] client's credentials + * @param path[in] path to a file or directory to setup + * @param pathType[in] type of path to setup. See description of + * app_install_path_type in security-manager.h for details + * @param processUIds[in] list of owner applications' process UIds (PUIDs) + * @param authorGId[in] optional group id of all apps having the same author + */ +void setupPath( + const Credentials& creds, + const std::string &path, + app_install_path_type pathType, + const std::vector& processUIds, + const std::optional& authorGId); + /** * Sets Smack labels on a / non-recursively * @@ -61,6 +80,18 @@ void setupPath( */ void setupPkgBasePath(const std::string &basePath); +/** + * Sets ACL and DAC permissions on / non-recursively + * + * @param creds[in] client's credentials + * @param basePath[in] / path + * @param processUIds[in] list of owner applications' process UIds (PUIDs) + */ +void setupPkgBasePath( + const Credentials &creds, + const std::string &basePath, + const std::vector &processUIds); + /** * Changes Smack label on path to enable private sharing * diff --git a/src/common/service_impl.cpp b/src/common/service_impl.cpp index ad3bdff0..a3ee5868 100644 --- a/src/common/service_impl.cpp +++ b/src/common/service_impl.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -42,13 +43,15 @@ #include #include #include +#include +#include +#include #include #include -#include - #include +#include "acl.h" #include "check-proper-drop.h" #include "protocols.h" #include "privilege_db.h" @@ -346,17 +349,15 @@ bool ServiceImpl::authCheck(const Credentials &creds, return true; } -int ServiceImpl::labelPaths(path_req& req, bool isSharedRO) +int ServiceImpl::labelPaths(const Credentials &creds, + const path_req &req, + bool isSharedRO, + const std::vector &processUIds) { try { - - if (!smack_simple_check()) { - LogWarning("Running in no-smack mode, not labeling any paths"); - if (!m_privilegeDb.PkgNameExists(req.pkgName)) { - LogError("No such package: " << req.pkgName); - return SECURITY_MANAGER_ERROR_INPUT_PARAM; - } - return SECURITY_MANAGER_SUCCESS; + if (!m_privilegeDb.PkgNameExists(req.pkgName)) { + LogError("No such package: " << req.pkgName); + return SECURITY_MANAGER_ERROR_INPUT_PARAM; } std::string authorHash; @@ -371,7 +372,7 @@ int ServiceImpl::labelPaths(path_req& req, bool isSharedRO) return ret; } - LogWarning("Determinig if paths are legal"); + LogWarning("Determining if paths are legal"); // check if paths are inside of legal directories if (!pathsCheck(req.pkgPaths, pkgLegalBaseDirs)) return SECURITY_MANAGER_ERROR_NOT_PATH_OWNER; @@ -389,13 +390,24 @@ int ServiceImpl::labelPaths(path_req& req, bool isSharedRO) for (const auto &pkgPath : req.pkgPaths) { const std::string &path = pkgPath.first; app_install_path_type pathType = static_cast(pkgPath.second); - SmackLabels::setupPath(req.pkgName, path, pathType, authorHash); + if (smack_simple_check()) { + SmackLabels::setupPath(req.pkgName, path, pathType, authorHash); + } else { + gid_t authorGId; + if (m_privilegeDb.GetAuthorGId(req.pkgName, authorGId)) + SmackLabels::setupPath(creds, path, pathType, processUIds, authorGId); + else + SmackLabels::setupPath(creds, path, pathType, processUIds, std::nullopt); + } } LogWarning("Labeling base paths"); for (const auto &basePath : pkgLegalBaseDirs) { if (containSubDir(basePath, req.pkgPaths)) { - SmackLabels::setupPkgBasePath(basePath); + if (smack_simple_check()) + SmackLabels::setupPkgBasePath(basePath); + else + SmackLabels::setupPkgBasePath(creds, basePath, processUIds); } } @@ -415,6 +427,11 @@ int ServiceImpl::labelPaths(path_req& req, bool isSharedRO) } catch (const SmackException::Base &e) { LogError("Error while generating Smack labels: " << e.DumpToString()); return SECURITY_MANAGER_ERROR_SETTING_FILE_LABEL_FAILED; + } catch (const Acl::Exception::Base &e) { + LogError("Error while setting up ACL"); + return SECURITY_MANAGER_ERROR_SETTING_FILE_LABEL_FAILED; + } catch (const std::runtime_error &) { + return SECURITY_MANAGER_ERROR_UNKNOWN; } catch (const std::bad_alloc &e) { LogError("Memory allocation failed: " << e.what()); return SECURITY_MANAGER_ERROR_MEMORY; @@ -626,6 +643,7 @@ int ServiceImpl::appInstall(const Credentials &creds, app_inst_req &req) // [db] begin ScopedTransaction trans(m_privilegeDb); + std::vector processUids; for (auto &app : req.apps) { // [db] add app LogWarning("Adding privileges for app " << app.appName << " into security-manager's db"); @@ -633,6 +651,10 @@ int ServiceImpl::appInstall(const Credentials &creds, app_inst_req &req) // [cynara] update app policy LogWarning("Configuring privileges for app " << app.appName << " in cynara"); appInstallCynaraPolicies(app, req, ih); + + // collect process uids + if (!smack_simple_check() && (req.isHybrid || processUids.empty())) + processUids.emplace_back(getProcessUId(app, req)); } // [db] update shared ro @@ -656,7 +678,7 @@ int ServiceImpl::appInstall(const Credentials &creds, app_inst_req &req) // label paths LogWarning("Configuring package paths"); - ret = labelPaths(req, isAppSharedRO); + ret = labelPaths(creds, req, isAppSharedRO, processUids); if (ret != SECURITY_MANAGER_SUCCESS) return ret; @@ -752,6 +774,7 @@ int ServiceImpl::appUpdate(const Credentials &creds, app_inst_req &req) // [db] begin ScopedTransaction trans(m_privilegeDb); + std::vector processUids; for (auto &app : req.apps) { // [db] add app LogWarning("Adding privileges for app " << app.appName << " into security-manager's db"); @@ -759,6 +782,10 @@ int ServiceImpl::appUpdate(const Credentials &creds, app_inst_req &req) // [cynara] update app policy LogWarning("Configuring privileges for app " << app.appName << " in cynara"); appInstallCynaraPolicies(app, req, ih); + + // collect process uids + if (!smack_simple_check() && (req.isHybrid || processUids.empty())) + processUids.emplace_back(getProcessUId(app, req)); } // [db] update shared ro @@ -781,7 +808,7 @@ int ServiceImpl::appUpdate(const Credentials &creds, app_inst_req &req) // label paths LogWarning("Configuring package paths"); - ret = labelPaths(req, isAppSharedRO); + ret = labelPaths(creds, req, isAppSharedRO, processUids); if (ret != SECURITY_MANAGER_SUCCESS) return ret; @@ -2088,12 +2115,30 @@ int ServiceImpl::pathsRegister(const Credentials &creds, path_req req) } bool isRequestSharedRO = isSharedRO(req.pkgPaths); + std::vector processUids; try { if (isRequestSharedRO) { ScopedTransaction trans(m_privilegeDb); m_privilegeDb.SetSharedROPackage(req.pkgName); + if (!smack_simple_check()) { + // collect process uids + std::vector apps; + m_privilegeDb.GetUserAppsFromPkg(creds.uid, req.pkgName, apps); + for (const auto &appName : apps) { + uid_t processUid; + if (!m_privilegeDb.GetProcessUId(req.pkgName, appName, processUid)) { + LogError("Failed to get process UId from the db"); + return SECURITY_MANAGER_ERROR_SERVER_ERROR; + } + if (!processUids.empty() && processUid == processUids.front()) + break; // non-hybrid + + processUids.emplace_back(processUid); + } + } + trans.commit(); } } catch (const PrivilegeDb::Exception::IOError &e) { @@ -2104,7 +2149,7 @@ int ServiceImpl::pathsRegister(const Credentials &creds, path_req req) return SECURITY_MANAGER_ERROR_SERVER_ERROR; } - return labelPaths(req, isRequestSharedRO); + return labelPaths(creds, req, isRequestSharedRO, processUids); } int ServiceImpl::shmAppName(const Credentials &creds, const std::string &shmName, const std::string &appName) diff --git a/src/common/smack-labels.cpp b/src/common/smack-labels.cpp index 0113ac35..52c052df 100644 --- a/src/common/smack-labels.cpp +++ b/src/common/smack-labels.cpp @@ -30,6 +30,8 @@ #include #include +#include +#include #include #include #include @@ -46,6 +48,7 @@ #include #include +#include "acl.h" #include "security-manager.h" #include "service_impl_utils.h" #include "smack-check.h" @@ -72,7 +75,7 @@ static inline void pathSetSmack(const char *path, const Smack::Label &label, class PathSetup { public: explicit PathSetup(bool transmute) : m_transmute(transmute) {} - void setup(const std::string &path) + void setup(const std::string &path) const { // setting access label on everything in given directory and below char *const path_argv[] = { const_cast(path.c_str()), NULL }; @@ -106,7 +109,7 @@ public: virtual ~PathSetup() = default; protected: - virtual void setupFile(FTSENT *ftsent) = 0; + virtual void setupFile(FTSENT *ftsent) const = 0; bool m_transmute; }; @@ -119,7 +122,7 @@ public: } protected: - void setupFile(FTSENT *ftsent) override + void setupFile(FTSENT *ftsent) const override { pathSetSmack(ftsent->fts_path, m_label, XATTR_NAME_SMACK); if (m_transmute && S_ISDIR(ftsent->fts_statp->st_mode)) @@ -173,11 +176,143 @@ void setupPath( smack.setup(path); } +struct Dac { + // ACL for files and dirs. Effective permissions will be "ANDed" with ACL mask so + // non-executable files won't get 'x' + Acl acl; + uid_t owner; + gid_t group; +}; + +class DacSetup: public PathSetup { +public: + DacSetup(Dac dac) : PathSetup(true), m_dac(std::move(dac)) {} + + void setupFile(FTSENT *ftsent) const override + { + if (0 != chown(ftsent->fts_path, m_dac.owner, m_dac.group)) + ThrowErrno(SmackException::FileError, "chown() for path: " << ftsent->fts_path); + + m_dac.acl.apply(ftsent->fts_path, ACL_TYPE_ACCESS); + + /* + * For newly created files: + * - let the owner come from process (be it app PUId or system_access). The creator clearly + * already has RWX. + * - force the group = system_access using DAC set-group-ID bit so even files created by + * app will be accessible by sys daemons. SM needs CAP_FSETID and system_access group for + * that. + * - Default ACL rules will be applied from the parent directory. + */ + if (m_transmute && ftsent->fts_level == 0) { + // need to re-read the mode after ACL application above + struct stat statbuf; + if (0 != stat(ftsent->fts_path, &statbuf)) + ThrowErrno(SmackException::FileError, "stat() for path: " << ftsent->fts_path); + + // add set-group-ID for system_access + if (0 != chmod(ftsent->fts_path, S_ISGID | statbuf.st_mode)) + ThrowErrno(SmackException::FileError, "chmod() for path: " << ftsent->fts_path); + + // this works on all files/directories below + m_dac.acl.apply(ftsent->fts_path, ACL_TYPE_DEFAULT); + } + } + +private: + Dac m_dac; +}; + +void setupPath(const Credentials& creds, + const std::string &path, + app_install_path_type pathType, + const std::vector &processUIds, + const std::optional &authorGId) +{ + Dac dac; + // Make the real user (owner=5001) the owner. No app should run with this uid + dac.owner = creds.uid; + // Make system_access the group. No app should run with it. + dac.group = getSystemAccessGid(); + + acl_perm_t appPerms; + acl_perm_t otherPerms; + bool follow_symlink; + std::vector entries; + + switch (pathType) { + case SECURITY_MANAGER_PATH_OWNER_RW_OTHER_RO: + appPerms = Acl::RWX; + otherPerms = Acl::R; + follow_symlink = true; + break; + + case SECURITY_MANAGER_PATH_RW: + appPerms = Acl::RWX; + otherPerms = 0; + follow_symlink = false; + break; + + case SECURITY_MANAGER_PATH_RO: + appPerms = Acl::RX; + otherPerms = 0; + follow_symlink = false; + break; + + case SECURITY_MANAGER_PATH_PUBLIC_RO: + appPerms = Acl::RX; + otherPerms = Acl::R; + follow_symlink = false; + break; + + case SECURITY_MANAGER_PATH_TRUSTED_RW: + if (!authorGId.has_value()) + ThrowMsg(SmackException::InvalidParam, "You must define author to use PATH_TRUSED_RW"); + + appPerms = Acl::RWX; + otherPerms = 0; + follow_symlink = false; + entries.emplace_back(std::make_unique(appPerms, *authorGId)); + break; + + default: + LogError("Path type not known."); + Throw(SmackException::InvalidPathType); + } + + for (auto &processUId : processUIds) + entries.emplace_back(std::make_unique(appPerms, processUId)); + + // RWX for the owner(5001) and system_access + dac.acl = Acl::Make(Acl::RWX, Acl::RWX, otherPerms, std::move(entries)); + + DacSetup dacSetup(std::move(dac)); + if (follow_symlink) + dacSetup.setup(realPath(path)); + dacSetup.setup(path); +} + void setupPkgBasePath(const std::string &basePath) { pathSetSmack(basePath.c_str(), LABEL_FOR_APP_PUBLIC_RO_PATH, XATTR_NAME_SMACK); } +void setupPkgBasePath(const Credentials &creds, + const std::string &basePath, + const std::vector &processUIds) +{ + // apply public RO access permissions + if (0 != chown(basePath.c_str(), creds.uid, getSystemAccessGid())) + ThrowErrno(SmackException::FileError, "chown() for path: " << basePath); + + std::vector entries; + for (auto &processUId : processUIds) + entries.emplace_back(std::make_unique(Acl::RX, processUId)); + + auto acl = Acl::Make(Acl::RWX, Acl::RWX, Acl::RX, std::move(entries)); + acl.apply(basePath, ACL_TYPE_ACCESS); +} + void setupSharedPrivatePath(const std::string &pkgName, const std::string &path) { pathSetSmack(path.c_str(), generateSharedPrivateLabel(pkgName, path), XATTR_NAME_SMACK); } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 6f18f6dc..fa3ab337 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -33,6 +33,7 @@ PKG_CHECK_MODULES(COMMON_DEP REQUIRED mount libcap openssl3 + libacl ) FIND_PACKAGE(Threads REQUIRED)