From: Youngjae Cho Date: Thu, 18 Jul 2024 06:51:42 +0000 (+0900) Subject: halcc: Remove checking SYSTEMD_SCOPE environment variable X-Git-Tag: accepted/tizen/unified/20240722.104230^0 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fheads%2Faccepted%2Ftizen_unified_dev;p=platform%2Fhal%2Fapi%2Fcommon.git halcc: Remove checking SYSTEMD_SCOPE environment variable The environment variable is used for testing whether the execution context is systemd system generator. And for this, getenv() had been utilized. However, there was a blame that the getenv() is vulnerable as the environment variable could be controlled externally. Therefore, removed using SYSTEMD_SCOPE environment variable. Due to this, it has become that someone can create compatibility result file, /opt/etc/hal/.hal-backend-compatibility, outside of systemd system generator context. Nevertheless, we can screen out the worst case that unprivileged process generates the file because the directory /opt/etc has attribute of {root,system_share,0775}. Change-Id: I89fd5abc5c2445ce827371f61244bbc9e388d438 Signed-off-by: Youngjae Cho --- diff --git a/packaging/systemd-hal-compatibility-checker-generator b/packaging/systemd-hal-compatibility-checker-generator index b96456d..089071c 100644 --- a/packaging/systemd-hal-compatibility-checker-generator +++ b/packaging/systemd-hal-compatibility-checker-generator @@ -2,12 +2,4 @@ PATH=/bin:/usr/bin:/sbin:/usr/sbin -# FIXME: Remove SYSTEMD_SCOPE after upgrading systemd version beyond 251. -# -# As of systemd version 251, systemd sets this environment variable to "system" -# if it is invoked from generator of system service manager. But we are currently -# on version 244 so manually set the variable to inform the binary that it is -# generator context of system service manager. -export SYSTEMD_SCOPE=system - hal-compatibility-checker --skip-if-result-exist diff --git a/src/hal-api-compatibility-checker.c b/src/hal-api-compatibility-checker.c index bdb53d3..c941103 100644 --- a/src/hal-api-compatibility-checker.c +++ b/src/hal-api-compatibility-checker.c @@ -253,33 +253,6 @@ 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 bool is_system_generator_context(const char *module_name) -{ - char *systemd_scope = NULL; - - /** - * If the generator is invoked from the system service manager this variable - * is set to "system"; if invoked from the per-user service manager it is set to "user". - * - * Added in systemd version 251. - */ - systemd_scope = getenv("SYSTEMD_SCOPE"); - - if (!systemd_scope) { - _E("%s: Reject to create %s, not a generator context", - module_name, compatibility_result_path); - return false; - } - - if (strncmp(systemd_scope, "system", sizeof("system")) != 0) { - _E("%s: Reject to create %s, only system servie manager is allowed to do it", - module_name, compatibility_result_path); - return false; - } - - return true; -} - static int mkdir_one(const char *dir, mode_t mode) { if (!dir) @@ -500,10 +473,6 @@ static int load_module_compatibility_info_fallback(enum hal_module module, halcc_manifest_free(manifest); manifest = NULL; - /* Writing result is only allowed on system generator context */ - if (!is_system_generator_context(info->module_name)) - return 0; - /* Incomplete data, no versions. Don't write it */ if (skip_version_check) return 0; diff --git a/tests/unittest/test-hal-compatibility-checker.cc b/tests/unittest/test-hal-compatibility-checker.cc index c944db3..3195c09 100644 --- a/tests/unittest/test-hal-compatibility-checker.cc +++ b/tests/unittest/test-hal-compatibility-checker.cc @@ -262,8 +262,6 @@ TEST_F(HalccObjectTest, hal_create_result_file_success) enum hal_common_backend_compatibility compatibility; int ret; - /* mock systemd generator context */ - setenv("SYSTEMD_SCOPE", "system", 1); set_compatibility_result_path(g_compatibility_result_path); ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); @@ -275,220 +273,6 @@ TEST_F(HalccObjectTest, hal_create_result_file_success) unlink(g_compatibility_result_path); unset_compatibility_result_path(); reset_compatibility_info(); - unsetenv("SYSTEMD_SCOPE"); -} - -TEST_F(HalccObjectTest, hal_create_result_file_fail) -{ - enum hal_common_backend_compatibility compatibility; - int ret; - - set_compatibility_result_path(g_compatibility_result_path); - - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - - ret = access(g_compatibility_result_path, F_OK | ACCESS_DO_NOT_HOOK); - ASSERT_EQ(ret, -1); - - unset_compatibility_result_path(); - reset_compatibility_info(); -} - -TEST_F(HalccObjectTest, hal_check_compatibility_on_generator_context) -{ - enum hal_common_backend_compatibility compatibility; - int ret; - - setenv("SYSTEMD_SCOPE", "system", 1); - set_compatibility_result_path(g_compatibility_result_path); - - ret = access(g_compatibility_result_path, F_OK | ACCESS_DO_NOT_HOOK); - ASSERT_EQ(ret, -1); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 1, 0); /* compatible with 1.2 */ - mock_hal_backend_data_set_version(HAL_MODULE_TDM, 5, 0); /* compatible with 5.5 */ - - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_COMPATIBLE); - - ret = access(g_compatibility_result_path, F_OK | ACCESS_DO_NOT_HOOK); - ASSERT_EQ(ret, 0); - - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_TDM, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_COMPATIBLE); - - mock_hal_backend_data_unset_version(HAL_MODULE_DEVICE_DISPLAY); - mock_hal_backend_data_unset_version(HAL_MODULE_TDM); - - unlink(g_compatibility_result_path); - unset_compatibility_result_path(); - reset_compatibility_info(); - unsetenv("SYSTEMD_SCOPE"); -} - -TEST_F(HalccObjectTest, hal_check_compatibility_not_on_generator_context) -{ - /** - * As it is not on generator context and there is no result file, it parses - * manifest file for every call for hal_api_cc_check_backend_compatibility(), - * dlopen backend and derive compatibility by comparing their version. - * - * However, this test driver, technically, won't dlopen but instead modify memory - * of dlopened backend data. This has effect of changing HAL image with specified - * version, make testing various HAL backend version handy. - */ - enum hal_common_backend_compatibility compatibility; - int ret; - - set_compatibility_result_path(g_compatibility_result_path); - - ret = access(g_compatibility_result_path, F_OK | ACCESS_DO_NOT_HOOK); - ASSERT_EQ(ret, -1); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 1, 1); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_COMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 1, 1); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_COMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 1, 2); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_COMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 1, 3); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_INCOMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 2, 0); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_COMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 2, 1); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_COMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 2, 2); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_INCOMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 3, 0); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_COMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 3, 1); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_INCOMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 4, 0); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_INCOMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_TDM, 5, 5); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_TDM, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_COMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_TDM, 5, 6); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_TDM, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_INCOMPATIBLE); - - mock_hal_backend_data_set_version(HAL_MODULE_TDM, 6, 5); - ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_TDM, &compatibility); - ASSERT_EQ(ret, 0); - ASSERT_EQ(compatibility, HAL_COMMON_BACKEND_COMPATIBILITY_INCOMPATIBLE); - - ret = access(g_compatibility_result_path, F_OK | ACCESS_DO_NOT_HOOK); - ASSERT_EQ(ret, -1); - - mock_hal_backend_data_unset_version(HAL_MODULE_DEVICE_DISPLAY); - mock_hal_backend_data_unset_version(HAL_MODULE_TDM); - unset_compatibility_result_path(); -} - -TEST_F(HalccObjectTest, hal_get_backend_without_generator_without_result_file) -{ - int ret; - void *funcs; - - set_compatibility_result_path(g_compatibility_result_path); - - ret = access(g_compatibility_result_path, F_OK | ACCESS_DO_NOT_HOOK); - ASSERT_EQ(ret, -1); - - funcs = malloc(10000); - ASSERT_NE(funcs, nullptr); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 1, 1); - ret = hal_common_get_backend(HAL_MODULE_DEVICE_DISPLAY, (void **) funcs); - ASSERT_EQ(ret, 0); - - ret = access(g_compatibility_result_path, F_OK | ACCESS_DO_NOT_HOOK); - ASSERT_EQ(ret, -1); - - free(funcs); - funcs = NULL; - - mock_hal_backend_data_unset_version(HAL_MODULE_DEVICE_DISPLAY); - unset_compatibility_result_path(); - reset_compatibility_info(); -} - -TEST_F(HalccObjectTest, hal_get_backend_with_generator_without_result_file) -{ - int ret; - void *funcs; - - setenv("SYSTEMD_SCOPE", "system", 1); - set_compatibility_result_path(g_compatibility_result_path); - - ret = access(g_compatibility_result_path, F_OK | ACCESS_DO_NOT_HOOK); - ASSERT_EQ(ret, -1); - - funcs = malloc(10000); - ASSERT_NE(funcs, nullptr); - - mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 1, 1); - ret = hal_common_get_backend(HAL_MODULE_DEVICE_DISPLAY, (void **) funcs); - ASSERT_EQ(ret, 0); - - ret = access(g_compatibility_result_path, F_OK | ACCESS_DO_NOT_HOOK); - ASSERT_EQ(ret, -1); - - /** - * FIXME: - * Temporarily allow incompatible version until all of hal modules specify - * their manifest. - * - * mock_hal_backend_data_set_version(HAL_MODULE_DEVICE_DISPLAY, 1, 5); - * ret = hal_common_get_backend(HAL_MODULE_DEVICE_DISPLAY, (void **) funcs); - * ASSERT_EQ(ret, -EINVAL); - */ - - ret = access(g_compatibility_result_path, F_OK | ACCESS_DO_NOT_HOOK); - ASSERT_EQ(ret, -1); - - free(funcs); - funcs = NULL; - - mock_hal_backend_data_unset_version(HAL_MODULE_DEVICE_DISPLAY); - unset_compatibility_result_path(); - unsetenv("SYSTEMD_SCOPE"); } TEST_F(HalccObjectTest, hal_get_manifest_version_success_with_result_file) @@ -497,7 +281,6 @@ TEST_F(HalccObjectTest, hal_get_manifest_version_success_with_result_file) enum hal_common_backend_compatibility compatibility; struct versions_info vi = { 0 , }; - setenv("SYSTEMD_SCOPE", "system", 1); set_compatibility_result_path(g_compatibility_result_path); ret = hal_api_cc_check_backend_compatibility(HAL_MODULE_DEVICE_DISPLAY, &compatibility); @@ -523,30 +306,4 @@ TEST_F(HalccObjectTest, hal_get_manifest_version_success_with_result_file) unlink(g_compatibility_result_path); unset_compatibility_result_path(); reset_compatibility_info(); - unsetenv("SYSTEMD_SCOPE"); -} - -TEST_F(HalccObjectTest, hal_get_manifest_version_success_without_result_file) -{ - int ret; - struct versions_info vi = { 0 , }; - - ret = access(g_compatibility_result_path, F_OK | ACCESS_DO_NOT_HOOK); - ASSERT_EQ(ret, -1); - - ret = hal_common_get_supported_interface_versions(HAL_MODULE_DEVICE_DISPLAY, - &vi.major_versions, &vi.minor_versions, &vi.num_versions); - ASSERT_EQ(ret, 0); - - ASSERT_THAT(&vi, HasExactVersionArray("1.2")); - ASSERT_THAT(&vi, HasExactVersionArray("2.1")); - ASSERT_THAT(&vi, HasExactVersionArray("3.0")); - - free(vi.major_versions); - vi.major_versions = NULL; - - free(vi.minor_versions); - vi.minor_versions = NULL; - - reset_compatibility_info(); }