Resolve Angle failure because it breaks with is_high_integrity

Angle drops the security enforcement of the loader on environment
variables and the additional "is_high_integrity" checks on my
previous loader change caused failures in the Angle run.
Revert the high-integrity checks and update the layer test to
work properly.

diff --git a/loader/loader.c b/loader/loader.c
index a154a1d..1adb03c 100644 (file)
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -2969,19 +2969,13 @@ static VkResult read_data_files_in_search_paths(const struct loader_instance *in
     if (path_override != NULL) {
         override_path = path_override;
     } else if (env_override != NULL) {
-        // Don't allow setuid apps to use the env var
-        if (is_high_integrity()) {
-            loader_log(inst, VULKAN_LOADER_WARN_BIT, 0,
-                       "read_data_files_in_search_paths: Ignoring override %s due to high-integrity", env_override);
-        } else {
-            override_env = loader_secure_getenv(env_override, inst);
+        override_env = loader_secure_getenv(env_override, inst);
 
-            // The ICD override is actually a specific list of filenames, not directories
-            if (is_icd && NULL != override_env) {
-                is_directory_list = false;
-            }
-            override_path = override_env;
+        // The ICD override is actually a specific list of filenames, not directories
+        if (is_icd && NULL != override_env) {
+            is_directory_list = false;
         }
+        override_path = override_env;
     }
 
     // Add two by default for NULL terminator and one path separator on end (just in case)
@@ -3063,7 +3057,7 @@ static VkResult read_data_files_in_search_paths(const struct loader_instance *in
                         cur_path_ptr += rel_size;
                         *cur_path_ptr++ = PATH_SEPARATOR;
                         // only for ICD manifests
-                        if (!is_high_integrity() && env_override != NULL && strcmp(VK_ICD_FILENAMES_ENV_VAR, env_override) == 0) {
+                        if (env_override != NULL && strcmp(VK_ICD_FILENAMES_ENV_VAR, env_override) == 0) {
                             use_first_found_manifest = true;
                         }
                     }

diff --git a/tests/framework/shim/CMakeLists.txt b/tests/framework/shim/CMakeLists.txt
index 5a51ae1..f1b2707 100644 (file)
--- a/tests/framework/shim/CMakeLists.txt
+++ b/tests/framework/shim/CMakeLists.txt
@@ -17,7 +17,7 @@
 
 add_library(shim-common STATIC shim_common.cpp)
 target_link_libraries(shim-common PUBLIC testing_framework_util)
-target_include_directories(shim-common PUBLIC ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR})
+target_include_directories(shim-common PUBLIC ${CMAKE_BINARY_DIR}/loader ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR})
 
 if (WIN32)
     # need adapters.h which is in the loader folder

diff --git a/tests/framework/shim/unix_shim.cpp b/tests/framework/shim/unix_shim.cpp
index ff2c6c0..a1f5b99 100644 (file)
--- a/tests/framework/shim/unix_shim.cpp
+++ b/tests/framework/shim/unix_shim.cpp
@@ -25,6 +25,10 @@
  * Author: Charles Giessen <charles@lunarg.com>
  */
 
+#if !defined(VULKAN_NON_CMAKE_BUILD)
+#include "loader_cmake_config.h"
+#endif  // !defined(VULKAN_NON_CMAKE_BUILD)
+
 #include "shim.h"
 
 static PlatformShim platform_shim;
@@ -48,12 +52,24 @@ FRAMEWORK_EXPORT PlatformShim* get_platform_shim() {
 #define FOPEN_FUNC_NAME fopen
 #define GETEUID_FUNC_NAME geteuid
 #define GETEGID_FUNC_NAME getegid
+#if defined(HAVE_SECURE_GETENV)
+#define SECURE_GETENV_FUNC_NAME secure_getenv
+#endif
+#if defined(HAVE___SECURE_GETENV)
+#define __SECURE_GETENV_FUNC_NAME __secure_getenv
+#endif
 #elif defined(__APPLE__)
 #define OPENDIR_FUNC_NAME my_opendir
 #define ACCESS_FUNC_NAME my_access
 #define FOPEN_FUNC_NAME my_fopen
 #define GETEUID_FUNC_NAME my_geteuid
 #define GETEGID_FUNC_NAME my_getegid
+#if defined(HAVE_SECURE_GETENV)
+#define SECURE_GETENV_FUNC_NAME my_secure_getenv
+#endif
+#if defined(HAVE___SECURE_GETENV)
+#define __SECURE_GETENV_FUNC_NAME my__secure_getenv
+#endif
 #endif
 
 using PFN_OPENDIR = DIR* (*)(const char* path_name);
@@ -61,12 +77,21 @@ using PFN_ACCESS = int (*)(const char* pathname, int mode);
 using PFN_FOPEN = FILE* (*)(const char* filename, const char* mode);
 using PFN_GETEUID = uid_t (*)(void);
 using PFN_GETEGID = gid_t (*)(void);
+#if defined(HAVE_SECURE_GETENV) || defined(HAVE___SECURE_GETENV)
+using PFN_SEC_GETENV = char* (*)(const char* name);
+#endif
 
 static PFN_OPENDIR real_opendir = nullptr;
 static PFN_ACCESS real_access = nullptr;
 static PFN_FOPEN real_fopen = nullptr;
 static PFN_GETEUID real_geteuid = nullptr;
 static PFN_GETEGID real_getegid = nullptr;
+#if defined(HAVE_SECURE_GETENV)
+static PFN_SEC_GETENV real_secure_getenv = nullptr;
+#endif
+#if defined(HAVE___SECURE_GETENV)
+static PFN_SEC_GETENV real__secure_getenv = nullptr;
+#endif
 
 FRAMEWORK_EXPORT DIR* OPENDIR_FUNC_NAME(const char* path_name) {
     if (!real_opendir) real_opendir = (PFN_OPENDIR)dlsym(RTLD_NEXT, "opendir");
@@ -139,6 +164,29 @@ FRAMEWORK_EXPORT gid_t GETEGID_FUNC_NAME(void) {
     }
 }
 
+#if defined(HAVE_SECURE_GETENV)
+FRAMEWORK_EXPORT char* SECURE_GETENV_FUNC_NAME(const char *name) {
+    if (!real_secure_getenv) real_secure_getenv = (PFN_SEC_GETENV)dlsym(RTLD_NEXT, "secure_getenv");
+
+    if (platform_shim.use_fake_elevation) {
+        return NULL;
+    } else {
+        return real_secure_getenv(name);
+    }
+}
+#endif
+#if defined(HAVE___SECURE_GETENV)
+FRAMEWORK_EXPORT char* __SECURE_GETENV_FUNC_NAME(const char *name) {
+    if (!real__secure_getenv) real__secure_getenv = (PFN_SEC_GETENV)dlsym(RTLD_NEXT, "__secure_getenv");
+
+    if (platform_shim.use_fake_elevation) {
+        return NULL;
+    } else {
+        return real__secure_getenv(name);
+    }
+}
+#endif
+
 /* Shiming functions on apple is limited by the linker prefering to not use functions in the
  * executable in loaded dylibs. By adding an interposer, we redirect the linker to use our
  * version of the function over the real one, thus shimming the system function.
@@ -157,5 +205,11 @@ __attribute__((used)) static Interposer _interpose_access MACOS_ATTRIB = {VOIDP_
 __attribute__((used)) static Interposer _interpose_fopen MACOS_ATTRIB = {VOIDP_CAST(my_fopen), VOIDP_CAST(fopen)};
 __attribute__((used)) static Interposer _interpose_euid MACOS_ATTRIB = {VOIDP_CAST(my_geteuid), VOIDP_CAST(geteuid)};
 __attribute__((used)) static Interposer _interpose_egid MACOS_ATTRIB = {VOIDP_CAST(my_getegid), VOIDP_CAST(getegid)};
+#if defined(HAVE_SECURE_GETENV)
+__attribute__((used)) static Interposer _interpose_secure_getenv MACOS_ATTRIB = {VOIDP_CAST(my_secure_getenv), VOIDP_CAST(secure_getenv)};
+#endif
+#if defined(HAVE___SECURE_GETENV)
+__attribute__((used)) static Interposer _interpose__secure_getenv MACOS_ATTRIB = {VOIDP_CAST(my__secure_getenv), VOIDP_CAST(__secure_getenv)};
+#endif
 #endif
 }  // extern "C"

diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp
index b2231f7..c865e8b 100644 (file)
--- a/tests/loader_regression_tests.cpp
+++ b/tests/loader_regression_tests.cpp
@@ -2202,7 +2202,6 @@ TEST(EnvironmentVariables, VK_LAYER_PATH) {
     EXPECT_TRUE(env.debug_log.find("/tmp/carol"));
     EXPECT_TRUE(env.debug_log.find("/tandy"));
     EXPECT_TRUE(env.debug_log.find((HOME / "/ with spaces/").str()));
-    EXPECT_FALSE(env.debug_log.find("Ignoring override VK_LAYER_PATH due to high-integrity"));
 
     env.debug_log.clear();
 
@@ -2213,7 +2212,6 @@ TEST(EnvironmentVariables, VK_LAYER_PATH) {
     FillDebugUtilsCreateDetails(inst2.create_info, env.debug_log);
     inst2.CheckCreate();
 
-    EXPECT_TRUE(env.debug_log.find("Ignoring override VK_LAYER_PATH due to high-integrity"));
     EXPECT_FALSE(env.debug_log.find("/tmp/carol"));
 
     env.platform_shim->set_elevated_privilege(false);