Skip to content

Commit

Permalink
Give the root PID ownership of the pseudoconsole window (#14196)
Browse files Browse the repository at this point in the history
This is a partial fix for the Get-Credential issue. While investigating it, I found that the pseudoconsole window is not marked as being "owned" (in NTUSER) by the PID/TID of the console application that is "hosted" "in" it. Doing this does not (and cannot) fix `Get-Credential` failing in DefTerm scenarios.

ConsoleSetWindowOwner is one of the operations that can be done by a conhost to any window, and so the RemoteConsoleControl can call through to the Win32 ConsoleControl to pull it off.

I chose to add SetWindowOwner to the IConsoleControl interface instead of moving ConsoleControl::Control into the interface to reduce the amount of churn and better separate interface responsibilities.

References #14119
  • Loading branch information
DHowett authored Dec 1, 2022
1 parent 37aa295 commit 62ffa4b
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 13 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2183,6 +2183,7 @@ wnd
WNDALLOC
WNDCLASS
WNDCLASSEX
WNDCLASSEXW
WNDCLASSW
Wndproc
WNegative
Expand Down
8 changes: 5 additions & 3 deletions src/interactivity/base/InteractivityFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -305,14 +305,16 @@ using namespace Microsoft::Console::Interactivity;
try
{
static const auto PSEUDO_WINDOW_CLASS = L"PseudoConsoleWindow";
WNDCLASS pseudoClass{ 0 };
WNDCLASSEXW pseudoClass{ 0 };
switch (level)
{
case ApiLevel::Win32:
{
pseudoClass.cbSize = sizeof(WNDCLASSEXW);
pseudoClass.lpszClassName = PSEUDO_WINDOW_CLASS;
pseudoClass.lpfnWndProc = s_PseudoWindowProc;
RegisterClass(&pseudoClass);
pseudoClass.cbWndExtra = GWL_CONSOLE_WNDALLOC; // this is required to store the owning thread/process override in NTUSER
auto windowClassAtom{ RegisterClassExW(&pseudoClass) };

// Note that because we're not specifying WS_CHILD, this window
// will become an _owned_ window, not a _child_ window. This is
Expand All @@ -333,7 +335,7 @@ using namespace Microsoft::Console::Interactivity;

// Attempt to create window.
hwnd = CreateWindowExW(exStyles,
PSEUDO_WINDOW_CLASS,
reinterpret_cast<LPCWSTR>(windowClassAtom),
nullptr,
windowStyle,
0,
Expand Down
7 changes: 7 additions & 0 deletions src/interactivity/base/RemoteConsoleControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,11 @@ template<typename T>
return _SendTypedPacket(_pipe.get(), HostSignals::EndTask, data);
}

[[nodiscard]] NTSTATUS RemoteConsoleControl::SetWindowOwner(HWND hwnd, DWORD processId, DWORD threadId)
{
// This call doesn't need to get forwarded to the root conhost. Just handle
// it in-proc, to set the owner of OpenConsole
return _control.SetWindowOwner(hwnd, processId, threadId);
}

#pragma endregion
1 change: 1 addition & 0 deletions src/interactivity/base/RemoteConsoleControl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ namespace Microsoft::Console::Interactivity
[[nodiscard]] NTSTATUS NotifyConsoleApplication(_In_ DWORD dwProcessId);
[[nodiscard]] NTSTATUS SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground);
[[nodiscard]] NTSTATUS EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags);
[[nodiscard]] NTSTATUS SetWindowOwner(HWND hwnd, DWORD processId, DWORD threadId);

private:
wil::unique_handle _pipe;
Expand Down
1 change: 1 addition & 0 deletions src/interactivity/inc/IConsoleControl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ namespace Microsoft::Console::Interactivity
[[nodiscard]] virtual NTSTATUS NotifyConsoleApplication(DWORD dwProcessId) = 0;
[[nodiscard]] virtual NTSTATUS SetForeground(HANDLE hProcess, BOOL fForeground) = 0;
[[nodiscard]] virtual NTSTATUS EndTask(DWORD dwProcessId, DWORD dwEventType, ULONG ulCtrlFlags) = 0;
[[nodiscard]] virtual NTSTATUS SetWindowOwner(HWND hwnd, DWORD processId, DWORD threadId) = 0;
};
}
5 changes: 5 additions & 0 deletions src/interactivity/onecore/ConsoleControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,9 @@ using namespace Microsoft::Console::Interactivity::OneCore;
sizeof(*a));
}

