diff --git a/src/windows/service/exe/LxssUserSession.cpp b/src/windows/service/exe/LxssUserSession.cpp index 2ad6469c9..28f0be5f1 100644 --- a/src/windows/service/exe/LxssUserSession.cpp +++ b/src/windows/service/exe/LxssUserSession.cpp @@ -939,13 +939,39 @@ HRESULT LxssUserSessionImpl::MoveDistribution(_In_ LPCGUID DistroGuid, _In_ LPCW THROW_WIN32(error.value()); } + // Read the original VHD owner before the move so we can restore it after. + // Cross-volume MoveFileEx may set the owner to BUILTIN\Administrators for + // elevated callers, which breaks HcsGrantVmAccess (needs WRITE_DAC via + // ownership) from non-elevated contexts. + PSID originalOwner = nullptr; + wil::unique_hlocal originalDescriptor; + THROW_IF_WIN32_ERROR(GetNamedSecurityInfoW( + distro.VhdFilePath.c_str(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, &originalOwner, nullptr, nullptr, nullptr, &originalDescriptor)); + // Move the VHD to the new location. THROW_IF_WIN32_BOOL_FALSE(MoveFileEx(distro.VhdFilePath.c_str(), newVhdPath.c_str(), MOVEFILE_COPY_ALLOWED | MOVEFILE_WRITE_THROUGH)); + // Restore the original VHD owner on the moved file. + auto setVhdOwner = [&originalOwner](const std::filesystem::path& vhdPath) { + wil::unique_hfile vhdHandle(CreateFileW( + vhdPath.c_str(), WRITE_OWNER, FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, FILE_FLAG_OPEN_REPARSE_POINT, nullptr)); + THROW_LAST_ERROR_IF(!vhdHandle); + + auto runAsSelf = wil::run_as_self(); + auto privileges = wsl::windows::common::security::AcquirePrivilege(SE_RESTORE_NAME); + THROW_IF_WIN32_ERROR( + ::SetSecurityInfo(vhdHandle.get(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, originalOwner, nullptr, nullptr, nullptr)); + }; + + setVhdOwner(newVhdPath); + auto revert = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { THROW_IF_WIN32_BOOL_FALSE(MoveFileEx( newVhdPath.c_str(), distro.VhdFilePath.c_str(), MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH)); + // Fix ownership on the reverted VHD in case MoveFileEx copied across volumes. + LOG_IF_FAILED(wil::ResultFromException([&] { setVhdOwner(distro.VhdFilePath); })); + // Write the location back to the original path in case the second registry write failed. Otherwise, this is a no-op. registration.Write(Property::BasePath, distro.BasePath.c_str()); }); diff --git a/test/windows/UnitTests.cpp b/test/windows/UnitTests.cpp index ee14a5ffe..288de4cee 100644 --- a/test/windows/UnitTests.cpp +++ b/test/windows/UnitTests.cpp @@ -2801,6 +2801,80 @@ Error code: Wsl/InstallDistro/WSL_E_DISTRO_NOT_FOUND } } + TEST_METHOD(MoveVhdOwnership) + { + constexpr auto name = L"move-owner-test-distro"; + constexpr auto moveElevatedFolder = L"move-owner-elevated"; + constexpr auto moveNonElevatedFolder = L"move-owner-non-elevated"; + + // Import a WSL2 distro. + VERIFY_ARE_EQUAL(LxsstuLaunchWsl(std::format(L"--import {} . \"{}\" --version 2", name, g_testDistroPath)), 0L); + + auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [name]() { + LxsstuLaunchWsl(std::format(L"--unregister {}", name)); + std::filesystem::remove_all(moveElevatedFolder); + std::filesystem::remove_all(moveNonElevatedFolder); + }); + + auto verifyVhdOwner = [](const std::wstring& path) { + PSID ownerSid = nullptr; + wil::unique_hlocal descriptor; + THROW_IF_WIN32_ERROR(GetNamedSecurityInfoW( + path.c_str(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, &ownerSid, nullptr, nullptr, nullptr, &descriptor)); + + auto userToken = wil::open_current_access_token(TOKEN_QUERY); + auto tokenUser = wil::get_token_information(userToken.get()); + + VERIFY_IS_TRUE(EqualSid(ownerSid, tokenUser->User.Sid)); + }; + + const auto nonElevatedToken = GetNonElevatedToken(); + + // Move as elevated, launch as non-elevated. + // This is the primary bug scenario: MoveFileEx sets owner to BUILTIN\Administrators, + // then HcsGrantVmAccess fails with E_ACCESSDENIED when impersonating the non-elevated user. + { + WslShutdown(); + VERIFY_ARE_EQUAL(LxsstuLaunchWsl(std::format(L"--manage {} --move {}", name, moveElevatedFolder)), 0L); + + auto vhdPath = std::format(L"{}\\ext4.vhdx", moveElevatedFolder); + VERIFY_IS_TRUE(std::filesystem::exists(vhdPath)); + verifyVhdOwner(vhdPath); + + WslShutdown(); + auto [out, err] = LxsstuLaunchWslAndCaptureOutput(std::format(L"-d {} echo ok", name), 0, nullptr, nonElevatedToken.get()); + VERIFY_ARE_EQUAL(out, L"ok\n"); + } + + // Move as non-elevated, launch as elevated. + { + WslShutdown(); + VERIFY_ARE_EQUAL( + LxsstuLaunchWsl( + std::format(L"--manage {} --move {}", name, moveNonElevatedFolder), + nullptr, + nullptr, + nullptr, + nonElevatedToken.get()), + 0L); + + auto vhdPath = std::format(L"{}\\ext4.vhdx", moveNonElevatedFolder); + VERIFY_IS_TRUE(std::filesystem::exists(vhdPath)); + verifyVhdOwner(vhdPath); + + WslShutdown(); + auto [out, err] = LxsstuLaunchWslAndCaptureOutput(std::format(L"-d {} echo ok", name)); + VERIFY_ARE_EQUAL(out, L"ok\n"); + } + + // Also launch as non-elevated after the non-elevated move. + { + WslShutdown(); + auto [out, err] = LxsstuLaunchWslAndCaptureOutput(std::format(L"-d {} echo ok", name), 0, nullptr, nonElevatedToken.get()); + VERIFY_ARE_EQUAL(out, L"ok\n"); + } + } + WSL2_TEST_METHOD(Resize) { constexpr auto name = L"resize-test-distro";