dbclient half-close?

[Matt Johnston]

Hi Catalin,

Sorry for taking so long to get back on this. I've committed
it now, slightly changed the check_close case since there's
no point waiting to flush data to an exited child.

Cheers,
Matt


On Sun, Aug 25, 2013 at 03:37:38AM -0400, Catalin Patulea wrote:
> ping?
> 
> On Fri, Jul 26, 2013 at 9:31 PM, Matt Johnston <matt at ucc.asn.au> wrote:
> > Hi Catalin,
> >
> > Thanks for looking at that - the last patch looks sensible, I'll give it a
> > good test. There are a lot of subtle scenarios in channel closing (and
> > variations between OSes).
> >
> > Cheers,
> > Matt
> >
> > Catalin Patulea <cat at vv.carleton.ca> wrote:
> >>Hm, that broke channel-close-by-child-exit. One more try, where we
> >>check for the child exiting and close writefd as a result. If writefd
> >>is the last remaining open pipe to the child, then we also close the
> >>channel as a whole.
> >>
> >>On Wed, Jul 17, 2013 at 3:25 PM, Catalin Patulea <cat at vv.carleton.ca>
> >>wrote:
> >>> Attached patch should fix both, and use hard tabs so should apply
> >>easily.
> >>>
> >>> Rather than replacing readfd with writefd, *both* are checked for
> >>> FD_CLOSED before closing the entire channel. Then each direction can
> >>> be initially closed independently.
> >>>
> >>> On Wed, Jul 17, 2013 at 7:57 PM, Catalin Patulea <cat at vv.carleton.ca>
> >>wrote:
> >>>> dbclient handling of remote EOF/local not at EOF also appears
> >>broken:
> >>>>
> >>>> openssh:
> >>>> $ ssh -v root at 1.2.3.4 'exec cat >/dev/null 2>/dev/null'
> >>>> [...]
> >>>> debug1: Entering interactive session.
> >>>> debug1: Sending command: exec cat >/dev/null 2>/dev/null
> >>>> # remote has sent EOF by virtue of &>/dev/null, but local->remote
> >>>> # direction still active
> >>>> foo # local sends
> >>>> bar
> >>>> ^D # local EOF, channel tears down
> >>>> debug1: client_input_channel_req: channel 1 rtype exit-status reply
> >>0
> >>>> debug1: channel 1: free: client-session, nchannels 2
> >>>> <exit>
> >>>>
> >>>> Note 'exec' is required to replace shell and prevent it from holding
> >>a
> >>>> ref to the stdout pipe.
> >>>>
> >>>> dropbear:
> >>>> $ ./dbclient -v root at lun.nanobit.org 'exec cat >/dev/null
> >>2>/dev/null'
> >>>> [...]
> >>>> TRACE (31787) 1374079900.330457: process_packet: packet type = 96,
> >>len 5
> >>>> TRACE (31787) 1374079900.330499: enter recv_msg_channel_eof
> >>>> TRACE (31787) 1374079900.330513: CLOSE some fd 1
> >>>> TRACE (31787) 1374079900.330526: sending close, readfd is closed
> >>>> # remote EOF should *not* cause entire channel teardown;
> >>>> # local may still have something to write ("foo bar" from openssh
> >>>> # example).
> >>>> TRACE (31787) 1374079900.330537: enter send_msg_channel_close
> >>0x1f78660
> >>>> TRACE (31787) 1374079900.330549: enter cli_tty_cleanup
> >>>> TRACE (31787) 1374079900.330560: leave cli_tty_cleanup: not in raw
> >>mode
> >>>> TRACE (31787) 1374079900.330606: CLOSE some fd 0
> >>>> TRACE (31787) 1374079900.330618: CLOSE some fd 2
> >>>> <exit>
> >>>>
> >>>>
> >>>> On Sat, Jul 13, 2013 at 12:51 PM, Catalin Patulea
> >><cat at vv.carleton.ca> wrote:
> >>>>> Maybe the check on common-channel.c:338 should be against writefd
> >>>>> instead of readfd? This would get set by
> >>>>> close_chan_fd(channel->writefd) once recv_eof happens.
> >>>>>
> >>>>> This patch indeed causes 'foo' to surface after input EOF:
> >>>>>
> >>>>> diff -r 69cb561cc4c4 common-channel.c
> >>>>> --- a/common-channel.c Sat Jul 13 11:53:24 2013 +0300
> >>>>> +++ b/common-channel.c Sat Jul 13 12:50:41 2013 +0300
> >>>>> @@ -335,7 +335,7 @@
> >>>>> }
> >>>>>
> >>>>> /* And if we can't receive any more data from them either, close
> >>up */
> >>>>> - if (channel->readfd == FD_CLOSED
> >>>>> + if (channel->writefd == FD_CLOSED
> >>>>> && (ERRFD_IS_WRITE(channel) || channel->errfd == FD_CLOSED)
> >>>>> && !channel->sent_close
> >>>>> && close_allowed
> >>>>>
> >>>>> On Sat, Jul 13, 2013 at 12:31 PM, Catalin Patulea
> >><cat at vv.carleton.ca> wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> I'm seeing a difference in how dbclient handles EOF on input
> >>compared
> >>>>>> to openssh client. openssh client propagates input EOF to the
> >>remote
> >>>>>> command, but continues pumping command stdout. dbclient seems to
> >>abort
> >>>>>> before flushing the stdout buffer.
> >>>>>>
> >>>>>> In the following examples, 1.2.3.4 is an openwrt router running
> >>>>>> dropbear server. The remote command is designed to wait for EOF,
> >>then
> >>>>>> output something to stdout.
> >>>>>>
> >>>>>> openssh client:
> >>>>>> $ ssh root at 1.2.3.4 'cat; echo foo'
> >>>>>> ^D
> >>>>>> foo
> >>>>>>
> >>>>>> dbclient:
> >>>>>> $ ./dbclient root at 1.2.3.4 'cat; echo foo'
> >>>>>> ^D
> >>>>>> <no output>
> >>>>>>
> >>>>>> I build dropbear with DEBUG_TRACE and these are the last few
> >>lines:
> >>>>>> TRACE (...): empty queue dequeing
> >>>>>> ^D
> >>>>>> TRACE (...): send normal readfd
> >>>>>> TRACE (...): enter send_msg_channel_data
> >>>>>> TRACE (...): enter send_msg_channel_data isextended 0 fd 0
> >>>>>> TRACE (...): maxlen 16375
> >>>>>> TRACE (...): CLOSE some fd 0
> >>>>>> TRACE (...): leave send_msg_channel_data: len 0 read err 17 or EOF
> >>for fd 0
> >>>>>> TRACE (...): enter send_msg_channel_eof
> >>>>>> TRACE (...): leave send_msg_channel_eof
> >>>>>> TRACE (...): sending close, readfd is closed
> >>>>>> TRACE (...): enter send_msg_channel_close 0xcbfdc0
> >>>>>> TRACE (...): enter cli_tty_cleanup
> >>>>>> TRACE (...): leave cli_tty_cleanup: not in raw mode
> >>>>>> TRACE (...): CLOSE some fd -1
> >>>>>> TRACE (...): CLOSE some fd 2
> >>>>>>
> >>>>>> I tried reading through the relevant sections of channel-common.c
> >>but
> >>>>>> I could use some guidance/background. Is this behaviour
> >>intentional?
> >>>>>>
> >>>>>> Catalin