-
Notifications
You must be signed in to change notification settings - Fork 358
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
SSH_MSG_CHANNEL_EOF is sent after SSH_MSG_CHANNEL_CLOSE #410
Comments
Can you provide code for reproducing this? |
Though maybe not necessary. I think I see where this is coming from: the |
will the below changes fix the issue ? AbstractChannel
AbstractChannel.sendEof() AbstractChannel.handleClose()
|
I don't think so. First, storing that IoWriteFuture in a field is kind of hacky, and second, I have another fix in the works, but I need to see if I can provoke that error reliably so that I can write a unit test for it. |
I think I need code that shows this problem all the same. So far I have not been able to provoke this condition. |
We always call close channel immediately after exitStatus is read. this does not call graceful close and send SSH_MSG_CHANNEL_CLOSE. My understandingof the issue is on reception of peer's SSH_MSG_CHANNEL_CLOSE, handleClose is calling close(false) that sends SSH_MSG_CHANNEL_CLOSE from client
|
I also couldn't reproduce this issue with sample code. this comes only in our application during PSR testing where more than 50k command are executed over ssh. |
I'm getting confused. Who initiates the channel closing when that problem occurs? Client or server? Or both? Is client code closing the output stream explicitly, or does that happen during the graceful channel closing? Is the server also an Apche MINA sshd 2.10.0 server? If not, what is it? |
All right. I have now a test case that reproduces this. It's hyper-ugly test code, and to reproduce the race I had to instrument the code in The race will not be fixed by the proposal you made. That still has the race : first, the field would have to be volatile at least, and second, Fixing this will require a bit more work. I don't think it's solvable with the |
When a channel using a ChannelAsyncOutputStream, do not only wait until all pending data has been written but also wait for the EOF having been sent before proceeding with closing the channel. For channels using a synchronous ChannelOutputStream, no local fix was found. Instead give the channel access to the IoWriteFuture for sending the EOF packet, and if one was sent, ensure that closing the channel proceeds only after the EOF has been sent. Otherwise there may be a race between sending that EOF and sending the CLOSE, and sometimes the EOF might be sent after the CLOSE. Bug: apache#410
When a channel using a ChannelAsyncOutputStream, do not only wait until all pending data has been written but also wait for the EOF having been sent before proceeding with closing the channel. For channels using a synchronous ChannelOutputStream, no local fix was found. Instead give the channel access to the IoWriteFuture for sending the EOF packet, and if one was sent, ensure that closing the channel proceeds only after the EOF has been sent. Otherwise there may be a race between sending that EOF and sending the CLOSE, and sometimes the EOF might be sent after the CLOSE. Bug: apache#410
Do you have any tentative date 2.10.1 release that has the fix? |
No, I don't. The 2.10.1-SNAPSHOT from the Apache Snapshot repository does have the fix. There is no fixed release schedule; the project is maintained by private individuals in their spare time. I think it would be good if we could do a release in October, but I cannot promise anything. |
I'm wondering if there is any update on the release date. |
Thanks for reminding me. I've asked for a new release. |
Is 2.9.3 release only for CVE fixes ? If you are planning to release 2.9.3 before 2.11.0 . would it be possible to include this fix in 2.9.3 ? |
Guillaume just included the CVE fix in 2.9.3. I don't have time to backport this fix and ask for a respin. |
We are using this in a large enterprise product. To fix an issue, we were upgrading the SSH client to 2.9.2, but we found the following problem during PSR testing: https://issues.apache.org/jira/browse/SSHD-1316 This forced us to move to 2.10.0 and also make code changes related to CancelOption in Future calls. Then, we hit a race condition message error. We spent more than 3 months on upgrading sshd client as we were iteratively working on multiple versions and postponing our release a few times. Now, we are facing a lot of pressure from customers to release soon. These bugs are having a significant impact on our product and our customers. I hope you understand our situation. I understand that you are very busy, and I appreciate your time and consideration. I would like to ask if it would be possible to include these 2 bug fixes in the next release of 2.9.3. |
Thanks , I see 2.11.0 version is released now. |
Version
2.10.0
Bug description
when we close channelOutputStream and ClientChannel with in few milli seconds delay. some time sending of SSH_MSG_CHANNEL_EOF message goes after sending of SSH_MSG_CHANNEL_CLOSE message. If this happens then server is responding with "SSH_MSG_DISCONNECT reason=SSH2_DISCONNECT_PROTOCOL_ERROR " , close all channels and the session.
see sequence of all messages sent from channel it 8 and 9 from a session:
the above log shows "sendEof()" is called before channel close() method but the SSH_MSG_CHANNEL_EOF was send after close that cause the session to be closed.
Actual behavior
channel 8 send eof after close message that disconnect the session.
Expected behavior
I guess there should be some synchronization between sending of these 2 messages
Relevant log output
No response
Other information
No response
The text was updated successfully, but these errors were encountered: