From 6b807ae229721c103c50c6f386cdd85390a36703 Mon Sep 17 00:00:00 2001 From: Yanbing Date: Mon, 1 May 2017 14:18:20 -0700 Subject: [PATCH] file permission on ssh_config, authorized_keys, private keys, host keys, public keys. (#110) 1. Add file permission check when load or add ssh_config, authorized_keys, private keys, host keys,. 2. set the owner and ACE for create secure file, ex, private key in ssh-keygen.exe 3. Update script OpenSSHTestHelper.psm1 to be able to run Install-OpenSSH if the sshd is running on the machine. 4. add OpenSSHBinPath to path. 5. change indents in agentconfig.c 6. update test script to represent the changes 7. Add tests for: * authorized_keys and ssh-keygen testing * host keys file perm testing * user private key file perm testing * ssh-add test * user ssh_config --- auth.c | 13 +- authfile.c | 21 +- contrib/win32/openssh/OpenSSHBuildHelper.psm1 | 26 +- contrib/win32/openssh/OpenSSHCommonUtils.psm1 | 78 +++- contrib/win32/openssh/OpenSSHTestHelper.psm1 | 82 +++-- contrib/win32/openssh/libssh.vcxproj | 2 + contrib/win32/openssh/libssh.vcxproj.filters | 6 + contrib/win32/openssh/unittest-bitmap.vcxproj | 4 +- .../win32/openssh/unittest-hostkeys.vcxproj | 8 +- contrib/win32/openssh/unittest-kex.vcxproj | 4 +- contrib/win32/openssh/unittest-match.vcxproj | 4 +- contrib/win32/openssh/unittest-sshbuf.vcxproj | 4 +- contrib/win32/openssh/unittest-sshkey.vcxproj | 8 +- .../openssh/unittest-win32compat.vcxproj | 4 +- .../win32/win32compat/ssh-agent/agentconfig.c | 42 ++- contrib/win32/win32compat/w32-sshfileperm.c | 348 ++++++++++++++++++ readconf.c | 16 +- .../Authorized_keys_fileperm.Tests.ps1 | 168 +++++++++ regress/pesterTests/Cfginclude.Tests.ps1 | 152 ++++++++ regress/pesterTests/CommonUtils.psm1 | 106 ++++++ .../pesterTests/Hostkey_fileperm.Tests.ps1 | 138 +++++++ regress/pesterTests/Keygen.Tests.ps1 | 62 ++++ regress/pesterTests/SCP.Tests.ps1 | 4 +- .../pesterTests/Userkey_fileperm.Tests.ps1 | 185 ++++++++++ regress/pesterTests/testdata/ssh_config | 4 + .../testdata/sshtest_userPermTestkey_ed25519 | 8 + .../sshtest_userPermTestkey_ed25519.pub | 1 + regress/pesterTests/testdata/test_known_hosts | 1 + ssh-keygen.c | 44 ++- sshfileperm.h | 30 ++ 30 files changed, 1478 insertions(+), 95 deletions(-) create mode 100644 contrib/win32/win32compat/w32-sshfileperm.c create mode 100644 regress/pesterTests/Authorized_keys_fileperm.Tests.ps1 create mode 100644 regress/pesterTests/Cfginclude.Tests.ps1 create mode 100644 regress/pesterTests/CommonUtils.psm1 create mode 100644 regress/pesterTests/Hostkey_fileperm.Tests.ps1 create mode 100644 regress/pesterTests/Keygen.Tests.ps1 create mode 100644 regress/pesterTests/Userkey_fileperm.Tests.ps1 create mode 100644 regress/pesterTests/testdata/ssh_config create mode 100644 regress/pesterTests/testdata/sshtest_userPermTestkey_ed25519 create mode 100644 regress/pesterTests/testdata/sshtest_userPermTestkey_ed25519.pub create mode 100644 regress/pesterTests/testdata/test_known_hosts create mode 100644 sshfileperm.h diff --git a/auth.c b/auth.c index 3cd6e7936..41e89d460 100644 --- a/auth.c +++ b/auth.c @@ -76,6 +76,7 @@ #include "authfile.h" #include "ssherr.h" #include "compat.h" +#include "sshfileperm.h" /* import */ extern ServerOptions options; @@ -489,10 +490,6 @@ int auth_secure_path(const char *name, struct stat *stp, const char *pw_dir, uid_t uid, char *err, size_t errlen) { -#ifdef WINDOWS - error("auth_secure_path should not be called in Windows yet"); - return -1; -#else /* !WINDOWS */ char buf[PATH_MAX], homedir[PATH_MAX]; char *cp; int comparehome = 0; @@ -545,7 +542,6 @@ auth_secure_path(const char *name, struct stat *stp, const char *pw_dir, break; } return 0; -#endif /* !WINDOWS */ } /* @@ -585,7 +581,12 @@ auth_openfile(const char *file, struct passwd *pw, int strict_modes, strerror(errno)); return NULL; } - /* TODO check permissions */ + if (strict_modes && check_secure_file_permission(file, pw) != 0) { + fclose(f); + logit("Authentication refused."); + auth_debug_add("Ignored %s", file_type); + return NULL; + } #else /* !WINDOWS */ if ((fd = open(file, O_RDONLY|O_NONBLOCK)) == -1) { if (log_missing || errno != ENOENT) diff --git a/authfile.c b/authfile.c index 255907e4e..357b34104 100644 --- a/authfile.c +++ b/authfile.c @@ -49,6 +49,7 @@ #include "sshbuf.h" #include "ssherr.h" #include "krl.h" +#include "sshfileperm.h" #define MAX_KEY_FILE_SIZE (1024 * 1024) @@ -60,6 +61,14 @@ sshkey_save_private_blob(struct sshbuf *keybuf, const char *filename) if ((fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0600)) < 0) return SSH_ERR_SYSTEM_ERROR; +#ifdef WINDOWS /* WINDOWS */ + /* + Set the owner of the private key file to current user and only grant + current user the full control access + */ + if (set_secure_file_permission(filename, NULL) != 0) + return SSH_ERR_SYSTEM_ERROR; +#endif /* WINDOWS */ if (atomicio(vwrite, fd, (u_char *)sshbuf_ptr(keybuf), sshbuf_len(keybuf)) != sshbuf_len(keybuf)) { oerrno = errno; @@ -193,19 +202,25 @@ sshkey_perm_ok(int fd, const char *filename) #ifdef HAVE_CYGWIN if (check_ntsec(filename)) #endif - -#ifndef WINDOWS /*TODO - implement permission checks on Windows*/ + +#ifdef WINDOWS /*implement permission checks on Windows*/ + if(check_secure_file_permission(filename, NULL) != 0) { + error("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@"); + error("@ WARNING: UNPROTECTED PRIVATE KEY FILE! @"); + error("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@"); + error("Permissions for '%s' are too open.", filename); +#else if ((st.st_uid == getuid()) && (st.st_mode & 077) != 0) { error("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@"); error("@ WARNING: UNPROTECTED PRIVATE KEY FILE! @"); error("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@"); error("Permissions 0%3.3o for '%s' are too open.", (u_int)st.st_mode & 0777, filename); +#endif /* !WINDOWS */ error("It is required that your private key files are NOT accessible by others."); error("This private key will be ignored."); return SSH_ERR_KEY_BAD_PERMISSIONS; } -#endif /* !WINDOWS */ return 0; } diff --git a/contrib/win32/openssh/OpenSSHBuildHelper.psm1 b/contrib/win32/openssh/OpenSSHBuildHelper.psm1 index 71dc04e23..0e5e22213 100644 --- a/contrib/win32/openssh/OpenSSHBuildHelper.psm1 +++ b/contrib/win32/openssh/OpenSSHBuildHelper.psm1 @@ -307,6 +307,12 @@ function Package-OpenSSH if ($NativeHostArch -eq 'x86') { $packageName = "OpenSSH-Win32" } + while((($service = Get-Service ssh-agent -ErrorAction Ignore) -ne $null) -and ($service.Status -ine 'Stopped')) + { + Stop-Service ssh-agent -Force + #sleep to wait the servicelog file write + Start-Sleep 5 + } $packageDir = Join-Path $buildDir $packageName Remove-Item $packageDir -Recurse -Force -ErrorAction SilentlyContinue @@ -320,23 +326,23 @@ function Package-OpenSSH if ((-not(Test-Path (Join-Path $buildDir $file)))) { Throw "Cannot find $file under $buildDir. Did you run Build-OpenSSH?" } - Copy-Item (Join-Path $buildDir $file) $packageDir + Copy-Item (Join-Path $buildDir $file) $packageDir -Force if ($file.EndsWith(".exe")) { $pdb = $file.Replace(".exe", ".pdb") - Copy-Item (Join-Path $buildDir $pdb) $symbolsDir + Copy-Item (Join-Path $buildDir $pdb) $symbolsDir -Force } if ($file.EndsWith(".dll")) { $pdb = $file.Replace(".dll", ".pdb") - Copy-Item (Join-Path $buildDir $pdb) $symbolsDir + Copy-Item (Join-Path $buildDir $pdb) $symbolsDir -Force } } if ($DestinationPath -ne "") { - if (Test-Path $DestinationPath) { - Remove-Item $DestinationPath\* -Force + if (Test-Path $DestinationPath) { + Remove-Item $DestinationPath\* -Force -Recurse } else { - New-Item -ItemType Directory $DestinationPath | Out-Null + New-Item -ItemType Directory $DestinationPath -Force | Out-Null } Copy-Item -Path $packageDir\* -Destination $DestinationPath -Force -Recurse } @@ -493,8 +499,12 @@ function Install-OpenSSH Package-OpenSSH -NativeHostArch $NativeHostArch -Configuration $Configuration -DestinationPath $OpenSSHDir Push-Location $OpenSSHDir - & ( "$OpenSSHDir\install-sshd.ps1") - .\ssh-keygen.exe -A + & "$OpenSSHDir\install-sshd.ps1" + & "$OpenSSHDir\ssh-keygen.exe" -A + + $keyFiles = Get-ChildItem "$OpenSSHDir\ssh_host_*_key*" | % { + Add-PermissionToFileACL -FilePath $_.FullName -User "NT Service\sshd" -Perm "Read" + } #machine will be reboot after Install-openssh anyway diff --git a/contrib/win32/openssh/OpenSSHCommonUtils.psm1 b/contrib/win32/openssh/OpenSSHCommonUtils.psm1 index e8ff87951..aa63a7497 100644 --- a/contrib/win32/openssh/OpenSSHCommonUtils.psm1 +++ b/contrib/win32/openssh/OpenSSHCommonUtils.psm1 @@ -28,4 +28,80 @@ function Get-RepositoryRoot throw new-object System.IO.DirectoryNotFoundException("Could not find the root of the GIT repository") } -Export-ModuleMember -Function Get-RepositoryRoot \ No newline at end of file +<# +.Synopsis + Sets the Secure File ACL. + 1. Removed all user acl except Administrators group, system, and current user + 2. whether or not take the owner + +.Outputs + N/A + +.Inputs + FilePath - The path to the file + takeowner - if want to take the ownership +#> +function Cleanup-SecureFileACL +{ + [CmdletBinding()] + param([string]$FilePath, [System.Security.Principal.NTAccount] $Owner) + + $myACL = Get-ACL $filePath + $myACL.SetAccessRuleProtection($True, $True) + Set-Acl -Path $filePath -AclObject $myACL + + $myACL = Get-ACL $filePath + if($owner -ne $null) + { + $myACL.SetOwner($owner) + } + + if($myACL.Access) + { + $myACL.Access | % { + if (($_ -ne $null) -and ($_.IdentityReference.Value -ine "BUILTIN\Administrators") -and + ($_.IdentityReference.Value -ine "NT AUTHORITY\SYSTEM") -and + ($_.IdentityReference.Value -ine "$(whoami)")) + { + if(-not ($myACL.RemoveAccessRule($_))) + { + throw "failed to remove access of $($_.IdentityReference.Value) rule in setup " + } + } + } + } + + Set-Acl -Path $filePath -AclObject $myACL +} + +<# +.Synopsis + add a file permission to an account + +.Outputs + N/A + +.Inputs + FilePath - The path to the file + User - account name + Perm - The permission to grant. +#> +function Add-PermissionToFileACL +{ + [CmdletBinding()] + param( + [string]$FilePath, + [System.Security.Principal.NTAccount] $User, + [System.Security.AccessControl.FileSystemRights]$Perm + ) + + $myACL = Get-ACL $filePath + + $objACE = New-Object System.Security.AccessControl.FileSystemAccessRule ` + ($User, $perm, "None", "None", "Allow") + $myACL.AddAccessRule($objACE) + + Set-Acl -Path $filePath -AclObject $myACL +} + +Export-ModuleMember -Function Get-RepositoryRoot, Add-PermissionToFileACL, Cleanup-SecureFileACL \ No newline at end of file diff --git a/contrib/win32/openssh/OpenSSHTestHelper.psm1 b/contrib/win32/openssh/OpenSSHTestHelper.psm1 index 27405582a..cec2f71a6 100644 --- a/contrib/win32/openssh/OpenSSHTestHelper.psm1 +++ b/contrib/win32/openssh/OpenSSHTestHelper.psm1 @@ -108,6 +108,10 @@ function Setup-OpenSSHTestEnvironment } $Global:OpenSSHTestInfo.Add("OpenSSHBinPath", $script:OpenSSHBinPath) + if (-not ($env:Path.ToLower().Contains($script:OpenSSHBinPath.ToLower()))) + { + $env:Path = "$($script:OpenSSHBinPath);$($env:path)" + } $warning = @" WARNING: Following changes will be made to OpenSSH configuration @@ -125,15 +129,12 @@ WARNING: Following changes will be made to OpenSSH configuration if (-not $Quiet) { Write-Warning $warning $continue = Read-Host -Prompt "Do you want to continue with the above changes? [Yes] Y; [No] N (default is `"Y`")" - if( ($continue -eq "") -or ($continue -ieq "Y") -or ($continue -ieq "Yes") ) - { - } - elseif( ($continue -ieq "N") -or ($continue -ieq "No") ) + if( ($continue -ieq "N") -or ($continue -ieq "No") ) { Write-Host "User decided not to make the changes." return } - else + elseif(($continue -ne "") -and ($continue -ine "Y") -and ($continue -ine "Yes")) { Throw "User entered invalid option ($continue). Exit now." } @@ -152,9 +153,21 @@ WARNING: Following changes will be made to OpenSSH configuration Copy-Item (Join-Path $script:OpenSSHBinPath sshd_config) $backupConfigPath -Force } - # copy new sshd_config - Copy-Item (Join-Path $Script:E2ETestDirectory sshd_config) (Join-Path $script:OpenSSHBinPath sshd_config) -Force - Copy-Item "$($Script:E2ETestDirectory)\sshtest*hostkey*" $script:OpenSSHBinPath -Force + # copy new sshd_config + Copy-Item (Join-Path $Script:E2ETestDirectory sshd_config) (Join-Path $script:OpenSSHBinPath sshd_config) -Force + + #workaround for the cariggage new line added by git before copy them + Get-ChildItem "$($Script:E2ETestDirectory)\sshtest_*key*" | % { + (Get-Content $_.FullName -Raw).Replace("`r`n","`n") | Set-Content $_.FullName -Force + } + + #copy sshtest keys + Copy-Item "$($Script:E2ETestDirectory)\sshtest*hostkey*" $script:OpenSSHBinPath -Force + $owner = New-Object System.Security.Principal.NTAccount($env:USERDOMAIN, $env:USERNAME) + Get-ChildItem "$($script:OpenSSHBinPath)\sshtest*hostkey*" -Exclude *.pub | % { + Cleanup-SecureFileACL -FilePath $_.FullName -Owner $owner + Add-PermissionToFileACL -FilePath $_.FullName -User "NT Service\sshd" -Perm "Read" + } Restart-Service sshd -Force #Backup existing known_hosts and replace with test version @@ -174,46 +187,51 @@ WARNING: Following changes will be made to OpenSSH configuration #TODO - this is Windows specific. Need to be in PAL foreach ($user in $OpenSSHTestAccounts) { - try + try { $objUser = New-Object System.Security.Principal.NTAccount($user) $strSID = $objUser.Translate([System.Security.Principal.SecurityIdentifier]) } catch - { + { #only add the local user when it does not exists on the machine net user $user $Script:OpenSSHTestAccountsPassword /ADD 2>&1 >> $Script:TestSetupLogFile - } + } } - #setup single sign on for ssouser - #TODO - this is Windows specific. Need to be in PAL - $ssousersid = Get-UserSID -User sshtest_ssouser - $ssouserProfileRegistry = Join-Path "HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList" $ssousersid - if (-not (Test-Path $ssouserProfileRegistry) ) { + #setup single sign on for ssouser + $ssouserProfile = Get-LocalUserProfile -User $SSOUser + $Global:OpenSSHTestInfo.Add("SSOUserProfile", $ssouserProfile) + $Global:OpenSSHTestInfo.Add("PubKeyUserProfile", (Get-LocalUserProfile -User $PubKeyUser)) + + New-Item -ItemType Directory -Path (Join-Path $ssouserProfile .ssh) -Force -ErrorAction SilentlyContinue | out-null + $authorizedKeyPath = Join-Path $ssouserProfile .ssh\authorized_keys + $testPubKeyPath = Join-Path $Script:E2ETestDirectory sshtest_userssokey_ed25519.pub + Copy-Item $testPubKeyPath $authorizedKeyPath -Force -ErrorAction SilentlyContinue + Add-PermissionToFileACL -FilePath $authorizedKeyPath -User "NT Service\sshd" -Perm "Read" + $testPriKeypath = Join-Path $Script:E2ETestDirectory sshtest_userssokey_ed25519 + Cleanup-SecureFileACL -FilePath $testPriKeypath -owner $owner + cmd /c "ssh-add $testPriKeypath 2>&1 >> $Script:TestSetupLogFile" +} +#TODO - this is Windows specific. Need to be in PAL +function Get-LocalUserProfile +{ + param([string]$User) + $sid = Get-UserSID -User $User + $userProfileRegistry = Join-Path "HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList" $sid + if (-not (Test-Path $userProfileRegistry) ) { #create profile if (-not($env:DISPLAY)) { $env:DISPLAY = 1 } $env:SSH_ASKPASS="$($env:ComSpec) /c echo $($OpenSSHTestAccountsPassword)" - cmd /c "ssh -p 47002 sshtest_ssouser@localhost echo %userprofile% > profile.txt" + $ret = ssh -p 47002 "$User@localhost" echo %userprofile% if ($env:DISPLAY -eq 1) { Remove-Item env:\DISPLAY } remove-item "env:SSH_ASKPASS" -ErrorAction SilentlyContinue - } - $ssouserProfile = (Get-ItemProperty -Path $ssouserProfileRegistry -Name 'ProfileImagePath').ProfileImagePath - New-Item -ItemType Directory -Path (Join-Path $ssouserProfile .ssh) -Force -ErrorAction SilentlyContinue | out-null - $authorizedKeyPath = Join-Path $ssouserProfile .ssh\authorized_keys - $testPubKeyPath = Join-Path $Script:E2ETestDirectory sshtest_userssokey_ed25519.pub - #workaround for the cariggage new line added by git - (Get-Content $testPubKeyPath -Raw).Replace("`r`n","`n") | Set-Content $testPubKeyPath -Force - Copy-Item $testPubKeyPath $authorizedKeyPath -Force -ErrorAction SilentlyContinue - $acl = get-acl $authorizedKeyPath - $ar = New-Object System.Security.AccessControl.FileSystemAccessRule("NT Service\sshd", "Read", "Allow") - $acl.SetAccessRule($ar) - Set-Acl $authorizedKeyPath $acl - $testPriKeypath = Join-Path $Script:E2ETestDirectory sshtest_userssokey_ed25519 - (Get-Content $testPriKeypath -Raw).Replace("`r`n","`n") | Set-Content $testPriKeypath -Force - cmd /c "ssh-add $testPriKeypath 2>&1 >> $Script:TestSetupLogFile" + } + + (Get-ItemProperty -Path $userProfileRegistry -Name 'ProfileImagePath').ProfileImagePath } + <# .SYNOPSIS This function installs the tools required by our tests diff --git a/contrib/win32/openssh/libssh.vcxproj b/contrib/win32/openssh/libssh.vcxproj index dc6ce5506..fd4a7aa86 100644 --- a/contrib/win32/openssh/libssh.vcxproj +++ b/contrib/win32/openssh/libssh.vcxproj @@ -283,12 +283,14 @@ + true + diff --git a/contrib/win32/openssh/libssh.vcxproj.filters b/contrib/win32/openssh/libssh.vcxproj.filters index 2af40a2d5..7cc27be02 100644 --- a/contrib/win32/openssh/libssh.vcxproj.filters +++ b/contrib/win32/openssh/libssh.vcxproj.filters @@ -285,10 +285,16 @@ Source Files + + Source Files + Header Files + + Header Files + \ No newline at end of file diff --git a/contrib/win32/openssh/unittest-bitmap.vcxproj b/contrib/win32/openssh/unittest-bitmap.vcxproj index cc462efda..0eb327c5a 100644 --- a/contrib/win32/openssh/unittest-bitmap.vcxproj +++ b/contrib/win32/openssh/unittest-bitmap.vcxproj @@ -75,14 +75,14 @@ false $(Platform)\$(Configuration)\$(TargetName)\ unittest-bitmap - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); false $(Platform)\$(Configuration)\$(TargetName)\ unittest-bitmap - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); diff --git a/contrib/win32/openssh/unittest-hostkeys.vcxproj b/contrib/win32/openssh/unittest-hostkeys.vcxproj index 477e59a1e..bea0a5ef0 100644 --- a/contrib/win32/openssh/unittest-hostkeys.vcxproj +++ b/contrib/win32/openssh/unittest-hostkeys.vcxproj @@ -75,14 +75,14 @@ false $(Platform)\$(Configuration)\$(TargetName)\ unittest-hostkeys - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); false $(Platform)\$(Configuration)\$(TargetName)\ unittest-hostkeys - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); @@ -123,7 +123,7 @@ targetos.manifest - xcopy /Y $(ProjectDir)..\..\..\regress\unittests\hostkeys\testdata\* $(OutDir)hostkeys\ + xcopy /Y $(ProjectDir)..\..\..\regress\unittests\hostkeys\testdata\* $(OutDir) @@ -149,7 +149,7 @@ targetos.manifest - xcopy /Y $(ProjectDir)..\..\..\regress\unittests\hostkeys\testdata\* $(OutDir)hostkeys\ + xcopy /Y $(ProjectDir)..\..\..\regress\unittests\hostkeys\testdata\* $(OutDir) diff --git a/contrib/win32/openssh/unittest-kex.vcxproj b/contrib/win32/openssh/unittest-kex.vcxproj index a133038e7..c457b0a27 100644 --- a/contrib/win32/openssh/unittest-kex.vcxproj +++ b/contrib/win32/openssh/unittest-kex.vcxproj @@ -75,14 +75,14 @@ false $(Platform)\$(Configuration)\$(TargetName)\ unittest-kex - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); false $(Platform)\$(Configuration)\$(TargetName)\ unittest-kex - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); diff --git a/contrib/win32/openssh/unittest-match.vcxproj b/contrib/win32/openssh/unittest-match.vcxproj index 7e0f421c4..e0588ee79 100644 --- a/contrib/win32/openssh/unittest-match.vcxproj +++ b/contrib/win32/openssh/unittest-match.vcxproj @@ -75,14 +75,14 @@ false $(Platform)\$(Configuration)\$(TargetName)\ unittest-match - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); false $(Platform)\$(Configuration)\$(TargetName)\ unittest-match - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); diff --git a/contrib/win32/openssh/unittest-sshbuf.vcxproj b/contrib/win32/openssh/unittest-sshbuf.vcxproj index a63568c5f..636637c42 100644 --- a/contrib/win32/openssh/unittest-sshbuf.vcxproj +++ b/contrib/win32/openssh/unittest-sshbuf.vcxproj @@ -75,14 +75,14 @@ false $(Platform)\$(Configuration)\$(TargetName)\ unittest-sshbuf - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); false $(Platform)\$(Configuration)\$(TargetName)\ unittest-sshbuf - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); diff --git a/contrib/win32/openssh/unittest-sshkey.vcxproj b/contrib/win32/openssh/unittest-sshkey.vcxproj index c3ac4b593..09271046b 100644 --- a/contrib/win32/openssh/unittest-sshkey.vcxproj +++ b/contrib/win32/openssh/unittest-sshkey.vcxproj @@ -75,14 +75,14 @@ false $(Platform)\$(Configuration)\$(TargetName)\ unittest-sshkey - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); false $(Platform)\$(Configuration)\$(TargetName)\ unittest-sshkey - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); @@ -123,7 +123,7 @@ targetos.manifest - xcopy /Y $(ProjectDir)..\..\..\regress\unittests\sshkey\testdata\* $(OutDir)sshkey\ + xcopy /Y $(ProjectDir)..\..\..\regress\unittests\sshkey\testdata\* $(OutDir) @@ -149,7 +149,7 @@ targetos.manifest - xcopy /Y $(ProjectDir)..\..\..\regress\unittests\sshkey\testdata\* $(OutDir)sshkey\ + xcopy /Y $(ProjectDir)..\..\..\regress\unittests\sshkey\testdata\* $(OutDir) diff --git a/contrib/win32/openssh/unittest-win32compat.vcxproj b/contrib/win32/openssh/unittest-win32compat.vcxproj index a522687a8..ffd121ff9 100644 --- a/contrib/win32/openssh/unittest-win32compat.vcxproj +++ b/contrib/win32/openssh/unittest-win32compat.vcxproj @@ -82,14 +82,14 @@ false $(Platform)\$(Configuration)\$(TargetName)\ unittest-win32compat - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); false $(Platform)\$(Configuration)\$(TargetName)\ unittest-win32compat - $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\ + $(OpenSSH-Bin-Path)$(Platform)\$(Configuration)\$(TargetName)\ $(OpenSSH-Src-Path)contrib\win32\win32compat\inc;$(VC_IncludePath);$(WindowsSDK_IncludePath); diff --git a/contrib/win32/win32compat/ssh-agent/agentconfig.c b/contrib/win32/win32compat/ssh-agent/agentconfig.c index c8d3461e9..a136b7d4d 100644 --- a/contrib/win32/win32compat/ssh-agent/agentconfig.c +++ b/contrib/win32/win32compat/ssh-agent/agentconfig.c @@ -54,7 +54,8 @@ struct passwd *privsep_pw = NULL; static char *config_file_name = _PATH_SERVER_CONFIG_FILE; int auth_sock = -1; -int auth2_methods_valid(const char * c, int i) { +int +auth2_methods_valid(const char * c, int i) { return 1; } @@ -69,21 +70,21 @@ mm_user_key_allowed(struct passwd *pw, Key *k, int i) return 0; } -int kexgex_server(struct ssh * sh) { +int +kexgex_server(struct ssh * sh) { return -1; } -static -int GetCurrentModulePath(wchar_t *path, int pathSize) +static int +GetCurrentModulePath(wchar_t *path, int pathSize) { if (GetModuleFileNameW(NULL, path, pathSize)) { int i; int lastSlashPos = 0; for (i = 0; path[i]; i++) { - if (path[i] == L'/' || path[i] == L'\\') { - lastSlashPos = i; - } + if (path[i] == L'/' || path[i] == L'\\') + lastSlashPos = i; } path[lastSlashPos] = 0; @@ -92,20 +93,21 @@ int GetCurrentModulePath(wchar_t *path, int pathSize) return -1; } -int load_config() { +int +load_config() { wchar_t basePath[PATH_MAX] = { 0 }; wchar_t path[PATH_MAX] = { 0 }; - if (GetCurrentModulePath(basePath, PATH_MAX) == -1) - return -1; + if (GetCurrentModulePath(basePath, PATH_MAX) == -1) + return -1; wcsncpy(path, basePath, PATH_MAX); - wcsncat(path, L"/sshd_config", PATH_MAX); + wcsncat(path, L"/sshd_config", PATH_MAX); - if ((config_file_name = utf16_to_utf8(path)) == NULL) - return -1; + if ((config_file_name = utf16_to_utf8(path)) == NULL) + return -1; - buffer_init(&cfg); + buffer_init(&cfg); initialize_server_options(&options); load_server_config(config_file_name, &cfg); parse_server_config(&options, config_file_name, &cfg, NULL); @@ -114,17 +116,17 @@ int load_config() { return 0; } -int config_log_level() { +int +config_log_level() { return options.log_level; } -int pubkey_allowed(struct sshkey* pubkey, HANDLE user_token) { +int +pubkey_allowed(struct sshkey* pubkey, HANDLE user_token) { struct passwd *pw; - int ret; - char *user = NULL, *user_home = NULL; - + if ((pw = w32_getpwtoken(user_token)) == NULL) return 0; - return user_key_allowed(pw, pubkey, 1); + return user_key_allowed(pw, pubkey, 1); } \ No newline at end of file diff --git a/contrib/win32/win32compat/w32-sshfileperm.c b/contrib/win32/win32compat/w32-sshfileperm.c new file mode 100644 index 000000000..60b4a782e --- /dev/null +++ b/contrib/win32/win32compat/w32-sshfileperm.c @@ -0,0 +1,348 @@ +/* +* Author: Yanbing Wang +* +* Copyright (c) 2009, 2011 NoMachine +* All rights reserved +* +* Support file permission check on Win32 based operating systems. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions +* are met: +* +* 1. Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* 2. Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in the +* documentation and/or other materials provided with the distribution. +* +* THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR +* IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES +* OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. +* IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, +* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT +* NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF +* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ + +#include +#include +#include +#include +#include +#include + +#include "inc\pwd.h" +#include "sshfileperm.h" +#include "misc_internal.h" +#include "debug.h" + +#define SSHD_ACCOUNT L"NT Service\\sshd" + +/* +* The function is to check if user prepresented by pw is secure to access to the file. +* Check the owner of the file is one of these types: Local Administrators groups, system account, +* direct user accounts in local administrators, or user represented by pw +* Check the users have access permission to the file don't voilate the following rules: + 1. no user other than local administrators group, system account, user represented by pw, + and owner accounts have write permission on the file + 2. sshd account can only have read permission + 3. user represented by pw and file owner should at least have read permission. +* Returns 0 on success and -1 on failure +*/ +int +check_secure_file_permission(const char *name, struct passwd * pw) +{ + PSECURITY_DESCRIPTOR pSD = NULL; + wchar_t * name_utf16 = NULL; + PSID owner_sid = NULL, user_sid = NULL; + PACL dacl = NULL; + DWORD error_code = ERROR_SUCCESS; + BOOL is_valid_sid = FALSE, is_valid_acl = FALSE; + struct passwd * pwd = pw; + int ret = 0; + + if (pwd == NULL) + if ((pwd = getpwuid(0)) == NULL) + fatal("getpwuid failed."); + + if (ConvertStringSidToSid(pwd->pw_sid, &user_sid) == FALSE || + (IsValidSid(user_sid) == FALSE)) { + debug3("failed to retrieve the sid of the pwd"); + ret = -1; + goto cleanup; + } + if ((name_utf16 = utf8_to_utf16(name)) == NULL) { + errno = ENOMEM; + goto cleanup; + } + + /*Get the owner sid of the file.*/ + if ((error_code = GetNamedSecurityInfoW(name_utf16, SE_FILE_OBJECT, + OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION, + &owner_sid, NULL, &dacl, NULL, &pSD)) != ERROR_SUCCESS) { + debug3("failed to retrieve the owner sid and dacl of file %s with error code: %d", name, error_code); + errno = EOTHER; + ret = -1; + goto cleanup; + } + if (((is_valid_sid = IsValidSid(owner_sid)) == FALSE) || ((is_valid_acl = IsValidAcl(dacl)) == FALSE)) { + debug3("IsValidSid: %d; is_valid_acl: %d", is_valid_sid, is_valid_acl); + ret = -1; + goto cleanup; + } + if (!IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) && + !IsWellKnownSid(owner_sid, WinLocalSystemSid) && + !EqualSid(owner_sid, user_sid) && + !is_admin_account(owner_sid)) { + debug3("Bad owner on %s", name); + ret = -1; + goto cleanup; + } + /* + iterate all aces of the file to find out if there is voilation of the following rules: + 1. no others than administrators group, system account, and current user, owner accounts have write permission on the file + 2. sshd account can only have read permission + 3. this user and file owner should at least have read permission + */ + for (DWORD i = 0; i < dacl->AceCount; i++) { + PVOID current_ace = NULL; + PACE_HEADER current_aceHeader = NULL; + PSID current_trustee_sid = NULL; + ACCESS_MASK current_access_mask = 0; + + if (!GetAce(dacl, i, ¤t_ace)) { + debug3("GetAce() failed"); + errno = EOTHER; + ret = -1; + goto cleanup; + } + + current_aceHeader = (PACE_HEADER)current_ace; + // Determine the location of the trustee's sid and the value of the access mask + switch (current_aceHeader->AceType) { + case ACCESS_ALLOWED_ACE_TYPE: { + PACCESS_ALLOWED_ACE pAllowedAce = (PACCESS_ALLOWED_ACE)current_ace; + current_trustee_sid = &(pAllowedAce->SidStart); + current_access_mask = pAllowedAce->Mask; + break; + } + case ACCESS_DENIED_ACE_TYPE: { + PACCESS_DENIED_ACE pDeniedAce = (PACCESS_DENIED_ACE)current_ace; + current_trustee_sid = &(pDeniedAce->SidStart); + if(pDeniedAce->Mask & (FILE_GENERIC_READ & ~(SYNCHRONIZE | READ_CONTROL)) != 0) { + if (EqualSid(current_trustee_sid, owner_sid)){ + debug3("Bad permission on %s. The owner of the file should at least have read permission.", name); + ret = -1; + goto cleanup; + } + else if (EqualSid(current_trustee_sid, user_sid)) { + debug3("Bad permission on %s. The user should at least have read permission.", name); + ret = -1; + goto cleanup; + } + } + continue; + } + default: { + // Not interested ACE + continue; + } + } + + /*no need to check administrators group, owner account, user account and system account*/ + if (IsWellKnownSid(current_trustee_sid, WinBuiltinAdministratorsSid) || + IsWellKnownSid(current_trustee_sid, WinLocalSystemSid) || + EqualSid(current_trustee_sid, owner_sid) || + EqualSid(current_trustee_sid, user_sid) || + is_admin_account(current_trustee_sid)) { + continue; + } + else if(is_sshd_account(current_trustee_sid)){ + if ((current_access_mask & ~FILE_GENERIC_READ) != 0){ + debug3("Bad permission. %s can only read access to %s", SSHD_ACCOUNT, name); + ret = -1; + break; + } + } + else { + ret = -1; + debug3("Bad permission. Other user or group than owner, admin user and local system have access to file %s.", name); + break; + } + } +cleanup: + if (pSD) + LocalFree(pSD); + if (user_sid) + FreeSid(user_sid); + if(name_utf16) + free(name_utf16); + return ret; +} + +static BOOL +is_sshd_account(PSID user_sid) { + wchar_t user_name[UNCLEN], full_name[UNCLEN + DNLEN + 2]; + DWORD name_length = UNCLEN, domain_name_length = 0, full_name_len = UNCLEN + DNLEN + 2; + SID_NAME_USE sid_type = SidTypeInvalid; + BOOL ret = FALSE; + + if (LookupAccountSidLocalW(user_sid, user_name, &name_length, full_name, &full_name_len, &sid_type) == FALSE) + { + debug3("LookupAccountSidLocalW() failed with error: %d. ", GetLastError()); + errno = ENOENT; + return FALSE; + } + domain_name_length = wcslen(full_name); + full_name[domain_name_length] = L'\\'; + wmemcpy(full_name + domain_name_length + 1, user_name, wcslen(user_name)+1); + return (wcsicmp(full_name, SSHD_ACCOUNT) == 0); +} + +/* + * Check if the user is in local administrators group + * currently only check if the user is directly in the group + * Returns TRUE if the user is in administrators group; otherwise false; +*/ +static BOOL +is_admin_account(PSID user_sid) +{ + DWORD entries_read = 0, total_entries = 0, i = 0, name_length = UNCLEN, domain_name_length = DNLEN; + LPLOCALGROUP_MEMBERS_INFO_1 local_groups_member_info = NULL; + PSID admins_sid = NULL; + wchar_t admins_group_name[UNCLEN], domain_name[DNLEN]; + SID_NAME_USE sid_type = SidTypeInvalid; + NET_API_STATUS status; + BOOL ret = FALSE; + + if (ConvertStringSidToSidW(L"S-1-5-32-544", &admins_sid) == FALSE || + (IsValidSid(user_sid) == FALSE)) { + debug3("ConvertStringSidToSidW failed with error code: %d.", GetLastError()); + goto done; + } + + if (LookupAccountSidLocalW(admins_sid, admins_group_name, &name_length, + domain_name, &domain_name_length, &sid_type) == FALSE) { + debug3("LookupAccountSidLocalW() failed with error: %d. ", GetLastError()); + errno = ENOENT; + goto done; + } + + status = NetLocalGroupGetMembers(NULL, admins_group_name, 1, (LPBYTE*)&local_groups_member_info, + MAX_PREFERRED_LENGTH, &entries_read, &total_entries, NULL); + if (status != NERR_Success) { + debug3("NetLocalGroupGetMembers failed with error code: %d.", status); + goto done; + } + + for (i = 0; i < entries_read; i++) { + if (local_groups_member_info[i].lgrmi1_sidusage == SidTypeDeletedAccount) + continue; + else if(EqualSid(local_groups_member_info[i].lgrmi1_sid, user_sid)) { + ret = TRUE; + break; + } + } + +done: + if (local_groups_member_info) + NetApiBufferFree(local_groups_member_info); + if(admins_sid) + LocalFree(admins_sid); + return ret; +} + +/* +* Set the owner of the secure file to the user represented by pw and only grant +* it the full control access +*/ +int +set_secure_file_permission(const char *name, struct passwd * pw) +{ + PSECURITY_DESCRIPTOR pSD = NULL; + PSID owner_sid = NULL; + PACL dacl = NULL; + wchar_t *name_utf16 = NULL, *sid_utf16 = NULL, sddl[256]; + DWORD error_code = ERROR_SUCCESS; + struct passwd * pwd = pw; + BOOL present, defaulted; + int ret = 0; + + if (pwd == NULL) + if ((pwd = getpwuid(0)) == NULL) + fatal("getpwuid failed."); + + if (ConvertStringSidToSid(pwd->pw_sid, &owner_sid) == FALSE) { + debug3("failed to retrieve the sid of the pwd with error code: %d", GetLastError()); + ret = -1; + goto cleanup; + } + + if((IsValidSid(owner_sid) == FALSE)) { + debug3("IsValidSid(owner_sid): FALSE"); + ret = -1; + goto cleanup; + } + + if ((sid_utf16 = utf8_to_utf16(pwd->pw_sid)) == NULL) { + debug3("Failed to get utf16 of the sid string"); + errno = ENOMEM; + ret = -1; + goto cleanup; + } + swprintf(sddl, sizeof(sddl) - 1, L"D:P(A;;FA;;;%s)", sid_utf16); + if(ConvertStringSecurityDescriptorToSecurityDescriptorW(sddl, SDDL_REVISION, &pSD, NULL) == FALSE) { + debug3("ConvertStringSecurityDescriptorToSecurityDescriptorW failed with error code %d", GetLastError()); + ret = -1; + goto cleanup; + } + + if (IsValidSecurityDescriptor(pSD) == FALSE) { + debug3("IsValidSecurityDescriptor return FALSE"); + ret = -1; + goto cleanup; + } + + if (GetSecurityDescriptorDacl(pSD, &present, &dacl, &defaulted) == FALSE) { + debug3("GetSecurityDescriptorDacl failed with error code %d", GetLastError()); + ret = -1; + goto cleanup; + } + if (!present || dacl == NULL) { + debug3("failed to find the acl from security descriptior."); + ret = -1; + goto cleanup; + } + + if ((name_utf16 = utf8_to_utf16(name)) == NULL) { + debug3("Failed to get utf16 of the name"); + errno = ENOMEM; + ret = -1; + goto cleanup; + } + + /*Set the owner sid and acl of the file.*/ + if ((error_code = SetNamedSecurityInfoW(name_utf16, SE_FILE_OBJECT, + OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION, + owner_sid, NULL, dacl, NULL)) != ERROR_SUCCESS) { + debug3("failed to set the owner sid and dacl of file %s with error code: %d", name, error_code); + errno = EOTHER; + ret = -1; + goto cleanup; + } +cleanup: + if (pSD) + LocalFree(pSD); + if (name_utf16) + free(name_utf16); + if(sid_utf16) + free(sid_utf16); + if (owner_sid) + FreeSid(owner_sid); + + return ret; +} \ No newline at end of file diff --git a/readconf.c b/readconf.c index 90ec46459..fa354b1e8 100644 --- a/readconf.c +++ b/readconf.c @@ -39,6 +39,7 @@ #include #include #include +#include #ifdef USE_SYSTEM_GLOB # include #else @@ -1735,8 +1736,16 @@ read_config_file_depth(const char *filename, struct passwd *pw, if ((f = fopen(filename, "r")) == NULL) return 0; -#ifndef WINDOWS /* TODO - implement permission checks for Windows */ - if (flags & SSHCONF_CHECKPERM) { + if (flags & SSHCONF_CHECKPERM) { +#if WINDOWS + /* + file permissions are designed differently on windows + implementation on windows to make sure the config file is owned by the user and + nobody else except Administrators group and current user of calling process, and SYSTEM account has the write permission + */ + if (check_secure_file_permission(filename, pw) != 0) + fatal("Bad owner or permissions on %s", filename); +#else struct stat sb; if (fstat(fileno(f), &sb) == -1) @@ -1744,8 +1753,9 @@ read_config_file_depth(const char *filename, struct passwd *pw, if (((sb.st_uid != 0 && sb.st_uid != getuid()) || (sb.st_mode & 022) != 0)) fatal("Bad owner or permissions on %s", filename); - } #endif /* !WINDOWS */ + } + debug("Reading configuration data %.200s", filename); diff --git a/regress/pesterTests/Authorized_keys_fileperm.Tests.ps1 b/regress/pesterTests/Authorized_keys_fileperm.Tests.ps1 new file mode 100644 index 000000000..7c0bde2a1 --- /dev/null +++ b/regress/pesterTests/Authorized_keys_fileperm.Tests.ps1 @@ -0,0 +1,168 @@ +Import-Module $PSScriptRoot\CommonUtils.psm1 -Force +Describe "Tests for authorized_keys file permission" -Tags "CI" { + BeforeAll { + if($OpenSSHTestInfo -eq $null) + { + Throw "`$OpenSSHTestInfo is null. Please run Setup-OpenSSHTestEnvironment to setup test environment." + } + + $testDir = "$($OpenSSHTestInfo["TestDataPath"])\authorized_keys_fileperm" + if( -not (Test-path $testDir -PathType Container)) + { + $null = New-Item $testDir -ItemType directory -Force -ErrorAction SilentlyContinue + } + + $fileName = "test.txt" + $filePath = Join-Path $testDir $fileName + $logName = "log.txt" + $logPath = Join-Path $testDir $logName + $server = $OpenSSHTestInfo["Target"] + $port = 47003 + $ssouser = $OpenSSHTestInfo["SSOUser"] + $PwdUser = $OpenSSHTestInfo["PasswdUser"] + $ssouserProfile = $OpenSSHTestInfo["SSOUserProfile"] + $script:logNum = 0 + + # for the first time, delete the existing log files. + if ($OpenSSHTestInfo['DebugMode']) + { + Clear-Content "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\ssh-agent.log" -Force -ErrorAction ignore + } + + Remove-Item -Path $filePath -Force -ErrorAction ignore + } + + AfterEach { + if( $OpenSSHTestInfo["DebugMode"]) + { + Copy-Item "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\ssh-agent.log" "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\failedagent$script:logNum.log" -Force -ErrorAction ignore + Copy-Item $logPath "$($script:logNum)$($logPath)" -Force -ErrorAction ignore + Clear-Content $logPath -Force -ErrorAction ignore + $script:logNum++ + + # clear the ssh-agent so that next testcase will get fresh logs. + Clear-Content "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\ssh-agent.log" -Force -ErrorAction ignore + } + } + + Context "Authorized key file permission" { + BeforeAll { + $ssouserSSHProfilePath = Join-Path $ssouserProfile .testssh + if(-not (Test-Path $ssouserSSHProfilePath -PathType Container)) { + New-Item $ssouserSSHProfilePath -ItemType directory -Force -ErrorAction Stop | Out-Null + } + $authorizedkeyPath = Join-Path $ssouserProfile .testssh\authorized_keys + $Source = Join-Path $ssouserProfile .ssh\authorized_keys + $testknownhosts = Join-path $PSScriptRoot testdata\test_known_hosts + if(Test-Path $authorizedkeyPath) { + Set-SecureFileACL -filepath $authorizedkeyPath + } + Copy-Item $Source $ssouserSSHProfilePath -Force -ErrorAction Stop + + Remove-Item $filePath -Force -ErrorAction Ignore + Get-Process -Name sshd | Where-Object {$_.SI -ne 0} | Stop-process + #add wrong password so ssh does not prompt password if failed with authorized keys + Add-PasswordSetting -Pass "WrongPass" + } + + AfterAll { + if(Test-Path $authorizedkeyPath) { + Set-SecureFileACL -filepath $authorizedkeyPath + Remove-Item $authorizedkeyPath -Force -ErrorAction Ignore + } + if(Test-Path $ssouserSSHProfilePath) { + Remove-Item $ssouserSSHProfilePath -Force -ErrorAction Ignore + } + Remove-PasswordSetting + } + + AfterEach { + Remove-Item -Path $filePath -Force -ErrorAction ignore + } + + It 'Authorized key file -- positive (authorized_keys is owned by current user and running process can access to the file)' { + #setup to have current user (admin user) as owner and grant it full control + Set-SecureFileACL -filepath $authorizedkeyPath + + #Run + Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-o `"AuthorizedKeysFile .testssh/authorized_keys`"", "-E $logPath") -NoNewWindow + $o = ssh -p $port $ssouser@$server -o "UserKnownHostsFile $testknownhosts" echo 1234 + $o | Should Be "1234" + + #Cleanup + Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } } + } + + It 'Authorized key file -- positive (Secured file and sshd can access to the file)' { + #setup to have ssouser as owner and grant it full control + $objUser = New-Object System.Security.Principal.NTAccount($ssouser) + Set-SecureFileACL -filepath $authorizedkeyPath -Owner $objUser + + #add running process account Read access the file authorized_keys + $currentUser = New-Object System.Security.Principal.NTAccount($($env:USERDOMAIN), $($env:USERNAME)) + Add-PermissionToFileACL -FilePath $authorizedkeyPath -User $currentUser -Perm "Read" + + #Run + Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-o `"AuthorizedKeysFile .testssh/authorized_keys`"", "-E $logPath") -NoNewWindow + $o = ssh -p $port $ssouser@$server -o "UserKnownHostsFile $testknownhosts" echo 1234 + $o | Should Be "1234" + + #Cleanup + Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } } + } + + It 'Authorized key file -- negative (other account can access private key file)' { + #setup to have current user as owner and grant it full control + Set-SecureFileACL -filepath $authorizedkeyPath + #add $PwdUser to access the file authorized_keys + $objPwdUser = New-Object System.Security.Principal.NTAccount($PwdUser) + Add-PermissionToFileACL -FilePath $authorizedkeyPath -User $objPwdUser -Perm "Read" + + #Run + Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-o `"AuthorizedKeysFile .testssh/authorized_keys`"", "-E $logPath") -NoNewWindow + ssh -p $port -E $filePath -o "UserKnownHostsFile $testknownhosts" $ssouser@$server echo 1234 + $LASTEXITCODE | Should Not Be 0 + $matches = Get-Content $filePath | Select-String -pattern "Permission denied" + $matches.Count | Should Not Be 0 + + #Cleanup + Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } } + } + + It 'Authorized key file -- negative (the authorized_keys has wrong owner)' { + #setup to have ssouser as owner and grant it full control + $objPwdUser = New-Object System.Security.Principal.NTAccount($PwdUser) + Set-SecureFileACL -filepath $authorizedkeyPath -owner $objPwdUser + + #add current user full access the file authorized_keys + $currentUser = New-Object System.Security.Principal.NTAccount($($env:USERDOMAIN), $($env:USERNAME)) + Add-PermissionToFileACL -FilePath $authorizedkeyPath -User $currentUser -Perm "FullControl" + + #Run + Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-o `"AuthorizedKeysFile .testssh/authorized_keys`"", "-E $logPath") -NoNewWindow + ssh -p $port -E $filePath -o "UserKnownHostsFile $testknownhosts" $ssouser@$server echo 1234 + $LASTEXITCODE | Should Not Be 0 + $matches = Get-Content $filePath | Select-String -pattern "Permission denied" + $matches.Count | Should Not Be 0 + + #Cleanup + Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } } + } + It 'Authorized key file -- negative (the running process does not have read access to the authorized_keys)' { + #setup to have ssouser as owner and grant it full control + $objUser = New-Object System.Security.Principal.NTAccount($ssouser) + Set-SecureFileACL -filepath $authorizedkeyPath -Owner $objUser + + #Run + Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-o `"AuthorizedKeysFile .testssh/authorized_keys`"", "-E $logPath") -NoNewWindow + ssh -p $port -E $filePath -o "UserKnownHostsFile $testknownhosts" $ssouser@$server echo 1234 + $LASTEXITCODE | Should Not Be 0 + + $matches = Get-Content $filePath | Select-String -pattern "Permission denied" + $matches.Count | Should Not Be 0 + + #Cleanup + Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } } + } + } +} diff --git a/regress/pesterTests/Cfginclude.Tests.ps1 b/regress/pesterTests/Cfginclude.Tests.ps1 new file mode 100644 index 000000000..391b45308 --- /dev/null +++ b/regress/pesterTests/Cfginclude.Tests.ps1 @@ -0,0 +1,152 @@ +Describe "Tests for ssh config" -Tags "CI" { + BeforeAll { + if($OpenSSHTestInfo -eq $null) + { + Throw "`$OpenSSHTestInfo is null. Please run Setup-OpenSSHTestEnvironment to setup test environment." + } + + if(-not (Test-Path $OpenSSHTestInfo["TestDataPath"])) + { + $null = New-Item $OpenSSHTestInfo["TestDataPath"] -ItemType directory -Force -ErrorAction SilentlyContinue + } + $testDir = "$($OpenSSHTestInfo["TestDataPath"])\cfginclude" + $null = New-Item $testDir -ItemType directory -Force -ErrorAction SilentlyContinue + $fileName = "test.txt" + $filePath = Join-Path $testDir $fileName + $logName = "log.txt" + $logPath = Join-Path $testDir $logName + + $server = $OpenSSHTestInfo["Target"] + $port = $OpenSSHTestInfo["Port"] + $ssouser = $OpenSSHTestInfo["SSOUser"] + $script:logNum = 0 + + # for the first time, delete the existing log files. + if ($OpenSSHTestInfo['DebugMode']) + { + Clear-Content "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\ssh-agent.log" -Force -ErrorAction ignore + Clear-Content "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\sshd.log" -Force -ErrorAction ignore + } + Remove-Item -Path $filePath -Force -ErrorAction ignore + + function Set-SecureFileACL + { + [CmdletBinding()] + param( + [string]$FilePath, + [System.Security.Principal.NTAccount]$Owner = $null, + [System.Security.AccessControl.FileSystemAccessRule]$ACE = $null + ) + + $myACL = Get-ACL -Path $FilePath + $myACL.SetAccessRuleProtection($True, $True) + Set-Acl -Path $FilePath -AclObject $myACL + + $myACL = Get-ACL $FilePath + if($owner -ne $null) + { + $myACL.SetOwner($Owner) + } + + if($myACL.Access) + { + $myACL.Access | % { + if (($_ -ne $null) -and ($_.IdentityReference.Value -ine "BUILTIN\Administrators") -and + ($_.IdentityReference.Value -ine "NT AUTHORITY\SYSTEM") -and + ($_.IdentityReference.Value -ine "$(whoami)")) + { + if(-not ($myACL.RemoveAccessRule($_))) + { + throw "failed to remove access of $($_.IdentityReference.Value) rule in setup " + } + } + } + } + if($ACE -ne $null) + { + $myACL.AddAccessRule($ACE) + } + + Set-Acl -Path $FilePath -AclObject $myACL + } + } + + AfterAll { + Remove-Item -Path $filePath -Force -ErrorAction ignore + } + + AfterEach { + if( $OpenSSHTestInfo["DebugMode"]) + { + Copy-Item "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\ssh-agent.log" "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\failedagent$script:logNum.log" -Force -ErrorAction ignore + Copy-Item "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\sshd.log" "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\failedsshd$script:logNum.log" -Force -ErrorAction ignore + Copy-Item $logPath "$($script:logNum)$($logPath)" -Force -ErrorAction ignore + Clear-Content $logPath -Force -ErrorAction ignore + $script:logNum++ + + # clear the ssh-agent, sshd logs so that next testcase will get fresh logs. + Clear-Content "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\ssh-agent.log" -Force -ErrorAction ignore + Clear-Content "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\sshd.log" -Force -ErrorAction ignore + } + Remove-Item -Path $filePath -Force -ErrorAction ignore + } + + Context "User SSHConfig -- ReadConfig" { + BeforeAll { + $userConfigFile = Join-Path $home ".ssh\config" + Copy-item "$PSScriptRoot\testdata\ssh_config" $userConfigFile -force + $oldACL = Get-ACL $userConfigFile + } + AfterEach { + Set-Acl -Path $userConfigFile -AclObject $oldACL + } + + AfterAll { + Remove-Item -Path $userConfigFile -Force -ErrorAction ignore + } + + It 'User SSHConfig -- ReadConfig (admin user is the owner)' { + #setup + Set-SecureFileACL -filepath $userConfigFile + + #Run + $str = "ssh -p $($port) -E $logPath $($Options) $($ssouser)@$($server) hostname > $filePath" + cmd /c $str + $LASTEXITCODE | Should Be 0 + + #validate file content. + Get-Content $filePath | Should be $env:COMPUTERNAME + + #clean up + Set-Acl -Path $userConfigFile -AclObject $oldACL + } + It 'User SSHConfig -- ReadConfig (wrong owner)' { + #setup + Set-SecureFileACL -filepath $userConfigFile -owner $ssouser + + #Run + $str = "ssh -p $($port) -E $logPath $($Options) $($ssouser)@$($server) hostname > $filePath" + cmd /c $str + + #clean up + $LASTEXITCODE | Should Not Be 0 + } + + It 'User SSHConfig -- ReadConfig (wrong permission)' { + #setup + $owner = New-Object System.Security.Principal.NTAccount($($env:USERDOMAIN), $($env:USERNAME)) + + $objUser = New-Object System.Security.Principal.NTAccount($ssouser) + $objACE = New-Object System.Security.AccessControl.FileSystemAccessRule ` + ($objUser, "Read, Write", "None", "None", "Allow") + Set-SecureFileACL -filepath $userConfigFile -owner $owner -Ace $objACE + + #Run + $str = "ssh -p $($port) -E $logPath $($Options) $($ssouser)@$($server) hostname > $filePath" + cmd /c $str + + #clean up + $LASTEXITCODE | Should Not Be 0 + } + } +} diff --git a/regress/pesterTests/CommonUtils.psm1 b/regress/pesterTests/CommonUtils.psm1 new file mode 100644 index 000000000..e8374a670 --- /dev/null +++ b/regress/pesterTests/CommonUtils.psm1 @@ -0,0 +1,106 @@ +Enum PlatformType { + Windows + Linux + OSX +} + +function Get-Platform { + # Use the .NET Core APIs to determine the current platform; if a runtime + # exception is thrown, we are on FullCLR, not .NET Core. + try { + $Runtime = [System.Runtime.InteropServices.RuntimeInformation] + $OSPlatform = [System.Runtime.InteropServices.OSPlatform] + + $IsLinux = $Runtime::IsOSPlatform($OSPlatform::Linux) + $IsOSX = $Runtime::IsOSPlatform($OSPlatform::OSX) + $IsWindows = $Runtime::IsOSPlatform($OSPlatform::Windows) + } catch { + try { + $IsLinux = $false + $IsOSX = $false + $IsWindows = $true + } + catch { } + } + if($IsOSX) { + [PlatformType]::OSX + } elseif($IsLinux) { + [PlatformType]::Linux + } else { + [PlatformType]::Windows + } +} + +function Set-SecureFileACL +{ + param( + [string]$FilePath, + [System.Security.Principal.NTAccount]$Owner = $null + ) + + $myACL = Get-ACL -Path $FilePath + $myACL.SetAccessRuleProtection($True, $True) + Set-Acl -Path $FilePath -AclObject $myACL + + $myACL = Get-ACL $FilePath + $actualOwner = $null + if($owner -eq $null) + { + $actualOwner = New-Object System.Security.Principal.NTAccount($($env:USERDOMAIN), $($env:USERNAME)) + } + else + { + $actualOwner = $Owner + } + + $myACL.SetOwner($actualOwner) + + if($myACL.Access) + { + $myACL.Access | % { + if(-not ($myACL.RemoveAccessRule($_))) + { + throw "failed to remove access of $($_.IdentityReference.Value) rule in setup " + } + } + } + + $objACE = New-Object System.Security.AccessControl.FileSystemAccessRule ` + ($actualOwner, "FullControl", "None", "None", "Allow") + $myACL.AddAccessRule($objACE) + + Set-Acl -Path $FilePath -AclObject $myACL +} + +function Add-PermissionToFileACL +{ + param( + [string]$FilePath, + [System.Security.Principal.NTAccount] $User, + [System.Security.AccessControl.FileSystemRights]$Perm + ) + + $myACL = Get-ACL $filePath + + $objACE = New-Object System.Security.AccessControl.FileSystemAccessRule ` + ($User, $perm, "None", "None", "Allow") + $myACL.AddAccessRule($objACE) + + Set-Acl -Path $filePath -AclObject $myACL +} + +function Add-PasswordSetting +{ + param([string] $pass) + $platform = Get-Platform + if ($platform -eq [PlatformType]::Windows) { + if (-not($env:DISPLAY)) {$env:DISPLAY = 1} + $env:SSH_ASKPASS="$($env:ComSpec) /c echo $pass" + } +} + +function Remove-PasswordSetting +{ + if ($env:DISPLAY -eq 1) { Remove-Item env:\DISPLAY } + Remove-item "env:SSH_ASKPASS" -ErrorAction SilentlyContinue +} \ No newline at end of file diff --git a/regress/pesterTests/Hostkey_fileperm.Tests.ps1 b/regress/pesterTests/Hostkey_fileperm.Tests.ps1 new file mode 100644 index 000000000..7a0d2da32 --- /dev/null +++ b/regress/pesterTests/Hostkey_fileperm.Tests.ps1 @@ -0,0 +1,138 @@ +Import-Module $PSScriptRoot\CommonUtils.psm1 -Force +Describe "Tests for host keys file permission" -Tags "CI" { + BeforeAll { + if($OpenSSHTestInfo -eq $null) + { + Throw "`$OpenSSHTestInfo is null. Please run Setup-OpenSSHTestEnvironment to setup test environment." + } + + $testDir = "$($OpenSSHTestInfo["TestDataPath"])\hostkey_fileperm" + if( -not (Test-path $testDir -PathType Container)) + { + $null = New-Item $testDir -ItemType directory -Force -ErrorAction SilentlyContinue + } + + $fileName = "test.txt" + $filePath = Join-Path $testDir $fileName + $logName = "log.txt" + $logPath = Join-Path $testDir $logName + $port = 47003 + $ssouser = $OpenSSHTestInfo["SSOUser"] + $script:logNum = 0 + + # for the first time, delete the existing log files. + if ($OpenSSHTestInfo['DebugMode']) + { + Clear-Content "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\ssh-agent.log" -Force -ErrorAction ignore + } + + Remove-Item -Path $filePath -Force -ErrorAction ignore + } + + AfterEach { + if( $OpenSSHTestInfo["DebugMode"]) + { + Copy-Item "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\ssh-agent.log" "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\failedagent$script:logNum.log" -Force -ErrorAction ignore + Copy-Item $logPath "$($script:logNum)$($logPath)" -Force -ErrorAction ignore + Clear-Content $logPath -Force -ErrorAction ignore + $script:logNum++ + + # clear the ssh-agent so that next testcase will get fresh logs. + Clear-Content "$($OpenSSHTestInfo['OpenSSHBinPath'])\logs\ssh-agent.log" -Force -ErrorAction ignore + } + Remove-Item -Path $filePath -Force -ErrorAction ignore + } + + Context "Host key files permission" { + BeforeAll { + $hostKeyFilePath = "$($OpenSSHTestInfo['OpenSSHBinPath'])\hostkeyFilePermTest_ed25519_key" + if(Test-path $hostKeyFilePath -PathType Leaf){ + Set-SecureFileACL -filepath $hostKeyFilePath + } + if(Test-path "$hostKeyFilePath.pub" -PathType Leaf){ + Set-SecureFileACL -filepath "$hostKeyFilePath.pub" + } + Remove-Item -path "$hostKeyFilePath*" -Force -ErrorAction Ignore + ssh-keygen.exe -t ed25519 -f $hostKeyFilePath -P `"`" + + Remove-Item $filePath -Force -ErrorAction Ignore + Get-Process -Name sshd | Where-Object {$_.SI -ne 0} | Stop-process + } + + AfterEach { + Remove-Item -Path $filePath -Force -ErrorAction ignore + } + + It 'Host keys -- positive (Secured private key and sshd can access to public key file)' { + #setup to have current user as owner and grant it full control + Set-SecureFileACL -filepath $hostKeyFilePath + #grant sshd Read permission to public key + Add-PermissionToFileACL -FilePath "$hostKeyFilePath.pub" -User "NT Service\sshd" -Perm "Read" + + #Run + Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-h $hostKeyFilePath", "-E $filePath") -NoNewWindow + Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } } + + + #validate file content does not contain unprotected. + $matches = Get-Content $filePath | Select-String -pattern "UNPROTECTED PRIVATE KEY FILE!" + $matches.Count | Should Be 0 + } + + It 'Host keys -- negative (other account can access private key file)' { + #setup to have current user as owner and grant it full control + Set-SecureFileACL -filepath $hostKeyFilePath + #add ssouser to access the private key + $objUser = New-Object System.Security.Principal.NTAccount($ssouser) + Add-PermissionToFileACL -FilePath $hostKeyFilePath -User $objUser -Perm "Read" + + #grant sshd Read permission to the public key + Add-PermissionToFileACL -FilePath "$hostKeyFilePath.pub" -User "NT Service\sshd" -Perm "Read" + + #Run + Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-h $hostKeyFilePath", "-E $filePath") -NoNewWindow + Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } } + + #validate file content does not contain unprotected. + $matches = Get-Content $filePath | Select-String -pattern "key_load_private: bad permissions" + $matches.Count | Should Be 1 + } + + It 'Host keys -- negative (the private key has wrong owner)' { + #setup to have ssouser as owner and grant it full control + $objUser = New-Object System.Security.Principal.NTAccount($ssouser) + Set-SecureFileACL -filepath $hostKeyFilePath -owner $objUser + $currentUser = New-Object System.Security.Principal.NTAccount($($env:USERDOMAIN), $($env:USERNAME)) + Add-PermissionToFileACL -FilePath $hostKeyFilePath -User $currentUser -Perm "FullControl" + + #grant sshd Read permission to the public key + Add-PermissionToFileACL -FilePath "$hostKeyFilePath.pub" -User "NT Service\sshd" -Perm "Read" + + #Run + Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-h $hostKeyFilePath", "-E $filePath") -NoNewWindow + Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } } + + #validate file content does not contain unprotected. + $matches = Get-Content $filePath | Select-String -pattern "key_load_private: bad permissions" + $matches.Count | Should Be 1 + } + It 'Host keys -- negative (the running process does not have read access to pub key)' { + #setup to have ssouser as owner and grant it full control + Set-SecureFileACL -filepath $hostKeyFilePath + + $objUser = New-Object System.Security.Principal.NTAccount($ssouser) + Set-SecureFileACL -filepath "$hostKeyFilePath.pub" -owner $objUser + + #grant current user Read permission to public key + Add-PermissionToFileACL -FilePath "$hostKeyFilePath.pub" -User $objUser -Perm "Read" + + #Run + Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-h $hostKeyFilePath", "-E $filePath") -NoNewWindow + Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } } + + #validate file content does not contain unprotected. + $matches = Get-Content $filePath | Select-String -pattern "key_load_public: Permission denied" + $matches.Count | Should Be 1 + } + } +} diff --git a/regress/pesterTests/Keygen.Tests.ps1 b/regress/pesterTests/Keygen.Tests.ps1 new file mode 100644 index 000000000..da6bb41a7 --- /dev/null +++ b/regress/pesterTests/Keygen.Tests.ps1 @@ -0,0 +1,62 @@ +Describe "Tests for ssh-keygen" -Tags "CI" { + BeforeAll { + if($OpenSSHTestInfo -eq $null) + { + Throw "`$OpenSSHTestInfo is null. Please run Setup-OpenSSHTestEnvironment to setup test environment." + } + + $testDir = "$($OpenSSHTestInfo["TestDataPath"])\keygen" + if( -not (Test-path $testDir -PathType Container)) + { + $null = New-Item $testDir -ItemType directory -Force -ErrorAction SilentlyContinue + } + #only validate owner and ACE of the file + function ValidKeyFile { + param($Path) + + $currentUser = New-Object System.Security.Principal.NTAccount($($env:USERDOMAIN), $($env:USERNAME)) + $myACL = Get-ACL $Path + $myACL.Owner.Equals($currentUser.Value) | Should Be $true + $myACL.Access | Should Not Be $null + $myACL.Access.Count | Should Be 1 + + $myACL.Access[0].IdentityReference.Equals($currentUser) | Should Be $true + $myACL.Access[0].AccessControlType | Should Be ([System.Security.AccessControl.AccessControlType]::Allow) + $myACL.Access[0].FileSystemRights | Should Be ([System.Security.AccessControl.FileSystemRights]::FullControl) + $myACL.Access[0].IsInherited | Should Be $false + $myACL.Access[0].InheritanceFlags | Should Be ([System.Security.AccessControl.InheritanceFlags]::None) + $myACL.Access[0].PropagationFlags | Should Be ([System.Security.AccessControl.PropagationFlags]::None) + } + } + + Context "Keygen key files" { + BeforeEach { + Remove-Item $testDir\* -Force -ErrorAction ignore + Remove-Item "$PSScriptRoot\ssh_host_*_key*" -Force -ErrorAction ignore + } + + It 'Keygen -A' { + ssh-keygen -A + + Get-ChildItem "$PSScriptRoot\ssh_host_*_key" | % { + ValidKeyFile -Path $_.FullName + } + + Get-ChildItem "$PSScriptRoot\ssh_host_*_key.pub" | % { + ValidKeyFile -Path $_.FullName + } + } + + It 'Keygen -t -f' { + $pwd = "testpassword" + + foreach($type in @("rsa","dsa","ecdsa","ed25519")) + { + $keyPath = Join-Path $testDir "id_$type" + ssh-keygen -t $type -P $pwd -f $keyPath + ValidKeyFile -Path $keyPath + ValidKeyFile -Path "$keyPath.pub" + } + } + } +} diff --git a/regress/pesterTests/SCP.Tests.ps1 b/regress/pesterTests/SCP.Tests.ps1 index 30c78a24c..f013849ee 100644 --- a/regress/pesterTests/SCP.Tests.ps1 +++ b/regress/pesterTests/SCP.Tests.ps1 @@ -66,13 +66,13 @@ Describe "Tests for scp command" -Tags "CI" { Source = $SourceFilePath Destination = "$($ssouser)@$($server):$DestinationDir" Options = "-P $port -C -q" - }, + }<#, @{ Title = 'simple copy remote file to local dir' Source = "$($ssouser)@$($server):$SourceFilePath" Destination = $DestinationDir Options = "-P $port " - } + }#> ) $testData1 = @( diff --git a/regress/pesterTests/Userkey_fileperm.Tests.ps1 b/regress/pesterTests/Userkey_fileperm.Tests.ps1 new file mode 100644 index 000000000..ed7fe807f --- /dev/null +++ b/regress/pesterTests/Userkey_fileperm.Tests.ps1 @@ -0,0 +1,185 @@ +Import-Module $PSScriptRoot\CommonUtils.psm1 -Force +Describe "Tests for user Key file permission" -Tags "CI" { + BeforeAll { + if($OpenSSHTestInfo -eq $null) + { + Throw "`$OpenSSHTestInfo is null. Please run Setup-OpenSSHTestEnvironment to setup test environment." + } + $testDir = "$($OpenSSHTestInfo["TestDataPath"])\usertkey_fileperm" + if( -not (Test-path $testDir -PathType Container)) + { + $null = New-Item $testDir -ItemType directory -Force -ErrorAction SilentlyContinue + } + $fileName = "test.txt" + $filePath = Join-Path $testDir $fileName + $port = $OpenSSHTestInfo["Port"] + $ssouser = $OpenSSHTestInfo["SSOUser"] + $pubKeyUser = $OpenSSHTestInfo["PubKeyUser"] + $pubKeyUserProfile = $OpenSSHTestInfo["PubKeyUserProfile"] + $server = $OpenSSHTestInfo["Target"] + $keyFileName = "sshtest_userPermTestkey_ed25519" + $keyFilePath = Join-Path $testDir $keyFileName + Remove-Item -path "$keyFilePath*" -Force -ErrorAction Ignore + ssh-keygen.exe -t ed25519 -f $keyFilePath -P `"`" + $userName = "$env:USERNAME@$env:USERDOMAIN" + if(Test-Path $keyFilePath) { + Set-SecureFileACL -filepath $keyFilePath + } + #add wrong password so ssh does not prompt password if failed with authorized keys + Add-PasswordSetting -Pass "WrongPass" + } + + AfterAll { + Remove-PasswordSetting + } + + <# comment the test out since ssh-add have impact on + existing default test environment. + Context "ssh-add key files" { + BeforeEach { + ssh-add -D 2>&1 > $fileName + } + + AfterEach { + ssh-add -D 2>&1 > $fileName + if(Test-Path $keyFilePath) { + Set-SecureFileACL -filepath $keyFilePath + } + } + + It 'ssh-add positive (Secured private key owned by current user)' { + #setup to have current user as owner and grant it full control + Set-SecureFileACL -filepath $keyFilePath + ssh-add $keyFilePath 2>&1 > $fileName + $LASTEXITCODE | Should Be 0 + $o = ssh-add -l + $matches = $o | Select-String -pattern "no identities" + $matches.Count | Should Be 0 + + $matches = $o | Select-String -pattern $userName + $matches.Count | Should Be 1 + } + + It 'ssh-add positive (Secured private key owned by Administrators group)' { + #setup to have local admin group as owner and grant it full control + $objAdmin = New-Object System.Security.Principal.NTAccount("BUILTIN", "Administrators") + Set-SecureFileACL -filepath $keyFilePath -Owner $objAdmin + ssh-add $keyFilePath 2>&1 > $fileName + $LASTEXITCODE | Should Be 0 + $o = ssh-add -l + $matches = $o | Select-String -pattern "no identities" + $matches.Count | Should Be 0 + + $matches = $o | Select-String -pattern $userName + $matches.Count | Should Be 1 + } + + It 'ssh-add -- negative (other account can access private key file)' { + #setup to have current user as owner and grant it full control + Set-SecureFileACL -filepath $keyFilePath + + #add ssouser to access the private key + $objUser = New-Object System.Security.Principal.NTAccount($ssouser) + Add-PermissionToFileACL -FilePath $keyFilePath -User $objUser -Perm "Read" + + ssh-add $keyFilePath 2>&1 > $fileName + $LASTEXITCODE | Should Not Be 0 + $o = ssh-add -l + $matches = $o | Select-String -pattern "no identities" + $matches.Count | Should Be 1 + } + + It 'ssh-add -- negative (the private key has wrong owner)' { + #setup to have ssouser as owner and grant it full control + $objUser = New-Object System.Security.Principal.NTAccount($ssouser) + Set-SecureFileACL -filepath $keyFilePath -owner $objUser + $currentUser = New-Object System.Security.Principal.NTAccount($($env:USERDOMAIN), $($env:USERNAME)) + Add-PermissionToFileACL -FilePath $keyFilePath -User $currentUser -Perm "FullControl" + + ssh-add $keyFilePath 2>&1 > $fileName + $LASTEXITCODE | Should Not Be 0 + $o = ssh-add -l + $matches = $o | Select-String -pattern "no identities" + $matches.Count | Should Be 1 + } + }#> + + Context "ssh with private key file" { + BeforeAll { + $pubKeyUserProfilePath = Join-Path $pubKeyUserProfile .ssh + if(-not (Test-Path $pubKeyUserProfilePath -PathType Container)) { + New-Item $pubKeyUserProfilePath -ItemType directory -Force -ErrorAction Stop | Out-Null + } + Set-SecureFileACL -filepath $keyFilePath + $testAuthorizedKeyPath = Join-Path $pubKeyUserProfilePath authorized_keys + Copy-Item "$keyFilePath.pub" $testAuthorizedKeyPath -Force -ErrorAction SilentlyContinue + Add-PermissionToFileACL -FilePath $testAuthorizedKeyPath -User "NT Service\sshd" -Perm "Read" + Remove-Item -Path $filePath -Force -ErrorAction ignore + } + AfterAll { + if(Test-Path $testAuthorizedKeyPath) { + Set-SecureFileACL -filepath $testAuthorizedKeyPath + Remove-Item $testAuthorizedKeyPath -Force -ErrorAction Ignore + } + if(Test-Path $pubKeyUserProfilePath) { + Remove-Item $pubKeyUserProfilePath -Force -ErrorAction Ignore + } + } + + AfterEach { + if(Test-Path $keyFilePath) { + Set-SecureFileACL -filepath $keyFilePath + } + + Remove-Item -Path $filePath -Force -ErrorAction ignore + } + + It 'ssh with private key file -- positive (Secured private key owned by current user)' { + Set-SecureFileACL -filepath $keyFilePath + #Run + $o = ssh -p $port -i $keyFilePath $pubKeyUser@$server echo 1234 + $o | Should Be "1234" + } + + It 'ssh with private key file -- positive (Secured private key owned by Administrators group)' { + #setup to have local admin group as owner and grant it full control + $objAdmin = New-Object System.Security.Principal.NTAccount("BUILTIN", "Administrators") + Set-SecureFileACL -filepath $keyFilePath -Owner $objAdmin + + #Run + $o = ssh -p $port -i $keyFilePath $pubKeyUser@$server echo 1234 + $o | Should Be "1234" + } + + It 'ssh with private key file -- negative (other account can access private key file)' { + #setup to have current user as owner and grant it full control + Set-SecureFileACL -filepath $keyFilePath + + #add ssouser to access the private key + $objUser = New-Object System.Security.Principal.NTAccount($ssouser) + Add-PermissionToFileACL -FilePath $keyFilePath -User $objUser -Perm "Read" + + #Run + $o = ssh -p $port -i $keyFilePath -E $filePath $pubKeyUser@$server echo 1234 + $LASTEXITCODE | Should Not Be 0 + + $matches = Get-Content $filePath | Select-String -pattern "UNPROTECTED PRIVATE KEY FILE!" + $matches.Count | Should Be 1 + } + + It 'ssh with private key file -- (the private key has wrong owner)' { + #setup to have ssouser as owner and grant it full control + $objUser = New-Object System.Security.Principal.NTAccount($ssouser) + Set-SecureFileACL -filepath $keyFilePath -owner $objUser + + $currentUser = New-Object System.Security.Principal.NTAccount($($env:USERDOMAIN), $($env:USERNAME)) + Add-PermissionToFileACL -FilePath $keyFilePath -User $currentUser -Perm "FullControl" + + $o = ssh -p $port -i $keyFilePath -E $filePath $pubKeyUser@$server echo 1234 + $LASTEXITCODE | Should Not Be 0 + + $matches = Get-Content $filePath | Select-String -pattern "UNPROTECTED PRIVATE KEY FILE!" + $matches.Count | Should Be 1 + } + } +} diff --git a/regress/pesterTests/testdata/ssh_config b/regress/pesterTests/testdata/ssh_config new file mode 100644 index 000000000..4c499e423 --- /dev/null +++ b/regress/pesterTests/testdata/ssh_config @@ -0,0 +1,4 @@ +# test usage of ssh_config + +PubkeyAcceptedKeyTypes ssh-ed25519* + diff --git a/regress/pesterTests/testdata/sshtest_userPermTestkey_ed25519 b/regress/pesterTests/testdata/sshtest_userPermTestkey_ed25519 new file mode 100644 index 000000000..0b15bf991 --- /dev/null +++ b/regress/pesterTests/testdata/sshtest_userPermTestkey_ed25519 @@ -0,0 +1,8 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACAN1tdRDiL3ZAZMPT3c3/3Gg/XbWPK3M0gAZPhIFHivHgAAALBPa9N1T2vT +dQAAAAtzc2gtZWQyNTUxOQAAACAN1tdRDiL3ZAZMPT3c3/3Gg/XbWPK3M0gAZPhIFHivHg +AAAEAkxz77KuyYDchGmc6owF2ykq2rMzRqqQaEpJgyTrsLVA3W11EOIvdkBkw9Pdzf/caD +9dtY8rczSABk+EgUeK8eAAAAJm5ld2xvZ2luQFlBTkJJTkdXMksxMlIyQFlhbmJpbmd3Mm +sxMnIyAQIDBAUGBw== +-----END OPENSSH PRIVATE KEY----- diff --git a/regress/pesterTests/testdata/sshtest_userPermTestkey_ed25519.pub b/regress/pesterTests/testdata/sshtest_userPermTestkey_ed25519.pub new file mode 100644 index 000000000..9e5377d5f --- /dev/null +++ b/regress/pesterTests/testdata/sshtest_userPermTestkey_ed25519.pub @@ -0,0 +1 @@ +ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIA3W11EOIvdkBkw9Pdzf/caD9dtY8rczSABk+EgUeK8e newlogin@YANBINGW2K12R2@Yanbingw2k12r2 diff --git a/regress/pesterTests/testdata/test_known_hosts b/regress/pesterTests/testdata/test_known_hosts new file mode 100644 index 000000000..7689cdaf2 --- /dev/null +++ b/regress/pesterTests/testdata/test_known_hosts @@ -0,0 +1 @@ +[localhost]:47003 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMtJMxwn+iJU0X4+EC7PSj/cfcMbdP6ahhodtXx+6RHv sshtest_hostkey_ed25519 diff --git a/ssh-keygen.c b/ssh-keygen.c index 6d0f032e6..8fa3535df 100644 --- a/ssh-keygen.c +++ b/ssh-keygen.c @@ -59,6 +59,7 @@ #include "krl.h" #include "digest.h" #include "utf8.h" +#include "sshfileperm.h" #ifdef WITH_OPENSSL # define DEFAULT_KEY_TYPE_NAME "rsa" @@ -1046,7 +1047,16 @@ do_gen_all_hostkeys(struct passwd *pw) /* Windows POSIX adpater does not support fdopen() on open(file)*/ if ((f = fopen(identity_file, "w")) == NULL) { error("fopen %s failed: %s", identity_file, strerror(errno)); - /* TODO - set permissions on file */ + sshkey_free(public); + first = 0; + continue; + } + /* + Set the owner of the private key file to the user represented by pw + and only grant it the full control access + */ + if (set_secure_file_permission(identity_file, pw) !=0) { + error("set_secure_file_permission on %s failed!", identity_file); #else /* !WINDOWS */ fd = open(identity_file, O_WRONLY | O_CREAT | O_TRUNC, 0644); if (fd == -1) { @@ -1497,6 +1507,14 @@ do_change_comment(struct passwd *pw) strlcat(identity_file, ".pub", sizeof(identity_file)); fd = open(identity_file, O_WRONLY | O_CREAT | O_TRUNC, 0644); +#ifdef WINDOWS + /* + Set the owner of the private key file to user represented by pw and only grant + it the full control access + */ + if (set_secure_file_permission(identity_file, pw) != 0) + fatal("set_secure_file_permission on %s failed!", identity_file); +#endif /* WINDOWS*/ if (fd == -1) fatal("Could not save your public key in %s", identity_file); f = fdopen(fd, "w"); @@ -1680,6 +1698,15 @@ do_ca_sign(struct passwd *pw, int argc, char **argv) if ((fd = open(out, O_WRONLY|O_CREAT|O_TRUNC, 0644)) == -1) fatal("Could not open \"%s\" for writing: %s", out, strerror(errno)); +#ifdef WINDOWS + /* + Set the owner of the private key file to user represented by pw and only grant + it the full control access + */ + if (set_secure_file_permission(out, pw) != 0) + fatal("set_secure_file_permission on %s failed!", identity_file); +#endif /* WINDOWS */ + if ((f = fdopen(fd, "w")) == NULL) fatal("%s: fdopen: %s", __func__, strerror(errno)); if ((r = sshkey_write(public, f)) != 0) @@ -2189,6 +2216,14 @@ do_gen_krl(struct passwd *pw, int updating, int argc, char **argv) fatal("Couldn't generate KRL"); if ((fd = open(identity_file, O_WRONLY|O_CREAT|O_TRUNC, 0644)) == -1) fatal("open %s: %s", identity_file, strerror(errno)); +#ifdef WINDOWS + /* + Set the owner of the private key file to user represented by pw and only grant + it the full control access + */ + if (set_secure_file_permission(identity_file, pw) != 0) + fatal("set_secure_file_permission on %s failed!", identity_file); +#endif /* WINDOWS */ if (atomicio(vwrite, fd, (void *)sshbuf_ptr(kbuf), sshbuf_len(kbuf)) != sshbuf_len(kbuf)) fatal("write %s: %s", identity_file, strerror(errno)); @@ -2758,7 +2793,12 @@ passphrase_again: /* Windows POSIX adpater does not support fdopen() on open(file)*/ if ((f = fopen(identity_file, "w")) == NULL) fatal("fopen %s failed: %s", identity_file, strerror(errno)); - /* TODO - set permissions on file */ + /* + Set the owner of the private key file to the user represented by pw and only grant + it the full control access + */ + if (set_secure_file_permission(identity_file, pw) != 0) + error("set_secure_file_permission on %s failed!", identity_file); #else /* !WINDOWS */ if ((fd = open(identity_file, O_WRONLY|O_CREAT|O_TRUNC, 0644)) == -1) fatal("Unable to save public key to %s: %s", diff --git a/sshfileperm.h b/sshfileperm.h new file mode 100644 index 000000000..5c5c3ab27 --- /dev/null +++ b/sshfileperm.h @@ -0,0 +1,30 @@ +/* +* Copyright (c) 2004, 2005 Darren Tucker. All rights reserved. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions +* are met: +* 1. Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* 2. Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in the +* documentation and/or other materials provided with the distribution. +* +* THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR +* IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES +* OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. +* IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, +* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT +* NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF +* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ + +#ifndef _SSH_FILE_PERM_H +#define _SSH_FILE_PERM_H + +int check_secure_file_permission(const char *, struct passwd *); +int set_secure_file_permission(const char *, struct passwd *); +#endif /* _SSH_FILE_PERM_H */ \ No newline at end of file