tyler.durden
/
openssh-portable
mirror of https://github.com/PowerShell/openssh-portable.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix issue when checking file permission and generate key under system context ()
This commit is contained in:
Yanbing 2017-05-26 14:24:23 -07:00 committed by Manoj Ampalam
parent 921aafc728
commit 5989efcad6
7 changed files with 106 additions and 121 deletions

13
contrib/win32/win32compat/fileio.c
View File

@ -359,14 +359,21 @@ createFile_flags_setup(int flags, u_short mode, struct createFile_flags* cf_flag
/*map mode*/
if ((pwd = getpwuid(0)) == NULL)
fatal("getpwuid failed.");
fatal("getpwuid failed.");
if ((sid_utf16 = utf8_to_utf16(pwd->pw_sid)) == NULL) {
debug3("Failed to get utf16 of the sid string");
errno = ENOMEM;
goto cleanup;
}
if ((mode & S_IRWXU) != 0) {
if (ConvertStringSidToSid(pwd->pw_sid, &owner_sid) == FALSE ||
(IsValidSid(owner_sid) == FALSE)) {
debug3("cannot retrieve SID of user %s", pwd->pw_name);
goto cleanup;
}
if (!IsWellKnownSid(owner_sid, WinLocalSystemSid) && ((mode & S_IRWXU) != 0)) {
if (st_mode_to_file_att((mode & S_IRWXU) >> 6, owner_access) != 0) {
debug3("st_mode_to_file_att()");
goto cleanup;
@ -399,6 +406,8 @@ createFile_flags_setup(int flags, u_short mode, struct createFile_flags* cf_flag
ret = 0;
cleanup:
if (owner_sid)
LocalFree(owner_sid);
if (sid_utf16)
free(sid_utf16);
return ret;

52
contrib/win32/win32compat/w32-sshfileperm.c
View File

@ -41,12 +41,11 @@
#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
* The function is to check if current user is secure to access to the file.
* Check the owner of the file is one of these types: Local Administrators groups, system account, current user account
* 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, and owner accounts have write permission on the file
2. sshd account can only have read permission
3. file owner should at least have read permission.
1. no user other than local administrators group, system account, and pwd user have write permission on the file
2. sshd account can only have read permission
* Returns 0 on success and -1 on failure
*/
int
@ -68,11 +67,12 @@ check_secure_file_permission(const char *name, struct passwd * pw)
if (ConvertStringSidToSid(pwd->pw_sid, &user_sid) == FALSE ||
(IsValidSid(user_sid) == FALSE)) {
debug3("failed to retrieve the sid of the pwd");
debug3("failed to retrieve sid of user %s", pwd->pw_name);
ret = -1;
goto cleanup;
}
if ((name_utf16 = utf8_to_utf16(name)) == NULL) {
ret = -1;
errno = ENOMEM;
goto cleanup;
}
@ -100,9 +100,8 @@ check_secure_file_permission(const char *name, struct passwd * pw)
}
/*
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 owner account have write permission on the file
1. no others than administrators group, system account, and current user account have write permission on the file
2. sshd account can only have read permission
3. file owner should at least have read permission
*/
for (DWORD i = 0; i < dacl->AceCount; i++) {
PVOID current_ace = NULL;
@ -118,36 +117,18 @@ check_secure_file_permission(const char *name, struct passwd * pw)
}
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;
}
}
/* only interested in Allow ACE */
if(current_aceHeader->AceType != ACCESS_ALLOWED_ACE_TYPE)
continue;
}
default: {
// Not interested ACE
continue;
}
}
/*no need to check administrators group, owner account, and system account*/
PACCESS_ALLOWED_ACE pAllowedAce = (PACCESS_ALLOWED_ACE)current_ace;
current_trustee_sid = &(pAllowedAce->SidStart);
current_access_mask = pAllowedAce->Mask;
/*no need to check administrators group, pwd 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)) {
continue;
}
else if(is_sshd_account(current_trustee_sid)){
@ -173,12 +154,13 @@ cleanup:
if (pSD)
LocalFree(pSD);
if (user_sid)
FreeSid(user_sid);
LocalFree(user_sid);
if(name_utf16)
free(name_utf16);
return ret;
}
/*TODO: optimize to get sshd sid first and then call EqualSid*/
static BOOL
is_sshd_account(PSID user_sid) {
wchar_t user_name[UNCLEN], full_name[UNCLEN + DNLEN + 2];

41
regress/pesterTests/Authorized_keys_fileperm.Tests.ps1
View File

@ -68,7 +68,7 @@ Describe "Tests for authorized_keys file permission" -Tags "CI" {
$logPath = Join-Path $testDir "$tC.$tI.$logName"
}
It "$tC.$tI-authorized_keys-positive(Secured file and running process can access to the file)" {
It "$tC.$tI-authorized_keys-positive(pwd user is the owner and running process can access to the file)" {
#setup to have ssouser as owner and grant ssouser read and write, admins group, and local system full control
Adjust-UserKeyFileACL -Filepath $authorizedkeyPath -Owner $objUser -OwnerPerms "Read, Write"
@ -81,10 +81,11 @@ Describe "Tests for authorized_keys file permission" -Tags "CI" {
Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } }
}
It "$tC.$tI-authorized_keys-positive(authorized_keys is owned by local system and running process can access to the file)" {
It "$tC.$tI-authorized_keys-positive(authorized_keys is owned by local system)" {
#setup to have system as owner and grant it full control
Set-FileOwnerAndACL -Filepath $authorizedkeyPath -Owner $systemAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath $authorizedkeyPath -User $adminAccount -Perms "FullControl"
Add-PermissionToFileACL -FilePath $authorizedkeyPath -User $objUser -Perms "Read, Write"
#Run
Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-o `"AuthorizedKeysFile .testssh/authorized_keys`"", "-E $logPath") -NoNewWindow
@ -95,11 +96,27 @@ Describe "Tests for authorized_keys file permission" -Tags "CI" {
Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } }
}
It "$tC.$tI-authorized_keys-positive(authorized_keys is owned by admins group and running process can access to the file)" {
It "$tC.$tI-authorized_keys-positive(authorized_keys is owned by admins group and pwd does not have explict ACE)" {
#setup to have admin group as owner and grant it full control
Set-FileOwnerAndACL -Filepath $authorizedkeyPath -Owner $adminAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath $authorizedkeyPath -User $systemAccount -Perms "FullControl"
#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 "$tC.$tI-authorized_keys-positive(authorized_keys is owned by admins group and pwd have explict ACE)" {
#setup to have admin group as owner and grant it full control
Set-FileOwnerAndACL -Filepath $authorizedkeyPath -Owner $adminAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath $authorizedkeyPath -User $systemAccount -Perms "FullControl"
Add-PermissionToFileACL -FilePath $authorizedkeyPath -User $objUser -Perms "Read, Write"
#Run
Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-o `"AuthorizedKeysFile .testssh/authorized_keys`"", "-E $logPath") -NoNewWindow
@ -178,24 +195,6 @@ Describe "Tests for authorized_keys file permission" -Tags "CI" {
$matches = Get-Content $filePath | Select-String -pattern "^Permission denied"
$matches.Count | Should BeGreaterThan 2
#Cleanup
Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } }
}
It "$tC.$tI-authorized_keys-negative(the owner of authorized_keys file is denied to access to it)" {
Set-FileOwnerAndACL -Filepath $authorizedkeyPath -Owner $objUser -OwnerPerms "Read","Write"
Add-PermissionToFileACL -FilePath $authorizedkeyPath -User $systemAccount -Perms "FullControl"
Add-PermissionToFileACL -FilePath $authorizedkeyPath -User $adminAccount -Perms "FullControl"
#add rule to denied the owner
Add-PermissionToFileACL -FilePath $authorizedkeyPath -User $objUser -Perm "Read" -AccessType Deny
#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 BeGreaterThan 2
#Cleanup
Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } }
}

