diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index 1ce9612596..e2da1807d0 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -40,6 +40,8 @@ ALLINPUT Allman Allmodule ALLNOISE +allowlist +allowlisted ALLOWUNDO ALLVIEW ALPHATYPE @@ -120,6 +122,8 @@ backticks Badflags Badmode Badparam +basename +basenames bbwe BCIE Belarusian @@ -182,6 +186,7 @@ CALG callbackptr calpwstr Cangjie +canonicalize CANRENAME Canvascustomlayout CAPTUREBLT @@ -366,6 +371,7 @@ debouncer debugbreak decryptor Dedup +deduped Deduplicator DEFAULTBOOTSTRAPPERINSTALLFOLDER DEFAULTCOLOR @@ -679,6 +685,7 @@ hbr HBRBACKGROUND hbrush hcblack +HCCE HCRYPTHASH HCRYPTPROV hcursor @@ -1384,6 +1391,7 @@ Pitjantjatjara PKBDLLHOOKSTRUCT pkgfamily PKI +PKIX plib ploc ploca @@ -1753,6 +1761,7 @@ SNAPPROCESS snk snwprintf softline +softpub SOURCECLIENTAREAONLY sourced sourcedoc @@ -1977,6 +1986,7 @@ uncompilable UNCPRIORITY UNDNAME unescaped +unforgeable ungroup UNICODETEXT unins @@ -1992,6 +2002,7 @@ unparsable unremapped Unsend Unsubscribes +untampered untriaged unvirtualized unwide @@ -2145,6 +2156,7 @@ winrt winsdk winsta WINTHRESHOLD +wintrust WINVER winword winxamlmanager diff --git a/src/common/UnitTests-CommonUtils/PipeCallerAuth.Tests.cpp b/src/common/UnitTests-CommonUtils/PipeCallerAuth.Tests.cpp new file mode 100644 index 0000000000..1cfd1f1a0f --- /dev/null +++ b/src/common/UnitTests-CommonUtils/PipeCallerAuth.Tests.cpp @@ -0,0 +1,216 @@ +#include "pch.h" +#include "TestHelpers.h" + +#include + +#include +#include + +using namespace Microsoft::VisualStudio::CppUnitTestFramework; + +namespace UnitTestsCommonUtils +{ + namespace + { + std::wstring CurrentExePath() + { + wchar_t buf[MAX_PATH * 2] = {}; + GetModuleFileNameW(nullptr, buf, ARRAYSIZE(buf)); + return buf; + } + + std::wstring DirOf(const std::wstring& p) + { + const auto pos = p.find_last_of(L"\\/"); + return pos == std::wstring::npos ? p : p.substr(0, pos); + } + + std::wstring BaseOf(const std::wstring& p) + { + const auto pos = p.find_last_of(L"\\/"); + return pos == std::wstring::npos ? p : p.substr(pos + 1); + } + + // Sets up a real connected named-pipe pair inside this process so AuthenticateClient can be + // exercised end-to-end. The "client" is this test host, so its image path / PID are what the + // policy is matched against. + struct ConnectedPipe + { + HANDLE server = INVALID_HANDLE_VALUE; + HANDLE client = INVALID_HANDLE_VALUE; + ~ConnectedPipe() + { + if (server != INVALID_HANDLE_VALUE) + { + CloseHandle(server); + } + if (client != INVALID_HANDLE_VALUE) + { + CloseHandle(client); + } + } + }; + + bool MakeConnectedPipe(ConnectedPipe& out) + { + static LONG counter = 0; + const std::wstring name = L"\\\\.\\pipe\\pt_auth_test_" + + std::to_wstring(GetCurrentProcessId()) + L"_" + + std::to_wstring(GetTickCount64()) + L"_" + + std::to_wstring(InterlockedIncrement(&counter)); + HANDLE server = CreateNamedPipeW(name.c_str(), + PIPE_ACCESS_DUPLEX, + PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT, + 1, + 4096, + 4096, + 0, + nullptr); + if (server == INVALID_HANDLE_VALUE) + { + return false; + } + + HANDLE client = INVALID_HANDLE_VALUE; + std::thread connectThread([&]() { + client = CreateFileW(name.c_str(), GENERIC_READ | GENERIC_WRITE, 0, nullptr, OPEN_EXISTING, 0, nullptr); + }); + const BOOL connected = ConnectNamedPipe(server, nullptr) ? TRUE : (GetLastError() == ERROR_PIPE_CONNECTED); + connectThread.join(); + + if (!connected || client == INVALID_HANDLE_VALUE) + { + CloseHandle(server); + if (client != INVALID_HANDLE_VALUE) + { + CloseHandle(client); + } + return false; + } + out.server = server; + out.client = client; + return true; + } + } + + TEST_CLASS(PipeCallerAuthTests) + { + public: + // A disabled policy is a pass-through (preserves the managed start(nullptr) server and tests). + TEST_METHOD(DisabledPolicy_Accepts) + { + interop_auth::CallerPolicy policy; + interop_auth::VerificationCache cache; + const auto res = interop_auth::AuthenticateClient(nullptr, policy, cache); + Assert::IsTrue(res.accepted); + } + + TEST_METHOD(GetModuleVersion_KnownBinary_NonZero) + { + wchar_t sys[MAX_PATH] = {}; + GetSystemDirectoryW(sys, ARRAYSIZE(sys)); + const std::wstring kernel = std::wstring(sys) + L"\\kernel32.dll"; + Assert::IsTrue(interop_auth::GetModuleVersion(kernel) != 0ULL); + } + + TEST_METHOD(GetModuleVersion_BogusPath_Zero) + { + Assert::AreEqual(0ULL, interop_auth::GetModuleVersion(L"Z:\\does\\not\\exist.exe")); + } + + // Legitimate caller (this test host, matched by its own dir + basename) is accepted. + TEST_METHOD(EnabledPolicy_MatchingCaller_Accepts) + { + ConnectedPipe cp; + Assert::IsTrue(MakeConnectedPipe(cp), L"failed to set up connected pipe"); + + const std::wstring exe = CurrentExePath(); + interop_auth::CallerPolicy policy; + policy.enabled = true; + policy.expectedDirectory = DirOf(exe); + policy.allowedBasenames = { BaseOf(exe) }; + policy.expectedVersion = 0; // skip version match + policy.requireMicrosoftSignature = false; // test host is not Microsoft-signed + + interop_auth::VerificationCache cache; + const auto res = interop_auth::AuthenticateClient(cp.server, policy, cache); + Assert::IsTrue(res.accepted, L"legitimate self caller should be accepted"); + Assert::AreEqual(GetCurrentProcessId(), res.pid); + } + + // Reproduces the PoC path: a caller whose image is not on the allow-list is rejected with no + // dispatch, and the required rejection log callback fires. + TEST_METHOD(EnabledPolicy_WrongBasename_Rejects) + { + ConnectedPipe cp; + Assert::IsTrue(MakeConnectedPipe(cp), L"failed to set up connected pipe"); + + const std::wstring exe = CurrentExePath(); + interop_auth::CallerPolicy policy; + policy.enabled = true; + policy.expectedDirectory = DirOf(exe); + policy.allowedBasenames = { L"definitely_not_the_test_host.exe" }; + policy.requireMicrosoftSignature = false; + + bool logged = false; + policy.logReject = [&](const interop_auth::AuthResult&) { logged = true; }; + + interop_auth::VerificationCache cache; + const auto res = interop_auth::AuthenticateClient(cp.server, policy, cache); + Assert::IsFalse(res.accepted, L"caller with non-allowlisted basename must be rejected"); + Assert::AreEqual(L"bad-basename", res.reasonCode); + Assert::IsTrue(logged, L"rejection must invoke the log callback"); + } + + // A caller image outside the expected directory is rejected. + TEST_METHOD(EnabledPolicy_WrongDirectory_Rejects) + { + ConnectedPipe cp; + Assert::IsTrue(MakeConnectedPipe(cp), L"failed to set up connected pipe"); + + const std::wstring exe = CurrentExePath(); + interop_auth::CallerPolicy policy; + policy.enabled = true; + policy.expectedDirectory = L"C:\\Windows\\System32"; // not where the test host lives + policy.allowedBasenames = { BaseOf(exe) }; + policy.requireMicrosoftSignature = false; + + interop_auth::VerificationCache cache; + const auto res = interop_auth::AuthenticateClient(cp.server, policy, cache); + Assert::IsFalse(res.accepted); + Assert::AreEqual(L"bad-directory", res.reasonCode); + } + + // Each pipe server owns its own cache, so the same client process is evaluated independently + // per policy — an accept verdict in one server's cache never bleeds into another server that + // has a different (stricter) policy. + TEST_METHOD(SeparateCaches_AreIndependent) + { + const std::wstring exe = CurrentExePath(); + + interop_auth::CallerPolicy acceptPolicy; + acceptPolicy.enabled = true; + acceptPolicy.expectedDirectory = DirOf(exe); + acceptPolicy.allowedBasenames = { BaseOf(exe) }; + acceptPolicy.requireMicrosoftSignature = false; + + interop_auth::CallerPolicy rejectPolicy = acceptPolicy; + rejectPolicy.allowedBasenames = { L"not_the_test_host.exe" }; + + interop_auth::VerificationCache cacheA; // e.g. the Settings server's cache + interop_auth::VerificationCache cacheB; // e.g. the Quick Access server's cache + + ConnectedPipe cp1; + Assert::IsTrue(MakeConnectedPipe(cp1), L"failed to set up connected pipe 1"); + const auto rA = interop_auth::AuthenticateClient(cp1.server, acceptPolicy, cacheA); + Assert::IsTrue(rA.accepted, L"self caller accepted under the permissive policy"); + + // Same client process (same pid + creation time), different server/cache/policy. + ConnectedPipe cp2; + Assert::IsTrue(MakeConnectedPipe(cp2), L"failed to set up connected pipe 2"); + const auto rB = interop_auth::AuthenticateClient(cp2.server, rejectPolicy, cacheB); + Assert::IsFalse(rB.accepted, L"a separate server cache must not inherit the other's accept verdict"); + Assert::AreEqual(L"bad-basename", rB.reasonCode); + } + }; +} diff --git a/src/common/UnitTests-CommonUtils/UnitTests-CommonUtils.vcxproj b/src/common/UnitTests-CommonUtils/UnitTests-CommonUtils.vcxproj index 409569d5ca..1fd90f31ae 100644 --- a/src/common/UnitTests-CommonUtils/UnitTests-CommonUtils.vcxproj +++ b/src/common/UnitTests-CommonUtils/UnitTests-CommonUtils.vcxproj @@ -48,6 +48,10 @@ + + + NotUsing + diff --git a/src/common/interop/PowerToys.Interop.vcxproj b/src/common/interop/PowerToys.Interop.vcxproj index 4b38d2609b..3fe8f79480 100644 --- a/src/common/interop/PowerToys.Interop.vcxproj +++ b/src/common/interop/PowerToys.Interop.vcxproj @@ -120,6 +120,7 @@ + @@ -147,6 +148,9 @@ TwoWayPipeMessageIPCManaged.idl + + NotUsing + diff --git a/src/common/interop/PowerToys.Interop.vcxproj.filters b/src/common/interop/PowerToys.Interop.vcxproj.filters index 9a2b3edb6f..962aae5bc0 100644 --- a/src/common/interop/PowerToys.Interop.vcxproj.filters +++ b/src/common/interop/PowerToys.Interop.vcxproj.filters @@ -45,6 +45,9 @@ Header Files + + Header Files + Header Files @@ -80,6 +83,9 @@ Source Files + + Source Files + Source Files diff --git a/src/common/interop/pipe_caller_auth.cpp b/src/common/interop/pipe_caller_auth.cpp new file mode 100644 index 0000000000..a72fc9fb03 --- /dev/null +++ b/src/common/interop/pipe_caller_auth.cpp @@ -0,0 +1,433 @@ +#include "pipe_caller_auth.h" + +#include +#include +#include + +#include +#include +#include +#include + +#pragma comment(lib, "wintrust.lib") +#pragma comment(lib, "crypt32.lib") +// Note: the file version is read via the PE resource (kernel32 only), NOT the version.dll APIs, to +// avoid a link-name collision with PowerToys' own static "Version.lib" project that the interop DLL +// references. + +namespace interop_auth +{ + namespace + { + std::wstring ToLower(std::wstring s) + { + for (auto& c : s) + { + c = static_cast(towlower(c)); + } + return s; + } + + std::wstring CanonicalizePath(const std::wstring& path) + { + // Backup semantics so this works for both files and directories. + HANDLE h = CreateFileW(path.c_str(), + 0, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, + nullptr); + if (h == INVALID_HANDLE_VALUE) + { + // Fail closed: a path we cannot open/canonicalize must not slip past the + // directory-prefix check as a non-canonical raw path. + return {}; + } + wchar_t buf[1024] = {}; + DWORD len = GetFinalPathNameByHandleW(h, buf, ARRAYSIZE(buf), FILE_NAME_NORMALIZED); + CloseHandle(h); + if (len == 0 || len >= ARRAYSIZE(buf)) + { + return {}; + } + std::wstring result(buf, len); + if (result.rfind(L"\\\\?\\", 0) == 0) + { + result.erase(0, 4); + } + return result; + } + + std::wstring BaseName(const std::wstring& path) + { + const auto pos = path.find_last_of(L"\\/"); + return pos == std::wstring::npos ? path : path.substr(pos + 1); + } + + bool PathIsUnderDirectory(const std::wstring& canonicalFile, const std::wstring& directory) + { + if (directory.empty() || canonicalFile.empty()) + { + return false; + } + std::wstring dir = ToLower(CanonicalizePath(directory)); + if (dir.empty()) + { + // CanonicalizePath failed closed; an empty prefix must not match every path. + return false; + } + if (!dir.empty() && dir.back() != L'\\') + { + dir.push_back(L'\\'); + } + const std::wstring file = ToLower(canonicalFile); + return file.size() > dir.size() && file.compare(0, dir.size(), dir) == 0; + } + + bool BasenameAllowed(const std::wstring& canonicalFile, const std::vector& allowed) + { + const std::wstring base = ToLower(BaseName(canonicalFile)); + for (const auto& a : allowed) + { + if (ToLower(a) == base) + { + return true; + } + } + return false; + } + + bool LeafIsMicrosoft(PCCERT_CONTEXT cert) + { + if (!cert) + { + return false; + } + wchar_t name[256] = {}; + const DWORD n = CertGetNameStringW(cert, CERT_NAME_SIMPLE_DISPLAY_TYPE, 0, nullptr, name, ARRAYSIZE(name)); + // Case-insensitive: cert display-name casing can vary and this is a secondary check on top + // of the machine-root Authenticode chain in ChainsToMachineRoot. + return n > 1 && wcsstr(ToLower(name).c_str(), L"microsoft corporation") != nullptr; + } + + // Anchor the signer's chain in the LOCAL MACHINE root store only (HCCE_LOCAL_MACHINE) rather than + // WinVerifyTrust's default user+machine union. The Runner runs as the same user as a potential + // attacker, so a user-writable CurrentUser\Root could otherwise forge a "Microsoft" signer; a + // non-admin cannot plant a machine root, so this defeats the forge. + bool ChainsToMachineRoot(PCCERT_CONTEXT leaf, HCERTSTORE additionalStore) + { + if (!leaf) + { + return false; + } + + char codeSigningOid[] = szOID_PKIX_KP_CODE_SIGNING; + LPSTR oids[] = { codeSigningOid }; + CERT_CHAIN_PARA para = {}; + para.cbSize = sizeof(para); + para.RequestedUsage.dwType = USAGE_MATCH_TYPE_AND; + para.RequestedUsage.Usage.cUsageIdentifier = 1; + para.RequestedUsage.Usage.rgpszUsageIdentifier = oids; + + // Cached-only revocation: never hit the network; treat "unknown/offline" as not-revoked. + const DWORD flags = CERT_CHAIN_CACHE_ONLY_URL_RETRIEVAL | + CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT; + + PCCERT_CHAIN_CONTEXT chain = nullptr; + if (!CertGetCertificateChain(HCCE_LOCAL_MACHINE, leaf, nullptr, additionalStore, ¶, flags, nullptr, &chain)) + { + return false; + } + + bool ok = false; + const DWORD ignore = CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION; + if ((chain->TrustStatus.dwErrorStatus & ~ignore) == 0) + { + CERT_CHAIN_POLICY_PARA policyPara = {}; + policyPara.cbSize = sizeof(policyPara); + CERT_CHAIN_POLICY_STATUS policyStatus = {}; + policyStatus.cbSize = sizeof(policyStatus); + if (CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_AUTHENTICODE, chain, &policyPara, &policyStatus)) + { + ok = (policyStatus.dwError == 0); + } + } + + CertFreeCertificateChain(chain); + return ok; + } + + // Establishes that the file has a valid, untampered Authenticode signature (blocks unsigned, + // tampered, and signature-stapled binaries). The *trust anchor* decision is intentionally NOT + // taken from here (it consults the default user+machine store) — ChainsToMachineRoot re-anchors + // it against the machine store. + bool HasIntactAuthenticodeSignature(const std::wstring& path) + { + WINTRUST_FILE_INFO fileInfo = {}; + fileInfo.cbStruct = sizeof(fileInfo); + fileInfo.pcwszFilePath = path.c_str(); + + WINTRUST_DATA wd = {}; + wd.cbStruct = sizeof(wd); + wd.dwUIChoice = WTD_UI_NONE; + wd.fdwRevocationChecks = WTD_REVOKE_NONE; + wd.dwUnionChoice = WTD_CHOICE_FILE; + wd.pFile = &fileInfo; + wd.dwStateAction = WTD_STATEACTION_VERIFY; + wd.dwProvFlags = WTD_SAFER_FLAG | WTD_CACHE_ONLY_URL_RETRIEVAL; + + GUID action = WINTRUST_ACTION_GENERIC_VERIFY_V2; + HWND noWindow = static_cast(INVALID_HANDLE_VALUE); + const LONG status = WinVerifyTrust(noWindow, &action, &wd); + + wd.dwStateAction = WTD_STATEACTION_CLOSE; + WinVerifyTrust(noWindow, &action, &wd); + + return status == ERROR_SUCCESS; + } + + bool VerifyMicrosoftSignedMachineRoot(const std::wstring& path) + { + // 1) Integrity + valid signature (default store). Rejects unsigned / tampered / stapled. + if (!HasIntactAuthenticodeSignature(path)) + { + return false; + } + + // 2) Re-anchor trust in the machine root store and confirm the signer leaf is Microsoft. + HCERTSTORE hStore = nullptr; + HCRYPTMSG hMsg = nullptr; + if (!CryptQueryObject(CERT_QUERY_OBJECT_FILE, + path.c_str(), + CERT_QUERY_CONTENT_FLAG_PKCS7_SIGNED_EMBED, + CERT_QUERY_FORMAT_FLAG_BINARY, + 0, + nullptr, + nullptr, + nullptr, + &hStore, + &hMsg, + nullptr)) + { + return false; + } + + bool result = false; + DWORD signerSize = 0; + if (CryptMsgGetParam(hMsg, CMSG_SIGNER_INFO_PARAM, 0, nullptr, &signerSize) && signerSize > 0) + { + std::vector signerBuf(signerSize); + if (CryptMsgGetParam(hMsg, CMSG_SIGNER_INFO_PARAM, 0, signerBuf.data(), &signerSize)) + { + auto* signer = reinterpret_cast(signerBuf.data()); + CERT_INFO certInfo = {}; + certInfo.Issuer = signer->Issuer; + certInfo.SerialNumber = signer->SerialNumber; + PCCERT_CONTEXT leaf = CertGetSubjectCertificateFromStore( + hStore, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, &certInfo); + if (leaf) + { + result = LeafIsMicrosoft(leaf) && ChainsToMachineRoot(leaf, hStore); + CertFreeCertificateContext(leaf); + } + } + } + + if (hMsg) + { + CryptMsgClose(hMsg); + } + if (hStore) + { + CertCloseStore(hStore, 0); + } + return result; + } + + // --- Per-process verification cache ------------------------------------------------------- + // The cache itself is the header-only interop_auth::VerificationCache, owned per pipe server so + // verdicts are physically partitioned by policy (key = pid + creation-time). Only the helper to + // read a process's unforgeable creation-time key lives here. + unsigned long long ProcessCreationKey(HANDLE process) + { + FILETIME create = {}, exit = {}, kernel = {}, user = {}; + if (!GetProcessTimes(process, &create, &exit, &kernel, &user)) + { + return 0; + } + return (static_cast(create.dwHighDateTime) << 32) | create.dwLowDateTime; + } + } + + unsigned long long GetModuleVersion(const std::wstring& path) + { + if (path.empty()) + { + return 0; + } + // Read the fixed file-version from the PE's RT_VERSION resource using kernel32-only APIs + // (LoadLibraryEx as a data/resource image), avoiding the version.dll import lib. + HMODULE mod = LoadLibraryExW(path.c_str(), nullptr, LOAD_LIBRARY_AS_IMAGE_RESOURCE | LOAD_LIBRARY_AS_DATAFILE); + if (!mod) + { + return 0; + } + unsigned long long result = 0; + if (HRSRC res = FindResourceW(mod, MAKEINTRESOURCEW(1 /* VS_VERSION_INFO */), RT_VERSION)) + { + if (HGLOBAL glob = LoadResource(mod, res)) + { + const void* locked = LockResource(glob); + const DWORD size = SizeofResource(mod, res); + if (locked != nullptr && size >= sizeof(VS_FIXEDFILEINFO)) + { + std::vector bytes(size); + memcpy(bytes.data(), locked, size); + for (size_t i = 0; i + sizeof(VS_FIXEDFILEINFO) <= bytes.size(); i += sizeof(DWORD)) + { + VS_FIXEDFILEINFO ffi{}; + memcpy(&ffi, &bytes[i], sizeof(ffi)); + if (ffi.dwSignature == 0xFEEF04BD) + { + result = (static_cast(ffi.dwFileVersionMS) << 32) | ffi.dwFileVersionLS; + break; + } + } + } + } + } + FreeLibrary(mod); + return result; + } + + unsigned long long GetOwnModuleVersion() + { + wchar_t self[MAX_PATH * 2] = {}; + const DWORD n = GetModuleFileNameW(nullptr, self, ARRAYSIZE(self)); + if (n == 0 || n >= ARRAYSIZE(self)) + { + return 0; + } + return GetModuleVersion(self); + } + + AuthResult AuthenticateClient(HANDLE pipe, const CallerPolicy& policy, VerificationCache& cache) + { + AuthResult res; + if (!policy.enabled) + { + res.accepted = true; + return res; + } + + ULONG pid = 0; + if (!GetNamedPipeClientProcessId(pipe, &pid)) + { + res.reasonCode = L"no-client-pid"; + return res; + } + res.pid = pid; + + if (policy.expectedClientPid.has_value() && policy.expectedClientPid.value() != pid) + { + res.reasonCode = L"pid-mismatch"; + if (policy.logReject) + { + policy.logReject(res); + } + return res; + } + + // Hold the process handle for the whole check so the PID cannot be recycled under us. + HANDLE process = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid); + if (!process) + { + res.reasonCode = L"open-process-failed"; + if (policy.logReject) + { + policy.logReject(res); + } + return res; + } + + const unsigned long long createTime = ProcessCreationKey(process); + + { + AuthResult cached; + if (cache.TryGet(pid, createTime, cached)) + { + cached.pid = pid; + CloseHandle(process); + // Do not re-log on a cache hit (dedup across the per-message connections). + return cached; + } + } + + wchar_t imageBuf[MAX_PATH * 2] = {}; + DWORD cch = ARRAYSIZE(imageBuf); + std::wstring canonical; + if (QueryFullProcessImageNameW(process, 0, imageBuf, &cch)) + { + canonical = CanonicalizePath(imageBuf); + } + res.imagePath = canonical; + + const wchar_t* reason = L""; + bool accepted = false; + + if (canonical.empty()) + { + reason = L"image-path-failed"; + } + else if (!PathIsUnderDirectory(canonical, policy.expectedDirectory)) + { + reason = L"bad-directory"; + } + else if (!BasenameAllowed(canonical, policy.allowedBasenames)) + { + reason = L"bad-basename"; + } + else if (policy.expectedVersion != 0 && GetModuleVersion(canonical) != policy.expectedVersion) + { + reason = L"version-mismatch"; + } + else + { + bool signatureOk = true; + if (policy.requireMicrosoftSignature) + { +#ifdef _DEBUG + // DEV-ONLY: local builds are not Microsoft-signed. Directory, basename and version are + // still enforced above. This relaxation is physically compiled out of Release. + signatureOk = true; +#else + signatureOk = VerifyMicrosoftSignedMachineRoot(canonical); +#endif + } + if (!signatureOk) + { + reason = L"not-microsoft-signed"; + } + else + { + accepted = true; + } + } + + CloseHandle(process); + + res.accepted = accepted; + res.reasonCode = reason; + + cache.Put(pid, createTime, res); + + // Required rejection logging (once per process instance, deduped by the cache above). + if (!accepted && policy.logReject) + { + policy.logReject(res); + } + + return res; + } +} diff --git a/src/common/interop/pipe_caller_auth.h b/src/common/interop/pipe_caller_auth.h new file mode 100644 index 0000000000..9b1e1aeed4 --- /dev/null +++ b/src/common/interop/pipe_caller_auth.h @@ -0,0 +1,150 @@ +#pragma once + +#include + +#include +#include +#include +#include +#include +#include + +// Per-connection authentication of a named-pipe client for the Runner control channel. +// +// The Runner is the *server* for the privileged Settings/Quick Access command pipes. Because a +// same-user attacker shares the connecting user's SID, integrity level, and logon session, the pipe +// DACL cannot distinguish the legitimate Settings/Quick Access child from an attacker. The only usable +// discriminator is the *binary identity* of the connecting process, so we authenticate it before any +// message is dispatched (fail-closed). See the design doc for the full rationale. +namespace interop_auth +{ + struct AuthResult + { + bool accepted = false; + DWORD pid = 0; + std::wstring imagePath; // canonical image path of the connecting process (for logging) + const wchar_t* reasonCode = L""; // static string; safe to copy/store + }; + + struct CallerPolicy + { + // When false the gate is a no-op (preserves the managed start(nullptr) server and tests). + bool enabled = false; + + // Optional exact-PID pin (v1: unset — off). + std::optional expectedClientPid; + + // Canonical directory the caller image must live under (runner-relative, e.g. + // \WinUI3Apps). Derived at runtime, not hardcoded, so it adapts to Debug/Release. + std::wstring expectedDirectory; + + // Allowed image basenames, e.g. { L"PowerToys.Settings.exe" }. + std::vector allowedBasenames; + + // Runner's own file version; caller must match exactly (anti-downgrade). 0 disables the check. + unsigned long long expectedVersion = 0; + + // Require a machine-root-anchored Microsoft Authenticode signature. Compiled out in Debug builds + // (local binaries are unsigned) while directory/basename/version stay enforced. + bool requireMicrosoftSignature = true; + + // Optional sink invoked once per rejected process instance (deduped via the per-process cache). + // The Runner supplies a lambda that logs via its own Logger; interop itself has no logger. + std::function logReject; + }; + + // Per-server verification cache. Header-only (all members inline) so it introduces NO out-of-line + // symbols: the common transport header is also compiled by the Workspaces duplicate transport, which + // does not link the auth translation unit, so an out-of-line ctor/dtor here would break its link. + // Each pipe server owns one instance, so cached verdicts are physically partitioned by policy and the + // key is simply (pid, process-creation-time). Thread-safe. + class VerificationCache + { + public: + // On a fresh (within-TTL) hit, fills out.accepted/imagePath/reasonCode and returns true. + bool TryGet(DWORD pid, unsigned long long createTime, AuthResult& out) + { + std::scoped_lock lock(m_mutex); + const auto it = m_map.find(Key{ pid, createTime }); + if (it != m_map.end() && (GetTickCount64() - it->second.tick) <= kTtlMs) + { + out.accepted = it->second.accepted; + out.imagePath = it->second.imagePath; + out.reasonCode = it->second.reason; + return true; + } + return false; + } + + // Stores/refreshes the verdict for this process instance (evicts expired/oldest entries first). + void Put(DWORD pid, unsigned long long createTime, const AuthResult& verdict) + { + std::scoped_lock lock(m_mutex); + evictLocked(); + m_map[Key{ pid, createTime }] = + Entry{ verdict.accepted, verdict.imagePath, verdict.reasonCode, GetTickCount64() }; + } + + private: + struct Key + { + DWORD pid = 0; + unsigned long long createTime = 0; + bool operator<(const Key& other) const + { + return pid < other.pid || (pid == other.pid && createTime < other.createTime); + } + }; + + struct Entry + { + bool accepted = false; + std::wstring imagePath; + const wchar_t* reason = L""; + unsigned long long tick = 0; + }; + + void evictLocked() + { + const unsigned long long now = GetTickCount64(); + for (auto it = m_map.begin(); it != m_map.end();) + { + if (now - it->second.tick > kTtlMs) + { + it = m_map.erase(it); + } + else + { + ++it; + } + } + while (m_map.size() >= kCap) + { + auto oldest = m_map.begin(); + for (auto it = m_map.begin(); it != m_map.end(); ++it) + { + if (it->second.tick < oldest->second.tick) + { + oldest = it; + } + } + m_map.erase(oldest); + } + } + + static constexpr unsigned long long kTtlMs = 60'000; // per-process verdict lifetime + static constexpr size_t kCap = 64; // small LRU cap (few legit clients) + std::mutex m_mutex; + std::map m_map; + }; + + // Authenticates the client connected on `pipe` against `policy`, using `cache` (owned by the caller, + // typically one per pipe server) to avoid re-verifying every message. Never throws. + AuthResult AuthenticateClient(HANDLE pipe, const CallerPolicy& policy, VerificationCache& cache); + + // File version packed as (VersionMS << 32) | VersionLS. Returns 0 on failure. + unsigned long long GetModuleVersion(const std::wstring& path); + + // Version of the current process's own module (e.g. the Runner). Returns 0 on failure. + unsigned long long GetOwnModuleVersion(); +} diff --git a/src/common/interop/two_way_pipe_message_ipc.cpp b/src/common/interop/two_way_pipe_message_ipc.cpp index 4b09e67626..1a716c9d6f 100644 --- a/src/common/interop/two_way_pipe_message_ipc.cpp +++ b/src/common/interop/two_way_pipe_message_ipc.cpp @@ -31,6 +31,11 @@ void TwoWayPipeMessageIPC::start(HANDLE _restricted_pipe_token) impl->start(_restricted_pipe_token); } +void TwoWayPipeMessageIPC::start(HANDLE _restricted_pipe_token, const interop_auth::CallerPolicy& caller_policy) +{ + impl->start(_restricted_pipe_token, caller_policy); +} + void TwoWayPipeMessageIPC::end() { impl->end(); @@ -53,6 +58,18 @@ void TwoWayPipeMessageIPC::TwoWayPipeMessageIPCImpl::send(std::wstring msg) void TwoWayPipeMessageIPC::TwoWayPipeMessageIPCImpl::start(HANDLE _restricted_pipe_token) { + // Legacy overload = no caller authentication: explicitly clear any previously-set policy so this + // path can never inherit a policy from a prior parameterized start on the same instance. + caller_policy = {}; + output_queue_thread = std::thread(&TwoWayPipeMessageIPCImpl::consume_output_queue_thread, this); + input_queue_thread = std::thread(&TwoWayPipeMessageIPCImpl::consume_input_queue_thread, this); + input_pipe_thread = std::thread(&TwoWayPipeMessageIPCImpl::start_named_pipe_server, this, _restricted_pipe_token); +} + +void TwoWayPipeMessageIPC::TwoWayPipeMessageIPCImpl::start(HANDLE _restricted_pipe_token, const interop_auth::CallerPolicy& _caller_policy) +{ + // Start threads inline (do not chain into the legacy overload, which would clear the policy). + caller_policy = _caller_policy; output_queue_thread = std::thread(&TwoWayPipeMessageIPCImpl::consume_output_queue_thread, this); input_queue_thread = std::thread(&TwoWayPipeMessageIPCImpl::consume_input_queue_thread, this); input_pipe_thread = std::thread(&TwoWayPipeMessageIPCImpl::start_named_pipe_server, this, _restricted_pipe_token); @@ -353,6 +370,21 @@ void TwoWayPipeMessageIPC::TwoWayPipeMessageIPCImpl::handle_pipe_connection(HAND { return; } + + // Authenticate the connecting client before reading/queuing anything. Fail-closed: an unauthenticated + // caller gets no dispatch. When the policy is disabled (managed server / tests) this is a no-op. + if (caller_policy.enabled) + { + const interop_auth::AuthResult auth = interop_auth::AuthenticateClient(input_pipe_handle, caller_policy, caller_cache); + if (!auth.accepted) + { + FlushFileBuffers(input_pipe_handle); + DisconnectNamedPipe(input_pipe_handle); + CloseHandle(input_pipe_handle); + return; + } + } + constexpr DWORD readBlockBytes = BUFSIZE; std::wstring message; size_t iBlock = 0; @@ -411,7 +443,8 @@ void TwoWayPipeMessageIPC::TwoWayPipeMessageIPCImpl::start_named_pipe_server(HAN WRITE_DAC, PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | - PIPE_WAIT, + PIPE_WAIT | + PIPE_REJECT_REMOTE_CLIENTS, PIPE_UNLIMITED_INSTANCES, BUFSIZE, BUFSIZE, diff --git a/src/common/interop/two_way_pipe_message_ipc.h b/src/common/interop/two_way_pipe_message_ipc.h index 3844c835c4..96e467ff7f 100644 --- a/src/common/interop/two_way_pipe_message_ipc.h +++ b/src/common/interop/two_way_pipe_message_ipc.h @@ -1,5 +1,6 @@ #pragma once #include +#include "pipe_caller_auth.h" class TwoWayPipeMessageIPC { public: @@ -11,6 +12,9 @@ public: ~TwoWayPipeMessageIPC(); void send(std::wstring msg); void start(HANDLE _restricted_pipe_token); + // Overload that authenticates every connecting client before dispatch (fail-closed). Used by the + // Runner for its privileged server pipes; the existing start(HANDLE) keeps the gate disabled. + void start(HANDLE _restricted_pipe_token, const interop_auth::CallerPolicy& caller_policy); void end(); private: diff --git a/src/common/interop/two_way_pipe_message_ipc_impl.h b/src/common/interop/two_way_pipe_message_ipc_impl.h index d74c99564b..495bb95758 100644 --- a/src/common/interop/two_way_pipe_message_ipc_impl.h +++ b/src/common/interop/two_way_pipe_message_ipc_impl.h @@ -13,6 +13,7 @@ public: void send(std::wstring msg); TwoWayPipeMessageIPCImpl(std::wstring _input_pipe_name, std::wstring _output_pipe_name, callback_function p_func); void start(HANDLE _restricted_pipe_token); + void start(HANDLE _restricted_pipe_token, const interop_auth::CallerPolicy& _caller_policy); void end(); private: @@ -29,6 +30,8 @@ private: HANDLE current_connect_pipe_handle = NULL; bool closed = false; TwoWayPipeMessageIPC::callback_function dispatch_inc_message_function; + interop_auth::CallerPolicy caller_policy; + interop_auth::VerificationCache caller_cache; void send_pipe_message(std::wstring message); void consume_output_queue_thread(); diff --git a/src/runner/quick_access_host.cpp b/src/runner/quick_access_host.cpp index b546ee244e..882be9fbb9 100644 --- a/src/runner/quick_access_host.cpp +++ b/src/runner/quick_access_host.cpp @@ -184,7 +184,19 @@ namespace QuickAccessHost try { - quick_access_ipc->start(token.get()); + interop_auth::CallerPolicy qa_caller_policy; + qa_caller_policy.enabled = true; + qa_caller_policy.expectedDirectory = get_module_folderpath() + L"\\WinUI3Apps"; + qa_caller_policy.allowedBasenames = { L"PowerToys.QuickAccess.exe" }; + qa_caller_policy.expectedVersion = interop_auth::GetOwnModuleVersion(); + qa_caller_policy.requireMicrosoftSignature = true; + qa_caller_policy.logReject = [](const interop_auth::AuthResult& r) { + Logger::warn(L"Rejected unauthenticated Quick Access pipe client: pid={} image='{}' reason={}", + r.pid, + r.imagePath, + r.reasonCode); + }; + quick_access_ipc->start(token.get(), qa_caller_policy); } catch (...) { diff --git a/src/runner/runner.vcxproj b/src/runner/runner.vcxproj index 34eeefdfc2..e5b89c5748 100644 --- a/src/runner/runner.vcxproj +++ b/src/runner/runner.vcxproj @@ -59,6 +59,9 @@ + + NotUsing + diff --git a/src/runner/settings_window.cpp b/src/runner/settings_window.cpp index 2b42598efb..8c95b0f99f 100644 --- a/src/runner/settings_window.cpp +++ b/src/runner/settings_window.cpp @@ -582,7 +582,24 @@ void run_settings_window(bool show_oobe_window, bool show_scoobe_window, std::op { std::unique_lock lock{ ipc_mutex }; current_settings_ipc = new TwoWayPipeMessageIPC(powertoys_pipe_name, settings_pipe_name, receive_json_send_to_main_thread); - current_settings_ipc->start(hToken); + + // Authenticate the connecting client (Settings) before dispatching any privileged command. + // The expected image directory is derived from the Runner's own module folder so it adapts to + // both installed and dev-build layouts; only Microsoft-signed PowerToys.Settings.exe at the + // Runner's own version is accepted (signature check is compiled out in Debug). + interop_auth::CallerPolicy settings_caller_policy; + settings_caller_policy.enabled = true; + settings_caller_policy.expectedDirectory = get_module_folderpath() + L"\\WinUI3Apps"; + settings_caller_policy.allowedBasenames = { L"PowerToys.Settings.exe" }; + settings_caller_policy.expectedVersion = interop_auth::GetOwnModuleVersion(); + settings_caller_policy.requireMicrosoftSignature = true; + settings_caller_policy.logReject = [](const interop_auth::AuthResult& r) { + Logger::warn(L"Rejected unauthenticated Settings pipe client: pid={} image='{}' reason={}", + r.pid, + r.imagePath, + r.reasonCode); + }; + current_settings_ipc->start(hToken, settings_caller_policy); // Register callback for bug report status changes BugReportManager::instance().register_callback([](bool isRunning) {