Added support for AuthorizedKeysCommand and AuthorizedPrincipalsCommand (#409)

auth.c

@@ -527,7 +527,7 @@ auth_openfile(const char *file, struct passwd *pw, int strict_modes,
                         strerror(errno));
                 return NULL;
         }
-	if (strict_modes && check_secure_file_permission(file, pw) != 0) {
+	if (strict_modes && check_secure_file_permission(file, pw, 0) != 0) {
 		fclose(f);
 		logit("Authentication refused.");
 		auth_debug_add("Ignored %s", file_type);
@@ -925,11 +925,20 @@ subprocess(const char *tag, struct passwd *pw, const char *command,
 		restore_uid();
 		return 0;
 	}
+#ifdef WINDOWS
+	if (check_secure_file_permission(av[0], pw, 1) != 0) {
+		error("Permissions on %s:\"%s\" are too open", tag, av[0]);
+		restore_uid();
+		return 0;
+	}
+#else
 	if (safe_path(av[0], &st, NULL, 0, errmsg, sizeof(errmsg)) != 0) {
 		error("Unsafe %s \"%s\": %s", tag, av[0], errmsg);
 		restore_uid();
 		return 0;
 	}
+#endif
+
 	/* Prepare to keep the child's stdout if requested */
 	if (pipe(p) == -1) {
 		error("%s: pipe: %s", tag, strerror(errno));
@@ -938,6 +947,20 @@ subprocess(const char *tag, struct passwd *pw, const char *command,
 	}
 	restore_uid();
 
+#ifdef FORK_NOT_SUPPORTED
+	{
+		posix_spawn_file_actions_t actions;
+		pid = -1;
+
+		if (posix_spawn_file_actions_init(&actions) != 0 ||
+			posix_spawn_file_actions_adddup2(&actions, p[1], STDOUT_FILENO) != 0)
+			fatal("posix_spawn initialization failed");
+		else if (__posix_spawn_asuser((pid_t*)&pid, av[0], &actions, NULL, av, NULL, pw->pw_name) != 0)
+			fatal("posix_spawn: %s", strerror(errno));
+
+		posix_spawn_file_actions_destroy(&actions);
+	}
+#else 
 	switch ((pid = fork())) {
 	case -1: /* error */
 		error("%s: fork: %s", tag, strerror(errno));
@@ -1004,6 +1027,7 @@ subprocess(const char *tag, struct passwd *pw, const char *command,
 	default: /* parent */
 		break;
 	}
+#endif
 
 	close(p[1]);
 	if ((flags & SSH_SUBPROCESS_STDOUT_CAPTURE) == 0)

auth2-pubkey.c

@@ -311,6 +311,12 @@ check_principals_line(struct ssh *ssh, char *cp, const struct sshkey_cert *cert,
 	while (ep > cp && (*ep == '\n' || *ep == ' ' || *ep == '\t'))
 		*ep-- = '\0';
 
+#ifdef SUPPORT_CRLF
+	/* account for \r at line end */
+	if (*ep == '\r')
+		*ep-- = '\0';
+#endif
+
 	/*
 	 * If the line has internal whitespace then assume it has
 	 * key options.

authfile.c

@@ -154,7 +154,7 @@ sshkey_perm_ok(int fd, const char *filename)
 #endif
 		
 #ifdef WINDOWS  /*implement permission checks on Windows*/
-	if(check_secure_file_permission(filename, NULL) != 0) {
+	if(check_secure_file_permission(filename, NULL, 0) != 0) {
 		error("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
 		error("@         WARNING: UNPROTECTED PRIVATE KEY FILE!          @");
 		error("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");

contrib/win32/win32compat/fileio.c

@@ -906,31 +906,64 @@ fileio_lseek(struct w32_io* pio, unsigned __int64 offset, int origin)
 	return 0;
 }
 
-/* 
+/* fdopen() to be used  on pipe handles */
- * fdopen implementation - use with caution
+static FILE*
- * this implementation deviates from POSIX spec the following way
+fileio_fdopen_pipe(struct w32_io* pio, const char *mode)
- * - the underlying file descriptor is closed automatically
+{
- * hence no further POSIX io operations (read, write, close, etc) on the 
+	int fd_flags = 0;
- * underlying file descriptor are supported
+	FILE* ret;
- */
+	debug4("fdopen - io:%p", pio);
-FILE*
+
-fileio_fdopen(struct w32_io* pio, const char *mode)
+	if (mode[1] == '\0') {
+		switch (*mode) {
+		case 'r':
+			fd_flags = _O_RDONLY;
+			break;
+		case 'w':
+			break;
+		case 'a':
+			fd_flags = _O_APPEND;
+			break;
+		default:
+			errno = ENOTSUP;
+			debug3("fdopen - ERROR unsupported mode %s", mode);
+			return NULL;
+		}
+	}
+	else {
+		errno = ENOTSUP;
+		debug3("fdopen - ERROR unsupported mode %s", mode);
+		return NULL;
+	}
+
+	int fd = _open_osfhandle((intptr_t)pio->handle, fd_flags);
+
+	if (fd == -1 || (ret = _fdopen(fd, mode)) == NULL) {
+		errno = EOTHER;
+		debug3("fdopen - ERROR:%d _open_osfhandle()", errno);
+		return NULL;
+	}
+
+	// overwrite underlying win32 handle - its expected to be closed via fclose
+	// and close pio
+	pio->handle = NULL;
+	int w32_close(int);
+	w32_close(pio->table_index);
+	return ret;
+}
+
+/* fdopen() to be used  on file handles */
+static FILE*
+fileio_fdopen_disk(struct w32_io* pio, const char *mode)
 {
 	wchar_t *file_path, *wmode = NULL;
 	FILE* ret = NULL;
-	DWORD type = 0;
 	
 	debug4("fdopen - io:%p", pio);
 
 	if ((wmode = utf8_to_utf16(mode)) == NULL)
 		goto cleanup;
 
-	/* for non-disk files, just return the descriptor */
-	type = GetFileType(pio->handle);
-	if (type != FILE_TYPE_DISK) {
-		return _fdopen(pio->table_index, mode);
-	}
-
 	file_path = get_final_path_by_handle(pio->handle);
 	if (!file_path) 
 		goto cleanup;
@@ -950,6 +983,31 @@ cleanup:
 	return ret;
 }
 
+/*
+ * fdopen implementation - use with caution
+ * this implementation deviates from POSIX spec the following way
+ * - the underlying file descriptor is closed automatically
+ * hence no further POSIX io operations (read, write, close, etc) on the
+ * underlying file descriptor are supported
+ */
+FILE*
+fileio_fdopen(struct w32_io* pio, const char *mode)
+{
+	DWORD type = 0;
+
+	debug4("fdopen - io:%p", pio);
+
+	type = GetFileType(pio->handle);
+	if (type == FILE_TYPE_DISK) {
+		return fileio_fdopen_disk(pio, mode);
+	} else if (type == FILE_TYPE_PIPE) {
+		return fileio_fdopen_pipe(pio, mode);
+	} else {
+		errno = ENOTSUP;
+		return NULL;
+	}
+}
+
 void
 fileio_on_select(struct w32_io* pio, BOOL rd)
 {

contrib/win32/win32compat/w32-sshfileperm.c

@@ -45,15 +45,14 @@
 * 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 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
-check_secure_file_permission(const char *input_path, struct passwd * pw)
+check_secure_file_permission(const char *input_path, struct passwd * pw, int read_ok)
 {	
 	PSECURITY_DESCRIPTOR pSD = NULL;
 	wchar_t * path_utf16 = NULL;
-	PSID owner_sid = NULL, user_sid = NULL;
+	PSID owner_sid = NULL, user_sid = NULL, ti_sid = NULL;
 	PACL dacl = NULL;
 	DWORD error_code = ERROR_SUCCESS; 
 	BOOL is_valid_sid = FALSE, is_valid_acl = FALSE;
@@ -68,6 +67,8 @@ check_secure_file_permission(const char *input_path, struct passwd * pw)
 		goto cleanup;
 	}
 
+	ti_sid = get_sid("NT SERVICE\\TrustedInstaller");
+
 	/*Get the owner sid of the file.*/
 	if ((error_code = GetNamedSecurityInfoW(path_utf16, SE_FILE_OBJECT,
 		OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION,
@@ -83,8 +84,9 @@ check_secure_file_permission(const char *input_path, struct passwd * pw)
 		goto cleanup;
 	}
 	if (!IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) &&
-		!IsWellKnownSid(owner_sid, WinLocalSystemSid) &&
+	    !IsWellKnownSid(owner_sid, WinLocalSystemSid) &&
-		!EqualSid(owner_sid, user_sid)) {
+	    !EqualSid(owner_sid, user_sid) &&
+	    !(ti_sid && EqualSid(owner_sid, ti_sid))) {
 		debug3("Bad owner on %S", path_utf16);
 		ret = -1;
 		goto cleanup;
@@ -92,7 +94,6 @@ check_secure_file_permission(const char *input_path, 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 current user account have write permission on the file
-		2. sshd account can only have read permission
 	*/
 	for (DWORD i = 0; i < dacl->AceCount; i++) {
 		PVOID current_ace = NULL;
@@ -118,8 +119,12 @@ check_secure_file_permission(const char *input_path, struct passwd * pw)
 		
 		/*no need to check administrators group, pwd user account, and system account*/
 		if (IsWellKnownSid(current_trustee_sid, WinBuiltinAdministratorsSid) ||
-			IsWellKnownSid(current_trustee_sid, WinLocalSystemSid) ||
+		    IsWellKnownSid(current_trustee_sid, WinLocalSystemSid) ||
-			EqualSid(current_trustee_sid, user_sid)) {
+		    EqualSid(current_trustee_sid, user_sid) ||
+		    (ti_sid && EqualSid(current_trustee_sid, ti_sid))) {
+			continue;
+		} else if (read_ok && (current_access_mask & (FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_APPEND_DATA)) == 0 ) {
+			/* if read is allowed, allow ACES that do not give write access*/
 			continue;
 		} else {
 
@@ -159,6 +164,8 @@ cleanup:
 		LocalFree(pSD);
 	if (user_sid)
 		free(user_sid);
+	if (ti_sid)
+		free(ti_sid);
 	if(path_utf16)
 		free(path_utf16);
 	return ret;

readconf.c

@@ -1810,7 +1810,7 @@ read_config_file_depth(const char *filename, struct passwd *pw,
 		implementation on windows to make sure the config file is owned by a user, administrators group, or LOCALSYSTEM account
 		and nobody else except Administrators group, LOCALSYSTEM, and file owner account has the write permission
 		*/
-		if (check_secure_file_permission(filename, pw) != 0)
+		if (check_secure_file_permission(filename, pw, 1) != 0)
 			fatal("Bad owner or permissions on %s", filename);
 #else
 		struct stat sb;

regress/pesterTests/AuthorizedKeysCommand.Tests.ps1

@@ -0,0 +1,57 @@
+If ($PSVersiontable.PSVersion.Major -le 2) {$PSScriptRoot = Split-Path -Parent $MyInvocation.MyCommand.Path}
+Import-Module $PSScriptRoot\CommonUtils.psm1 -Force
+$tC = 1
+$tI = 0
+$suite = "authorizedKeysCommand"
+        
+Describe "E2E scenarios for AuthorizedKeysCommand" -Tags "CI" {
+    BeforeAll {        
+        if($OpenSSHTestInfo -eq $null)
+        {
+            Throw "`$OpenSSHTestInfo is null. Please run Set-OpenSSHTestEnvironment to set test environments."
+        }
+
+        $server = $OpenSSHTestInfo["Target"]
+        $port = 47004
+        $opensshbinpath = $OpenSSHTestInfo['OpenSSHBinPath']
+        $ssouser = $OpenSSHTestInfo["SSOUser"]
+        $sshdconfig = Join-Path $Global:OpenSSHTestInfo["ServiceConfigDir"] sshd_config
+
+        $testDir = Join-Path $OpenSSHTestInfo["TestDataPath"] $suite
+        if(-not (Test-Path $testDir))
+        {
+            $null = New-Item $testDir -ItemType directory -Force -ErrorAction SilentlyContinue
+        }
+    }
+
+    BeforeEach {
+        $stderrFile=Join-Path $testDir "$tC.$tI.stderr.txt"
+        $stdoutFile=Join-Path $testDir "$tC.$tI.stdout.txt"
+        $logFile = Join-Path $testDir "$tC.$tI.log.txt"
+    }        
+
+    AfterEach {$tI++;}
+
+    Context "$tC - basic test cases" {
+        
+        BeforeAll {$tI=1}
+        AfterAll{$tC++}
+
+        It "$tC.$tI - keys command with %k argument" {
+            #override authorizedkeysfile location to an unknown location, so AuthorizedKeysCommand gets executed
+            $kcOutFile = Join-Path $testDir "$tC.$tI.kcout.txt"
+            Remove-Item -Force $kcOutFile -ErrorAction SilentlyContinue
+            $sshdArgs = "-d -f $sshdconfig  -E $logFile -o `"AuthorizedKeysFile .fake/authorized_keys`""
+            $sshdArgs += " -o `"AuthorizedKeysCommand=$env:windir\system32\cmd.exe /c echo ssh-ed25519 %k & whoami > $kcOutFile`""
+            $sshdArgs += " -o `"AuthorizedKeysCommandUser=$ssouser`""
+            $sshdArgs += " -o PasswordAuthentication=no"
+            Start-SSHDTestDaemon -WorkDir $opensshbinpath -Arguments $sshdArgs -Port $port
+            $o = ssh -p $port test_target echo 1234
+            Stop-SSHDTestDaemon -Port $port
+            $o | Should Be "1234"
+            #check the command is run as AuthorizedKeysCommandUser
+            (gc $kcOutFile).Contains($ssouser) | Should Be $true
+        }
+
+    }
+}

regress/pesterTests/CertAuth.Tests.ps1

@@ -15,6 +15,9 @@ Describe "E2E scenarios for certificate authentication" -Tags "CI" {
         $port = $OpenSSHTestInfo["Port"]
         $pkuser = $OpenSSHTestInfo["PubKeyUser"]
         $cakey = $OpenSSHTestInfo["CA_Private_Key"]
+        $opensshbinpath = $OpenSSHTestInfo['OpenSSHBinPath']
+        $ssouser = $OpenSSHTestInfo["SSOUser"]
+        $sshdconfig = Join-Path $Global:OpenSSHTestInfo["ServiceConfigDir"] sshd_config
         
         $testDir = Join-Path $OpenSSHTestInfo["TestDataPath"] $suite
         if(-not (Test-Path $testDir))
@@ -61,6 +64,28 @@ Describe "E2E scenarios for certificate authentication" -Tags "CI" {
             $o | Should Be "1234"
             Remove-PasswordSetting            
         }
+
+        It "$tC.$tI - authenticate using certificate via AuthorizedPrincipalsCommand" {
+            $pcOutFile = Join-Path $testDir "$tC.$tI.pcout.txt"
+            $logFile = Join-Path $testDir "$tC.$tI.log.txt"
+            Remove-Item -Force $pcOutFile -ErrorAction SilentlyContinue
+            $sshdArgs = "-d -f $sshdconfig  -E $logFile -o `"AuthorizedKeysFile .fake/authorized_keys`""
+            $sshdArgs += " -o `"AuthorizedPrincipalsCommand=$env:windir\system32\cmd.exe /c echo otheruser& echo $pkuser& whoami > $pcOutFile`""
+            $sshdArgs += " -o `"AuthorizedPrincipalsCommandUser=$ssouser`""
+            $sshdArgs += " -o PasswordAuthentication=no"
+
+            Start-SSHDTestDaemon -WorkDir $opensshbinpath -Arguments $sshdArgs -Port 47004
+            
+            #set up SSH_ASKPASS for key passphrase
+            Add-PasswordSetting -Pass $keypassphrase
+            $o = ssh -i $user_key -p 47004 $pkuser@$server echo 2345
+            Remove-PasswordSetting   
+            
+            Stop-SSHDTestDaemon -Port 47004
+            $o | Should Be "2345"
+            #check the command is run as AuthorizedPrincipalsCommandUser
+            (gc $pcOutFile).Contains($ssouser) | Should Be $true          
+        }
     }
 
 }

regress/pesterTests/Cfginclude.Tests.ps1

@@ -127,7 +127,7 @@ Describe "Tests for ssh config" -Tags "CI" {
 
         It "$tC.$tI-User SSHConfig-ReadConfig negative (others has permission)" {
             #setup
-            Repair-FilePermission -Filepath $userConfigFile -Owners $currentUserSid -FullAccessNeeded $adminsSid,$systemSid,$currentUserSid -ReadAccessNeeded $objUserSid -confirm:$false
+            Repair-FilePermission -Filepath $userConfigFile -Owners $currentUserSid -FullAccessNeeded $adminsSid,$systemSid,$currentUserSid,$objUserSid -confirm:$false
 
             #Run
             cmd /c "ssh test_target echo 1234 2> $logPath"

regress/pesterTests/data/known_hosts

@@ -5,5 +5,6 @@
 [localhost]:47002 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMtJMxwn+iJU0X4+EC7PSj/cfcMbdP6ahhodtXx+6RHv sshtest_hostkey_ed25519
 [localhost]:47002 ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDU+NcQ5NuRutQJoZVjDmP/vE6IYZOaE59FTUjaoZkuPl4prdOPgqAnCwSy9XtnfzPm/oe62SyYIHgj8wRzhqjMU8g8aGqfv9ryF+hpNXZrFYXIdkdxnubzfb4e70RRRoTH8P5vuY8sAn0FIRlV/3EDkSKBFy2W3InMTO6l8gbkzzkgbn1GLvH06QJVdb2PcHksSn7dJBVHWASYi3TJWWu4muI+ZNfothujxAHqjKTJuJ9apDZIc0tnkPmlifRmolSUS4OAH2KWZ+5Gwaj7gsB8bk4QuA+QCT60OCcuzCcy4FBuXvvXkM9MBe/P2KZjVLAn86SriRtoE4RI+9R9S7DV sshtest_hostkey_rsa
 [localhost]:47003 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMtJMxwn+iJU0X4+EC7PSj/cfcMbdP6ahhodtXx+6RHv sshtest_hostkey_ed25519
+[localhost]:47004 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMtJMxwn+iJU0X4+EC7PSj/cfcMbdP6ahhodtXx+6RHv sshtest_hostkey_ed25519
 ###OpenSSHE2ETests
 

servconf.c

@@ -2044,7 +2044,7 @@ process_server_config_line(ServerOptions *options, char *line,
 			    linenum);
 		len = strspn(cp, WHITESPACE);
 		if (*activep && options->authorized_keys_command == NULL) {
-			if (cp[len] != '/' && strcasecmp(cp + len, "none") != 0)
+			if (!path_absolute(cp + len) && strcasecmp(cp + len, "none") != 0)
 				fatal("%.200s line %d: AuthorizedKeysCommand "
 				    "must be an absolute path",
 				    filename, linenum);
@@ -2070,7 +2070,7 @@ process_server_config_line(ServerOptions *options, char *line,
 		len = strspn(cp, WHITESPACE);
 		if (*activep &&
 		    options->authorized_principals_command == NULL) {
-			if (cp[len] != '/' && strcasecmp(cp + len, "none") != 0)
+			if (!path_absolute(cp + len) && strcasecmp(cp + len, "none") != 0)
 				fatal("%.200s line %d: "
 				    "AuthorizedPrincipalsCommand must be "
 				    "an absolute path", filename, linenum);

sshfileperm.h

@@ -25,5 +25,5 @@
 #ifndef _SSH_FILE_PERM_H
 #define _SSH_FILE_PERM_H
 
-int check_secure_file_permission(const char *, struct passwd *);
+int check_secure_file_permission(const char *, struct passwd *, int);
 #endif /* _SSH_FILE_PERM_H */