Bagajjal/fix minor issues (#568)

contrib/win32/openssh/keygen.vcxproj

@@ -403,7 +403,6 @@
     <ClCompile Include="$(OpenSSH-Src-Path)contrib\win32\win32compat\wmain_common.c" />
     <ClCompile Include="$(OpenSSH-Src-Path)contrib\win32\win32compat\win32-utf8.c" />
     <ClCompile Include="$(OpenSSH-Src-Path)ssh-sk-client.c" />
-    <ClCompile Include="$(OpenSSH-Src-Path)ssh-pkcs11.c" />
   </ItemGroup>
   <ItemGroup>
     <ResourceCompile Include="version.rc" />

contrib/win32/openssh/keygen.vcxproj.filters

@@ -30,9 +30,6 @@
     <ClCompile Include="$(OpenSSH-Src-Path)ssh-sk-client.c">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="$(OpenSSH-Src-Path)ssh-pkcs11.c">
-      <Filter>Source Files</Filter>
-    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ResourceCompile Include="version.rc">

contrib/win32/openssh/libssh.vcxproj.filters

@@ -48,6 +48,7 @@
     <ClCompile Include="$(OpenSSH-Src-Path)ssh-dss.c" />
     <ClCompile Include="$(OpenSSH-Src-Path)ssh-ecdsa.c" />
     <ClCompile Include="$(OpenSSH-Src-Path)ssh-ed25519.c" />
+    <ClCompile Include="$(OpenSSH-Src-Path)ssh-pkcs11.c" />
     <ClCompile Include="$(OpenSSH-Src-Path)ssh-rsa.c" />
     <ClCompile Include="$(OpenSSH-Src-Path)sshbuf-getput-basic.c" />
     <ClCompile Include="$(OpenSSH-Src-Path)sshbuf-getput-crypto.c" />

contrib/win32/openssh/ssh.vcxproj.filters

@@ -314,9 +314,6 @@
     <ClCompile Include="$(OpenSSH-Src-Path)ssh-sk-client.c">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="$(OpenSSH-Src-Path)ssh-pkcs11.c">
-      <Filter>Source Files</Filter>
-    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ResourceCompile Include="version.rc">

contrib/win32/win32compat/pwd.c

@@ -340,7 +340,7 @@ get_username(const PSID sid)
 
 	struct passwd *p = get_passwd(NULL, sid);
 	if (p && p->pw_name)
-		return strdup(p->pw_name);
+		return _strdup(p->pw_name);
 	else
 		return NULL;
 }

contrib/win32/win32compat/ssh-agent/agent-main.c

@@ -113,6 +113,8 @@ fix_cwd()
 	_wchdir(path);
 }
 
+extern void sanitise_stdfd(void);
+
 int 
 wmain(int argc, wchar_t **argv) 
 {

contrib/win32/win32compat/ssh-agent/agent.c

@@ -28,6 +28,7 @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
+#include "config.h"
 #include "agent.h"
 #include <sddl.h>
 #include <UserEnv.h>
@@ -37,7 +38,7 @@
 #define BUFSIZE 5 * 1024
 
 char* sshagent_con_username;
-int sshagent_client_pid;
+HANDLE sshagent_client_primary_token;
 
 static HANDLE ioc_port = NULL;
 static BOOL debug_mode = FALSE;
@@ -199,6 +200,12 @@ agent_cleanup_connection(struct agent_connection* con)
 		free(sshagent_con_username);
 		sshagent_con_username = NULL;
 	}
+#ifdef ENABLE_PKCS11
+	if (sshagent_client_primary_token)
+		CloseHandle(sshagent_client_primary_token);
+
+	pkcs11_terminate();
+#endif
 }
 
 void 
@@ -278,7 +285,6 @@ get_con_client_info(struct agent_connection* con)
 		error("cannot retrieve client impersonation token");
 		goto done;
 	}
-	sshagent_client_pid = client_pid;
 
 	if (GetTokenInformation(client_primary_token, TokenUser, NULL, 0, &info_len) == TRUE ||
 		(info = (TOKEN_USER*)malloc(info_len)) == NULL ||
@@ -300,6 +306,11 @@ get_con_client_info(struct agent_connection* con)
 		goto done;
 	}
 
+	// Get client primary token
+	if (DuplicateTokenEx(client_primary_token, TOKEN_QUERY | TOKEN_ASSIGN_PRIMARY | TOKEN_DUPLICATE, NULL, SecurityImpersonation, TokenPrimary, &sshagent_client_primary_token) == FALSE) {
+		error_f("Failed to duplicate the primary token. error:%d", GetLastError());
+	}
+
 	// Get username
 	sshagent_con_username= get_username(info->User.Sid);
 	if (sshagent_con_username)

contrib/win32/win32compat/ssh-agent/keyagent-request.c

