Skip to content

Commit 50b93fb

Browse files
author
Ben Hillis
committed
Prevent session name squatting for default WSLc sessions
- Server now determines default session name and settings from caller's token, preventing malicious users from squatting reserved session names - CreateSession rejects explicit use of reserved names (wslc-cli, wslc-cli-admin) with E_ACCESSDENIED - Null StoragePath remains valid for ephemeral sessions; empty string is rejected as E_INVALIDARG - Add dedicated EnterSession API for custom sessions - Extract UserSettings into shared wslcsettings library used by both wslc.exe and wslservice.exe - Move EnumVariantMap.h to common - wslc.exe no longer parses settings.yaml; service handles all config - Fix std::terminate crash in CustomDmesgOutput test when CreateSession fails by adding a scope_exit guard to join the reader thread - Add session name squatting E2E test
1 parent 4f16583 commit 50b93fb

25 files changed

+378
-207
lines changed

localization/strings/en-US/Resources.resw

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1935,6 +1935,15 @@ Usage:
19351935
<value>Session termination failed: '{}'</value>
19361936
<comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
19371937
</data>
1938+
<data name="MessageWslcDefaultSessionNotFound" xml:space="preserve">
1939+
<value>Default session not found</value>
1940+
</data>
1941+
<data name="MessageWslcOpenDefaultSessionFailed" xml:space="preserve">
1942+
<value>Failed to open default session</value>
1943+
</data>
1944+
<data name="MessageWslcTerminateDefaultSessionFailed" xml:space="preserve">
1945+
<value>Default session termination failed</value>
1946+
</data>
19381947
<data name="MessageWslcShellExited" xml:space="preserve">
19391948
<value>{} exited with: {}</value>
19401949
<comment>{FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>

src/windows/common/CMakeLists.txt

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ set(SOURCES
4949
WslTelemetry.cpp
5050
wslutil.cpp
5151
install.cpp
52+
WSLCUserSettings.cpp
5253
)
5354

5455
set(HEADERS
@@ -130,11 +131,19 @@ set(HEADERS
130131
WslSecurity.h
131132
WslTelemetry.h
132133
wslutil.h
134+
EnumVariantMap.h
135+
WSLCUserSettings.h
136+
WSLCSessionDefaults.h
133137
)
134138

135139
add_library(common STATIC ${SOURCES} ${HEADERS})
136-
add_dependencies(common wslserviceidl localization wslservicemc wslinstalleridl)
140+
add_dependencies(common wslserviceidl localization wslservicemc wslinstalleridl yaml-cpp)
137141

138142
target_precompile_headers(common PRIVATE precomp.h)
139143
set_target_properties(common PROPERTIES FOLDER windows)
140144
target_include_directories(common PUBLIC ${CMAKE_CURRENT_BINARY_DIR}/../service/mc/${TARGET_PLATFORM}/${CMAKE_BUILD_TYPE})
145+
146+
# WSLCUserSettings.cpp uses yaml-cpp headers.
147+
set_source_files_properties(WSLCUserSettings.cpp PROPERTIES
148+
INCLUDE_DIRECTORIES "${yaml-cpp_SOURCE_DIR}/include"
149+
COMPILE_DEFINITIONS "YAML_CPP_STATIC_DEFINE")
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*++
2+
3+
Copyright (c) Microsoft. All rights reserved.
4+
5+
Module Name:
6+
7+
WSLCSessionDefaults.h
8+
9+
Abstract:
10+
11+
Shared constants for WSLc session naming and storage.
12+
13+
--*/
14+
#pragma once
15+
16+
#include <cstdint>
17+
18+
namespace wsl::windows::wslc {
19+
20+
inline constexpr const wchar_t DefaultSessionName[] = L"wslc-cli";
21+
inline constexpr const wchar_t DefaultAdminSessionName[] = L"wslc-cli-admin";
22+
inline constexpr const wchar_t DefaultStorageSubPath[] = L"wslc\\sessions";
23+
inline constexpr uint32_t DefaultBootTimeoutMs = 30000;
24+
25+
} // namespace wsl::windows::wslc

src/windows/wslc/settings/UserSettings.cpp renamed to src/windows/common/WSLCUserSettings.cpp

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,23 @@ Copyright (c) Microsoft. All rights reserved.
44
55
Module Name:
66
7-
UserSettings.cpp
7+
WSLCUserSettings.cpp
88
99
Abstract:
1010
1111
Implementation of UserSettings — YAML loading and validation.
1212
1313
--*/
14-
#include "UserSettings.h"
14+
#include "precomp.h"
15+
#include "WSLCUserSettings.h"
1516
#include "filesystem.hpp"
1617
#include "string.hpp"
1718
#include "wslutil.h"
19+
20+
#pragma warning(push)
21+
#pragma warning(disable : 4251 4275)
1822
#include <yaml-cpp/yaml.h>
23+
#pragma warning(pop)
1924
#include <algorithm>
2025
#include <format>
2126
#include <fstream>
@@ -25,7 +30,6 @@ using namespace wsl::windows::common::string;
2530

2631
namespace wsl::windows::wslc::settings {
2732

28-
// Default settings file template — written on first run.
2933
// All entries are commented out; the values shown are the built-in defaults.
3034
// TODO: localization for comments needed?
3135
static constexpr std::string_view s_DefaultSettingsTemplate =
@@ -239,6 +243,22 @@ UserSettings const& UserSettings::Instance()
239243
return instance;
240244
}
241245

246+
WSLCFeatureFlags UserSettings::BuildFeatureFlags() const
247+
{
248+
WSLCFeatureFlags flags = WslcFeatureFlagsNone;
249+
if (Get<Setting::SessionHostFileShareMode>() == HostFileShareMode::VirtioFs)
250+
{
251+
WI_SetFlag(flags, WslcFeatureFlagsVirtioFs);
252+
}
253+
254+
if (Get<Setting::SessionDnsTunneling>())
255+
{
256+
WI_SetFlag(flags, WslcFeatureFlagsDnsTunneling);
257+
}
258+
259+
return flags;
260+
}
261+
242262
UserSettings::UserSettings() : UserSettings(SettingsDir())
243263
{
244264
}
Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ Copyright (c) Microsoft. All rights reserved.
44
55
Module Name:
66
7-
UserSettings.h
7+
WSLCUserSettings.h
88
99
Abstract:
1010
@@ -137,6 +137,9 @@ class UserSettings
137137
return m_settings.GetOrDefault<S>();
138138
}
139139

140+
// Builds WSLCFeatureFlags from the current settings.
141+
WSLCFeatureFlags BuildFeatureFlags() const;
142+
140143
std::vector<Warning> const& GetWarnings() const
141144
{
142145
return m_warnings;
@@ -156,9 +159,7 @@ class UserSettings
156159
// Overwrites the settings file with the commented-out defaults template.
157160
void Reset() const;
158161

159-
protected:
160-
// Loads settings from an explicit directory. Used by the singleton (via
161-
// the private zero-arg constructor) and by test subclasses.
162+
// Loads settings from an explicit directory.
162163
explicit UserSettings(const std::filesystem::path& settingsDir);
163164
~UserSettings() = default;
164165

src/windows/service/exe/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ target_link_libraries(wslservice
7171
legacy_stdio_definitions
7272
VirtDisk.lib
7373
Winhttp.lib
74-
Synchronization.lib)
74+
Synchronization.lib
75+
yaml-cpp)
7576

7677
target_precompile_headers(wslservice REUSE_FROM common)
7778
set_target_properties(wslservice PROPERTIES FOLDER windows)

src/windows/service/exe/WSLCSessionManager.cpp

Lines changed: 127 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,71 @@ Module Name:
2929

3030
#include "WSLCSessionManager.h"
3131
#include "HcsVirtualMachine.h"
32+
#include "WSLCUserSettings.h"
33+
#include "WSLCSessionDefaults.h"
3234
#include "wslutil.h"
35+
#include "filesystem.hpp"
3336

3437
using wsl::windows::service::wslc::CallingProcessTokenInfo;
3538
using wsl::windows::service::wslc::HcsVirtualMachine;
3639
using wsl::windows::service::wslc::WSLCSessionManagerImpl;
3740
namespace wslutil = wsl::windows::common::wslutil;
41+
namespace settings = wsl::windows::wslc::settings;
42+
43+
namespace {
44+
45+
// Session settings built server-side from the caller's settings.yaml.
46+
struct SessionSettings
47+
{
48+
std::wstring DisplayName;
49+
std::wstring StoragePath;
50+
WSLCSessionSettings Settings{};
51+
52+
NON_COPYABLE(SessionSettings);
53+
NON_MOVABLE(SessionSettings);
54+
55+
// Default session: name and storage path determined from caller's token.
56+
static std::unique_ptr<SessionSettings> Default(HANDLE UserToken, bool Elevated)
57+
{
58+
auto localAppData = wsl::windows::common::filesystem::GetLocalAppDataPath(UserToken);
59+
settings::UserSettings userSettings(localAppData / L"wslc");
60+
61+
std::wstring name = Elevated ? wsl::windows::wslc::DefaultAdminSessionName : wsl::windows::wslc::DefaultSessionName;
62+
63+
auto customPath = userSettings.Get<settings::Setting::SessionStoragePath>();
64+
std::filesystem::path basePath =
65+
customPath.empty() ? (localAppData / wsl::windows::wslc::DefaultStorageSubPath) : std::filesystem::path{customPath};
66+
auto storagePath = (basePath / name).wstring();
67+
68+
return std::unique_ptr<SessionSettings>(
69+
new SessionSettings(std::move(name), std::move(storagePath), WSLCSessionStorageFlagsNone, userSettings));
70+
}
71+
72+
// Custom session: caller provides name and storage path.
73+
static SessionSettings Custom(HANDLE UserToken, LPCWSTR Name, LPCWSTR Path, WSLCSessionStorageFlags StorageFlags = WSLCSessionStorageFlagsNone)
74+
{
75+
auto localAppData = wsl::windows::common::filesystem::GetLocalAppDataPath(UserToken);
76+
settings::UserSettings userSettings(localAppData / L"wslc");
77+
return SessionSettings(Name, Path, StorageFlags, userSettings);
78+
}
79+
80+
private:
81+
SessionSettings(std::wstring name, std::wstring path, WSLCSessionStorageFlags storageFlags, const settings::UserSettings& userSettings) :
82+
DisplayName(std::move(name)), StoragePath(std::move(path))
83+
{
84+
Settings.DisplayName = DisplayName.c_str();
85+
Settings.StoragePath = StoragePath.c_str();
86+
Settings.CpuCount = userSettings.Get<settings::Setting::SessionCpuCount>();
87+
Settings.MemoryMb = userSettings.Get<settings::Setting::SessionMemoryMb>();
88+
Settings.MaximumStorageSizeMb = userSettings.Get<settings::Setting::SessionStorageSizeMb>();
89+
Settings.BootTimeoutMs = wsl::windows::wslc::DefaultBootTimeoutMs;
90+
Settings.NetworkingMode = userSettings.Get<settings::Setting::SessionNetworkingMode>();
91+
Settings.FeatureFlags = userSettings.BuildFeatureFlags();
92+
Settings.StorageFlags = storageFlags;
93+
}
94+
};
95+
96+
} // namespace
3897

3998
WSLCSessionManagerImpl::~WSLCSessionManagerImpl()
4099
{
@@ -51,22 +110,42 @@ WSLCSessionManagerImpl::~WSLCSessionManagerImpl()
51110

52111
void WSLCSessionManagerImpl::CreateSession(const WSLCSessionSettings* Settings, WSLCSessionFlags Flags, IWSLCSession** WslcSession)
53112
{
54-
// Ensure that the session display name is non-null and not too long.
55-
THROW_HR_IF(E_INVALIDARG, Settings->DisplayName == nullptr);
56-
THROW_HR_IF(E_INVALIDARG, wcslen(Settings->DisplayName) >= std::size(WSLCSessionInformation{}.DisplayName));
57-
THROW_HR_IF_MSG(
58-
E_INVALIDARG,
59-
WI_IsAnyFlagSet(Settings->StorageFlags, ~WSLCSessionStorageFlagsValid),
60-
"Invalid storage flags: %i",
61-
Settings->StorageFlags);
62-
63113
auto tokenInfo = GetCallingProcessTokenInfo();
114+
const auto callerToken = wsl::windows::common::security::GetUserToken(TokenImpersonation);
115+
116+
// Resolve display name upfront (for both default and custom sessions).
117+
std::wstring resolvedDisplayName;
118+
if (Settings == nullptr)
119+
{
120+
// Default session: name determined from token.
121+
resolvedDisplayName = tokenInfo.Elevated ? wsl::windows::wslc::DefaultAdminSessionName : wsl::windows::wslc::DefaultSessionName;
122+
Flags = WSLCSessionFlagsOpenExisting | WSLCSessionFlagsPersistent;
123+
}
124+
else
125+
{
126+
THROW_HR_IF(E_INVALIDARG, Settings->DisplayName == nullptr || wcslen(Settings->DisplayName) == 0);
127+
THROW_HR_IF(E_INVALIDARG, Settings->StoragePath != nullptr && wcslen(Settings->StoragePath) == 0);
128+
THROW_HR_IF(E_INVALIDARG, wcslen(Settings->DisplayName) >= std::size(WSLCSessionInformation{}.DisplayName));
129+
THROW_HR_IF_MSG(
130+
E_INVALIDARG,
131+
WI_IsAnyFlagSet(Settings->StorageFlags, ~WSLCSessionStorageFlagsValid),
132+
"Invalid storage flags: %i",
133+
Settings->StorageFlags);
134+
135+
// Reserved names can only be assigned server-side via null Settings.
136+
THROW_HR_IF(
137+
E_ACCESSDENIED,
138+
wsl::shared::string::IsEqual(Settings->DisplayName, wsl::windows::wslc::DefaultSessionName, true) ||
139+
wsl::shared::string::IsEqual(Settings->DisplayName, wsl::windows::wslc::DefaultAdminSessionName, true));
140+
141+
resolvedDisplayName = Settings->DisplayName;
142+
}
64143

65144
std::lock_guard lock(m_wslcSessionsLock);
66145

67146
// Check for an existing session first.
68147
auto result = ForEachSession<HRESULT>([&](auto& entry, const wil::com_ptr<IWSLCSession>& session) noexcept -> std::optional<HRESULT> {
69-
if (!wsl::shared::string::IsEqual(entry.DisplayName.c_str(), Settings->DisplayName))
148+
if (!wsl::shared::string::IsEqual(entry.DisplayName.c_str(), resolvedDisplayName.c_str()))
70149
{
71150
return {};
72151
}
@@ -88,6 +167,14 @@ void WSLCSessionManagerImpl::CreateSession(const WSLCSessionSettings* Settings,
88167

89168
wslutil::StopWatch stopWatch;
90169

170+
// Initialize settings for the default session.
171+
std::unique_ptr<SessionSettings> defaultSettings;
172+
if (Settings == nullptr)
173+
{
174+
defaultSettings = SessionSettings::Default(callerToken.get(), tokenInfo.Elevated);
175+
Settings = &defaultSettings->Settings;
176+
}
177+
91178
HRESULT creationResult = wil::ResultFromException([&]() {
92179
// Get caller info.
93180
const auto callerProcess = wslutil::OpenCallingProcess(PROCESS_QUERY_LIMITED_INFORMATION);
@@ -103,13 +190,13 @@ void WSLCSessionManagerImpl::CreateSession(const WSLCSessionSettings* Settings,
103190
AddSessionProcessToJobObject(factory.get());
104191

105192
// Create the session via the factory.
106-
const auto sessionSettings = CreateSessionSettings(sessionId, creatorPid, Settings);
193+
const auto sessionSettings = CreateSessionSettings(sessionId, creatorPid, Settings, resolvedDisplayName.c_str());
107194
wil::com_ptr<IWSLCSession> session;
108195
wil::com_ptr<IWSLCSessionReference> serviceRef;
109196
THROW_IF_FAILED(factory->CreateSession(&sessionSettings, vm.Get(), &session, &serviceRef));
110197

111198
// Track the session via its service ref, along with metadata and security info.
112-
m_sessions.push_back({std::move(serviceRef), sessionId, creatorPid, Settings->DisplayName, std::move(tokenInfo)});
199+
m_sessions.push_back({std::move(serviceRef), sessionId, creatorPid, resolvedDisplayName, std::move(tokenInfo)});
113200

114201
// For persistent sessions, also hold a strong reference to keep them alive.
115202
const bool persistent = WI_IsFlagSet(Flags, WSLCSessionFlagsPersistent);
@@ -122,19 +209,18 @@ void WSLCSessionManagerImpl::CreateSession(const WSLCSessionSettings* Settings,
122209
});
123210

124211
// This telemetry event is used to keep track of session creation performance (via CreationTimeMs) and failure reasons (via Result).
125-
126212
WSL_LOG_TELEMETRY(
127213
"WSLCCreateSession",
128214
PDT_ProductAndServicePerformance,
129215
TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA),
130-
TraceLoggingValue(Settings->DisplayName, "Name"),
216+
TraceLoggingValue(resolvedDisplayName.c_str(), "Name"),
131217
TraceLoggingValue(stopWatch.ElapsedMilliseconds(), "CreationTimeMs"),
132218
TraceLoggingValue(creationResult, "Result"),
133219
TraceLoggingValue(tokenInfo.Elevated, "Elevated"),
134220
TraceLoggingValue(static_cast<uint32_t>(Flags), "Flags"),
135221
TraceLoggingLevel(WINEVENT_LEVEL_INFO));
136222

137-
THROW_IF_FAILED_MSG(creationResult, "Failed to create session: %ls", Settings->DisplayName);
223+
THROW_IF_FAILED_MSG(creationResult, "Failed to create session: %ls", resolvedDisplayName.c_str());
138224
}
139225

140226
void WSLCSessionManagerImpl::OpenSession(ULONG Id, IWSLCSession** Session)
@@ -160,6 +246,14 @@ void WSLCSessionManagerImpl::OpenSessionByName(LPCWSTR DisplayName, IWSLCSession
160246
{
161247
auto tokenInfo = GetCallingProcessTokenInfo();
162248

249+
// Null name = default session, resolved from caller's token.
250+
std::wstring resolvedName;
251+
if (DisplayName == nullptr)
252+
{
253+
resolvedName = tokenInfo.Elevated ? wsl::windows::wslc::DefaultAdminSessionName : wsl::windows::wslc::DefaultSessionName;
254+
DisplayName = resolvedName.c_str();
255+
}
256+
163257
auto result = ForEachSession<HRESULT>([&](auto& entry, const wil::com_ptr<IWSLCSession>& session) noexcept -> std::optional<HRESULT> {
164258
if (!wsl::shared::string::IsEqual(entry.DisplayName.c_str(), DisplayName))
165259
{
@@ -207,12 +301,23 @@ void WSLCSessionManagerImpl::GetVersion(_Out_ WSLCVersion* Version)
207301
Version->Revision = WSL_PACKAGE_VERSION_REVISION;
208302
}
209303

210-
WSLCSessionInitSettings WSLCSessionManagerImpl::CreateSessionSettings(_In_ ULONG SessionId, _In_ DWORD CreatorPid, _In_ const WSLCSessionSettings* Settings)
304+
void WSLCSessionManagerImpl::EnterSession(_In_ LPCWSTR DisplayName, _In_ LPCWSTR StoragePath, IWSLCSession** WslcSession)
305+
{
306+
THROW_HR_IF(E_POINTER, DisplayName == nullptr || StoragePath == nullptr);
307+
THROW_HR_IF(E_INVALIDARG, DisplayName[0] == L'\0' || StoragePath[0] == L'\0');
308+
309+
const auto callerToken = wsl::windows::common::security::GetUserToken(TokenImpersonation);
310+
auto sessionSettings = SessionSettings::Custom(callerToken.get(), DisplayName, StoragePath, WSLCSessionStorageFlagsNoCreate);
311+
CreateSession(&sessionSettings.Settings, WSLCSessionFlagsNone, WslcSession);
312+
}
313+
314+
WSLCSessionInitSettings WSLCSessionManagerImpl::CreateSessionSettings(
315+
_In_ ULONG SessionId, _In_ DWORD CreatorPid, _In_ const WSLCSessionSettings* Settings, _In_ LPCWSTR ResolvedDisplayName)
211316
{
212317
WSLCSessionInitSettings sessionSettings{};
213318
sessionSettings.SessionId = SessionId;
214319
sessionSettings.CreatorPid = CreatorPid;
215-
sessionSettings.DisplayName = Settings->DisplayName;
320+
sessionSettings.DisplayName = ResolvedDisplayName;
216321
sessionSettings.StoragePath = Settings->StoragePath;
217322
sessionSettings.MaximumStorageSizeMb = Settings->MaximumStorageSizeMb;
218323
sessionSettings.BootTimeoutMs = Settings->BootTimeoutMs;
@@ -292,6 +397,11 @@ HRESULT WSLCSessionManager::CreateSession(const WSLCSessionSettings* WslcSession
292397
return CallImpl(&WSLCSessionManagerImpl::CreateSession, WslcSessionSettings, Flags, WslcSession);
293398
}
294399

400+
HRESULT WSLCSessionManager::EnterSession(_In_ LPCWSTR DisplayName, _In_ LPCWSTR StoragePath, IWSLCSession** WslcSession)
401+
{
402+
return CallImpl(&WSLCSessionManagerImpl::EnterSession, DisplayName, StoragePath, WslcSession);
403+
}
404+
295405
HRESULT WSLCSessionManager::ListSessions(_Out_ WSLCSessionInformation** Sessions, _Out_ ULONG* SessionsCount)
296406
{
297407
return CallImpl(&WSLCSessionManagerImpl::ListSessions, Sessions, SessionsCount);

0 commit comments

Comments
 (0)