[[nodiscard]] NTSTATUS ConsoleControl::SetWindowOwner(HWND, DWORD, DWORD) noexcept
{
return STATUS_SUCCESS;
}

#pragma endregion
1 change: 1 addition & 0 deletions src/interactivity/onecore/ConsoleControl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@ namespace Microsoft::Console::Interactivity::OneCore
[[nodiscard]] NTSTATUS NotifyConsoleApplication(_In_ DWORD dwProcessId) noexcept override;
[[nodiscard]] NTSTATUS SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground) noexcept override;
[[nodiscard]] NTSTATUS EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) override;
[[nodiscard]] NTSTATUS SetWindowOwner(HWND hwnd, DWORD processId, DWORD threadId) noexcept override;
};
}
12 changes: 12 additions & 0 deletions src/interactivity/win32/ConsoleControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,18 @@ using namespace Microsoft::Console::Interactivity::Win32;
sizeof(ConsoleEndTaskParams));
}

[[nodiscard]] NTSTATUS ConsoleControl::SetWindowOwner(HWND hwnd, DWORD processId, DWORD threadId) noexcept
{
CONSOLEWINDOWOWNER ConsoleOwner;
ConsoleOwner.hwnd = hwnd;
ConsoleOwner.ProcessId = processId;
ConsoleOwner.ThreadId = threadId;

return Control(ConsoleControl::ControlType::ConsoleSetWindowOwner,
&ConsoleOwner,
sizeof(ConsoleOwner));
}

#pragma endregion

#pragma region Public Methods
Expand Down
1 change: 1 addition & 0 deletions src/interactivity/win32/ConsoleControl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ namespace Microsoft::Console::Interactivity::Win32
[[nodiscard]] NTSTATUS NotifyConsoleApplication(_In_ DWORD dwProcessId);
[[nodiscard]] NTSTATUS SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground);
[[nodiscard]] NTSTATUS EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags);
[[nodiscard]] NTSTATUS SetWindowOwner(HWND hwnd, DWORD processId, DWORD threadId) noexcept override;

// Public Members
[[nodiscard]] NTSTATUS Control(_In_ ConsoleControl::ControlType ConsoleCommand,
Expand Down
22 changes: 12 additions & 10 deletions src/interactivity/win32/windowio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,13 @@ VOID SetConsoleWindowOwner(const HWND hwnd, _Inout_opt_ ConsoleProcessHandle* pP
else
{
// Find a process to own the console window. If there are none then let's use conhost's.
pProcessData = gci.ProcessHandleList.GetFirstProcess();
pProcessData = gci.ProcessHandleList.FindProcessInList(ConsoleProcessList::ROOT_PROCESS_ID);
if (!pProcessData)
{
// No root process ID? Pick the oldest existing process.
pProcessData = gci.ProcessHandleList.GetFirstProcess();
}

if (pProcessData != nullptr)
{
dwProcessId = pProcessData->dwProcessId;
Expand All @@ -92,16 +98,8 @@ VOID SetConsoleWindowOwner(const HWND hwnd, _Inout_opt_ ConsoleProcessHandle* pP
}
}

CONSOLEWINDOWOWNER ConsoleOwner;
ConsoleOwner.hwnd = hwnd;
ConsoleOwner.ProcessId = dwProcessId;
ConsoleOwner.ThreadId = dwThreadId;

// Comment out this line to enable UIA tree to be visible until UIAutomationCore.dll can support our scenario.
LOG_IF_FAILED(ServiceLocator::LocateConsoleControl<Microsoft::Console::Interactivity::Win32::ConsoleControl>()
->Control(ConsoleControl::ControlType::ConsoleSetWindowOwner,
&ConsoleOwner,
sizeof(ConsoleOwner)));
LOG_IF_NTSTATUS_FAILED(ServiceLocator::LocateConsoleControl()->SetWindowOwner(hwnd, dwProcessId, dwThreadId));
}

// ----------------------------
Expand Down Expand Up @@ -1048,6 +1046,10 @@ DWORD WINAPI ConsoleInputThreadProcWin32(LPVOID /*lpParameter*/)
// successfully created with the owner configured when the window is
// first created. See GH#13066 for details.
ServiceLocator::LocateGlobals().getConsoleInformation().GetVtIo()->CreatePseudoWindow();

// Register the pseudoconsole window as being owned by the root process.
const auto pseudoWindow = ServiceLocator::LocatePseudoWindow();
SetConsoleWindowOwner(pseudoWindow, nullptr);
}

UnlockConsole();
Expand Down

0 comments on commit 62ffa4b

Please sign in to comment.