ssh-agent: perform AGENTC_REMOVE_ALL_IDENTITIES on SIGUSR1

[Steffen Nurpmeso]

Steffen Nurpmeso wrote in
 <20210908220546.suMBN%steffen at sdaoden.eu>:
 |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.
 ...
 |Instead the signal handler shall set a volatile sig_atomic_t that
 |the main loop later picks up.

Well ok, thinking about this for more than just five minutes
(sorry) i think care should be taken to cause graceful loop wakeup
in any case.

Since _i think_ the poll(2) will not immediately tick due to
SA_RESTART if the "timeout" parameter is not -1, which _i think_
will be caused by setting a maximum lifetime for keys, here i add
a few lines of code which create a socket(2), and perform
a non-blocking connect(2) to the agent socket, followed by an
immediate close(2).  Now this tests fine here and causes the
connect(2) to wakeup the poll(2), even if the socket is already
closed once the signal handler returns.  These calls should be ok
from within a signal handler (alternatively a brute setting of a
O_NONBLOCK fcntl(2) on the AF_UNIX socket instead of calling
set_nonblock() would relax problems further).

With that more code than i wanted is necessary to implement key
clearance on SIGUSR1, but at least the patches combined will
(hopefully portably) at least do what they want.

Thank you.

Ciao from Germany,

diff --git a/ssh-agent.c b/ssh-agent.c
index bc24dadaa8..d0af500e42 100644
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -173,6 +173,7 @@ static int restrict_websafe = 1;
 
 /* Request to remove_all_identities() pending */
 static volatile sig_atomic_t remove_all_next_tick = 0;
+static int remove_all_needs_sock = 0;
 
 static void
 close_socket(SocketEntry *e)
@@ -1355,7 +1356,27 @@ cleanup_handler(int sig)
 static void
 remove_all_handler(int sig)
 {
+	struct sockaddr_un sunaddr;
+	int sock;
+
+	if (remove_all_next_tick)
+		return;
 	remove_all_next_tick = 1;
+
+	if (!remove_all_needs_sock)
+		return;
+
+	/* (Try to) Cause an immediate tick */
+	if ((sock = socket(PF_UNIX, SOCK_STREAM, 0)) != -1) {
+		memset(&sunaddr, 0, sizeof(sunaddr));
+		sunaddr.sun_family = AF_UNIX;
+		(void) strlcpy(sunaddr.sun_path, socket_name,
+		    sizeof(sunaddr.sun_path));
+		(void) set_nonblock(sock);
+		(void) connect(sock, (const struct sockaddr *)&sunaddr,
+		    sizeof(sunaddr));
+		(void) close(sock);
+	}
 }
 
 static void
@@ -1643,6 +1664,7 @@ skip:
 
 	while (1) {
 		prepare_poll(&pfd, &npfd, &timeout, maxfds);
+		remove_all_needs_sock = (timeout != -1);
 		result = poll(pfd, npfd, timeout);
 		saved_errno = errno;
 		if (parent_alive_interval != 0)

--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)