Skip to content

Commit 68079d8

Browse files
brycehutchingsrpavlik
authored andcommitted
Log message for developer when secure environment prevents environment variable from being used
1 parent 2c1b77d commit 68079d8

File tree

4 files changed

+57
-13
lines changed

4 files changed

+57
-13
lines changed

src/api_layers/api_dump.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,20 @@ struct ApiDumpRecordInfo {
6969
static ApiDumpRecordInfo g_record_info = {};
7070
static std::mutex g_record_mutex = {};
7171

72+
// For routing platform_utils.hpp messages.
73+
void LogPlatformUtilsError(const std::string &message) {
74+
std::cerr << message << std::endl;
75+
#if defined(XR_OS_WINDOWS)
76+
OutputDebugStringA((message + "\n").c_str());
77+
#endif
78+
}
79+
void LogPlatformUtilsWarning(const std::string &message) {
80+
std::cout << message << std::endl;
81+
#if defined(XR_OS_WINDOWS)
82+
OutputDebugStringA((message + "\n").c_str());
83+
#endif
84+
}
85+
7286
// HTML utilities
7387
bool ApiDumpLayerWriteHtmlHeader() {
7488
try {

src/api_layers/core_validation.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,20 @@ bool CoreValidationWriteHtmlFooter() {
211211
}
212212
}
213213

214+
// For routing platform_utils.hpp messages.
215+
void LogPlatformUtilsError(const std::string &message) {
216+
std::cerr << message << std::endl;
217+
#if defined(XR_OS_WINDOWS)
218+
OutputDebugStringA((message + "\n").c_str());
219+
#endif
220+
}
221+
void LogPlatformUtilsWarning(const std::string &message) {
222+
std::cout << message << std::endl;
223+
#if defined(XR_OS_WINDOWS)
224+
OutputDebugStringA((message + "\n").c_str());
225+
#endif
226+
}
227+
214228
// Function to record all the core validation information
215229
void CoreValidLogMessage(GenValidUsageXrInstanceInfo *instance_info, const std::string &message_id,
216230
GenValidUsageDebugSeverity message_severity, const std::string &command_name,

src/common/platform_utils.hpp

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@
3636
#include "common_config.h"
3737
#endif // OPENXR_HAVE_COMMON_CONFIG
3838

39+
// Consumers of this file must ensure these two functions are implemented. For example, the loader will implement these functions so
40+
// that it can route messages through the loader's logging system.
41+
void LogPlatformUtilsError(const std::string& message);
42+
void LogPlatformUtilsWarning(const std::string& message);
43+
3944
// Environment variables
4045
#if defined(XR_OS_LINUX) || defined(XR_OS_APPLE)
4146

@@ -78,6 +83,12 @@ static inline std::string PlatformUtilsGetEnv(const char* name) {
7883
static inline std::string PlatformUtilsGetSecureEnv(const char* name) {
7984
auto str = detail::ImplGetSecureEnv(name);
8085
if (str == nullptr) {
86+
str = detail::ImplGetEnv(name);
87+
if (!str.empty()) {
88+
LogPlatformUtilsWarning(std::string("!!! WARNING !!! Environment variable ") + name +
89+
" is being ignored due to running with secure execution. The value '" + str +
90+
"' will NOT be used.");
91+
}
8192
return {};
8293
}
8394
return str;
@@ -130,12 +141,6 @@ static inline bool PlatformGetGlobalRuntimeFileName(uint16_t major_version, std:
130141

131142
#elif defined(XR_OS_WINDOWS)
132143

133-
#if !defined(NDEBUG)
134-
inline void LogError(const std::string& error) { OutputDebugStringA(error.c_str()); }
135-
#else
136-
#define LogError(x)
137-
#endif
138-
139144
inline std::wstring utf8_to_wide(const std::string& utf8Text) {
140145
if (utf8Text.empty()) {
141146
return {};
@@ -144,7 +149,7 @@ inline std::wstring utf8_to_wide(const std::string& utf8Text) {
144149
std::wstring wideText;
145150
const int wideLength = ::MultiByteToWideChar(CP_UTF8, 0, utf8Text.data(), (int)utf8Text.size(), nullptr, 0);
146151
if (wideLength == 0) {
147-
LogError("utf8_to_wide get size error: " + std::to_string(::GetLastError()));
152+
LogPlatformUtilsError("utf8_to_wide get size error: " + std::to_string(::GetLastError()));
148153
return {};
149154
}
150155

@@ -153,7 +158,7 @@ inline std::wstring utf8_to_wide(const std::string& utf8Text) {
153158
wchar_t* wideString = const_cast<wchar_t*>(wideText.data()); // mutable data() only exists in c++17
154159
const int length = ::MultiByteToWideChar(CP_UTF8, 0, utf8Text.data(), (int)utf8Text.size(), wideString, wideLength);
155160
if (length != wideLength) {
156-
LogError("utf8_to_wide convert string error: " + std::to_string(::GetLastError()));
161+
LogPlatformUtilsError("utf8_to_wide convert string error: " + std::to_string(::GetLastError()));
157162
return {};
158163
}
159164

@@ -168,7 +173,7 @@ inline std::string wide_to_utf8(const std::wstring& wideText) {
168173
std::string narrowText;
169174
int narrowLength = ::WideCharToMultiByte(CP_UTF8, 0, wideText.data(), (int)wideText.size(), nullptr, 0, nullptr, nullptr);
170175
if (narrowLength == 0) {
171-
LogError("wide_to_utf8 get size error: " + std::to_string(::GetLastError()));
176+
LogPlatformUtilsError("wide_to_utf8 get size error: " + std::to_string(::GetLastError()));
172177
return {};
173178
}
174179

@@ -178,7 +183,7 @@ inline std::string wide_to_utf8(const std::wstring& wideText) {
178183
const int length =
179184
::WideCharToMultiByte(CP_UTF8, 0, wideText.data(), (int)wideText.size(), narrowString, narrowLength, nullptr, nullptr);
180185
if (length != narrowLength) {
181-
LogError("wide_to_utf8 convert string error: " + std::to_string(::GetLastError()));
186+
LogPlatformUtilsError("wide_to_utf8 convert string error: " + std::to_string(::GetLastError()));
182187
return {};
183188
}
184189

@@ -244,7 +249,7 @@ static inline std::string PlatformUtilsGetEnv(const char* name) {
244249
// call if there was enough capacity. Else it returns the required capacity (including null terminator).
245250
const DWORD length = ::GetEnvironmentVariableW(wname.c_str(), wValueData, (DWORD)wValue.size());
246251
if ((length == 0) || (length >= wValue.size())) { // If error or the variable increased length between calls...
247-
LogError("GetEnvironmentVariable get value error: " + std::to_string(::GetLastError()));
252+
LogPlatformUtilsError("GetEnvironmentVariable get value error: " + std::to_string(::GetLastError()));
248253
return {};
249254
}
250255

@@ -255,13 +260,20 @@ static inline std::string PlatformUtilsGetEnv(const char* name) {
255260

256261
// Acts the same as PlatformUtilsGetEnv except returns an empty string if IsHighIntegrityLevel.
257262
static inline std::string PlatformUtilsGetSecureEnv(const char* name) {
263+
// No secure version for Windows so the below integrity check is needed.
264+
const std::string envValue = PlatformUtilsGetEnv(name);
265+
258266
// Do not allow high integrity processes to act on data that can be controlled by medium integrity processes.
259267
if (IsHighIntegrityLevel()) {
268+
if (!envValue.empty()) {
269+
LogPlatformUtilsWarning(std::string("!!! WARNING !!! Environment variable ") + name +
270+
" is being ignored due to running from an elevated context. The value '" + envValue +
271+
"' will NOT be used.");
272+
}
260273
return {};
261274
}
262275

263-
// No secure version for Windows so the above integrity check is needed.
264-
return PlatformUtilsGetEnv(name);
276+
return envValue;
265277
}
266278

267279
// Sets an environment variable via UTF8 strings.

src/loader/loader_logger.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@
2626
#include <utility>
2727
#include <vector>
2828

29+
// For routing platform_utils.hpp messages into the LoaderLogger.
30+
void LogPlatformUtilsError(const std::string& message) { LoaderLogger::LogErrorMessage("platform_utils", message); }
31+
void LogPlatformUtilsWarning(const std::string& message) { LoaderLogger::LogWarningMessage("platform_utils", message); }
32+
2933
bool LoaderLogRecorder::LogDebugUtilsMessage(XrDebugUtilsMessageSeverityFlagsEXT /*message_severity*/,
3034
XrDebugUtilsMessageTypeFlagsEXT /*message_type*/,
3135
const XrDebugUtilsMessengerCallbackDataEXT* /*callback_data*/) {

0 commit comments

Comments
 (0)