Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Teleport doesn't handle PuTTY winadj messages #19365

Closed
webvictim opened this issue Dec 14, 2022 · 1 comment · Fixed by #19468
Closed

Teleport doesn't handle PuTTY winadj messages #19365

webvictim opened this issue Dec 14, 2022 · 1 comment · Fixed by #19468
Assignees

Comments

@webvictim
Copy link
Contributor

webvictim commented Dec 14, 2022

Expected behavior: PuTTY sometimes tries to request a channel with the name winadj@putty.projects.tartarus.org. It does this expecting a channel request failure in response and uses the latency of the entire roundtrip to calculate some internal timing information used for dynamically tuning window size.

It's possible to get this to happen if you spam a lot of commands or text into the window in a short space of time. I can recreate this fairly reliably (and did so originally by accident) by setting the window size to 200x80 or so and running top

References:

Teleport should respond to this message with a failure and allow PuTTY to work using its default configuration with no bugfixes/workarounds on the PuTTY side.

Current behavior: Teleport does not understand the message, reponds with node doesn't support request type 'winadj@putty.projects.tartarus.org' and ultimately closes the connection:

Incoming packet #0x104, type 95 / 0x5f (SSH2_MSG_CHANNEL_EXTENDED_DATA)
  00000000  00 00 01 00 00 00 00 01 00 00 00 47 6e 6f 64 65  ...........Gnode
  00000010  20 64 6f 65 73 6e 27 74 20 73 75 70 70 6f 72 74   doesn't support
  00000020  20 72 65 71 75 65 73 74 20 74 79 70 65 20 27 77   request type 'w
  00000030  69 6e 61 64 6a 40 70 75 74 74 79 2e 70 72 6f 6a  inadj@putty.proj
  00000040  65 63 74 73 2e 74 61 72 74 61 72 75 73 2e 6f 72  ects.tartarus.or
  00000050  67 27 0a                                         g'.
Incoming packet #0x105, type 100 / 0x64 (SSH2_MSG_CHANNEL_FAILURE)
  00000000  00 00 01 00                                      ....
Incoming raw data at 2022-12-13 17:02:08
  00000000  97 d4 45 2d 2c a2 51 a0 c6 ca de 1f ac 9d e3 c9  ..E-,.Q.........
  00000010  06 ea dd c3 f0 19 af bc 06 23 04 2c 8a 73 86 da  .........#.,.s..
  00000020  d0 f0 22 13 0d db c0 a1 c2 aa a9 6c 41 c1 70 63  .."........lA.pc
Incoming packet #0x106, type 97 / 0x61 (SSH2_MSG_CHANNEL_CLOSE)
  00000000  00 00 01 00                                      ....
Event Log: Sent EOF message
Event Log: Main session channel closed
Outgoing packet #0x64, type 96 / 0x60 (SSH2_MSG_CHANNEL_EOF)
  00000000  00 00 00 00                                      ....
Outgoing packet #0x65, type 97 / 0x61 (SSH2_MSG_CHANNEL_CLOSE)
  00000000  00 00 00 00                                      ....
Event Log: All channels closed

This should be fixed in a similar way to #12372, although as it's expected that a failure message is sent in response (rather than simply ignoring the request) it might require a slightly different approach.

Bug details:

  • Teleport version: <=11.1.2
@webvictim webvictim self-assigned this Dec 14, 2022
avatus pushed a commit that referenced this issue Feb 28, 2023
* sshserver: Correctly handle PuTTY winadj channel requests

Fixes #19365

* Actually send a failure

* dispatch changes

* Reimplement wantReply handling

* Also handle windadj in forwarding server

* lint: Fix typo

* lint: Invert return order to put err last

* Use nil return payload instead

* Use trace.ReplyAlreadyHandled instead

* Handle reply internally and set req.WantReply to false

* Use t.Cleanup()

* behaviour is not a misspelling but whatever makes you happy

* Suggestions from code review
webvictim added a commit that referenced this issue Feb 28, 2023
* sshserver: Correctly handle PuTTY winadj channel requests

Fixes #19365

* Actually send a failure

* dispatch changes

* Reimplement wantReply handling

* Also handle windadj in forwarding server

* lint: Fix typo

* lint: Invert return order to put err last

* Use nil return payload instead

* Use trace.ReplyAlreadyHandled instead

* Handle reply internally and set req.WantReply to false

* Use t.Cleanup()

* behaviour is not a misspelling but whatever makes you happy

* Suggestions from code review
webvictim added a commit that referenced this issue Feb 28, 2023
* sshserver: Correctly handle PuTTY winadj channel requests

Fixes #19365

* Actually send a failure

* dispatch changes

* Reimplement wantReply handling

* Also handle windadj in forwarding server

* lint: Fix typo

* lint: Invert return order to put err last

* Use nil return payload instead

* Use trace.ReplyAlreadyHandled instead

* Handle reply internally and set req.WantReply to false

* Use t.Cleanup()

* behaviour is not a misspelling but whatever makes you happy

* Suggestions from code review
webvictim added a commit that referenced this issue Feb 28, 2023
* sshserver: Correctly handle PuTTY winadj channel requests

Fixes #19365

* Actually send a failure

* dispatch changes

* Reimplement wantReply handling

* Also handle windadj in forwarding server

* lint: Fix typo

* lint: Invert return order to put err last

* Use nil return payload instead

* Use trace.ReplyAlreadyHandled instead

* Handle reply internally and set req.WantReply to false

* Use t.Cleanup()

* behaviour is not a misspelling but whatever makes you happy

* Suggestions from code review
webvictim added a commit that referenced this issue Mar 1, 2023
…22421)

* sshserver: Correctly handle PuTTY winadj channel requests

Fixes #19365

* Actually send a failure

* dispatch changes

* Reimplement wantReply handling

* Also handle windadj in forwarding server

* lint: Fix typo

* lint: Invert return order to put err last

* Use nil return payload instead

* Use trace.ReplyAlreadyHandled instead

* Handle reply internally and set req.WantReply to false

* Use t.Cleanup()

* behaviour is not a misspelling but whatever makes you happy

* Suggestions from code review
@webvictim
Copy link
Contributor Author

This issue was fixed and backported. The fix should be out in the following versions when released:

  • v10.3.14
  • v11.3.6
  • v12.0.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant