Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: OpenSSH: Dev

openssh PTY allocation

 

 

OpenSSH dev RSS feed   Index | Next | Previous | View Threaded


morty at frakir

Jul 23, 2011, 11:47 AM

Post #1 of 21 (1834 views)
Permalink
openssh PTY allocation

We recently upgraded to openss 5.8p2 from a somewhat older version.
This broke openssh login to ScreenOS devices. These devices don't
support PTY allocation. Apparently, ssh now reacts to PTY allocation
failure by failing the login. This is a change from the previous
behavior. The simple workaround is ssh -T $device.

I see in the ChangeLog that some device would hang with PTY allocation
disabled. So apparently, we have mutually exclusive bad behaviors.
From the ChangeLog:

- djm [at] cvs 2010/04/10 02:08:44
[clientloop.c]
bz#1698: kill channel when pty allocation requests fail. Fixed
stuck client if the server refuses pty allocation.
ok dtucker@ "think so" markus@

There don't seem to be any ssh_config options that control PTY
allocation behavior. Any way we can get some options? There should
be at least one, maybe two ssh_config options to control PTY behavior:
one to control the PTY failure behavior, something like
ExitOnPTYFailure yes|no|no-warn, and one to give the equivalent of -T
or -t, i.e. something like PTYEnable: yes|no|force-yes|yes-if-login.

Note that, IME, for the ChangeLog case, that is a device that normally
allows PTY creation and the PTY fails, the result looks like a hang
but really isn't. Rather, what you have is a non-interactive shell.
The shell works but doesn't display a prompt. So, for example, back
under Solaris 2.6, if you allocated all 128 default PTYs and then
login with ssh, the login will just sit there after login without
displaying a prompt. But it really isn't hung -- you can run
non-interactive commands and see the responses.

Thanks.

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


dtucker at zip

Jul 23, 2011, 6:24 PM

Post #2 of 21 (1819 views)
Permalink
Re: openssh PTY allocation [In reply to]

On Sun, Jul 24, 2011 at 4:47 AM, Mordechai T. Abzug <morty [at] frakir> wrote:
> We recently upgraded to openss 5.8p2 from a somewhat older version.
> This broke openssh login to ScreenOS devices.  These devices don't
> support PTY allocation.  Apparently, ssh now reacts to PTY allocation
> failure by failing the login.  This is a change from the previous
> behavior.  The simple workaround is ssh -T $device.
[...]
> There don't seem to be any ssh_config options that control PTY
> allocation behavior.  Any way we can get some options?  There should
> be at least one, maybe two ssh_config options to control PTY behavior:
> one to control the PTY failure behavior, something like
> ExitOnPTYFailure yes|no|no-warn, and one to give the equivalent of -T
> or -t, i.e. something like PTYEnable: yes|no|force-yes|yes-if-login.

Damien already added a config option ("RequestTTY") but there hasn't
been a release since then (this problem didn't become apparent until
after the 5.8 release when people used it with the problem devices).

RequestTTY
Specifies whether to request a pseudo-tty for the session. The
argument may be one of: ``no'' (never request a TTY), ``yes''
(always request a TTY when standard input is a TTY), ``force''
(always request a TTY) or ``auto'' (request a TTY when opening a
login session). This option mirrors the -t and -T flags for
ssh(1).

--
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


morty at frakir

Jul 23, 2011, 8:16 PM

Post #3 of 21 (1818 views)
Permalink
Re: openssh PTY allocation [In reply to]

On Sun, Jul 24, 2011 at 11:24:45AM +1000, Darren Tucker wrote:

> Damien already added a config option ("RequestTTY") but there hasn't
> been a release since then (this problem didn't become apparent until
> after the 5.8 release when people used it with the problem devices).
>
> RequestTTY
> Specifies whether to request a pseudo-tty for the session. The
> argument may be one of: ``no'' (never request a TTY), ``yes''
> (always request a TTY when standard input is a TTY), ``force''
> (always request a TTY) or ``auto'' (request a TTY when opening a
> login session). This option mirrors the -t and -T flags for
> ssh(1).

That sounds great. It is indeed one of the two features I asked for.
But I think we still need a second option, in the event that a TTY/PTY
is requested and fails, to decide what should happen. Something like
ExitOnTTYFailure yes|no|no-warn. That way, sites where a TTY/PTY
error is normal for many devices and not catastrophic for other
devices don't need to configure RequestTTY for each device. Instead,
we can just do a:

Host *
ExitOnTTYFailure no-warn
RequestTTY auto

The above corresponds to the behavior for openssh and ssh.com's ssh1
for many years.

Similarly, sites where a TTY/PTY error is always a problem can just
set:

Host *
ExitOnTTYFailure yes
RequestTTY auto

Only sites with a mixture of devices, some that don't support PTYs and
some where PTY allocation failure is catastrophic, will need to do
per-device RequestTTY. I suspect that the last case will be very
rare. So most of us will be happy with global ExitOnTTYFailure
settings and RequestTTY auto.

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


morty at frakir

Jul 26, 2011, 8:01 PM

Post #4 of 21 (1810 views)
Permalink
Re: openssh PTY allocation [In reply to]

[snip]

We continue to have issues with login to ScreenOS devices.
Apparently, the simple workaround of ssh -T $device breaks command
editing.

We're also have problems with scp to ScreenOS devices. Apparently,
ScreenOS doesn't support "--" option termination.

The following patch seems to "fix" both issues:

diff -ur openssh-5.8p2-orig//clientloop.c openssh-5.8p2-morty/clientloop.c
--- openssh-5.8p2-orig//clientloop.c Sun Jan 16 12:18:35 2011
+++ openssh-5.8p2-morty/clientloop.c Tue Jul 26 16:55:17 2011
@@ -1982,7 +1982,7 @@
memset(&ws, 0, sizeof(ws));

channel_request_start(id, "pty-req", 1);
- client_expect_confirm(id, "PTY allocation", 1);
+ client_expect_confirm(id, "PTY allocation", 0);
packet_put_cstring(term != NULL ? term : "");
packet_put_int((u_int)ws.ws_col);
packet_put_int((u_int)ws.ws_row);
diff -ur openssh-5.8p2-orig//scp.c openssh-5.8p2-morty/scp.c
--- openssh-5.8p2-orig//scp.c Thu Jan 6 11:41:21 2011
+++ openssh-5.8p2-morty/scp.c Tue Jul 26 17:02:25 2011
@@ -273,7 +273,6 @@
addargs(&args, "-l");
addargs(&args, "%s", remuser);
}
- addargs(&args, "--");
addargs(&args, "%s", host);
addargs(&args, "%s", cmd);

@@ -322,7 +321,6 @@
addargs(&args, "-l");
addargs(&args, "%s", remuser);
}
- addargs(&args, "--");
addargs(&args, "%s", host);
addargs(&args, "%s", cmd);

@@ -601,12 +599,12 @@
host = cleanhostname(argv[i]);
suser = NULL;
}
- xasprintf(&bp, "%s -f -- %s", cmd, src);
+ xasprintf(&bp, "%s -f %s", cmd, src);
if (do_cmd(host, suser, bp, &remin, &remout) < 0)
exit(1);
(void) xfree(bp);
host = cleanhostname(thost);
- xasprintf(&bp, "%s -t -- %s", cmd, targ);
+ xasprintf(&bp, "%s -t %s", cmd, targ);
if (do_cmd2(host, tuser, bp, remin, remout) < 0)
exit(1);
(void) xfree(bp);
@@ -641,7 +639,6 @@
} else {
host = cleanhostname(argv[i]);
}
- addargs(&alist, "--");
addargs(&alist, "%s", host);
addargs(&alist, "%s", cmd);
addargs(&alist, "%s", src);
@@ -652,7 +649,7 @@
errs = 1;
} else { /* local to remote */
if (remin == -1) {
- xasprintf(&bp, "%s -t -- %s", cmd, targ);
+ xasprintf(&bp, "%s -t %s", cmd, targ);
host = cleanhostname(thost);
if (do_cmd(host, tuser, bp, &remin,
&remout) < 0)
@@ -685,7 +682,6 @@
addargs(&alist, "-r");
if (pflag)
addargs(&alist, "-p");
- addargs(&alist, "--");
addargs(&alist, "%s", argv[i]);
addargs(&alist, "%s", argv[argc-1]);
if (do_local_cmd(&alist))
@@ -705,7 +701,7 @@
suser = pwd->pw_name;
}
host = cleanhostname(host);
- xasprintf(&bp, "%s -f -- %s", cmd, src);
+ xasprintf(&bp, "%s -f %s", cmd, src);
if (do_cmd(host, suser, bp, &remin, &remout) < 0) {
(void) xfree(bp);
++errs;
diff -ur openssh-5.8p2-orig//version.h openssh-5.8p2-morty/version.h
--- openssh-5.8p2-orig//version.h Thu May 5 01:56:54 2011
+++ openssh-5.8p2-morty/version.h Tue Jul 26 17:02:39 2011
@@ -2,5 +2,5 @@

#define SSH_VERSION "OpenSSH_5.8"

-#define SSH_PORTABLE "p2"
+#define SSH_PORTABLE "p2-MORTY-p2"
#define SSH_RELEASE SSH_VERSION SSH_PORTABLE
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


djm at mindrot

Jul 27, 2011, 12:25 AM

Post #5 of 21 (1802 views)
Permalink
Re: openssh PTY allocation [In reply to]

On Sat, 23 Jul 2011, Mordechai T. Abzug wrote:

> We recently upgraded to openss 5.8p2 from a somewhat older version.
> This broke openssh login to ScreenOS devices. These devices don't
> support PTY allocation. Apparently, ssh now reacts to PTY allocation
> failure by failing the login. This is a change from the previous
> behavior. The simple workaround is ssh -T $device.

The problem is a bug in ScreenOS, it refuses pty-req channel requests
when the tty modes blob exceeds 256 bytes in length. If you want a
workaround that preserves the usability of the tty, then comment out
a couple of less-important modes in ttymodes.h and recompile

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


cmadams at hiwaay

Jul 27, 2011, 7:16 AM

Post #6 of 21 (1803 views)
Permalink
Re: openssh PTY allocation [In reply to]

Once upon a time, Damien Miller <djm [at] mindrot> said:
> The problem is a bug in ScreenOS, it refuses pty-req channel requests
> when the tty modes blob exceeds 256 bytes in length. If you want a
> workaround that preserves the usability of the tty, then comment out
> a couple of less-important modes in ttymodes.h and recompile

I hate to say it, but is there any way to get a reasonable work-around
into upstream OpenSSH? Unfortunately, there are a ton of ScreenOS
devices out there, and even if Juniper fixed the SSH bugs tomorrow, all
those devices won't be updated overnight (if ever). This will be a
serious irritation for network admins as OS distributions update to
newer OpenSSH versions (where most users get their OpenSSH).

Ideally, it would be something easy to enable on the command line (e.g.
a short option, not "-o WorkAroundBrokenScreenOS=1"). I'd be willing to
work on a patch if there is some hope it might be accepted.
--
Chris Adams <cmadams [at] hiwaay>
Systems and Network Administrator - HiWAAY Internet Services
I don't speak for anybody but myself - that's enough trouble.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


mfriedl at gmail

Jul 27, 2011, 2:13 PM

Post #7 of 21 (1798 views)
Permalink
Re: openssh PTY allocation [In reply to]

Do you have a banner vendor string that identifies all broken versions?

On Wednesday, July 27, 2011, Chris Adams <cmadams [at] hiwaay> wrote:
> Once upon a time, Damien Miller <djm [at] mindrot> said:
>> The problem is a bug in ScreenOS, it refuses pty-req channel requests
>> when the tty modes blob exceeds 256 bytes in length. If you want a
>> workaround that preserves the usability of the tty, then comment out
>> a couple of less-important modes in ttymodes.h and recompile
>
> I hate to say it, but is there any way to get a reasonable work-around
> into upstream OpenSSH?  Unfortunately, there are a ton of ScreenOS
> devices out there, and even if Juniper fixed the SSH bugs tomorrow, all
> those devices won't be updated overnight (if ever).  This will be a
> serious irritation for network admins as OS distributions update to
> newer OpenSSH versions (where most users get their OpenSSH).
>
> Ideally, it would be something easy to enable on the command line (e.g.
> a short option, not "-o WorkAroundBrokenScreenOS=1").  I'd be willing to
> work on a patch if there is some hope it might be accepted.
> --
> Chris Adams <cmadams [at] hiwaay>
> Systems and Network Administrator - HiWAAY Internet Services
> I don't speak for anybody but myself - that's enough trouble.
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev [at] mindrot
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
>
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


cmadams at hiwaay

Jul 27, 2011, 2:24 PM

Post #8 of 21 (1803 views)
Permalink
Re: openssh PTY allocation [In reply to]

Once upon a time, Markus Friedl <mfriedl [at] gmail> said:
> Do you have a banner vendor string that identifies all broken versions?

Well, I can't say all broken versions, but it is my understanding that
this affects current ScreenOS versions (and probably old versions as
well). I'd have to set up a test to try different versions, but it
appears that all ScreenOS devices use the banner "SSH-2.0-NetScreen"
(NetScreen is the name of the product line from before Juniper bought
them; they have not changed that internally).

--
Chris Adams <cmadams [at] hiwaay>
Systems and Network Administrator - HiWAAY Internet Services
I don't speak for anybody but myself - that's enough trouble.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


morty at frakir

Jul 28, 2011, 8:52 AM

Post #9 of 21 (1792 views)
Permalink
Re: openssh PTY allocation [In reply to]

On Wed, Jul 27, 2011 at 05:25:05PM +1000, Damien Miller wrote:

> The problem is a bug in ScreenOS, it refuses pty-req channel requests
> when the tty modes blob exceeds 256 bytes in length. If you want a
> workaround that preserves the usability of the tty, then comment out
> a couple of less-important modes in ttymodes.h and recompile

Any suggestions on which modes are less important?

I already came up with a different workaround. Here is the TTY
portion of the approach:

diff -ur openssh-5.8p2-orig//clientloop.c openssh-5.8p2-morty/clientloop.c
--- openssh-5.8p2-orig//clientloop.c Sun Jan 16 12:18:35 2011
+++ openssh-5.8p2-morty/clientloop.c Tue Jul 26 16:55:17 2011
@@ -1982,7 +1982,7 @@
memset(&ws, 0, sizeof(ws));

channel_request_start(id, "pty-req", 1);
- client_expect_confirm(id, "PTY allocation", 1);
+ client_expect_confirm(id, "PTY allocation", 0);
packet_put_cstring(term != NULL ? term : "");
packet_put_int((u_int)ws.ws_col);
packet_put_int((u_int)ws.ws_row);

Which approach is better? I'd be happy to submit a patch that adds an
option to implement the preferred approach.

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


mfriedl at gmail

Jul 28, 2011, 8:59 AM

Post #10 of 21 (1791 views)
Permalink
Re: openssh PTY allocation [In reply to]

make it conditional on "SSH-2.0-NetScreen"
cf compat.c

On Thu, Jul 28, 2011 at 5:52 PM, Morty Abzug <morty [at] frakir> wrote:
> On Wed, Jul 27, 2011 at 05:25:05PM +1000, Damien Miller wrote:
>
>> The problem is a bug in ScreenOS, it refuses pty-req channel requests
>> when the tty modes blob exceeds 256 bytes in length. If you want a
>> workaround that preserves the usability of the tty, then comment out
>> a couple of less-important modes in ttymodes.h and recompile
>
> Any suggestions on which modes are less important?
>
> I already came up with a different workaround.  Here is the TTY
> portion of the approach:
>
> diff -ur openssh-5.8p2-orig//clientloop.c openssh-5.8p2-morty/clientloop.c
> --- openssh-5.8p2-orig//clientloop.c    Sun Jan 16 12:18:35 2011
> +++ openssh-5.8p2-morty/clientloop.c   Tue Jul 26 16:55:17 2011
> @@ -1982,7 +1982,7 @@
>                        memset(&ws, 0, sizeof(ws));
>
>                channel_request_start(id, "pty-req", 1);
> -               client_expect_confirm(id, "PTY allocation", 1);
> +               client_expect_confirm(id, "PTY allocation", 0);
>                packet_put_cstring(term != NULL ? term : "");
>                packet_put_int((u_int)ws.ws_col);
>                packet_put_int((u_int)ws.ws_row);
>
> Which approach is better?  I'd be happy to submit a patch that adds an
> option to implement the preferred approach.
>
> - Morty
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev [at] mindrot
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
>
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


gert at greenie

Jul 28, 2011, 9:00 AM

Post #11 of 21 (1798 views)
Permalink
Re: openssh PTY allocation [In reply to]

Hi,

On Thu, Jul 28, 2011 at 11:52:47AM -0400, Morty Abzug wrote:
> On Wed, Jul 27, 2011 at 05:25:05PM +1000, Damien Miller wrote:
>
> > The problem is a bug in ScreenOS, it refuses pty-req channel requests
> > when the tty modes blob exceeds 256 bytes in length. If you want a
> > workaround that preserves the usability of the tty, then comment out
> > a couple of less-important modes in ttymodes.h and recompile
>
> Any suggestions on which modes are less important?

In that context, I think CS7, PARENB, PARODDB, IXON, IXOFF, IXANY, IUCLC,
PARMRK would be the ones I'd skip, given that use of 7-bit and parity
terminals is unlikely, and that the netscreens are not going to honour
xon/xoff flow control (IXON/IXOFF/IXANY) anyway.

gert
--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany gert [at] greenie
fax: +49-89-35655025 gert [at] net
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


morty at frakir

Jul 28, 2011, 11:34 AM

Post #12 of 21 (1795 views)
Permalink
Re: openssh PTY allocation [In reply to]

On Thu, Jul 28, 2011 at 06:00:38PM +0200, Gert Doering wrote:
> Hi,
>
> On Thu, Jul 28, 2011 at 11:52:47AM -0400, Morty Abzug wrote:
> > On Wed, Jul 27, 2011 at 05:25:05PM +1000, Damien Miller wrote:
> >
> > > The problem is a bug in ScreenOS, it refuses pty-req channel requests
> > > when the tty modes blob exceeds 256 bytes in length. If you want a
> > > workaround that preserves the usability of the tty, then comment out
> > > a couple of less-important modes in ttymodes.h and recompile
> >
> > Any suggestions on which modes are less important?
>
> In that context, I think CS7, PARENB, PARODDB, IXON, IXOFF, IXANY, IUCLC,
> PARMRK would be the ones I'd skip, given that use of 7-bit and parity
> terminals is unlikely, and that the netscreens are not going to honour
> xon/xoff flow control (IXON/IXOFF/IXANY) anyway.

Thanks.

I tested with #ifdef all of the above (CS7, PARENB, PARODDB, IXON,
IXOFF, IXANY, IUCLC, and PARMRK.) This worked to get to one of our
firewalls (ScreenOS 6.3.0r7.0) but not another (ScreenOS 5.3.0r3.0).
So the problem appears to depend to some extent on ScreenOS version or
some other variable that is device-specific.

Meanwhile, I have that other workaround, i.e. to make the ssh client
not consider PTY allocation failure a fatal exit. It appears to work
for all of our ScreenOS devices.

Questions/comments:

(1) From a patch perspective, which approach is preferable -- making
PTY allocation failure not a fatal error, or commenting out a
bunch of ttymodes? [.Assuming a set of ttymodes can be found that
makes this work, of course.] I would lean towards the former
approach, since it seems inherently more robust/consistent.

(2) Commenting out stuff in ttymodes.h thing appears to be a
compile-time option. Is there a way to make it a run-time option?

(3) What would be a good name for an option to workaround this? I
lean towards ExitOnTTYFailure.

(4) What would be a good name for an option to workaround the scp "--"
problem?

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


djm at mindrot

Jul 29, 2011, 12:59 AM

Post #13 of 21 (1794 views)
Permalink
Re: openssh PTY allocation [In reply to]

Try this compat hack:


Index: ttymodes.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/ttymodes.c,v
retrieving revision 1.29
diff -u -p -r1.29 ttymodes.c
--- ttymodes.c 2 Nov 2008 00:16:16 -0000 1.29
+++ ttymodes.c 29 Jul 2011 07:58:29 -0000
@@ -295,8 +295,11 @@ tty_make_modes(int fd, struct termios *t
put_arg(&buf, tio.c_cc[NAME]);

#define TTYMODE(NAME, FIELD, OP) \
- buffer_put_char(&buf, OP); \
- put_arg(&buf, ((tio.FIELD & NAME) != 0));
+ if (!compat20 || (datafellows & SSH_BUG_SCREENOS_PTY) == 0 || \
+ buffer_len(&buf) < 256 - 5) { \
+ buffer_put_char(&buf, OP); \
+ put_arg(&buf, ((tio.FIELD & NAME) != 0)); \
+ }

#include "ttymodes.h"

Index: compat.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/compat.c,v
retrieving revision 1.78
diff -u -p -r1.78 compat.c
--- compat.c 11 Sep 2008 14:22:37 -0000 1.78
+++ compat.c 29 Jul 2011 07:58:29 -0000
@@ -146,6 +146,8 @@ compat_datafellows(const char *version)
SSH_BUG_IGNOREMSG },
{ "*SSH Compatible Server*", /* Netscreen */
SSH_BUG_PASSWORDPAD },
+ { "NetScreen",
+ SSH_BUG_SCREENOS_PTY },
{ "*OSU_0*,"
"OSU_1.0*,"
"OSU_1.1*,"
Index: compat.h
===================================================================
RCS file: /cvs/src/usr.bin/ssh/compat.h,v
retrieving revision 1.42
diff -u -p -r1.42 compat.h
--- compat.h 11 Sep 2008 14:22:37 -0000 1.42
+++ compat.h 29 Jul 2011 07:58:29 -0000
@@ -58,6 +58,7 @@
#define SSH_OLD_FORWARD_ADDR 0x01000000
#define SSH_BUG_RFWD_ADDR 0x02000000
#define SSH_NEW_OPENSSH 0x04000000
+#define SSH_BUG_SCREENOS_PTY 0x08000000

void enable_compat13(void);
void enable_compat20(void);

On Thu, 28 Jul 2011, Morty Abzug wrote:

> On Thu, Jul 28, 2011 at 06:00:38PM +0200, Gert Doering wrote:
> > Hi,
> >
> > On Thu, Jul 28, 2011 at 11:52:47AM -0400, Morty Abzug wrote:
> > > On Wed, Jul 27, 2011 at 05:25:05PM +1000, Damien Miller wrote:
> > >
> > > > The problem is a bug in ScreenOS, it refuses pty-req channel requests
> > > > when the tty modes blob exceeds 256 bytes in length. If you want a
> > > > workaround that preserves the usability of the tty, then comment out
> > > > a couple of less-important modes in ttymodes.h and recompile
> > >
> > > Any suggestions on which modes are less important?
> >
> > In that context, I think CS7, PARENB, PARODDB, IXON, IXOFF, IXANY, IUCLC,
> > PARMRK would be the ones I'd skip, given that use of 7-bit and parity
> > terminals is unlikely, and that the netscreens are not going to honour
> > xon/xoff flow control (IXON/IXOFF/IXANY) anyway.
>
> Thanks.
>
> I tested with #ifdef all of the above (CS7, PARENB, PARODDB, IXON,
> IXOFF, IXANY, IUCLC, and PARMRK.) This worked to get to one of our
> firewalls (ScreenOS 6.3.0r7.0) but not another (ScreenOS 5.3.0r3.0).
> So the problem appears to depend to some extent on ScreenOS version or
> some other variable that is device-specific.
>
> Meanwhile, I have that other workaround, i.e. to make the ssh client
> not consider PTY allocation failure a fatal exit. It appears to work
> for all of our ScreenOS devices.
>
> Questions/comments:
>
> (1) From a patch perspective, which approach is preferable -- making
> PTY allocation failure not a fatal error, or commenting out a
> bunch of ttymodes? [.Assuming a set of ttymodes can be found that
> makes this work, of course.] I would lean towards the former
> approach, since it seems inherently more robust/consistent.
>
> (2) Commenting out stuff in ttymodes.h thing appears to be a
> compile-time option. Is there a way to make it a run-time option?
>
> (3) What would be a good name for an option to workaround this? I
> lean towards ExitOnTTYFailure.
>
> (4) What would be a good name for an option to workaround the scp "--"
> problem?
>
> - Morty
>
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


morty at frakir

Aug 1, 2011, 10:51 PM

Post #14 of 21 (1751 views)
Permalink
Re: openssh PTY allocation [In reply to]

Thanks. I tried this. It only works for one of the two devices I've
been testing with. The device that works runs ScreenOS 6.3.0r7.0.
The device that's still broken runs ScreenOS 5.3.0r3.0.

Knocking the threshold down from 256 to 128, though, yields joy with
both devices. 129 and 130 work, while 131 doesn't; presumably the
success of 129 and 130 is due to fenceposts. Presumably the earlier
version of ScreenOS had a version with a 128 limit; ScreenOS ran into
interop problems at the time; ScreenOS coders bumped it to 256; and
now they/we are having interop problems again. Maybe some even-older
version of ScreenOS had an even lower limit. Hence why I'm thinking
that it might be better to make PTY allocation failure a non-fatal
error rather than trying to guess the other side's worst case buffer
len.

Note that this doesn't do anything for scp. The compat hack stuff
does not seem to be available within scp.c, probably because it
doesn't look directly at the ssh banner.

Combining your compat concept with my earlier patch yields:

diff -ur openssh-5.8p2-orig/clientloop.c openssh-5.8p2-morty-p4/clientloop.c
--- openssh-5.8p2-orig/clientloop.c Sun Jan 16 12:18:35 2011
+++ openssh-5.8p2-morty-p4/clientloop.c Tue Aug 2 05:35:36 2011
@@ -1982,7 +1982,11 @@
memset(&ws, 0, sizeof(ws));

channel_request_start(id, "pty-req", 1);
- client_expect_confirm(id, "PTY allocation", 1);
+ if (datafellows & SSH_BUG_SCREENOS_PTY) {
+ client_expect_confirm(id, "PTY allocation", 0);
+ } else {
+ client_expect_confirm(id, "PTY allocation", 1);
+ }
packet_put_cstring(term != NULL ? term : "");
packet_put_int((u_int)ws.ws_col);
packet_put_int((u_int)ws.ws_row);
diff -ur openssh-5.8p2-orig/compat.c openssh-5.8p2-morty-p4/compat.c
--- openssh-5.8p2-orig/compat.c Mon Nov 3 08:20:14 2008
+++ openssh-5.8p2-morty-p4/compat.c Tue Aug 2 05:35:36 2011
@@ -148,6 +148,8 @@
SSH_BUG_IGNOREMSG },
{ "*SSH Compatible Server*", /* Netscreen */
SSH_BUG_PASSWORDPAD },
+ { "NetScreen",
+ SSH_BUG_SCREENOS_PTY },
{ "*OSU_0*,"
"OSU_1.0*,"
"OSU_1.1*,"
diff -ur openssh-5.8p2-orig/compat.h openssh-5.8p2-morty-p4/compat.h
--- openssh-5.8p2-orig/compat.h Mon Nov 3 08:20:14 2008
+++ openssh-5.8p2-morty-p4/compat.h Tue Aug 2 05:35:36 2011
@@ -58,6 +58,7 @@
#define SSH_OLD_FORWARD_ADDR 0x01000000
#define SSH_BUG_RFWD_ADDR 0x02000000
#define SSH_NEW_OPENSSH 0x04000000
+#define SSH_BUG_SCREENOS_PTY 0x08000000

void enable_compat13(void);
void enable_compat20(void);
diff -ur openssh-5.8p2-orig/scp.c openssh-5.8p2-morty-p4/scp.c
--- openssh-5.8p2-orig/scp.c Thu Jan 6 11:41:21 2011
+++ openssh-5.8p2-morty-p4/scp.c Tue Aug 2 05:35:36 2011
@@ -273,7 +273,6 @@
addargs(&args, "-l");
addargs(&args, "%s", remuser);
}
- addargs(&args, "--");
addargs(&args, "%s", host);
addargs(&args, "%s", cmd);

