Mailing List Archive: OpenSSH: Dev

Re: ssh localhost yes | true (follow up)

Index | Next | Previous | View Threaded

djm at mindrot

Apr 23, 2009, 6:38 AM

Post #1 of 6 (1085 views)
Permalink

Re: ssh localhost yes \| true (follow up)

On Thu, 23 Apr 2009, Kyle wrote:

> I filed bug 6810722 with Apple and sent them a patch that makes the half-close
> fix work again without causing problems with shells (like Apple's bash that
> attempt to detect when they're being run by a remote shell daemon such as sshd
> by apparently checking standard input to see if it's a socket).
>
> On Apr 21, 2009, at 01:27, Kyle McKay wrote:
> > On Apr 20, 2009, at 23:53, Damien Miller wrote:
> > > There may be differences depending on whether
> > > Apple has modified their sshd to avoid using pipes (thwarting the
> > > half-close fix in the process), but you will need to post a full debug
> > > log from the server to tell.
> > >
> > > -d
> >
> > Yup, you're right. Apple has disabled the USE_PIPES define in their source
> > code.
>
>
> FYI, patch is below.

I think it violates some assumptions we make in channels.c to mix
socketpairs and pipes like this, but I have to check.

-d
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

markus.r.friedl at arcor

Apr 23, 2009, 6:43 AM

Post #2 of 6 (1024 views)
Permalink

Re: ssh localhost yes \| true (follow up) [In reply to]

On Thu, Apr 23, 2009 at 11:38:03PM +1000, Damien Miller wrote:
> I think it violates some assumptions we make in channels.c to mix
> socketpairs and pipes like this, but I have to check.

yes, it violates assumptions.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

mackyle at gmail

Apr 23, 2009, 12:30 PM

Post #3 of 6 (1017 views)
Permalink

Re: ssh localhost yes \| true (follow up) [In reply to]

On Apr 23, 2009, at 06:43, Markus Friedl wrote:
> On Thu, Apr 23, 2009 at 11:38:03PM +1000, Damien Miller wrote:
>> I think it violates some assumptions we make in channels.c to mix
>> socketpairs and pipes like this, but I have to check.
>
> yes, it violates assumptions.

I'm running with the changes and haven't noticed any problems.

scp is working, X tunnels are working, port forwarding is working,
bash is happy (ssh localhost printenv shows it's running ~/.bashrc)
the half-close fix is working (ssh localhost yes | true doesn't hang).

channels.c clearly works with pipes or sockets, it doesn't have any
tests of USE_PIPES in it, it's not calling fstat and I don't see any
tests of S_IFSOCK or S_IFIFO so I'm unclear on how it would be able to
tell the difference between 3 separate pairs of pipes (normal
USE_PIPES case), 1 shared socket pair + 1 separate socket pair
(normal !USE_PIPES case) and 1 socket pair + 2 pairs of pipes.

Of course I'm looking at the version of channels.c that Apple's using:

http://www.opensource.apple.com/darwinsource/10.5.6/OpenSSH-95.1.5/openssh/channels.c

which is "channels.c,v 1.286 2008/07/16 11:52:19 djm" plus an Apple
patch, so maybe there's newer version of channels.c that Apple's not
using that is sensitive to the mixed combination?

Is there something else I should be looking at/testing?

Kyle

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

djm at mindrot

Apr 23, 2009, 2:54 PM

Post #4 of 6 (1015 views)
Permalink

Re: ssh localhost yes \| true (follow up) [In reply to]

On Thu, 23 Apr 2009, Kyle McKay wrote:

> On Apr 23, 2009, at 06:43, Markus Friedl wrote:
> > On Thu, Apr 23, 2009 at 11:38:03PM +1000, Damien Miller wrote:
> >> I think it violates some assumptions we make in channels.c to mix
> >> socketpairs and pipes like this, but I have to check.
> >
> > yes, it violates assumptions.
>
> I'm running with the changes and haven't noticed any problems.
>
> scp is working, X tunnels are working, port forwarding is working,
> bash is happy (ssh localhost printenv shows it's running ~/.bashrc)
> the half-close fix is working (ssh localhost yes | true doesn't hang).
>
> channels.c clearly works with pipes or sockets, it doesn't have any
> tests of USE_PIPES in it, it's not calling fstat and I don't see any
> tests of S_IFSOCK or S_IFIFO so I'm unclear on how it would be able to
> tell the difference between 3 separate pairs of pipes (normal
> USE_PIPES case), 1 shared socket pair + 1 separate socket pair
> (normal !USE_PIPES case) and 1 socket pair + 2 pairs of pipes.

The test is in channel_register_fds(), look for c->sock.

Why not fix the bug in bash instead of putting weird hacks in ssh?

-d
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

mackyle at gmail

Apr 23, 2009, 4:32 PM

Post #5 of 6 (1005 views)
Permalink

Re: ssh localhost yes \| true (follow up) [In reply to]

On Apr 23, 2009, at 14:54, Damien Miller wrote:
> On Thu, 23 Apr 2009, Kyle McKay wrote:
>
>> On Apr 23, 2009, at 06:43, Markus Friedl wrote:
>>> On Thu, Apr 23, 2009 at 11:38:03PM +1000, Damien Miller wrote:
>>>> I think it violates some assumptions we make in channels.c to mix
>>>> socketpairs and pipes like this, but I have to check.
>>>
>>> yes, it violates assumptions.
>>
>> I'm running with the changes and haven't noticed any problems.
>>
>> scp is working, X tunnels are working, port forwarding is working,
>> bash is happy (ssh localhost printenv shows it's running ~/.bashrc)
>> the half-close fix is working (ssh localhost yes | true doesn't
>> hang).
>>
>> channels.c clearly works with pipes or sockets, it doesn't have any
>> tests of USE_PIPES in it, it's not calling fstat and I don't see any
>> tests of S_IFSOCK or S_IFIFO so I'm unclear on how it would be able
>> to
>> tell the difference between 3 separate pairs of pipes (normal
>> USE_PIPES case), 1 shared socket pair + 1 separate socket pair
>> (normal !USE_PIPES case) and 1 socket pair + 2 pairs of pipes.
>
> The test is in channel_register_fds(), look for c->sock.

Thanks for pointing me to that.

Looks like you get some extra behavior if rfd == wfd, but you're not
getting that behavior normally when USE_PIPES is always defined in
session.c, so nothing obvious jumps out at me as breakage-waiting-to-
happen with the patch applied.

> Why not fix the bug in bash instead of putting weird hacks in ssh?
>
> -d

Apparently bash isn't the only program to test that and sending Apple
a bunch of patches for a bunch of different programs is likely to
delay getting any fix accepted (be lucky if they take any fix at all
and issue an official update containing the fix within the next 6
months).

In fact, Ubuntu bash gets it right, ("ssh localhost yes | true" does
not hang while "ssh localhost printenv" indicates ~/.bashrc is being
run) so that suggests Apple's bash sources are behind the times or
also contain bug-inducing patches or there is some other Darwin-
specific behavior going on that breaks things.

Why not enhance ssh to officially support a socket/pipe combination?

Kyle
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

djm at mindrot

Apr 23, 2009, 5:43 PM

Post #6 of 6 (1014 views)
Permalink

Re: ssh localhost yes \| true (follow up) [In reply to]

On Thu, 23 Apr 2009, Kyle McKay wrote:

> > > tests of S_IFSOCK or S_IFIFO so I'm unclear on how it would be able to
> > > tell the difference between 3 separate pairs of pipes (normal
> > > USE_PIPES case), 1 shared socket pair + 1 separate socket pair
> > > (normal !USE_PIPES case) and 1 socket pair + 2 pairs of pipes.
> >
> > The test is in channel_register_fds(), look for c->sock.
>
> Thanks for pointing me to that.
>
> Looks like you get some extra behavior if rfd == wfd, but you're not getting
> that behavior normally when USE_PIPES is always defined in session.c, so
> nothing obvious jumps out at me as breakage-waiting-to-happen with the patch
> applied.

channel code bugs rarely manifest in obvious ways, and they are difficult
to debug when they do. I can't say for certain that mixing socketpairs and
pipes will or won't cause problems, but I don't think it is a good path to
go down if it is just papering over other problems.

> Apparently bash isn't the only program to test that and sending Apple a bunch
> of patches for a bunch of different programs is likely to delay getting any
> fix accepted (be lucky if they take any fix at all and issue an official
> update containing the fix within the next 6 months).
>
> In fact, Ubuntu bash gets it right, ("ssh localhost yes | true" does not hang
> while "ssh localhost printenv" indicates ~/.bashrc is being run) so that
> suggests Apple's bash sources are behind the times or also contain
> bug-inducing patches or there is some other Darwin-specific behavior going on
> that breaks things.
>
> Why not enhance ssh to officially support a socket/pipe combination?

I don't think it is an "enhancement" to work around a bug that will be
fixed in a couple of months. It is just added complexity that we will have
to maintain forever.

-d
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads