From: Youngjae Cho Date: Tue, 7 Jan 2025 02:33:17 +0000 (+0900) Subject: halapi: common: Change .hal-backend-compatibility smack label to '_' from 'System' X-Git-Tag: accepted/tizen/9.0/unified/20250118.074051~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9f1dd8ab8d57daf09a8361a4da4facca8957a65a;p=platform%2Fhal%2Fapi%2Fcommon.git halapi: common: Change .hal-backend-compatibility smack label to '_' from 'System' To allow access from unprivileged process, changed the smack label of /opt/etc/hal/.hal-backend-compatibility to '_'. • Jan 07 10:09:01 localhost audit[9954]: AVC lsm=SMACK fn=smack_inode_permission action=denied subject="User" object="System" requested=r pid=9954 comm="launchpad-loade" name=".hal-backend-compatibility" dev="vda2" ino=128002 To set the file smack label to '_', several options were considered below, and we selected the 3rd option. 1) Set the file smack label '_' using setxattr() : The setxattr() for changing smack label requires capability CAP_MAC_ADMIN. However, the capability is being strictly managed and granted exceptionally to those who need to manipulate security policy, for example, processes that launching application. Our issue is not such case. 2) Create the directory /opt/etc/hal in advance with smack label '_' with transmute flag : Instead of granting CAP_MAC_ADMIN to hal-compatibility-checker.service, make /opt/etc/hal directory with smack label '_' plus transmute flag in advance (by post install script or else). Transmute flag on a directory makes a file created on that directory inherit directory's smack label instead of a process' smack label that created the file. However it additionaly requires smack rule "System _ rwxat" to be effective. The trailing 't' must be appended to the current smack rule, but security team reviewed and concluded that it might cause side effect so it was rejected. 3) Create the file /opt/etc/hal/.hal-backend-compatibility as well as the directory /opt/etc/hal in advance with smack label '_'. And introduce additional file that denotes whether it requires update. : Create all directory and file in advance with smack label '_'. If a file has been created with a specific smack label, the smack label of a file is retained while being written by other smack labeled process. As the .hal-backend-compatibility file has changed to be persistent (we use systemd-tmpfiles to create it so it always exists), we need additional information whether it needs to be updated. So introduced empty file .hal-backend-compatibility-loaded. The file will be created after creating .hal-backend-compatibility. If the file exists, hal-compatibility-checker.service won't be processed. Change-Id: I985675457f40f89a28c9a05cd032e10c6220f8de Signed-off-by: Youngjae Cho --- diff --git a/packaging/500.hal-api-common.sh b/packaging/500.hal-api-common.sh index ef15dec..1e0bd68 100644 --- a/packaging/500.hal-api-common.sh +++ b/packaging/500.hal-api-common.sh @@ -1,6 +1,6 @@ #!/bin/sh PATH=/bin:/usr/bin:/sbin:/usr/sbin -if [ -e "/opt/etc/hal/.hal-backend-compatibility" ]; then - rm -f /opt/etc/hal/.hal-backend-compatibility +if [ -e "/opt/etc/hal/.hal-backend-compatibility-loaded" ]; then + rm -f /opt/etc/hal/.hal-backend-compatibility-loaded fi diff --git a/packaging/hal-api-common.spec b/packaging/hal-api-common.spec index 4b9e75f..f4dcf8b 100644 --- a/packaging/hal-api-common.spec +++ b/packaging/hal-api-common.spec @@ -10,7 +10,7 @@ ### main package ######### Name: %{name} Summary: %{name} interface -Version: 1.0.1 +Version: 1.0.2 Release: 1 Group: Development/Libraries License: Apache-2.0 @@ -24,6 +24,7 @@ Source6: haltest.target Source7: reboot-haltest Source8: reboot-normal Source9: 500.%{name}.sh +Source10: hal-compatibility-checker.conf Requires(post): /sbin/ldconfig Requires(postun): /sbin/ldconfig @@ -80,6 +81,7 @@ rm -rf %{buildroot} %if %{enable_halcc} mkdir -p %{buildroot}%{_unitdir} cp %{SOURCE4} %{buildroot}%{_unitdir} +install -D -m 0644 %{SOURCE10} %{buildroot}%{_tmpfilesdir}/hal-compatibility-checker.conf %endif mkdir -p %{buildroot}/hal @@ -115,6 +117,7 @@ rm -f %{_unitdir}/sysinit.target.wants/hal-compatibility-checker.service %if %{enable_halcc} %{_bindir}/hal-compatibility-checker %{_unitdir}/hal-compatibility-checker.service +%{_tmpfilesdir}/hal-compatibility-checker.conf %endif %{_datadir}/upgrade/scripts/500.%{name}.sh diff --git a/packaging/hal-compatibility-checker.conf b/packaging/hal-compatibility-checker.conf new file mode 100644 index 0000000..6071804 --- /dev/null +++ b/packaging/hal-compatibility-checker.conf @@ -0,0 +1,6 @@ +# Creates files with proper permssion, smack label +# It is necessary for the hal-api-common or hal-compatibility-checker to cache hal compatibility information +d /opt/etc/hal 0755 system_fw system_fw - - +t /opt/etc/hal - - - - security.SMACK64="_" +f /opt/etc/hal/.hal-backend-compatibility 0755 system_fw system_fw - - +t /opt/etc/hal/.hal-backend-compatibility - - - - security.SMACK64="_" diff --git a/packaging/hal-compatibility-checker.service b/packaging/hal-compatibility-checker.service index 819c3e2..16669a3 100644 --- a/packaging/hal-compatibility-checker.service +++ b/packaging/hal-compatibility-checker.service @@ -1,13 +1,16 @@ [Unit] Description=Generate compatibility information between platform and hal DefaultDependencies=no +Wants=systemd-tmpfiles-setup.service +After=systemd-tmpfiles-setup.service Before=sysinit.target -ConditionPathExists=!/opt/etc/hal/.hal-backend-compatibility +ConditionPathExists=!/opt/etc/hal/.hal-backend-compatibility-loaded [Service] Type=oneshot SmackProcessLabel=System ExecStart=/usr/bin/hal-compatibility-checker --skip-if-result-exist +ExecStartPost=/usr/bin/touch /opt/etc/hal/.hal-backend-compatibility-loaded RemainAfterExit=yes User=system_fw Group=system_fw diff --git a/src/hal-api-compatibility-checker.c b/src/hal-api-compatibility-checker.c index 98b97e9..6c56bc7 100644 --- a/src/hal-api-compatibility-checker.c +++ b/src/hal-api-compatibility-checker.c @@ -37,6 +37,7 @@ #include "hal-api-compatibility-checker-parser.h" #define HAL_CC_DEFAULT_COMPATIBILITY_RESULT_PATH "/opt/etc/hal/.hal-backend-compatibility" +#define HAL_CC_DEFAULT_COMPATIBILITY_LOADED_PATH "/opt/etc/hal/.hal-backend-compatibility-loaded" #define COMPAT_INFO_MODULE_NAME_MAX 64 @@ -50,6 +51,7 @@ struct compatibility_info { static struct compatibility_info g_compatibility_info[HAL_MODULE_END]; static const char *compatibility_result_path = HAL_CC_DEFAULT_COMPATIBILITY_RESULT_PATH; +static const char *compatibility_loaded_path = HAL_CC_DEFAULT_COMPATIBILITY_LOADED_PATH; #ifdef HAL_API_COMMON_UNITTEST void hal_api_cc_set_compatibility_result_path(const char *path) @@ -69,6 +71,19 @@ void hal_api_cc_reset_compatibility_info(void) { memset(g_compatibility_info, 0, sizeof(g_compatibility_info)); } + +void hal_api_cc_set_compatibility_loaded_path(const char *path) +{ + if (!path) + return; + + compatibility_loaded_path = path; +} + +void hal_api_cc_unset_compatibility_loaded_path(void) +{ + compatibility_loaded_path = HAL_CC_DEFAULT_COMPATIBILITY_LOADED_PATH; +} #endif /* HAL_API_COMMON_UNITTEST */ static int get_module_by_name(const char *name, enum hal_module *module) @@ -97,95 +112,6 @@ static int get_module_by_name(const char *name, enum hal_module *module) return -EINVAL; } -static int set_owner(int fd) -{ - static uid_t uid_system_fw = -1; - static gid_t gid_system_fw = -1; - - if (fd == -1) - return -EINVAL; - - if (uid_system_fw == -1) { - struct passwd pwd; - struct passwd *result; - char *buf = NULL; - size_t bufsize = 1024; - int ret; - - buf = malloc(bufsize); - if (!buf) - return -ENOMEM; - - errno = 0; - - while ((ret = getpwnam_r("system_fw", &pwd, buf, bufsize, &result)) == ERANGE) { - char *tmp; - - bufsize *= 2; - tmp = realloc(buf, bufsize); - if (!tmp) { - free(buf); - return -ENOMEM; - } - - buf = tmp; - } - - if (!result) { - free(buf); - if (ret == 0) - return -ENOENT; - else - return -errno; - } - - uid_system_fw = pwd.pw_uid; - - free(buf); - } - - if (gid_system_fw == -1) { - struct group grp; - struct group *result; - char *buf = NULL; - size_t bufsize = 1024; - int ret; - - buf = malloc(bufsize); - if (!buf) - return -ENOMEM; - - errno = 0; - - while ((ret = getgrnam_r("system_fw", &grp, buf, bufsize, &result)) == ERANGE) { - char *tmp; - - bufsize *= 2; - tmp = realloc(buf, bufsize); - if (!tmp) { - free(buf); - return -ENOMEM; - } - - buf = tmp; - } - - if (!result) { - free(buf); - if (ret == 0) - return -ENOENT; - else - return -errno; - } - - gid_system_fw = grp.gr_gid; - - free(buf); - } - - return fchown(fd, uid_system_fw, gid_system_fw); -} - static void __convert_hal_to_info(void *data_hal, void *data_info, bool skip_version_check) { halcc_hal *hal; @@ -253,140 +179,79 @@ static void convert_hal_to_info_skip_version_check(void *data_hal, void *data_in __convert_hal_to_info(data_hal, data_info, true); } -static int mkdir_one(const char *dir, mode_t mode) -{ - if (!dir) - return -EINVAL; - - if (access(dir, F_OK) == 0) - return 0; - - return mkdir(dir, mode); -} - -static int create_directory(const char *path) +static int open_result_file(const char *path, int *fd_out) { - char directory_path[PATH_MAX] = { 0 , }; - char *p; int ret; + int fd_result = -1; + int fd_loaded = -1; + int flags = O_RDWR; - if (!path) - return -EINVAL; - - if (path[0] != '/') + if (!fd_out) return -EINVAL; - if (strlen(path) > PATH_MAX - 1) - return -ENAMETOOLONG; - - // copy path except the last filename - strncpy(directory_path, path, strrchr(path, '/') - path); - - if (access(directory_path, F_OK) == 0) - return 0; - - p = strchr(directory_path + 1, '/'); - for (;;) { - if (!p) - break; - - *p = '\0'; - ret = mkdir_one(directory_path, 0755); - if (ret < 0) - return ret; - *p = '/'; - - p = strchr(p + 1, '/'); - } - - return mkdir_one(directory_path, 0755); - -} - -static int open_result_file(const char *path, int *fd_out, bool reset, const char *module_name) -{ - int ret; - int fd; - mode_t mode = O_WRONLY | O_CREAT; - - if (reset) - mode |= O_TRUNC; - else - mode |= O_EXCL; - - ret = create_directory(path); - if (ret < 0) { - errno = -ret; - _E("%s: Failed to create directory for %s, %m", module_name, path); - return ret; - } - - fd = open(path, mode, 0644); - if (fd == -1) { - if (errno == EEXIST) { - /* file exists and reset is false: use that one */ - fd = open(path, O_WRONLY, 0); - if (fd == -1) - return -errno; - - *fd_out = fd; - return 0; - } + /* If it is the first access to this file, reset the file. */ + if (access(compatibility_loaded_path, F_OK) != 0) + flags |= O_TRUNC; + /** + * The file must be there at this moment with proper credentials(onwer,smack) + * as it must have been generated by systemd-tmpfiles. + * + * See hal-compatibility-checker.conf. + */ + fd_result = open(path, flags, 0); + if (fd_result == -1) { ret = -errno; - _E("%s: Failed to create %s, %m", module_name, path); + _E("Failed to open %s, %m", path); return ret; } - /* set a new file size */ - ret = ftruncate(fd, sizeof(struct compatibility_info) * HAL_MODULE_END); + /* set the file size */ + ret = ftruncate(fd_result, sizeof(struct compatibility_info) * HAL_MODULE_END); if (ret < 0) { ret = -errno; - _E("%s: Failed to ftruncate %s, %m", module_name, path); - close(fd); + _E("Failed to ftruncate %s, %m", path); + close(fd_result); return ret; } - /* system_fw:system_fw */ - ret = set_owner(fd); - if (ret < 0) { - errno = -ret; - _E("%s: Failed to set owner, ret=%d, %m\n", module_name, ret); - close(fd); - return ret; + + /* create file that makes following call not to truncate result file */ + fd_loaded = open(compatibility_loaded_path, O_RDWR | O_CREAT, 0755); + if (fd_loaded < 0) { + _E("Failed to create %s, compatibility info will be incomplete, %m", + compatibility_loaded_path); + close(fd_result); + fd_result = -1; + return -errno; } - *fd_out = fd; + close(fd_loaded); + fd_loaded = -1; + + *fd_out = fd_result; return 0; } -static int write_module_comaptibility_info(enum hal_module module, +static int write_module_comaptibility_info(int fd, enum hal_module module, struct compatibility_info *info) { - int fd = -1; int ret; ssize_t n_write; off_t offset; - ret = open_result_file(compatibility_result_path, &fd, false, info->module_name); - if (ret < 0) { - _E("Failed to create open result file %s", compatibility_result_path); - return ret; - } + assert(fd > 0); offset = sizeof(struct compatibility_info) * module; n_write = pwrite(fd, info, sizeof(*info), offset); if (n_write == -1) { ret = -errno; _E("%s: Failed to write info, %m", info->module_name); - close(fd); return ret; } - close(fd); - return 0; } @@ -409,9 +274,9 @@ static int load_module_compatibility_info(enum hal_module module, return 0; } - fd = open(compatibility_result_path, O_RDONLY, 0); - if (fd == -1) - return -errno; + ret = open_result_file(compatibility_result_path, &fd); + if (ret < 0) + return ret; offset = sizeof(struct compatibility_info) * module; n_read = pread(fd, info, sizeof(*info), offset); @@ -437,6 +302,7 @@ static int load_module_compatibility_info_fallback(enum hal_module module, halcc_manifest *manifest = NULL; struct compatibility_info infos[HAL_MODULE_END] = { 0 , }; struct __hal_module_info *module_info = NULL; + int fd = -1; int ret; assert(info); @@ -478,13 +344,22 @@ static int load_module_compatibility_info_fallback(enum hal_module module, return 0; /* Write all available(initialized) info */ + ret = open_result_file(compatibility_result_path, &fd); + if (ret < 0) { + _E("Failed to create open result file %s", compatibility_result_path); + return ret; + } + for (enum hal_module index = HAL_MODULE_UNKNOWN + 1; index < HAL_MODULE_END; ++index) { if (!infos[index].initialized) continue; - write_module_comaptibility_info(index, &infos[index]); + write_module_comaptibility_info(fd, index, &infos[index]); memcpy(&g_compatibility_info[module], &infos[index], sizeof(struct compatibility_info)); } + close(fd); + fd = -1; + return 0; } diff --git a/src/hal-api-compatibility-checker.h b/src/hal-api-compatibility-checker.h index 1eb7c39..a84016f 100644 --- a/src/hal-api-compatibility-checker.h +++ b/src/hal-api-compatibility-checker.h @@ -34,6 +34,8 @@ int hal_api_cc_get_supported_interface_versions(enum hal_module module, #ifdef HAL_API_COMMON_UNITTEST /* For test use only */ void hal_api_cc_set_compatibility_result_path(const char *path); void hal_api_cc_unset_compatibility_result_path(void); +void hal_api_cc_set_compatibility_loaded_path(const char *path); +void hal_api_cc_unset_compatibility_loaded_path(void); void hal_api_cc_reset_compatibility_info(void); #endif /* HAL_API_COMMON_UNITTEST */ diff --git a/tests/unittest/test-hal-compatibility-checker.cc b/tests/unittest/test-hal-compatibility-checker.cc index ca6fe11..244a6b3 100644 --- a/tests/unittest/test-hal-compatibility-checker.cc +++ b/tests/unittest/test-hal-compatibility-checker.cc @@ -49,6 +49,12 @@ class HalccObjectTest : public ::testing::Test if (ret == -1) FAIL(); + ret = asprintf(&g_compatibility_loaded_path, "%s/.hal-backend-compatibility-loaded", g_cwd); + if (ret == -1) + FAIL(); + + hal_api_cc_set_compatibility_loaded_path(g_compatibility_loaded_path); + ret = halcc_manifest_new(&g_manifest); ASSERT_EQ(ret, 0); @@ -61,6 +67,8 @@ class HalccObjectTest : public ::testing::Test mock_hal_backend_data_unset_version((enum hal_module) module); unlink(g_compatibility_result_path); unset_module_info_manifest(); + hal_api_cc_unset_compatibility_loaded_path(); + free(g_compatibility_loaded_path); free(g_compatibility_result_path); free(g_manifest_directory); free(g_cwd); @@ -68,22 +76,26 @@ class HalccObjectTest : public ::testing::Test } void SetUp() override { - + /* create empty dummy file */ + fclose(fopen(g_compatibility_result_path, "w")); } void TearDown() override { - + /* remove dummy file*/ + unlink(g_compatibility_result_path); } static halcc_manifest *g_manifest; static char *g_manifest_directory; static char *g_compatibility_result_path; + static char *g_compatibility_loaded_path; static char *g_cwd; }; halcc_manifest* HalccObjectTest::g_manifest = NULL; char* HalccObjectTest::g_manifest_directory = NULL; char* HalccObjectTest::g_compatibility_result_path = NULL; +char* HalccObjectTest::g_compatibility_loaded_path = NULL; char* HalccObjectTest::g_cwd = NULL; static bool have_exact_version_within_halcc_hal(halcc_hal *hal, const char *version) diff --git a/tools/hal-compatibility-checker/main.c b/tools/hal-compatibility-checker/main.c index cfd7250..ec21f0d 100644 --- a/tools/hal-compatibility-checker/main.c +++ b/tools/hal-compatibility-checker/main.c @@ -30,7 +30,7 @@ #define DEFAULT_PLATFORM_MANIFEST_DIR "/etc/hal" #define DEFAULT_HAL_INFO_INI "/hal/etc/hal-info.ini" -#define DEFAULT_HAL_BACKEND_COMPATIBILITY_PATH "/opt/etc/hal/.hal-backend-compatibility" +#define DEFAULT_HAL_BACKEND_COMPATIBILITY_LOADED_PATH "/opt/etc/hal/.hal-backend-compatibility-loaded" #define HCC_BUF_MAX (256) #define BOLD(STR) "\e[1m"STR"\e[m" @@ -138,7 +138,7 @@ int main(int argc, char *argv[]) switch (index) { case OPT_SKIP_IF_RESULT_EXIST: skip_if_result_exist = true; - skip_if_result_exist_path = optarg ? : DEFAULT_HAL_BACKEND_COMPATIBILITY_PATH; + skip_if_result_exist_path = optarg ? : DEFAULT_HAL_BACKEND_COMPATIBILITY_LOADED_PATH; break; case OPT_RESET: reset_file = true; @@ -160,7 +160,7 @@ int main(int argc, char *argv[]) } if (reset_file) { - unlink(DEFAULT_HAL_BACKEND_COMPATIBILITY_PATH); + unlink(DEFAULT_HAL_BACKEND_COMPATIBILITY_LOADED_PATH); return 0; }