bin/60588: pam_ssh mishandles ssh-agent child process

["campbell+netbsd%mumble.net@localhost via gnats" <gnats-admin%NetBSD.org@localhost>]

>Number:         60588
>Category:       bin
>Synopsis:       pam_ssh mishandles ssh-agent child process
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Aug 14 20:30:01 +0000 2026
>Originator:     Taylor R Campbell
>Release:        current, 11, 10, 9, ...
>Organization:
The NetBSD Agent, Insshallah
>Environment:
>Description:

	The pam_ssh(3) session management module runs ssh-agent(1) on
	the user's behalf in the background during the login session
	by:

	1. running `ssh-agent -s' as a child of the login process,

	2. parsing the /bin/sh output (approximately: see PR 60578) for
	   SSH_AUTH_SOCK and SSH_AGENT_PID (and any other SSH_*
	   setting) to put in the environment,

	and then, when the session closes,

	3. killing the process in SSH_AGENT_PID with SIGTERM, and

	4. waiting for the process in SSH_AGENT_PID.

	Unfortunately, this is wrong, because `ssh-agent -s' itself
	forks and then exits, so the SSH_AGENT_PID is not a child of
	the login process.  So:

	(a) The `ssh-agent -s' process remains a zombie indefinitely
	    until the login process exits and the `ssh-agent -s'
	    process is reparented to pid 1/init.

	(b) By the time we get to step (3), the pid may have been
	    recycled and pam_ssh may kill some random other process.

	(c) It is invalid for the login process to wait for the process
	    in SSH_AGENT_PID, because it's not a child of the login
	    process.

	This was reported as FreeBSD bug 99217 back in 2006:
	https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=99217

>How-To-Repeat:

	code inspection while reviewing PR 60578, which linked to
	https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=285043

>Fix:

	1. Patch ssh-agent(1) so that when it is done printing output
	   (for the `-s' or `-c' options'), it will close STDOUT_FILENO
	   (or will dup2 another fd like STDERR_FILENO to STDOUT_FILENO
	   so the total number of open fds remains the same, since
	   ssh-agent(1) is careful about fd rlimits).  This way the
	   other endpoint of ssh-agent's stdout pipe will be notified
	   when it is done printing output.

	2. Patch pam_ssh(3) so that it runs `ssh-agent -Ds' instead
	   (-D: foreground mode), and then simply kills and waits for
	   the ssh-agent(1) child it forked.

	(Warning: Untested!)

# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1786731447 0
#      Fri Aug 14 18:17:27 2026 +0000
# Branch trunk
# Node ID 7cefc251515900cc421839f9b42e3baeaeef45f1
# Parent  0912ccb2dc6af164f0e0a92b2dff65f958fbd33e
# EXP-Topic riastradh-pr60578-pamsshagent
ssh-agent(1): Close stdout when done writing to it.

Actually, dup stderr to stdout, so that fd 1 remains open (this way
we don't disrupt the carefully counted file descriptor rlimit), but
if fd 1 was one endpoint of a pipe supplied by the parent, the other
endpoint will reflect the closure and return EOF to reads so the
parent knows when the child is done writing.

This way, parents can reliably be notified what the socket is once it
is bound and listening.  (Passing the socket path with `-a <path>'
wouldn't work because the parent has no way to know when ssh-agent(1)
is bound and listening.)

PR bin/NNNNN: pam_ssh zombie attack

diff -r 0912ccb2dc6a -r 7cefc2515159 crypto/external/bsd/openssh/dist/ssh-agent.c
--- a/crypto/external/bsd/openssh/dist/ssh-agent.c	Fri Aug 14 17:20:54 2026 +0000
+++ b/crypto/external/bsd/openssh/dist/ssh-agent.c	Fri Aug 14 18:17:27 2026 +0000
@@ -2456,6 +2456,15 @@ main(int ac, char **av)
 		free(cp);
 		printf("echo Agent pid %ld;\n", (long)parent_pid);
 		fflush(stdout);
+
+		/*
+		 * Close stdout so caller knows the output is finished.
+		 */
+		if (dup2(STDERR_FILENO, STDOUT_FILENO) == -1) {
+			perror("dup2");
+			cleanup_exit(1);
+		}
+
 		goto skip;
 	}
 	pid = fork();
# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1786731497 0
#      Fri Aug 14 18:18:17 2026 +0000
# Branch trunk
# Node ID 3143ea8917167c0d8084dd4538b421011b64728d
# Parent  7cefc251515900cc421839f9b42e3baeaeef45f1
# EXP-Topic riastradh-pr60578-pamsshagent
WIP: pam_ssh: Fix zombie attack.

PR bin/NNNNN: pam_ssh zombie attack

diff -r 7cefc2515159 -r 3143ea891716 lib/libpam/modules/pam_ssh/pam_ssh.c
--- a/lib/libpam/modules/pam_ssh/pam_ssh.c	Fri Aug 14 18:17:27 2026 +0000
+++ b/lib/libpam/modules/pam_ssh/pam_ssh.c	Fri Aug 14 18:18:17 2026 +0000
@@ -89,7 +89,7 @@ static const char *pam_ssh_keyfiles[] = 
 };
 
 static const char *pam_ssh_agent = "/usr/bin/ssh-agent";
-static const char *const pam_ssh_agent_argv[] = { "ssh_agent", "-s", NULL };
+static const char *const pam_ssh_agent_argv[] = { "ssh_agent", "-Ds", NULL };
 static const char *const pam_ssh_agent_envp[] = { NULL };
 
 /*
@@ -285,6 +285,7 @@ pam_ssh_start_agent(pam_handle_t *pamh, 
 {
 	int agent_pipe[2];
 	pid_t pid;
+	char pidstr[32];
 	FILE *f;
 
 	/* get a pipe which we will use to read the agent's output */
@@ -345,6 +346,10 @@ done:
 	pam_ssh_process_agent_output(pamh, f);
 	fclose(f);
 
+	if ((size_t)snprintf(pidstr, sizeof(pidstr), "%jd", (intmax_t)pid) <
+	    sizeof(pidstr))
+		pam_setenv(pamh, "SSH_AGENT_PID", pidstr, 1);
+
 	return (PAM_SUCCESS);
 }