[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#594295: openssh-client: Please either support or ignore ControlPersist in stable, for compatibility



On Tue, Aug 24, 2010 at 11:44:47PM -0700, Josh Triplett wrote:
> Thank you for packaging 5.6, and thanks to upstream for the awesome
> ControlPersist support.
> 
> I use the same home directory across many systems, some running stable,
> and some running unstable.  This means I can't use any option in
> ~/.ssh/config that stable's ssh does not support, because that would
> break compatibility with the systems that run stable.  Given the usual
> lifetime of a Debian stable release, it would help greatly if stable's
> ssh would either support ControlPersist, or at a minimum ignore it for
> compatibility.

I can sympathise with this.  I'd rather make it do something useful than
ignore it.  However, since we're frozen, I'm CCing the release team for
their input.

The patch would look like the attached (plus adjustments in some other
patches on top of it).

-- 
Colin Watson                                       [cjwatson@debian.org]
Description: Backport ControlPersist support for background multiplex masters
 This backports two revisions from upstream:
 .
   - djm@cvs.openbsd.org 2010/07/19 09:15:12
     [clientloop.c readconf.c readconf.h ssh.c ssh_config.5]
     add a "ControlPersist" option that automatically starts a background
     ssh(1) multiplex master when connecting. This connection can stay alive
     indefinitely, or can be set to automatically close after a user-specified
     duration of inactivity. bz#1330 - patch by dwmw2 AT infradead.org, but
     further hacked on by wmertens AT cisco.com, apb AT cequrux.com,
     martin-mindrot-bugzilla AT earth.li and myself; "looks ok" markus@
 .
   - djm@cvs.openbsd.org 2010/08/12 21:49:44
     [ssh.c]
     close any extra file descriptors inherited from parent at start and
     reopen stdin/stdout to /dev/null when forking for ControlPersist.
 .
     prevents tools that fork and run a captive ssh for communication from
     failing to exit when the ssh completes while they wait for these fds to
     close. The inherited fds may persist arbitrarily long if a background
     mux master has been started by ControlPersist. cvs and scp were effected
     by this.
 .
     "please commit" markus@
Origin: upstream, http://bazaar.launchpad.net/~vcs-imports/openssh/main/revision/6117, http://bazaar.launchpad.net/~vcs-imports/openssh/main/revision/6135
Bug-Debian: http://bugs.debian.org/594295
Forwarded: not-needed
Last-Update: 2010-08-25

Index: b/clientloop.c
===================================================================
--- a/clientloop.c
+++ b/clientloop.c
@@ -149,6 +149,9 @@
 /* Flag indicating whether the user's terminal is in non-blocking mode. */
 static int in_non_blocking_mode = 0;
 
+/* Time when backgrounded control master using ControlPersist should exit */
+static time_t control_persist_exit_time = 0;
+
 /* Common data for the client loop code. */
 volatile sig_atomic_t quit_pending; /* Set non-zero to quit the loop. */
 static int escape_char1;	/* Escape character. (proto1 only) */
@@ -255,6 +258,34 @@
 	return (double) tv.tv_sec + (double) tv.tv_usec / 1000000.0;
 }
 
+/*
+ * Sets control_persist_exit_time to the absolute time when the
+ * backgrounded control master should exit due to expiry of the
+ * ControlPersist timeout.  Sets it to 0 if we are not a backgrounded
+ * control master process, or if there is no ControlPersist timeout.
+ */
+static void
+set_control_persist_exit_time(void)
+{
+	if (muxserver_sock == -1 || !options.control_persist
+	    || options.control_persist_timeout == 0)
+		/* not using a ControlPersist timeout */
+		control_persist_exit_time = 0;
+	else if (channel_still_open()) {
+		/* some client connections are still open */
+		if (control_persist_exit_time > 0)
+			debug2("%s: cancel scheduled exit", __func__);
+		control_persist_exit_time = 0;
+	} else if (control_persist_exit_time <= 0) {
+		/* a client connection has recently closed */
+		control_persist_exit_time = time(NULL) +
+			(time_t)options.control_persist_timeout;
+		debug2("%s: schedule exit in %d seconds", __func__,
+		    options.control_persist_timeout);
+	}
+	/* else we are already counting down to the timeout */
+}
+
 #define SSH_X11_PROTO "MIT-MAGIC-COOKIE-1"
 void
 client_x11_get_proto(const char *display, const char *xauth_path,
@@ -528,6 +559,7 @@
     int *maxfdp, u_int *nallocp, int rekeying)
 {
 	struct timeval tv, *tvp;
+	int timeout_secs;
 	int ret;
 
 	/* Add any selections by the channel mechanism. */
@@ -571,16 +603,27 @@
 	/*
 	 * Wait for something to happen.  This will suspend the process until
 	 * some selected descriptor can be read, written, or has some other
-	 * event pending.
+	 * event pending, or a timeout expires.
 	 */
 
-	if (options.server_alive_interval == 0 || !compat20)
+	timeout_secs = INT_MAX; /* we use INT_MAX to mean no timeout */
+	if (options.server_alive_interval > 0 && compat20)
+		timeout_secs = options.server_alive_interval;
+	set_control_persist_exit_time();
+	if (control_persist_exit_time > 0) {
+		timeout_secs = MIN(timeout_secs,
+			control_persist_exit_time - time(NULL));
+		if (timeout_secs < 0)
+			timeout_secs = 0;
+	}
+	if (timeout_secs == INT_MAX)
 		tvp = NULL;
 	else {
-		tv.tv_sec = options.server_alive_interval;
+		tv.tv_sec = timeout_secs;
 		tv.tv_usec = 0;
 		tvp = &tv;
 	}
+
 	ret = select((*maxfdp)+1, *readsetp, *writesetp, NULL, tvp);
 	if (ret < 0) {
 		char buf[100];
@@ -1482,6 +1525,18 @@
 		 */
 		if (FD_ISSET(connection_out, writeset))
 			packet_write_poll();
+
+		/*
+		 * If we are a backgrounded control master, and the
+		 * timeout has expired without any active client
+		 * connections, then quit.
+		 */
+		if (control_persist_exit_time > 0) {
+			if (time(NULL) >= control_persist_exit_time) {
+				debug("ControlPersist timeout expired");
+				break;
+			}
+		}
 	}
 	if (readset)
 		xfree(readset);
Index: b/readconf.c
===================================================================
--- a/readconf.c
+++ b/readconf.c
@@ -130,7 +130,8 @@
 	oAddressFamily, oGssAuthentication, oGssDelegateCreds,
 	oGssTrustDns, oGssKeyEx, oGssClientIdentity, oGssRenewalRekey,
 	oServerAliveInterval, oServerAliveCountMax, oIdentitiesOnly,
-	oSendEnv, oControlPath, oControlMaster, oHashKnownHosts,
+	oSendEnv, oControlPath, oControlMaster, oControlPersist,
+	oHashKnownHosts,
 	oTunnel, oTunnelDevice, oLocalCommand, oPermitLocalCommand,
 	oVisualHostKey, oUseRoaming, oZeroKnowledgePasswordAuthentication,
 	oDeprecated, oUnsupported
@@ -235,6 +236,7 @@
 	{ "sendenv", oSendEnv },
 	{ "controlpath", oControlPath },
 	{ "controlmaster", oControlMaster },
+	{ "controlpersist", oControlPersist },
 	{ "hashknownhosts", oHashKnownHosts },
 	{ "tunnel", oTunnel },
 	{ "tunneldevice", oTunnelDevice },
@@ -897,6 +899,30 @@
 			*intptr = value;
 		break;
 
+	case oControlPersist:
+		/* no/false/yes/true, or a time spec */
+		intptr = &options->control_persist;
+		arg = strdelim(&s);
+		if (!arg || *arg == '\0')
+			fatal("%.200s line %d: Missing ControlPersist"
+			    " argument.", filename, linenum);
+		value = 0;
+		value2 = 0;	/* timeout */
+		if (strcmp(arg, "no") == 0 || strcmp(arg, "false") == 0)
+			value = 0;
+		else if (strcmp(arg, "yes") == 0 || strcmp(arg, "true") == 0)
+			value = 1;
+		else if ((value2 = convtime(arg)) >= 0)
+			value = 1;
+		else
+			fatal("%.200s line %d: Bad ControlPersist argument.",
+			    filename, linenum);
+		if (*activep && *intptr == -1) {
+			*intptr = value;
+			options->control_persist_timeout = value2;
+		}
+		break;
+
 	case oHashKnownHosts:
 		intptr = &options->hash_known_hosts;
 		goto parse_flag;
@@ -1100,6 +1126,8 @@
 	options->num_send_env = 0;
 	options->control_path = NULL;
 	options->control_master = -1;
+	options->control_persist = -1;
+	options->control_persist_timeout = 0;
 	options->hash_known_hosts = -1;
 	options->tun_open = -1;
 	options->tun_local = -1;
@@ -1241,6 +1269,10 @@
 		options->server_alive_count_max = 3;
 	if (options->control_master == -1)
 		options->control_master = 0;
+	if (options->control_persist == -1) {
+		options->control_persist = 0;
+		options->control_persist_timeout = 0;
+	}
 	if (options->hash_known_hosts == -1)
 		options->hash_known_hosts = 0;
 	if (options->tun_open == -1)
Index: b/readconf.h
===================================================================
--- a/readconf.h
+++ b/readconf.h
@@ -117,6 +117,8 @@
 
 	char	*control_path;
 	int	control_master;
+	int     control_persist; /* ControlPersist flag */
+	int     control_persist_timeout; /* ControlPersist timeout (seconds) */
 
 	int	hash_known_hosts;
 
Index: b/ssh.c
===================================================================
--- a/ssh.c
+++ b/ssh.c
@@ -127,6 +127,15 @@
 int stdin_null_flag = 0;
 
 /*
+ * Flag indicating that the current process should be backgrounded and
+ * a new slave launched in the foreground for ControlPersist.
+ */
+int need_controlpersist_detach = 0;
+
+/* Copies of flags for ControlPersist foreground slave */
+int ostdin_null_flag, ono_shell_flag, ono_tty_flag, otty_flag;
+
+/*
  * Flag indicating that ssh should fork after authentication.  This is useful
  * so that the passphrase can be entered manually, and then ssh goes to the
  * background.
@@ -228,6 +237,12 @@
 	init_rng();
 
 	/*
+	 * Discard other fds that are hanging around. These can cause problem
+	 * with backgrounded ssh processes started by ControlPersist.
+	 */
+	closefrom(STDERR_FILENO + 1);
+
+	/*
 	 * Save the original real uid.  It will be needed later (uid-swapping
 	 * may clobber the real uid).
 	 */
@@ -866,6 +881,61 @@
 	return exit_status;
 }
 
