From 33c2cd458a1fb00077c0ae62cd8043e388d50943 Mon Sep 17 00:00:00 2001 From: Leon Liang Date: Mon, 24 Jan 2022 18:39:50 -0600 Subject: [PATCH] PR 6616045: Hand off to Windows Terminal Stable by default (!) This commit also introduces a check that we are in an interactive session before we perform handoff, so as to not break service accounts. It also adds some tracing. --- src/host/sources.inc | 2 + src/inc/conint.h | 1 + .../win32/ut_interactivity_win32/sources | 2 + src/internal/stubs.cpp | 9 +++ src/propslib/DelegationConfig.cpp | 37 +++++++++++- src/server/IoDispatchers.cpp | 57 ++++++++++++++++++- src/terminal/adapter/ut_adapter/sources | 2 + src/terminal/parser/ut_parser/sources | 2 + 8 files changed, 109 insertions(+), 3 deletions(-) diff --git a/src/host/sources.inc b/src/host/sources.inc index 543fab3f6d5..042622c404e 100644 --- a/src/host/sources.inc +++ b/src/host/sources.inc @@ -170,6 +170,7 @@ TARGETLIBS = \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-rawinput-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-sysparams-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-window-ext-l1.lib \ + $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-winstamin-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-shell-shell32-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-uxtheme-themes-l1.lib \ $(ONECORESHELL_INTERNAL_LIB_VPATH_L)\api-ms-win-shell-dataobject-l1.lib \ @@ -234,6 +235,7 @@ DELAYLOAD = \ ext-ms-win-rtcore-ntuser-rawinput-l1.dll; \ ext-ms-win-rtcore-ntuser-sysparams-l1.dll; \ ext-ms-win-rtcore-ntuser-window-ext-l1.dll; \ + ext-ms-win-rtcore-ntuser-winstamin-l1.dll; \ ext-ms-win-shell-shell32-l1.dll; \ ext-ms-win-uiacore-l1.dll; \ ext-ms-win-uxtheme-themes-l1.dll; \ diff --git a/src/inc/conint.h b/src/inc/conint.h index e744981f99d..26a2a9e0107 100644 --- a/src/inc/conint.h +++ b/src/inc/conint.h @@ -48,5 +48,6 @@ namespace Microsoft::Console::Internal namespace DefaultApp { [[nodiscard]] HRESULT CheckDefaultAppPolicy(bool& isEnabled) noexcept; + [[nodiscard]] HRESULT CheckShouldTerminalBeDefault(bool& isEnabled) noexcept; } } diff --git a/src/interactivity/win32/ut_interactivity_win32/sources b/src/interactivity/win32/ut_interactivity_win32/sources index 7f128c12b68..0be29330cb6 100644 --- a/src/interactivity/win32/ut_interactivity_win32/sources +++ b/src/interactivity/win32/ut_interactivity_win32/sources @@ -69,6 +69,7 @@ TARGETLIBS = \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-rawinput-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-sysparams-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-window-ext-l1.lib \ + $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-winstamin-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-shell-shell32-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-uxtheme-themes-l1.lib \ $(ONECORESHELL_INTERNAL_LIB_VPATH_L)\api-ms-win-shell-dataobject-l1.lib \ @@ -132,6 +133,7 @@ DELAYLOAD = \ ext-ms-win-rtcore-ntuser-rawinput-l1.dll; \ ext-ms-win-rtcore-ntuser-sysparams-l1.dll; \ ext-ms-win-rtcore-ntuser-window-ext-l1.dll; \ + ext-ms-win-rtcore-ntuser-winstamin-l1.dll; \ ext-ms-win-shell-shell32-l1.dll; \ ext-ms-win-uiacore-l1.dll; \ ext-ms-win-uxtheme-themes-l1.dll; \ diff --git a/src/internal/stubs.cpp b/src/internal/stubs.cpp index f9570d6dde2..da6423d14ec 100644 --- a/src/internal/stubs.cpp +++ b/src/internal/stubs.cpp @@ -37,3 +37,12 @@ void EdpPolicy::AuditClipboard(const std::wstring_view /*destinationName*/) noex isEnabled = true; return S_OK; } + +[[nodiscard]] HRESULT DefaultApp::CheckShouldTerminalBeDefault(bool& isEnabled) noexcept +{ + // False since setting Terminal as the default app is an OS feature and probably + // should not be done in the open source conhost. We can always decide to turn it + // on in the future though. + isEnabled = false; + return S_OK; +} diff --git a/src/propslib/DelegationConfig.cpp b/src/propslib/DelegationConfig.cpp index a37e2587867..dbc7a84a34a 100644 --- a/src/propslib/DelegationConfig.cpp +++ b/src/propslib/DelegationConfig.cpp @@ -13,6 +13,8 @@ #include #include +#include "../inc/conint.h" + using namespace Microsoft::WRL; using namespace Microsoft::WRL::Wrappers; using namespace ABI::Windows::Foundation; @@ -25,6 +27,9 @@ using namespace ABI::Windows::ApplicationModel::AppExtensions; #define DELEGATION_CONSOLE_KEY_NAME L"DelegationConsole" #define DELEGATION_TERMINAL_KEY_NAME L"DelegationTerminal" +#define SYSTEM_DELEGATION_CONSOLE_KEY_NAME L"SystemDelegationConsole" +#define SYSTEM_DELEGATION_TERMINAL_KEY_NAME L"SystemDelegationTerminal" + #define DELEGATION_CONSOLE_EXTENSION_NAME L"com.microsoft.windows.console.host" #define DELEGATION_TERMINAL_EXTENSION_NAME L"com.microsoft.windows.terminal.host" @@ -262,13 +267,41 @@ CATCH_RETURN() [[nodiscard]] HRESULT DelegationConfig::s_GetDefaultConsoleId(IID& iid) noexcept { iid = { 0 }; - return s_Get(DELEGATION_CONSOLE_KEY_NAME, iid); + + auto hr = s_Get(DELEGATION_CONSOLE_KEY_NAME, iid); + + bool defApp = false; + if (SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckShouldTerminalBeDefault(defApp)) && defApp) + { + if (FAILED(hr)) + { + // If we can't find a user-defined delegation console/terminal, use the system-defined + // delegation console/terminal instead. + hr = s_Get(SYSTEM_DELEGATION_CONSOLE_KEY_NAME, iid); + } + } + + return hr; } [[nodiscard]] HRESULT DelegationConfig::s_GetDefaultTerminalId(IID& iid) noexcept { iid = { 0 }; - return s_Get(DELEGATION_TERMINAL_KEY_NAME, iid); + + auto hr = s_Get(DELEGATION_TERMINAL_KEY_NAME, iid); + + bool defApp = false; + if (SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckShouldTerminalBeDefault(defApp)) && defApp) + { + if (FAILED(hr)) + { + // If we can't find a user-defined delegation console/terminal, use the system-defined + // delegation console/terminal instead. + hr = s_Get(SYSTEM_DELEGATION_TERMINAL_KEY_NAME, iid); + } + } + + return hr; } [[nodiscard]] HRESULT DelegationConfig::s_Get(PCWSTR value, IID& iid) noexcept diff --git a/src/server/IoDispatchers.cpp b/src/server/IoDispatchers.cpp index c2f3265bc4e..81707aaf415 100644 --- a/src/server/IoDispatchers.cpp +++ b/src/server/IoDispatchers.cpp @@ -131,6 +131,31 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleCloseObject(_In_ PCONSOLE_API_MSG pMessag return pMessage; } +// LsaGetLoginSessionData might also fit the bill here, but it looks like it does RPC with lsass.exe. Using user32 is cheaper. +static bool _isInteractiveUserSession() +{ + DWORD sessionId{}; + if (ProcessIdToSessionId(GetCurrentProcessId(), &sessionId) && sessionId == 0) + { + return false; + } + + // don't call CloseWindowStation on GetProcessWindowStation handle or switch this to a unique_hwinsta + HWINSTA winsta{ GetProcessWindowStation() }; + if (winsta) + { + USEROBJECTFLAGS flags{}; + if (GetUserObjectInformationW(winsta, UOI_FLAGS, &flags, sizeof(flags), nullptr)) + { + // An invisible window station suggests that we aren't interactive. + return WI_IsFlagSet(flags.dwFlags, WSF_VISIBLE); + } + } + + // Assume that we are interactive if the flags can't be looked up or there's no window station + return true; +} + // Routine Description: // - Uses some information about current console state and // the incoming process state and preferences to determine @@ -152,6 +177,15 @@ static bool _shouldAttemptHandoff(const Globals& globals, #else + // Service desktops and non-interactive sessions should not + // try to hand off -- they probably don't have any terminals + // installed, and we don't want to risk breaking a service if + // they *do*. + if (!_isInteractiveUserSession()) + { + return false; + } + // This console was started with a command line argument to // specifically block handoff to another console. We presume // this was for good reason (compatibility) and give up here. @@ -341,6 +375,13 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API // Start it if it was successfully created. THROW_IF_FAILED(hostSignalThread->Start()); + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "ConsoleHandoffSucceeded", + TraceLoggingDescription("successfully handed off console connection"), + TraceLoggingGuid(Globals.handoffConsoleClsid.value(), "handoffCLSID"), + TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); + // Unlock in case anything tries to spool down as we exit. UnlockConsole(); @@ -350,7 +391,21 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API // Exit process to clean up any outstanding things we have open. ExitProcess(S_OK); } - CATCH_LOG(); // Just log, don't do anything more. We'll move on to launching normally on failure. + catch (...) + { + const auto hr = wil::ResultFromCaughtException(); + + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "ConsoleHandoffFailed", + TraceLoggingDescription("failed while attempting handoff"), + TraceLoggingGuid(Globals.handoffConsoleClsid.value(), "handoffCLSID"), + TraceLoggingHResult(hr), + TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); + + // Just log, don't do anything more. We'll move on to launching normally on failure. + LOG_CAUGHT_EXCEPTION(); + } } Status = NTSTATUS_FROM_HRESULT(gci.ProcessHandleList.AllocProcessData(dwProcessId, diff --git a/src/terminal/adapter/ut_adapter/sources b/src/terminal/adapter/ut_adapter/sources index 873a8ccfb03..73a19ce6f86 100644 --- a/src/terminal/adapter/ut_adapter/sources +++ b/src/terminal/adapter/ut_adapter/sources @@ -72,6 +72,7 @@ TARGETLIBS = \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-rawinput-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-sysparams-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-window-ext-l1.lib \ + $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-winstamin-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-shell-shell32-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-uxtheme-themes-l1.lib \ $(ONECORESHELL_INTERNAL_LIB_VPATH_L)\api-ms-win-shell-dataobject-l1.lib \ @@ -131,6 +132,7 @@ DELAYLOAD = \ ext-ms-win-rtcore-ntuser-rawinput-l1.dll; \ ext-ms-win-rtcore-ntuser-sysparams-l1.dll; \ ext-ms-win-rtcore-ntuser-window-ext-l1.dll; \ + ext-ms-win-rtcore-ntuser-winstamin-l1.dll; \ ext-ms-win-shell-shell32-l1.dll; \ ext-ms-win-uiacore-l1.dll; \ ext-ms-win-uxtheme-themes-l1.dll; \ diff --git a/src/terminal/parser/ut_parser/sources b/src/terminal/parser/ut_parser/sources index 1db2c9d5050..57b201b9985 100644 --- a/src/terminal/parser/ut_parser/sources +++ b/src/terminal/parser/ut_parser/sources @@ -66,6 +66,7 @@ TARGETLIBS = \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-rawinput-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-sysparams-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-window-ext-l1.lib \ + $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-rtcore-ntuser-winstamin-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-shell-shell32-l1.lib \ $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\ext-ms-win-uxtheme-themes-l1.lib \ $(ONECORESHELL_INTERNAL_LIB_VPATH_L)\api-ms-win-shell-dataobject-l1.lib \ @@ -128,6 +129,7 @@ DELAYLOAD = \ ext-ms-win-rtcore-ntuser-rawinput-l1.dll; \ ext-ms-win-rtcore-ntuser-sysparams-l1.dll; \ ext-ms-win-rtcore-ntuser-window-ext-l1.dll; \ + ext-ms-win-rtcore-ntuser-winstamin-l1.dll; \ ext-ms-win-shell-shell32-l1.dll; \ ext-ms-win-uiacore-l1.dll; \ ext-ms-win-uxtheme-themes-l1.dll; \