@@ -605,15 +605,21 @@ int process_add_smartcard_key(struct sshbuf* request, struct sshbuf* response, s
 	// Remove 'drive root' if exists
 	if (canonical_provider[0] == '/')
 		memmove(canonical_provider, canonical_provider + 1, strlen(canonical_provider));
-	if (get_user_root(con, &user_root) != 0 ||
-		is_reg_sub_key_exists(user_root, SSH_PKCS11_PROVIDERS_ROOT, canonical_provider))
-		goto done;
 
 	count = pkcs11_add_provider(canonical_provider, pin, &keys, NULL);
 	if (count <= 0) {
-		debug("failed to add key to store");
+		error_f("failed to add key to store. count:%d", count);
 		goto done;
 	}
+
+	// If HKCU registry already has the provider then remove the provider and associated keys.
+	// This allows customers to add new keys.
+	if (get_user_root(con, &user_root) != 0 ||
+		is_reg_sub_key_exists(user_root, SSH_PKCS11_PROVIDERS_ROOT, canonical_provider)) {
+		remove_matching_subkeys_from_registry(user_root, SSH_KEYS_ROOT, L"comment", canonical_provider);
+		remove_matching_subkeys_from_registry(user_root, SSH_PKCS11_PROVIDERS_ROOT, L"provider", canonical_provider);
+	}
+
 	for (i = 0; i < count; i++) {
 		key = keys[i];
 		if (sa.lpSecurityDescriptor)
@@ -637,7 +643,7 @@ int process_add_smartcard_key(struct sshbuf* request, struct sshbuf* response, s
 			RegSetValueExW(sub, L"pub", 0, REG_BINARY, pubkey_blob, (DWORD)pubkey_blob_len) != 0 ||
 			RegSetValueExW(sub, L"type", 0, REG_DWORD, (BYTE*)&key->type, 4) != 0 ||
 			RegSetValueExW(sub, L"comment", 0, REG_BINARY, canonical_provider, (DWORD)strlen(canonical_provider)) != 0) {
-			error("failed to add key to store");
+			error_f("failed to add key to store");
 			goto done;
 		}
 	}

ssh-pkcs11-client.c

@@ -53,7 +53,8 @@
 #include "pkcs11.h"
 
 static char module_path[PATH_MAX + 1];
-extern int sshagent_client_pid;
+extern char* sshagent_con_username;
+extern HANDLE sshagent_client_primary_token;
 
 struct pkcs11_provider {
 	char			*name;
@@ -172,15 +173,6 @@ find_helper(void)
 static int fd = -1;
 static pid_t pid = -1;
 
-#ifdef WINDOWS
-static void
-pkcs11_terminate_helper() {
-	HANDLE helper = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
-	TerminateProcess(helper, 1);
-	CloseHandle(helper);
-}
-#endif /* WINDOWS */
-
 static void
 send_msg(struct sshbuf *m)
 {
@@ -251,10 +243,20 @@ pkcs11_terminate(void)
 		pkcs11_del_provider(p->name);
 		TAILQ_REMOVE(&pkcs11_providers, p, next);
 	}
-	pkcs11_terminate_helper();
+
+	if (pid != -1) {
+		kill(pid, SIGTERM);
+		waitpid(pid, NULL, 0);
+		pid = -1;
+	}
 #endif /* WINDOWS */
+
 	if (fd >= 0)
 		close(fd);
+
+#ifdef WINDOWS
+	fd = -1;
+#endif
 }
 
 static int
@@ -464,13 +466,17 @@ pkcs11_start_helper(void)
 	av[1] = verbosity;
 	av[2] = NULL;
 
-	if ((client_process_handle = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, sshagent_client_pid)) == NULL ||
+	if (!sshagent_con_username) {
-		OpenProcessToken(client_process_handle, TOKEN_QUERY | TOKEN_ASSIGN_PRIMARY, &client_token) == FALSE) {
+		error_f("sshagent_con_username is NULL");
-		error_f("failed retrieve user token of the client process");
 		goto out;
-
 	}
-	if (posix_spawnp_as_user((pid_t *)&pid, av[0], &actions, NULL, av, NULL, client_token) != 0) {
+
+	if (!sshagent_client_primary_token) {
+		error_f("sshagent_client_primary_token is NULL for user:%s", sshagent_con_username);
+		goto out;
+	}
+
+	if (posix_spawnp_as_user((pid_t *)&pid, av[0], &actions, NULL, av, NULL, sshagent_client_primary_token) != 0) {
 		error_f("failed to spwan process %s", av[0]);
 		goto out;
 	}

ssh-pkcs11.h

@@ -22,12 +22,10 @@
 #define	SSH_PKCS11_ERR_PIN_REQUIRED		4
 #define	SSH_PKCS11_ERR_PIN_LOCKED		5
 
-
+int	pkcs11_init(int);
-int             pkcs11_init(int);
+void	pkcs11_terminate(void);
-void            pkcs11_terminate(void);
+int	pkcs11_add_provider(char*, char*, struct sshkey***, char***);
-int             pkcs11_add_provider(char *, char *, struct sshkey ***, char ***);
+int	pkcs11_del_provider(char*);
-int             pkcs11_del_provider(char *);
-
 #ifdef WITH_PKCS11_KEYGEN
 struct sshkey *
 	pkcs11_gakp(char *, char *, unsigned int, char *, unsigned int,