Re: bin/60588: pam_ssh mishandles ssh-agent child process

["Taylor R Campbell via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR bin/60588; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: bin/60588: pam_ssh mishandles ssh-agent child process
Date: Mon, 9 Nov 2026 20:50:35 +0000

 This is a multi-part message in MIME format.
 --=_PNVW4IqkU7iReupGDEmOFrgjPRRoyxgK
 
 Updated patch to make pam_ssh's ssh-agent child inherit fd 2/stderr so
 it has a chance of actually working when it closes fd 2.
 
 --=_PNVW4IqkU7iReupGDEmOFrgjPRRoyxgK
 Content-Type: text/plain; charset="ISO-8859-1"; name="pr60588-pamsshzombie-v2"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename="pr60588-pamsshzombie-v2.patch"
 
 # 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 35162dc176dc0ebedd084e4fdf73db80769e4580
 # Parent  4fbd288ac1da9e7d9592ddecf04f7513e244152f
 # EXP-Topic riastradh-pr60588-pamsshagentchild
 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/60588: pam_ssh mishandles ssh-agent child process
 
 diff -r 4fbd288ac1da -r 35162dc176dc crypto/external/bsd/openssh/dist/ssh-a=
 gent.c
 --- a/crypto/external/bsd/openssh/dist/ssh-agent.c	Mon Aug 10 13:36:10 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) =3D=3D -1) {
 +			perror("dup2");
 +			cleanup_exit(1);
 +		}
 +
  		goto skip;
  	}
  	pid =3D 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 a8faae1379d8e65e6b84d5a429f3c642b212c83c
 # Parent  35162dc176dc0ebedd084e4fdf73db80769e4580
 # EXP-Topic riastradh-pr60588-pamsshagentchild
 pam_ssh: Handle ssh-agent(1) child process properly.
 
 PR bin/60588: pam_ssh mishandles ssh-agent child process
 
 diff -r 35162dc176dc -r a8faae1379d8 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[] =3D=20
  };
 =20
  static const char *pam_ssh_agent =3D "/usr/bin/ssh-agent";
 -static const char *const pam_ssh_agent_argv[] =3D { "ssh_agent", "-s", NUL=
 L };
 +static const char *const pam_ssh_agent_argv[] =3D { "ssh_agent", "-Ds", NU=
 LL };
  static const char *const pam_ssh_agent_envp[] =3D { NULL };
 =20
  /*
 @@ -285,6 +285,7 @@ pam_ssh_start_agent(pam_handle_t *pamh,=20
  {
  	int agent_pipe[2];
  	pid_t pid;
 +	char pidstr[32];
  	FILE *f;
 =20
  	/* get a pipe which we will use to read the agent's output */
 @@ -324,7 +325,10 @@ pam_ssh_start_agent(pam_handle_t *pamh,=20
  		(void)close(STDIN_FILENO);
  		(void)open(_PATH_DEVNULL, O_RDONLY);
  		(void)dup2(agent_pipe[1], STDOUT_FILENO);
 -		(void)dup2(agent_pipe[1], STDERR_FILENO);
 +		/*
 +		 * preserve stderr (could redirect to openpam_log but
 +		 * would require a lot more work to multiplex the I/O)
 +		 */
  #ifdef F_CLOSEM
  		(void)fcntl(3, F_CLOSEM, 0);
  #else
 @@ -345,6 +349,10 @@ done:
  	pam_ssh_process_agent_output(pamh, f);
  	fclose(f);
 =20
 +	if ((size_t)snprintf(pidstr, sizeof(pidstr), "%ld", (long)pid) <
 +	    sizeof(pidstr))
 +		pam_setenv(pamh, "SSH_AGENT_PID", pidstr, 1);
 +
  	return (PAM_SUCCESS);
  }
 =20
 # HG changeset patch
 # User Taylor R Campbell <riastradh%NetBSD.org@localhost>
 # Date 1786739705 0
 #      Fri Aug 14 20:35:05 2026 +0000
 # Branch trunk
 # Node ID 42af4aaef3dbbc4996bed16751867ae8662a7a6e
 # Parent  a8faae1379d8e65e6b84d5a429f3c642b212c83c
 # EXP-Topic riastradh-pr60588-pamsshagentchild
 pam_ssh(3): Use `ssh-agent', not `ssh_agent', as argv[0].
 
 Prompted by:
 
 PR bin/60588: pam_ssh mishandles ssh-agent child process
 
 diff -r a8faae1379d8 -r 42af4aaef3db lib/libpam/modules/pam_ssh/pam_ssh.c
 --- a/lib/libpam/modules/pam_ssh/pam_ssh.c	Fri Aug 14 18:18:17 2026 +0000
 +++ b/lib/libpam/modules/pam_ssh/pam_ssh.c	Fri Aug 14 20:35:05 2026 +0000
 @@ -89,7 +89,7 @@ static const char *pam_ssh_keyfiles[] =3D=20
  };
 =20
  static const char *pam_ssh_agent =3D "/usr/bin/ssh-agent";
 -static const char *const pam_ssh_agent_argv[] =3D { "ssh_agent", "-Ds", NU=
 LL };
 +static const char *const pam_ssh_agent_argv[] =3D { "ssh-agent", "-Ds", NU=
 LL };
  static const char *const pam_ssh_agent_envp[] =3D { NULL };
 =20
  /*
 
 --=_PNVW4IqkU7iReupGDEmOFrgjPRRoyxgK--