openssh bugs - Oct 2015 - [Bug 2478] New: Escape sequences (starting with ~) don't work when remote shell is BusyBox ash

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

            Bug ID: 2478
           Summary: Escape sequences (starting with ~) don't work when
                    remote shell is BusyBox ash
           Product: Portable OpenSSH
           Version: 7.1p1
          Hardware: ix86
                OS: Linux
            Status: NEW
          Severity: minor
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: alexinbeijing at gmail.com

Created attachment 2725
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2725&action=edit
Fix

This was originally send directly to the mailing list, but I am also
posting it here as requested by Damien Miller.

This patch was prompted by the observation that SSH escapes (using ~)
don't work at all when the remote shell is BusyBox ash (used by Alpine
Linux and some other minimalist and/or embedded Linux distributions).

This is because the BusyBox shell *always* sends a "query cursor
position" terminal control escape sequence each time you press the
<ENTER> key. Quick as a flash, as your pinky finger is descending
towards the tilda key, the terminal sends back a "report cursor
position" sequence. The result is that the SSH client *never* sees your
tilda as immediately following the newline.

Those ANSI terminal control sequences always begin with "<ESC>["
and
end
with an alphabetic character. Hence the call to isalpha() in my code.

Can someone confirm that isalpha() *always* treats a-z and A-Z as
alphabetic, no matter what the locale is?

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

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

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2725|                            |ok-
              Flags|                            |

--- Comment #1 from Darren Tucker <dtucker at zip.com.au> ---
Comment on attachment 2725
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2725
Fix
>+		if (ch == 0x1B && buf[i+1] == '[') {
If the escape char is the last byte in the buffer then the
control-sequence detection won't work and you'll get an out-of-bounds
read.

Admittedly this is unlikely in regular usage, but certainly possible
for a malicious server and maybe possible by traffic shaping a legit
connection.

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

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

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #2 from Damien Miller <djm at mindrot.org> ---
Yeah, I think it should skip over the escape char if it is the last
character. escape_pending won't be reset and the client will try to go
read some more tty data and call process_escapes again (I think).

I'm not sure the pushing back of escape sequences to the "bin"
buffer
is correct though - it might end up messing up the order of things.
Something a bit more tricky is probably needed.

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

--- Comment #3 from Alex Dowad <alexinbeijing at gmail.com> ---
dtucker: Thanks for pointing that out. I realized the same when falling
asleep yesterday. Will post an updated patch.

djm: "I'm not sure the pushing back of escape sequences to the bin
buffer is correct" -- this is exactly what happens to them in the
existing code, without this patch. That aspect is unchanged. The only
thing which changes, is that <CR><escape><escaped char>
sequences are
processed by the client *even if* an ANSI escape appears in between the
<CR> and <escape> or between the <escape> and <escaped
char>.

There's no reason why OpenSSH should intercept and change or remove
ANSI escape sequences. That would make a remote shell behave
differently from a local shell. By writing them into 'bin', just as
they appear in the input, SSH can be 'transparent' as far as the
interaction between terminal emulator and remote shell goes.

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

Alex Dowad <alexinbeijing at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2725|0                           |1
        is obsolete|                            |

--- Comment #4 from Alex Dowad <alexinbeijing at gmail.com> ---
Created attachment 2726
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2726&action=edit
Patch v2

Please have a look at the v2. It treats ANSI terminal escapes
consistently regardless of whether they are 'cut off' at the end of
'buf' (and straddle 2 separate calls to process_escapes) or not.

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

--- Comment #5 from Alex Dowad <alexinbeijing at gmail.com> ---
I've just looked at the possibility of adding some tests for ssh escape
sequences to your regression tests.

One difficulty is that the ~C command prompt works by reading/writing
/dev/tty directly, rather than using stdin/stdout. This makes it hard
to capture what the user sees and compare it to expected output.

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

--- Comment #6 from Alex Dowad <alexinbeijing at gmail.com> ---
Ping...

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

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at zip.com.au
             Blocks|                            |2543


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2543
[Bug 2543] Tracking bug for OpenSSH 7.3 release
-- 
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=2478

--- Comment #7 from Damien Miller <djm at mindrot.org> ---
retarget unfinished bugs to next release

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2594

--- Comment #8 from Damien Miller <djm at mindrot.org> ---
retarget unfinished bugs to next release


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2594
[Bug 2594] Tracking bug for OpenSSH 7.4 release
-- 
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=2478

--- Comment #9 from Damien Miller <djm at mindrot.org> ---
retarget unfinished bugs to next release

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

--- Comment #10 from Damien Miller <djm at mindrot.org> ---
retarget unfinished bugs to next release

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2543                        |


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2543
[Bug 2543] Tracking bug for OpenSSH 7.3 release
-- 
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=2478

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2594                        |

--- Comment #11 from Damien Miller <djm at mindrot.org> ---
I think this patch needs more work - at the very least it needs to
conditionalise its special-casing of ANSI sequences to be enabled only
for ANSI-like $TERM (-inals).

BusyBox's behaviour does seem kind of dumb though...


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2594
[Bug 2594] Tracking bug for OpenSSH 7.4 release
-- 
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.