| Summary: | SSH2_MSG_CHANNEL_FAILURE on closed channel | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Simon Tatham <anakin> | ||||||||
| Component: | sshd | Assignee: | Assigned to nobody <unassigned-bugs> | ||||||||
| Status: | CLOSED FIXED | ||||||||||
| Severity: | normal | CC: | ahmedsayeed1982, anakin, djm | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 5.1p1 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 2226 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Simon Tatham
2010-09-14 04:03:45 AEST
Created attachment 1925 [details]
putty.log : SSH packet log of failing connection (redacted)
Sorry about that; my packet log was apparently too big for Bugzilla to accept. I've attached a redacted version, produced using the Perl one-liner
perl -ne 'if (/^\S/) { splice @lines, 2, $#lines-3, " ...\n" if $#lines >= 5; print @lines; @lines = (); print; } else { push @lines, $_; }'
and hopefully that still shows the sequence of events (from PuTTY's viewpoint) and all important numbers and details without the huge wodges of irrelevant channel data.
Created attachment 1926 [details]
openssh-channel-failure.diff : proposed patch
Here's a trivial patch which seems to work for me: after I apply this (against last month's snapshot, not 5.1p1) my complex makefile runs to completion without any issues at the SSH level. As I discussed in my comments earlier, it simply suppresses generation of SSH2_MSG_CHANNEL_{SUCCESS,FAILURE} for channels on which SSH2_MSG_CHANNEL_CLOSE has already been sent. (It lacks comments and rationale, though; it's merely a proof of concept.)
Thanks for the detailed report. I don't see anything in RFC 4254 section 5.3 indicating that a channel shouldn't send notifications while it is half-closed. Could you explain how you arrived at this interpretation? Because if you send anything that arrives at the other side when _it_ thinks the channel is fully closed, that's definitely in violation of 5.3. So if you send stuff after you've sent CLOSE, then it _might_ cross in transit with the other side's CLOSE, in which case it would arrive at the other side when that side had already both sent and received CLOSE. I discussed this last night with a friend, and he pointed out that there is an alternative protocol fix which also works, but it's more clearly contradictory of explicit text in the RFC. Instead of ruling that requests received after we send CLOSE may not be responded to, we could instead rule that all requests are responded to and modify section 5.3 to state that a channel number may be reused after you have both sent and received CLOSE _and_ received replies to all outstanding channel requests. However, it's clear that the current situation leads to a problem, so _something_ needs fixing one way or the other. If you don't agree with my analysis, should we take this to ietf-secsh and see if we can get a consensus? Retarget unclosed bugs from 5.7=>5.8 Retarget unresolved bugs/features to 6.0 release Retarget unresolved bugs/features to 6.0 release Retarget unresolved bugs/features to 6.0 release (try again - bugzilla's "change several" isn't) Retarget from 6.0 to 6.1 Retarget 6.0 => 6.1 I've now worked around this issue on the PuTTY side (at least on the current development trunk), by implementing a more robust handling of SSH2_MSG_CHANNEL_CLOSE: _we_ now do not send CHANNEL_CLOSE until we have seen replies to all our outstanding channel requests, and therefore the question of which order the server's close and the request replies appear in is moot. Retarget uncompleted bugs from 6.1 => 6.2 Retarget bugs from 6.1 => 6.2 retarget to openssh-6.3 Retarget to openssh-6.4 Retarget 6.3 -> 6.4 Retarget incomplete bugs / feature requests to 6.6 release Retarget incomplete bugs / feature requests to 6.6 release Retarget to 6.7 release, since 6.6 was mostly bugfixing. Remove from 6.6 tracking bug Created attachment 2431 [details]
improved fix handling both client and server
This issue has come up again recently and is currently under discussion on ietf-ssh. The general consensus seems to be that RFC 4254 has an ambiguity in it and that we should issue a clarification saying that responses to any outstanding or future channel requests should be implied by CHANNEL_CLOSE, and hence no actual reply messages should be sent after sending CHANNEL_CLOSE.
OpenSSH is the only implementation I know of which does not do this (and PuTTY's window size tuning mechanism does cause us to notice...), so could I request higher priority on fixing it, please? I've attached a patch that I think is right.
(I notice that I've attached a patch before, but it only dealt with the server side; this does the same thing on the client side too. I've only been able to test the server-side fix, but the client-side fix matches it as far as I can see so it should be right.)
Patch applied - this will be in OpenSSH 6.7 Close all bugs left open from 6.6 and 6.7 releases. [spam removed] |