bug: remove racy code to detect GCE on Windows

The code used to detect if a program was running on Windows by launching
a powershell script (which requires powershell to be installed), writing
the output to a file with a fixed name (which requires write permissions
in the local directory), and then reading the file (which may have
changed), and then deleting it (which may fail and introduces a race
with any other program running in the same directory).

This version reads a key from the Windows registry. That could fail if
the application does not have permissions to read the registry, but at
least does not crash when it does, and it is not inheritently racy.

src/core/lib/security/credentials/alts/check_gcp_environment_windows.cc

@@ -31,80 +31,70 @@
 #include <grpc/support/log.h>
 #include <grpc/support/sync.h>
 
-#define GRPC_ALTS_EXPECT_NAME_GOOGLE "Google"
-#define GRPC_ALTS_WINDOWS_CHECK_COMMAND "powershell.exe"
-#define GRPC_ALTS_WINDOWS_CHECK_COMMAND_ARGS \
-  "(Get-WmiObject -Class Win32_BIOS).Manufacturer"
-#define GRPC_ALTS_WINDOWS_CHECK_BIOS_FILE "windows_bios.data"
-
-const size_t kBiosDataBufferSize = 256;
-
-static bool g_compute_engine_detection_done = false;
-static bool g_is_on_compute_engine = false;
-static gpr_mu g_mu;
-static gpr_once g_once = GPR_ONCE_INIT;
-
 namespace grpc_core {
 namespace internal {
 
-bool check_bios_data(const char* bios_data_file) {
-  char* bios_data = read_bios_file(bios_data_file);
-  bool result = !strcmp(bios_data, GRPC_ALTS_EXPECT_NAME_GOOGLE);
-  remove(GRPC_ALTS_WINDOWS_CHECK_BIOS_FILE);
-  gpr_free(bios_data);
-  return result;
+bool check_bios_data(const char*) {
+  return false;
 }
 
-}  // namespace internal
-}  // namespace grpc_core
+bool check_windows_registry_product_name(
+  HKEY root_key, const char* reg_key_path, const char* reg_key_name) {
+  const size_t kProductNameBufferSize = 256;
+  char const expected_substr[] = "Google";
 
-static void init_mu(void) { gpr_mu_init(&g_mu); }
+  // Get the size of the string first to allocate our buffer. This includes
+  // enough space for the trailing NUL character that will be included.
+  DWORD buffer_size{};
+  auto rc = ::RegGetValueA(
+      root_key, reg_key_path, reg_key_name,
+      RRF_RT_REG_SZ,
+      nullptr,        // We know the type will be REG_SZ.
+      nullptr,        // We're only fetching the size; no buffer given yet.
+      &buffer_size);  // Fetch the size in bytes of the value, if it exists.
+  if (rc != 0) {
+    return false;
+  }
 
-static bool run_powershell() {
-  SECURITY_ATTRIBUTES sa;
-  sa.nLength = sizeof(sa);
-  sa.lpSecurityDescriptor = NULL;
-  sa.bInheritHandle = TRUE;
-  HANDLE h = CreateFile(_T(GRPC_ALTS_WINDOWS_CHECK_BIOS_FILE), GENERIC_WRITE,
-                        FILE_SHARE_WRITE | FILE_SHARE_READ, &sa, OPEN_ALWAYS,
-                        FILE_ATTRIBUTE_NORMAL, NULL);
-  if (h == INVALID_HANDLE_VALUE) {
-    gpr_log(GPR_ERROR, "CreateFile failed (%d).", GetLastError());
+  if (buffer_size > kProductNameBufferSize) {
     return false;
   }
-  PROCESS_INFORMATION pi;
-  STARTUPINFO si;
-  DWORD flags = CREATE_NO_WINDOW;
-  ZeroMemory(&pi, sizeof(pi));
-  ZeroMemory(&si, sizeof(si));
-  si.cb = sizeof(si);
-  si.dwFlags |= STARTF_USESTDHANDLES;
-  si.hStdInput = NULL;
-  si.hStdError = h;
-  si.hStdOutput = h;
-  TCHAR cmd[kBiosDataBufferSize];
-  _sntprintf(cmd, kBiosDataBufferSize, _T("%s %s"),
-             _T(GRPC_ALTS_WINDOWS_CHECK_COMMAND),
-             _T(GRPC_ALTS_WINDOWS_CHECK_COMMAND_ARGS));
-  if (!CreateProcess(NULL, cmd, NULL, NULL, TRUE, flags, NULL, NULL, &si,
-                     &pi)) {
-    gpr_log(GPR_ERROR, "CreateProcess failed (%d).\n", GetLastError());
+
+  // Retrieve the product name string.
+  char buffer[kProductNameBufferSize];
+  buffer_size = kProductNameBufferSize;
+  rc = ::RegGetValueA(
+      root_key, reg_key_path, reg_key_name,
+      RRF_RT_REG_SZ,
+      nullptr,                      // We know the type will be REG_SZ.
+      static_cast<void*>(buffer),   // Fetch the string value this time.
+      &buffer_size);  // The string size in bytes, not including trailing NUL.
+  if (rc != 0) {
     return false;
   }
-  WaitForSingleObject(pi.hProcess, INFINITE);
-  CloseHandle(pi.hProcess);
-  CloseHandle(pi.hThread);
-  CloseHandle(h);
-  return true;
+
+  return strstr(buffer, expected_substr) != nullptr;
 }
 
+}  // namespace internal
+}  // namespace grpc_core
+
+static bool g_compute_engine_detection_done = false;
+static bool g_is_on_compute_engine = false;
+static gpr_mu g_mu;
+static gpr_once g_once = GPR_ONCE_INIT;
+
+static void init_mu(void) { gpr_mu_init(&g_mu); }
+
 bool grpc_alts_is_running_on_gcp() {
+  char const reg_key_path[] = "SYSTEM\\HardwareConfig\\Current\\";
+  char const reg_key_name[] = "SystemProductName";
+
   gpr_once_init(&g_once, init_mu);
   gpr_mu_lock(&g_mu);
   if (!g_compute_engine_detection_done) {
-    g_is_on_compute_engine =
-        run_powershell() &&
-        grpc_core::internal::check_bios_data(GRPC_ALTS_WINDOWS_CHECK_BIOS_FILE);
+    g_is_on_compute_engine = grpc_core::internal::check_windows_registry_product_name(
+      HKEY_LOCAL_MACHINE, reg_key_path, reg_key_name);
     g_compute_engine_detection_done = true;
   }
   gpr_mu_unlock(&g_mu);

test/core/security/check_gcp_environment_windows_test.cc

@@ -29,23 +29,42 @@
 #include <grpc/support/log.h>
 #include "src/core/lib/gpr/tmpfile.h"
 
+namespace grpc_core {
+namespace internal {
+
+bool check_windows_registry_product_name(
+  HKEY root_key, const char* reg_key_path, const char* reg_key_name);
+
+}  // namespace internal
+}  // namespace grpc_core
+
 static bool check_bios_data_windows_test(const char* data) {
-  /* Create a file with contents data. */
-  char* filename = nullptr;
-  FILE* fp = gpr_tmpfile("check_gcp_environment_test", &filename);
-  GPR_ASSERT(filename != nullptr);
-  GPR_ASSERT(fp != nullptr);
-  GPR_ASSERT(fwrite(data, 1, strlen(data), fp) == strlen(data));
-  fclose(fp);
-  bool result = grpc_core::internal::check_bios_data(
-      reinterpret_cast<const char*>(filename));
-  /* Cleanup. */
-  remove(filename);
-  gpr_free(filename);
+  char const reg_key_path[] = "SYSTEM\\HardwareConfig\\Current\\";
+  char const reg_key_name[] = "grpcTestValueName";
+  // Modify the registry for the current user to contain the
+  // test value. We cannot use the system registry because the
+  // user may not have privileges to change it.
+  auto rc = RegSetKeyValueA(
+    HKEY_CURRENT_USER, reg_key_path, reg_key_name, REG_SZ,
+    reinterpret_cast<const BYTE*>(data),
+    static_cast<DWORD>(strlen(data)));
+  if (rc != 0) {
+    return false;
+  }
+
+  auto result = grpc_core::internal::check_windows_registry_product_name(
+    HKEY_CURRENT_USER, reg_key_path, reg_key_name);
+  
+  (void)RegDeleteKeyValueA(HKEY_CURRENT_USER, reg_key_path, reg_key_name);
+
   return result;
 }
 
 static void test_gcp_environment_check_success() {
+  // This is the only value observed in production.
+  GPR_ASSERT(check_bios_data_windows_test("Google Compute Engine"));
+  // Be generous and accept other values that were accepted by the previous
+  // implementation.
   GPR_ASSERT(check_bios_data_windows_test("Google"));
   GPR_ASSERT(check_bios_data_windows_test("Google\n"));
   GPR_ASSERT(check_bios_data_windows_test("Google\r"));