openssh bugs - Jul 2007 - [Bug 1330] New: RFE: 'ControlPersist' support -- automatically fork and leave ControlMaster behind as a dæmon

http://bugzilla.mindrot.org/show_bug.cgi?id=1330

           Summary: RFE: 'ControlPersist' support -- automatically fork
and
                    leave ControlMaster behind as a d?mon
           Product: Portable OpenSSH
           Version: 4.6p1
          Platform: Other
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ssh
        AssignedTo: bitbucket at mindrot.org
        ReportedBy: dwmw2 at infradead.org


Created an attachment (id=1319)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1319)
implement boolean ControlPersist option.

Could do with an idle timeout really. Perhaps we should make the
'ControlPersist' option take a number of seconds as an argument?


-- 
Configure bugmail: http://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

http://bugzilla.mindrot.org/show_bug.cgi?id=1330





--- Comment #1 from David Woodhouse <dwmw2 at infradead.org>  2007-07-06
01:31:43 ---
I wonder if it should close stdin/stdout/stderr when it forks too.
Currently, it keeps at least stderr open. This causes cvs to continue
waiting for it indefinitely.


-- 
Configure bugmail: http://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

http://bugzilla.mindrot.org/show_bug.cgi?id=1330


Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org




--- Comment #2 from Damien Miller <djm at mindrot.org>  2007-07-06
14:04:24 ---
Perhaps it would be better to fork prior to connection for
ControlPersist=yes, rather than at the end of the session. E.g.
something like the following:

if (master_socket_exists)
   control_client()
if (control_persist):
   fork()
   if (parent):
       wait_for_master_socket()
       control_client()
   else:
       daemon() /* will take care of detaching from tty, etc. */
       connect_and_establish_master()

Another handy thing might be to make ControlPersist accept a time
value, using misc.c:convtime(), for how long to stick around after the
last active channel has closed.


-- 
Configure bugmail: http://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

http://bugzilla.mindrot.org/show_bug.cgi?id=1330





--- Comment #3 from David Woodhouse <dwmw2 at infradead.org>  2007-07-06
15:48:15 ---
I think I tried precisely that but didn't like it -- you don't want to
detach from the tty before connecting, because then you're forced to
use $SSH_ASKPASS instead of the tty when you need a pass{word,phrase}.

We might be able to fork before connection, but we'd probably want to
detach from the tty only _after_ the authentication is done.


-- 
Configure bugmail: http://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

http://bugzilla.mindrot.org/show_bug.cgi?id=1330


Wout Mertens <wmertens at cisco.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #1319 is|0                           |1
           obsolete|                            |




--- Comment #4 from Wout Mertens <wmertens at cisco.com>  2007-07-10
22:02:33 ---
Created an attachment (id=1322)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1322)
patch against 4.6p1 with some fixes

This patch is against 4.6p1.

It now initializes the flag value, which is needed to be able to turn
it off. If it's not initialized to -1 it's set to some random value and
that means it's on.

It also replaces the daemonizing code with a call to daemon().

It works fine for me. :-)


-- 
Configure bugmail: http://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

http://bugzilla.mindrot.org/show_bug.cgi?id=1330





--- Comment #5 from Wout Mertens <wmertens at cisco.com>  2007-07-23
12:52:39 ---
Created an attachment (id=1331)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1331)
Persist and idletimeout combined

This combines the controlpersist keyword with the controltimeout and
idletimeout keywords.

See bug #1338 for more info on the idletimeout patch.

With this patch, you can ssh/scp/sftp to a host, interact with it, and
then when you disconnect, the connection will daemonize and stay alive
until you haven't used the connection for a predetermined time.

Example:

ssh -o ControlPath=/tmp/ssh-$USER-%h:%p -o ControlMaster=auto -o
ControlPersist=yes -o ControlTimeout=5m someserver


I cleaned up the patch from #1338 some more, now that I understand the
code better. I think the patch is now done. 


-- 
Configure bugmail: http://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

http://bugzilla.mindrot.org/show_bug.cgi?id=1330





