ssh-agent: perform AGENTC_REMOVE_ALL_IDENTITIES on SIGUSR1

[Steffen Nurpmeso]

Steffen Nurpmeso wrote in
 <20210908114659.EuomV%steffen at sdaoden.eu>:
 |A long time ago i asked for the possibility to remove all
 |identities from a running agent by sending a signal.
 ...
 |The necessary code addition to ssh-agent is minimal, and builds
 |upon existing code.  The work is done in a signal handler, because
 |ssh_signal() sets SA_RESTART and thus the loop would not wake up
 |to tick.  On the other hand ssh_signal() fills the signal mask, so
 |signal recursion of any form cannot happen, therefore processing
 |the list in the signal handler seems safe.

Ha!  Not true, i have been kindly enlightened that the signal
can happen at anytime and thus my approach may corrupt state, so
"it cannot be coded this way".
Instead the signal handler shall set a volatile sig_atomic_t that
the main loop later picks up.

diff --git a/ssh-agent.1 b/ssh-agent.1
index ed8c870969..dd2b0e8af1 100644
--- a/ssh-agent.1
+++ b/ssh-agent.1
@@ -178,6 +178,9 @@ will automatically use them if present.
 is also used to remove keys from
 .Nm
 and to query the keys that are held in one.
+.Nm
+will remove all keys when it receives the user signal
+.Dv SIGUSR1 .
 .Pp
 Connections to
 .Nm
diff --git a/ssh-agent.c b/ssh-agent.c
index 48a47d45a9..bc24dadaa8 100644
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -171,6 +171,9 @@ static int fingerprint_hash = SSH_FP_HASH_DEFAULT;
 /* Refuse signing of non-SSH messages for web-origin FIDO keys */
 static int restrict_websafe = 1;
 
+/* Request to remove_all_identities() pending */
+static volatile sig_atomic_t remove_all_next_tick = 0;
+
 static void
 close_socket(SocketEntry *e)
 {
@@ -529,11 +532,10 @@ process_remove_identity(SocketEntry *e)
 }
 
 static void
-process_remove_all_identities(SocketEntry *e)
+remove_all_identities(void)
 {
 	Identity *id;
 
-	debug2_f("entering");
 	/* Loop over all identities and clear the keys. */
 	for (id = TAILQ_FIRST(&idtab->idlist); id;
 	    id = TAILQ_FIRST(&idtab->idlist)) {
@@ -543,6 +545,13 @@ process_remove_all_identities(SocketEntry *e)
 
 	/* Mark that there are no identities. */
 	idtab->nentries = 0;
+}
+
+static void
+process_remove_all_identities(SocketEntry *e)
+{
+	debug2_f("entering");
+	remove_all_identities();
 
 	/* Send success. */
 	send_status(e, 1);
@@ -1342,6 +1351,13 @@ cleanup_handler(int sig)
 	_exit(2);
 }
 
+/*ARGSUSED*/
+static void
+remove_all_handler(int sig)
+{
+	remove_all_next_tick = 1;
+}
+
 static void
 check_parent_exists(void)
 {
@@ -1619,6 +1635,7 @@ skip:
 	ssh_signal(SIGINT, (d_flag | D_flag) ? cleanup_handler : SIG_IGN);
 	ssh_signal(SIGHUP, cleanup_handler);
 	ssh_signal(SIGTERM, cleanup_handler);
+	ssh_signal(SIGUSR1, remove_all_handler);
 
 	if (pledge("stdio rpath cpath unix id proc exec", NULL) == -1)
 		fatal("%s: pledge: %s", __progname, strerror(errno));
@@ -1630,7 +1647,11 @@ skip:
 		saved_errno = errno;
 		if (parent_alive_interval != 0)
 			check_parent_exists();
-		(void) reaper();	/* remove expired keys */
+		if (remove_all_next_tick) {
+			remove_all_next_tick = 0;
+			remove_all_identities();
+		} else
+			(void) reaper();	/* remove expired keys */
 		if (result == -1) {
 			if (saved_errno == EINTR)
 				continue;

--steffen
|
|Der Kragenbaer,                The moon bear,
|der holt sich munter           he cheerfully and one by one
|einen nach dem anderen runter  wa.ks himself off
|(By Robert Gernhardt)