Bug 3061 - requesting invalid terminal modes no longer aborts connection
Summary: requesting invalid terminal modes no longer aborts connection
Status: CLOSED INVALID
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: 8.0p1
Hardware: All Linux
: P5 minor
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-29 13:41 AEST by Michael Hudson-Doyle
Modified: 2021-03-04 09:54 AEDT (History)
4 users (show)

See Also:


Attachments
test case using golang.org/x/crypto (901 bytes, text/plain)
2019-08-29 13:41 AEST, Michael Hudson-Doyle
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Hudson-Doyle 2019-08-29 13:41:11 AEST
Created attachment 3312 [details]
test case using golang.org/x/crypto

Running the attach test go test case against "OpenSSH_7.6p1 Ubuntu-4ubuntu0.3, OpenSSL 1.0.2n  7 Dec 2017" (i.e. Ubuntu 18.04) yields:

2019/08/29 15:33:23 request for pseudo terminal failed: EOF
exit status 1

Running it against "OpenSSH_7.9p1 Ubuntu-10, OpenSSL 1.1.1b  26 Feb 2019" (i.e. Ubuntu 19.10) exits with code 0.

Going over the git history, it looks like this 1) could be a consequence of the translation of ssh_tty_parse_modes to the sshbuf API 2) does not seem intentional.
Comment 1 Darren Tucker 2019-08-29 14:26:45 AEST
FWIW since your example uses opcode 255, the current behaviour seems more in compliance with RFC4254 than the previous behaviour: https://tools.ietf.org/html/rfc4254#page-19:

   Opcodes 160 to 255 are not yet
   defined, and cause parsing to stop (they should only be used after
   any other data).

other than that it doesn't take a strong stance on unknown opcodes:

   the server MAY ignore any modes it does not know about
Comment 2 Michael Hudson-Doyle 2019-08-29 18:42:02 AEST
Yes, I think this change in behaviour is probably a good thing and closer to the spirit of the RFC (and quite possibly what was the intention all along). I wasn't sure about the "cause parsing to stop" comment, I guess that just means stop parsing the tty_modes command/packet? I can't remember enough about the SSH protocol to remember how these things are framed...
Comment 3 Darren Tucker 2019-08-29 18:54:15 AEST
(In reply to Michael Hudson-Doyle from comment #2)
> Yes, I think this change in behaviour is probably a good thing and
> closer to the spirit of the RFC (and quite possibly what was the
> intention all along). I wasn't sure about the "cause parsing to
> stop" comment, I guess that just means stop parsing the tty_modes
> command/packet?

I'd interpret that as "cease processing the ttymode opcodes from the packet and applying them to the pty".   From the reading the code it seems like that's the current behaviour too.
Comment 4 Damien Miller 2019-09-13 14:46:08 AEST
Yes, I think the current behaviour is better. There are still a few cases where the new code fatally errors in malformed ttymodes where it could conceivably continue (without applying changes to the tty). Not sure whether these are worth fixing though...
Comment 5 Michael Hudson-Doyle 2019-09-17 14:04:16 AEST
OK, thanks for the comments and clarification. I'll close the bug.
Comment 6 Damien Miller 2021-03-04 09:54:23 AEDT
close bugs that were resolved in OpenSSH 8.5 release cycle