@@ -322,7 +321,6 @@
addargs(&args, "-l");
addargs(&args, "%s", remuser);
}
- addargs(&args, "--");
addargs(&args, "%s", host);
addargs(&args, "%s", cmd);

@@ -601,12 +599,12 @@
host = cleanhostname(argv[i]);
suser = NULL;
}
- xasprintf(&bp, "%s -f -- %s", cmd, src);
+ xasprintf(&bp, "%s -f %s", cmd, src);
if (do_cmd(host, suser, bp, &remin, &remout) < 0)
exit(1);
(void) xfree(bp);
host = cleanhostname(thost);
- xasprintf(&bp, "%s -t -- %s", cmd, targ);
+ xasprintf(&bp, "%s -t %s", cmd, targ);
if (do_cmd2(host, tuser, bp, remin, remout) < 0)
exit(1);
(void) xfree(bp);
@@ -641,7 +639,6 @@
} else {
host = cleanhostname(argv[i]);
}
- addargs(&alist, "--");
addargs(&alist, "%s", host);
addargs(&alist, "%s", cmd);
addargs(&alist, "%s", src);
@@ -652,7 +649,7 @@
errs = 1;
} else { /* local to remote */
if (remin == -1) {
- xasprintf(&bp, "%s -t -- %s", cmd, targ);
+ xasprintf(&bp, "%s -t %s", cmd, targ);
host = cleanhostname(thost);
if (do_cmd(host, tuser, bp, &remin,
&remout) < 0)
@@ -685,7 +682,6 @@
addargs(&alist, "-r");
if (pflag)
addargs(&alist, "-p");
- addargs(&alist, "--");
addargs(&alist, "%s", argv[i]);
addargs(&alist, "%s", argv[argc-1]);
if (do_local_cmd(&alist))
@@ -705,7 +701,7 @@
suser = pwd->pw_name;
}
host = cleanhostname(host);
- xasprintf(&bp, "%s -f -- %s", cmd, src);
+ xasprintf(&bp, "%s -f %s", cmd, src);
if (do_cmd(host, suser, bp, &remin, &remout) < 0) {
(void) xfree(bp);
++errs;


On Fri, Jul 29, 2011 at 05:59:09PM +1000, Damien Miller wrote:
> Try this compat hack:
>
>
> Index: ttymodes.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ssh/ttymodes.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 ttymodes.c
> --- ttymodes.c 2 Nov 2008 00:16:16 -0000 1.29
> +++ ttymodes.c 29 Jul 2011 07:58:29 -0000
> @@ -295,8 +295,11 @@ tty_make_modes(int fd, struct termios *t
> put_arg(&buf, tio.c_cc[NAME]);
>
> #define TTYMODE(NAME, FIELD, OP) \
> - buffer_put_char(&buf, OP); \
> - put_arg(&buf, ((tio.FIELD & NAME) != 0));
> + if (!compat20 || (datafellows & SSH_BUG_SCREENOS_PTY) == 0 || \
> + buffer_len(&buf) < 256 - 5) { \
> + buffer_put_char(&buf, OP); \
> + put_arg(&buf, ((tio.FIELD & NAME) != 0)); \
> + }
>
> #include "ttymodes.h"
>
> Index: compat.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ssh/compat.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 compat.c
> --- compat.c 11 Sep 2008 14:22:37 -0000 1.78
> +++ compat.c 29 Jul 2011 07:58:29 -0000
> @@ -146,6 +146,8 @@ compat_datafellows(const char *version)
> SSH_BUG_IGNOREMSG },
> { "*SSH Compatible Server*", /* Netscreen */
> SSH_BUG_PASSWORDPAD },
> + { "NetScreen",
> + SSH_BUG_SCREENOS_PTY },
> { "*OSU_0*,"
> "OSU_1.0*,"
> "OSU_1.1*,"
> Index: compat.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/ssh/compat.h,v
> retrieving revision 1.42
> diff -u -p -r1.42 compat.h
> --- compat.h 11 Sep 2008 14:22:37 -0000 1.42
> +++ compat.h 29 Jul 2011 07:58:29 -0000
> @@ -58,6 +58,7 @@
> #define SSH_OLD_FORWARD_ADDR 0x01000000
> #define SSH_BUG_RFWD_ADDR 0x02000000
> #define SSH_NEW_OPENSSH 0x04000000
> +#define SSH_BUG_SCREENOS_PTY 0x08000000
>
> void enable_compat13(void);
> void enable_compat20(void);
>
> On Thu, 28 Jul 2011, Morty Abzug wrote:
>
> > On Thu, Jul 28, 2011 at 06:00:38PM +0200, Gert Doering wrote:
> > > Hi,
> > >
> > > On Thu, Jul 28, 2011 at 11:52:47AM -0400, Morty Abzug wrote:
> > > > On Wed, Jul 27, 2011 at 05:25:05PM +1000, Damien Miller wrote:
> > > >
> > > > > The problem is a bug in ScreenOS, it refuses pty-req channel requests
> > > > > when the tty modes blob exceeds 256 bytes in length. If you want a
> > > > > workaround that preserves the usability of the tty, then comment out
> > > > > a couple of less-important modes in ttymodes.h and recompile
> > > >
> > > > Any suggestions on which modes are less important?
> > >
> > > In that context, I think CS7, PARENB, PARODDB, IXON, IXOFF, IXANY, IUCLC,
> > > PARMRK would be the ones I'd skip, given that use of 7-bit and parity
> > > terminals is unlikely, and that the netscreens are not going to honour
> > > xon/xoff flow control (IXON/IXOFF/IXANY) anyway.
> >
> > Thanks.
> >
> > I tested with #ifdef all of the above (CS7, PARENB, PARODDB, IXON,
> > IXOFF, IXANY, IUCLC, and PARMRK.) This worked to get to one of our
> > firewalls (ScreenOS 6.3.0r7.0) but not another (ScreenOS 5.3.0r3.0).
> > So the problem appears to depend to some extent on ScreenOS version or
> > some other variable that is device-specific.
> >
> > Meanwhile, I have that other workaround, i.e. to make the ssh client
> > not consider PTY allocation failure a fatal exit. It appears to work
> > for all of our ScreenOS devices.
> >
> > Questions/comments:
> >
> > (1) From a patch perspective, which approach is preferable -- making
> > PTY allocation failure not a fatal error, or commenting out a
> > bunch of ttymodes? [.Assuming a set of ttymodes can be found that
> > makes this work, of course.] I would lean towards the former
> > approach, since it seems inherently more robust/consistent.
> >
> > (2) Commenting out stuff in ttymodes.h thing appears to be a
> > compile-time option. Is there a way to make it a run-time option?
> >
> > (3) What would be a good name for an option to workaround this? I
> > lean towards ExitOnTTYFailure.
> >
> > (4) What would be a good name for an option to workaround the scp "--"
> > problem?
> >
> > - Morty
> >

--
Mordechai T. Abzug
Linux red-sonja 2.6.31-23-generic #75-Ubuntu SMP Fri Mar 18 18:16:06 UTC 2011 x86_64 GNU/Linux
If you have any trouble sounding condescending, find a Unix user to show you
how it's done. -Scott Adams (of Dilbert fame)
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


djm at mindrot

Aug 2, 2011, 1:16 AM

Post #15 of 21 (1750 views)
Permalink
Re: openssh PTY allocation [In reply to]

On Tue, 2 Aug 2011, Morty Abzug wrote:

> Thanks. I tried this. It only works for one of the two devices I've
> been testing with. The device that works runs ScreenOS 6.3.0r7.0.
> The device that's still broken runs ScreenOS 5.3.0r3.0.

I tested it with ns5gt.5.4.0r20.0.

> Knocking the threshold down from 256 to 128, though, yields joy with
> both devices. 129 and 130 work, while 131 doesn't; presumably the
> success of 129 and 130 is due to fenceposts. Presumably the earlier
> version of ScreenOS had a version with a 128 limit; ScreenOS ran into
> interop problems at the time; ScreenOS coders bumped it to 256; and
> now they/we are having interop problems again. Maybe some even-older
> version of ScreenOS had an even lower limit. Hence why I'm thinking
> that it might be better to make PTY allocation failure a non-fatal
> error rather than trying to guess the other side's worst case buffer
> len.

That could be unpredictable, the ScreenOS end refuses the whole pty-req
so it is working under the assumption of no PTY whereas the client would
(by ignoring the failure) continue to expect one there.

Really, I'd prefer that Juniper just fix the bug (it is probably
trivial for them to do so) rather than peppering our tree with compat
hacks that must be maintained in perpetuity.

> Note that this doesn't do anything for scp. The compat hack stuff
> does not seem to be available within scp.c, probably because it
> doesn't look directly at the ssh banner.

scp is inherently unfixable as a protocol.

-d

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


morty at frakir

Aug 2, 2011, 2:06 AM

Post #16 of 21 (1776 views)
Permalink
Re: openssh PTY allocation [In reply to]

On Tue, Aug 02, 2011 at 06:16:14PM +1000, Damien Miller wrote:
> On Tue, 2 Aug 2011, Morty Abzug wrote:
>
> > Thanks. I tried this. It only works for one of the two devices I've
> > been testing with. The device that works runs ScreenOS 6.3.0r7.0.
> > The device that's still broken runs ScreenOS 5.3.0r3.0.
>
> I tested it with ns5gt.5.4.0r20.0.

Then they must have increased the buffer between 5.3.0r3.0 and
5.4.0r20.0.

> > Hence why I'm thinking
> > that it might be better to make PTY allocation failure a non-fatal
> > error rather than trying to guess the other side's worst case buffer
> > len.

> That could be unpredictable, the ScreenOS end refuses the whole
> pty-req so it is working under the assumption of no PTY whereas the
> client would (by ignoring the failure) continue to expect one there.

ScreenOS isn't a *nix. In practice, even with a PTY allocation
failure error, its command-line function acts as much like a PTY as it
ever does. For example, you can do full command-line editing and
scroll through command history even after getting a PTY allocation
failure error. I suspect ScreenOS is making a bunch of hardcoded
assumptions about terminal-like behaviors that completely violate
normal *nix conventions, but make sense in a non-*nix environment that
needs to play well with *nix. So the PTY allocation failure really
does appear to be meaningless in the case of a ScreenOS device.

> Really, I'd prefer that Juniper just fix the bug (it is probably
> trivial for them to do so) rather than peppering our tree with
> compat hacks that must be maintained in perpetuity.

