From b686ec432ef08037ff6fd0349e11b1d77341e2c5 Mon Sep 17 00:00:00 2001 From: Jonas Andersen Date: Fri, 15 Sep 2017 09:32:07 +0200 Subject: [PATCH 1/9] PowerShell/PowerShell#4843 Credentials on Set-Service --- .../commands/management/Service.cs | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs index c8986ab7089..e8c042be1c7 100644 --- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs +++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs @@ -1466,6 +1466,19 @@ public class SetServiceCommand : ServiceOperationBaseCommand } internal string displayName = null; + /// + /// Account under which the service should run + /// + /// + [Parameter] + [Credential()] + public PSCredential Credential + { + get { return credential; } + set { credential = value; } + } + internal PSCredential credential = null; + /// @@ -1589,6 +1602,7 @@ protected override void ProcessRecord() { ServiceController service = null; string ServiceComputerName = null; + IntPtr password = IntPtr.Zero; foreach (string computer in ComputerName) { bool objServiceShouldBeDisposed = false; @@ -1679,9 +1693,9 @@ protected override void ProcessRecord() continue; } - // modify startup type or display name + // modify startup type or display name or credential if (!String.IsNullOrEmpty(DisplayName) - || (ServiceStartMode)(-1) != StartupType) + || (ServiceStartMode)(-1) != StartupType || null != Credential) { DWORD dwStartType = NativeMethods.SERVICE_NO_CHANGE; switch (StartupType) @@ -1701,6 +1715,14 @@ protected override void ProcessRecord() "bad StartupType"); break; } + + // set up the Credential parameter + string username = null; + if (null != Credential) + { + username = Credential.UserName; + password = Marshal.SecureStringToCoTaskMemUnicode(Credential.Password); + } bool succeeded = NativeMethods.ChangeServiceConfigW( hService, NativeMethods.SERVICE_NO_CHANGE, @@ -1710,8 +1732,8 @@ protected override void ProcessRecord() null, IntPtr.Zero, null, - null, - IntPtr.Zero, + username, + password, DisplayName ); if (!succeeded) @@ -1847,6 +1869,10 @@ protected override void ProcessRecord() } //End try finally { + if (IntPtr.Zero != password) + { + Marshal.ZeroFreeCoTaskMemUnicode(password); + } if (objServiceShouldBeDisposed) { service.Dispose(); From 58383f420b810111509bf72f66b1100a656f7cdd Mon Sep 17 00:00:00 2001 From: Jonas Andersen Date: Fri, 15 Sep 2017 12:00:02 +0200 Subject: [PATCH 2/9] Clean up --- .../commands/management/Service.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs index e8c042be1c7..61d4232705d 100644 --- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs +++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs @@ -1472,12 +1472,7 @@ public class SetServiceCommand : ServiceOperationBaseCommand /// [Parameter] [Credential()] - public PSCredential Credential - { - get { return credential; } - set { credential = value; } - } - internal PSCredential credential = null; + public PSCredential Credential { get; set; } @@ -1693,7 +1688,7 @@ protected override void ProcessRecord() continue; } - // modify startup type or display name or credential + // Modify startup type or display name or credential if (!String.IsNullOrEmpty(DisplayName) || (ServiceStartMode)(-1) != StartupType || null != Credential) { @@ -1716,7 +1711,6 @@ protected override void ProcessRecord() break; } - // set up the Credential parameter string username = null; if (null != Credential) { From 6fe54d4736579c79e92487e5ac8a4b1c620cb0ed Mon Sep 17 00:00:00 2001 From: Jonas Andersen Date: Fri, 15 Sep 2017 13:55:08 +0200 Subject: [PATCH 3/9] [Feature]Test added for Set-Service with credentials --- .../Set-Service.Tests.ps1 | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) 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 5a30db2fe0b..1497a29b31e 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 @@ -110,6 +110,39 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows" $newServiceCommand.$parameter | Should Be $value } + It "Set-Service can create a new service called '' and change the credentials" -TestCases @( + @{name = "testsetcredential"; + startCredential = [System.Management.Automation.PSCredential]::new("username", + (ConvertTo-SecureString "PlainTextPassword" -AsPlainText -Force)); + endCredential = [System.Management.Automation.PSCredential]::new("username2", + (ConvertTo-SecureString "PlainTextPassword" -AsPlainText -Force))} + ) { + param($name, $startCredential, $endCredential) + try { + $parameters = @{ + Name = $name; + BinaryPathName = "$PSHOME\powershell.exe"; + StartupType = "Manual"; + Credential = $startCredential + } + $service = New-Service @parameters + $service | Should Not BeNullOrEmpty + $service = Get-CimInstance Win32_Service -Filter "name='$name'" + $service | Should Not BeNullOrEmpty + $service.StartName | Should Be $startCredential.UserName + Set-Service -Name $name -Credential $endCredential + $service = Get-CimInstance Win32_Service -Filter "name='$name'" + $service | Should Not BeNullOrEmpty + $service.StartName | Should Be $endCredential.UserName + } + finally { + $service = Get-CimInstance Win32_Service -Filter "name='$name'" + if ($service -ne $null) { + $service | Remove-CimInstance + } + } + } + It "New-Service can create a new service called ''" -TestCases @( @{name = "testautomatic"; startupType = "Automatic"; description = "foo" ; displayname = "one"}, @{name = "testmanual" ; startupType = "Manual" ; description = "bar" ; displayname = "two"}, From f09e87d4e4dff07368073b1629375f25ad4d8e88 Mon Sep 17 00:00:00 2001 From: Jonas Andersen Date: Fri, 15 Sep 2017 14:47:39 +0200 Subject: [PATCH 4/9] [Feature]Test with windows builtin users --- .../Microsoft.PowerShell.Management/Set-Service.Tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 1497a29b31e..ea3aaf5a263 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 @@ -112,9 +112,9 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows" It "Set-Service can create a new service called '' and change the credentials" -TestCases @( @{name = "testsetcredential"; - startCredential = [System.Management.Automation.PSCredential]::new("username", + startCredential = [System.Management.Automation.PSCredential]::new(".\Guest", (ConvertTo-SecureString "PlainTextPassword" -AsPlainText -Force)); - endCredential = [System.Management.Automation.PSCredential]::new("username2", + endCredential = [System.Management.Automation.PSCredential]::new(".\Administrator", (ConvertTo-SecureString "PlainTextPassword" -AsPlainText -Force))} ) { param($name, $startCredential, $endCredential) From d1a02ce21e06744ab011c2ecbfeb4400447df062 Mon Sep 17 00:00:00 2001 From: Jonas Andersen Date: Fri, 15 Sep 2017 16:18:39 +0200 Subject: [PATCH 5/9] [Feature]Descriptive test in Set-Service with credentials --- .../Microsoft.PowerShell.Management/Set-Service.Tests.ps1 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 ea3aaf5a263..2590eaee8aa 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 @@ -110,7 +110,7 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows" $newServiceCommand.$parameter | Should Be $value } - It "Set-Service can create a new service called '' and change the credentials" -TestCases @( + It "Set-Service can change credentials of a service" -TestCases @( @{name = "testsetcredential"; startCredential = [System.Management.Automation.PSCredential]::new(".\Guest", (ConvertTo-SecureString "PlainTextPassword" -AsPlainText -Force)); @@ -136,9 +136,9 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows" $service.StartName | Should Be $endCredential.UserName } finally { - $service = Get-CimInstance Win32_Service -Filter "name='$name'" + $service = Get-CimInstance Win32_Service -Filter "name='$name'" -ErrorAction SilentlyContinue if ($service -ne $null) { - $service | Remove-CimInstance + $service | Remove-CimInstance -ErrorAction SilentlyContinue } } } From 3bd33761cee883c6bc894002c304b5f336854347 Mon Sep 17 00:00:00 2001 From: Jonas Andersen Date: Fri, 15 Sep 2017 18:51:30 +0200 Subject: [PATCH 6/9] [Feature]Set-Service with credentials creates users for the test --- .../Set-Service.Tests.ps1 | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) 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 2590eaee8aa..dc23f7eeefe 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 @@ -111,35 +111,39 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows" } It "Set-Service can change credentials of a service" -TestCases @( - @{name = "testsetcredential"; - startCredential = [System.Management.Automation.PSCredential]::new(".\Guest", - (ConvertTo-SecureString "PlainTextPassword" -AsPlainText -Force)); - endCredential = [System.Management.Automation.PSCredential]::new(".\Administrator", - (ConvertTo-SecureString "PlainTextPassword" -AsPlainText -Force))} + @{name = "testsetcredential"; startUsername = "user1"; endUsername = "user2"} ) { - param($name, $startCredential, $endCredential) + param($name, $startUsername, $endUsername) try { + $testPass = "Secret123!" + $null = net user $startUsername $testPass /add + $null = net user $endUsername $testPass /add + $password = ConvertTo-SecureString $testPass -AsPlainText -Force + $creds = [pscredential]::new(".\$startUsername", $password) $parameters = @{ Name = $name; BinaryPathName = "$PSHOME\powershell.exe"; StartupType = "Manual"; - Credential = $startCredential + Credential = $creds } $service = New-Service @parameters $service | Should Not BeNullOrEmpty $service = Get-CimInstance Win32_Service -Filter "name='$name'" $service | Should Not BeNullOrEmpty - $service.StartName | Should Be $startCredential.UserName - Set-Service -Name $name -Credential $endCredential + $service.StartName | Should Be $creds.UserName + $creds = [pscredential]::new(".\$endUsername", $password) + Set-Service -Name $name -Credential $creds $service = Get-CimInstance Win32_Service -Filter "name='$name'" $service | Should Not BeNullOrEmpty - $service.StartName | Should Be $endCredential.UserName + $service.StartName | Should Be $creds.UserName } finally { $service = Get-CimInstance Win32_Service -Filter "name='$name'" -ErrorAction SilentlyContinue if ($service -ne $null) { $service | Remove-CimInstance -ErrorAction SilentlyContinue } + $null = net user $startUsername /delete + $null = net user $endUsername /delete } } From c08466f5d098b90c606fd06bcaf74bdd849c0204 Mon Sep 17 00:00:00 2001 From: Jonas Andersen Date: Fri, 15 Sep 2017 19:10:17 +0200 Subject: [PATCH 7/9] [Feature]Clean Set-Service credential test --- .../Set-Service.Tests.ps1 | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) 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 dc23f7eeefe..e417674e737 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 @@ -129,19 +129,14 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows" $service = New-Service @parameters $service | Should Not BeNullOrEmpty $service = Get-CimInstance Win32_Service -Filter "name='$name'" - $service | Should Not BeNullOrEmpty - $service.StartName | Should Be $creds.UserName + $service.StartName | Should BeExactly $creds.UserName $creds = [pscredential]::new(".\$endUsername", $password) Set-Service -Name $name -Credential $creds $service = Get-CimInstance Win32_Service -Filter "name='$name'" - $service | Should Not BeNullOrEmpty - $service.StartName | Should Be $creds.UserName + $service.StartName | Should BeExactly $creds.UserName } finally { - $service = Get-CimInstance Win32_Service -Filter "name='$name'" -ErrorAction SilentlyContinue - if ($service -ne $null) { - $service | Remove-CimInstance -ErrorAction SilentlyContinue - } + Get-CimInstance Win32_Service -Filter "name='$name'" | Remove-CimInstance -ErrorAction SilentlyContinue $null = net user $startUsername /delete $null = net user $endUsername /delete } From e0f2cd89b619c1575032eaa079fa98dcf6d1f7c6 Mon Sep 17 00:00:00 2001 From: Jonas Andersen Date: Fri, 15 Sep 2017 19:22:23 +0200 Subject: [PATCH 8/9] [Feature]Remove TestCases from Set-Service with credential test --- .../Set-Service.Tests.ps1 | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) 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 e417674e737..b9baaec8e35 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 @@ -110,35 +110,37 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows" $newServiceCommand.$parameter | Should Be $value } - It "Set-Service can change credentials of a service" -TestCases @( - @{name = "testsetcredential"; startUsername = "user1"; endUsername = "user2"} - ) { - param($name, $startUsername, $endUsername) + It "Set-Service can change credentials of a service" { + param() try { + $startUsername = "user1" + $endUsername = "user2" $testPass = "Secret123!" - $null = net user $startUsername $testPass /add - $null = net user $endUsername $testPass /add + $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) $parameters = @{ - Name = $name; + Name = $servicename; BinaryPathName = "$PSHOME\powershell.exe"; StartupType = "Manual"; Credential = $creds } $service = New-Service @parameters $service | Should Not BeNullOrEmpty - $service = Get-CimInstance Win32_Service -Filter "name='$name'" + $service = Get-CimInstance Win32_Service -Filter "name='$servicename'" $service.StartName | Should BeExactly $creds.UserName + $creds = [pscredential]::new(".\$endUsername", $password) - Set-Service -Name $name -Credential $creds - $service = Get-CimInstance Win32_Service -Filter "name='$name'" + Set-Service -Name $servicename -Credential $creds + $service = Get-CimInstance Win32_Service -Filter "name='$servicename'" $service.StartName | Should BeExactly $creds.UserName } finally { - Get-CimInstance Win32_Service -Filter "name='$name'" | Remove-CimInstance -ErrorAction SilentlyContinue - $null = net user $startUsername /delete - $null = net user $endUsername /delete + Get-CimInstance Win32_Service -Filter "name='$servicename'" | Remove-CimInstance -ErrorAction SilentlyContinue + net user $startUsername /delete > $null + net user $endUsername /delete > $null } } From ad1b5dda255ef1497d0d55d8619dfeae770e45e2 Mon Sep 17 00:00:00 2001 From: Jonas Andersen Date: Fri, 15 Sep 2017 19:32:26 +0200 Subject: [PATCH 9/9] [Feature]Remove unnecessary param in Set-Service with credentials test --- .../Microsoft.PowerShell.Management/Set-Service.Tests.ps1 | 1 - 1 file changed, 1 deletion(-) 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 b9baaec8e35..10ae344eb79 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 @@ -111,7 +111,6 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows" } It "Set-Service can change credentials of a service" { - param() try { $startUsername = "user1" $endUsername = "user2"