ssh -f and -O ControlPersist=yes, ControlMaster=yes leaves stderr open

[Mihai Moldovan]

Hi


I'm trying to wrap ssh in an application using glib. For now, I'm launching the
ssh client in master mode and want it to detach, keeping the control socket around.

I figured I could do that using the -f flag and the usual Control* options to
force ssh to daemonize (ideally without executing any command), but it turns out
that glib doesn't recognize the daemonized process as exited, since its...
(grand-?) child process stderr pipe keeps connected (hereditary) to the original
stderr pipe, which, in my tests, happens to be the stderr pipe of my shell.


Now, that's not a tragedy per se, but I wonder if this is considered a feature
or rather a bug.

I usually expect a daemonizing process to close its file descriptors - at least
in children. The OpenSSH client does not do that in some cases...


From current master's ssh.c:

------------------------------------>snip<------------------------------------
static int ssh_session2(struct ssh *ssh, struct passwd *pw) {
[...]
	if (options.control_persist && muxserver_sock != -1) {
		[...]
		if (!fork_after_authentication_flag)
			need_controlpersist_detach = 1;
		fork_after_authentication_flag = 1;
	}
------------------------------------>snip<------------------------------------

Note how need_controlpersist_detach is only set to 1 if
fork_after_authentication_flag is not set (i.e., -f has not been passed). This
probably makes sense to not fork twice, but let's look further:

------------------------------------>snip<------------------------------------
/* Do fork() after authentication. Used by "ssh -f" */
static void fork_postauth(void) {
	if (need_controlpersist_detach)
		control_persist_detach();
	[...]
	if (daemon(1, 1) == -1)
------------------------------------>snip<------------------------------------

No closing of FDs through daemon()... and since need_controlpersist_detach will
not be true if fork_after_authentication_flag is, the code that closes the other
FDs will not run.


I wonder if that (not closing stderr) is generally a sane idea, not just in my,
admittedly, advanced use case. stdin and stdout will be closed, which makes
sense, but stderr is kept connected. That *might* be useful because stderr is
the default logging place (and you wouldn't be able to get error messages from
the backgrounded child otherwise) *and* the documentation doesn't talk about
daemonizing, but merely "backgrounding" (is this nitpicking). It feels odd, though.


And it's especially odd in my use case, because the control-persist feature
explicitly includes this code (if -f has not been passed):

------------------------------------>snip<------------------------------------
static void control_persist_detach(void) {
[...]
	switch ((pid = fork())) {
	[...]
	case 0:
		/* Child: master process continues mainloop */
		break;
	default:
		/* Parent: set up mux slave to connect to backgrounded master */
		[...]
		/* muxclient() doesn't return on success. */
		fatal("Failed to connect to new control master");
	}
	[...]
		keep_stderr = log_is_on_stderr() && debug_flag;
		if (dup2(devnull, STDIN_FILENO) == -1 ||
		    dup2(devnull, STDOUT_FILENO) == -1 ||
		    (!keep_stderr && dup2(devnull, STDERR_FILENO) == -1))
		[...]
	daemon(1, 1);
------------------------------------>snip<------------------------------------

Note how stderr is explicitly to be closed by the master (child) process UNLESS
debugging is turned on, which sounds like an okay consideration for debugging
things.


What's your opinion? Is this a bug or an intended feature?



Mihai

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: <http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20200314/76d99284/attachment.asc>