Skip to content

Commit 8d63613

Browse files
Ben HillisCopilot
andcommitted
Address PR feedback: per-user session names, custom error codes, impersonation
- Append username to default session names so different users don't collide (e.g. wslc-cli-alice, wslc-cli-admin-bob) [OneBlue] - Impersonate caller when loading settings.yaml server-side [OneBlue] - Factor name resolution into ResolveDefaultSessionName helper [OneBlue] - Add WSLC_E_SESSION_RESERVED and WSLC_E_INVALID_SESSION_NAME error codes for better diagnosability [dkbennett] - Use prefix-based reserved name check (blocks all wslc-cli-* names) - Fix pre-existing HostFileShareMode namespace qualification bug - Remove unused variable in GetDefaultStoragePath test helper [Copilot] - Update all E2E tests for username-qualified session names Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9cad036 commit 8d63613

File tree

8 files changed

+154
-59
lines changed

8 files changed

+154
-59
lines changed

src/windows/common/wslutil.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ static const std::map<HRESULT, LPCWSTR> g_commonErrors{
153153
X(WSLC_E_VOLUME_NOT_FOUND),
154154
X(WSLC_E_CONTAINER_NOT_RUNNING),
155155
X(WSLC_E_CONTAINER_IS_RUNNING),
156+
X(WSLC_E_SESSION_RESERVED),
157+
X(WSLC_E_INVALID_SESSION_NAME),
156158
X_WIN32(RPC_S_SERVER_UNAVAILABLE),
157159
X_WIN32(ERROR_ELEVATION_REQUIRED)};
158160

src/windows/service/exe/WSLCSessionManager.cpp

Lines changed: 59 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -52,28 +52,33 @@ struct SessionSettings
5252
NON_COPYABLE(SessionSettings);
5353
NON_MOVABLE(SessionSettings);
5454

55-
// Default session: name and storage path determined from caller's token.
56-
static std::unique_ptr<SessionSettings> Default(HANDLE UserToken, bool Elevated)
55+
// Load user settings under impersonation.
56+
static settings::UserSettings LoadUserSettings(HANDLE UserToken)
5757
{
5858
auto localAppData = wsl::windows::common::filesystem::GetLocalAppDataPath(UserToken);
59-
settings::UserSettings userSettings(localAppData / L"wslc");
59+
auto runAsUser = wil::impersonate_token(UserToken);
60+
return settings::UserSettings(localAppData / L"wslc");
61+
}
6062

61-
std::wstring name = Elevated ? wsl::windows::wslc::DefaultAdminSessionName : wsl::windows::wslc::DefaultSessionName;
63+
// Default session: name and storage path determined from caller's token.
64+
static std::unique_ptr<SessionSettings> Default(HANDLE UserToken, bool Elevated, const std::wstring& ResolvedName)
65+
{
66+
auto userSettings = LoadUserSettings(UserToken);
67+
auto localAppData = wsl::windows::common::filesystem::GetLocalAppDataPath(UserToken);
6268

6369
auto customPath = userSettings.Get<settings::Setting::SessionStoragePath>();
6470
std::filesystem::path basePath =
6571
customPath.empty() ? (localAppData / wsl::windows::wslc::DefaultStorageSubPath) : std::filesystem::path{customPath};
66-
auto storagePath = (basePath / name).wstring();
72+
auto storagePath = (basePath / ResolvedName).wstring();
6773

6874
return std::unique_ptr<SessionSettings>(
69-
new SessionSettings(std::move(name), std::move(storagePath), WSLCSessionStorageFlagsNone, userSettings));
75+
new SessionSettings(std::wstring(ResolvedName), std::move(storagePath), WSLCSessionStorageFlagsNone, userSettings));
7076
}
7177

7278
// Custom session: caller provides name and storage path.
7379
static SessionSettings Custom(HANDLE UserToken, LPCWSTR Name, LPCWSTR Path, WSLCSessionStorageFlags StorageFlags = WSLCSessionStorageFlagsNone)
7480
{
75-
auto localAppData = wsl::windows::common::filesystem::GetLocalAppDataPath(UserToken);
76-
settings::UserSettings userSettings(localAppData / L"wslc");
81+
auto userSettings = LoadUserSettings(UserToken);
7782
return SessionSettings(Name, Path, StorageFlags, userSettings);
7883
}
7984

