Fix sftp scp file permission (#212)

PowerShell/Win32-OpenSSH#884
Convert the mode properly to file permissions.
If mode has "read" permission then we set the file permission to "read & execute"
If mode has "write" permission then we set the file permission to "Write & Modify"
Inherit the file permissions from the parent folder when sftp / scp creates the file on windows.
sftp - put & get.
scp - from local to remote windows machine.

contrib/win32/win32compat/fileio.c

@@ -271,20 +271,11 @@ st_mode_to_file_att(int mode, wchar_t * attributes)
 	case S_IRWXO:
 		swprintf_s(attributes, MAX_ATTRIBUTE_LENGTH, L"FA");
 		break;
-	case S_IXOTH:
-		swprintf_s(attributes, MAX_ATTRIBUTE_LENGTH, L"FX");
-		break;
-	case S_IWOTH:
-		swprintf_s(attributes, MAX_ATTRIBUTE_LENGTH, L"FW");
-		break;
-	case S_IROTH:
-		swprintf_s(attributes, MAX_ATTRIBUTE_LENGTH, L"FR");
-		break;
 	default:
 		if((mode & S_IROTH) != 0)
-			att |= FILE_GENERIC_READ;
+			att |= (FILE_GENERIC_READ | FILE_EXECUTE);
 		if ((mode & S_IWOTH) != 0)
-			att |= FILE_GENERIC_WRITE;
+			att |= (FILE_GENERIC_WRITE | DELETE);
 		if ((mode & S_IXOTH) != 0)
 			att |= FILE_GENERIC_EXECUTE;
 		swprintf_s(attributes, MAX_ATTRIBUTE_LENGTH, L"%#lx", att);
@@ -295,13 +286,13 @@ st_mode_to_file_att(int mode, wchar_t * attributes)
 
 /* maps open() file modes and flags to ones needed by CreateFile */
 static int
-createFile_flags_setup(int flags, u_short mode, struct createFile_flags* cf_flags)
+createFile_flags_setup(int flags, mode_t mode, struct createFile_flags* cf_flags)
 {
 	/* check flags */
 	int rwflags = flags & 0x3, c_s_flags = flags & 0xfffffff0, ret = -1;
 	PSECURITY_DESCRIPTOR pSD = NULL;
 	wchar_t sddl[SDDL_LENGTH + 1] = { 0 }, owner_ace[MAX_ACE_LENGTH + 1] = {0}, everyone_ace[MAX_ACE_LENGTH + 1] = {0};
-	wchar_t owner_access[MAX_ATTRIBUTE_LENGTH + 1] = {0}, everyone_access[MAX_ATTRIBUTE_LENGTH + 1] = {0}, *sid_utf16;
+	wchar_t owner_access[MAX_ATTRIBUTE_LENGTH + 1] = {0}, everyone_access[MAX_ATTRIBUTE_LENGTH + 1] = {0}, *sid_utf16 = NULL;
 	PACL dacl = NULL;
 	struct passwd * pwd;
 	PSID owner_sid = NULL;
@@ -323,13 +314,6 @@ createFile_flags_setup(int flags, u_short mode, struct createFile_flags* cf_flag
 		return -1;
 	}
 
-	/*validate mode*/
-	if (mode & ~(S_IRWXU | S_IRWXG | S_IRWXO)) {
-		debug3("open - ERROR: unsupported mode: %d", mode);
-		errno = ENOTSUP;
-		return -1;
-	}
-
 	cf_flags->dwShareMode = 0;
 
 	switch (rwflags) {
@@ -360,7 +344,15 @@ createFile_flags_setup(int flags, u_short mode, struct createFile_flags* cf_flag
 
 	cf_flags->dwFlagsAndAttributes = FILE_FLAG_OVERLAPPED | FILE_FLAG_BACKUP_SEMANTICS;
 
-	/*map mode*/
+	// If the mode is USHRT_MAX then we will inherit the permissions from the parent folder.
+	if (mode != USHRT_MAX) {
+		/*validate mode*/
+		if (mode & ~(S_IRWXU | S_IRWXG | S_IRWXO)) {
+			debug3("open - ERROR: unsupported mode: %d", mode);
+			errno = ENOTSUP;
+			return -1;
+		}
+
 		if ((pwd = getpwuid(0)) == NULL)
 			fatal("getpwuid failed.");
 
@@ -402,6 +394,7 @@ createFile_flags_setup(int flags, u_short mode, struct createFile_flags* cf_flag
 			debug3("IsValidSecurityDescriptor return FALSE");
 			goto cleanup;
 		}
+	}
 
 	cf_flags->securityAttributes.lpSecurityDescriptor = pSD;
 	cf_flags->securityAttributes.bInheritHandle = TRUE;
@@ -418,7 +411,7 @@ cleanup:
 
 /* open() implementation. Uses CreateFile to open file, console, device, etc */
 struct w32_io*
-fileio_open(const char *path_utf8, int flags, u_short mode)
+fileio_open(const char *path_utf8, int flags, mode_t mode)
 {
 	struct w32_io* pio = NULL;
 	struct createFile_flags cf_flags;

contrib/win32/win32compat/inc/sys/stat.h

@@ -19,6 +19,10 @@
 # define S_ISUID            0x800 
 # define S_ISGID            0x400
 
+#define READ_PERMISSIONS (FILE_READ_DATA | FILE_READ_ATTRIBUTES | FILE_READ_EA)
+#define WRITE_PERMISSIONS (FILE_WRITE_DATA | FILE_APPEND_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA)
+#define EXECUTE_PERMISSIONS (READ_PERMISSIONS | FILE_EXECUTE)
+
 int w32_fstat(int fd, struct w32_stat *buf);
 #define fstat(a,b)	w32_fstat((a), (b))
 
@@ -48,3 +52,5 @@ struct w32_stat {
 
 
 void strmode(mode_t mode, char *p);
+
+int get_others_file_permissions(wchar_t * file_name, int isReadOnlyFile);

contrib/win32/win32compat/misc.c

@@ -34,6 +34,8 @@
 #include <Shlwapi.h>
 #include <conio.h>
 #include <LM.h>
+#include <Sddl.h>
+#include <Aclapi.h>
 
 #include "inc\unistd.h"
 #include "inc\sys\stat.h"
@@ -592,6 +594,7 @@ int
 file_attr_to_st_mode(wchar_t * path, DWORD attributes)
 {
 	int mode = S_IREAD;
+	BOOL isReadOnlyFile = FALSE;
 	if ((attributes & FILE_ATTRIBUTE_DIRECTORY) != 0 || is_root_or_empty(path))
 		mode |= S_IFDIR | _S_IEXEC;
 	else {
@@ -603,10 +606,13 @@ file_attr_to_st_mode(wchar_t * path, DWORD attributes)
 	}
 	if (!(attributes & FILE_ATTRIBUTE_READONLY))
 		mode |= S_IWRITE;
+	else
+		isReadOnlyFile = TRUE;
+
+	// We don't populate the group permissions as its not applicable to windows OS.
+	// propagate owner read/write/execute bits to other fields.	
+	mode |= get_others_file_permissions(path, isReadOnlyFile);
 
-	// propagate owner read/write/execute bits to group/other fields.
-	mode |= (mode & 0700) >> 3;
-	mode |= (mode & 0700) >> 6;
 	return mode;
 }
 
@@ -1304,3 +1310,104 @@ to_lower_case(char *s)
 	for (; *s; s++)
 		*s = tolower((u_char)*s);
 }
+
+static int
+get_final_mode(int allow_mode, int deny_mode)
+{	
+	// If deny permissions are not specified then return allow permissions.
+	if (!deny_mode) return allow_mode;
+
+	// If allow permissions are not specified then return allow permissions (0).
+	if (!allow_mode) return allow_mode;
+	
+	if(deny_mode & S_IROTH)
+		allow_mode = (allow_mode | S_IROTH) ^ S_IROTH;
+
+	if (deny_mode & S_IWOTH)
+		allow_mode = (allow_mode | S_IWOTH) ^ S_IWOTH;
+
+	if (deny_mode & S_IXOTH)
+		allow_mode = (allow_mode | S_IXOTH) ^ S_IXOTH;
+
+	return allow_mode;
+}
+
+int
+get_others_file_permissions(wchar_t * file_name, int isReadOnlyFile)
+{
+	PSECURITY_DESCRIPTOR pSD = NULL;
+	PSID owner_sid = NULL, current_trustee_sid = NULL;
+	PACL dacl = NULL;
+	DWORD error_code = ERROR_SUCCESS;
+	BOOL is_valid_sid = FALSE, is_valid_acl = FALSE;
+	int ret = 0, allow_mode_world = 0, allow_mode_auth_users = 0, deny_mode_world = 0, deny_mode_auth_users = 0;
+	wchar_t *w_sid = NULL;
+
+	/*Get the owner sid of the file.*/
+	if ((error_code = GetNamedSecurityInfoW(file_name, 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: %ls with error code: %d", file_name, error_code);
+		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);
+		goto cleanup;
+	}
+
+	for (DWORD i = 0; i < dacl->AceCount; i++) {
+		PVOID current_ace = NULL;
+		PACE_HEADER current_aceHeader = NULL;
+		ACCESS_MASK current_access_mask = 0;
+		int mode_tmp = 0;
+		if (!GetAce(dacl, i, &current_ace)) {
+			debug3("GetAce() failed");
+			goto cleanup;
+		}
+
+		current_aceHeader = (PACE_HEADER)current_ace;
+		/* only interested in Allow ACE */
+		if (!(current_aceHeader->AceType == ACCESS_ALLOWED_ACE_TYPE || 
+		    current_aceHeader->AceType == ACCESS_DENIED_ACE_TYPE))
+			continue;
+
+		PACCESS_ALLOWED_ACE pAllowedAce = (PACCESS_ALLOWED_ACE)current_ace;
+		current_trustee_sid = &(pAllowedAce->SidStart);
+
+		if (!(IsWellKnownSid(current_trustee_sid, WinWorldSid) || 
+		    IsWellKnownSid(current_trustee_sid, WinAuthenticatedUserSid)))
+			continue;
+
+		current_access_mask = pAllowedAce->Mask;
+		if ((current_access_mask & READ_PERMISSIONS) == READ_PERMISSIONS)
+			mode_tmp |= S_IROTH;
+
+		if (!isReadOnlyFile && ((current_access_mask & WRITE_PERMISSIONS) == WRITE_PERMISSIONS))
+			mode_tmp |= S_IWOTH;
+
+		if ((current_access_mask & EXECUTE_PERMISSIONS) == EXECUTE_PERMISSIONS)
+			mode_tmp |= S_IXOTH;
+
+		if (IsWellKnownSid(current_trustee_sid, WinWorldSid)) {
+			if(current_aceHeader->AceType == ACCESS_ALLOWED_ACE_TYPE)
+				allow_mode_world |= mode_tmp;
+			else
+				deny_mode_world |= mode_tmp;
+		} else if (IsWellKnownSid(current_trustee_sid, WinAuthenticatedUserSid)) {
+			if (current_aceHeader->AceType == ACCESS_ALLOWED_ACE_TYPE)
+				allow_mode_auth_users |= mode_tmp;
+			else
+				deny_mode_auth_users |= mode_tmp;
+		}	
+	}
+	
+	allow_mode_world = get_final_mode(allow_mode_world, deny_mode_world);
+	allow_mode_auth_users = get_final_mode(allow_mode_auth_users, deny_mode_auth_users);
+
+	ret = allow_mode_world ? allow_mode_world : allow_mode_auth_users;
+cleanup:
+	if (pSD)
+		LocalFree(pSD);
+	return ret;
+}

contrib/win32/win32compat/w32fd.c

@@ -416,14 +416,14 @@ w32_open(const char *pathname, int flags, ... /* arg */)
 	int min_index = fd_table_get_min_index();
 	struct w32_io* pio;
 	va_list valist;
-	u_short mode = 0;
+	mode_t mode = 0;
 
 	errno = 0;
 	if (min_index == -1)
 		return -1;
 	if (flags & O_CREAT) {
 		va_start(valist, flags);
-		mode = va_arg(valist, u_short);
+		mode = va_arg(valist, mode_t);
 		va_end(valist);
 	}
 

contrib/win32/win32compat/w32fd.h

@@ -34,6 +34,7 @@
 
 #include <Windows.h>
 #include <stdio.h>
+#include "inc\sys\types.h"
 
 enum w32_io_type {
 	UNKNOWN_FD = 0,
@@ -144,7 +145,7 @@ int fileio_close(struct w32_io* pio);
 int fileio_pipe(struct w32_io* pio[2]);
 struct w32_io* fileio_afunix_socket();
 int fileio_connect(struct w32_io*, char*);
-struct w32_io* fileio_open(const char *pathname, int flags, u_short mode);
+struct w32_io* fileio_open(const char *pathname, int flags, mode_t mode);
 int fileio_read(struct w32_io* pio, void *dst, size_t max);
 int fileio_write(struct w32_io* pio, const void *buf, size_t max);
 int fileio_fstat(struct w32_io* pio, struct _stat64 *buf);

regress/pesterTests/KeyUtils.Tests.ps1

@@ -49,10 +49,14 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
             $myACL.Access | Should Not Be $null
             
             $ReadAccessPerm = ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::Read.value__) -bor `
+                    ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::ReadAndExecute.value__)  -bor `
                     ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::Synchronize.value__)
             $ReadWriteAccessPerm = ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::Read.value__) -bor `
+                    ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::ReadAndExecute.value__)  -bor `
                     ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::Write.value__)  -bor `
+                    ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::Modify.value__)  -bor `
                     ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::Synchronize.value__)
+
             $FullControlPerm = [System.UInt32] [System.Security.AccessControl.FileSystemRights]::FullControl.value__
     
             if($FilePath.EndsWith(".pub")) {

regress/pesterTests/Log_fileperm.Tests.ps1

@@ -42,8 +42,11 @@ Describe "Tests for log file permission" -Tags "CI" {
             $myACL.Access | Should Not Be $null            
 
             $ReadWriteAccessPerm = ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::Read.value__) -bor `
+                    ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::ReadAndExecute.value__)  -bor `
                     ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::Write.value__)  -bor `
+                    ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::Modify.value__)  -bor `
                     ([System.UInt32] [System.Security.AccessControl.FileSystemRights]::Synchronize.value__)
+
             $FullControlPerm = [System.UInt32] [System.Security.AccessControl.FileSystemRights]::FullControl.value__
             
             $myACL.Access.Count | Should Be 3

scp.c

@@ -1227,7 +1227,12 @@ sink(int argc, char **argv)
 		}
 		omode = mode;
 		mode |= S_IWUSR;
-		if ((ofd = open(np, O_WRONLY|O_CREAT, mode)) < 0) {
+#ifdef WINDOWS
+		// In windows, we would like to inherit the parent folder permissions by setting mode to USHRT_MAX.
+		if ((ofd = open(np, O_WRONLY | O_CREAT, USHRT_MAX)) < 0) {
+#else
+		if ((ofd = open(np, O_WRONLY | O_CREAT, mode)) < 0) {
+#endif // WINDOWS
 bad:			run_err("%s: %s", np, strerror(errno));
 			continue;
 		}

sftp-client.c

@@ -1219,8 +1219,13 @@ do_download(struct sftp_conn *conn, const char *remote_path,
 		return(-1);
 	}
 
+#ifdef WINDOWS
+	// In windows, we would like to inherit the parent folder permissions by setting mode to USHRT_MAX.
+	local_fd = open(local_path, O_WRONLY | O_CREAT | (resume_flag ? 0 : O_TRUNC), USHRT_MAX);
+#else
 	local_fd = open(local_path,
 		O_WRONLY | O_CREAT | (resume_flag ? 0 : O_TRUNC), mode | S_IWUSR);
+#endif // WINDOWS
 	if (local_fd == -1) {
 		error("Couldn't open local file \"%s\" for writing: %s",
 		    local_path, strerror(errno));

sftp-server.c

@@ -696,7 +696,12 @@ process_open(u_int32_t id)
 		verbose("Refusing open request in read-only mode");
 		status = SSH2_FX_PERMISSION_DENIED;
 	} else {
+#ifdef WINDOWS
+		// In windows, we would like to inherit the parent folder permissions by setting mode to USHRT_MAX.
+		fd = open(name, flags, USHRT_MAX);
+#else
 		fd = open(name, flags, mode);
+#endif // WINDOWS
 		if (fd < 0) {
 			status = errno_to_portable(errno);
 		} else {