openssh bugs - Aug 2019 - [Bug 3061] New: requesting invalid terminal modes no longer aborts connection

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

            Bug ID: 3061
           Summary: requesting invalid terminal modes no longer aborts
                    connection
           Product: Portable OpenSSH
           Version: 8.0p1
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: minor
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: mwhudson at debian.org

Created attachment 3312
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3312&action=edit
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.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

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

Michael Hudson-Doyle <mwhudson at debian.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mwhudson at debian.org

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

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

Colin Watson <cjwatson at debian.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cjwatson at debian.org

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

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

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at dtucker.net
   Attachment #3312|text/x-go                   |text/plain
          mime type|                            |

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

--- Comment #1 from Darren Tucker <dtucker at dtucker.net> ---
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

-- 
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=3061

--- Comment #2 from Michael Hudson-Doyle <mwhudson at debian.org> ---
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...

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

--- Comment #3 from Darren Tucker <dtucker at dtucker.net> ---
(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.

-- 
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=3061

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
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...

-- 
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=3061

Michael Hudson-Doyle <mwhudson at debian.org> changed:

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

--- Comment #5 from Michael Hudson-Doyle <mwhudson at debian.org> ---
OK, thanks for the comments and clarification. I'll close the bug.

-- 
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=3061

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
close bugs that were resolved in OpenSSH 8.5 release cycle

-- 
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.