Created attachment 2024 [details] implementation Preserving the command line from the invoking ssh command doesn't make much sense, so use setproctitle() to hide the arguments. And chdir into /.
Comment on attachment 2024 [details] implementation >@@ -230,12 +230,25 @@ main(int ac, char **av) > struct servent *sp; > Forward fwd; > >- /* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */ >- sanitise_stdfd(); >- > __progname = ssh_get_progname(av[0]); > init_rng(); > >+#ifndef HAVE_SETPROCTITLE >+ /* Prepare for later setproctitle emulation */ >+ { >+ /* Save argv. Duplicate so setproctitle emulation doesn't clobber it */ >+ char **saved_argv = xcalloc(ac + 1, sizeof(*saved_argv)); >+ for (i = 0; i < ac; i++) >+ saved_argv[i] = xstrdup(av[i]); >+ saved_argv[i] = NULL; >+ compat_init_setproctitle(ac, av); >+ av = saved_argv; compat_init_setproctitle() should save everything that is necessary. Is it not working for you? Also, why reorder the sanitise_stdfd() call?
(In reply to comment #1) > > compat_init_setproctitle() should save everything that is necessary. Is > it not working for you? > > Also, why reorder the sanitise_stdfd() call? I copied from sshd.c which includes the different order of the sanitise_stdfd() to make int consistent. I just put the save_argv code inside the HAVE_SETPROCTITLE guard too, because I don't see a need for this without setproctitle.
Created attachment 2040 [details] OpenBSD patch openbsd patch
What about the chdir(/)? I think this is mandatory for daemons. I also suggest to change the proctitle after a mux stopped listening. Maybe something like this in client_stop_mux(): setproctitle("[closed mux]"); Thanks.
chdir("/") is required for system daemons so they don't block the unmounting of filesystems by holding directory fds open. This seems to be less important for user background processes, where the user would probably remain logged in anyway. Furthermore, in ssh's case it will require ongoing access to known_hosts over the life of the connection (as the hostkey can legally change during key re-exchange) and to the mux socket itself. I guess you could make a case for doing chdir(dirname(UserKnownHostsFile)) or chdir to the directory of the mux socket, but that is really a separate issue. as for doing setproctitle for gracefully-closed mux masters: good idea. I'll add it to the patch.
actually, unconditionally doing setproctitle() in process_mux_stop_listening() is incorrect. It should only be done for forked masters.
> chdir("/") is required for system daemons so they don't block the > unmounting of filesystems by holding directory fds open. This is exactly my problem, say I do some ssh/scp from a directory on an USB stick or an sshfs mount, than the persistent mux should not prevent unmounting of this, completely unrelated, mount point.
> actually, unconditionally doing setproctitle() in > process_mux_stop_listening() is incorrect. It should only be done for > forked masters. Sure, but I proposed doing it in client_stop_mux() and there under the options.control_persist condition.
ok, doing chdir() is a separate issue - please file a bug for that and we can discuss a solution there. I guess it comes down to choosing between the directory containing the mux socket, the user's home directory or the directory containing UserKnownHostsFile.
(In reply to comment #9) > ok, doing chdir() is a separate issue - please file a bug for that and > we can discuss a solution there. I guess it comes down to choosing > between the directory containing the mux socket, the user's home > directory or the directory containing UserKnownHostsFile. Done: #1902
applied - this will be in openssh-5.9. thanks!
close resolved bugs now that openssh-5.9 has been released