+static void
+control_persist_detach(void)
+{
+	pid_t pid;
+	int devnull;
+
+	debug("%s: backgrounding master process", __func__);
+
+ 	/*
+ 	 * master (current process) into the background, and make the
+ 	 * foreground process a client of the backgrounded master.
+ 	 */
+	switch ((pid = fork())) {
+	case -1:
+		fatal("%s: fork: %s", __func__, strerror(errno));
+	case 0:
+		/* Child: master process continues mainloop */
+ 		break;
+ 	default:
+		/* Parent: set up mux slave to connect to backgrounded master */
+		debug2("%s: background process is %ld", __func__, (long)pid);
+		stdin_null_flag = ostdin_null_flag;
+		no_shell_flag = ono_shell_flag;
+		no_tty_flag = ono_tty_flag;
+		tty_flag = otty_flag;
+ 		close(muxserver_sock);
+ 		muxserver_sock = -1;
+ 		muxclient(options.control_path);
+		/* muxclient() doesn't return on success. */
+ 		fatal("Failed to connect to new control master");
+ 	}
+	if ((devnull = open(_PATH_DEVNULL, O_RDWR)) == -1) {
+		error("%s: open(\"/dev/null\"): %s", __func__,
+		    strerror(errno));
+	} else {
+		if (dup2(devnull, STDIN_FILENO) == -1 ||
+		    dup2(devnull, STDOUT_FILENO) == -1)
+			error("%s: dup2: %s", __func__, strerror(errno));
+		if (devnull > STDERR_FILENO)
+			close(devnull);
+	}
+}
+
+/* Do fork() after authentication. Used by "ssh -f" */
+static void
+fork_postauth(void)
+{
+	if (need_controlpersist_detach)
+		control_persist_detach();
+	debug("forking to background");
+	fork_after_authentication_flag = 0;
+	if (daemon(1, 1) < 0)
+		fatal("daemon() failed: %.200s", strerror(errno));
+}
+
 /* Callback for remote forward global requests */
 static void
 ssh_confirm_remote_forward(int type, u_int32_t seq, void *ctxt)
