From ffb1549aa91688bfc910da214fca56b701554aec Mon Sep 17 00:00:00 2001 From: PRASOON KARUNAN V <12897753+kvprasoon@users.noreply.github.com> Date: Fri, 12 Jul 2019 01:53:57 +0530 Subject: [PATCH] Add -SecurityDescriptorSddl parameter to Set-Service (#8626) --- .../commands/management/Service.cs | 62 ++++++++++++++++++- .../resources/ServiceResources.resx | 5 +- .../utils/PInvokeDllNames.cs | 1 + .../Set-Service.Tests.ps1 | 47 ++++++++++---- 4 files changed, 101 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs index 6700a582d..7fe3c06e7 100644 --- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs +++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs @@ -14,6 +14,7 @@ using System.ComponentModel; // Win32Exception using System.Runtime.Serialization; using System.Runtime.InteropServices; // Marshal, DllImport using System.Security.Permissions; +using System.Security.AccessControl; using NakedWin32Handle = System.IntPtr; using DWORD = System.UInt32; @@ -1560,10 +1561,23 @@ namespace Microsoft.PowerShell.Commands startupType = value; } } + // We set the initial value to an invalid value so that we can // distinguish when this is and is not set. internal ServiceStartupType startupType = ServiceStartupType.InvalidValue; + /// + /// Sets the SecurityDescriptorSddl of the service using a SDDL string. + /// + [Parameter] + [Alias("sd")] + [ValidateNotNullOrEmpty] + public string SecurityDescriptorSddl + { + get; + set; + } + /// /// The following is the definition of the input parameter "Status". /// This specifies what state the service should be in (e.g. Running, Stopped, @@ -1718,8 +1732,9 @@ namespace Microsoft.PowerShell.Commands hService = NativeMethods.OpenServiceW( hScManager, Name, - NativeMethods.SERVICE_CHANGE_CONFIG + NativeMethods.SERVICE_CHANGE_CONFIG | NativeMethods.WRITE_DAC | NativeMethods.WRITE_OWNER ); + if (IntPtr.Zero == hService) { int lastError = Marshal.GetLastWin32Error(); @@ -1870,6 +1885,37 @@ namespace Microsoft.PowerShell.Commands } } + if(!string.IsNullOrEmpty(SecurityDescriptorSddl)) + { + var rawSecurityDescriptor = new RawSecurityDescriptor(SecurityDescriptorSddl); + RawAcl rawDiscretionaryAcl = rawSecurityDescriptor.DiscretionaryAcl ; + var discretionaryAcl = new DiscretionaryAcl (false, false, rawDiscretionaryAcl ); + + byte[] rawDacl = new byte[discretionaryAcl.BinaryLength]; + discretionaryAcl.GetBinaryForm(rawDacl, 0); + rawSecurityDescriptor.DiscretionaryAcl = new RawAcl(rawDacl, 0); + byte[] securityDescriptorByte = new byte[rawSecurityDescriptor.BinaryLength]; + rawSecurityDescriptor.GetBinaryForm(securityDescriptorByte, 0); + + status = NativeMethods.SetServiceObjectSecurity( + hService, + SecurityInfos.DiscretionaryAcl, + securityDescriptorByte); + + if (!status) + { + int lastError = Marshal.GetLastWin32Error(); + Win32Exception exception = new Win32Exception(lastError); + bool accessDenied = exception.NativeErrorCode == NativeMethods.ERROR_ACCESS_DENIED; + WriteNonTerminatingError( + service, + exception, + nameof(ServiceResources.CouldNotSetServiceSecurityDescriptorSddl), + StringUtil.Format(ServiceResources.CouldNotSetServiceSecurityDescriptorSddl, Name, exception.Message), + accessDenied ? ErrorCategory.PermissionDenied : ErrorCategory.InvalidOperation); + } + } + if (PassThru.IsPresent) { // To display the service, refreshing the service would not show the display name after updating @@ -2554,6 +2600,7 @@ namespace Microsoft.PowerShell.Commands internal const int ERROR_SERVICE_ALREADY_RUNNING = 1056; internal const int ERROR_SERVICE_NOT_ACTIVE = 1062; internal const int ERROR_INSUFFICIENT_BUFFER = 122; + internal const DWORD ERROR_ACCESS_DENIED = 0x5; internal const DWORD SC_MANAGER_CONNECT = 1; internal const DWORD SC_MANAGER_CREATE_SERVICE = 2; internal const DWORD SC_MANAGER_ALL_ACCESS = 0xf003f; @@ -2567,7 +2614,8 @@ namespace Microsoft.PowerShell.Commands internal const DWORD SERVICE_CONFIG_DESCRIPTION = 1; internal const DWORD SERVICE_CONFIG_DELAYED_AUTO_START_INFO = 3; internal const DWORD SERVICE_CONFIG_SERVICE_SID_INFO = 5; - + internal const DWORD WRITE_DAC = 262144; + internal const DWORD WRITE_OWNER =524288; internal const DWORD SERVICE_WIN32_OWN_PROCESS = 0x10; internal const DWORD SERVICE_ERROR_NORMAL = 1; @@ -2688,6 +2736,16 @@ namespace Microsoft.PowerShell.Commands [In] IntPtr lpPassword ); + + [DllImport(PinvokeDllNames.SetServiceObjectSecurityDllName, CharSet = CharSet.Unicode, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern + bool SetServiceObjectSecurity( + NakedWin32Handle hSCManager, + System.Security.AccessControl.SecurityInfos dwSecurityInformation, + byte[] lpSecurityDescriptor + ); + /// /// CreateJobObject API creates or opens a job object. /// diff --git a/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx b/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx index 7f8538e5d..3792ce8db 100644 --- a/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx +++ b/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx @@ -168,6 +168,9 @@ Service '{1} ({0})' automatic (delayed start) cannot be configured due to the following error: {2} + + Service '{0}' security descriptor cannot be configured due to the following error: {1} + Service '{1} ({0})' cannot be created due to the following error: {2} @@ -179,7 +182,7 @@ Service '{1} ({0})' cannot be removed due to the following error: {2} - + 'Cannot access dependent services of '{1} ({0})' diff --git a/src/System.Management.Automation/utils/PInvokeDllNames.cs b/src/System.Management.Automation/utils/PInvokeDllNames.cs index a9c8cc022..d0ce5ea84 100644 --- a/src/System.Management.Automation/utils/PInvokeDllNames.cs +++ b/src/System.Management.Automation/utils/PInvokeDllNames.cs @@ -137,5 +137,6 @@ namespace System.Management.Automation internal const string DeleteServiceDllName = "api-ms-win-service-management-l1-1-0.dll"; /*124*/ internal const string QueryServiceConfigDllName = "api-ms-win-service-management-l2-1-0.dll"; /*125*/ internal const string QueryServiceConfig2DllName = "api-ms-win-service-management-l2-1-0.dll"; /*126*/ + internal const string SetServiceObjectSecurityDllName = "api-ms-win-service-management-l2-1-0.dll"; /*127*/ } } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 index 5020e3dce..98963a306 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 @@ -1,5 +1,7 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +Import-Module (Join-Path -Path $PSScriptRoot '..\Microsoft.PowerShell.Security\certificateCommon.psm1') + Describe "Set/New/Remove-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows" { BeforeAll { $originalDefaultParameterValues = $PSDefaultParameterValues.Clone() @@ -8,14 +10,16 @@ Describe "Set/New/Remove-Service cmdlet tests" -Tags "Feature", "RequireAdminOnW } if ($IsWindows) { $userName = "testuserservices" - $testPass = "Secret123!" - net user $userName $testPass /add > $null - $password = ConvertTo-SecureString $testPass -AsPlainText -Force - $creds = [pscredential]::new(".\$userName", $password) + $Password = ([char[]]([char]33..[char]95) + ([char[]]([char]97..[char]126)) + 0..9 | Sort-Object {Get-Random})[0..12] -join '' + $testPass = (New-Object -TypeName Net.NetworkCredential("", $Password)).SecurePassword + $creds = [pscredential]::new(".\$userName", $testPass) + $SecurityDescriptorSddl = 'D:(A;;CCLCSWRPWPDTLOCRRC;;;SY)(A;;CCDCLCSWRPWPDTLOCRSDRCWDWO;;;BA)(A;;CCLCSWLOCRRC;;;SU)' + $WrongSecurityDescriptorSddl = 'D:(A;;CCLCSWRPWPDTLOCRRC;;;SY)(A;;CCDCLCSWRPWPDTLOCRSDRCWDWO;;;BB)(A;;CCLCSWLOCRRC;;;SU)' + net user $userName $creds.GetNetworkCredential().Password /add > $null $testservicename1 = "testservice1" $testservicename2 = "testservice2" - $svcbinaryname = "TestService" + $svcbinaryname = "TestService" $svccmd = Get-Command $svcbinaryname $svccmd | Should -Not -BeNullOrEmpty $svcfullpath = $svccmd.Path @@ -72,12 +76,34 @@ Describe "Set/New/Remove-Service cmdlet tests" -Tags "Feature", "RequireAdminOnW @{ script = {Set-Service foo -StartupType bar -ErrorAction Stop}; errorid = "CannotConvertArgumentNoMessage,Microsoft.PowerShell.Commands.SetServiceCommand" + }, + @{ + script = {Set-Service -Name $testservicename1 -SecurityDescriptorSddl $WrongSecurityDescriptorSddl }; + errorid = "System.ArgumentException,Microsoft.PowerShell.Commands.SetServiceCommand" } ) { param($script, $errorid) { & $script } | Should -Throw -ErrorId $errorid } + + It "Sets securitydescriptor of service using Set-Service " { + Set-Service -Name $TestServiceName1 -SecurityDescriptor $SecurityDescriptorSddl + $Counter = 0 + $ExpectedSDDL = ConvertFrom-SddlString -Sddl $SecurityDescriptorSddl + + # Selecting the first item in the output array as below command gives plain text output from the native sc.exe. + $UpdatedSDDL = ConvertFrom-SddlString -Sddl (sc sdshow $TestServiceName1)[1] + + $UpdatedSDDL.Owner | Should -Be $ExpectedSDDL.Owner + $UpdatedSDDL.Group | Should -Be $ExpectedSDDL.Group + $UpdatedSDDL.DiscretionaryAcl.Count | Should -Be $ExpectedSDDL.DiscretionaryAcl.Count + $UpdatedSDDL.DiscretionaryAcl | ForEach-Object -Process { + $_ | Should -Be $ExpectedSDDL.DiscretionaryAcl[$Counter] + $Counter++ + } + } + It "Set-Service can change '' to ''" -TestCases @( @{parameter = "Description"; value = "hello"}, @{parameter = "DisplayName"; value = "test spooler"}, @@ -142,12 +168,12 @@ Describe "Set/New/Remove-Service cmdlet tests" -Tags "Feature", "RequireAdminOnW try { $startUsername = "user1" $endUsername = "user2" - $testPass = "Secret123!" $servicename = "testsetcredential" - net user $startUsername $testPass /add > $null - net user $endUsername $testPass /add > $null - $password = ConvertTo-SecureString $testPass -AsPlainText -Force - $creds = [pscredential]::new(".\$startUsername", $password) + $Password = ([char[]]([char]33..[char]95) + ([char[]]([char]97..[char]126)) + 0..9 | Sort-Object {Get-Random})[0..12] -join '' + $testPass = (New-Object -TypeName Net.NetworkCredential("", $Password)).SecurePassword + $creds = [pscredential]::new(".\$endUsername", $testPass) + net user $startUsername $creds.GetNetworkCredential().Password /add > $null + net user $endUsername $creds.GetNetworkCredential().Password /add > $null $parameters = @{ Name = $servicename; BinaryPathName = "$PSHOME\pwsh.exe"; @@ -159,7 +185,6 @@ Describe "Set/New/Remove-Service cmdlet tests" -Tags "Feature", "RequireAdminOnW $service = Get-CimInstance Win32_Service -Filter "name='$servicename'" $service.StartName | Should -BeExactly $creds.UserName - $creds = [pscredential]::new(".\$endUsername", $password) Set-Service -Name $servicename -Credential $creds $service = Get-CimInstance Win32_Service -Filter "name='$servicename'" $service.StartName | Should -BeExactly $creds.UserName