I've already reported this to our Juniper SE, who says he passed it on
to their maintenance engineering. However, I'll echo what Chris Adams
said in an earlier message:

Unfortunately, there are a ton of ScreenOS devices out there, and
even if Juniper fixed the SSH bugs tomorrow, all those devices won't
be updated overnight (if ever). This will be a serious irritation
for network admins as OS distributions update to newer OpenSSH
versions (where most users get their OpenSSH).

In a lot of environments, upgrading openssh is a whole lot easier than
upgrading firewalls. Firewalls are customer-facing service devices
with monetary repercussions for downtime, whereas openssh on a network
engineering station is an internal application that customers don't
care about.

And for that matter, openssh is what changed -- older openssh worked
fine with these same revisions of ScreenOS. This may be
Juniper/Netscreen's mistake, but this is most easily patched on
openssh's side.

> > Note that this doesn't do anything for scp. The compat hack stuff
> > does not seem to be available within scp.c, probably because it
> > doesn't look directly at the ssh banner.
>
> scp is inherently unfixable as a protocol.

Again, this formerly worked. I have scripts that were built around
scp to ScreenOS devices. Older openssh worked fine. The addition of
the "--" option to the remote scp invocation is what appears to have
broken interop between openssh's scp and ScreenOS's scp.

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