@@ -892,12 +962,8 @@
 	}
 	if (++remote_forward_confirms_received == options.num_remote_forwards) {
 		debug("All remote forwarding requests processed");
-		if (fork_after_authentication_flag) {
-			fork_after_authentication_flag = 0;
-			if (daemon(1, 1) < 0)
-				fatal("daemon() failed: %.200s",
-				    strerror(errno));
-		}
+		if (fork_after_authentication_flag)
+			fork_postauth();
 	}
 }
 
@@ -1139,12 +1205,13 @@
 	 * If requested and we are not interested in replies to remote
 	 * forwarding requests, then let ssh continue in the background.
 	 */
-	if (fork_after_authentication_flag &&
-	    (!options.exit_on_forward_failure ||
-	    options.num_remote_forwards == 0)) {
-		fork_after_authentication_flag = 0;
-		if (daemon(1, 1) < 0)
-			fatal("daemon() failed: %.200s", strerror(errno));
+	if (fork_after_authentication_flag) {
+		if (options.exit_on_forward_failure &&
+		    options.num_remote_forwards > 0) {
+			debug("deferring postauth fork until remote forward "
+			    "confirmation received");
+		} else
+			fork_postauth();
 	}
 
 	/*
@@ -1263,6 +1330,31 @@
 	/* XXX should be pre-session */
 	ssh_init_forwarding();
 
+	/* Start listening for multiplex clients */
+	muxserver_listen();
+
+ 	/*
+	 * If we are in control persist mode, then prepare to background
+	 * ourselves and have a foreground client attach as a control
+	 * slave. NB. we must save copies of the flags that we override for
+	 * the backgrounding, since we defer attachment of the slave until
+	 * after the connection is fully established (in particular,
+	 * async rfwd replies have been received for ExitOnForwardFailure).
+	 */
+ 	if (options.control_persist && muxserver_sock != -1) {
+		ostdin_null_flag = stdin_null_flag;
+		ono_shell_flag = no_shell_flag;
+		ono_tty_flag = no_tty_flag;
+		otty_flag = tty_flag;
+ 		stdin_null_flag = 1;
+ 		no_shell_flag = 1;
+ 		no_tty_flag = 1;
+ 		tty_flag = 0;
+		if (!fork_after_authentication_flag)
+			need_controlpersist_detach = 1;
+		fork_after_authentication_flag = 1;
+ 	}
+
 	if (!no_shell_flag || (datafellows & SSH_BUG_DUMMYCHAN))
 		id = ssh_session2_open();
 
@@ -1281,15 +1373,9 @@
 	    options.permit_local_command)
 		ssh_local_cmd(options.local_command);
 
-	/* Start listening for multiplex clients */
-	muxserver_listen();
-
 	/* If requested, let ssh continue in the background. */
-	if (fork_after_authentication_flag) {
-		fork_after_authentication_flag = 0;
-		if (daemon(1, 1) < 0)
-			fatal("daemon() failed: %.200s", strerror(errno));
-	}
+	if (fork_after_authentication_flag)
+		fork_postauth();
 
 	if (options.use_roaming)
 		request_roaming();
Index: b/ssh_config.5
===================================================================
--- a/ssh_config.5
+++ b/ssh_config.5
@@ -319,6 +319,28 @@
 used for opportunistic connection sharing include
 at least %h, %p, and %r.
 This ensures that shared connections are uniquely identified.
+.It Cm ControlPersist
+When used in conjunction with
+.Cm ControlMaster ,
+specifies that the master connection should remain open
+in the background (waiting for future client connections)
+after the initial client connection has been closed.
+If set to
+.Dq no ,
+then the master connection will not be placed into the background,
+and will close as soon as the initial client connection is closed.
+If set to
+.Dq yes ,
+then the master connection will remain in the background indefinitely
+(until killed or closed via a mechanism such as the
+.Xr ssh 1
+.Dq Fl O No exit
+option).
+If set to a time in seconds, or a time in any of the formats documented in
+.Xr sshd_config 5 ,
+then the backgrounded master connection will automatically terminate
+after it has remained idle (with no client connections) for the
+specified time.
 .It Cm DynamicForward
 Specifies that a TCP port on the local machine be forwarded
 over the secure channel, and the application

Reply to: