From 352fd38c3b0844f9ab0ed913d2ce76277ab1258f Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Tue, 22 Apr 2025 07:17:11 +1000 Subject: [PATCH] Update `Get-Service` to ignore common errors when retrieving non-critical properties for a service (#24245) --- .../commands/management/Service.cs | 217 ++++++++++-------- .../resources/ServiceResources.resx | 5 +- .../Get-Service.Tests.ps1 | 57 +++++ 3 files changed, 175 insertions(+), 104 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs index d2a96b5086..739baa15ba 100644 --- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs +++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs @@ -610,146 +610,163 @@ namespace Microsoft.PowerShell.Commands /// protected override void ProcessRecord() { - foreach (ServiceController service in MatchingServices()) + nint scManagerHandle = nint.Zero; + if (!DependentServices && !RequiredServices) { - if (!DependentServices.IsPresent && !RequiredServices.IsPresent) + // As Get-Service only works on local services we get this once + // to retrieve extra properties added by PowerShell. + scManagerHandle = NativeMethods.OpenSCManagerW( + lpMachineName: null, + lpDatabaseName: null, + dwDesiredAccess: NativeMethods.SC_MANAGER_CONNECT); + if (scManagerHandle == nint.Zero) { - WriteObject(AddProperties(service)); + Win32Exception exception = new(); + string message = StringUtil.Format(ServiceResources.FailToOpenServiceControlManager, exception.Message); + ServiceCommandException serviceException = new ServiceCommandException(message, exception); + ErrorRecord err = new ErrorRecord( + serviceException, + "FailToOpenServiceControlManager", + ErrorCategory.PermissionDenied, + null); + ThrowTerminatingError(err); } - else - { - if (DependentServices.IsPresent) - { - foreach (ServiceController dependantserv in service.DependentServices) - { - WriteObject(dependantserv); - } - } + } - if (RequiredServices.IsPresent) + try + { + foreach (ServiceController service in MatchingServices()) + { + if (!DependentServices.IsPresent && !RequiredServices.IsPresent) { - foreach (ServiceController servicedependedon in service.ServicesDependedOn) + WriteObject(AddProperties(scManagerHandle, service)); + } + else + { + if (DependentServices.IsPresent) { - WriteObject(servicedependedon); + foreach (ServiceController dependantserv in service.DependentServices) + { + WriteObject(dependantserv); + } + } + + if (RequiredServices.IsPresent) + { + foreach (ServiceController servicedependedon in service.ServicesDependedOn) + { + WriteObject(servicedependedon); + } } } } } + finally + { + if (scManagerHandle != nint.Zero) + { + bool succeeded = NativeMethods.CloseServiceHandle(scManagerHandle); + Diagnostics.Assert(succeeded, "SCManager handle close failed"); + } + } } #endregion Overrides +#nullable enable /// /// Adds UserName, Description, BinaryPathName, DelayedAutoStart and StartupType to a ServiceController object. /// + /// Handle to the local SCManager instance. /// /// ServiceController as PSObject with UserName, Description and StartupType added. - private PSObject AddProperties(ServiceController service) + private static PSObject AddProperties(nint scManagerHandle, ServiceController service) { - NakedWin32Handle hScManager = IntPtr.Zero; - NakedWin32Handle hService = IntPtr.Zero; - int lastError = 0; - PSObject serviceAsPSObj = PSObject.AsPSObject(service); + NakedWin32Handle hService = nint.Zero; + + // As these are optional values, a failure due to permissions or + // other problem is ignored and the properties are set to null. + bool? isDelayedAutoStart = null; + string? binPath = null; + string? description = null; + string? startName = null; + ServiceStartupType startupType = ServiceStartupType.InvalidValue; try { - hScManager = NativeMethods.OpenSCManagerW( - lpMachineName: service.MachineName, - lpDatabaseName: null, - dwDesiredAccess: NativeMethods.SC_MANAGER_CONNECT - ); - if (hScManager == IntPtr.Zero) - { - lastError = Marshal.GetLastWin32Error(); - Win32Exception exception = new(lastError); - WriteNonTerminatingError( - service, - exception, - "FailToOpenServiceControlManager", - ServiceResources.FailToOpenServiceControlManager, - ErrorCategory.PermissionDenied); - } - + // We don't use service.ServiceHandle as that requests + // SERVICE_ALL_ACCESS when we only need SERVICE_QUERY_CONFIG. hService = NativeMethods.OpenServiceW( - hScManager, + scManagerHandle, service.ServiceName, NativeMethods.SERVICE_QUERY_CONFIG ); - if (hService == IntPtr.Zero) + if (hService != nint.Zero) { - lastError = Marshal.GetLastWin32Error(); - Win32Exception exception = new(lastError); - WriteNonTerminatingError( - service, - exception, - "CouldNotGetServiceInfo", - ServiceResources.CouldNotGetServiceInfo, - ErrorCategory.PermissionDenied); + if (NativeMethods.QueryServiceConfig2( + hService, + NativeMethods.SERVICE_CONFIG_DESCRIPTION, + out NativeMethods.SERVICE_DESCRIPTIONW descriptionInfo)) + { + description = descriptionInfo.lpDescription; + } + + if (NativeMethods.QueryServiceConfig2( + hService, + NativeMethods.SERVICE_CONFIG_DELAYED_AUTO_START_INFO, + out NativeMethods.SERVICE_DELAYED_AUTO_START_INFO autostartInfo)) + { + isDelayedAutoStart = autostartInfo.fDelayedAutostart; + } + + if (NativeMethods.QueryServiceConfig( + hService, + out NativeMethods.QUERY_SERVICE_CONFIG serviceInfo)) + { + binPath = serviceInfo.lpBinaryPathName; + startName = serviceInfo.lpServiceStartName; + if (isDelayedAutoStart.HasValue) + { + startupType = NativeMethods.GetServiceStartupType( + (ServiceStartMode)serviceInfo.dwStartType, + isDelayedAutoStart.Value); + } + } } - - NativeMethods.SERVICE_DESCRIPTIONW description = new(); - bool querySuccessful = NativeMethods.QueryServiceConfig2(hService, NativeMethods.SERVICE_CONFIG_DESCRIPTION, out description); - - NativeMethods.SERVICE_DELAYED_AUTO_START_INFO autostartInfo = new(); - querySuccessful = querySuccessful && NativeMethods.QueryServiceConfig2(hService, NativeMethods.SERVICE_CONFIG_DELAYED_AUTO_START_INFO, out autostartInfo); - - NativeMethods.QUERY_SERVICE_CONFIG serviceInfo = new(); - querySuccessful = querySuccessful && NativeMethods.QueryServiceConfig(hService, out serviceInfo); - - if (!querySuccessful) - { - WriteNonTerminatingError( - service: service, - innerException: null, - errorId: "CouldNotGetServiceInfo", - errorMessage: ServiceResources.CouldNotGetServiceInfo, - category: ErrorCategory.PermissionDenied - ); - } - - PSProperty noteProperty = new("UserName", serviceInfo.lpServiceStartName); - serviceAsPSObj.Properties.Add(noteProperty, true); - serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#UserName"); - - noteProperty = new PSProperty("Description", description.lpDescription); - serviceAsPSObj.Properties.Add(noteProperty, true); - serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#Description"); - - noteProperty = new PSProperty("DelayedAutoStart", autostartInfo.fDelayedAutostart); - serviceAsPSObj.Properties.Add(noteProperty, true); - serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#DelayedAutoStart"); - - noteProperty = new PSProperty("BinaryPathName", serviceInfo.lpBinaryPathName); - serviceAsPSObj.Properties.Add(noteProperty, true); - serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#BinaryPathName"); - - noteProperty = new PSProperty("StartupType", NativeMethods.GetServiceStartupType(service.StartType, autostartInfo.fDelayedAutostart)); - serviceAsPSObj.Properties.Add(noteProperty, true); - serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#StartupType"); } finally { if (hService != IntPtr.Zero) { bool succeeded = NativeMethods.CloseServiceHandle(hService); - if (!succeeded) - { - Diagnostics.Assert(lastError != 0, "ErrorCode not success"); - } - } - - if (hScManager != IntPtr.Zero) - { - bool succeeded = NativeMethods.CloseServiceHandle(hScManager); - if (!succeeded) - { - Diagnostics.Assert(lastError != 0, "ErrorCode not success"); - } + Diagnostics.Assert(succeeded, "Failed to close service handle"); } } + PSObject serviceAsPSObj = PSObject.AsPSObject(service); + PSNoteProperty noteProperty = new("UserName", startName); + serviceAsPSObj.Properties.Add(noteProperty, true); + serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#UserName"); + + noteProperty = new PSNoteProperty("Description", description); + serviceAsPSObj.Properties.Add(noteProperty, true); + serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#Description"); + + noteProperty = new PSNoteProperty("DelayedAutoStart", isDelayedAutoStart); + serviceAsPSObj.Properties.Add(noteProperty, true); + serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#DelayedAutoStart"); + + noteProperty = new PSNoteProperty("BinaryPathName", binPath); + serviceAsPSObj.Properties.Add(noteProperty, true); + serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#BinaryPathName"); + + noteProperty = new PSNoteProperty("StartupType", startupType); + serviceAsPSObj.Properties.Add(noteProperty, true); + serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#StartupType"); + return serviceAsPSObj; } } +#nullable disable #endregion GetServiceCommand #region ServiceOperationBaseCommand diff --git a/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx b/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx index 3792ce8db9..e1213c3fb0 100644 --- a/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx +++ b/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx @@ -159,9 +159,6 @@ Service '{1} ({0})' cannot be configured due to the following error: {2} - - Service '{1} ({0})' cannot be queried due to the following error: {2} - Service '{1} ({0})' description cannot be configured due to the following error: {2} @@ -211,7 +208,7 @@ Service '{1} ({0})' resume failed. - Failed to configure the service '{1} ({0})' due to the following error: {2}. Run PowerShell as admin and run your command again. + Failed to open SCManager due to the following error: {0}. Run PowerShell as admin and run your command again. The startup type '{0}' is not supported by {1}. diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Get-Service.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Get-Service.Tests.ps1 index 9429229c53..04882a1aaf 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Get-Service.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Get-Service.Tests.ps1 @@ -82,3 +82,60 @@ Describe "Get-Service cmdlet tests" -Tags "CI" { { & $script } | Should -Throw -ErrorId $errorid } } + +Describe 'Get-Service Admin tests' -Tag CI,RequireAdminOnWindows { + BeforeAll { + $originalDefaultParameterValues = $PSDefaultParameterValues.Clone() + if ( -not $IsWindows ) { + $PSDefaultParameterValues["it:skip"] = $true + } + } + AfterAll { + $global:PSDefaultParameterValues = $originalDefaultParameterValues + } + + BeforeEach { + $serviceParams = @{ + Name = "PowerShellTest-$([Guid]::NewGuid().Guid)" + BinaryPathName = "$env:SystemRoot\System32\cmd.exe" + StartupType = 'Manual' + } + $service = New-Service @serviceParams + } + AfterEach { + $service | Remove-Service + } + + It "Ignores invalid description MUI entry" { + $serviceRegPath = "HKLM:\SYSTEM\CurrentControlSet\Services\$($service.Name)" + Set-ItemProperty -LiteralPath $serviceRegPath -Name Description -Value '@Fake.dll,-0' + $actual = Get-Service -Name $service.Name -ErrorAction Stop + + $actual.Name | Should -Be $service.Name + $actual.Status | Should -Be Stopped + $actual.Description | Should -BeNullOrEmpty + $actual.UserName | Should -Be LocalSystem + $actual.StartupType | Should -Be Manual + } + + It "Ignores no SERVICE_QUERY_CONFIG access" { + $sddl = ((sc.exe sdshow $service.Name) -join "").Trim() + $sd = ConvertFrom-SddlString -Sddl $sddl + $sd.RawDescriptor.DiscretionaryAcl.AddAccess( + [System.Security.AccessControl.AccessControlType]::Deny, + [System.Security.Principal.WindowsIdentity]::GetCurrent().User, + 0x1, # SERVICE_QUERY_CONFIG + [System.Security.AccessControl.InheritanceFlags]::None, + [System.Security.AccessControl.PropagationFlags]::None) + $newSddl = $sd.RawDescriptor.GetSddlForm([System.Security.AccessControl.AccessControlSections]::All) + $null = sc.exe sdset $service.Name $newSddl + + $actual = Get-Service -Name $service.Name -ErrorAction Stop + + $actual.Name | Should -Be $service.Name + $actual.Status | Should -Be Stopped + $actual.Description | Should -BeNullOrEmpty + $actual.UserName | Should -BeNullOrEmpty + $actual.StartupType | Should -Be InvalidValue + } +}