Fix file permissions check

contrib/win32/win32compat/w32-sshfileperm.c

@@ -46,6 +46,12 @@
 #define COMMA_SPACE_LEN			2
 #define BACKSLASH_LEN			1
 
+/* Access rights that are considered write-class for SSH file/folder security checks.
+ * No non-trusted principal may hold any of these rights on a secured path. */
+#define SSH_SECURE_WRITE_MASK \
+	(FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_APPEND_DATA | \
+	 WRITE_DAC | WRITE_OWNER | DELETE)
+
 extern int log_on_stderr;
 
 /*
@@ -133,7 +139,7 @@ check_secure_file_permission(const char *input_path, struct passwd * pw, int rea
 		    EqualSid(current_trustee_sid, user_sid) ||
 		    (ti_sid && EqualSid(current_trustee_sid, ti_sid))) {
 			continue;
-		} else if (read_ok && (current_access_mask & (FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_APPEND_DATA)) == 0 ) {
+		} else if (read_ok && (current_access_mask & SSH_SECURE_WRITE_MASK) == 0) {
 			/* if read is allowed, allow ACES that do not give write access*/
 			continue;
 		} else {
@@ -250,7 +256,7 @@ check_secure_folder_permission(const wchar_t* path_utf16, int read_ok)
 			IsWellKnownSid(current_trustee_sid, WinLocalSystemSid)) {
 			continue;
 		}