24
regress/pesterTests/Cfginclude.Tests.ps1
View File

@ -96,7 +96,7 @@ Describe "Tests for ssh config" -Tags "CI" {
$o | Should Be "1234"
}
It "$tC.$tI-User SSHConfig-ReadConfig positive (admin is the owner)" {
It "$tC.$tI-User SSHConfig-ReadConfig positive (admin is the owner and current user has no explict ACE)" {
#setup
Set-FileOwnerAndACL -Filepath $userConfigFile -Owner $adminAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath $userConfigFile -User $systemAccount -Perms "FullControl"
@ -106,6 +106,17 @@ Describe "Tests for ssh config" -Tags "CI" {
$o | Should Be "1234"
}
It "$tC.$tI-User SSHConfig-ReadConfig positive (admin is the owner and current user has explict ACE)" {
#setup
Set-FileOwnerAndACL -Filepath $userConfigFile -Owner $adminAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath $userConfigFile -User $systemAccount -Perms "FullControl"
Add-PermissionToFileACL -FilePath $userConfigFile -User $currentUser -Perms "Read, Write"
#Run
$o = ssh test_target echo 1234
$o | Should Be "1234"
}
It "$tC.$tI-User SSHConfig-ReadConfig negative (wrong owner)" {
#setup
Set-FileOwnerAndACL -Filepath $userConfigFile -Owner $ssouser -OwnerPerms "Read","Write"
@ -125,17 +136,6 @@ Describe "Tests for ssh config" -Tags "CI" {
Add-PermissionToFileACL -FilePath $userConfigFile -User $adminAccount -Perms "FullControl"
Add-PermissionToFileACL -FilePath $userConfigFile -User $objUser -Perms "Read"
#Run
cmd /c "ssh test_target echo 1234 2> $logPath"
$LASTEXITCODE | Should Not Be 0
Get-Content $logPath | Should Match "^Bad owner or permissions on [a-fA-F]:[/\\]{1,}Users[/\\]{1,}\w+[/\\]{1,}.ssh[/\\]{1,}config$"
}
It "$tC.$tI-User SSHConfig-ReadConfig negative (owner is denied Read permission)" {
#setup
Set-FileOwnerAndACL -Filepath $userConfigFile -Owner $systemAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath $userConfigFile -User $adminAccount -Perms "FullControl"
Add-PermissionToFileACL -FilePath $userConfigFile -User $systemAccount -Perms "Read" -AccessType Deny
#Run
cmd /c "ssh test_target echo 1234 2> $logPath"
$LASTEXITCODE | Should Not Be 0

39
regress/pesterTests/Hostkey_fileperm.Tests.ps1
View File

@ -69,8 +69,24 @@ Describe "Tests for host keys file permission" -Tags "CI" {
#Run
Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-h $hostKeyFilePath", "-E $logPath") -NoNewWindow
Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } }
Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } }
#validate file content does not contain unprotected info.
$logPath | Should Not Contain "UNPROTECTED PRIVATE KEY FILE!"
}
It "$tC.$tI-Host keys-positive (both public and private keys are owned by admin groups and pwd user has explicit ACE)" {
Set-FileOwnerAndACL -Filepath $hostKeyFilePath -Owner $adminAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath $hostKeyFilePath -User $systemAccount -Perms "FullControl"
Add-PermissionToFileACL -FilePath "$hostKeyFilePath.pub" -User $currentUser -Perms "Read"
Set-FileOwnerAndACL -Filepath "$hostKeyFilePath.pub" -Owner $adminAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath "$hostKeyFilePath.pub" -User $systemAccount -Perms "FullControl"
Add-PermissionToFileACL -FilePath "$hostKeyFilePath.pub" -User $everyOne -Perms "Read"
#Run
Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-h $hostKeyFilePath", "-E $logPath") -NoNewWindow
Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 2 } }
#validate file content does not contain unprotected info.
$logPath | Should Not Contain "UNPROTECTED PRIVATE KEY FILE!"
@ -126,6 +142,7 @@ Describe "Tests for host keys file permission" -Tags "CI" {
#validate file content contains unprotected info.
$logPath | Should Contain "key_load_private: bad permissions"
}
It "$tC.$tI-Host keys-negative (the running process does not have read access to public key)" {
#setup to have ssouser as owner and grant it full control
Set-FileOwnerAndACL -FilePath $hostKeyFilePath -Owner $systemAccount -OwnerPerms "FullControl"
@ -140,25 +157,5 @@ Describe "Tests for host keys file permission" -Tags "CI" {
#validate file content contains unprotected info.
$logPath | Should Contain "key_load_public: Permission denied"
}
It "$tC.$tI-Host keys-negative (the owner of private host key is denied Read access to private key)" {
#setup to have ssouser as owner and grant it full control
Set-FileOwnerAndACL -FilePath $hostKeyFilePath -Owner $systemAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath $hostKeyFilePath -User $adminAccount -Perms "FullControl"
Set-FileOwnerAndACL -Filepath "$hostKeyFilePath.pub" -Owner $systemAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath "$hostKeyFilePath.pub" -User $adminAccount -Perms "FullControl"
Add-PermissionToFileACL -FilePath "$hostKeyFilePath.pub" -User $everyOne -Perms "Read"
#add rule to denied the owner
Add-PermissionToFileACL -FilePath $hostKeyFilePath -User $systemAccount -Perms "Read" -AccessType Deny
#Run
Start-Process -FilePath sshd.exe -WorkingDirectory $($OpenSSHTestInfo['OpenSSHBinPath']) -ArgumentList @("-d", "-p $port", "-h $hostKeyFilePath", "-E $logPath") -NoNewWindow
Get-Process -Name sshd | % { if($_.SI -ne 0) { Start-sleep 1; Stop-Process $_; Start-sleep 1 } }
#validate file content does not contain unprotected.
$logPath | Should Contain "key_load_private: bad permissions"
}
}
}

