From fb8fe46c3ebf0d59068d73e0f9b265b8839da516 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:30:17 -0400 Subject: [PATCH] fix: harden Windows broker uninstall and release CI Strengthens Windows reliability and release safety by making driver uninstall failures explicit, verifying broker/service/package cleanup, and improving broker pipe reconnection handling for transient pipe states. Adds extensive Windows broker client/service tests (including persistence and Polar transport failure injection), updates CI coverage exclusions, signing-identity and release-artifact validation, and aligns docs/artifact naming with AMD64-only installer limits. --- .github/workflows/ci.yml | 63 ++- README.md | 11 + .../libvirtualhid-driver-installer-patch.xml | 4 +- docs/store-review-validation.md | 2 +- docs/windows-driver.md | 23 +- scripts/windows/uninstall-driver.ps1 | 195 +++++--- .../windows/broker/libvirtualhid_broker.cpp | 44 +- .../windows/windows_broker_client.cpp | 72 ++- tests/CMakeLists.txt | 17 +- .../windows_broker_client_test_hooks.hpp | 10 + .../windows_broker_service_test_hooks.hpp | 50 ++ .../windows_broker_client_test_hooks.cpp | 47 +- .../windows_broker_service_test_hooks.cpp | 435 +++++++++++++++++ tests/unit/test_windows_broker_client.cpp | 55 +++ tests/unit/test_windows_broker_service.cpp | 444 ++++++++++++++++++ 15 files changed, 1359 insertions(+), 113 deletions(-) create mode 100644 tests/fixtures/include/fixtures/windows_broker_service_test_hooks.hpp create mode 100644 tests/fixtures/windows_broker_service_test_hooks.cpp create mode 100644 tests/unit/test_windows_broker_service.cpp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d1ea48..cc29672 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -363,10 +363,12 @@ jobs: throw "OpenCppCoverage.exe was not found." } + # The broker test hook compiles a private copy only for failure injection. & $openCppCoverage ` --sources "$env:GITHUB_WORKSPACE\examples" ` --sources "$env:GITHUB_WORKSPACE\src" ` --sources "$env:GITHUB_WORKSPACE\tools" ` + --excluded_sources "$env:GITHUB_WORKSPACE\src\platform\windows\broker" ` "--export_type=cobertura:$env:GITHUB_WORKSPACE\cmake-build-ci\reports\coverage.xml" ` --working_dir "$env:GITHUB_WORKSPACE\cmake-build-ci\tests" ` -- ` @@ -413,11 +415,13 @@ jobs: GCOV_EXECUTABLE: ${{ matrix.gcov_executable }} MSYS2_PATH_TYPE: inherit run: | + # The broker test hook compiles a private copy only for failure injection. uv run --project ../third-party/lizardbyte-common --locked --no-sync gcovr . -r .. \ --filter ../examples/ \ --filter ../src/ \ --filter ../tools/ \ --gcov-executable "${GCOV_EXECUTABLE}" \ + --exclude ../src/platform/windows/broker/ \ --exclude ../tests/ \ --exclude ../third-party/ \ --exclude-noncode-lines \ @@ -590,7 +594,8 @@ jobs: New-Item -ItemType Directory -Force -Path artifacts | Out-Null Copy-Item ` -LiteralPath .\cmake-build-driver\cpack_artifacts\libvirtualhid.msi ` - -Destination .\artifacts\libvirtualhid-Windows-Driver-installer.msi + -Destination ` + ".\artifacts\libvirtualhid-Windows-AMD64-driver-installer.msi" - name: Export Azure driver signing certificate if: >- @@ -635,6 +640,39 @@ jobs: files-folder-recurse: false signing-account-name: ${{ vars.AZURE_SIGNING_ACCOUNT }} + - name: Validate release signing identities + if: >- + github.event_name == 'push' && + needs.setup_release.outputs.publish_release == 'true' && + vars.AZURE_SIGNING_ACCOUNT != '' + shell: pwsh + run: | + $catalogPath = Join-Path ` + $env:GITHUB_WORKSPACE ` + "cmake-build-driver\src\platform\windows\driver\package\$env:DRIVER_BUILD_CONFIG\libvirtualhid.cat" + $installerPath = Get-ChildItem -LiteralPath .\artifacts -Filter *.msi | + Select-Object -ExpandProperty FullName -First 1 + if (!$installerPath) { + throw "The signed Windows driver installer was not found." + } + + $catalogSignature = Get-AuthenticodeSignature -FilePath $catalogPath + $installerSignature = Get-AuthenticodeSignature -FilePath $installerPath + foreach ($signature in @($catalogSignature, $installerSignature)) { + if ($signature.Status -ne "Valid" -or !$signature.SignerCertificate) { + throw "A release signature is invalid: $($signature.StatusMessage)" + } + } + if ($catalogSignature.SignerCertificate.Subject -cne ` + $installerSignature.SignerCertificate.Subject) { + throw "The catalog and MSI were signed with different identities." + } + Write-Host ( + "Validated release signer " + + "$($installerSignature.SignerCertificate.Subject) " + + "[$($installerSignature.SignerCertificate.Thumbprint)]." + ) + - name: Debug wix if: always() shell: pwsh @@ -771,12 +809,33 @@ jobs: run: | mkdir -p artifacts for name in Linux-GCC Linux-Clang macOS Windows-MinGW-UCRT64 Windows-MSVC; do + release_name="${name}" + case "${name}" in + Windows-MinGW-UCRT64) release_name="Windows-AMD64-MinGW-UCRT64" ;; + Windows-MSVC) release_name="Windows-AMD64-MSVC" ;; + esac zip -r \ - "artifacts/libvirtualhid-${{ needs.setup_release.outputs.release_tag }}-${name}.zip" \ + "artifacts/libvirtualhid-${release_name}.zip" \ "install-${name}" done cp windows-driver-installer/*.msi artifacts/ + - name: Validate release metadata + env: + RELEASE_COMMIT: ${{ needs.setup_release.outputs.release_commit }} + RELEASE_TAG: ${{ needs.setup_release.outputs.release_tag }} + RELEASE_VERSION: ${{ needs.setup_release.outputs.release_version }} + run: | + test -n "${RELEASE_TAG}" + test -n "${RELEASE_VERSION}" + test "${RELEASE_COMMIT}" = "${GITHUB_SHA}" + test -s "artifacts/libvirtualhid-Linux-GCC.zip" + test -s "artifacts/libvirtualhid-Linux-Clang.zip" + test -s "artifacts/libvirtualhid-macOS.zip" + test -s "artifacts/libvirtualhid-Windows-AMD64-driver-installer.msi" + test -s "artifacts/libvirtualhid-Windows-AMD64-MinGW-UCRT64.zip" + test -s "artifacts/libvirtualhid-Windows-AMD64-MSVC.zip" + - name: Create/Update GitHub Release if: needs.setup_release.outputs.publish_release == 'true' uses: LizardByte/actions/actions/release_create@d0ae7f82215a479fe2b74f4088c53ee6460513dd # v2026.728.214955 diff --git a/README.md b/README.md index 6539940..e69ea65 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,17 @@ The library is designed around gamepad use first because remote streaming hosts are the first consumer class. Non-gamepad device types are available through the same API where the backend exposes them. +## ⚠️ Known Windows Limitations + +- Steam does not expose the Xbox Series Share button from the VHF child through + the same Xbox HIDAPI path used by physical controllers. That path requires a + non-VHF Xbox HIDAPI/GIP transport. +- PlayStation and Nintendo rumble parsing is covered by protocol and installed + driver tests but has not yet completed broad validation with real client + applications. +- The published Windows driver installer is AMD64-only. Windows ARM64 release + packages require a different Microsoft driver-signing path. + ## 🔁 Alternatives Alternatives exist if `libvirtualhid` does not meet your needs. diff --git a/cmake/packaging/wix_resources/libvirtualhid-driver-installer-patch.xml b/cmake/packaging/wix_resources/libvirtualhid-driver-installer-patch.xml index ad8be0e..05ac291 100644 --- a/cmake/packaging/wix_resources/libvirtualhid-driver-installer-patch.xml +++ b/cmake/packaging/wix_resources/libvirtualhid-driver-installer-patch.xml @@ -16,13 +16,13 @@ Directory="INSTALL_ROOT" ExeCommand=""[WindowsFolder]System32\WindowsPowerShell\v1.0\powershell.exe" -WindowStyle Hidden -NoProfile -ExecutionPolicy Bypass -File "[INSTALL_ROOT]scripts\windows\uninstall-driver.ps1" -Force -RemoveCertificateSubject "CN=libvirtualhid CI Test Driver Signing"" Execute="deferred" - Return="ignore" + Return="check" Impersonate="no" /> diff --git a/docs/store-review-validation.md b/docs/store-review-validation.md index 3d35d80..2b7c56a 100644 --- a/docs/store-review-validation.md +++ b/docs/store-review-validation.md @@ -60,7 +60,7 @@ Expected result: ## Manual Review Steps 1. Install the released, production-signed - `libvirtualhid-Windows-Driver-installer.msi`. + `libvirtualhid-Windows-AMD64-driver-installer.msi`. 2. Reboot only if Windows reports that a reboot is required. 3. Open PowerShell. 4. Run the required validation tool from the submission notes. diff --git a/docs/windows-driver.md b/docs/windows-driver.md index 94516e6..88cdafb 100644 --- a/docs/windows-driver.md +++ b/docs/windows-driver.md @@ -159,8 +159,13 @@ with a service SID. The service `ImagePath` is stored as a literal quoted path, and installation fails if the registry value is not safely quoted. This avoids CWE-428 unquoted-service-path escalation when the install root contains spaces. The install helper also clears any legacy broker service `Environment` value so -licensing configuration cannot be overridden on the user's machine. The uninstall -helper stops and deletes that service before removing the driver package. +licensing configuration cannot be overridden on the user's machine. The +uninstall helper stops and deletes that service before removing the driver +package. It discovers staged OEM INF names through language-neutral DISM and +CIM objects instead of parsing localized `pnputil` labels. Uninstall fails if a +command fails or if the broker service, root device, or staged driver package +is still present after cleanup, so the MSI cannot silently report a complete +removal while driver state remains. The installed-driver test fails if the root device is not started, if `\\.\LibVirtualHid` cannot be opened, or if a held `gamepad_adapter` instance @@ -322,6 +327,20 @@ do not alter the public platform-neutral profile API. Consumers that display raw HID strings may still show the Windows VHF product label because VHF does not provide a product/manufacturer string callback. +### Current Release Limits + +- Steam does not expose the Xbox Series Share button from the VHF child through + the same Xbox HIDAPI path used by physical controllers. Supporting that path + requires a non-VHF Xbox HIDAPI/GIP transport. +- PlayStation and Nintendo rumble parsing is covered by protocol and installed + driver tests, but has not yet completed broad validation with real client + applications. +- The published Windows driver installer is AMD64-only. Windows ARM64 release + packages require a Microsoft dashboard signing path that is not part of the + current Azure Trusted Signing workflow. +- Every production gamepad creation requires a successful online license + validation response. There is no offline grace period. + ## Signing Windows driver packages require a signed catalog for normal installation. diff --git a/scripts/windows/uninstall-driver.ps1 b/scripts/windows/uninstall-driver.ps1 index e7d7b6c..7e40c77 100644 --- a/scripts/windows/uninstall-driver.ps1 +++ b/scripts/windows/uninstall-driver.ps1 @@ -28,11 +28,11 @@ function Invoke-CheckedCommand { [Parameter(Mandatory = $true)] [string[]] $Arguments, - [switch] $IgnoreFailure + [int[]] $SuccessExitCodes = @(0) ) & $FilePath @Arguments - if ($LASTEXITCODE -ne 0 -and -not $IgnoreFailure) { + if ($LASTEXITCODE -notin $SuccessExitCodes) { throw "$FilePath exited with code $LASTEXITCODE" } } @@ -52,57 +52,105 @@ function Remove-LibVirtualHidBrokerService { } if ($PSCmdlet.ShouldProcess($Name, "Delete libvirtualhid broker service")) { - Invoke-CheckedCommand -FilePath "sc.exe" -Arguments @("delete", $Name) -IgnoreFailure + $service.Dispose() + Invoke-CheckedCommand -FilePath "sc.exe" -Arguments @("delete", $Name) + + $deadline = [DateTime]::UtcNow.AddSeconds(15) + while ([DateTime]::UtcNow -lt $deadline) { + if (-not (Get-Service -Name $Name -ErrorAction SilentlyContinue)) { + return + } + Start-Sleep -Milliseconds 250 + } + throw "The $Name service still exists after deletion." } } -function Find-Devcon { - if ($env:DEVCON_EXE -and (Test-Path -LiteralPath $env:DEVCON_EXE)) { - return $env:DEVCON_EXE - } +function Find-PublishedName { + param( + [string] $TargetOriginalName, + [string] $TargetHardwareId + ) - $roots = @( - $env:WDKContentRoot, - $env:WindowsSdkDir, - "${env:ProgramFiles(x86)}\Windows Kits\10" - ) | Where-Object { $_ -and (Test-Path -LiteralPath $_) } + $publishedNames = @() + $dismFailure = $null + try { + $publishedNames = @( + Get-WindowsDriver -Online -All -ErrorAction Stop | + Where-Object { + $_.Driver -match "^oem\d+\.inf$" -and + [IO.Path]::GetFileName($_.OriginalFileName) -ieq $TargetOriginalName + } | + Select-Object -ExpandProperty Driver -Unique + ) + } catch { + $dismFailure = $_.Exception.Message + Write-Verbose "DISM driver-store enumeration failed: $dismFailure" + } + if ($publishedNames.Count -gt 0) { + return $publishedNames + } + if (-not $dismFailure) { + return @() + } - foreach ($root in $roots) { - $candidate = Get-ChildItem -LiteralPath $root -Recurse -Filter devcon.exe -ErrorAction SilentlyContinue | - Where-Object { $_.FullName -match "\\x64\\devcon\.exe$" } | - Select-Object -First 1 - if ($candidate) { - return $candidate.FullName - } + # Win32_PnPSignedDriver provides a language-neutral fallback for the package + # currently bound to the root device if DISM cannot enumerate the store. + $targetDeviceIds = @( + Get-LibVirtualHidRootDeviceInstanceId -TargetHardwareId $TargetHardwareId + ) + if ($targetDeviceIds.Count -eq 0) { + throw "DISM could not enumerate the driver store and no bound device is available for CIM fallback: $dismFailure" } - return $null + $cimPublishedNames = @( + Get-CimInstance -ClassName Win32_PnPSignedDriver -ErrorAction Stop | + Where-Object { + $_.DeviceID -in $targetDeviceIds -and + $_.InfName -match "^oem\d+\.inf$" + } | + Select-Object -ExpandProperty InfName -Unique + ) + if ($cimPublishedNames.Count -eq 0) { + throw "DISM could not enumerate the driver store and CIM did not identify the bound package: $dismFailure" + } + return $cimPublishedNames } -function Find-PublishedName { - param([string] $TargetOriginalName) +function Assert-PublishedName { + param([string] $Name) - $drivers = & pnputil.exe /enum-drivers - $currentPublished = $null - $currentOriginal = $null - $publishedNames = @() + if ($Name -notmatch "^oem\d+\.inf$") { + throw "The published driver package name is invalid: $Name" + } +} - foreach ($line in $drivers) { - if ($line -match "^\s*Published Name\s*:\s*(.+)$") { - $currentPublished = $Matches[1].Trim() - $currentOriginal = $null - continue - } +function Assert-LibVirtualHidRemoved { + param( + [string] $TargetOriginalName, + [string] $TargetHardwareId, + [string] $ServiceName + ) - if ($line -match "^\s*Original Name\s*:\s*(.+)$") { - $currentOriginal = $Matches[1].Trim() - if ($currentPublished -and $currentOriginal -ieq $TargetOriginalName) { - $publishedNames += $currentPublished - } - } + if (Get-Service -Name $ServiceName -ErrorAction SilentlyContinue) { + throw "The $ServiceName service remains installed." + } + + $remainingDevices = @( + Get-LibVirtualHidRootDeviceInstanceId -TargetHardwareId $TargetHardwareId + ) + if ($remainingDevices.Count -gt 0) { + throw "libvirtualhid device instances remain installed: $($remainingDevices -join ', ')" } - return $publishedNames + $remainingPackages = @( + Find-PublishedName ` + -TargetOriginalName $TargetOriginalName ` + -TargetHardwareId $TargetHardwareId + ) + if ($remainingPackages.Count -gt 0) { + throw "libvirtualhid driver packages remain staged: $($remainingPackages -join ', ')" + } } function Remove-DriverCertificate { @@ -125,47 +173,56 @@ function Remove-DriverCertificate { } } +$publishedNames = @() +if ($PublishedName) { + Assert-PublishedName -Name $PublishedName + $publishedNames += $PublishedName +} else { + try { + $publishedNames = @( + Find-PublishedName ` + -TargetOriginalName $OriginalName ` + -TargetHardwareId $HardwareId + ) + } catch { + throw "Unable to discover the staged libvirtualhid driver package through Windows APIs: $($_.Exception.Message)" + } +} + Remove-LibVirtualHidBrokerService -Name $BrokerServiceName -$devcon = Find-Devcon -if ($devcon -and $PSCmdlet.ShouldProcess($HardwareId, "Remove libvirtualhid development device")) { - Invoke-CheckedCommand -FilePath $devcon -Arguments @("remove", $HardwareId) -IgnoreFailure -} +$deviceInstanceIds = @( + Get-LibVirtualHidRootDeviceInstanceId -TargetHardwareId $HardwareId + Get-LibVirtualHidRegistryRootDevice -TargetHardwareId $HardwareId | + Select-Object -ExpandProperty InstanceId +) | Select-Object -Unique -foreach ($instanceId in (Get-LibVirtualHidRootDeviceInstanceId -TargetHardwareId $HardwareId)) { +foreach ($instanceId in $deviceInstanceIds) { if ($PSCmdlet.ShouldProcess($instanceId, "Remove libvirtualhid development device with pnputil")) { - Invoke-CheckedCommand -FilePath "pnputil.exe" -Arguments @("/remove-device", $instanceId) -IgnoreFailure - } -} - -foreach ($instanceId in (Get-LibVirtualHidRegistryRootDevice -TargetHardwareId $HardwareId | Select-Object -ExpandProperty InstanceId -Unique)) { - if ($PSCmdlet.ShouldProcess($instanceId, "Remove libvirtualhid registry-discovered development device with pnputil")) { - Invoke-CheckedCommand -FilePath "pnputil.exe" -Arguments @("/remove-device", $instanceId) -IgnoreFailure + Invoke-CheckedCommand -FilePath "pnputil.exe" -Arguments @("/remove-device", $instanceId) } } -$publishedNames = @() -if ($PublishedName) { - $publishedNames += $PublishedName -} else { - $publishedNames = @(Find-PublishedName -TargetOriginalName $OriginalName) -} - if ($publishedNames.Count -eq 0) { Write-Warning "No staged libvirtualhid driver package matching $OriginalName was found." - Remove-DriverCertificate -Subject $RemoveCertificateSubject - return -} - -foreach ($driverPackage in $publishedNames) { - $deleteArgs = @("/delete-driver", $driverPackage, "/uninstall") - if ($Force) { - $deleteArgs += "/force" - } +} else { + foreach ($driverPackage in $publishedNames) { + Assert-PublishedName -Name $driverPackage + $deleteArgs = @("/delete-driver", $driverPackage, "/uninstall") + if ($Force) { + $deleteArgs += "/force" + } - if ($PSCmdlet.ShouldProcess($driverPackage, "Delete libvirtualhid driver package")) { - Invoke-CheckedCommand -FilePath "pnputil.exe" -Arguments $deleteArgs + if ($PSCmdlet.ShouldProcess($driverPackage, "Delete libvirtualhid driver package")) { + Invoke-CheckedCommand -FilePath "pnputil.exe" -Arguments $deleteArgs + } } } +if (-not $WhatIfPreference) { + Assert-LibVirtualHidRemoved ` + -TargetOriginalName $OriginalName ` + -TargetHardwareId $HardwareId ` + -ServiceName $BrokerServiceName +} Remove-DriverCertificate -Subject $RemoveCertificateSubject diff --git a/src/platform/windows/broker/libvirtualhid_broker.cpp b/src/platform/windows/broker/libvirtualhid_broker.cpp index a2f47e2..ae6cc9c 100644 --- a/src/platform/windows/broker/libvirtualhid_broker.cpp +++ b/src/platform/windows/broker/libvirtualhid_broker.cpp @@ -58,7 +58,7 @@ #include #include -namespace { +namespace lvh::detail::windows_broker_service { using UniqueHandle = std::unique_ptr; using UniqueLocalMemory = std::unique_ptr; @@ -1707,11 +1707,28 @@ namespace { const Response &response, HANDLE requested_stop_event ) { - return write_pipe_message( + if (!write_pipe_message( + pipe, + std::as_bytes(std::span {&response, 1}), + requested_stop_event + )) { + return false; + } + + // DisconnectNamedPipe discards responses that the client has not read yet. + // A well-behaved one-request client closes its handle after reading the + // response, so wait for that close before disconnecting the server end. The + // overlapped read retains the normal I/O timeout and stop-event handling, + // preventing an uncooperative client from blocking the service indefinitely. + std::array extra_request {}; + DWORD bytes_read = 0; + static_cast(read_pipe_message( pipe, - std::as_bytes(std::span {&response, 1}), - requested_stop_event - ); + extra_request, + requested_stop_event, + bytes_read + )); + return true; } template @@ -1967,20 +1984,25 @@ namespace { return static_cast(result); } -} // namespace +} // namespace lvh::detail::windows_broker_service int main(int argc, char **argv) { if (argc > 1 && std::string_view {argv[1]} == "--console") { - return run_console(); + return lvh::detail::windows_broker_service::run_console(); } if (argc > 1 && std::string_view {argv[1]} == "--service-name") { - (void) broker_instance_name; + (void) lvh::detail::windows_broker_service::broker_instance_name; return 0; } - std::wstring mutable_service_name {service_name}; + std::wstring mutable_service_name { + lvh::detail::windows_broker_service::service_name, + }; if (std::array dispatch_table {{ - {mutable_service_name.data(), service_main}, + { + mutable_service_name.data(), + lvh::detail::windows_broker_service::service_main, + }, {nullptr, nullptr}, }}; StartServiceCtrlDispatcherW(dispatch_table.data()) != FALSE) { @@ -1989,7 +2011,7 @@ int main(int argc, char **argv) { const auto error = GetLastError(); if (error == ERROR_FAILED_SERVICE_CONTROLLER_CONNECT) { - return run_console(); + return lvh::detail::windows_broker_service::run_console(); } return static_cast(error); diff --git a/src/platform/windows/windows_broker_client.cpp b/src/platform/windows/windows_broker_client.cpp index 4852861..d05eb11 100644 --- a/src/platform/windows/windows_broker_client.cpp +++ b/src/platform/windows/windows_broker_client.cpp @@ -40,6 +40,7 @@ namespace lvh::detail::windows_broker { constexpr auto broker_service_name = L"libvirtualhid_broker"; constexpr auto pipe_client_access = GENERIC_READ | FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES; constexpr auto pipe_client_granted_access = FILE_GENERIC_READ | FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES; + constexpr auto pipe_retry_interval = 10U; constexpr auto pipe_wait_timeout = 5000U; static_assert(pipe_client_access == 0x80000102U); @@ -53,6 +54,52 @@ namespace lvh::detail::windows_broker { return {handle, &::CloseHandle}; } + static HANDLE open_broker_pipe() { + return ::CreateFileA( + LVH_WINDOWS_BROKER_PIPE_PATH, + pipe_client_access, + 0, + nullptr, + OPEN_EXISTING, + 0, + nullptr + ); + } + + static bool wait_to_retry_broker_pipe(DWORD &last_error) { + if (last_error == ERROR_FILE_NOT_FOUND) { + ::Sleep(pipe_retry_interval); + return true; + } + if (last_error != ERROR_PIPE_BUSY) { + return false; + } + if (::WaitNamedPipeA(LVH_WINDOWS_BROKER_PIPE_PATH, pipe_retry_interval) != FALSE) { + return true; + } + + last_error = ::GetLastError(); + return last_error == ERROR_SEM_TIMEOUT || last_error == ERROR_FILE_NOT_FOUND; + } + + static UniqueHandle connect_to_broker_pipe() { + DWORD last_error = ERROR_FILE_NOT_FOUND; + for (auto attempt = 0U; attempt < pipe_wait_timeout / pipe_retry_interval; ++attempt) { + if (HANDLE pipe = open_broker_pipe(); pipe != INVALID_HANDLE_VALUE) { + return make_unique_handle(pipe); + } + + last_error = ::GetLastError(); + if (!wait_to_retry_broker_pipe(last_error)) { + ::SetLastError(last_error); + return make_unique_handle(INVALID_HANDLE_VALUE); + } + } + + ::SetLastError(last_error); + return make_unique_handle(INVALID_HANDLE_VALUE); + } + static UniqueServiceHandle make_unique_service_handle(SC_HANDLE handle) { return {handle, &::CloseServiceHandle}; } @@ -176,30 +223,7 @@ namespace lvh::detail::windows_broker { std::span response, std::string_view operation ) { - auto pipe = windows_broker_client_implementation::make_unique_handle( - ::CreateFileA( - LVH_WINDOWS_BROKER_PIPE_PATH, - windows_broker_client_implementation::pipe_client_access, - 0, - nullptr, - OPEN_EXISTING, - 0, - nullptr - ) - ); - if (!pipe && ::GetLastError() == ERROR_PIPE_BUSY && ::WaitNamedPipeA(LVH_WINDOWS_BROKER_PIPE_PATH, windows_broker_client_implementation::pipe_wait_timeout) != FALSE) { - pipe = windows_broker_client_implementation::make_unique_handle( - ::CreateFileA( - LVH_WINDOWS_BROKER_PIPE_PATH, - windows_broker_client_implementation::pipe_client_access, - 0, - nullptr, - OPEN_EXISTING, - 0, - nullptr - ) - ); - } + auto pipe = windows_broker_client_implementation::connect_to_broker_pipe(); if (!pipe) { return OperationStatus::failure( ErrorCode::backend_unavailable, diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4097d48..d465399 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -53,12 +53,20 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux") find_package(X11 QUIET) endif() elseif(WIN32) + if(NOT TARGET nlohmann_json::nlohmann_json) + include("${PROJECT_SOURCE_DIR}/cmake/cpm/CPM.cmake") + CPMUsePackageLock("${PROJECT_SOURCE_DIR}/package-lock.cmake") + CPMGetPackage(nlohmann_json) + endif() + list(APPEND LIBVIRTUALHID_TEST_SOURCES "${PROJECT_SOURCE_DIR}/src/platform/windows/driver/rotating_trace_log.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/fixtures/windows_broker_client_test_hooks.cpp" + "${CMAKE_CURRENT_SOURCE_DIR}/fixtures/windows_broker_service_test_hooks.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/fixtures/windows_backend_test_hooks.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/unit/test_windows_backend.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/unit/test_windows_broker_client.cpp" + "${CMAKE_CURRENT_SOURCE_DIR}/unit/test_windows_broker_service.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/unit/test_windows_consumers.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/unit/test_windows_driver_protocol.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/unit/test_windows_rotating_trace_log.cpp") @@ -107,12 +115,19 @@ elseif(WIN32) target_compile_definitions(${TEST_BINARY} PRIVATE LIBVIRTUALHID_TEST_BINARY_DIR="${CMAKE_CURRENT_BINARY_DIR}") + if(MSVC) + target_compile_options(${TEST_BINARY} PRIVATE /EHsc) + endif() target_link_libraries(${TEST_BINARY} PRIVATE + nlohmann_json::nlohmann_json + advapi32 + crypt32 dinput8 dxguid hid - setupapi) + setupapi + winhttp) endif() if(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND LIBVIRTUALHID_ENABLE_XTEST AND X11_FOUND AND X11_XTest_FOUND) diff --git a/tests/fixtures/include/fixtures/windows_broker_client_test_hooks.hpp b/tests/fixtures/include/fixtures/windows_broker_client_test_hooks.hpp index 1659bc7..f5197f3 100644 --- a/tests/fixtures/include/fixtures/windows_broker_client_test_hooks.hpp +++ b/tests/fixtures/include/fixtures/windows_broker_client_test_hooks.hpp @@ -13,6 +13,13 @@ namespace lvh::detail::test { enum class BrokerServiceScenario { + pipe_unavailable_once, + pipe_never_available, + pipe_access_denied, + pipe_busy_once, + pipe_busy_timeout_once, + pipe_busy_disappears_once, + pipe_busy_failure, pipe_process_failure, zero_pipe_process, service_manager_failure, @@ -27,6 +34,9 @@ namespace lvh::detail::test { OperationStatus status; std::uint32_t closed_pipe_handles = 0; std::uint32_t closed_service_handles = 0; + std::uint32_t create_attempts = 0; + std::uint32_t sleep_attempts = 0; + std::uint32_t wait_attempts = 0; bool transacted = false; }; diff --git a/tests/fixtures/include/fixtures/windows_broker_service_test_hooks.hpp b/tests/fixtures/include/fixtures/windows_broker_service_test_hooks.hpp new file mode 100644 index 0000000..ccbfa62 --- /dev/null +++ b/tests/fixtures/include/fixtures/windows_broker_service_test_hooks.hpp @@ -0,0 +1,50 @@ +/** + * @file tests/fixtures/include/fixtures/windows_broker_service_test_hooks.hpp + * @brief Private Windows broker persistence and Polar transport test hooks. + */ +#pragma once + +// standard includes +#include +#include + +namespace lvh::detail::test { + + enum class BrokerPersistenceFailure { + dpapi, + create_file, + partial_write, + flush, + close, + }; + + struct BrokerPersistenceFailureResult { + bool saved = false; + std::string message; + }; + + BrokerPersistenceFailureResult broker_persistence_failure( + BrokerPersistenceFailure failure + ); + + enum class BrokerPolarScenario { + open_failure, + connect_failure, + request_failure, + send_failure, + receive_failure, + structured_error, + malformed_error, + success, + }; + + struct BrokerPolarResult { + bool transport_ok = false; + std::uint32_t http_status = 0; + std::string body; + std::string error; + }; + + BrokerPolarResult broker_polar_scenario(BrokerPolarScenario scenario); + +} // namespace lvh::detail::test diff --git a/tests/fixtures/windows_broker_client_test_hooks.cpp b/tests/fixtures/windows_broker_client_test_hooks.cpp index fcce62d..8ce22d2 100644 --- a/tests/fixtures/windows_broker_client_test_hooks.cpp +++ b/tests/fixtures/windows_broker_client_test_hooks.cpp @@ -29,6 +29,9 @@ namespace { DWORD last_error = ERROR_ACCESS_DENIED; std::uint32_t closed_pipe_handles = 0; std::uint32_t closed_service_handles = 0; + std::uint32_t create_attempts = 0; + std::uint32_t sleep_attempts = 0; + std::uint32_t wait_attempts = 0; bool transacted = false; }; @@ -75,11 +78,48 @@ namespace { DWORD, HANDLE ) { + using enum lvh::detail::test::BrokerServiceScenario; + + ++fake_state().create_attempts; + const auto scenario = fake_state().scenario; + if ((scenario == pipe_unavailable_once && fake_state().create_attempts == 1U) || scenario == pipe_never_available) { + fake_state().last_error = ERROR_FILE_NOT_FOUND; + return INVALID_HANDLE_VALUE; + } + if (scenario == pipe_access_denied) { + fake_state().last_error = ERROR_ACCESS_DENIED; + return INVALID_HANDLE_VALUE; + } + if (fake_state().create_attempts == 1U && (scenario == pipe_busy_once || scenario == pipe_busy_timeout_once || scenario == pipe_busy_disappears_once || scenario == pipe_busy_failure)) { + fake_state().last_error = ERROR_PIPE_BUSY; + return INVALID_HANDLE_VALUE; + } return fake_pipe_handle(); } + void WINAPI fake_sleep(DWORD) { + ++fake_state().sleep_attempts; + // The deterministic retry test must not wait in real time. + } + BOOL WINAPI fake_wait_named_pipe_a(LPCSTR, DWORD) { - return FALSE; + using enum lvh::detail::test::BrokerServiceScenario; + + ++fake_state().wait_attempts; + switch (fake_state().scenario) { + case pipe_busy_once: + return TRUE; + case pipe_busy_timeout_once: + fake_state().last_error = ERROR_SEM_TIMEOUT; + return FALSE; + case pipe_busy_disappears_once: + fake_state().last_error = ERROR_FILE_NOT_FOUND; + return FALSE; + case pipe_busy_failure: + default: + fake_state().last_error = ERROR_ACCESS_DENIED; + return FALSE; + } } BOOL WINAPI fake_get_named_pipe_server_process_id(HANDLE, PULONG process_id) { @@ -158,6 +198,7 @@ namespace { #define OpenServiceW fake_open_service #define QueryServiceStatusEx fake_query_service_status #define SetNamedPipeHandleState fake_set_named_pipe_handle_state +#define Sleep fake_sleep #define TransactNamedPipe fake_transact_named_pipe #define WaitNamedPipeA fake_wait_named_pipe_a #define call_bytes call_bytes_for_windows_broker_client_test_hooks @@ -173,6 +214,7 @@ namespace { #undef OpenServiceW #undef QueryServiceStatusEx #undef SetNamedPipeHandleState +#undef Sleep #undef TransactNamedPipe #undef WaitNamedPipeA #undef call_bytes @@ -200,6 +242,9 @@ namespace lvh::detail::test { .status = std::move(status), .closed_pipe_handles = fake_state().closed_pipe_handles, .closed_service_handles = fake_state().closed_service_handles, + .create_attempts = fake_state().create_attempts, + .sleep_attempts = fake_state().sleep_attempts, + .wait_attempts = fake_state().wait_attempts, .transacted = fake_state().transacted, }; } diff --git a/tests/fixtures/windows_broker_service_test_hooks.cpp b/tests/fixtures/windows_broker_service_test_hooks.cpp new file mode 100644 index 0000000..b004f17 --- /dev/null +++ b/tests/fixtures/windows_broker_service_test_hooks.cpp @@ -0,0 +1,435 @@ +/** + * @file tests/fixtures/windows_broker_service_test_hooks.cpp + * @brief Windows broker persistence and Polar transport test hook definitions. + */ + +// local includes +#include "fixtures/windows_broker_service_test_hooks.hpp" + +#ifndef NOMINMAX + #define NOMINMAX +#endif +#ifndef WIN32_LEAN_AND_MEAN + #define WIN32_LEAN_AND_MEAN +#endif + +// platform includes +// clang-format off +#include +#include +#include +#include +// clang-format on + +// standard includes +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + + using lvh::detail::test::BrokerPersistenceFailure; + using lvh::detail::test::BrokerPolarScenario; + + struct BrokerServiceTestState { + BrokerPersistenceFailure persistence_failure = BrokerPersistenceFailure::dpapi; + HANDLE persistence_file = INVALID_HANDLE_VALUE; + BrokerPolarScenario polar_scenario = BrokerPolarScenario::success; + std::size_t polar_body_offset = 0; + }; + + BrokerServiceTestState &broker_service_test_state() { + static BrokerServiceTestState state; + return state; + } + + constexpr std::string_view structured_error_body = + R"({"detail":[{"msg":"License activation was rejected."}]})"; + constexpr std::string_view malformed_error_body = "not-json"; + constexpr std::string_view success_body = R"({"status":"granted"})"; + + std::string_view polar_body() { + switch (broker_service_test_state().polar_scenario) { + case BrokerPolarScenario::structured_error: + return structured_error_body; + case BrokerPolarScenario::malformed_error: + return malformed_error_body; + default: + return success_body; + } + } + + BOOL WINAPI broker_test_crypt_protect_data( + DATA_BLOB *plain_text, + LPCWSTR description, + DATA_BLOB *optional_entropy, + std::byte *reserved, + CRYPTPROTECT_PROMPTSTRUCT *prompt, + DWORD flags, + DATA_BLOB *cipher_text + ) { + if (broker_service_test_state().persistence_failure == BrokerPersistenceFailure::dpapi) { + ::SetLastError(ERROR_ACCESS_DENIED); + return FALSE; + } + return ::CryptProtectData( + plain_text, + description, + optional_entropy, + reserved, + prompt, + flags, + cipher_text + ); + } + + BOOL WINAPI broker_test_create_directory_w( + LPCWSTR path, + LPSECURITY_ATTRIBUTES + ) { + return ::CreateDirectoryW(path, nullptr); + } + + HANDLE WINAPI broker_test_create_file_w( + LPCWSTR filename, + DWORD desired_access, + DWORD share_mode, + LPSECURITY_ATTRIBUTES, + DWORD creation_disposition, + DWORD flags_and_attributes, + HANDLE template_file + ) { + if (broker_service_test_state().persistence_failure == BrokerPersistenceFailure::create_file) { + ::SetLastError(ERROR_ACCESS_DENIED); + return INVALID_HANDLE_VALUE; + } + broker_service_test_state().persistence_file = ::CreateFileW( + filename, + desired_access, + share_mode, + nullptr, + creation_disposition, + flags_and_attributes, + template_file + ); + return broker_service_test_state().persistence_file; + } + + BOOL WINAPI broker_test_write_file( + HANDLE file, + const std::byte *buffer, + DWORD bytes_to_write, + LPDWORD bytes_written, + LPOVERLAPPED overlapped + ) { + if (file == broker_service_test_state().persistence_file && broker_service_test_state().persistence_failure == BrokerPersistenceFailure::partial_write) { + const auto partial_size = bytes_to_write == 0U ? 0U : bytes_to_write - 1U; + const auto result = ::WriteFile( + file, + buffer, + partial_size, + bytes_written, + overlapped + ); + ::SetLastError(ERROR_WRITE_FAULT); + return result; + } + return ::WriteFile(file, buffer, bytes_to_write, bytes_written, overlapped); + } + + BOOL WINAPI broker_test_flush_file_buffers(HANDLE file) { + if (file == broker_service_test_state().persistence_file && broker_service_test_state().persistence_failure == BrokerPersistenceFailure::flush) { + ::SetLastError(ERROR_WRITE_FAULT); + return FALSE; + } + return ::FlushFileBuffers(file); + } + + BOOL WINAPI broker_test_close_handle(HANDLE handle) { + if (handle == broker_service_test_state().persistence_file) { + broker_service_test_state().persistence_file = INVALID_HANDLE_VALUE; + const auto closed = ::CloseHandle(handle); + if (broker_service_test_state().persistence_failure == BrokerPersistenceFailure::close) { + ::SetLastError(ERROR_INVALID_HANDLE); + return FALSE; + } + return closed; + } + return ::CloseHandle(handle); + } + + BOOL WINAPI broker_test_lookup_account_name_w( + LPCWSTR, + LPCWSTR, + PSID sid, + LPDWORD sid_size, + LPWSTR domain, + LPDWORD domain_size, + PSID_NAME_USE sid_name_use + ) { + std::array system_sid {}; + auto system_sid_size = static_cast(sizeof(system_sid)); + if (::CreateWellKnownSid(WinLocalSystemSid, nullptr, system_sid.data(), &system_sid_size) == FALSE) { + return FALSE; + } + + if (sid == nullptr || sid_size == nullptr || *sid_size < system_sid_size || domain == nullptr || domain_size == nullptr || *domain_size < 1U) { + if (sid_size != nullptr) { + *sid_size = system_sid_size; + } + if (domain_size != nullptr) { + *domain_size = 1U; + } + ::SetLastError(ERROR_INSUFFICIENT_BUFFER); + return FALSE; + } + + if (::CopySid(system_sid_size, sid, system_sid.data()) == FALSE) { + return FALSE; + } + domain[0] = L'\0'; + *domain_size = 0U; + *sid_name_use = SidTypeWellKnownGroup; + return TRUE; + } + + DWORD WINAPI broker_test_set_named_security_info_w( + LPWSTR, + SE_OBJECT_TYPE, + SECURITY_INFORMATION, + PSID, + PSID, + PACL, + PACL + ) { + return ERROR_SUCCESS; + } + + HINTERNET WINAPI broker_test_win_http_open( + LPCWSTR, + DWORD, + LPCWSTR, + LPCWSTR, + DWORD + ) { + if (broker_service_test_state().polar_scenario == BrokerPolarScenario::open_failure) { + ::SetLastError(ERROR_WINHTTP_CANNOT_CONNECT); + return nullptr; + } + static std::byte session {}; + return &session; + } + + HINTERNET WINAPI broker_test_win_http_connect( + HINTERNET, + LPCWSTR, + INTERNET_PORT, + DWORD + ) { + if (broker_service_test_state().polar_scenario == BrokerPolarScenario::connect_failure) { + ::SetLastError(ERROR_WINHTTP_CANNOT_CONNECT); + return nullptr; + } + static std::byte connection {}; + return &connection; + } + + HINTERNET WINAPI broker_test_win_http_open_request( + HINTERNET, + LPCWSTR, + LPCWSTR, + LPCWSTR, + LPCWSTR, + LPCWSTR const *, + DWORD + ) { + if (broker_service_test_state().polar_scenario == BrokerPolarScenario::request_failure) { + ::SetLastError(ERROR_INVALID_PARAMETER); + return nullptr; + } + static std::byte request {}; + return &request; + } + + BOOL WINAPI broker_test_win_http_send_request( + HINTERNET, + LPCWSTR, + DWORD, + std::byte *, + DWORD, + DWORD, + DWORD_PTR + ) { + if (broker_service_test_state().polar_scenario == BrokerPolarScenario::send_failure) { + ::SetLastError(ERROR_WINHTTP_CONNECTION_ERROR); + return FALSE; + } + return TRUE; + } + + BOOL WINAPI broker_test_win_http_receive_response(HINTERNET, std::byte *) { + if (broker_service_test_state().polar_scenario == BrokerPolarScenario::receive_failure) { + ::SetLastError(ERROR_WINHTTP_CONNECTION_ERROR); + return FALSE; + } + return TRUE; + } + + BOOL WINAPI broker_test_win_http_query_headers( + HINTERNET, + DWORD, + LPCWSTR, + std::byte *buffer, + LPDWORD buffer_length, + LPDWORD + ) { + if (buffer == nullptr || buffer_length == nullptr || *buffer_length < sizeof(DWORD)) { + ::SetLastError(ERROR_INSUFFICIENT_BUFFER); + return FALSE; + } + const auto status = + broker_service_test_state().polar_scenario == BrokerPolarScenario::structured_error || + broker_service_test_state().polar_scenario == BrokerPolarScenario::malformed_error ? + 422U : + 200U; + std::memcpy(buffer, &status, sizeof(status)); + *buffer_length = sizeof(status); + return TRUE; + } + + BOOL WINAPI broker_test_win_http_query_data_available( + HINTERNET, + LPDWORD available + ) { + if (available == nullptr) { + ::SetLastError(ERROR_INVALID_PARAMETER); + return FALSE; + } + *available = static_cast(polar_body().size() - broker_service_test_state().polar_body_offset); + return TRUE; + } + + BOOL WINAPI broker_test_win_http_read_data( + HINTERNET, + std::byte *buffer, + DWORD bytes_to_read, + LPDWORD bytes_read + ) { + const auto body = polar_body(); + const auto count = std::min( + bytes_to_read, + body.size() - broker_service_test_state().polar_body_offset + ); + std::memcpy(buffer, body.data() + broker_service_test_state().polar_body_offset, count); + broker_service_test_state().polar_body_offset += count; + *bytes_read = static_cast(count); + return TRUE; + } + + BOOL WINAPI broker_test_win_http_close_handle(HINTERNET) { + return TRUE; + } + +} // namespace + +#define CloseHandle broker_test_close_handle +#define CreateDirectoryW broker_test_create_directory_w +#define CreateFileW broker_test_create_file_w +#define CryptProtectData(plain_text, description, optional_entropy, reserved, prompt, flags, cipher_text) \ + broker_test_crypt_protect_data(plain_text, description, optional_entropy, static_cast(static_cast(reserved)), prompt, flags, cipher_text) +#define FlushFileBuffers broker_test_flush_file_buffers +#define LookupAccountNameW broker_test_lookup_account_name_w +#define SetNamedSecurityInfoW broker_test_set_named_security_info_w +#define WinHttpCloseHandle broker_test_win_http_close_handle +#define WinHttpConnect broker_test_win_http_connect +#define WinHttpOpen broker_test_win_http_open +#define WinHttpOpenRequest broker_test_win_http_open_request +#define WinHttpQueryDataAvailable broker_test_win_http_query_data_available +#define WinHttpQueryHeaders(request, info_level, name, buffer, buffer_length, index) \ + broker_test_win_http_query_headers(request, info_level, name, static_cast(static_cast(buffer)), buffer_length, index) +#define WinHttpReadData(request, buffer, bytes_to_read, bytes_read) \ + broker_test_win_http_read_data(request, static_cast(static_cast(buffer)), bytes_to_read, bytes_read) +#define WinHttpReceiveResponse(request, reserved) \ + broker_test_win_http_receive_response(request, static_cast(static_cast(reserved))) +#define WinHttpSendRequest(request, headers, headers_length, optional, optional_length, total_length, context) \ + broker_test_win_http_send_request(request, headers, headers_length, static_cast(static_cast(optional)), optional_length, total_length, context) +#define WriteFile(file, buffer, bytes_to_write, bytes_written, overlapped) \ + broker_test_write_file(file, static_cast(static_cast(buffer)), bytes_to_write, bytes_written, overlapped) +#define main libvirtualhid_broker_test_main +#include "../../src/platform/windows/broker/libvirtualhid_broker.cpp" +#undef CloseHandle +#undef CreateDirectoryW +#undef CreateFileW +#undef CryptProtectData +#undef FlushFileBuffers +#undef LookupAccountNameW +#undef SetNamedSecurityInfoW +#undef WinHttpCloseHandle +#undef WinHttpConnect +#undef WinHttpOpen +#undef WinHttpOpenRequest +#undef WinHttpQueryDataAvailable +#undef WinHttpQueryHeaders +#undef WinHttpReadData +#undef WinHttpReceiveResponse +#undef WinHttpSendRequest +#undef WriteFile +#undef main + +namespace lvh::detail::test { + + BrokerPersistenceFailureResult broker_persistence_failure( + BrokerPersistenceFailure failure + ) { + broker_service_test_state().persistence_failure = failure; + broker_service_test_state().persistence_file = INVALID_HANDLE_VALUE; + + std::array temporary_root {}; + if (const auto root_size = ::GetTempPathW(static_cast(temporary_root.size()), temporary_root.data()); root_size == 0U || root_size >= temporary_root.size()) { + return {.message = "Unable to resolve a temporary test directory."}; + } + + const auto directory = std::filesystem::path {temporary_root.data()} / + std::format("libvirtualhid-broker-test-{}", ::GetCurrentProcessId()); + const auto path = directory / "state.dat"; + std::error_code ignored; + std::filesystem::remove_all(directory, ignored); + + std::string message; + const auto saved = lvh::detail::windows_broker_service::save_protected_state( + path, + "test-state", + L"libvirtualhid broker test", + "test", + message + ); + + std::filesystem::remove_all(directory, ignored); + broker_service_test_state().persistence_file = INVALID_HANDLE_VALUE; + return {.saved = saved, .message = std::move(message)}; + } + + BrokerPolarResult broker_polar_scenario(BrokerPolarScenario scenario) { + broker_service_test_state().polar_scenario = scenario; + broker_service_test_state().polar_body_offset = 0; + const auto result = + lvh::detail::windows_broker_service::post_polar_license_request( + L"/test", + nlohmann::json {{"key", "test-key"}} + ); + return { + .transport_ok = result.transport_ok, + .http_status = result.http_status, + .body = result.body, + .error = result.error, + }; + } + +} // namespace lvh::detail::test diff --git a/tests/unit/test_windows_broker_client.cpp b/tests/unit/test_windows_broker_client.cpp index 02c019b..e39372e 100644 --- a/tests/unit/test_windows_broker_client.cpp +++ b/tests/unit/test_windows_broker_client.cpp @@ -20,6 +20,14 @@ namespace { std::uint32_t closed_service_handles; }; + struct PipeCase { + lvh::detail::test::BrokerServiceScenario scenario; + std::string_view name; + std::uint32_t create_attempts; + std::uint32_t sleep_attempts; + std::uint32_t wait_attempts; + }; + } // namespace TEST(WindowsBrokerClientTest, RejectsUnverifiedBrokerServiceEndpoints) { @@ -56,3 +64,50 @@ TEST(WindowsBrokerClientTest, TransactsOnlyWithRunningInstalledBrokerService) { EXPECT_EQ(result.closed_service_handles, 2U); EXPECT_TRUE(result.transacted); } + +TEST(WindowsBrokerClientTest, RetriesWhileTheBrokerPipeIsBeingRecreated) { + using enum lvh::detail::test::BrokerServiceScenario; + + for (const auto &[scenario, name, create_attempts, sleep_attempts, wait_attempts] : { + PipeCase {pipe_unavailable_once, "missing once", 2U, 1U, 0U}, + PipeCase {pipe_busy_once, "busy then available", 2U, 0U, 1U}, + PipeCase {pipe_busy_timeout_once, "busy wait timed out", 2U, 0U, 1U}, + PipeCase {pipe_busy_disappears_once, "busy pipe disappeared", 2U, 0U, 1U}, + }) { + SCOPED_TRACE(name); + const auto result = + lvh::detail::test::verify_broker_service_scenario(scenario); + + EXPECT_TRUE(result.status.ok()); + EXPECT_EQ(result.create_attempts, create_attempts); + EXPECT_EQ(result.sleep_attempts, sleep_attempts); + EXPECT_EQ(result.wait_attempts, wait_attempts); + EXPECT_EQ(result.closed_pipe_handles, 1U); + EXPECT_EQ(result.closed_service_handles, 2U); + EXPECT_TRUE(result.transacted); + } +} + +TEST(WindowsBrokerClientTest, ReportsBrokerPipeConnectionFailures) { + using enum lvh::detail::test::BrokerServiceScenario; + + for (const auto &[scenario, name, create_attempts, sleep_attempts, wait_attempts] : { + PipeCase {pipe_access_denied, "access denied", 1U, 0U, 0U}, + PipeCase {pipe_busy_failure, "busy wait failed", 1U, 0U, 1U}, + PipeCase {pipe_never_available, "retry deadline exhausted", 500U, 500U, 0U}, + }) { + SCOPED_TRACE(name); + const auto result = + lvh::detail::test::verify_broker_service_scenario(scenario); + + EXPECT_FALSE(result.status.ok()); + EXPECT_EQ(result.status.code(), lvh::ErrorCode::backend_unavailable); + EXPECT_FALSE(result.status.message().empty()); + EXPECT_EQ(result.create_attempts, create_attempts); + EXPECT_EQ(result.sleep_attempts, sleep_attempts); + EXPECT_EQ(result.wait_attempts, wait_attempts); + EXPECT_EQ(result.closed_pipe_handles, 0U); + EXPECT_EQ(result.closed_service_handles, 0U); + EXPECT_FALSE(result.transacted); + } +} diff --git a/tests/unit/test_windows_broker_service.cpp b/tests/unit/test_windows_broker_service.cpp new file mode 100644 index 0000000..75fac5c --- /dev/null +++ b/tests/unit/test_windows_broker_service.cpp @@ -0,0 +1,444 @@ +/** + * @file tests/unit/test_windows_broker_service.cpp + * @brief Direct tests for the installed Windows broker service boundary. + */ + +// local includes +#include "fixtures/fixtures.hpp" +#include "fixtures/windows_broker_service_test_hooks.hpp" +#include "lvh_windows_broker_protocol.h" + +#ifndef NOMINMAX + #define NOMINMAX +#endif +#ifndef WIN32_LEAN_AND_MEAN + #define WIN32_LEAN_AND_MEAN +#endif + +// platform includes +#include +#include + +// standard includes +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + + using UniqueServiceHandle = std::unique_ptr; + + constexpr auto pipe_retry_interval = 10U; + constexpr auto pipe_wait_timeout = 5000U; + + struct BrokerResponsePrefix { + std::uint32_t version = 0; + std::uint32_t size = 0; + std::uint32_t status = 0; + std::uint32_t reserved = 0; + }; + + struct RawBrokerResponse { + DWORD error = ERROR_SUCCESS; + std::vector bytes; + }; + + HANDLE open_broker_pipe() { + return ::CreateFileA( + LVH_WINDOWS_BROKER_PIPE_PATH, + GENERIC_READ | GENERIC_WRITE, + 0, + nullptr, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + nullptr + ); + } + + bool wait_to_retry_broker_pipe(DWORD &last_error) { + if (last_error == ERROR_FILE_NOT_FOUND) { + ::Sleep(pipe_retry_interval); + return true; + } + if (last_error != ERROR_PIPE_BUSY) { + return false; + } + if (::WaitNamedPipeA(LVH_WINDOWS_BROKER_PIPE_PATH, pipe_retry_interval) != FALSE) { + return true; + } + + last_error = ::GetLastError(); + return last_error == ERROR_SEM_TIMEOUT || last_error == ERROR_FILE_NOT_FOUND; + } + + HANDLE connect_to_broker() { + DWORD last_error = ERROR_FILE_NOT_FOUND; + for (auto attempt = 0U; attempt < pipe_wait_timeout / pipe_retry_interval; ++attempt) { + if (HANDLE pipe = open_broker_pipe(); pipe != INVALID_HANDLE_VALUE) { + if (DWORD read_mode = PIPE_READMODE_MESSAGE; ::SetNamedPipeHandleState(pipe, &read_mode, nullptr, nullptr) == FALSE) { + const auto error = ::GetLastError(); + static_cast(::CloseHandle(pipe)); + ::SetLastError(error); + return INVALID_HANDLE_VALUE; + } + return pipe; + } + + last_error = ::GetLastError(); + if (!wait_to_retry_broker_pipe(last_error)) { + ::SetLastError(last_error); + return INVALID_HANDLE_VALUE; + } + } + ::SetLastError(last_error); + return INVALID_HANDLE_VALUE; + } + + RawBrokerResponse transact_raw(std::span request) { + const auto pipe = connect_to_broker(); + if (pipe == INVALID_HANDLE_VALUE) { + return {.error = ::GetLastError()}; + } + + if (DWORD bytes_written = 0; ::WriteFile(pipe, request.data(), static_cast(request.size()), &bytes_written, nullptr) == FALSE || bytes_written != request.size()) { + const auto error = ::GetLastError(); + static_cast(::CloseHandle(pipe)); + return {.error = error}; + } + + std::array response_buffer {}; + DWORD bytes_read = 0; + if (::ReadFile(pipe, response_buffer.data(), static_cast(response_buffer.size()), &bytes_read, nullptr) == FALSE) { + const auto error = ::GetLastError(); + static_cast(::CloseHandle(pipe)); + return {.error = error}; + } + static_cast(::CloseHandle(pipe)); + + return { + .bytes = std::vector { + response_buffer.begin(), + response_buffer.begin() + bytes_read, + }, + }; + } + + template + RawBrokerResponse transact_raw(const Request &request) { + return transact_raw(std::as_bytes(std::span {&request, 1})); + } + + BrokerResponsePrefix response_prefix(const RawBrokerResponse &response) { + BrokerResponsePrefix prefix {}; + if (response.bytes.size() >= sizeof(prefix)) { + std::memcpy(&prefix, response.bytes.data(), sizeof(prefix)); + } + return prefix; + } + + LvhWindowsBrokerStatusRequest status_request() { + LvhWindowsBrokerStatusRequest request {}; + request.header.version = LVH_WINDOWS_BROKER_PROTOCOL_VERSION; + request.header.size = sizeof(request); + request.header.type = std::to_underlying(LvhWindowsBrokerRequestType::status); + return request; + } + + bool broker_is_available() { + const auto response = transact_raw(status_request()); + return response.error == ERROR_SUCCESS && + response_prefix(response).status == + std::to_underlying(LvhWindowsBrokerStatusCode::success); + } + + void expect_status( + const RawBrokerResponse &response, + LvhWindowsBrokerStatusCode expected + ) { + ASSERT_EQ(response.error, ERROR_SUCCESS); + ASSERT_GE(response.bytes.size(), sizeof(BrokerResponsePrefix)); + const auto prefix = response_prefix(response); + EXPECT_EQ(prefix.version, LVH_WINDOWS_BROKER_PROTOCOL_VERSION); + EXPECT_EQ(prefix.size, response.bytes.size()); + EXPECT_EQ(prefix.status, std::to_underlying(expected)); + } + + bool wait_for_service_state(SC_HANDLE service, DWORD expected_state) { + const auto deadline = std::chrono::steady_clock::now() + + std::chrono::seconds {15}; + while (std::chrono::steady_clock::now() < deadline) { + SERVICE_STATUS_PROCESS status {}; + if (DWORD bytes_needed = 0; ::QueryServiceStatusEx(service, SC_STATUS_PROCESS_INFO, std::bit_cast(std::as_writable_bytes(std::span {&status, 1}).data()), sizeof(status), &bytes_needed) == FALSE) { + return false; + } + if (status.dwCurrentState == expected_state) { + return true; + } + std::this_thread::sleep_for(std::chrono::milliseconds {100}); + } + return false; + } + + class ServiceStartGuard { + public: + explicit ServiceStartGuard(SC_HANDLE service): + service_ {service} {} + + ~ServiceStartGuard() { + SERVICE_STATUS_PROCESS status {}; + if (DWORD bytes_needed = 0; ::QueryServiceStatusEx(service_, SC_STATUS_PROCESS_INFO, std::bit_cast(std::as_writable_bytes(std::span {&status, 1}).data()), sizeof(status), &bytes_needed) != FALSE && status.dwCurrentState == SERVICE_RUNNING) { + return; + } + if (status.dwCurrentState == SERVICE_START_PENDING) { + static_cast(wait_for_service_state(service_, SERVICE_RUNNING)); + return; + } + if (status.dwCurrentState == SERVICE_STOP_PENDING && !wait_for_service_state(service_, SERVICE_STOPPED)) { + return; + } + + if (::StartServiceW(service_, 0, nullptr) == FALSE && ::GetLastError() != ERROR_SERVICE_ALREADY_RUNNING) { + return; + } + static_cast(wait_for_service_state(service_, SERVICE_RUNNING)); + } + + ServiceStartGuard(const ServiceStartGuard &) = delete; + ServiceStartGuard &operator=(const ServiceStartGuard &) = delete; + + private: + SC_HANDLE service_; + }; + + class WindowsBrokerServiceTest: public WindowsTest { + protected: + void SetUp() override { + if (!broker_is_available()) { + GTEST_SKIP() << "The installed libvirtualhid broker service is unavailable."; + } + } + }; + +} // namespace + +TEST_F(WindowsBrokerServiceTest, RejectsTruncatedOversizedAndUnknownMessages) { + std::array truncated {}; + expect_status( + transact_raw(truncated), + LvhWindowsBrokerStatusCode::invalid_argument + ); + + auto unknown = status_request(); + unknown.header.type = 999U; + expect_status( + transact_raw(unknown), + LvhWindowsBrokerStatusCode::invalid_argument + ); + + std::vector oversized(1024U); + auto oversized_header = status_request().header; + oversized_header.size = static_cast(oversized.size()); + std::memcpy(oversized.data(), &oversized_header, sizeof(oversized_header)); + expect_status( + transact_raw(oversized), + LvhWindowsBrokerStatusCode::invalid_argument + ); +} + +TEST_F(WindowsBrokerServiceTest, RejectsMalformedAndUnterminatedRequests) { + auto malformed_status = status_request(); + malformed_status.header.reserved0 = 1U; + expect_status( + transact_raw(malformed_status), + LvhWindowsBrokerStatusCode::invalid_argument + ); + + LvhWindowsBrokerLicenseRequest unterminated {}; + unterminated.header.version = LVH_WINDOWS_BROKER_PROTOCOL_VERSION; + unterminated.header.size = sizeof(unterminated); + unterminated.header.type = + std::to_underlying(LvhWindowsBrokerRequestType::activate_license); + std::ranges::fill(unterminated.license_key, 'x'); + std::ranges::fill(unterminated.instance_name, 'y'); + expect_status( + transact_raw(unterminated), + LvhWindowsBrokerStatusCode::invalid_argument + ); +} + +TEST_F(WindowsBrokerServiceTest, RecoversFromClientDisconnectsAndConcurrentCalls) { + { + const auto pipe = connect_to_broker(); + ASSERT_NE(pipe, INVALID_HANDLE_VALUE); + std::array partial {}; + DWORD bytes_written = 0; + const auto wrote_partial_request = + ::WriteFile( + pipe, + partial.data(), + static_cast(partial.size()), + &bytes_written, + nullptr + ); + static_cast(::CloseHandle(pipe)); + ASSERT_NE(wrote_partial_request, FALSE); + } + + expect_status( + transact_raw(status_request()), + LvhWindowsBrokerStatusCode::success + ); + + std::array responses; + { + std::array calls; + for (std::size_t index = 0; index < calls.size(); ++index) { + calls[index] = std::jthread {[&responses, index]() { + responses[index] = transact_raw(status_request()); + }}; + } + } + for (const auto &response : responses) { + expect_status(response, LvhWindowsBrokerStatusCode::success); + } +} + +TEST_F(WindowsBrokerServiceTest, DriverRejectsDirectGamepadCreation) { + const auto control = ::CreateFileA( + LVH_WINDOWS_CONTROL_DEVICE_PATH, + GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, + nullptr, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + nullptr + ); + if (control == INVALID_HANDLE_VALUE) { + GTEST_SKIP() << "The installed libvirtualhid control device is unavailable."; + } + + LvhWindowsCreateGamepadRequest request {}; + request.version = LVH_WINDOWS_CONTROL_PROTOCOL_VERSION; + request.size = sizeof(request); + LvhWindowsCreateGamepadResponse response {}; + DWORD bytes_returned = 0; + const auto request_result = + ::DeviceIoControl( + control, + LVH_WINDOWS_IOCTL_CREATE_GAMEPAD, + &request, + sizeof(request), + &response, + sizeof(response), + &bytes_returned, + nullptr + ); + const auto request_error = ::GetLastError(); + static_cast(::CloseHandle(control)); + EXPECT_EQ(request_result, FALSE); + EXPECT_EQ(request_error, ERROR_ACCESS_DENIED); +} + +TEST(WindowsBrokerImplementationTest, ReportsDpapiAndPolarFailures) { + const auto dpapi_result = lvh::detail::test::broker_persistence_failure( + lvh::detail::test::BrokerPersistenceFailure::dpapi + ); + EXPECT_FALSE(dpapi_result.saved); + EXPECT_TRUE(dpapi_result.message.starts_with("Unable to protect test state:")) + << dpapi_result.message; + + using enum lvh::detail::test::BrokerPolarScenario; + for (const auto scenario : { + open_failure, + connect_failure, + request_failure, + send_failure, + receive_failure, + }) { + const auto result = lvh::detail::test::broker_polar_scenario(scenario); + EXPECT_FALSE(result.transport_ok); + EXPECT_FALSE(result.error.empty()); + } + + const auto structured = + lvh::detail::test::broker_polar_scenario(structured_error); + EXPECT_TRUE(structured.transport_ok); + EXPECT_EQ(structured.http_status, 422U); + EXPECT_EQ(structured.error, "License activation was rejected."); + + const auto malformed = + lvh::detail::test::broker_polar_scenario(malformed_error); + EXPECT_TRUE(malformed.transport_ok); + EXPECT_EQ(malformed.http_status, 422U); + EXPECT_EQ(malformed.error, "The license service returned an error."); + + const auto succeeded = lvh::detail::test::broker_polar_scenario(success); + EXPECT_TRUE(succeeded.transport_ok); + EXPECT_EQ(succeeded.http_status, 200U); + EXPECT_TRUE(succeeded.error.empty()); +} + +TEST(WindowsBrokerImplementationTest, ReportsFilePersistenceFailures) { + using enum lvh::detail::test::BrokerPersistenceFailure; + for (const auto &[failure, expected_message] : + std::array { + std::pair {create_file, "Unable to write test state:"}, + std::pair {partial_write, "Unable to persist test state:"}, + std::pair {flush, "Unable to persist test state:"}, + std::pair {close, "Unable to close test state:"}, + }) { + const auto result = + lvh::detail::test::broker_persistence_failure(failure); + EXPECT_FALSE(result.saved); + EXPECT_TRUE(result.message.starts_with(expected_message)) << result.message; + } +} + +TEST_F(WindowsBrokerServiceTest, RestartsWithoutLosingTheServiceBoundary) { + auto manager = UniqueServiceHandle { + ::OpenSCManagerW(nullptr, nullptr, SC_MANAGER_CONNECT), + &::CloseServiceHandle, + }; + ASSERT_TRUE(manager); + + auto service = UniqueServiceHandle { + ::OpenServiceW( + manager.get(), + L"libvirtualhid_broker", + SERVICE_QUERY_STATUS | SERVICE_START | SERVICE_STOP + ), + &::CloseServiceHandle, + }; + if (!service && ::GetLastError() == ERROR_ACCESS_DENIED) { + GTEST_SKIP() << "Restarting the broker service requires elevation."; + } + ASSERT_TRUE(service); + const ServiceStartGuard service_start_guard {service.get()}; + + if (SERVICE_STATUS status {}; ::ControlService(service.get(), SERVICE_CONTROL_STOP, &status) == FALSE) { + const auto error = ::GetLastError(); + if (error != ERROR_SERVICE_NOT_ACTIVE) { + FAIL() << "Unable to stop the broker service: " << error; + } + } + ASSERT_TRUE(wait_for_service_state(service.get(), SERVICE_STOPPED)); + ASSERT_NE(::StartServiceW(service.get(), 0, nullptr), FALSE); + ASSERT_TRUE(wait_for_service_state(service.get(), SERVICE_RUNNING)); + + expect_status( + transact_raw(status_request()), + LvhWindowsBrokerStatusCode::success + ); +}