From a7ea6ec3b638be6c39fc1ee08de12800aa88294a Mon Sep 17 00:00:00 2001 From: bingbing8 Date: Tue, 12 Sep 2017 19:36:49 -0700 Subject: [PATCH 1/7] 1. fix some exception when appverifier is enabled on win7 (https://github.com/PowerShell/Win32-OpenSSH/issues/872) 2. enable sshdconfig tests on win7(https://github.com/PowerShell/Win32-OpenSSH/issues/873) 3. fix for https://github.com/PowerShell/Win32-OpenSSH/issues/874(Readfile does not return on win7 when no content in console ) 4. remove loggin to console in Readthread because write hangs here since write thread already closed (https://github.com/PowerShell/Win32-OpenSSH/issues/879) 5. fix VCTargetsPath --- contrib/win32/openssh/OpenSSHBuildHelper.psm1 | 2 +- contrib/win32/win32compat/termio.c | 12 ++++------ contrib/win32/win32compat/w32fd.c | 23 ++++++++++--------- regress/pesterTests/SSHDConfig.tests.ps1 | 6 ++--- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/contrib/win32/openssh/OpenSSHBuildHelper.psm1 b/contrib/win32/openssh/OpenSSHBuildHelper.psm1 index 2d43390d6f4..0e612e044a9 100644 --- a/contrib/win32/openssh/OpenSSHBuildHelper.psm1 +++ b/contrib/win32/openssh/OpenSSHBuildHelper.psm1 @@ -217,7 +217,7 @@ function Start-OpenSSHBootstrap [Environment]::SetEnvironmentVariable('Path', $newMachineEnvironmentPath, 'MACHINE') } - $VCTargetsPath = "${env:ProgramFiles(x86)}\MSBuild\Microsoft.Cpp\v4.0\V140" + $VCTargetsPath = "${env:ProgramFiles(x86)}\MSBuild\Microsoft.Cpp\v4.0\V140\" if([Environment]::GetEnvironmentVariable('VCTargetsPath', 'MACHINE') -eq $null) { [Environment]::SetEnvironmentVariable('VCTargetsPath', $VCTargetsPath, 'MACHINE') diff --git a/contrib/win32/win32compat/termio.c b/contrib/win32/win32compat/termio.c index 5494befca17..f7213975b51 100644 --- a/contrib/win32/win32compat/termio.c +++ b/contrib/win32/win32compat/termio.c @@ -103,12 +103,11 @@ ReadThread(_In_ LPVOID lpParameter) if (!SetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), dwAttributes)) error("SetConsoleMode on STD_INPUT_HANDLE failed with %d\n", GetLastError()); - } + } if (!ReadFile(WINHANDLE(pio), pio->read_details.buf, - pio->read_details.buf_size, &read_status.transferred, NULL)) { - read_status.error = GetLastError(); - debug("ReadThread - ReadFile failed %d, io:%p", GetLastError(), pio); + pio->read_details.buf_size, &read_status.transferred, NULL)) { + read_status.error = GetLastError(); return -1; } @@ -127,8 +126,7 @@ ReadThread(_In_ LPVOID lpParameter) } else { if (!ReadFile(WINHANDLE(pio), pio->read_details.buf, pio->read_details.buf_size, &read_status.transferred, NULL)) { - read_status.error = GetLastError(); - debug("ReadThread - ReadFile failed %d, io:%p", GetLastError(), pio); + read_status.error = GetLastError(); return -1; } } @@ -257,7 +255,7 @@ syncio_close(struct w32_io* pio) /* If io is pending, let worker threads exit. */ if (pio->read_details.pending) { /* For console - the read thread is blocked so terminate it. */ - if (FILETYPE(pio) == FILE_TYPE_CHAR && in_raw_mode) + if (FILETYPE(pio) == FILE_TYPE_CHAR) TerminateThread(pio->read_overlapped.hEvent, 0); else WaitForSingleObject(pio->read_overlapped.hEvent, INFINITE); diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 3cea3a42c17..5617b2c893a 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -556,17 +556,18 @@ w32_io_process_fd_flags(struct w32_io* pio, int flags) shi_flags = (flags & FD_CLOEXEC) ? 0 : HANDLE_FLAG_INHERIT; - if (SetHandleInformation(WINHANDLE(pio), HANDLE_FLAG_INHERIT, shi_flags) == FALSE) { - /* - * Ignore if handle is not valid yet. It will not be valid for - * UF_UNIX sockets that are not connected yet - */ - if (GetLastError() != ERROR_INVALID_HANDLE) { - debug3("fcntl - SetHandleInformation failed %d, io:%p", - GetLastError(), pio); - errno = EOTHER; - return -1; - } + HANDLE h = WINHANDLE(pio); + if (IS_INVALID_HANDLE(h)) { + /* + * Ignore if handle is not valid yet. It will not be valid for + * UF_UNIX sockets that are not connected yet + */ + } + else if (SetHandleInformation(h, HANDLE_FLAG_INHERIT, shi_flags) == FALSE) { + debug3("fcntl - SetHandleInformation failed %d, io:%p", + GetLastError(), pio); + errno = EOTHER; + return -1; } pio->fd_flags = flags; diff --git a/regress/pesterTests/SSHDConfig.tests.ps1 b/regress/pesterTests/SSHDConfig.tests.ps1 index 858d796da05..eb1d51057c3 100644 --- a/regress/pesterTests/SSHDConfig.tests.ps1 +++ b/regress/pesterTests/SSHDConfig.tests.ps1 @@ -25,7 +25,7 @@ Describe "Tests of sshd_config" -Tags "CI" { Add-Type -AssemblyName System.DirectoryServices.AccountManagement $ContextName = $env:COMPUTERNAME $ContextType = [System.DirectoryServices.AccountManagement.ContextType]::Machine - $PrincipalContext = [System.DirectoryServices.AccountManagement.PrincipalContext]::new($ContextType, $ContextName) + $PrincipalContext = new-object -TypeName System.DirectoryServices.AccountManagement.PrincipalContext -ArgumentList @($ContextType, $ContextName) $IdentityType = [System.DirectoryServices.AccountManagement.IdentityType]::SamAccountName function Add-LocalUser @@ -35,7 +35,7 @@ Describe "Tests of sshd_config" -Tags "CI" { if($user -eq $null) { try { - $user = [System.DirectoryServices.AccountManagement.UserPrincipal]::new($PrincipalContext,$UserName,$Password, $true) + $user = new-object -TypeName System.DirectoryServices.AccountManagement.UserPrincipal -ArgumentList @($PrincipalContext,$UserName,$Password, $true) $user.Save() } finally { @@ -51,7 +51,7 @@ Describe "Tests of sshd_config" -Tags "CI" { if($group -eq $null) { try { - $group = [System.DirectoryServices.AccountManagement.GroupPrincipal]::new($PrincipalContext,$groupName) + $group = new-object -TypeName System.DirectoryServices.AccountManagement.GroupPrincipal -ArgumentList @($PrincipalContext,$groupName) $group.Save() } finally { From 88f7601fd34821bde69cb40805a611cf78b5ec20 Mon Sep 17 00:00:00 2001 From: bingbing8 Date: Wed, 13 Sep 2017 14:35:59 -0700 Subject: [PATCH 2/7] ReadFile only waits infinitely only on win7 or earlier so terminates thread on win7 or earlier os only --- contrib/win32/win32compat/termio.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/contrib/win32/win32compat/termio.c b/contrib/win32/win32compat/termio.c index f7213975b51..9c06f82948b 100644 --- a/contrib/win32/win32compat/termio.c +++ b/contrib/win32/win32compat/termio.c @@ -245,6 +245,19 @@ syncio_initiate_write(struct w32_io* pio, DWORD num_bytes) return 0; } +static BOOL +is_windows8_or_Later() +{ + OSVERSIONINFO osvi; + + ZeroMemory(&osvi, sizeof(OSVERSIONINFO)); + osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); + + GetVersionEx(&osvi); + + return (osvi.dwMajorVersion > 6) || ((osvi.dwMajorVersion == 6) && (osvi.dwMinorVersion >= 2)); +} + /* tty close */ int syncio_close(struct w32_io* pio) @@ -255,7 +268,7 @@ syncio_close(struct w32_io* pio) /* If io is pending, let worker threads exit. */ if (pio->read_details.pending) { /* For console - the read thread is blocked so terminate it. */ - if (FILETYPE(pio) == FILE_TYPE_CHAR) + if (FILETYPE(pio) == FILE_TYPE_CHAR && (!is_windows8_or_Later() || in_raw_mode)) TerminateThread(pio->read_overlapped.hEvent, 0); else WaitForSingleObject(pio->read_overlapped.hEvent, INFINITE); From 053ec65a6c5b793c0078e70da29ed387d96911cd Mon Sep 17 00:00:00 2001 From: bingbing8 Date: Wed, 13 Sep 2017 15:35:11 -0700 Subject: [PATCH 3/7] fix for code review --- contrib/win32/win32compat/termio.c | 18 ++---------------- contrib/win32/win32compat/w32fd.c | 13 ++++++------- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/contrib/win32/win32compat/termio.c b/contrib/win32/win32compat/termio.c index 9c06f82948b..6c0cf5423b9 100644 --- a/contrib/win32/win32compat/termio.c +++ b/contrib/win32/win32compat/termio.c @@ -130,8 +130,7 @@ ReadThread(_In_ LPVOID lpParameter) return -1; } } - if (0 == QueueUserAPC(ReadAPCProc, main_thread, (ULONG_PTR)pio)) { - debug3("TermRead thread - ERROR QueueUserAPC failed %d, io:%p", GetLastError(), pio); + if (0 == QueueUserAPC(ReadAPCProc, main_thread, (ULONG_PTR)pio)) { pio->read_details.pending = FALSE; pio->read_details.error = GetLastError(); DebugBreak(); @@ -245,19 +244,6 @@ syncio_initiate_write(struct w32_io* pio, DWORD num_bytes) return 0; } -static BOOL -is_windows8_or_Later() -{ - OSVERSIONINFO osvi; - - ZeroMemory(&osvi, sizeof(OSVERSIONINFO)); - osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); - - GetVersionEx(&osvi); - - return (osvi.dwMajorVersion > 6) || ((osvi.dwMajorVersion == 6) && (osvi.dwMinorVersion >= 2)); -} - /* tty close */ int syncio_close(struct w32_io* pio) @@ -268,7 +254,7 @@ syncio_close(struct w32_io* pio) /* If io is pending, let worker threads exit. */ if (pio->read_details.pending) { /* For console - the read thread is blocked so terminate it. */ - if (FILETYPE(pio) == FILE_TYPE_CHAR && (!is_windows8_or_Later() || in_raw_mode)) + if (FILETYPE(pio) == FILE_TYPE_CHAR && (!IsWindows8OrGreater() || in_raw_mode)) TerminateThread(pio->read_overlapped.hEvent, 0); else WaitForSingleObject(pio->read_overlapped.hEvent, INFINITE); diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 5617b2c893a..31188b26a29 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -557,13 +557,12 @@ w32_io_process_fd_flags(struct w32_io* pio, int flags) shi_flags = (flags & FD_CLOEXEC) ? 0 : HANDLE_FLAG_INHERIT; HANDLE h = WINHANDLE(pio); - if (IS_INVALID_HANDLE(h)) { - /* - * Ignore if handle is not valid yet. It will not be valid for - * UF_UNIX sockets that are not connected yet - */ - } - else if (SetHandleInformation(h, HANDLE_FLAG_INHERIT, shi_flags) == FALSE) { + + /* + * Ignore if handle is not valid yet. It will not be valid for + * UF_UNIX sockets that are not connected yet + */ + if (!IS_INVALID_HANDLE(h) && (SetHandleInformation(h, HANDLE_FLAG_INHERIT, shi_flags) == FALSE)) { debug3("fcntl - SetHandleInformation failed %d, io:%p", GetLastError(), pio); errno = EOTHER; From d684b4ce75a6dc5d0ad1876e3b7ac73273938afb Mon Sep 17 00:00:00 2001 From: bingbing8 Date: Wed, 13 Sep 2017 15:48:41 -0700 Subject: [PATCH 4/7] minor fix --- contrib/win32/win32compat/w32fd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 31188b26a29..711da2e42a0 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -563,7 +563,7 @@ w32_io_process_fd_flags(struct w32_io* pio, int flags) * UF_UNIX sockets that are not connected yet */ if (!IS_INVALID_HANDLE(h) && (SetHandleInformation(h, HANDLE_FLAG_INHERIT, shi_flags) == FALSE)) { - debug3("fcntl - SetHandleInformation failed %d, io:%p", + debug3("fcntl - SetHandleInformation failed with error:%d, io:%p", GetLastError(), pio); errno = EOTHER; return -1; From 3f5a3c837997033056481cb170a0ae35ffaea2ce Mon Sep 17 00:00:00 2001 From: bingbing8 Date: Thu, 14 Sep 2017 12:00:30 -0700 Subject: [PATCH 5/7] minor update for code review --- contrib/win32/win32compat/misc_internal.h | 3 +++ contrib/win32/win32compat/termio.c | 4 ++-- contrib/win32/win32compat/w32fd.c | 3 +-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/contrib/win32/win32compat/misc_internal.h b/contrib/win32/win32compat/misc_internal.h index a2141ac2174..5305918af9b 100644 --- a/contrib/win32/win32compat/misc_internal.h +++ b/contrib/win32/win32compat/misc_internal.h @@ -12,7 +12,10 @@ } while(0) #define NULL_DEVICE "/dev/null" +#define IsWin7OrLess() (!IsWindows8OrGreater()) + #define IS_INVALID_HANDLE(h) ( ((NULL == h) || (INVALID_HANDLE_VALUE == h)) ? 1 : 0 ) +#define IS_VALID_HANDLE(h) (!IS_INVALID_HANDLE(h)) /* removes first '/' for Windows paths that are unix styled. Ex: /c:/ab.cd */ char * sanitized_path(const char *); diff --git a/contrib/win32/win32compat/termio.c b/contrib/win32/win32compat/termio.c index 6c0cf5423b9..e1238244396 100644 --- a/contrib/win32/win32compat/termio.c +++ b/contrib/win32/win32compat/termio.c @@ -103,7 +103,7 @@ ReadThread(_In_ LPVOID lpParameter) if (!SetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), dwAttributes)) error("SetConsoleMode on STD_INPUT_HANDLE failed with %d\n", GetLastError()); - } + } if (!ReadFile(WINHANDLE(pio), pio->read_details.buf, pio->read_details.buf_size, &read_status.transferred, NULL)) { @@ -254,7 +254,7 @@ syncio_close(struct w32_io* pio) /* If io is pending, let worker threads exit. */ if (pio->read_details.pending) { /* For console - the read thread is blocked so terminate it. */ - if (FILETYPE(pio) == FILE_TYPE_CHAR && (!IsWindows8OrGreater() || in_raw_mode)) + if (FILETYPE(pio) == FILE_TYPE_CHAR && (IsWin7OrLess() || in_raw_mode)) TerminateThread(pio->read_overlapped.hEvent, 0); else WaitForSingleObject(pio->read_overlapped.hEvent, INFINITE); diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 711da2e42a0..8a9ea829d62 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -562,7 +562,7 @@ w32_io_process_fd_flags(struct w32_io* pio, int flags) * Ignore if handle is not valid yet. It will not be valid for * UF_UNIX sockets that are not connected yet */ - if (!IS_INVALID_HANDLE(h) && (SetHandleInformation(h, HANDLE_FLAG_INHERIT, shi_flags) == FALSE)) { + if (IS_VALID_HANDLE(h) && (SetHandleInformation(h, HANDLE_FLAG_INHERIT, shi_flags) == FALSE)) { debug3("fcntl - SetHandleInformation failed with error:%d, io:%p", GetLastError(), pio); errno = EOTHER; @@ -797,7 +797,6 @@ w32_select(int fds, w32_fd_set* readfds, w32_fd_set* writefds, w32_fd_set* excep debug5("select - returning %d", out_ready_fds); return out_ready_fds; - } int From 73825c84f7c70aac34473a6f3ab842de10e01bdf Mon Sep 17 00:00:00 2001 From: bingbing8 Date: Fri, 15 Sep 2017 18:05:54 -0700 Subject: [PATCH 6/7] add comments for the added condition. --- contrib/win32/win32compat/termio.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/contrib/win32/win32compat/termio.c b/contrib/win32/win32compat/termio.c index e1238244396..8fd75d2a14b 100644 --- a/contrib/win32/win32compat/termio.c +++ b/contrib/win32/win32compat/termio.c @@ -253,7 +253,11 @@ syncio_close(struct w32_io* pio) /* If io is pending, let worker threads exit. */ if (pio->read_details.pending) { - /* For console - the read thread is blocked so terminate it. */ + /* + Terminate the read thread at the below situations: + 1. For console - the read thread is blocked by the while loop on raw mode + 2. On Win7 machine, ReadFile dees not return when no content to read in non-interactive mode. + */ if (FILETYPE(pio) == FILE_TYPE_CHAR && (IsWin7OrLess() || in_raw_mode)) TerminateThread(pio->read_overlapped.hEvent, 0); else From 7718896edac97c59636c23b892cd0fba47e9c6ea Mon Sep 17 00:00:00 2001 From: bingbing8 Date: Fri, 15 Sep 2017 18:06:23 -0700 Subject: [PATCH 7/7] minor updates on comments --- contrib/win32/win32compat/termio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/win32/win32compat/termio.c b/contrib/win32/win32compat/termio.c index 8fd75d2a14b..c735fb90043 100644 --- a/contrib/win32/win32compat/termio.c +++ b/contrib/win32/win32compat/termio.c @@ -256,7 +256,7 @@ syncio_close(struct w32_io* pio) /* Terminate the read thread at the below situations: 1. For console - the read thread is blocked by the while loop on raw mode - 2. On Win7 machine, ReadFile dees not return when no content to read in non-interactive mode. + 2. Function ReadFile on Win7 machine dees not return when no content to read in non-interactive mode. */ if (FILETYPE(pio) == FILE_TYPE_CHAR && (IsWin7OrLess() || in_raw_mode)) TerminateThread(pio->read_overlapped.hEvent, 0);