--- Comment #6 from Damien Miller <djm at mindrot.org>  2007-07-23
14:29:27 ---
(From update of attachment 1331)
Some comments on the diff:
>diff -ru openssh-4.6p1-orig/clientloop.c
openssh-4.6p1-idletimeout/clientloop.c
>--- openssh-4.6p1-orig/clientloop.c	2007-07-10 12:48:22.000000000 +0200
>+++ openssh-4.6p1-idletimeout/clientloop.c	2007-07-23 03:51:13.000000000
+0200
>-	if (options.server_alive_interval == 0 || !compat20)
>-		tvp = NULL;
>-	else {
>-		tv.tv_sec = options.server_alive_interval;
>-		tv.tv_usec = 0;
>-		tvp = &tv;
>-	}
>-	ret = select((*maxfdp)+1, *readsetp, *writesetp, NULL, tvp);
>+	ret = packet_select((*maxfdp)+1, *readsetp, *writesetp, NULL,
>+			compat20 ? options.server_alive_interval : 0);
I think it would be more clear to create a calculate_select_timeout()
function (or somesuch) that returns a timeval* rather than making a new
API around select()
>+	} else if (ret == 0) {
>+		if (packet_idle_timedout()) {
>+			char buf[100];
>+			snprintf(buf, sizeof buf, "Idle timeout.\r\n");
>+			buffer_append(&stderr_buffer, buf, strlen(buf));
>+			quit_pending = 1;
Why not use logit() or verbose() here?
> static void
>@@ -1343,8 +1344,13 @@
> client_channel_closed(int id, void *arg)
> {
> 	channel_cancel_cleanup(id);
>-	session_closed = 1;
> 	leave_raw_mode();
>+	if (options.control_persist && options.control_path != NULL
&& control_fd != -1) {
>+		packet_set_idle_timeout(options.control_timeout);
>+		daemon(0,0);
>+		return;
As mentioned in comment #2, I'd prefer to see the ControlPersist master
background itself when the connection is established (but after
authentication - e.g. as the -f option does it) and have the
foregrounded ssh process become a client of it.

>diff -ru openssh-4.6p1-orig/readconf.c openssh-4.6p1-idletimeout/readconf.c
>--- openssh-4.6p1-orig/readconf.c	2007-07-10 12:48:22.000000000 +0200
>+++ openssh-4.6p1-idletimeout/readconf.c	2007-07-20 21:35:15.000000000 +0200
>@@ -221,11 +221,14 @@
> 	{ "sendenv", oSendEnv },
> 	{ "controlpath", oControlPath },
> 	{ "controlmaster", oControlMaster },
>+	{ "controlpersist", oControlPersist },
>+	{ "controltimeout", oControlTimeout },
These would be better off as a single ControlPersist option that
accepted "yes"/"true", "no"/"false" or a
timeout parsed using
misc.c:convtime()

E.g. "ControlPersist 5m"
>+	{ "idletimeout", oIdleTimeout },
IdleTimeout is a separate issue, it can stay in here for now but it
will need to be removed to a separate patch before the ControlPersist
stuff goes in.


-- 
Configure bugmail: http://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

http://bugzilla.mindrot.org/show_bug.cgi?id=1330





--- Comment #7 from Damien Miller <djm at mindrot.org>  2007-07-23
15:26:10 ---
(In reply to comment #6)
> As mentioned in comment #2, I'd prefer to see the ControlPersist master
> background itself when the connection is established (but after
> authentication - e.g. as the -f option does it) and have the
> foregrounded ssh process become a client of it.
See ssh.c:ssh_session2() - at the point where daemon() is called in
this function, the control listener has already been established so it
would be ready for connection by the foregrounded control client.


-- 
Configure bugmail: http://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

http://bugzilla.mindrot.org/show_bug.cgi?id=1330





--- Comment #8 from Alan Barrett <apb at cequrux.com>  2007-08-11 07:26:36
---
Created an attachment (id=1338)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1338)
patch for three way ControlPersist=yes/no/timeout option.

I re-wrote the ControlPersist patch to do what I want.  Differences
from other versions include:

1. The control master backgrounds itself immediately after
authentication, before running the shell or command for the first
client connection.  The first client connection is run as a client of
the newly-backgrounded control master.

2. The ControlPersist option takes a three-way yes/no/timeout
parameter, instead of using a separate ControlTimeeout parameter.

3. The ControlPersist timeout doesn't start ticking until the last
client connection is closed.

4. This patch does not add an IdleTimeout parameter or any kind of idle
timeout handling for normal connections.


-- 
Configure bugmail: http://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

http://bugzilla.mindrot.org/show_bug.cgi?id=1330


Alan Barrett <apb at cequrux.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         OS/Version|Linux                       |All
                 CC|                            |apb at cequrux.com




-- 
Configure bugmail: http://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330


Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement




-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330





--- Comment #9 from David Woodhouse <dwmw2 at infradead.org>  2008-03-06
23:49:06 ---
Alan's patch works for me, and I believe it addresses all the requests
in comment #6?

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330





--- Comment #10 from Martin Ling <martin-mindrot-bugzilla at earth.li> 
2009-02-22 13:51:25 ---
Created an attachment (id=1605)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1605)
Updated patch for 5.1p1

Attached is an updated version of Alan's patch from comment #8, for
5.1p1.

I think this needs to be revised slightly further though, to allow a
timeout of zero - i.e. close the connection immediately when the last
client exits.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330


Martin Ling <martin-mindrot-bugzilla at earth.li> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|4.6p1                       |-current
           Platform|Other                       |All
                 CC|                            |martin-mindrot-bugzilla at ear
                   |                            |th.li




-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330


Tomas Mraz <t8m at centrum.cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|RFE: 'ControlPersist'       |RFE:
'ControlPersist'
                   |support -- automatically    |support -- automatically
                   |fork and leave              |fork and leave
                   |ControlMaster behind as a   |ControlMaster behind as a
                   |d?mon                       |daemon
                 CC|                            |t8m at centrum.cz




-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #11 from David Woodhouse <dwmw2 at infradead.org>  ---
(In reply to comment #10)
> I think this needs to be revised slightly further though, to allow a
> timeout of zero - i.e. close the connection immediately when the last
> client exits.
That's easily done...

--- readconf.c~    2010-06-05 03:08:33.000000000 +0100
+++ readconf.c    2010-06-05 03:20:37.000000000 +0100
@@ -906,7 +906,7 @@ parse_int:
             value = 0;
         else if (strcmp(arg, "yes") == 0 || strcmp(arg,
"true") == 0)
             value = 1;
-        else if ((value2 = convtime(arg)) > 0)
+        else if ((value2 = convtime(arg)) >= 0)
             value = 1;
         else
             fatal("%.200s line %d: Bad ControlPersist argument.",

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

David Woodhouse <dwmw2 at infradead.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1322|0                           |1
        is obsolete|                            |
   Attachment #1331|0                           |1
        is obsolete|                            |
   Attachment #1338|0                           |1
        is obsolete|                            |
   Attachment #1605|0                           |1
        is obsolete|                            |

--- Comment #12 from David Woodhouse <dwmw2 at infradead.org>  ---
Created attachment 1858
  --> https://bugzilla.mindrot.org/attachment.cgi?id=1858
Updated controlpersist patch for 5.5p1

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #13 from Martin Ling <martin-mindrot-bugzilla at earth.li> 
---
David,

That change to readconf.c doesn't seem to have the desired effect. With
"ControlPersist 0", the master backgrounds forever.

That said, if you tested that behaviour on 5.5p1 then it's probably my
fault - for convenience I'm using your 5.5p1 version of the patch
applied to the source of the current 5.3p1 Ubuntu package - it applied
okay there with one unrelated conflict which I fixed manually.

I'm also wondering if it would be clearer to split the configuration of
this into three options:

ControlTransfer (boolean):

  - False: The ssh invocation which opens the connection will not
terminate until all sessions have closed (as per previous behaviour).

  - True: The ssh invocation which opens the connection will terminate,
but control of the connection will be transferred to another process if
other sessions are using it.

ControlPersist (boolean):

  - False: The connection will be closed when the last session exits.

  - True: The connection will remain open when there are no sessions.

ControlPersistTimeout (time)

  - Zero: The connection will remain open forever (or until lost).

  - Non-zero: The connection will be closed when it has not been used
for the specified time.

With this arrangment, those like me who just want transparent
connection reuse without the complication of the first session becoming
uncloseable, can just set "ControlTransfer yes". Those who want
persistence (arguably a higher risk option) can enable it specifically,
with or without a timeout.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #14 from David Woodhouse <dwmw2 at infradead.org>  ---
(In reply to comment #13)
> David,
> 
> That change to readconf.c doesn't seem to have the desired effect. 
> With "ControlPersist 0", the master backgrounds forever.
It works in 5.5p1 because in 5.5p1 the 'master' is actually
backgrounded and even the 'first' connection is just behaving the same
as any later client would.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #15 from Martin Ling <martin-mindrot-bugzilla at earth.li> 
---
(In reply to comment #14)
> 
> It works in 5.5p1 because in 5.5p1 the 'master' is actually
> backgrounded and even the 'first' connection is just behaving the
same
> as any later client would.
Are you sure? I've just tried again using your patch on the original
5.5p1 source and I'm having the same problem.

I have these settings in ~/.ssh/config:

ControlMaster auto
ControlPath ~/.ssh/control/%h-%p-%r
ControlPersist 0

The steps I'm taking are:

- Check no ssh processes are running.
- Start one session to a server, which opens the connection.
- Start a second session to the same server, reusing the connection.
- Close the first session, which returns me to the shell.
- Close the second session, also returning me to the shell.

After this I see:

- One ssh process is still running in the background.
- Starting a third session again reuses the connection.

What I expect is:

- No ssh processes running.
- Starting a third session initiates a new connection.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #16 from David Woodhouse <dwmw2 at infradead.org>  ---
I _was_ sure, and had even tested it again this morning... but now you
seem to be right.

Actually making it work right tends to make the master disconnect
immediately even before the first client has connected, which isn't
ideal. Will look at it some more...

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #17 from Alan Barrett <apb at cequrux.com>  ---
(In reply to comment #13)
> I'm also wondering if it would be clearer to split the configuration
> of this into three options:
That seems more confusing to me, and I don't see what the advantage
would be.  As far as I can tell, everything your system of 3
variables could accomplish can also be accomplished with the existing
ControlPersist = yes/no/timespec option.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #18 from Alan Barrett <apb at cequrux.com>  ---
(In reply to comment #10)
> I think this needs to be revised slightly further though, to allow a
> timeout of zero - i.e. close the connection immediately when the last
> client exits.
It's been a long time since I looked at the code, but setting
ControlPersist=yes (as opposed to setting it to a time) was intended to
close the connection immediately when the last client disconnected.  I
think it's fine for a timeout of 0 to do the same thing, but others
might be confused by ControlPersist=no and ControlPersist=0 having
different meanings.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #19 from Martin Ling <martin-mindrot-bugzilla at earth.li> 
---
(In reply to comment #17)
>
> That seems more confusing to me, and I don't see what the advantage
> would be.  As far as I can tell, everything your system of 3
> variables could accomplish can also be accomplished with the existing
> ControlPersist = yes/no/timespec option.
I wasn't really intending to offer more possibilities, rather just
avoiding overloading one option with multiple meanings. As you point
out, having 'no' and '0' mean different things is confusing.

Maybe there's a good two-option compromise.

Do we agree on the use cases that need to be supported? I think they
are:

1. master session held open till slaves exit (original behaviour)

2. master backgrounded on first session exit, closes with last slave.

3. master backgrounded on first session exit, closes after timeout.

4. master backgrounded on first session exit, never closes.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #20 from Martin Ling <martin-mindrot-bugzilla at earth.li> 
---
(In reply to comment #18)
> 
> It's been a long time since I looked at the code, but setting
> ControlPersist=yes (as opposed to setting it to a time) was intended to
> close the connection immediately when the last client disconnected.  I
> think it's fine for a timeout of 0 to do the same thing, but others
> might be confused by ControlPersist=no and ControlPersist=0 having
> different meanings.
At the moment, ControlPersist=yes makes the connection remain forever.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #21 from Alan Barrett <apb at cequrux.com>  ---
(In reply to comment #20)
> At the moment, ControlPersist=yes makes the connection remain forever.
Oh, yes, you are right.  It even says that in my version of the
ssh_config man page.  I was confused by the code setting
control_persist_timeout=0 in the "yes" case.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #22 from Alan Barrett <apb at cequrux.com>  ---
(In reply to comment #19)
> Maybe there's a good two-option compromise.
> 
> Do we agree on the use cases that need to be supported?
I agree on the use cases.

I still prefer a single multi-way option, but perhaps somebody can
come up with better names for the option and its keyword values?

Here's my understanding of the current situation (modulo bugs):
> 1. master session held open till slaves exit (original behaviour)
ControlPersist = no/off
> 2. master backgrounded on first session exit, closes with last slave.
ControlPersist = 0
> 3. master backgrounded on first session exit, closes after timeout.
ControlPersist = <time>
> 4. master backgrounded on first session exit, never closes.
ControlPersist = yes/true

I think that the "0" and the "yes/true" cases would benefit
from
somebody inventing new keywords.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #23 from David Woodhouse <dwmw2 at infradead.org>  ---
I don't care too much about renaming the options -- I think 'yes'
and
'no' are fine. We just need to fix the zero timeout... for those who
_really_ care about that extra one second and can't just set it to 1.

I'm not going to get round to that today. It's the 21th century and I
just can't find the patience today for juggling multiple outstanding
patches on a project which is still stored in CVS and not git.

Feel free to provide incremental patches and I'll try to look at them.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #24 from Damien Miller <djm at mindrot.org>  ---
(In reply to comment #19)
> Do we agree on the use cases that need to be supported? I think they
> are:
> 
> 1. master session held open till slaves exit (original behaviour)
ControlPersist=no/false (default)
> 3. master backgrounded on first session exit, closes after timeout.
ControlPersist=timespec (use convtime() to parse it)
> 2. master backgrounded on first session exit, closes with last slave.
ControlPersist=0 (special case of (3) )
> 4. master backgrounded on first session exit, never closes.
ControlPersist=yes/true/till-the-cows-come-home

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #25 from Martin Ling <martin-mindrot-bugzilla at earth.li> 
---
Alan/Damien: Yes, this is how the current option supports those cases.
My remaining concern is with the strange difference that exists between
"no" and "0". My suggestion of a "ControlTransfer"
option was intended
to make this distinction clearer.

If everyone else is happy with the existing option then I'm fine with
that.

If not, here's a two-option alternative for discussion:

ControlTransfer (default no):

  - no: The ssh invocation which opens the connection will not
terminate until all sessions have closed.

  - yes: The ssh invocation which opens the connection will terminate,
but control of the connection will be transferred to another process if
other sessions are using it.

ControlPersist (default 0):

  - <time>: The connection will be closed after the specified time when
not in use.

  - "forever": The connection will not be closed.

There might be cases in which
ControlTransfer=no/ControlPersist=<nonzero> could be useful, e.g. in a
script designed to just keep a connection open for reuse.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #26 from Damien Miller <djm at mindrot.org>  ---
Created attachment 1897
  --> https://bugzilla.mindrot.org/attachment.cgi?id=1897
/home/djm/ssh-controlpersist1.diff

revised patch

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1858|0                           |1
        is obsolete|                            |

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |1708

--- Comment #27 from Damien Miller <djm at mindrot.org>  ---
I think the previous patch got the ExitOnForwardFailure case wrong
(though this already had a bug that I just fixed in -current), and
possibly the "ssh -f" case too. I'm pretty sure the revised diff
fixes
these. 

The main change is to defer the backgrounding when ExitOnForwardFailure
is set until after the confirm messages for -R forwards have been
received. This means that we need to proceed into the mainloop before
we background and set up a new foreground client, and this in turn
means that we need to save and restore the flags
(tty/no-tty/stdin-null) that we must clobber before we hit the
mainloop.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|RFE: 'ControlPersist'       |ControlPersist: fork
and
                   |support -- automatically    |leave ControlMaster behind
                   |fork and leave              |as a daemon
                   |ControlMaster behind as a   |
                   |daemon                      |

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #28 from Damien Miller <djm at mindrot.org>  ---
Patch applied - this will be in openssh-5.6. Thanks to everyone who
worked on this patch :)

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

--- Comment #29 from David Woodhouse <dwmw2 at infradead.org>  ---
Thanks. Now that we have this, it would be really useful if we could
also fix bug #1329.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1330

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #30 from Darren Tucker <dtucker at zip.com.au>  ---
With the release of OpenSSH 5.6p1 this bug is now considered closed. 
If you have further problems please reopen or file a new bug as
appropriate.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.