-		else if (read_ok && (current_access_mask & (FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_APPEND_DATA)) == 0) {
+		else if (read_ok && (current_access_mask & SSH_SECURE_WRITE_MASK) == 0) {
 			/* if read is allowed, allow ACES that do not give write access*/
 			continue;
 		}

regress/pesterTests/Authorized_keys_fileperm.Tests.ps1

@@ -166,7 +166,7 @@ Describe "Tests for authorized_keys file permission" -Tags "CI" {
 
             #Run
             Start-SSHDTestDaemon -WorkDir $opensshbinpath -Arguments "-d -f $sshdconfig -o `"AuthorizedKeysFile .testssh/authorized_keys`" -E $sshdlog" -Port $port
-            ssh -p $port -E $sshlog $ssouser@$server echo 1234
+            ssh -p $port -E $sshlog -o "PasswordAuthentication=no" $ssouser@$server echo 1234
             $LASTEXITCODE | Should Not Be 0
             Stop-SSHDTestDaemon -Port $port                  
             sleep $sshdDelay                  
@@ -184,22 +184,63 @@ Describe "Tests for authorized_keys file permission" -Tags "CI" {
 
             #Run
             Start-SSHDTestDaemon -workDir $opensshbinpath -Arguments "-d -f $sshdconfig -o `"AuthorizedKeysFile .testssh/authorized_keys`" -E $sshdlog" -Port $port
-            ssh -p $port -E $sshlog $ssouser@$server echo 1234
+            ssh -p $port -E $sshlog -o "PasswordAuthentication=no" $ssouser@$server echo 1234
             $LASTEXITCODE | Should Not Be 0            
             Stop-SSHDTestDaemon -Port $port
             sleep $sshdDelay
             $sshlog | Should Contain "Permission denied"
             $sshdlog | Should Contain "Authentication refused."
         }
 
+        It "$tC.$tI-authorized_keys-negative(other account has ChangePermissions on authorized_keys file)"  -skip:$skip {
+            Repair-AuthorizedKeyPermission -Filepath $authorizedkeyPath -confirm:$false
+            $objPwdUserSid = Get-UserSid -User $PwdUser
+            Set-FilePermission -FilePath $authorizedkeyPath -User $objPwdUserSid -Perm "ChangePermissions"
+
+            Start-SSHDTestDaemon -workDir $opensshbinpath -Arguments "-d -f $sshdconfig -o `"AuthorizedKeysFile .testssh/authorized_keys`" -E $sshdlog" -Port $port
+            ssh -p $port -E $sshlog -o "PasswordAuthentication=no" $ssouser@$server echo 1234
+            $LASTEXITCODE | Should Not Be 0
+            Stop-SSHDTestDaemon -Port $port
+            sleep $sshdDelay
+            $sshlog | Should Contain "Permission denied"
+            $sshdlog | Should Contain "Authentication refused."
+        }
+
+        It "$tC.$tI-authorized_keys-negative(other account has TakeOwnership on authorized_keys file)"  -skip:$skip {
+            Repair-AuthorizedKeyPermission -Filepath $authorizedkeyPath -confirm:$false
+            $objPwdUserSid = Get-UserSid -User $PwdUser
+            Set-FilePermission -FilePath $authorizedkeyPath -User $objPwdUserSid -Perm "TakeOwnership"
+
+            Start-SSHDTestDaemon -workDir $opensshbinpath -Arguments "-d -f $sshdconfig -o `"AuthorizedKeysFile .testssh/authorized_keys`" -E $sshdlog" -Port $port
+            ssh -p $port -E $sshlog -o "PasswordAuthentication=no" $ssouser@$server echo 1234
+            $LASTEXITCODE | Should Not Be 0
+            Stop-SSHDTestDaemon -Port $port
+            sleep $sshdDelay
+            $sshlog | Should Contain "Permission denied"
+            $sshdlog | Should Contain "Authentication refused."
+        }
+
+        It "$tC.$tI-authorized_keys-negative(other account has Delete on authorized_keys file)"  -skip:$skip {
+            Repair-AuthorizedKeyPermission -Filepath $authorizedkeyPath -confirm:$false
+            $objPwdUserSid = Get-UserSid -User $PwdUser
+            Set-FilePermission -FilePath $authorizedkeyPath -User $objPwdUserSid -Perm "Delete"
+
+            Start-SSHDTestDaemon -workDir $opensshbinpath -Arguments "-d -f $sshdconfig -o `"AuthorizedKeysFile .testssh/authorized_keys`" -E $sshdlog" -Port $port
+            ssh -p $port -E $sshlog -o "PasswordAuthentication=no" $ssouser@$server echo 1234
+            $LASTEXITCODE | Should Not Be 0
+            Stop-SSHDTestDaemon -Port $port
+            sleep $sshdDelay
+            $sshlog | Should Contain "Permission denied"
+            $sshdlog | Should Contain "Authentication refused."
+        }
+
         It "$tC.$tI-authorized_keys-negative(authorized_keys is owned by other non-admin user)"  -skip:$skip {
             #setup to have PwdUser as owner and grant it full control            
             $objPwdUserSid = Get-UserSid -User $PwdUser
             Repair-FilePermission -Filepath $authorizedkeyPath -Owner $objPwdUserSid -FullAccessNeeded $adminsSid,$systemSid,$objPwdUser -confirm:$false
 
-            #Run
             Start-SSHDTestDaemon -WorkDir $opensshbinpath -Arguments "-d -f $sshdconfig -o `"AuthorizedKeysFile .testssh/authorized_keys`" -E $sshdlog" -Port $port
-            ssh -p $port -E $sshlog $ssouser@$server echo 1234
+            ssh -p $port -E $sshlog -o "PasswordAuthentication=no" $ssouser@$server echo 1234
             $LASTEXITCODE | Should Not Be 0
             Stop-SSHDTestDaemon -Port $port
             sleep $sshdDelay

regress/pesterTests/Hostkey_fileperm.Tests.ps1

@@ -148,5 +148,32 @@ Describe "Tests for host keys file permission" -Tags "CI" {
             $logPath | Should Contain "bad permissions"
         }
 
+        It "$tC.$tI-Host keys-negative (other account has ChangePermissions on private key file)" {
+            Repair-FilePermission -Filepath $hostKeyFilePath -Owners $adminsSid -FullAccessNeeded $systemSid,$adminsSid -confirm:$false
+            Repair-FilePermission -Filepath "$hostKeyFilePath.pub" -Owners $adminsSid -FullAccessNeeded $systemSid,$adminsSid -ReadAccessNeeded $everyOneSid -confirm:$false
+            Set-FilePermission -FilePath $hostKeyFilePath -UserSid $objUserSid -Perm "ChangePermissions"
+            Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-h $hostKeyFilePath", "-E $logPath") -NoNewWindow
+            WaitForValidation -LogPath $logPath -Length 1100
+            $logPath | Should Contain "bad permissions"
+        }
+
+        It "$tC.$tI-Host keys-negative (other account has TakeOwnership on private key file)" {
+            Repair-FilePermission -Filepath $hostKeyFilePath -Owners $adminsSid -FullAccessNeeded $systemSid,$adminsSid -confirm:$false
+            Repair-FilePermission -Filepath "$hostKeyFilePath.pub" -Owners $adminsSid -FullAccessNeeded $systemSid,$adminsSid -ReadAccessNeeded $everyOneSid -confirm:$false
+            Set-FilePermission -FilePath $hostKeyFilePath -UserSid $objUserSid -Perm "TakeOwnership"
+            Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-h $hostKeyFilePath", "-E $logPath") -NoNewWindow
+            WaitForValidation -LogPath $logPath -Length 1100
+            $logPath | Should Contain "bad permissions"
+        }
+
+        It "$tC.$tI-Host keys-negative (other account has Delete on private key file)" {
+            Repair-FilePermission -Filepath $hostKeyFilePath -Owners $adminsSid -FullAccessNeeded $systemSid,$adminsSid -confirm:$false
+            Repair-FilePermission -Filepath "$hostKeyFilePath.pub" -Owners $adminsSid -FullAccessNeeded $systemSid,$adminsSid -ReadAccessNeeded $everyOneSid -confirm:$false
+            Set-FilePermission -FilePath $hostKeyFilePath -UserSid $objUserSid -Perm "Delete"
+            Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-h $hostKeyFilePath", "-E $logPath") -NoNewWindow
+            WaitForValidation -LogPath $logPath -Length 1100
+            $logPath | Should Contain "bad permissions"
+        }
+
     }
 }

regress/pesterTests/Setup.Tests.ps1

@@ -572,6 +572,17 @@ Describe "Setup Tests" -Tags "Setup" {
             if ((Get-Service sshd).Status -eq 'Running') {
                 net stop sshd
             }
+            # Register ETW manifest to capture OpenSSH events in the event log
+            $etwman = Join-Path $binPath "openssh-events.man"
+            if (Test-Path $etwman -PathType Leaf) {
+                wevtutil im "$etwman" 2>&1 | Out-Null
+            }
+        }
+        BeforeEach {
+            # Clear and enable the Operational event log channel before each test
+            wevtutil sl "OpenSSH/Operational" /e:false /q:true | Out-Null
+            wevtutil cl "OpenSSH/Operational" | Out-Null
+            wevtutil sl "OpenSSH/Operational" /e:true /q:true | Out-Null
         }
         AfterAll {
             $tC++
@@ -581,6 +592,10 @@ Describe "Setup Tests" -Tags "Setup" {
             if ($sshACL -eq $null) {
                 Remove-Item -Path $sshFolderPath -Recurse -Force
             }
+            $etwman = Join-Path $binPath "openssh-events.man"
+            if (Test-Path $etwman -PathType Leaf) {
+                wevtutil um "$etwman" 2>&1 | Out-Null
+            }
         }
         AfterEach {
             $tI++
@@ -605,5 +620,57 @@ Describe "Setup Tests" -Tags "Setup" {
             net start sshd
             $LASTEXITCODE | Should Be 0
         }
+
+        It "$tC.$tI - SSHD starts successfully and logs warning when other account has ChangePermissions on ssh folder" {
+            if (-not (Test-Path -Path $sshFolderPath)) {
+                New-Item -Path $sshFolderPath -ItemType Directory -Force
+            }
+            # Grant ChangePermissions (WRITE_DAC) to Authenticated Users - advisory warning, does not block startup
+            $acl = Get-Acl $sshFolderPath
+            $accessRule = New-Object System.Security.AccessControl.FileSystemAccessRule($authenticatedUserSid, "ChangePermissions", "Allow")
+            $acl.AddAccessRule($accessRule)
+            Set-Acl -Path $sshFolderPath -AclObject $acl
+
+            net start sshd
+            $LASTEXITCODE | Should Be 0
+            Start-Sleep -Seconds 1 # ensure event is logged before querying 
+            $events = wevtutil qe "OpenSSH/Operational" /f:text
+            ($events | Out-String) | Should Match "write access is granted"
+        }
+
+        It "$tC.$tI - SSHD starts successfully and logs warning when other account has TakeOwnership on ssh folder" {
+            if (-not (Test-Path -Path $sshFolderPath)) {
+                New-Item -Path $sshFolderPath -ItemType Directory -Force
+            }
+            # Grant TakeOwnership (WRITE_OWNER) to Authenticated Users - advisory warning, does not block startup
+            $acl = Get-Acl $sshFolderPath
+            $accessRule = New-Object System.Security.AccessControl.FileSystemAccessRule($authenticatedUserSid, "TakeOwnership", "Allow")
+            $acl.AddAccessRule($accessRule)
+            Set-Acl -Path $sshFolderPath -AclObject $acl
+
+            net start sshd
+            $LASTEXITCODE | Should Be 0
+            Start-Sleep -Seconds 1 # ensure event is logged before querying 
+            $events = wevtutil qe "OpenSSH/Operational" /f:text
+            ($events | Out-String) | Should Match "write access is granted"
+        }
+
+        It "$tC.$tI - SSHD starts successfully and logs warning when other account has Delete on ssh folder" {
+            if (-not (Test-Path -Path $sshFolderPath)) {
+                New-Item -Path $sshFolderPath -ItemType Directory -Force
+            }
+            # Grant Delete to Authenticated Users using explicit inheritance flags to match the
+            # existing ReadAndExecute ACE, avoiding OS merging ambiguity. Advisory warning, does not block startup.
+            $acl = Get-Acl $sshFolderPath
+            $accessRule = New-Object System.Security.AccessControl.FileSystemAccessRule($authenticatedUserSid, "Delete", "Allow")
+            $acl.AddAccessRule($accessRule)
+            Set-Acl -Path $sshFolderPath -AclObject $acl
+
+            net start sshd
+            $LASTEXITCODE | Should Be 0
+            Start-Sleep -Seconds 1 # ensure event is logged before querying 
+            $events = wevtutil qe "OpenSSH/Operational" /f:text
+            ($events | Out-String) | Should Match "write access is granted"
+        }
     }
 }