djm at mindrot

Aug 5, 2011, 9:26 PM

Post #17 of 21 (1717 views)
Permalink
Re: openssh PTY allocation [In reply to]

FYI here is a diff that installs workarounds for all of the problems
with ScreenOS that I'm aware of. These are:

- PTY allocation
- scp -- thing
- keepalives killing the connection
- multiplexing requests killing the connection

Not sure whether I want to commit these.

Index: clientloop.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/clientloop.c,v
retrieving revision 1.236
diff -u -p -r1.236 clientloop.c
--- clientloop.c 22 Jun 2011 22:08:42 -0000 1.236
+++ clientloop.c 6 Aug 2011 04:21:42 -0000
@@ -1375,7 +1375,11 @@ client_loop(int have_pty, int escape_cha
char buf[100];

debug("Entering interactive session.");
-
+ if ((datafellows & SSH_BUG_KEEPALIVE) != 0 &&
+ options.server_alive_interval != 0) {
+ debug2("Disabling keepalives due to server bug");
+ options.server_alive_interval = 0;
+ }
start_time = get_current_time();

/* Initialize variables. */
Index: compat.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/compat.c,v
retrieving revision 1.78
diff -u -p -r1.78 compat.c
--- compat.c 11 Sep 2008 14:22:37 -0000 1.78
+++ compat.c 6 Aug 2011 04:21:42 -0000
@@ -146,6 +146,10 @@ compat_datafellows(const char *version)
SSH_BUG_IGNOREMSG },
{ "*SSH Compatible Server*", /* Netscreen */
SSH_BUG_PASSWORDPAD },
+ { "NetScreen",
+ SSH_BUG_SCREENOS_PTY|
+ SSH_BUG_KEEPALIVE|
+ SSH_BUG_MULTIPLEX },
{ "*OSU_0*,"
"OSU_1.0*,"
"OSU_1.1*,"
Index: compat.h
===================================================================
RCS file: /cvs/src/usr.bin/ssh/compat.h,v
retrieving revision 1.42
diff -u -p -r1.42 compat.h
--- compat.h 11 Sep 2008 14:22:37 -0000 1.42
+++ compat.h 6 Aug 2011 04:21:42 -0000
@@ -58,6 +58,9 @@
#define SSH_OLD_FORWARD_ADDR 0x01000000
#define SSH_BUG_RFWD_ADDR 0x02000000
#define SSH_NEW_OPENSSH 0x04000000
+#define SSH_BUG_SCREENOS_PTY 0x08000000
+#define SSH_BUG_KEEPALIVE 0x10000000
+#define SSH_BUG_MULTIPLEX 0x20000000

void enable_compat13(void);
void enable_compat20(void);
Index: scp.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/scp.c,v
retrieving revision 1.170
diff -u -p -r1.170 scp.c
--- scp.c 9 Dec 2010 14:13:33 -0000 1.170
+++ scp.c 6 Aug 2011 04:21:42 -0000
@@ -580,12 +580,14 @@ toremote(char *targ, int argc, char **ar
host = cleanhostname(argv[i]);
suser = NULL;
}
- xasprintf(&bp, "%s -f -- %s", cmd, src);
+ xasprintf(&bp, "%s -f %s%s", cmd,
+ *src == '-' ? "-- " : "", src);
if (do_cmd(host, suser, bp, &remin, &remout) < 0)
exit(1);
(void) xfree(bp);
host = cleanhostname(thost);
- xasprintf(&bp, "%s -t -- %s", cmd, targ);
+ xasprintf(&bp, "%s -t %s%s", cmd,
+ *targ == '-' ? "-- " : "", targ);
if (do_cmd2(host, tuser, bp, remin, remout) < 0)
exit(1);
(void) xfree(bp);
@@ -631,7 +633,8 @@ toremote(char *targ, int argc, char **ar
errs = 1;
} else { /* local to remote */
if (remin == -1) {
- xasprintf(&bp, "%s -t -- %s", cmd, targ);
+ xasprintf(&bp, "%s -t %s%s", cmd,
+ *targ == '-' ? "-- " : "", targ);
host = cleanhostname(thost);
if (do_cmd(host, tuser, bp, &remin,
&remout) < 0)
Index: serverloop.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/serverloop.c,v
retrieving revision 1.160
diff -u -p -r1.160 serverloop.c
--- serverloop.c 15 May 2011 08:09:01 -0000 1.160
+++ serverloop.c 6 Aug 2011 04:21:42 -0000
@@ -277,6 +277,11 @@ wait_until_can_do_something(fd_set **rea
int ret;
int client_alive_scheduled = 0;

+ if ((datafellows & SSH_BUG_KEEPALIVE) != 0 &&
+ options.client_alive_interval != 0) {
+ debug2("Disabling keepalives due to client bug");
+ options.client_alive_interval = 0;
+ }
/*
* if using client_alive, set the max timeout accordingly,
* and indicate that this particular timeout was for client
Index: ssh.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/ssh.c,v
retrieving revision 1.364
diff -u -p -r1.364 ssh.c
--- ssh.c 2 Aug 2011 23:15:03 -0000 1.364
+++ ssh.c 6 Aug 2011 04:21:42 -0000
@@ -890,6 +890,13 @@ main(int ac, char **av)
}
}

+ if ((datafellows & SSH_BUG_MULTIPLEX) != 0 &&
+ options.control_path != NULL &&
+ options.control_master != SSHCTL_MASTER_NO) {
+ debug("Disabling multiplexing due to server bugs");
+ options.control_master = SSHCTL_MASTER_NO;
+ }
+
exit_status = compat20 ? ssh_session2() : ssh_session();
packet_close();

Index: ttymodes.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/ttymodes.c,v
retrieving revision 1.29
diff -u -p -r1.29 ttymodes.c
--- ttymodes.c 2 Nov 2008 00:16:16 -0000 1.29
+++ ttymodes.c 6 Aug 2011 04:21:42 -0000
@@ -295,8 +295,11 @@ tty_make_modes(int fd, struct termios *t
put_arg(&buf, tio.c_cc[NAME]);

#define TTYMODE(NAME, FIELD, OP) \
- buffer_put_char(&buf, OP); \
- put_arg(&buf, ((tio.FIELD & NAME) != 0));
+ if (!compat20 || (datafellows & SSH_BUG_SCREENOS_PTY) == 0 || \
+ buffer_len(&buf) < 256 - 5) { \
+ buffer_put_char(&buf, OP); \
+ put_arg(&buf, ((tio.FIELD & NAME) != 0)); \
+ }

#include "ttymodes.h"

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


gert at greenie

Aug 6, 2011, 1:47 AM

Post #18 of 21 (1710 views)
Permalink
Re: openssh PTY allocation [In reply to]

Hi,

On Sat, Aug 06, 2011 at 02:26:09PM +1000, Damien Miller wrote:
> FYI here is a diff that installs workarounds for all of the problems
> with ScreenOS that I'm aware of. These are:
>
> - PTY allocation
> - scp -- thing
> - keepalives killing the connection
> - multiplexing requests killing the connection
>
> Not sure whether I want to commit these.

As a pure user, not speaking for the developers, but having to SSH (and
SCP!) to Netscreens regularily - these look quite reasonable to me, and
I'd like to see something like this in the general code base.

(Otherwise I'm happy that you have provided the patch and will use that
to patch our local ssh installation)

gert
--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany gert [at] greenie
fax: +49-89-35655025 gert [at] net
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


morty at frakir

Aug 8, 2011, 3:30 PM

Post #19 of 21 (1697 views)
Permalink
Re: openssh PTY allocation [In reply to]

On Sat, Aug 06, 2011 at 02:26:09PM +1000, Damien Miller wrote:
> FYI here is a diff that installs workarounds for all of the problems
> with ScreenOS that I'm aware of. These are:
>
> - PTY allocation
> - scp -- thing
> - keepalives killing the connection
> - multiplexing requests killing the connection

Thanks for the patch. In my testing, it has the following issues:

(1) ssh still doesn't work for some of our devices. I think this is
because the ttymodes.c portion of your patch has "256" when it should
be "128".

(2) scp didn't actually work to any of my test netscreens for scp
$device:ns_sys_config /tmp. I tried scp -v $device:ns_sys_config /tmp
to see what the command was. I got:

debug1: Sending command: scp -v -f -- ns_sys_config

As you can see, "--" is still there.

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


djm at mindrot

Aug 8, 2011, 11:17 PM

Post #20 of 21 (1696 views)
Permalink
Re: openssh PTY allocation [In reply to]

On Mon, 8 Aug 2011, Morty Abzug wrote:

> On Sat, Aug 06, 2011 at 02:26:09PM +1000, Damien Miller wrote:
> > FYI here is a diff that installs workarounds for all of the problems
> > with ScreenOS that I'm aware of. These are:
> >
> > - PTY allocation
> > - scp -- thing
> > - keepalives killing the connection
> > - multiplexing requests killing the connection
>
> Thanks for the patch. In my testing, it has the following issues:
>
> (1) ssh still doesn't work for some of our devices. I think this is
> because the ttymodes.c portion of your patch has "256" when it should
> be "128".

Even if I do commit something like this diff (which is not guaranteed),
it certainly won't truncate the ttymodes at 128 bytes - fixed versions
of ScreenOS already exist for this problem and chopping so much off is
likely to leave a messed up TTY anyway.

> (2) scp didn't actually work to any of my test netscreens for scp
> $device:ns_sys_config /tmp. I tried scp -v $device:ns_sys_config /tmp
> to see what the command was. I got:
>
> debug1: Sending command: scp -v -f -- ns_sys_config
>
> As you can see, "--" is still there.

oops, I missed a case:

Index: scp.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/scp.c,v
retrieving revision 1.170
diff -u -p -r1.170 scp.c
--- scp.c 9 Dec 2010 14:13:33 -0000 1.170
+++ scp.c 9 Aug 2011 06:10:08 -0000
@@ -580,12 +580,14 @@ toremote(char *targ, int argc, char **ar
host = cleanhostname(argv[i]);
suser = NULL;
}
- xasprintf(&bp, "%s -f -- %s", cmd, src);
+ xasprintf(&bp, "%s -f %s%s", cmd,
+ *src == '-' ? "-- " : "", src);
if (do_cmd(host, suser, bp, &remin, &remout) < 0)
exit(1);
(void) xfree(bp);
host = cleanhostname(thost);
- xasprintf(&bp, "%s -t -- %s", cmd, targ);
+ xasprintf(&bp, "%s -t %s%s", cmd,
+ *targ == '-' ? "-- " : "", targ);
if (do_cmd2(host, tuser, bp, remin, remout) < 0)
exit(1);
(void) xfree(bp);
@@ -631,7 +633,8 @@ toremote(char *targ, int argc, char **ar
errs = 1;
} else { /* local to remote */
if (remin == -1) {
- xasprintf(&bp, "%s -t -- %s", cmd, targ);
+ xasprintf(&bp, "%s -t %s%s", cmd,
+ *targ == '-' ? "-- " : "", targ);
host = cleanhostname(thost);
if (do_cmd(host, tuser, bp, &remin,
&remout) < 0)
@@ -664,7 +667,8 @@ tolocal(int argc, char **argv)
addargs(&alist, "-r");
if (pflag)
addargs(&alist, "-p");
- addargs(&alist, "--");
+ if (*(argv[i]) == '-' || *(argv[argc-1]) == '-')
+ addargs(&alist, "--");
addargs(&alist, "%s", argv[i]);
addargs(&alist, "%s", argv[argc-1]);
if (do_local_cmd(&alist))
@@ -684,7 +688,8 @@ tolocal(int argc, char **argv)
suser = pwd->pw_name;
}
host = cleanhostname(host);
- xasprintf(&bp, "%s -f -- %s", cmd, src);
+ xasprintf(&bp, "%s -f %s%s",
+ cmd, *src == '-' ? "-- " : "", src);
if (do_cmd(host, suser, bp, &remin, &remout) < 0) {
(void) xfree(bp);
++errs;
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


morty at frakir

Aug 9, 2011, 5:50 PM

Post #21 of 21 (1693 views)
Permalink
Re: openssh PTY allocation [In reply to]

On Tue, Aug 09, 2011 at 04:17:05PM +1000, Damien Miller wrote:

> Even if I do commit something like this diff (which is not
> guaranteed), it certainly won't truncate the ttymodes at 128 bytes -
> fixed versions of ScreenOS already exist for this problem and
> chopping so much off is likely to leave a messed up TTY anyway.

In my testing, setting the threshold to 128 didn't cause any TTY
problems in practice. A lot of the older versions are in the field.
Is there any chance that you could set the number to 128?

> > As you can see, "--" is still there.

> oops, I missed a case:

Thanks!

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

OpenSSH dev RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.