33
regress/pesterTests/KeyUtils.Tests.ps1
View File

@ -234,7 +234,7 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
cmd /c "ssh-add -d $keyFilePath 2> nul "
}
It "$tC.$tI - ssh-add - positive (Secured private key owned by Administrators group)" {
It "$tC.$tI - ssh-add - positive (Secured private key owned by Administrators group and the current user has no explicit ACE)" {
#setup to have local admin group as owner and grant it full control
Set-FileOwnerAndACL -FilePath $keyFilePath -Owner $adminsAccount -OwnerPerms "FullControl"
@ -249,6 +249,22 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
cmd /c "ssh-add -d $keyFilePath 2> nul "
}
It "$tC.$tI - ssh-add - positive (Secured private key owned by Administrators group and the current user has explicit ACE)" {
#setup to have local admin group as owner and grant it full control
Set-FileOwnerAndACL -FilePath $keyFilePath -Owner $adminsAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath $keyFilePath -User $currentUser -Perm "Read, Write"
# for ssh-add to consume SSh_ASKPASS, stdin should not be TTY
cmd /c "ssh-add $keyFilePath < $nullFile 2> nul "
$LASTEXITCODE | Should Be 0
$allkeys = ssh-add -L
$pubkeyraw = ((Get-Content "$keyFilePath.pub").Split(' '))[1]
($allkeys | where { $_.contains($pubkeyraw) }).count | Should Be 1
#clean up
cmd /c "ssh-add -d $keyFilePath 2> nul "
}
It "$tC.$tI - ssh-add - positive (Secured private key owned by local system group)" {
#setup to have local admin group as owner and grant it full control
Set-FileOwnerAndACL -FilePath $keyFilePath -Owner $systemAccount -OwnerPerms "FullControl"
@ -292,21 +308,6 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
$pubkeyraw = ((Get-Content "$keyFilePath.pub").Split(' '))[1]
($allkeys | where { $_.contains($pubkeyraw) }).count | Should Be 0
}
It "$tC.$tI - ssh-add- negative (the owner is denied Read perm on private key)" {
#setup to have local ssytem account as owner and grant it full control
Set-FileOwnerAndACL -FilePath $keyFilePath -owner $systemAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath $keyFilePath -User $adminsAccount -Perm "FullControl"
#deny owner
Add-PermissionToFileACL -FilePath $keyFilePath -User $systemAccount -Perm "Read, Write" -AccessType Deny
cmd /c "ssh-add $keyFilePath < $nullFile 2> nul "
$LASTEXITCODE | Should Not Be 0
$allkeys = ssh-add -L
$pubkeyraw = ((Get-Content "$keyFilePath.pub").Split(' '))[1]
($allkeys | where { $_.contains($pubkeyraw) }).count | Should Be 0
}
}
Context "$tC - ssh-keyscan test cases" {

25
regress/pesterTests/Userkey_fileperm.Tests.ps1
View File

@ -79,7 +79,7 @@ Describe "Tests for user Key file permission" -Tags "CI" {
$o | Should Be "1234"
}
It "$tC.$tI-ssh with private key file -- positive(Secured private key owned by Administrators group)" {
It "$tC.$tI-ssh with private key file -- positive(Secured private key owned by Administrators group and current user has no explicit ACE)" {
#setup to have local admin group as owner and grant it full control
Set-FileOwnerAndACL -FilePath $keyFilePath -Owner $adminsAccount -OwnerPerms "FullControl"
@ -88,6 +88,16 @@ Describe "Tests for user Key file permission" -Tags "CI" {
$o | Should Be "1234"
}
It "$tC.$tI-ssh with private key file -- positive(Secured private key owned by Administrators group and current user has explicit ACE)" {
#setup to have local admin group as owner and grant it full control
Set-FileOwnerAndACL -FilePath $keyFilePath -Owner $adminsAccount -OwnerPerms "FullControl"
Add-PermissionToFileACL -FilePath $keyFilePath -User $currentUser -Perm "Read"
#Run
$o = ssh -p $port -i $keyFilePath $pubKeyUser@$server echo 1234
$o | Should Be "1234"
}
It "$tC.$tI-ssh with private key file -- positive (Secured private key owned by local system)" {
#setup to have local system as owner and grant it full control
Set-FileOwnerAndACL -FilePath $keyFilePath -Owner $systemAccount -OwnerPerms "FullControl"
@ -122,18 +132,5 @@ Describe "Tests for user Key file permission" -Tags "CI" {
$logPath | Should Contain "UNPROTECTED PRIVATE KEY FILE!"
}
It "$tC.$tI-ssh with private key file -- negative(the owner is denied read perm)" {
#setup to have local system as owner and grant it full control
Set-FileOwnerAndACL -FilePath $keyFilePath -Owner $systemAccount -OwnerPerms "Read, Write"
Add-PermissionToFileACL -FilePath $keyFilePath -User $adminsAccount -Perm "FullControl"
#deny local system read access
Add-PermissionToFileACL -FilePath $keyFilePath -User $systemAccount -Perm "Read,write" -AccessType Deny
$o = ssh -p $port -i $keyFilePath -E $logPath $pubKeyUser@$server echo 1234
$LASTEXITCODE | Should Not Be 0
$logPath | Should Contain "UNPROTECTED PRIVATE KEY FILE!"
}
}
}