disable adding pkcs11 providers to ssh-agent remotely unless overridd…

…en by cli arg (#699)
PowerShell · Oct 11, 2023 · f72324f · f72324f
1 parent effdb65
commit f72324f
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 29 deletions.
diff --git a/contrib/win32/win32compat/ssh-agent/agent-main.c b/contrib/win32/win32compat/ssh-agent/agent-main.c
@@ -37,6 +37,8 @@
 
 #pragma warning(push, 3)
 
+int remote_add_provider;
+
 int scm_start_service(DWORD, LPWSTR*);
 
 SERVICE_TABLE_ENTRYW dispatch_table[] =
@@ -116,42 +118,63 @@ fix_cwd()
 extern void sanitise_stdfd(void);
 
 int 
-wmain(int argc, wchar_t **argv) 
+wmain(int argc, wchar_t **wargv)
 {
 	_set_invalid_parameter_handler(invalid_parameter_handler);
 	w32posix_initialize();
 	fix_cwd();
+	/* Check if -Oallow-remote-pkcs11 has been passed in for all scenarios */
+	if (argc >= 2) {
+		for (int i = 0; i < argc; i++) {
+			if (wcsncmp(wargv[i], L"-O", 2) == 0) {
+				if (wcsncmp(wargv[i], L"-Oallow-remote-pkcs11", 21) == 0) {
+					remote_add_provider = 1;
+				}
+				else {
+					fatal("Unknown -O option; only allow-remote-pkcs11 is supported");
+				}
+			}
+		}
+	}
 	if (!StartServiceCtrlDispatcherW(dispatch_table)) {
 		if (GetLastError() == ERROR_FAILED_SERVICE_CONTROLLER_CONNECT) {
 			/* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */
 			sanitise_stdfd();
 
-			/* 
-			 * agent is not spawned by SCM 
-			 * Its either started in debug mode or a worker child 
+			/*
+			 * agent is not spawned by SCM
+			 * Its either started in debug mode or a worker child
+			 * If running in debug mode, args are (any order): -d, -dd, or -ddd and -Oallow-remote-pkcs11 (optional)
+			 * If a worker child, args are (in this order): int (connection handle) and -Oallow-remote-pkcs11 (optional) 
 			 */
-			if (argc == 2) {
-				if (wcsncmp(argv[1], L"-ddd", 4) == 0)
-					log_init("ssh-agent", 7, 1, 1);
-				else if (wcsncmp(argv[1], L"-dd", 3) == 0)
-					log_init("ssh-agent", 6, 1, 1);
-				else if (wcsncmp(argv[1], L"-d", 2) == 0)
-					log_init("ssh-agent", 5, 1, 1);
-
-				/* Set Ctrl+C handler if starting in debug mode */
-				if (wcsncmp(argv[1], L"-d", 2) == 0) {
-					SetConsoleCtrlHandler(ctrl_c_handler, TRUE);
-					agent_start(TRUE);
-					return 0;
-				}
 
-				/*agent process is likely a spawned child*/
-				char* h = 0;
-				h += _wtoi(*(argv + 1));
-				if (h != 0) {
-					log_init("ssh-agent", 3, 1, 0);
-					agent_process_connection(h);
-					return 0;
+			if (argc >= 2) {
+				for (int i = 0; i < argc; i++) {
+					if (wcsncmp(wargv[i], L"-ddd", 4) == 0) {
+						log_init("ssh-agent", 7, 1, 1);
+					}
+					else if (wcsncmp(wargv[i], L"-dd", 3) == 0) {
+						log_init("ssh-agent", 6, 1, 1);
+					}
+					else if (wcsncmp(wargv[i], L"-d", 2) == 0) {
+						log_init("ssh-agent", 5, 1, 1);
+					}
+
+					/* Set Ctrl+C handler if starting in debug mode */
+					if (wcsncmp(wargv[i], L"-d", 2) == 0) {
+						SetConsoleCtrlHandler(ctrl_c_handler, TRUE);
+						agent_start(TRUE);
+						return 0;
+					}
+
+					/*agent process is likely a spawned child*/
+					char* h = 0;
+					h += _wtoi(*(wargv + i));
+					if (h != 0) {
+						log_init("ssh-agent", 3, 1, 0);
+						agent_process_connection(h);
+						return 0;
+					}
 				}
 			}
 			/* to support linux compat scenarios where ssh-agent.exe is typically launched per session*/

diff --git a/contrib/win32/win32compat/ssh-agent/agent.c b/contrib/win32/win32compat/ssh-agent/agent.c
@@ -37,6 +37,8 @@
 
 #define BUFSIZE 5 * 1024
 
+extern int remote_add_provider;
+
 char* sshagent_con_username;
 HANDLE sshagent_client_primary_token;
 
@@ -167,10 +169,15 @@ agent_listen_loop()
 				memset(&si, 0, sizeof(STARTUPINFOW));
 				GetModuleFileNameW(NULL, module_path, PATH_MAX);
 				SetHandleInformation(con, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT);
-				if ((swprintf_s(path, PATH_MAX, L"%s %d", module_path, (int)(intptr_t)con) == -1 ) ||
-				    (CreateProcessW(NULL, path, NULL, NULL, TRUE,
-					DETACHED_PROCESS, NULL, NULL,
-					&si, &pi) == FALSE)) {
+				if (remote_add_provider == 1) {
+					if (swprintf_s(path, PATH_MAX, L"%s %d %s", module_path, (int)(intptr_t)con, L"-Oallow-remote-pkcs11") == -1)
+						verbose("Failed to create child process %ls ERROR:%d", module_path, GetLastError());
+				}
+				else {
+					if (swprintf_s(path, PATH_MAX, L"%s %d", module_path, (int)(intptr_t)con) == -1)
+						verbose("Failed to create child process %ls ERROR:%d", module_path, GetLastError());
+				}
+				if (CreateProcessW(NULL, path, NULL, NULL, TRUE, DETACHED_PROCESS, NULL, NULL, &si, &pi) == FALSE) {
 					verbose("Failed to create child process %ls ERROR:%d", module_path, GetLastError());
 				} else {
 					debug("spawned worker %d for agent client pid %d ", pi.dwProcessId, client_pid);

diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -44,6 +44,8 @@
 #define MAX_VALUE_NAME_LENGTH 16383
 #define MAX_VALUE_DATA_LENGTH 2048
 
+extern int remote_add_provider;
+
 /* 
  * get registry root where keys are stored 
  * user keys are stored in user's hive
@@ -660,6 +662,12 @@ int process_add_smartcard_key(struct sshbuf* request, struct sshbuf* response, s
 		goto done;
 	}
 
+	if (con->nsession_ids != 0 && !remote_add_provider) {
+		verbose("failed PKCS#11 add of \"%.100s\": remote addition of "
+		    "providers is disabled", provider);
+		goto done;
+	}
+
 	if (realpath(provider, canonical_provider) == NULL) {
 		error("failed PKCS#11 add of \"%.100s\": realpath: %s",
 			provider, strerror(errno));