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.

Author: bagajjal

appveyor.yml

@@ -38,4 +38,4 @@ on_finish:
   - ps: |
       Import-Module $env:APPVEYOR_BUILD_FOLDER\contrib\win32\openssh\AppveyorHelper.psm1 -DisableNameChecking
       Publish-Artifact
-      
+      

contrib/win32/win32compat/fileio.c

@@ -413,8 +413,6 @@ createFile_flags_setup(int flags, u_short mode, struct createFile_flags* cf_flag
 	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) {

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;

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 */

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.*'
 		}
 	}
 }

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();
 }

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); \
 }

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