@@ -90,7 +95,10 @@ struct SessionSettings
9095
Settings.NetworkingMode = userSettings.Get<settings::Setting::SessionNetworkingMode>();
9196
Settings.FeatureFlags = WslcFeatureFlagsNone;
9297
WI_SetFlagIf(Settings.FeatureFlags, WslcFeatureFlagsDnsTunneling, userSettings.Get<settings::Setting::SessionDnsTunneling>());
93-
WI_SetFlagIf(Settings.FeatureFlags, WslcFeatureFlagsVirtioFs, userSettings.Get<settings::Setting::SessionHostFileShareMode>() == HostFileShareMode::VirtioFs);
98+
WI_SetFlagIf(
99+
Settings.FeatureFlags,
100+
WslcFeatureFlagsVirtioFs,
101+
userSettings.Get<settings::Setting::SessionHostFileShareMode>() == settings::HostFileShareMode::VirtioFs);
94102
Settings.StorageFlags = storageFlags;
95103
}
96104
};
@@ -119,26 +127,23 @@ void WSLCSessionManagerImpl::CreateSession(const WSLCSessionSettings* Settings,
119127
std::wstring resolvedDisplayName;
120128
if (Settings == nullptr)
121129
{
122-
// Default session: name determined from token.
123-
resolvedDisplayName = tokenInfo.Elevated ? wsl::windows::wslc::DefaultAdminSessionName : wsl::windows::wslc::DefaultSessionName;
130+
// Default session: name determined from token, qualified with username.
131+
resolvedDisplayName = ResolveDefaultSessionName(tokenInfo);
124132
Flags = WSLCSessionFlagsOpenExisting | WSLCSessionFlagsPersistent;
125133
}
126134
else
127135
{
128-
THROW_HR_IF(E_INVALIDARG, Settings->DisplayName == nullptr || wcslen(Settings->DisplayName) == 0);
136+
THROW_HR_IF(WSLC_E_INVALID_SESSION_NAME, Settings->DisplayName == nullptr || wcslen(Settings->DisplayName) == 0);
129137
THROW_HR_IF(E_INVALIDARG, Settings->StoragePath != nullptr && wcslen(Settings->StoragePath) == 0);
130-
THROW_HR_IF(E_INVALIDARG, wcslen(Settings->DisplayName) >= std::size(WSLCSessionInformation{}.DisplayName));
138+
THROW_HR_IF(WSLC_E_INVALID_SESSION_NAME, wcslen(Settings->DisplayName) >= std::size(WSLCSessionInformation{}.DisplayName));
131139
THROW_HR_IF_MSG(
132140
E_INVALIDARG,
133141
WI_IsAnyFlagSet(Settings->StorageFlags, ~WSLCSessionStorageFlagsValid),
134142
"Invalid storage flags: %i",
135143
Settings->StorageFlags);
136144

137145
// Reserved names can only be assigned server-side via null Settings.
138-
THROW_HR_IF(
139-
E_ACCESSDENIED,
140-
wsl::shared::string::IsEqual(Settings->DisplayName, wsl::windows::wslc::DefaultSessionName, true) ||
141-
wsl::shared::string::IsEqual(Settings->DisplayName, wsl::windows::wslc::DefaultAdminSessionName, true));
146+
THROW_HR_IF(WSLC_E_SESSION_RESERVED, IsReservedSessionName(Settings->DisplayName));
142147

143148
resolvedDisplayName = Settings->DisplayName;
144149
}
@@ -173,7 +178,7 @@ void WSLCSessionManagerImpl::CreateSession(const WSLCSessionSettings* Settings,
173178
std::unique_ptr<SessionSettings> defaultSettings;
174179
if (Settings == nullptr)
175180
{
176-
defaultSettings = SessionSettings::Default(callerToken.get(), tokenInfo.Elevated);
181+
defaultSettings = SessionSettings::Default(callerToken.get(), tokenInfo.Elevated, resolvedDisplayName);
177182
Settings = &defaultSettings->Settings;
178183
}
179184

@@ -248,11 +253,11 @@ void WSLCSessionManagerImpl::OpenSessionByName(LPCWSTR DisplayName, IWSLCSession
248253
{
249254
auto tokenInfo = GetCallingProcessTokenInfo();
250255

251-
// Null name = default session, resolved from caller's token.
256+
// Null name = default session, resolved from caller's token + username.
252257
std::wstring resolvedName;
253258
if (DisplayName == nullptr)
254259
{
255-
resolvedName = tokenInfo.Elevated ? wsl::windows::wslc::DefaultAdminSessionName : wsl::windows::wslc::DefaultSessionName;
260+
resolvedName = ResolveDefaultSessionName(tokenInfo);
256261
DisplayName = resolvedName.c_str();
257262
}
258263

@@ -367,6 +372,40 @@ CallingProcessTokenInfo WSLCSessionManagerImpl::GetCallingProcessTokenInfo()
367372
return {std::move(tokenInfo), elevated};
368373
}
369374

375+
std::wstring WSLCSessionManagerImpl::ResolveDefaultSessionName(const CallingProcessTokenInfo& TokenInfo)
376+
{
377+
// Look up the username from the caller's SID so each user gets their own
378+
// default session (e.g. "wslc-cli-alice", "wslc-cli-admin-bob").
379+
wchar_t username[256 + 1] = {};
380+
DWORD usernameLen = ARRAYSIZE(username);
381+
wchar_t domain[MAX_PATH] = {};
382+
DWORD domainLen = ARRAYSIZE(domain);
383+
SID_NAME_USE sidType;
384+
THROW_IF_WIN32_BOOL_FALSE(LookupAccountSidW(nullptr, TokenInfo.TokenInfo->User.Sid, username, &usernameLen, domain, &domainLen, &sidType));
385+
386+
auto baseName = TokenInfo.Elevated ? wsl::windows::wslc::DefaultAdminSessionName : wsl::windows::wslc::DefaultSessionName;
387+
return std::format(L"{}-{}", baseName, username);
388+
}
389+
390+
bool WSLCSessionManagerImpl::IsReservedSessionName(LPCWSTR Name)
391+
{
392+
// Block any name that is exactly "wslc-cli" or starts with "wslc-cli-",
393+
// which covers the admin variant and all per-user resolved names.
394+
constexpr std::wstring_view prefix{wsl::windows::wslc::DefaultSessionName};
395+
std::wstring_view name{Name};
396+
if (name.size() < prefix.size())
397+
{
398+
return false;
399+
}
400+
401+
if (!wsl::shared::string::IsEqual(name.substr(0, prefix.size()), prefix, true))
402+
{
403+
return false;
404+
}
405+
406+
return name.size() == prefix.size() || name[prefix.size()] == L'-';
407+
}
408+
370409
HRESULT WSLCSessionManagerImpl::CheckTokenAccess(const SessionEntry& Entry, const CallingProcessTokenInfo& TokenInfo)
371410
{
372411
// Allow elevated tokens to access all sessions.

src/windows/service/exe/WSLCSessionManager.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,13 @@ class WSLCSessionManagerImpl
7979
void OpenSessionByName(_In_ LPCWSTR DisplayName, _Out_ IWSLCSession** Session);
8080

8181
private:
82+
// Resolves the default session name for a caller: appends the username
83+
// from the token SID so different users don't collide.
84+
static std::wstring ResolveDefaultSessionName(const CallingProcessTokenInfo& TokenInfo);
85+
86+
// Returns true if the name matches a reserved default session prefix.
87+
static bool IsReservedSessionName(LPCWSTR Name);
88+
8289
// Iterates over all sessions, cleaning up released sessions.
8390
// The routine receives a SessionEntry& and can return an optional<T> to stop iteration.
8491
template <typename T>

src/windows/service/inc/wslc.idl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -762,4 +762,6 @@ cpp_quote("#define WSLC_E_CONTAINER_PREFIX_AMBIGUOUS MAKE_HRESULT(SEVERITY_ERROR
762762
cpp_quote("#define WSLC_E_CONTAINER_NOT_FOUND MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, WSLC_E_BASE + 3) /* 0x80040603 */")
763763
cpp_quote("#define WSLC_E_VOLUME_NOT_FOUND MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, WSLC_E_BASE + 4) /* 0x80040604 */")
764764
cpp_quote("#define WSLC_E_CONTAINER_NOT_RUNNING MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, WSLC_E_BASE + 5) /* 0x80040605 */")
765-
cpp_quote("#define WSLC_E_CONTAINER_IS_RUNNING MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, WSLC_E_BASE + 6) /* 0x80040606 */")
765+
cpp_quote("#define WSLC_E_CONTAINER_IS_RUNNING MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, WSLC_E_BASE + 6) /* 0x80040606 */")
766+
cpp_quote("#define WSLC_E_SESSION_RESERVED MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, WSLC_E_BASE + 7) /* 0x80040607 */")
767+
cpp_quote("#define WSLC_E_INVALID_SESSION_NAME MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, WSLC_E_BASE + 8) /* 0x80040608 */")

test/windows/WSLCTests.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -341,23 +341,23 @@ class WSLCTests
341341
{
342342
auto settings = GetDefaultSessionSettings(nullptr);
343343
wil::com_ptr<IWSLCSession> session;
344-
VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, &session), E_INVALIDARG);
344+
VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, &session), WSLC_E_INVALID_SESSION_NAME);
345345
}
346346

347347
// Reject DisplayName at exact boundary (no room for null terminator).
348348
{
349349
std::wstring boundaryName(std::size(WSLCSessionInformation{}.DisplayName), L'x');
350350
auto settings = GetDefaultSessionSettings(boundaryName.c_str());
351351
wil::com_ptr<IWSLCSession> session;
352-
VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, &session), E_INVALIDARG);
352+
VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, &session), WSLC_E_INVALID_SESSION_NAME);
353353
}
354354

355355
// Reject too long DisplayName.
356356
{
357357
std::wstring longName(std::size(WSLCSessionInformation{}.DisplayName) + 1, L'x');
358358
auto settings = GetDefaultSessionSettings(longName.c_str());
359359
wil::com_ptr<IWSLCSession> session;
360-
VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, &session), E_INVALIDARG);
360+
VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, &session), WSLC_E_INVALID_SESSION_NAME);
361361
}
362362

363363
// Validate that creating a session on a non-existing storage fails if WSLCSessionStorageFlagsNoCreate is set.

0 commit comments

Comments
 (0)