Point OpenSSH key setup at the file sshd actually reads (#4936)

* Point OpenSSH key setup at the file sshd actually reads

The Remote Access feature created %USERPROFILE%\.ssh\authorized_keys and
told the user to put their public keys there. sshd does not read that
file for a member of the administrators group; the "Match Group
administrators" block in sshd_config sends those logons to
C:\ProgramData\ssh\administrators_authorized_keys instead. WinUtil always
relaunches itself elevated, so the account it was setting up is always an
administrator, and key auth for it never worked.

The function tried to work around that by commenting the block out, but
those regexes anchor on $, and .NET puts $ before the \n of a CRLF pair.
The sshd_config Windows ships is CRLF throughout, so the replace was a
silent no-op on a stock install. On an sshd_config with LF endings it did
apply, and that is worse than not working: sshd gives an administrator
logon a full token with no UAC prompt, which is why Windows keeps those
keys in ProgramData behind an ACL that requires elevation to write.
Moving the lookup into the profile lets anything running as the user at
medium integrity append a key and get an elevated shell unprompted.

Use administrators_authorized_keys and give it the ACL sshd requires
(inheritance off, Administrators and SYSTEM only, by SID so localized
installs work). Where the sshd_config edit did land, undo it and copy any
keys out of the profile file first so key auth is not cut off mid-session.
Keys are only copied when the block needs restoring, so a default config
never grants access sshd was not already granting.

Also stop creating the profile .ssh directory: under elevation it was the
elevating administrator's profile, not necessarily the caller's.

* Document where to put SSH keys for the OpenSSH server feature

docs/src/content/docs/guides/features.mdx

@@ -56,3 +56,7 @@ Open old-school Windows panels directly from WinUtil. Available panels include:
 Enable an OpenSSH server on your Windows machine for remote access.
 
 Only enable this if you intend to use remote shell access. After turning it on, verify your firewall rules and account permissions before exposing the machine to other devices.
+
+Because WinUtil runs elevated, the account it sets up is an administrator, and sshd reads administrator keys from `C:\ProgramData\ssh\administrators_authorized_keys` rather than from your profile. WinUtil creates that file and restricts it to Administrators and SYSTEM, which is what sshd requires. Add your public keys there. If an earlier WinUtil version changed `sshd_config` to read administrator keys from `%USERPROFILE%\.ssh\authorized_keys`, that is undone and any keys in it are copied across, so key auth keeps working.
+
+Non-administrator accounts keep using `%USERPROFILE%\.ssh\authorized_keys` and need no extra setup.

functions/private/Invoke-WinUtilSSHServer.ps1

@@ -25,36 +25,66 @@ function Invoke-WinUtilSSHServer {
         Write-Host "Firewall rule for OpenSSH Server created and enabled."
     }
 
-    # Check for the authorized_keys file
+    # An SSH logon for a member of the administrators group gets a full token
-    $sshFolderPath = "$Home\.ssh"
+    # with no UAC prompt, so sshd reads administrator keys from a machine-wide
-    $authorizedKeysPath = "$sshFolderPath\authorized_keys"
+    # file that only Administrators and SYSTEM may write. WinUtil always runs
+    # elevated, so the account being set up here is always an administrator.
+    $sshProgramDataPath = Join-Path $env:ProgramData "ssh"
+    $sshdConfigPath = Join-Path $sshProgramDataPath "sshd_config"
+    $authorizedKeysPath = Join-Path $sshProgramDataPath "administrators_authorized_keys"
+    $profileKeysPath = Join-Path $env:USERPROFILE ".ssh\authorized_keys"
 
-    if (-not (Test-Path -Path $sshFolderPath)) {
+    if (-not (Test-Path -Path $sshProgramDataPath)) {
-        Write-Host "Creating ssh directory..."
+        New-Item -Path $sshProgramDataPath -ItemType Directory -Force | Out-Null
-        New-Item -Path $sshFolderPath -ItemType Directory -Force
     }
 
+    # Earlier WinUtil versions commented out the administrators block in
+    # sshd_config. Detect that state before restoring it, so administrator keys
+    # already in use are carried over instead of silently stopping working.
+    $configContent = if (Test-Path -Path $sshdConfigPath) { [string](Get-Content -Path $sshdConfigPath -Raw) } else { "" }
+    $restoredContent = $configContent -replace '(?m)^# (Match Group administrators)$', '$1'
+    $restoredContent = $restoredContent -replace '(?m)^# (\s+AuthorizedKeysFile __PROGRAMDATA__/ssh/administrators_authorized_keys)$', '$1'
+    $configWasOverridden = $restoredContent -ne $configContent
+
     if (-not (Test-Path -Path $authorizedKeysPath)) {
-        Write-Host "Creating authorized_keys file..."
+        Write-Host "Creating administrators_authorized_keys file..."
-        New-Item -Path $authorizedKeysPath -ItemType File -Force
+        New-Item -Path $authorizedKeysPath -ItemType File -Force | Out-Null
-        Write-Host "authorized_keys file created at $authorizedKeysPath."
+        Write-Host "administrators_authorized_keys file created at $authorizedKeysPath."
     }
 
-    Write-Host "Configuring sshd_config for standard authorized_keys behavior..."
+    if ($configWasOverridden -and (Test-Path -Path $profileKeysPath)) {
-    $sshdConfigPath = "C:\ProgramData\ssh\sshd_config"
+        $currentKeys = @(Get-Content -Path $authorizedKeysPath)
+        $keysToMove = @(Get-Content -Path $profileKeysPath | Where-Object {
+            $_.Trim() -and -not $_.TrimStart().StartsWith("#") -and $currentKeys -notcontains $_
+        })
 
-    $configContent = Get-Content -Path $sshdConfigPath -Raw
+        if ($keysToMove.Count -gt 0) {
+            Add-Content -Path $authorizedKeysPath -Value $keysToMove
+            Write-Host "Moved $($keysToMove.Count) key(s) from $profileKeysPath to $authorizedKeysPath."
+        }
+    }
 
-    $updatedContent = $configContent -replace '(?m)^(Match Group administrators)$', '# $1'
+    # sshd ignores the file unless inheritance is off and access is limited to
-    $updatedContent = $updatedContent -replace '(?m)^(\s+AuthorizedKeysFile __PROGRAMDATA__/ssh/administrators_authorized_keys)$', '# $1'
+    # Administrators (S-1-5-32-544) and SYSTEM (S-1-5-18). SIDs keep this
+    # working on localized installs, where the group names differ.
+    $acl = Get-Acl -Path $authorizedKeysPath
+    $acl.SetAccessRuleProtection($true, $false)
+    foreach ($rule in @($acl.Access)) {
+        [void]$acl.RemoveAccessRule($rule)
+    }
+    foreach ($sid in @("S-1-5-32-544", "S-1-5-18")) {
+        [void]$acl.AddAccessRule([System.Security.AccessControl.FileSystemAccessRule]::new(
+            [System.Security.Principal.SecurityIdentifier]::new($sid), "FullControl", "Allow"))
+    }
+    Set-Acl -Path $authorizedKeysPath -AclObject $acl
 
-    if ($updatedContent -ne $configContent) {
+    if ($configWasOverridden) {
-        Set-Content -Path $sshdConfigPath -Value $updatedContent -Force
+        Set-Content -Path $sshdConfigPath -Value $restoredContent -Force
-        Write-Host "Commented out administrator-specific SSH key configuration in sshd_config"
+        Write-Host "Restored the administrator key file setting in sshd_config."
         Restart-Service -Name sshd -Force
     }
 
     Write-Host "OpenSSH server was successfully enabled."
-    Write-Host "The config file can be located at C:\ProgramData\ssh\sshd_config"
+    Write-Host "The config file can be located at $sshdConfigPath"
     Write-Host "Add your public keys to this file -> $authorizedKeysPath"
 }

pester/ssh-server.Tests.ps1

@@ -0,0 +1,174 @@
+#===========================================================================
+# Tests - OpenSSH Server Setup
+#===========================================================================
+
+BeforeAll {
+    $script:repoRoot = (Resolve-Path (Join-Path $PSScriptRoot "..")).Path
+
+    function Get-WindowsCapability {
+        param($Name, [switch]$Online)
+        [pscustomobject]@{ State = "Installed" }
+    }
+    function Add-WindowsCapability {
+        param($Name, [switch]$Online)
+    }
+    function Get-NetFirewallRule {
+        param($Name)
+        [pscustomobject]@{ Enabled = $true }
+    }
+    function New-NetFirewallRule {
+        param($Name, $DisplayName, $Enabled, $Direction, $Protocol, $Action, $LocalPort)
+    }
+    function Set-Service {
+        param($Name, $StartupType)
+    }
+    function Start-Service {
+        param($Name)
+    }
+    function Restart-Service {
+        param($Name, [switch]$Force)
+    }
+
+    . (Join-Path $script:repoRoot "functions\private\Invoke-WinUtilSSHServer.ps1")
+
+    $script:defaultAdministratorsBlock = "Match Group administrators`n       AuthorizedKeysFile __PROGRAMDATA__/ssh/administrators_authorized_keys`n"
+    $script:overriddenAdministratorsBlock = "# Match Group administrators`n#        AuthorizedKeysFile __PROGRAMDATA__/ssh/administrators_authorized_keys`n"
+
+    function script:New-SshdConfig {
+        param([string]$AdministratorsBlock)
+
+        $content = "# Default sshd_config`nPort 22`n`n$AdministratorsBlock"
+        Set-Content -Path $script:sshdConfigPath -Value $content -NoNewline
+        return $content
+    }
+
+    function script:Set-ProfileKeyFile {
+        param([string[]]$Keys)
+
+        New-Item -Path (Split-Path $script:profileKeysPath) -ItemType Directory -Force | Out-Null
+        Set-Content -Path $script:profileKeysPath -Value $Keys
+    }
+
+    function script:Get-ExplicitKeyFileAccess {
+        $acl = Get-Acl -Path $script:authorizedKeysPath
+        @($acl.GetAccessRules($true, $false, [System.Security.Principal.SecurityIdentifier])) |
+            ForEach-Object { $_.IdentityReference.Value }
+    }
+
+    function script:Get-AuthorizedKeyFileContent {
+        # The key file ends up readable only by Administrators and SYSTEM, so an
+        # unelevated run has to grant itself read access back through its
+        # ownership of the file before it can check the contents.
+        $currentUserSid = [System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value
+        icacls $script:authorizedKeysPath /grant "*${currentUserSid}:(R)" | Out-Null
+
+        @(Get-Content -Path $script:authorizedKeysPath)
+    }
+}
+
+Describe "Invoke-WinUtilSSHServer" {
+    BeforeEach {
+        $script:testRoot = Join-Path ([System.IO.Path]::GetTempPath()) "winutil-ssh-$([guid]::NewGuid())"
+        $script:programData = Join-Path $script:testRoot "ProgramData"
+        $script:userProfile = Join-Path $script:testRoot "Users\tester"
+        New-Item -Path (Join-Path $script:programData "ssh") -ItemType Directory -Force | Out-Null
+        New-Item -Path $script:userProfile -ItemType Directory -Force | Out-Null
+
+        $script:sshdConfigPath = Join-Path $script:programData "ssh\sshd_config"
+        $script:authorizedKeysPath = Join-Path $script:programData "ssh\administrators_authorized_keys"
+        $script:profileKeysPath = Join-Path $script:userProfile ".ssh\authorized_keys"
+
+        $script:savedProgramData = $env:ProgramData
+        $script:savedUserProfile = $env:USERPROFILE
+        $env:ProgramData = $script:programData
+        $env:USERPROFILE = $script:userProfile
+
+        Mock Write-Host { }
+        Mock Restart-Service { }
+    }
+
+    AfterEach {
+        $env:ProgramData = $script:savedProgramData
+        $env:USERPROFILE = $script:savedUserProfile
+        Remove-Item -Path $script:testRoot -Recurse -Force -ErrorAction SilentlyContinue
+    }
+
+    It "leaves the administrators block in a default sshd_config alone" {
+        $original = New-SshdConfig -AdministratorsBlock $script:defaultAdministratorsBlock
+
+        Invoke-WinUtilSSHServer
+
+        Get-Content -Path $script:sshdConfigPath -Raw | Should -BeExactly $original
+        Should -Invoke -CommandName Restart-Service -Times 0 -Exactly
+    }
+
+    It "leaves an sshd_config without an administrators block alone" {
+        $original = New-SshdConfig -AdministratorsBlock ""
+        Set-ProfileKeyFile -Keys @("ssh-ed25519 AAAAnotanadminkey laptop")
+
+        Invoke-WinUtilSSHServer
+
+        Get-Content -Path $script:sshdConfigPath -Raw | Should -BeExactly $original
+        Get-AuthorizedKeyFileContent | Should -Not -Contain "ssh-ed25519 AAAAnotanadminkey laptop"
+        Should -Invoke -CommandName Restart-Service -Times 0 -Exactly
+    }
+
+    It "creates administrators_authorized_keys limited to Administrators and SYSTEM" {
+        New-SshdConfig -AdministratorsBlock $script:defaultAdministratorsBlock | Out-Null
+
+        Invoke-WinUtilSSHServer
+
+        Test-Path -Path $script:authorizedKeysPath | Should -BeTrue
+        (Get-Acl -Path $script:authorizedKeysPath).AreAccessRulesProtected | Should -BeTrue
+
+        $sids = Get-ExplicitKeyFileAccess
+        $sids | Should -HaveCount 2
+        $sids | Should -Contain "S-1-5-32-544"
+        $sids | Should -Contain "S-1-5-18"
+    }
+
+    It "restores the administrators block when an earlier run commented it out" {
+        New-SshdConfig -AdministratorsBlock $script:overriddenAdministratorsBlock | Out-Null
+
+        Invoke-WinUtilSSHServer
+
+        $config = Get-Content -Path $script:sshdConfigPath -Raw
+        $config | Should -Match '(?m)^Match Group administrators$'
+        $config | Should -Match '(?m)^\s+AuthorizedKeysFile __PROGRAMDATA__/ssh/administrators_authorized_keys$'
+        Should -Invoke -CommandName Restart-Service -Times 1 -Exactly
+    }
+
+    It "moves profile keys into administrators_authorized_keys while restoring the block" {
+        New-SshdConfig -AdministratorsBlock $script:overriddenAdministratorsBlock | Out-Null
+        Set-ProfileKeyFile -Keys @("# my laptop", "", "ssh-ed25519 AAAAkeyone laptop", "ssh-ed25519 AAAAkeytwo desktop")
+
+        Invoke-WinUtilSSHServer
+
+        $keys = Get-AuthorizedKeyFileContent
+        $keys | Should -Contain "ssh-ed25519 AAAAkeyone laptop"
+        $keys | Should -Contain "ssh-ed25519 AAAAkeytwo desktop"
+        $keys | Should -Not -Contain "# my laptop"
+    }
+
+    It "does not copy profile keys when sshd_config is already at its default" {
+        New-SshdConfig -AdministratorsBlock $script:defaultAdministratorsBlock | Out-Null
+        Set-ProfileKeyFile -Keys @("ssh-ed25519 AAAAnotanadminkey laptop")
+
+        Invoke-WinUtilSSHServer
+
+        Get-AuthorizedKeyFileContent | Should -Not -Contain "ssh-ed25519 AAAAnotanadminkey laptop"
+    }
+
+    It "keeps keys that are already in administrators_authorized_keys" {
+        New-SshdConfig -AdministratorsBlock $script:overriddenAdministratorsBlock | Out-Null
+        Set-Content -Path $script:authorizedKeysPath -Value "ssh-ed25519 AAAAexisting server"
+        Set-ProfileKeyFile -Keys @("ssh-ed25519 AAAAexisting server", "ssh-ed25519 AAAAnew laptop")
+
+        Invoke-WinUtilSSHServer
+
+        $keys = @(Get-AuthorizedKeyFileContent | Where-Object { $_.Trim() })
+        $keys | Should -HaveCount 2
+        $keys | Should -Contain "ssh-ed25519 AAAAexisting server"
+        $keys | Should -Contain "ssh-ed25519 AAAAnew laptop"
+    }
+}