regress/pesterTests/Userkey_fileperm.Tests.ps1

@@ -119,7 +119,7 @@ Describe "Tests for user Key file permission" -Tags "CI" {
             Repair-FilePermission -FilePath $keyFilePath -Owners $currentUserSid -FullAccessNeeded $currentUser,$adminsSid,$systemSid -ReadAccessNeeded $objUserSid -confirm:$false
 
             #Run
-            $o = ssh -p $port -i $keyFilePath -E $logPath $pubKeyUser@$server echo 1234
+            $o = ssh -p $port -i $keyFilePath -E $logPath -o "PasswordAuthentication=no" $pubKeyUser@$server echo 1234
             $LASTEXITCODE | Should Not Be 0
 
             $logPath | Should Contain "UNPROTECTED PRIVATE KEY FILE!"
@@ -129,10 +129,34 @@ Describe "Tests for user Key file permission" -Tags "CI" {
             #setup to have ssouser as owner and grant it full control
             Repair-FilePermission -FilePath $keyFilePath -Owners $objUserSid -FullAccessNeeded $objUserSid,$adminsSid,$systemSid -ReadAccessNeeded $objUserSid -confirm:$false
 
-            $o = ssh -p $port -i $keyFilePath -E $logPath $pubKeyUser@$server echo 1234
+            $o = ssh -p $port -i $keyFilePath -E $logPath -o "PasswordAuthentication=no" $pubKeyUser@$server echo 1234
             $LASTEXITCODE | Should Not Be 0
 
             $logPath | Should Contain "UNPROTECTED PRIVATE KEY FILE!"
         }
+
+        It "$tC.$tI-ssh with private key file -- negative(other account has ChangePermissions on private key file)" {
+            Repair-FilePermission -FilePath $keyFilePath -Owners $currentUserSid -FullAccessNeeded $adminsSid,$systemSid,$currentUserSid -confirm:$false
+            Set-FilePermission -FilePath $keyFilePath -UserSid $objUserSid -Perm "ChangePermissions"
+            $o = ssh -p $port -i $keyFilePath -E $logPath -o "PasswordAuthentication=no" $pubKeyUser@$server echo 1234
+            $LASTEXITCODE | Should Not Be 0
+            $logPath | Should Contain "UNPROTECTED PRIVATE KEY FILE!"
+        }
+
+        It "$tC.$tI-ssh with private key file -- negative(other account has TakeOwnership on private key file)" {
+            Repair-FilePermission -FilePath $keyFilePath -Owners $currentUserSid -FullAccessNeeded $adminsSid,$systemSid,$currentUserSid -confirm:$false
+            Set-FilePermission -FilePath $keyFilePath -UserSid $objUserSid -Perm "TakeOwnership"
+            $o = ssh -p $port -i $keyFilePath -E $logPath -o "PasswordAuthentication=no" $pubKeyUser@$server echo 1234
+            $LASTEXITCODE | Should Not Be 0
+            $logPath | Should Contain "UNPROTECTED PRIVATE KEY FILE!"
+        }
+
+        It "$tC.$tI-ssh with private key file -- negative(other account has Delete on private key file)" {
+            Repair-FilePermission -FilePath $keyFilePath -Owners $currentUserSid -FullAccessNeeded $adminsSid,$systemSid,$currentUserSid -confirm:$false
+            Set-FilePermission -FilePath $keyFilePath -UserSid $objUserSid -Perm "Delete"
+            $o = ssh -p $port -i $keyFilePath -E $logPath -o "PasswordAuthentication=no" $pubKeyUser@$server echo 1234
+            $LASTEXITCODE | Should Not Be 0
+            $logPath | Should Contain "UNPROTECTED PRIVATE KEY FILE!"
+        }
     }
 }

regress/pesterTests/data/SSHD_Config

@@ -10,6 +10,7 @@
 # possible, but leave them commented.  Uncommented options override the
 # default value.
 
+PerSourcePenalties no
 Port 47002
 #AddressFamily any
 #ListenAddress 0.0.0.0