[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1117720: Bug#1117638: openssh-client 10.1p1-1 fails to read smart card



Hi Colin,

my TL;DR analysis:

* in 10.1 and up, ssh.c calls pkcs11_init(!options.batch_mode) which is
  intended to set interactive mode, but that ends up in the NOP implementation
  in ssh-pkcs11-client.c. That same file also contains a "proxy stub"
  implementation of pkcs11_add_provider(), which causes the new pkcs11 helper
  process to be forked off, which itself then calls the "real"
  pkcs11_add_provider(), but always does pkcs11_init(0) first. So we have
  "interactive = 1" in the parent, but "interactive = 0" in the helper.

* in 10.0, ssh-pkcs11-helper was not yet used, and the ssh client called
  pkcs11_init() and pkcs11_add_provider() from ssh-pkcs11.c directly, which
  caused the "interactive" flag to be set to the correct value.

I've now made a patch that plumbs through the "interactive" flag from the
parent ssh process to the pkcs11 helper. It's not a stylistic marvel, but
neither is the duplication of function names in the upstream source. ;)
I hope nobody ever links the wrong object files together...

Patch attached, feel free to forward upstream. With this I now get this log
output and everything works as before:

=====
debug1: Connection established.
debug3: pkcs11_start_helper: start helper for /usr/lib/x86_64-linux-gnu/opensc-pkcs11.so
debug3: pkcs11_start_helper: helper 0 for "/usr/lib/x86_64-linux-gnu/opensc-pkcs11.so" on fd 4 pid 32081
debug3: pkcs11_add_provider: add /usr/lib/x86_64-linux-gnu/opensc-pkcs11.so
debug1: pkcs11_start_helper: starting /usr/lib/openssh/ssh-pkcs11-helper -vvv -i
debug3: pkcs11_init: called, interactive = 1
debug1: process_add
debug3: process_add: add /usr/lib/x86_64-linux-gnu/opensc-pkcs11.so
=====

Jan

-- 
Index: openssh-10.2p1/ssh-pkcs11-client.c
===================================================================
--- openssh-10.2p1.orig/ssh-pkcs11-client.c
+++ openssh-10.2p1/ssh-pkcs11-client.c
@@ -40,6 +40,8 @@
 #include "ssh-pkcs11.h"
 #include "ssherr.h"
 
+int pkcs11_interactive = 0;
+
 /* borrows code from sftp-server and ssh-agent */
 
 /*
@@ -210,6 +212,8 @@ recv_msg(int fd, struct sshbuf *m)
 int
 pkcs11_init(int interactive)
 {
+	pkcs11_interactive = interactive;
+
 	return 0;
 }
 
@@ -330,6 +334,8 @@ pkcs11_start_helper(const char *path)
 {
 	int pair[2];
 	char *prog, *verbosity = NULL;
+	char *args[4];
+	int nargs = 1;
 	struct helper *helper;
 	pid_t pid;
 
@@ -358,11 +364,18 @@ pkcs11_start_helper(const char *path)
 		prog = getenv("SSH_PKCS11_HELPER");
 		if (prog == NULL || strlen(prog) == 0)
 			prog = _PATH_SSH_PKCS11_HELPER;
-		if (log_level_get() >= SYSLOG_LEVEL_DEBUG1)
-			verbosity = "-vvv";
-		debug_f("starting %s %s", prog,
-		    verbosity == NULL ? "" : verbosity);
-		execlp(prog, prog, verbosity, (char *)NULL);
+		args[0] = prog;
+		if (log_level_get() >= SYSLOG_LEVEL_DEBUG1) {
+			args[nargs] = verbosity = "-vvv";
+			nargs++;
+		}
+		if (pkcs11_interactive) {
+			args[nargs] = "-i";
+			nargs++;
+		}
+		args[nargs] = NULL;
+		debug_f("starting %s %s %s", args[0], args[1] ?: "", args[2] ?: "");
+		execvp(prog, args);
 		fprintf(stderr, "exec: %s: %s\n", prog, strerror(errno));
 		_exit(1);
 	}
Index: openssh-10.2p1/ssh-pkcs11-helper.c
===================================================================
--- openssh-10.2p1.orig/ssh-pkcs11-helper.c
+++ openssh-10.2p1/ssh-pkcs11-helper.c
@@ -218,13 +218,14 @@ main(int argc, char **argv)
 	char buf[4*4096];
 	extern char *__progname;
 	struct pollfd pfd[2];
+	int interactive = 0;
 
 	__progname = ssh_get_progname(argv[0]);
 	seed_rng();
 
 	log_init(__progname, log_level, log_facility, log_stderr);
 
-	while ((ch = getopt(argc, argv, "v")) != -1) {
+	while ((ch = getopt(argc, argv, "vi")) != -1) {
 		switch (ch) {
 		case 'v':
 			log_stderr = 1;
@@ -233,6 +234,9 @@ main(int argc, char **argv)
 			else if (log_level < SYSLOG_LEVEL_DEBUG3)
 				log_level++;
 			break;
+		case 'i':
+			interactive = 1;
+			break;
 		default:
 			fprintf(stderr, "usage: %s [-v]\n", __progname);
 			exit(1);
@@ -241,7 +245,7 @@ main(int argc, char **argv)
 
 	log_init(__progname, log_level, log_facility, log_stderr);
 
-	pkcs11_init(0);
+	pkcs11_init(interactive);
 	in = STDIN_FILENO;
 	out = STDOUT_FILENO;
 

Reply to: