Implement OpenSSH-like escape sequences in tsh

[awly]

Only applies to interactive sessions.
Watch the user keyboard input for one of:

• ~?: print help output for escape sequences
• ~.: disconnect the current session (for example when stuck due to
  traffic getting black-holed)

The full list of OpenSSH sequences is here:
https://man.openbsd.org/ssh#ESCAPE_CHARACTERS
We only support the two mentioned above for now. If the need arises, it
should be easy to add.

The implementation ended up pretty complicated, read the comment on
escape.NewReader for some info and reasoning.

Fixes #2555

[webvictim]

It looks like this closer is always going to be called as it's outside the switch block? Not sure if that's intentional or if I've misunderstood its function - it seems like it this be overenthusiastically closing the connection once attached to stdin even if there was no error received.

[awly]

this callback is only called with non-nil errors.
it's meant as a signal for the parent to prematurely terminate the connection.
it won't be called when the connection exits cleanly.

[webvictim]

Also, the man page you linked states that the way to send a single tilde by itself is to enter ~~ - do we handle that specifically, or does it just fall through as-is because nothing catches it?

I can't think of many situations where someone would just want to type ~ and nothing else after it into a terminal, but just in case...

[awly]

Also, the man page you linked states that the way to send a single tilde by itself is to enter ~~ - do we handle that specifically, or does it just fall through as-is because nothing catches it?

I can't think of many situations where someone would just want to type ~ and nothing else after it into a terminal, but just in case...

So I chose to not implement that behavior. It complicates things a bit and doesn't seem all that useful.
Instead, all characters are forwarded over as is. When you type slowly, server will get the ~ right away. If you then press ., connection will close.

If you think there's value in implementing the OpenSSH behavior more precisely, I can play around with it.

The only real use-case we're targeting (I assume) is:

• open a shell, do some work
• unfathomable network things happen, connection "hangs"
• type some commands, nothing happens
• try <Ctrl>-C, <Ctrl>-D, faceroll the keyboard
• give up, press <Enter><Enter><Enter>~. to kill the connection

Implementing ~~ escaping with fancy blocking logic here probably doesn't matter

[webvictim]

I don't think there is particular value in it, no. I agree with your assessment of the case we're targetting.

I just wanted to make sure that we didn't have a situation where you type a single ~ character and nothing appears in the terminal until such time as you follow it up with something else. It sounds like that will work fine.

[fspmarshall]

Few notes based on comparing this function to the process_escapes function in openssh:

• Help escape sequence is not filtered out (openssh appears to filter out the help sequence).
• Disconnect escape sequence drops all preceding bytes (openssh appears to send all data up to the disconnect escape sequence).
• \r~ starts an escape, but \n~ does not (openssh treats \r and \n interchangeably here).
• Reads ending in partial escape sequence (e.g. \r~) will send the escape character (~) to remote (openssh does not send the escape character until the following character is available to ensure that partial escape sequences are not transmitted to remote).

I think its preferable if we keep parity with openssh on these points (potentially confusing if we don't, since we shadow their syntax).

[fspmarshall]

Also, the escape character should probably be configurable (or at least the behavior should be able to be deactivated). I don't know what this might break, but its probably best not to roll out a behavioral change like this without the ability to work around it if it does break something.

[russjones]

I think @fspmarshall covered this in the fourth bullet, but this will not correctly recognize sequences that are split between buffers. Processing character-by-character is safer.

[awly]

Just a meta-response: I agree that it's not a perfect replica of openssh behavior. But it works for the use-case it's addressing (killing stuck sessions).

That being said, I'll massage the code some more to prevent escape sequences from going out to the remote side.

Disconnect escape sequence drops all preceding bytes (openssh appears to send all data up to the disconnect escape sequence).

What does it do when remote side is unresponsive?

\r~ starts an escape, but \n~ does not (openssh treats \r and \n interchangeably here).

Nice catch, will fix!

Also, the escape character should probably be configurable (or at least the behavior should be able to be deactivated).

This makes sense, I'll add a tsh flag.
Although I'd like to not over-complicate this niche feature.

this will not correctly recognize sequences that are split between buffers.

It will recognize sequences split between buffers via the prev buffer. But it will not block them the same way as openssh does.

[fspmarshall]

What does it do when remote side is unresponsive?

Ah, good point. Take this with a grain of salt because my knowledge of both openssh and C in general is pretty weak, and the state-machine around channels in openssh is complex.... but I believe it performs a single non-blocking write attempt with any remaining pre-escape sequence data and then exits. Thats a much weaker guarantee than I originally assumed, so I'd say forget that suggestion and keep dropping the preceeding bytes.

[awly]

Ack. I don't see an easy way to do a non-blocking write at our level of abstraction in lib/client/session.go so I'll keep it as is.

Refactored the reader logic to read one character at a time and filter out escape sequences.
Also added handling for both \r and \n.

Next is the tsh flag plumbing...

[awly]

Added --no-enable-escape-sequences flag to tsh.
PTAL

[fspmarshall]

Performing single-byte Reads and Writes is potentially costly performance-wise. Inconsequential for tsh, but since this code will also run in the proxies (backs web UI terminal), I think its worth the extra effort to make this code batch-process when able. Should be able to achieve this with an intermediate buffer and an inner for-loop (rough translation of how I think openssh ends up behaving):

var rbuf,wbuf [1024]byte
newline, esc := true, false
for {
	rn, _ := input.Read(rbuf[:])
	wn := 0
Inner:
	for _, byte := range rbuf[:rn] {
		switch byte {
		case '?':
			if esc {
				esc, newline = false, false
				showHelp()
				continue Inner
			}
		case '.':
			if esc {
				return
			}
		case '~':
			if esc {
				esc = false
			}
			if newline {
				esc, newline = true, false
				continue Inner
			}
		}
		if esc {
			esc = false
			wbuf[wn] = '~'
			wn++
		}
		newline =  byte == '\r' || byte == '\n'
		wbuf[wn] = byte
		wn++
	}
	output(wbuf[:wn]) 
}

[fspmarshall]

Pretty sure this is 10KB not 10MB. Not sure which you wanted, but I find that defining constants (other than bitmasks) with bitshifting can be unintuitive. Multiples of 1024/1KB are more readable IMO since its a big enough number to keep the expression short, but small enough to be immediately recognizable (e.g. 10MB = 10 * 1024 * 1024).

[awly]

Shame on me :|
You're right, changed to a more readable version.

[awly]

Thanks @fspmarshall for the OpenSSH transcription, it was super helpful!
PTAL