From 4879602b69d5802b7a55085224bb931768c92a0a Mon Sep 17 00:00:00 2001 From: bagajjal Date: Fri, 26 May 2017 15:40:59 -0700 Subject: [PATCH] Docker#666 keyscan#731 posixcompatnewunittests (#152) docker ssh issue PowerShell/Win32-OpenSSH#666 a) fdopen changes to accept the /dev/null device b) fix the select (using same fdset as readfdset, exceptfdset) issue with the unix opensssh code. changed keyscan pester test to refer to localhost (127.0.0.1) instead of GitHub.com PowerShell/Win32-OpenSSH#731 Fix the ASSERT_HANDLE issue.. ASSERT_HANDLE should fail if handle is either NULL or INVALID_HANDLE. Added new testcases for the null device. --- appveyor.yml | 2 +- contrib/win32/win32compat/fileio.c | 4 +--- contrib/win32/win32compat/misc.c | 17 +++++++++++++++-- contrib/win32/win32compat/misc_internal.h | 2 ++ regress/pesterTests/KeyUtils.Tests.ps1 | 21 +++++++++++---------- regress/unittests/win32compat/file_tests.c | 19 ++++++++++++++----- regress/unittests/win32compat/tests.h | 4 ++-- sshconnect.c | 16 ++++++++++------ 8 files changed, 56 insertions(+), 29 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 075f61ce7..0c84dbf96 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -38,4 +38,4 @@ on_finish: - ps: | Import-Module $env:APPVEYOR_BUILD_FOLDER\contrib\win32\openssh\AppveyorHelper.psm1 -DisableNameChecking Publish-Artifact - + \ No newline at end of file diff --git a/contrib/win32/win32compat/fileio.c b/contrib/win32/win32compat/fileio.c index 3e2073cf0..248910236 100644 --- a/contrib/win32/win32compat/fileio.c +++ b/contrib/win32/win32compat/fileio.c @@ -413,8 +413,6 @@ cleanup: return ret; } - -#define NULL_DEVICE "/dev/null" /* open() implementation. Uses CreateFile to open file, console, device, etc */ struct w32_io* fileio_open(const char *path_utf8, int flags, u_short mode) @@ -433,7 +431,7 @@ fileio_open(const char *path_utf8, int flags, u_short mode) } /* if opening null device, point to Windows equivalent */ - if (strncmp(path_utf8, NULL_DEVICE, strlen(NULL_DEVICE)) == 0) + if (strncmp(path_utf8, NULL_DEVICE, strlen(NULL_DEVICE)+1) == 0) path_utf8 = "NUL"; if ((path_utf16 = utf8_to_utf16(path_utf8)) == NULL) { diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 871996f07..1836fcdb4 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -240,14 +240,27 @@ w32_fopen_utf8(const char *path, const char *mode) FILE* f; char utf8_bom[] = { 0xEF,0xBB,0xBF }; char first3_bytes[3]; + int status = 1; if (mode[1] != '\0') { errno = ENOTSUP; return NULL; } - if (MultiByteToWideChar(CP_UTF8, 0, path, -1, wpath, PATH_MAX) == 0 || - MultiByteToWideChar(CP_UTF8, 0, mode, -1, wmode, 5) == 0) { + if(NULL == path) { + errno = EINVAL; + debug3("fopen - ERROR:%d", errno); + return NULL; + } + + /* if opening null device, point to Windows equivalent */ + if (0 == strncmp(path, NULL_DEVICE, strlen(NULL_DEVICE)+1)) + wcsncpy_s(wpath, PATH_MAX, L"NUL", 3); + else + status = MultiByteToWideChar(CP_UTF8, 0, path, -1, wpath, PATH_MAX); + + if ((0 == status) || + (0 == MultiByteToWideChar(CP_UTF8, 0, mode, -1, wmode, 5))) { errno = EFAULT; debug3("WideCharToMultiByte failed for %c - ERROR:%d", path, GetLastError()); return NULL; diff --git a/contrib/win32/win32compat/misc_internal.h b/contrib/win32/win32compat/misc_internal.h index 6a6f92a30..0d7903be9 100644 --- a/contrib/win32/win32compat/misc_internal.h +++ b/contrib/win32/win32compat/misc_internal.h @@ -4,6 +4,8 @@ #define SSH_ASYNC_STDOUT "SSH_ASYNC_STDOUT" #define SSH_ASYNC_STDERR "SSH_ASYNC_STDERR" +#define NULL_DEVICE "/dev/null" + #define IS_INVALID_HANDLE(h) ( ((NULL == h) || (INVALID_HANDLE_VALUE == h)) ? 1 : 0 ) /* removes first '/' for Windows paths that are unix styled. Ex: /c:/ab.cd */ diff --git a/regress/pesterTests/KeyUtils.Tests.ps1 b/regress/pesterTests/KeyUtils.Tests.ps1 index ae898f9bb..7793dc04b 100644 --- a/regress/pesterTests/KeyUtils.Tests.ps1 +++ b/regress/pesterTests/KeyUtils.Tests.ps1 @@ -313,6 +313,7 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" { Context "$tC - ssh-keyscan test cases" { BeforeAll { $tI=1 + $port = $OpenSSHTestInfo["Port"] Remove-item (join-path $testDir "$tC.$tI.out.txt") -force -ErrorAction Ignore } BeforeEach { @@ -321,25 +322,25 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" { AfterAll{$tC++} It "$tC.$tI - ssh-keyscan with default arguments" { - cmd /c "ssh-keyscan -p 22 github.com 2>&1 > $outputFile" - $outputFile | Should Contain 'github.com ssh-rsa.*' + cmd /c "ssh-keyscan -p $port 127.0.0.1 2>&1 > $outputFile" + $outputFile | Should Contain '.*ssh-rsa.*' } It "$tC.$tI - ssh-keyscan with -p" { - cmd /c "ssh-keyscan -p 22 github.com 2>&1 > $outputFile" - $outputFile | Should Contain 'github.com ssh-rsa.*' + cmd /c "ssh-keyscan -p $port 127.0.0.1 2>&1 > $outputFile" + $outputFile | Should Contain '.*ssh-rsa.*' } It "$tC.$tI - ssh-keyscan with -f" { - Set-Content -Path tmp.txt -Value "github.com" - cmd /c "ssh-keyscan -f tmp.txt 2>&1 > $outputFile" - $outputFile | Should Contain 'github.com ssh-rsa.*' + Set-Content -Path tmp.txt -Value "127.0.0.1" + cmd /c "ssh-keyscan -p $port -f tmp.txt 2>&1 > $outputFile" + $outputFile | Should Contain '.*ssh-rsa.*' } It "$tC.$tI - ssh-keyscan with -f -t" { - Set-Content -Path tmp.txt -Value "github.com" - cmd /c "ssh-keyscan -f tmp.txt -t rsa,dsa 2>&1 > $outputFile" - $outputFile | Should Contain 'github.com ssh-rsa.*' + Set-Content -Path tmp.txt -Value "127.0.0.1" + cmd /c "ssh-keyscan -p $port -f tmp.txt -t rsa,dsa 2>&1 > $outputFile" + $outputFile | Should Contain '.*ssh-rsa.*' } } } diff --git a/regress/unittests/win32compat/file_tests.c b/regress/unittests/win32compat/file_tests.c index ae04883aa..72cdaaff1 100644 --- a/regress/unittests/win32compat/file_tests.c +++ b/regress/unittests/win32compat/file_tests.c @@ -204,6 +204,15 @@ void file_simple_fileio() ASSERT_INT_EQ(retValue, 0); } + { + // test null device + FILE *fp = fopen("/dev/null", "r"); + ASSERT_PTR_NE(fp, NULL); + + f = open("/dev/null", O_RDONLY); + ASSERT_INT_NE(f, -1); + } + TEST_DONE(); } @@ -429,7 +438,7 @@ file_miscellaneous_tests() int f1 = dup(f); ASSERT_INT_EQ(f1, -1); HANDLE h = w32_fd_to_handle(f); - ASSERT_HANDLE(h, retValue); + ASSERT_HANDLE(h); close(f); char *tmp_filename_1 = "tmp_1.txt"; @@ -443,16 +452,16 @@ file_miscellaneous_tests() free(tmp); h = w32_fd_to_handle(STDIN_FILENO); - ASSERT_HANDLE(h, retValue); + ASSERT_HANDLE(h); h = w32_fd_to_handle(STDOUT_FILENO); - ASSERT_HANDLE(h, retValue); + ASSERT_HANDLE(h); h = w32_fd_to_handle(STDERR_FILENO); - ASSERT_HANDLE(h, retValue); + ASSERT_HANDLE(h); retValue = w32_allocate_fd_for_handle(h, FALSE); - ASSERT_HANDLE(h, retValue); + ASSERT_HANDLE(h); TEST_DONE(); } diff --git a/regress/unittests/win32compat/tests.h b/regress/unittests/win32compat/tests.h index 69f956dc0..aa93a22e2 100644 --- a/regress/unittests/win32compat/tests.h +++ b/regress/unittests/win32compat/tests.h @@ -6,8 +6,8 @@ void miscellaneous_tests(); char *dup_str(char *inStr); void delete_dir_recursive(char *full_dir_path); -#define ASSERT_HANDLE(handle,retValue) \ -{ \ +#define ASSERT_HANDLE(handle) \ +{\ ASSERT_PTR_NE(handle, INVALID_HANDLE_VALUE); \ ASSERT_PTR_NE(handle, 0); \ } diff --git a/sshconnect.c b/sshconnect.c index 7774c0f9d..8da712f07 100644 --- a/sshconnect.c +++ b/sshconnect.c @@ -564,10 +564,12 @@ ssh_exchange_identification(int timeout_ms) size_t len; int fdsetsz, remaining, rc; struct timeval t_start, t_remaining; - fd_set *fdset; + fd_set *readfds; + fd_set *exceptfds; fdsetsz = howmany(connection_in + 1, NFDBITS) * sizeof(fd_mask); - fdset = xcalloc(1, fdsetsz); + readfds = xcalloc(1, fdsetsz); + exceptfds = xcalloc(1, fdsetsz); send_client_banner(connection_out, 0); @@ -578,9 +580,10 @@ ssh_exchange_identification(int timeout_ms) if (timeout_ms > 0) { gettimeofday(&t_start, NULL); ms_to_timeval(&t_remaining, remaining); - FD_SET(connection_in, fdset); - rc = select(connection_in + 1, fdset, NULL, - fdset, &t_remaining); + FD_SET(connection_in, readfds); + FD_SET(connection_in, exceptfds); + rc = select(connection_in + 1, readfds, NULL, + exceptfds, &t_remaining); ms_subtract_diff(&t_start, &remaining); if (rc == 0 || remaining <= 0) fatal("Connection timed out during " @@ -620,7 +623,8 @@ ssh_exchange_identification(int timeout_ms) debug("ssh_exchange_identification: %s", buf); } server_version_string = xstrdup(buf); - free(fdset); + free(readfds); + free(exceptfds); /* * Check that the versions match. In future this might accept