Fix pty-req terminal modes: deliver them unpadded, encode the right l… - #755
Open
tluyben wants to merge 1 commit into
Open
Fix pty-req terminal modes: deliver them unpadded, encode the right l…#755tluyben wants to merge 1 commit into
tluyben wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Forwarding a pty-req from a server-side channel to a client-side channel — what any SSH proxy or jump host does — produces a malformed request that the receiving end rejects, dropping the session as soon as the user asks for a pty. Two separate defects combine to cause it; each is worth fixing on its own.
Session::server_read_encrypted decodes pty modes into a fixed [(Pty, u32); 130] array and passes &modes[0..i] to handler.pty_request() — but sends the entire padded array to the channel stream:
terminal_modes: modes.into(), // 130 entries, 128 of them TTY_OP_END padding
So the same event, delivered two ways, disagrees: a handler callback sees the two modes the client sent, while an application reading ChannelMsg::RequestPty off the channel sees 130.
client::Session::request_pty writes the modes string as:
((1 + 5 * terminal_modes.len()) as u32).encode(&mut enc.write)?;
for &(code, value) in terminal_modes {
if code == Pty::TTY_OP_END { continue; } // <-- skipped, but counted above
...
}
Any TTY_OP_END entry in the input is counted in the length and then skipped when writing. Passing the padded array from (1) declares 651 bytes and writes 1, so the receiver reads far past the end of the modes string and fails the packet. This is reachable without a proxy: it fires for any caller that includes a terminator in its mode
list, which is a natural thing to do given the wire format.
Fix
Reproducing
An application holding the server-side channel:
async fn channel_open_session(&mut self, mut channel: Channel, reply: ChannelOpenHandle, _: &mut Session) -> Result<(), Self::Error> {
reply.accept().await;
tokio::spawn(async move {
while let Some(msg) = channel.wait().await {
if let ChannelMsg::RequestPty { terminal_modes, .. } = msg {
// 130 entries, regardless of what the client sent;
// handing these to Channel::request_pty emits a corrupt packet
}
}
});
Ok(())
}
AI Usage